lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 22 Dec 2022 09:18:00 -0800 From: Peter Delevoryas <peter@....dev> To: Paolo Abeni <pabeni@...hat.com> Cc: sam@...dozajonas.com, davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, joel@....id.au, gwshan@...ux.vnet.ibm.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH 3/3] net/ncsi: Add NC-SI 1.2 Get MC MAC Address command > On Dec 22, 2022, at 2:53 AM, Paolo Abeni <pabeni@...hat.com> wrote: > > On Tue, 2022-12-20 at 21:22 -0800, Peter Delevoryas wrote: >> 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.2WIP90_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 >> >> IMPORTANT NOTE: >> >> The code I'm submitting here is parsing the MAC addresses as if they are >> transmitted in *reverse* order. >> >> This is different from how every other NC-SI command is parsed in the >> Linux kernel, even though the spec describes the format in the same way >> for every command. >> >> The *reason* for this is that I was able to test this code against the >> new 200G Broadcom NIC, which reports that it supports NC-SI 1.2 in Get >> Version ID and successfully responds to this command. It transmits the >> MAC addresses in reverse byte order. >> >> Nvidia's new 200G NIC doesn't support NC-SI 1.2 yet. I don't know how >> they're planning to implement it. > > All the above looks like a good reason to wait for at least a > stable/documented H/W implementation, before pushing code to the > networking core. I guess that’s a good point. > >> net/ncsi/ncsi-cmd.c | 3 ++- >> net/ncsi/ncsi-manage.c | 9 +++++++-- >> net/ncsi/ncsi-pkt.h | 10 ++++++++++ >> net/ncsi/ncsi-rsp.c | 45 +++++++++++++++++++++++++++++++++++++++++- >> 4 files changed, 63 insertions(+), 4 deletions(-) >> >> diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c >> index dda8b76b7798..7be177f55173 100644 >> --- a/net/ncsi/ncsi-cmd.c >> +++ b/net/ncsi/ncsi-cmd.c >> @@ -269,7 +269,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 f56795769893..bc1887a2543d 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 7a805b86a12d..28a042688d0b 100644 >> --- a/net/ncsi/ncsi-rsp.c >> +++ b/net/ncsi/ncsi-rsp.c >> @@ -1140,6 +1140,48 @@ 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; >> + int j; >> + >> + rsp = (struct ncsi_rsp_gmcma_pkt *)skb_network_header(nr->rsp); >> + saddr.sa_family = ndev->type; >> + ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE; >> + >> + netdev_warn(ndev, "NCSI: Received %d provisioned MAC addresses\n", >> + rsp->address_count); >> + for (i = 0; i < rsp->address_count; i++) { >> + netdev_warn(ndev, "NCSI: MAC address %d: " >> + "%02x:%02x:%02x:%02x:%02x:%02x\n", i, >> + rsp->addresses[i][5], rsp->addresses[i][4], >> + rsp->addresses[i][3], rsp->addresses[i][2], >> + rsp->addresses[i][1], rsp->addresses[i][0]); >> + } > > You must avoid this kind of debug messages on 'warn' level (more > below). You could consider pr_debug() instead or completely drop the > message. Oh ok, I’ll change it to a debug message when I resubmit it. Thanks for your comments! > > Cheers, > > Paolo >
Powered by blists - more mailing lists