| Message ID | 20231205234843.4013767-3-patrick@stwcx.xyz |
|---|---|
| State | New |
| Headers | show |
| Series | [dev-5.6,1/3] net/ncsi: Simplify Kconfig/dts control flow | expand |
On Tue, 2023-12-05 at 17:48 -0600, Patrick Williams wrote: > From: Peter Delevoryas <peter@pjd.dev> > > This change adds support for the NC-SI 1.2 Get MC MAC Address > command, > specified here: > > https://www.dmtf.org/sites/default/files/standards/documents/DSP0222_1.2.0.pdf > > It serves the exact same function as the existing OEM Get MAC Address > commands, so if a channel reports that it supports NC-SI 1.2, we > prefer > to use the standard command rather than the OEM command. > > Verified with an invalid MAC address and 2 valid ones: > > [ 55.137072] ftgmac100 1e690000.ftgmac eth0: NCSI: Received 3 > provisioned MAC addresses > [ 55.137614] ftgmac100 1e690000.ftgmac eth0: NCSI: MAC address 0: > 00:00:00:00:00:00 > [ 55.138026] ftgmac100 1e690000.ftgmac eth0: NCSI: MAC address 1: > fa:ce:b0:0c:20:22 > [ 55.138528] ftgmac100 1e690000.ftgmac eth0: NCSI: MAC address 2: > fa:ce:b0:0c:20:23 > [ 55.139241] ftgmac100 1e690000.ftgmac eth0: NCSI: Unable to assign > 00:00:00:00:00:00 to device > [ 55.140098] ftgmac100 1e690000.ftgmac eth0: NCSI: Set MAC address > to fa:ce:b0:0c:20:22 > > Signed-off-by: Peter Delevoryas <peter@pjd.dev> > Signed-off-by: Patrick Williams <patrick@stwcx.xyz> > Signed-off-by: David S. Miller <davem@davemloft.net> > (cherry picked from commit b8291cf3d1180b5b61299922f17c9441616a805a) > --- > net/ncsi/ncsi-cmd.c | 3 ++- > net/ncsi/ncsi-manage.c | 9 +++++++-- > net/ncsi/ncsi-pkt.h | 10 ++++++++++ > net/ncsi/ncsi-rsp.c | 41 > ++++++++++++++++++++++++++++++++++++++++- > 4 files changed, 59 insertions(+), 4 deletions(-) > > diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c > index fd2236ee9a79..b3ff37a181d7 100644 > --- a/net/ncsi/ncsi-cmd.c > +++ b/net/ncsi/ncsi-cmd.c > @@ -270,7 +270,8 @@ static struct ncsi_cmd_handler { > { NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default }, > { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem }, > { NCSI_PKT_CMD_PLDM, 0, NULL }, > - { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default } > + { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }, > + { NCSI_PKT_CMD_GMCMA, 0, ncsi_cmd_handler_default } > }; > > static struct ncsi_request *ncsi_alloc_command(struct ncsi_cmd_arg > *nca) > diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c > index f3d7fe86fea1..745c788f1d1d 100644 > --- a/net/ncsi/ncsi-manage.c > +++ b/net/ncsi/ncsi-manage.c > @@ -1038,11 +1038,16 @@ static void ncsi_configure_channel(struct > ncsi_dev_priv *ndp) > case ncsi_dev_state_config_oem_gma: > nd->state = ncsi_dev_state_config_clear_vids; > > - nca.type = NCSI_PKT_CMD_OEM; > nca.package = np->id; > nca.channel = nc->id; > ndp->pending_req_num = 1; > - ret = ncsi_gma_handler(&nca, nc->version.mf_id); > + if (nc->version.major >= 1 && nc->version.minor >= > 2) { > + nca.type = NCSI_PKT_CMD_GMCMA; > + ret = ncsi_xmit_cmd(&nca); > + } else { > + nca.type = NCSI_PKT_CMD_OEM; > + ret = ncsi_gma_handler(&nca, nc- > >version.mf_id); > + } > if (ret < 0) > schedule_work(&ndp->work); > > diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h > index c9d1da34dc4d..f2f3b5c1b941 100644 > --- a/net/ncsi/ncsi-pkt.h > +++ b/net/ncsi/ncsi-pkt.h > @@ -338,6 +338,14 @@ struct ncsi_rsp_gpuuid_pkt { > __be32 checksum; > }; > > +/* Get MC MAC Address */ > +struct ncsi_rsp_gmcma_pkt { > + struct ncsi_rsp_pkt_hdr rsp; > + unsigned char address_count; > + unsigned char reserved[3]; > + unsigned char addresses[][ETH_ALEN]; > +}; > + > /* AEN: Link State Change */ > struct ncsi_aen_lsc_pkt { > struct ncsi_aen_pkt_hdr aen; /* AEN header */ > @@ -398,6 +406,7 @@ struct ncsi_aen_hncdsc_pkt { > #define NCSI_PKT_CMD_GPUUID 0x52 /* Get package > UUID */ > #define NCSI_PKT_CMD_QPNPR 0x56 /* Query Pending NC PLDM > request */ > #define NCSI_PKT_CMD_SNPR 0x57 /* Send NC PLDM Reply */ > +#define NCSI_PKT_CMD_GMCMA 0x58 /* Get MC MAC Address */ > > > /* NCSI packet responses */ > @@ -433,6 +442,7 @@ struct ncsi_aen_hncdsc_pkt { > #define NCSI_PKT_RSP_GPUUID (NCSI_PKT_CMD_GPUUID + 0x80) > #define NCSI_PKT_RSP_QPNPR (NCSI_PKT_CMD_QPNPR + 0x80) > #define NCSI_PKT_RSP_SNPR (NCSI_PKT_CMD_SNPR + 0x80) > +#define NCSI_PKT_RSP_GMCMA (NCSI_PKT_CMD_GMCMA + 0x80) > > /* NCSI response code/reason */ > #define NCSI_PKT_RSP_C_COMPLETED 0x0000 /* Command > Completed */ > diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c > index 480e80e3c283..bee290d0f48b 100644 > --- a/net/ncsi/ncsi-rsp.c > +++ b/net/ncsi/ncsi-rsp.c > @@ -1091,6 +1091,44 @@ static int ncsi_rsp_handler_netlink(struct > ncsi_request *nr) > return ret; > } > > +static int ncsi_rsp_handler_gmcma(struct ncsi_request *nr) > +{ > + struct ncsi_dev_priv *ndp = nr->ndp; > + struct net_device *ndev = ndp->ndev.dev; > + struct ncsi_rsp_gmcma_pkt *rsp; > + struct sockaddr saddr; > + int ret = -1; > + int i; > + > + rsp = (struct ncsi_rsp_gmcma_pkt *)skb_network_header(nr- > >rsp); > + saddr.sa_family = ndev->type; > + ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE; > + > + netdev_info(ndev, "NCSI: Received %d provisioned MAC > addresses\n", > + rsp->address_count); > + for (i = 0; i < rsp->address_count; i++) { > + netdev_info(ndev, "NCSI: MAC address %d: > %02x:%02x:%02x:%02x:%02x:%02x\n", > + i, rsp->addresses[i][0], rsp- > >addresses[i][1], > + rsp->addresses[i][2], rsp- > >addresses[i][3], > + rsp->addresses[i][4], rsp- > >addresses[i][5]); > + } > + > + for (i = 0; i < rsp->address_count; i++) { > + memcpy(saddr.sa_data, &rsp->addresses[i], ETH_ALEN); > + ret = ndev->netdev_ops->ndo_set_mac_address(ndev, > &saddr); > + if (ret < 0) { > + netdev_warn(ndev, "NCSI: Unable to assign > %pM to device\n", > + saddr.sa_data); > + continue; > + } > + netdev_warn(ndev, "NCSI: Set MAC address to %pM\n", > saddr.sa_data); > + break; > + } > + > + ndp->gma_flag = ret == 0; > + return ret; > +} > + > static struct ncsi_rsp_handler { > unsigned char type; > int payload; > @@ -1127,7 +1165,8 @@ static struct ncsi_rsp_handler { > { NCSI_PKT_RSP_PLDM, -1, ncsi_rsp_handler_pldm }, > { NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid }, > { NCSI_PKT_RSP_QPNPR, -1, ncsi_rsp_handler_pldm }, > - { NCSI_PKT_RSP_SNPR, -1, ncsi_rsp_handler_pldm } > + { NCSI_PKT_RSP_SNPR, -1, ncsi_rsp_handler_pldm }, > + { NCSI_PKT_RSP_GMCMA, -1, ncsi_rsp_handler_gmcma }, > }; > > int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev, Patrick, I've the fix about ndo_set_mac_address not so long in the past https://lore.kernel.org/all/20230828101151.684010399@linuxfoundation.org/ ndo_set_mac_address do not notify network layer about mac change. Thanks.
On Thu, Dec 07, 2023 at 12:23:38AM +0300, Ivan Mikhaylov wrote: > > > Patrick, I've the fix about ndo_set_mac_address not so long in the past > https://lore.kernel.org/all/20230828101151.684010399@linuxfoundation.org/ > > ndo_set_mac_address do not notify network layer about mac change. Hello Ivan, I think you're suggesting there is a bug in the code that was applied to net-next here? If so, we'll need to get a fix into net-next. These commits are just a backport request to the OpenBMC tree of the code that was already applied to net-next.
On Wed, 2023-12-06 at 22:17 -0600, Patrick Williams wrote: > On Thu, Dec 07, 2023 at 12:23:38AM +0300, Ivan Mikhaylov wrote: > > > > > > Patrick, I've the fix about ndo_set_mac_address not so long in the > > past > > https://lore.kernel.org/all/20230828101151.684010399@linuxfoundation.org/ > > > > ndo_set_mac_address do not notify network layer about mac change. > > Hello Ivan, > > I think you're suggesting there is a bug in the code that was applied > to > net-next here? If so, we'll need to get a fix into net-next. These > commits are just a backport request to the OpenBMC tree of the code > that > was already applied to net-next. > Patrick, yes, there is a bug, I'll write to the thread today/tomorrow with that commit about that problem. Need to think how to make a fix for this problem, reverting and make it right until it in net-next or fix above that commit. Thanks.
On Thu, Dec 07, 2023 at 10:44:05AM +0300, Ivan Mikhaylov wrote: > On Wed, 2023-12-06 at 22:17 -0600, Patrick Williams wrote: > > On Thu, Dec 07, 2023 at 12:23:38AM +0300, Ivan Mikhaylov wrote: > > > > > > > > > Patrick, I've the fix about ndo_set_mac_address not so long in the > > > past > > > https://lore.kernel.org/all/20230828101151.684010399@linuxfoundation.org/ > > > > > > ndo_set_mac_address do not notify network layer about mac change. > > > > Hello Ivan, > > > > I think you're suggesting there is a bug in the code that was applied > > to > > net-next here? If so, we'll need to get a fix into net-next. These > > commits are just a backport request to the OpenBMC tree of the code > > that > > was already applied to net-next. > > > > Patrick, yes, there is a bug, I'll write to the thread today/tomorrow > with that commit about that problem. Need to think how to make a fix > for this problem, reverting and make it right until it in net-next or > fix above that commit. Is this the fix? diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c index bee290d0f48b..b02e663e56dc 100644 --- a/net/ncsi/ncsi-rsp.c +++ b/net/ncsi/ncsi-rsp.c @@ -1115,7 +1115,9 @@ static int ncsi_rsp_handler_gmcma(struct ncsi_request *nr) for (i = 0; i < rsp->address_count; i++) { memcpy(saddr.sa_data, &rsp->addresses[i], ETH_ALEN); - ret = ndev->netdev_ops->ndo_set_mac_address(ndev, &saddr); + rtnl_lock(); + ret = dev_set_mac_address(ndev, &saddr, NULL); + rtnl_unlock(); if (ret < 0) { netdev_warn(ndev, "NCSI: Unable to assign %pM to device\n", saddr.sa_data);
On Thu, 2023-12-07 at 06:16 -0600, Patrick Williams wrote: > On Thu, Dec 07, 2023 at 10:44:05AM +0300, Ivan Mikhaylov wrote: > > On Wed, 2023-12-06 at 22:17 -0600, Patrick Williams wrote: > > > On Thu, Dec 07, 2023 at 12:23:38AM +0300, Ivan Mikhaylov wrote: > > > > > > > > > > > > Patrick, I've the fix about ndo_set_mac_address not so long in > > > > the > > > > past > > > > https://lore.kernel.org/all/20230828101151.684010399@linuxfoundation.org/ > > > > > > > > ndo_set_mac_address do not notify network layer about mac > > > > change. > > > > > > Hello Ivan, > > > > > > I think you're suggesting there is a bug in the code that was > > > applied > > > to > > > net-next here? If so, we'll need to get a fix into net-next. > > > These > > > commits are just a backport request to the OpenBMC tree of the > > > code > > > that > > > was already applied to net-next. > > > > > > > Patrick, yes, there is a bug, I'll write to the thread > > today/tomorrow > > with that commit about that problem. Need to think how to make a > > fix > > for this problem, reverting and make it right until it in net-next > > or > > fix above that commit. > > Is this the fix? > > diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c > index bee290d0f48b..b02e663e56dc 100644 > --- a/net/ncsi/ncsi-rsp.c > +++ b/net/ncsi/ncsi-rsp.c > @@ -1115,7 +1115,9 @@ static int ncsi_rsp_handler_gmcma(struct > ncsi_request *nr) > > for (i = 0; i < rsp->address_count; i++) { > memcpy(saddr.sa_data, &rsp->addresses[i], ETH_ALEN); > - ret = ndev->netdev_ops->ndo_set_mac_address(ndev, > &saddr); > + rtnl_lock(); > + ret = dev_set_mac_address(ndev, &saddr, NULL); > + rtnl_unlock(); > if (ret < 0) { > netdev_warn(ndev, "NCSI: Unable to assign %pM > to device\n", > saddr.sa_data); > Patrick, yes, this is the fix. Difference in calling of call_netdevice_notifiers from dev_set_mac_address to make sure that everybody aware about mac change which using this interface. Thanks.
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c index fd2236ee9a79..b3ff37a181d7 100644 --- a/net/ncsi/ncsi-cmd.c +++ b/net/ncsi/ncsi-cmd.c @@ -270,7 +270,8 @@ static struct ncsi_cmd_handler { { NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default }, { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem }, { NCSI_PKT_CMD_PLDM, 0, NULL }, - { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default } + { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }, + { NCSI_PKT_CMD_GMCMA, 0, ncsi_cmd_handler_default } }; static struct ncsi_request *ncsi_alloc_command(struct ncsi_cmd_arg *nca) diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c index f3d7fe86fea1..745c788f1d1d 100644 --- a/net/ncsi/ncsi-manage.c +++ b/net/ncsi/ncsi-manage.c @@ -1038,11 +1038,16 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) case ncsi_dev_state_config_oem_gma: nd->state = ncsi_dev_state_config_clear_vids; - nca.type = NCSI_PKT_CMD_OEM; nca.package = np->id; nca.channel = nc->id; ndp->pending_req_num = 1; - ret = ncsi_gma_handler(&nca, nc->version.mf_id); + if (nc->version.major >= 1 && nc->version.minor >= 2) { + nca.type = NCSI_PKT_CMD_GMCMA; + ret = ncsi_xmit_cmd(&nca); + } else { + nca.type = NCSI_PKT_CMD_OEM; + ret = ncsi_gma_handler(&nca, nc->version.mf_id); + } if (ret < 0) schedule_work(&ndp->work); diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h index c9d1da34dc4d..f2f3b5c1b941 100644 --- a/net/ncsi/ncsi-pkt.h +++ b/net/ncsi/ncsi-pkt.h @@ -338,6 +338,14 @@ struct ncsi_rsp_gpuuid_pkt { __be32 checksum; }; +/* Get MC MAC Address */ +struct ncsi_rsp_gmcma_pkt { + struct ncsi_rsp_pkt_hdr rsp; + unsigned char address_count; + unsigned char reserved[3]; + unsigned char addresses[][ETH_ALEN]; +}; + /* AEN: Link State Change */ struct ncsi_aen_lsc_pkt { struct ncsi_aen_pkt_hdr aen; /* AEN header */ @@ -398,6 +406,7 @@ struct ncsi_aen_hncdsc_pkt { #define NCSI_PKT_CMD_GPUUID 0x52 /* Get package UUID */ #define NCSI_PKT_CMD_QPNPR 0x56 /* Query Pending NC PLDM request */ #define NCSI_PKT_CMD_SNPR 0x57 /* Send NC PLDM Reply */ +#define NCSI_PKT_CMD_GMCMA 0x58 /* Get MC MAC Address */ /* NCSI packet responses */ @@ -433,6 +442,7 @@ struct ncsi_aen_hncdsc_pkt { #define NCSI_PKT_RSP_GPUUID (NCSI_PKT_CMD_GPUUID + 0x80) #define NCSI_PKT_RSP_QPNPR (NCSI_PKT_CMD_QPNPR + 0x80) #define NCSI_PKT_RSP_SNPR (NCSI_PKT_CMD_SNPR + 0x80) +#define NCSI_PKT_RSP_GMCMA (NCSI_PKT_CMD_GMCMA + 0x80) /* NCSI response code/reason */ #define NCSI_PKT_RSP_C_COMPLETED 0x0000 /* Command Completed */ diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c index 480e80e3c283..bee290d0f48b 100644 --- a/net/ncsi/ncsi-rsp.c +++ b/net/ncsi/ncsi-rsp.c @@ -1091,6 +1091,44 @@ static int ncsi_rsp_handler_netlink(struct ncsi_request *nr) return ret; } +static int ncsi_rsp_handler_gmcma(struct ncsi_request *nr) +{ + struct ncsi_dev_priv *ndp = nr->ndp; + struct net_device *ndev = ndp->ndev.dev; + struct ncsi_rsp_gmcma_pkt *rsp; + struct sockaddr saddr; + int ret = -1; + int i; + + rsp = (struct ncsi_rsp_gmcma_pkt *)skb_network_header(nr->rsp); + saddr.sa_family = ndev->type; + ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE; + + netdev_info(ndev, "NCSI: Received %d provisioned MAC addresses\n", + rsp->address_count); + for (i = 0; i < rsp->address_count; i++) { + netdev_info(ndev, "NCSI: MAC address %d: %02x:%02x:%02x:%02x:%02x:%02x\n", + i, rsp->addresses[i][0], rsp->addresses[i][1], + rsp->addresses[i][2], rsp->addresses[i][3], + rsp->addresses[i][4], rsp->addresses[i][5]); + } + + for (i = 0; i < rsp->address_count; i++) { + memcpy(saddr.sa_data, &rsp->addresses[i], ETH_ALEN); + ret = ndev->netdev_ops->ndo_set_mac_address(ndev, &saddr); + if (ret < 0) { + netdev_warn(ndev, "NCSI: Unable to assign %pM to device\n", + saddr.sa_data); + continue; + } + netdev_warn(ndev, "NCSI: Set MAC address to %pM\n", saddr.sa_data); + break; + } + + ndp->gma_flag = ret == 0; + return ret; +} + static struct ncsi_rsp_handler { unsigned char type; int payload; @@ -1127,7 +1165,8 @@ static struct ncsi_rsp_handler { { NCSI_PKT_RSP_PLDM, -1, ncsi_rsp_handler_pldm }, { NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid }, { NCSI_PKT_RSP_QPNPR, -1, ncsi_rsp_handler_pldm }, - { NCSI_PKT_RSP_SNPR, -1, ncsi_rsp_handler_pldm } + { NCSI_PKT_RSP_SNPR, -1, ncsi_rsp_handler_pldm }, + { NCSI_PKT_RSP_GMCMA, -1, ncsi_rsp_handler_gmcma }, }; int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,