| Message ID | 20180925000840.4111212-1-vijaykhemka@fb.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | net/ncsi: Add NCSI OEM command for FB Tiogapass | expand |
Hi,
I am sending this patch to mailing list only not maintainer as I need feedback on this patch before I ask for upstream in kernel. Below is the background of problem.
We are using 2 different cards Mellanox and Broadcom for our Tiogapass BMC. We have a requirement of getting mac address from card itself by sending OEM command and then set it to device. We get these mac address as a part of NCSI configuration and set once received response. I have tried to make code as generic as possible. I need your feedback on this patch.
Regards
-Vijay
On 9/24/18, 5:09 PM, "Vijay Khemka" <vijaykhemka@fb.com> wrote:
This patch adds OEM command to get mac address from NCSI device and and
configure the same to the network card.
ncsi_cmd_arg - Modified this structure to include bigger payload data.
ncsi_cmd_handler_oem: This function handles oem command request
ncsi_rsp_handler_oem: This function handles response for OEM command.
get_mac_address_oem_mlx: This function will send OEM command to get
mac address for Mellanox card
set_mac_affinity_mlx: This will send OEM command to set Mac affinity
for Mellanox card
Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
net/ncsi/Kconfig | 3 ++
net/ncsi/internal.h | 11 +++++--
net/ncsi/ncsi-cmd.c | 24 +++++++++++++--
net/ncsi/ncsi-manage.c | 68 ++++++++++++++++++++++++++++++++++++++++++
net/ncsi/ncsi-pkt.h | 16 ++++++++++
net/ncsi/ncsi-rsp.c | 33 +++++++++++++++++++-
6 files changed, 149 insertions(+), 6 deletions(-)
diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig
index 08a8a6031fd7..b8bf89fea7c8 100644
--- a/net/ncsi/Kconfig
+++ b/net/ncsi/Kconfig
@@ -10,3 +10,6 @@ config NET_NCSI
support. Enable this only if your system connects to a network
device via NCSI and the ethernet driver you're using supports
the protocol explicitly.
+config NCSI_OEM_CMD_GET_MAC
+ bool "Get NCSI OEM MAC Address"
+ depends on NET_NCSI
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 8055e3965cef..da17958e6a4b 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -68,6 +68,10 @@ enum {
NCSI_MODE_MAX
};
+#define NCSI_OEM_MFR_MLX_ID 0x8119
+#define NCSI_OEM_MLX_CMD_GET_MAC 0x1b00
+#define NCSI_OEM_MLX_CMD_SET_AFFINITY 0x010700
+
struct ncsi_channel_version {
u32 version; /* Supported BCD encoded NCSI version */
u32 alpha2; /* Supported BCD encoded NCSI version */
@@ -236,6 +240,7 @@ enum {
ncsi_dev_state_probe_dp,
ncsi_dev_state_config_sp = 0x0301,
ncsi_dev_state_config_cis,
+ ncsi_dev_state_config_oem_gma,
ncsi_dev_state_config_clear_vids,
ncsi_dev_state_config_svf,
ncsi_dev_state_config_ev,
@@ -301,9 +306,9 @@ struct ncsi_cmd_arg {
unsigned short payload; /* Command packet payload length */
unsigned int req_flags; /* NCSI request properties */
union {
- unsigned char bytes[16]; /* Command packet specific data */
- unsigned short words[8];
- unsigned int dwords[4];
+ unsigned char bytes[64]; /* Command packet specific data */
+ unsigned short words[32];
+ unsigned int dwords[16];
};
};
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 7567ca63aae2..3205e22c1734 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -211,6 +211,25 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb,
return 0;
}
+static int ncsi_cmd_handler_oem(struct sk_buff *skb,
+ struct ncsi_cmd_arg *nca)
+{
+ struct ncsi_cmd_oem_pkt *cmd;
+ unsigned int len;
+
+ len = sizeof(struct ncsi_cmd_pkt_hdr) + 4;
+ if (nca->payload < 26)
+ len += 26;
+ else
+ len += nca->payload;
+
+ cmd = skb_put_zero(skb, len);
+ memcpy(cmd->data, nca->bytes, nca->payload);
+ ncsi_cmd_build_header(&cmd->cmd.common, nca);
+
+ return 0;
+}
+
static struct ncsi_cmd_handler {
unsigned char type;
int payload;
@@ -244,7 +263,7 @@ static struct ncsi_cmd_handler {
{ NCSI_PKT_CMD_GNS, 0, ncsi_cmd_handler_default },
{ NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default },
{ NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_OEM, 0, NULL },
+ { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem },
{ NCSI_PKT_CMD_PLDM, 0, NULL },
{ NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }
};
@@ -317,7 +336,8 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
}
/* Get packet payload length and allocate the request */
- nca->payload = nch->payload;
+ if (nch->payload >= 0)
+ nca->payload = nch->payload;
nr = ncsi_alloc_command(nca);
if (!nr)
return -ENOMEM;
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 091284760d21..3b2b86560cc8 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -635,6 +635,58 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
return 0;
}
+#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
+/* NCSI Facebook OEM APIs */
+static void get_mac_address_oem_mlx(struct ncsi_dev_priv *ndp)
+{
+ struct ncsi_cmd_arg nca;
+ int ret = 0;
+
+ memset(&nca, 0, sizeof(struct ncsi_cmd_arg));
+ nca.ndp = ndp;
+ nca.channel = ndp->active_channel->id;
+ nca.package = ndp->active_package->id;
+ nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
+ nca.type = NCSI_PKT_CMD_OEM;
+ nca.payload = 8;
+
+ nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID);
+ nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_GET_MAC);
+
+ ret = ncsi_xmit_cmd(&nca);
+ if (ret)
+ netdev_err(ndp->ndev.dev,
+ "NCSI: Failed to transmit cmd 0x%x during probe\n",
+ nca.type);
+}
+
+static void set_mac_affinity_mlx(struct ncsi_dev_priv *ndp)
+{
+ struct ncsi_cmd_arg nca;
+ int ret = 0;
+
+ memset(&nca, 0, sizeof(struct ncsi_cmd_arg));
+ nca.ndp = ndp;
+ nca.channel = ndp->active_channel->id;
+ nca.package = ndp->active_package->id;
+ nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
+ nca.type = NCSI_PKT_CMD_OEM;
+ nca.payload = 60;
+
+ nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID);
+ nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_SET_AFFINITY);
+
+ memcpy(&(nca.bytes[8]), ndp->ndev.dev->dev_addr, ETH_ALEN);
+ nca.bytes[14] = 0x09;
+
+ ret = ncsi_xmit_cmd(&nca);
+ if (ret)
+ netdev_err(ndp->ndev.dev,
+ "NCSI: Failed to transmit cmd 0x%x during probe\n",
+ nca.type);
+}
+#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
+
static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
{
struct ncsi_dev *nd = &ndp->ndev;
@@ -685,6 +737,22 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
goto error;
}
+#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
+ /* Check Manufacture id if it is Mellanox then
+ * get and set mac address. To Do: Add code for
+ * other types of card if required
+ */
+ if (nc->version.mf_id == NCSI_OEM_MFR_MLX_ID)
+ nd->state = ncsi_dev_state_config_oem_gma;
+ else
+ nd->state = ncsi_dev_state_config_clear_vids;
+ break;
+ case ncsi_dev_state_config_oem_gma:
+ ndp->pending_req_num = 2;
+ get_mac_address_oem_mlx(ndp);
+ msleep(500);
+ set_mac_affinity_mlx(ndp);
+#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
nd->state = ncsi_dev_state_config_clear_vids;
break;
case ncsi_dev_state_config_clear_vids:
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 91b4b66438df..0653a893eb12 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -151,6 +151,22 @@ struct ncsi_cmd_snfc_pkt {
unsigned char pad[22];
};
+/* Oem Request Command */
+struct ncsi_cmd_oem_pkt {
+ struct ncsi_cmd_pkt_hdr cmd; /* Command header */
+ unsigned char data[64]; /* OEM Payload Data */
+ __be32 checksum; /* Checksum */
+};
+
+/* Oem Response Packet */
+struct ncsi_rsp_oem_pkt {
+ struct ncsi_rsp_pkt_hdr rsp; /* Command header */
+ __be32 mfr_id; /* Manufacture ID */
+ __be32 oem_cmd; /* oem command */
+ unsigned char data[32]; /* Payload data */
+ __be32 checksum; /* Checksum */
+};
+
/* Get Link Status */
struct ncsi_rsp_gls_pkt {
struct ncsi_rsp_pkt_hdr rsp; /* Response header */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 930c1d3796f0..3b94c96b9c7f 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -596,6 +596,37 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
return 0;
}
+static int ncsi_rsp_handler_oem(struct ncsi_request *nr)
+{
+ struct ncsi_rsp_oem_pkt *rsp;
+ struct ncsi_dev_priv *ndp = nr->ndp;
+ struct net_device *ndev = ndp->ndev.dev;
+ int ret = 0;
+ unsigned int oem_cmd, mfr_id;
+ const struct net_device_ops *ops = ndev->netdev_ops;
+ struct sockaddr saddr;
+
+ /* Get the response header */
+ rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+
+ oem_cmd = ntohl(rsp->oem_cmd);
+ mfr_id = ntohl(rsp->mfr_id);
+
+ /* Check for Mellanox manufacturer id */
+ if (mfr_id != NCSI_OEM_MFR_MLX_ID)
+ return 0;
+
+ if (oem_cmd == NCSI_OEM_MLX_CMD_GET_MAC) {
+ saddr.sa_family = ndev->type;
+ ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+ memcpy(saddr.sa_data, &(rsp->data[4]), ETH_ALEN);
+ ret = ops->ndo_set_mac_address(ndev, &saddr);
+ if (ret < 0)
+ netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n");
+ }
+ return ret;
+}
+
static int ncsi_rsp_handler_gvi(struct ncsi_request *nr)
{
struct ncsi_rsp_gvi_pkt *rsp;
@@ -932,7 +963,7 @@ static struct ncsi_rsp_handler {
{ NCSI_PKT_RSP_GNS, 172, ncsi_rsp_handler_gns },
{ NCSI_PKT_RSP_GNPTS, 172, ncsi_rsp_handler_gnpts },
{ NCSI_PKT_RSP_GPS, 8, ncsi_rsp_handler_gps },
- { NCSI_PKT_RSP_OEM, 0, NULL },
+ { NCSI_PKT_RSP_OEM, -1, ncsi_rsp_handler_oem },
{ NCSI_PKT_RSP_PLDM, 0, NULL },
{ NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid }
};
--
2.17.1
Hi Vijay, On Tue, 25 Sep 2018 at 09:39, Vijay Khemka <vijaykhemka@fb.com> wrote: > > This patch adds OEM command to get mac address from NCSI device and and > configure the same to the network card. > > ncsi_cmd_arg - Modified this structure to include bigger payload data. > ncsi_cmd_handler_oem: This function handes oem command request > ncsi_rsp_handler_oem: This function handles response for OEM command. > get_mac_address_oem_mlx: This function will send OEM command to get > mac address for Mellanox card > set_mac_affinity_mlx: This will send OEM command to set Mac affinity > for Mellanox card Thanks for the patch. The code looks okay, but I wanted to get some input from our NCSI maintainer as to how OEM commands would be structured. Sam, can you please provide some review here? Is the command supported on all melanox cards, just some, or does TiogaPass have a special firmware that enables it? We should include the netdev mailing list in this discussion as the patch needs to be acceptable for upstream. Cheers, Joel > > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com> > --- > net/ncsi/Kconfig | 3 ++ > net/ncsi/internal.h | 11 +++++-- > net/ncsi/ncsi-cmd.c | 24 +++++++++++++-- > net/ncsi/ncsi-manage.c | 68 ++++++++++++++++++++++++++++++++++++++++++ > net/ncsi/ncsi-pkt.h | 16 ++++++++++ > net/ncsi/ncsi-rsp.c | 33 +++++++++++++++++++- > 6 files changed, 149 insertions(+), 6 deletions(-) > > diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig > index 08a8a6031fd7..b8bf89fea7c8 100644 > --- a/net/ncsi/Kconfig > +++ b/net/ncsi/Kconfig > @@ -10,3 +10,6 @@ config NET_NCSI > support. Enable this only if your system connects to a network > device via NCSI and the ethernet driver you're using supports > the protocol explicitly. > +config NCSI_OEM_CMD_GET_MAC > + bool "Get NCSI OEM MAC Address" > + depends on NET_NCSI > diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h > index 8055e3965cef..da17958e6a4b 100644 > --- a/net/ncsi/internal.h > +++ b/net/ncsi/internal.h > @@ -68,6 +68,10 @@ enum { > NCSI_MODE_MAX > }; > > +#define NCSI_OEM_MFR_MLX_ID 0x8119 > +#define NCSI_OEM_MLX_CMD_GET_MAC 0x1b00 > +#define NCSI_OEM_MLX_CMD_SET_AFFINITY 0x010700 > + > struct ncsi_channel_version { > u32 version; /* Supported BCD encoded NCSI version */ > u32 alpha2; /* Supported BCD encoded NCSI version */ > @@ -236,6 +240,7 @@ enum { > ncsi_dev_state_probe_dp, > ncsi_dev_state_config_sp = 0x0301, > ncsi_dev_state_config_cis, > + ncsi_dev_state_config_oem_gma, > ncsi_dev_state_config_clear_vids, > ncsi_dev_state_config_svf, > ncsi_dev_state_config_ev, > @@ -301,9 +306,9 @@ struct ncsi_cmd_arg { > unsigned short payload; /* Command packet payload length */ > unsigned int req_flags; /* NCSI request properties */ > union { > - unsigned char bytes[16]; /* Command packet specific data */ > - unsigned short words[8]; > - unsigned int dwords[4]; > + unsigned char bytes[64]; /* Command packet specific data */ > + unsigned short words[32]; > + unsigned int dwords[16]; > }; > }; > > diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c > index 7567ca63aae2..3205e22c1734 100644 > --- a/net/ncsi/ncsi-cmd.c > +++ b/net/ncsi/ncsi-cmd.c > @@ -211,6 +211,25 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb, > return 0; > } > > +static int ncsi_cmd_handler_oem(struct sk_buff *skb, > + struct ncsi_cmd_arg *nca) > +{ > + struct ncsi_cmd_oem_pkt *cmd; > + unsigned int len; > + > + len = sizeof(struct ncsi_cmd_pkt_hdr) + 4; > + if (nca->payload < 26) > + len += 26; > + else > + len += nca->payload; > + > + cmd = skb_put_zero(skb, len); > + memcpy(cmd->data, nca->bytes, nca->payload); > + ncsi_cmd_build_header(&cmd->cmd.common, nca); > + > + return 0; > +} > + > static struct ncsi_cmd_handler { > unsigned char type; > int payload; > @@ -244,7 +263,7 @@ static struct ncsi_cmd_handler { > { NCSI_PKT_CMD_GNS, 0, ncsi_cmd_handler_default }, > { NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default }, > { NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default }, > - { NCSI_PKT_CMD_OEM, 0, NULL }, > + { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem }, > { NCSI_PKT_CMD_PLDM, 0, NULL }, > { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default } > }; > @@ -317,7 +336,8 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca) > } > > /* Get packet payload length and allocate the request */ > - nca->payload = nch->payload; > + if (nch->payload >= 0) > + nca->payload = nch->payload; > nr = ncsi_alloc_command(nca); > if (!nr) > return -ENOMEM; > diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c > index 091284760d21..3b2b86560cc8 100644 > --- a/net/ncsi/ncsi-manage.c > +++ b/net/ncsi/ncsi-manage.c > @@ -635,6 +635,58 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc, > return 0; > } > > +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC) > +/* NCSI Facebook OEM APIs */ > +static void get_mac_address_oem_mlx(struct ncsi_dev_priv *ndp) > +{ > + struct ncsi_cmd_arg nca; > + int ret = 0; > + > + memset(&nca, 0, sizeof(struct ncsi_cmd_arg)); > + nca.ndp = ndp; > + nca.channel = ndp->active_channel->id; > + nca.package = ndp->active_package->id; > + nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN; > + nca.type = NCSI_PKT_CMD_OEM; > + nca.payload = 8; > + > + nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID); > + nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_GET_MAC); > + > + ret = ncsi_xmit_cmd(&nca); > + if (ret) > + netdev_err(ndp->ndev.dev, > + "NCSI: Failed to transmit cmd 0x%x during probe\n", > + nca.type); > +} > + > +static void set_mac_affinity_mlx(struct ncsi_dev_priv *ndp) > +{ > + struct ncsi_cmd_arg nca; > + int ret = 0; > + > + memset(&nca, 0, sizeof(struct ncsi_cmd_arg)); > + nca.ndp = ndp; > + nca.channel = ndp->active_channel->id; > + nca.package = ndp->active_package->id; > + nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN; > + nca.type = NCSI_PKT_CMD_OEM; > + nca.payload = 60; > + > + nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID); > + nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_SET_AFFINITY); > + > + memcpy(&(nca.bytes[8]), ndp->ndev.dev->dev_addr, ETH_ALEN); > + nca.bytes[14] = 0x09; > + > + ret = ncsi_xmit_cmd(&nca); > + if (ret) > + netdev_err(ndp->ndev.dev, > + "NCSI: Failed to transmit cmd 0x%x during probe\n", > + nca.type); > +} > +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */ > + > static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) > { > struct ncsi_dev *nd = &ndp->ndev; > @@ -685,6 +737,22 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) > goto error; > } > > +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC) > + /* Check Manufacture id if it is Mellanox then > + * get and set mac address. To Do: Add code for > + * other types of card if required > + */ > + if (nc->version.mf_id == NCSI_OEM_MFR_MLX_ID) > + nd->state = ncsi_dev_state_config_oem_gma; > + else > + nd->state = ncsi_dev_state_config_clear_vids; > + break; > + case ncsi_dev_state_config_oem_gma: > + ndp->pending_req_num = 2; > + get_mac_address_oem_mlx(ndp); > + msleep(500); > + set_mac_affinity_mlx(ndp); > +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */ > nd->state = ncsi_dev_state_config_clear_vids; > break; > case ncsi_dev_state_config_clear_vids: > diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h > index 91b4b66438df..0653a893eb12 100644 > --- a/net/ncsi/ncsi-pkt.h > +++ b/net/ncsi/ncsi-pkt.h > @@ -151,6 +151,22 @@ struct ncsi_cmd_snfc_pkt { > unsigned char pad[22]; > }; > > +/* Oem Request Command */ > +struct ncsi_cmd_oem_pkt { > + struct ncsi_cmd_pkt_hdr cmd; /* Command header */ > + unsigned char data[64]; /* OEM Payload Data */ > + __be32 checksum; /* Checksum */ > +}; > + > +/* Oem Response Packet */ > +struct ncsi_rsp_oem_pkt { > + struct ncsi_rsp_pkt_hdr rsp; /* Command header */ > + __be32 mfr_id; /* Manufacture ID */ > + __be32 oem_cmd; /* oem command */ > + unsigned char data[32]; /* Payload data */ > + __be32 checksum; /* Checksum */ > +}; > + > /* Get Link Status */ > struct ncsi_rsp_gls_pkt { > struct ncsi_rsp_pkt_hdr rsp; /* Response header */ > diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c > index 930c1d3796f0..3b94c96b9c7f 100644 > --- a/net/ncsi/ncsi-rsp.c > +++ b/net/ncsi/ncsi-rsp.c > @@ -596,6 +596,37 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr) > return 0; > } > > +static int ncsi_rsp_handler_oem(struct ncsi_request *nr) > +{ > + struct ncsi_rsp_oem_pkt *rsp; > + struct ncsi_dev_priv *ndp = nr->ndp; > + struct net_device *ndev = ndp->ndev.dev; > + int ret = 0; > + unsigned int oem_cmd, mfr_id; > + const struct net_device_ops *ops = ndev->netdev_ops; > + struct sockaddr saddr; > + > + /* Get the response header */ > + rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp); > + > + oem_cmd = ntohl(rsp->oem_cmd); > + mfr_id = ntohl(rsp->mfr_id); > + > + /* Check for Mellanox manufacturer id */ > + if (mfr_id != NCSI_OEM_MFR_MLX_ID) > + return 0; > + > + if (oem_cmd == NCSI_OEM_MLX_CMD_GET_MAC) { > + saddr.sa_family = ndev->type; > + ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE; > + memcpy(saddr.sa_data, &(rsp->data[4]), ETH_ALEN); > + ret = ops->ndo_set_mac_address(ndev, &saddr); > + if (ret < 0) > + netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n"); > + } > + return ret; > +} > + > static int ncsi_rsp_handler_gvi(struct ncsi_request *nr) > { > struct ncsi_rsp_gvi_pkt *rsp; > @@ -932,7 +963,7 @@ static struct ncsi_rsp_handler { > { NCSI_PKT_RSP_GNS, 172, ncsi_rsp_handler_gns }, > { NCSI_PKT_RSP_GNPTS, 172, ncsi_rsp_handler_gnpts }, > { NCSI_PKT_RSP_GPS, 8, ncsi_rsp_handler_gps }, > - { NCSI_PKT_RSP_OEM, 0, NULL }, > + { NCSI_PKT_RSP_OEM, -1, ncsi_rsp_handler_oem }, > { NCSI_PKT_RSP_PLDM, 0, NULL }, > { NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid } > }; > -- > 2.17.1 >
Hi Joel,
Thanks, I am adding netdev mailing list here.
Yes, this command is supported for all Mellanox card. It is as per Mellanox specification.
Regards
-Vijay
On 9/24/18, 5:30 PM, "Joel Stanley" <joel@jms.id.au> wrote:
Hi Vijay,
On Tue, 25 Sep 2018 at 09:39, Vijay Khemka <vijaykhemka@fb.com> wrote:
>
> This patch adds OEM command to get mac address from NCSI device and and
> configure the same to the network card.
>
> ncsi_cmd_arg - Modified this structure to include bigger payload data.
> ncsi_cmd_handler_oem: This function handes oem command request
> ncsi_rsp_handler_oem: This function handles response for OEM command.
> get_mac_address_oem_mlx: This function will send OEM command to get
> mac address for Mellanox card
> set_mac_affinity_mlx: This will send OEM command to set Mac affinity
> for Mellanox card
Thanks for the patch. The code looks okay, but I wanted to get some
input from our NCSI maintainer as to how OEM commands would be
structured. Sam, can you please provide some review here?
Is the command supported on all melanox cards, just some, or does
TiogaPass have a special firmware that enables it?
We should include the netdev mailing list in this discussion as the
patch needs to be acceptable for upstream.
Cheers,
Joel
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
> net/ncsi/Kconfig | 3 ++
> net/ncsi/internal.h | 11 +++++--
> net/ncsi/ncsi-cmd.c | 24 +++++++++++++--
> net/ncsi/ncsi-manage.c | 68 ++++++++++++++++++++++++++++++++++++++++++
> net/ncsi/ncsi-pkt.h | 16 ++++++++++
> net/ncsi/ncsi-rsp.c | 33 +++++++++++++++++++-
> 6 files changed, 149 insertions(+), 6 deletions(-)
>
> diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig
> index 08a8a6031fd7..b8bf89fea7c8 100644
> --- a/net/ncsi/Kconfig
> +++ b/net/ncsi/Kconfig
> @@ -10,3 +10,6 @@ config NET_NCSI
> support. Enable this only if your system connects to a network
> device via NCSI and the ethernet driver you're using supports
> the protocol explicitly.
> +config NCSI_OEM_CMD_GET_MAC
> + bool "Get NCSI OEM MAC Address"
> + depends on NET_NCSI
> diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
> index 8055e3965cef..da17958e6a4b 100644
> --- a/net/ncsi/internal.h
> +++ b/net/ncsi/internal.h
> @@ -68,6 +68,10 @@ enum {
> NCSI_MODE_MAX
> };
>
> +#define NCSI_OEM_MFR_MLX_ID 0x8119
> +#define NCSI_OEM_MLX_CMD_GET_MAC 0x1b00
> +#define NCSI_OEM_MLX_CMD_SET_AFFINITY 0x010700
> +
> struct ncsi_channel_version {
> u32 version; /* Supported BCD encoded NCSI version */
> u32 alpha2; /* Supported BCD encoded NCSI version */
> @@ -236,6 +240,7 @@ enum {
> ncsi_dev_state_probe_dp,
> ncsi_dev_state_config_sp = 0x0301,
> ncsi_dev_state_config_cis,
> + ncsi_dev_state_config_oem_gma,
> ncsi_dev_state_config_clear_vids,
> ncsi_dev_state_config_svf,
> ncsi_dev_state_config_ev,
> @@ -301,9 +306,9 @@ struct ncsi_cmd_arg {
> unsigned short payload; /* Command packet payload length */
> unsigned int req_flags; /* NCSI request properties */
> union {
> - unsigned char bytes[16]; /* Command packet specific data */
> - unsigned short words[8];
> - unsigned int dwords[4];
> + unsigned char bytes[64]; /* Command packet specific data */
> + unsigned short words[32];
> + unsigned int dwords[16];
> };
> };
>
> diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
> index 7567ca63aae2..3205e22c1734 100644
> --- a/net/ncsi/ncsi-cmd.c
> +++ b/net/ncsi/ncsi-cmd.c
> @@ -211,6 +211,25 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb,
> return 0;
> }
>
> +static int ncsi_cmd_handler_oem(struct sk_buff *skb,
> + struct ncsi_cmd_arg *nca)
> +{
> + struct ncsi_cmd_oem_pkt *cmd;
> + unsigned int len;
> +
> + len = sizeof(struct ncsi_cmd_pkt_hdr) + 4;
> + if (nca->payload < 26)
> + len += 26;
> + else
> + len += nca->payload;
> +
> + cmd = skb_put_zero(skb, len);
> + memcpy(cmd->data, nca->bytes, nca->payload);
> + ncsi_cmd_build_header(&cmd->cmd.common, nca);
> +
> + return 0;
> +}
> +
> static struct ncsi_cmd_handler {
> unsigned char type;
> int payload;
> @@ -244,7 +263,7 @@ static struct ncsi_cmd_handler {
> { NCSI_PKT_CMD_GNS, 0, ncsi_cmd_handler_default },
> { NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default },
> { NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default },
> - { NCSI_PKT_CMD_OEM, 0, NULL },
> + { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem },
> { NCSI_PKT_CMD_PLDM, 0, NULL },
> { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }
> };
> @@ -317,7 +336,8 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
> }
>
> /* Get packet payload length and allocate the request */
> - nca->payload = nch->payload;
> + if (nch->payload >= 0)
> + nca->payload = nch->payload;
> nr = ncsi_alloc_command(nca);
> if (!nr)
> return -ENOMEM;
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index 091284760d21..3b2b86560cc8 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -635,6 +635,58 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
> return 0;
> }
>
> +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> +/* NCSI Facebook OEM APIs */
> +static void get_mac_address_oem_mlx(struct ncsi_dev_priv *ndp)
> +{
> + struct ncsi_cmd_arg nca;
> + int ret = 0;
> +
> + memset(&nca, 0, sizeof(struct ncsi_cmd_arg));
> + nca.ndp = ndp;
> + nca.channel = ndp->active_channel->id;
> + nca.package = ndp->active_package->id;
> + nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
> + nca.type = NCSI_PKT_CMD_OEM;
> + nca.payload = 8;
> +
> + nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID);
> + nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_GET_MAC);
> +
> + ret = ncsi_xmit_cmd(&nca);
> + if (ret)
> + netdev_err(ndp->ndev.dev,
> + "NCSI: Failed to transmit cmd 0x%x during probe\n",
> + nca.type);
> +}
> +
> +static void set_mac_affinity_mlx(struct ncsi_dev_priv *ndp)
> +{
> + struct ncsi_cmd_arg nca;
> + int ret = 0;
> +
> + memset(&nca, 0, sizeof(struct ncsi_cmd_arg));
> + nca.ndp = ndp;
> + nca.channel = ndp->active_channel->id;
> + nca.package = ndp->active_package->id;
> + nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
> + nca.type = NCSI_PKT_CMD_OEM;
> + nca.payload = 60;
> +
> + nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID);
> + nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_SET_AFFINITY);
> +
> + memcpy(&(nca.bytes[8]), ndp->ndev.dev->dev_addr, ETH_ALEN);
> + nca.bytes[14] = 0x09;
> +
> + ret = ncsi_xmit_cmd(&nca);
> + if (ret)
> + netdev_err(ndp->ndev.dev,
> + "NCSI: Failed to transmit cmd 0x%x during probe\n",
> + nca.type);
> +}
> +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
> +
> static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> {
> struct ncsi_dev *nd = &ndp->ndev;
> @@ -685,6 +737,22 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
> goto error;
> }
>
> +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
> + /* Check Manufacture id if it is Mellanox then
> + * get and set mac address. To Do: Add code for
> + * other types of card if required
> + */
> + if (nc->version.mf_id == NCSI_OEM_MFR_MLX_ID)
> + nd->state = ncsi_dev_state_config_oem_gma;
> + else
> + nd->state = ncsi_dev_state_config_clear_vids;
> + break;
> + case ncsi_dev_state_config_oem_gma:
> + ndp->pending_req_num = 2;
> + get_mac_address_oem_mlx(ndp);
> + msleep(500);
> + set_mac_affinity_mlx(ndp);
> +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
> nd->state = ncsi_dev_state_config_clear_vids;
> break;
> case ncsi_dev_state_config_clear_vids:
> diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
> index 91b4b66438df..0653a893eb12 100644
> --- a/net/ncsi/ncsi-pkt.h
> +++ b/net/ncsi/ncsi-pkt.h
> @@ -151,6 +151,22 @@ struct ncsi_cmd_snfc_pkt {
> unsigned char pad[22];
> };
>
> +/* Oem Request Command */
> +struct ncsi_cmd_oem_pkt {
> + struct ncsi_cmd_pkt_hdr cmd; /* Command header */
> + unsigned char data[64]; /* OEM Payload Data */
> + __be32 checksum; /* Checksum */
> +};
> +
> +/* Oem Response Packet */
> +struct ncsi_rsp_oem_pkt {
> + struct ncsi_rsp_pkt_hdr rsp; /* Command header */
> + __be32 mfr_id; /* Manufacture ID */
> + __be32 oem_cmd; /* oem command */
> + unsigned char data[32]; /* Payload data */
> + __be32 checksum; /* Checksum */
> +};
> +
> /* Get Link Status */
> struct ncsi_rsp_gls_pkt {
> struct ncsi_rsp_pkt_hdr rsp; /* Response header */
> diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> index 930c1d3796f0..3b94c96b9c7f 100644
> --- a/net/ncsi/ncsi-rsp.c
> +++ b/net/ncsi/ncsi-rsp.c
> @@ -596,6 +596,37 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
> return 0;
> }
>
> +static int ncsi_rsp_handler_oem(struct ncsi_request *nr)
> +{
> + struct ncsi_rsp_oem_pkt *rsp;
> + struct ncsi_dev_priv *ndp = nr->ndp;
> + struct net_device *ndev = ndp->ndev.dev;
> + int ret = 0;
> + unsigned int oem_cmd, mfr_id;
> + const struct net_device_ops *ops = ndev->netdev_ops;
> + struct sockaddr saddr;
> +
> + /* Get the response header */
> + rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
> +
> + oem_cmd = ntohl(rsp->oem_cmd);
> + mfr_id = ntohl(rsp->mfr_id);
> +
> + /* Check for Mellanox manufacturer id */
> + if (mfr_id != NCSI_OEM_MFR_MLX_ID)
> + return 0;
> +
> + if (oem_cmd == NCSI_OEM_MLX_CMD_GET_MAC) {
> + saddr.sa_family = ndev->type;
> + ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
> + memcpy(saddr.sa_data, &(rsp->data[4]), ETH_ALEN);
> + ret = ops->ndo_set_mac_address(ndev, &saddr);
> + if (ret < 0)
> + netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n");
> + }
> + return ret;
> +}
> +
> static int ncsi_rsp_handler_gvi(struct ncsi_request *nr)
> {
> struct ncsi_rsp_gvi_pkt *rsp;
> @@ -932,7 +963,7 @@ static struct ncsi_rsp_handler {
> { NCSI_PKT_RSP_GNS, 172, ncsi_rsp_handler_gns },
> { NCSI_PKT_RSP_GNPTS, 172, ncsi_rsp_handler_gnpts },
> { NCSI_PKT_RSP_GPS, 8, ncsi_rsp_handler_gps },
> - { NCSI_PKT_RSP_OEM, 0, NULL },
> + { NCSI_PKT_RSP_OEM, -1, ncsi_rsp_handler_oem },
> { NCSI_PKT_RSP_PLDM, 0, NULL },
> { NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid }
> };
> --
> 2.17.1
>
On Tue, 2018-09-25 at 18:16 +0000, Vijay Khemka wrote: > Hi Joel, > Thanks, I am adding netdev mailing list here. > Yes, this command is supported for all Mellanox card. It is as per Mellanox specification. > > Regards > -Vijay Hi Vijay, Thanks for the patch; before I get too into a review though I'd like to loop in Justin (cc'd) who I know is also working on an OEM command patch. The changes here are very specific (eg. a command specific config option "CONFIG_NCSI_OEM_CMD_GET_MAC"), which is ok on a small scale but if we start to add an increasing amount of commands could get out of hand. As I understand Justin's version adds a generic handler, using the NCSI Netlink interface to pass OEM commands and responses to and from userspace, which does the actual packet handling. It would be good to compare these two approaches first before committing to any one path Justin - could you weigh in here and give a description of your intended changes? Are you able to post your changes upstream so we can compare? Regards, Samuel > > On 9/24/18, 5:30 PM, "Joel Stanley" <joel@jms.id.au> wrote: > > Hi Vijay, > > On Tue, 25 Sep 2018 at 09:39, Vijay Khemka <vijaykhemka@fb.com> wrote: > > > > This patch adds OEM command to get mac address from NCSI device and and > > configure the same to the network card. > > > > ncsi_cmd_arg - Modified this structure to include bigger payload data. > > ncsi_cmd_handler_oem: This function handes oem command request > > ncsi_rsp_handler_oem: This function handles response for OEM command. > > get_mac_address_oem_mlx: This function will send OEM command to get > > mac address for Mellanox card > > set_mac_affinity_mlx: This will send OEM command to set Mac affinity > > for Mellanox card > > Thanks for the patch. The code looks okay, but I wanted to get some > input from our NCSI maintainer as to how OEM commands would be > structured. Sam, can you please provide some review here? > > Is the command supported on all melanox cards, just some, or does > TiogaPass have a special firmware that enables it? > > We should include the netdev mailing list in this discussion as the > patch needs to be acceptable for upstream. > > Cheers, > > Joel > > > > > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com> > > --- > > net/ncsi/Kconfig | 3 ++ > > net/ncsi/internal.h | 11 +++++-- > > net/ncsi/ncsi-cmd.c | 24 +++++++++++++-- > > net/ncsi/ncsi-manage.c | 68 ++++++++++++++++++++++++++++++++++++++++++ > > net/ncsi/ncsi-pkt.h | 16 ++++++++++ > > net/ncsi/ncsi-rsp.c | 33 +++++++++++++++++++- > > 6 files changed, 149 insertions(+), 6 deletions(-) > > > > diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig > > index 08a8a6031fd7..b8bf89fea7c8 100644 > > --- a/net/ncsi/Kconfig > > +++ b/net/ncsi/Kconfig > > @@ -10,3 +10,6 @@ config NET_NCSI > > support. Enable this only if your system connects to a network > > device via NCSI and the ethernet driver you're using supports > > the protocol explicitly. > > +config NCSI_OEM_CMD_GET_MAC > > + bool "Get NCSI OEM MAC Address" > > + depends on NET_NCSI > > diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h > > index 8055e3965cef..da17958e6a4b 100644 > > --- a/net/ncsi/internal.h > > +++ b/net/ncsi/internal.h > > @@ -68,6 +68,10 @@ enum { > > NCSI_MODE_MAX > > }; > > > > +#define NCSI_OEM_MFR_MLX_ID 0x8119 > > +#define NCSI_OEM_MLX_CMD_GET_MAC 0x1b00 > > +#define NCSI_OEM_MLX_CMD_SET_AFFINITY 0x010700 > > + > > struct ncsi_channel_version { > > u32 version; /* Supported BCD encoded NCSI version */ > > u32 alpha2; /* Supported BCD encoded NCSI version */ > > @@ -236,6 +240,7 @@ enum { > > ncsi_dev_state_probe_dp, > > ncsi_dev_state_config_sp = 0x0301, > > ncsi_dev_state_config_cis, > > + ncsi_dev_state_config_oem_gma, > > ncsi_dev_state_config_clear_vids, > > ncsi_dev_state_config_svf, > > ncsi_dev_state_config_ev, > > @@ -301,9 +306,9 @@ struct ncsi_cmd_arg { > > unsigned short payload; /* Command packet payload length */ > > unsigned int req_flags; /* NCSI request properties */ > > union { > > - unsigned char bytes[16]; /* Command packet specific data */ > > - unsigned short words[8]; > > - unsigned int dwords[4]; > > + unsigned char bytes[64]; /* Command packet specific data */ > > + unsigned short words[32]; > > + unsigned int dwords[16]; > > }; > > }; > > > > diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c > > index 7567ca63aae2..3205e22c1734 100644 > > --- a/net/ncsi/ncsi-cmd.c > > +++ b/net/ncsi/ncsi-cmd.c > > @@ -211,6 +211,25 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb, > > return 0; > > } > > > > +static int ncsi_cmd_handler_oem(struct sk_buff *skb, > > + struct ncsi_cmd_arg *nca) > > +{ > > + struct ncsi_cmd_oem_pkt *cmd; > > + unsigned int len; > > + > > + len = sizeof(struct ncsi_cmd_pkt_hdr) + 4; > > + if (nca->payload < 26) > > + len += 26; > > + else > > + len += nca->payload; > > + > > + cmd = skb_put_zero(skb, len); > > + memcpy(cmd->data, nca->bytes, nca->payload); > > + ncsi_cmd_build_header(&cmd->cmd.common, nca); > > + > > + return 0; > > +} > > + > > static struct ncsi_cmd_handler { > > unsigned char type; > > int payload; > > @@ -244,7 +263,7 @@ static struct ncsi_cmd_handler { > > { NCSI_PKT_CMD_GNS, 0, ncsi_cmd_handler_default }, > > { NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default }, > > { NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default }, > > - { NCSI_PKT_CMD_OEM, 0, NULL }, > > + { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem }, > > { NCSI_PKT_CMD_PLDM, 0, NULL }, > > { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default } > > }; > > @@ -317,7 +336,8 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca) > > } > > > > /* Get packet payload length and allocate the request */ > > - nca->payload = nch->payload; > > + if (nch->payload >= 0) > > + nca->payload = nch->payload; > > nr = ncsi_alloc_command(nca); > > if (!nr) > > return -ENOMEM; > > diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c > > index 091284760d21..3b2b86560cc8 100644 > > --- a/net/ncsi/ncsi-manage.c > > +++ b/net/ncsi/ncsi-manage.c > > @@ -635,6 +635,58 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc, > > return 0; > > } > > > > +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC) > > +/* NCSI Facebook OEM APIs */ > > +static void get_mac_address_oem_mlx(struct ncsi_dev_priv *ndp) > > +{ > > + struct ncsi_cmd_arg nca; > > + int ret = 0; > > + > > + memset(&nca, 0, sizeof(struct ncsi_cmd_arg)); > > + nca.ndp = ndp; > > + nca.channel = ndp->active_channel->id; > > + nca.package = ndp->active_package->id; > > + nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN; > > + nca.type = NCSI_PKT_CMD_OEM; > > + nca.payload = 8; > > + > > + nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID); > > + nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_GET_MAC); > > + > > + ret = ncsi_xmit_cmd(&nca); > > + if (ret) > > + netdev_err(ndp->ndev.dev, > > + "NCSI: Failed to transmit cmd 0x%x during probe\n", > > + nca.type); > > +} > > + > > +static void set_mac_affinity_mlx(struct ncsi_dev_priv *ndp) > > +{ > > + struct ncsi_cmd_arg nca; > > + int ret = 0; > > + > > + memset(&nca, 0, sizeof(struct ncsi_cmd_arg)); > > + nca.ndp = ndp; > > + nca.channel = ndp->active_channel->id; > > + nca.package = ndp->active_package->id; > > + nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN; > > + nca.type = NCSI_PKT_CMD_OEM; > > + nca.payload = 60; > > + > > + nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID); > > + nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_SET_AFFINITY); > > + > > + memcpy(&(nca.bytes[8]), ndp->ndev.dev->dev_addr, ETH_ALEN); > > + nca.bytes[14] = 0x09; > > + > > + ret = ncsi_xmit_cmd(&nca); > > + if (ret) > > + netdev_err(ndp->ndev.dev, > > + "NCSI: Failed to transmit cmd 0x%x during probe\n", > > + nca.type); > > +} > > +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */ > > + > > static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) > > { > > struct ncsi_dev *nd = &ndp->ndev; > > @@ -685,6 +737,22 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) > > goto error; > > } > > > > +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC) > > + /* Check Manufacture id if it is Mellanox then > > + * get and set mac address. To Do: Add code for > > + * other types of card if required > > + */ > > + if (nc->version.mf_id == NCSI_OEM_MFR_MLX_ID) > > + nd->state = ncsi_dev_state_config_oem_gma; > > + else > > + nd->state = ncsi_dev_state_config_clear_vids; > > + break; > > + case ncsi_dev_state_config_oem_gma: > > + ndp->pending_req_num = 2; > > + get_mac_address_oem_mlx(ndp); > > + msleep(500); > > + set_mac_affinity_mlx(ndp); > > +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */ > > nd->state = ncsi_dev_state_config_clear_vids; > > break; > > case ncsi_dev_state_config_clear_vids: > > diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h > > index 91b4b66438df..0653a893eb12 100644 > > --- a/net/ncsi/ncsi-pkt.h > > +++ b/net/ncsi/ncsi-pkt.h > > @@ -151,6 +151,22 @@ struct ncsi_cmd_snfc_pkt { > > unsigned char pad[22]; > > }; > > > > +/* Oem Request Command */ > > +struct ncsi_cmd_oem_pkt { > > + struct ncsi_cmd_pkt_hdr cmd; /* Command header */ > > + unsigned char data[64]; /* OEM Payload Data */ > > + __be32 checksum; /* Checksum */ > > +}; > > + > > +/* Oem Response Packet */ > > +struct ncsi_rsp_oem_pkt { > > + struct ncsi_rsp_pkt_hdr rsp; /* Command header */ > > + __be32 mfr_id; /* Manufacture ID */ > > + __be32 oem_cmd; /* oem command */ > > + unsigned char data[32]; /* Payload data */ > > + __be32 checksum; /* Checksum */ > > +}; > > + > > /* Get Link Status */ > > struct ncsi_rsp_gls_pkt { > > struct ncsi_rsp_pkt_hdr rsp; /* Response header */ > > diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c > > index 930c1d3796f0..3b94c96b9c7f 100644 > > --- a/net/ncsi/ncsi-rsp.c > > +++ b/net/ncsi/ncsi-rsp.c > > @@ -596,6 +596,37 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr) > > return 0; > > } > > > > +static int ncsi_rsp_handler_oem(struct ncsi_request *nr) > > +{ > > + struct ncsi_rsp_oem_pkt *rsp; > > + struct ncsi_dev_priv *ndp = nr->ndp; > > + struct net_device *ndev = ndp->ndev.dev; > > + int ret = 0; > > + unsigned int oem_cmd, mfr_id; > > + const struct net_device_ops *ops = ndev->netdev_ops; > > + struct sockaddr saddr; > > + > > + /* Get the response header */ > > + rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp); > > + > > + oem_cmd = ntohl(rsp->oem_cmd); > > + mfr_id = ntohl(rsp->mfr_id); > > + > > + /* Check for Mellanox manufacturer id */ > > + if (mfr_id != NCSI_OEM_MFR_MLX_ID) > > + return 0; > > + > > + if (oem_cmd == NCSI_OEM_MLX_CMD_GET_MAC) { > > + saddr.sa_family = ndev->type; > > + ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE; > > + memcpy(saddr.sa_data, &(rsp->data[4]), ETH_ALEN); > > + ret = ops->ndo_set_mac_address(ndev, &saddr); > > + if (ret < 0) > > + netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n"); > > + } > > + return ret; > > +} > > + > > static int ncsi_rsp_handler_gvi(struct ncsi_request *nr) > > { > > struct ncsi_rsp_gvi_pkt *rsp; > > @@ -932,7 +963,7 @@ static struct ncsi_rsp_handler { > > { NCSI_PKT_RSP_GNS, 172, ncsi_rsp_handler_gns }, > > { NCSI_PKT_RSP_GNPTS, 172, ncsi_rsp_handler_gnpts }, > > { NCSI_PKT_RSP_GPS, 8, ncsi_rsp_handler_gps }, > > - { NCSI_PKT_RSP_OEM, 0, NULL }, > > + { NCSI_PKT_RSP_OEM, -1, ncsi_rsp_handler_oem }, > > { NCSI_PKT_RSP_PLDM, 0, NULL }, > > { NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid } > > }; > > -- > > 2.17.1 > > > >
> Hi Vijay, > Thanks for the patch; before I get too into a review though I'd like to > loop in Justin (cc'd) who I know is also working on an OEM command patch. > The changes here are very specific (eg. a command specific config option > "CONFIG_NCSI_OEM_CMD_GET_MAC"), which is ok on a small scale but if we > start to add an increasing amount of commands could get out of hand. > As I understand Justin's version adds a generic handler, using the NCSI > Netlink interface to pass OEM commands and responses to and from > userspace, which does the actual packet handling. > It would be good to compare these two approaches first before committing > to any one path Hi Sam, My oem command handler is generic and can be used for any oem commands and oem response handler can be made more generic. We can certainly write a wrapper to support netlink oem command to receive form user space. There are Mellanox specific functions which sends Mellanox specific request. These needed as a part of initial configuration. We can remove Kconfig option with more generic approach. -Vijay
> As I understand Justin's version adds a generic handler, using the NCSI > Netlink interface to pass OEM commands and responses to and from > userspace, which does the actual packet handling. Thanks for the direction Sam! Justin, if you don't mind, can you share the patches you have to add the support? This actually would solve a few other things we are trying to accomplish. <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size: 12pt; color: rgb(0, 0, 0); font-family: Calibri, Helvetica, sans-serif, EmojiFont, "Apple Color Emoji", "Segoe UI Emoji", NotoColorEmoji, "Segoe UI Symbol", "Android Emoji", EmojiSymbols;" dir="ltr"> <p style="margin-top:0;margin-bottom:0"><span style="font-size: 11pt;"> > As I understand Justin's version adds a generic handler, using the NCSI</span><br> </p> <div style="color: rgb(0, 0, 0);"> <div class="BodyFragment"><font size="2"><span style="font-size:11pt;"> <div class="PlainText"> > Netlink interface to pass OEM commands and responses to and from<br> > userspace, which does the actual packet handling.<br> </div> <div class="PlainText">Thanks for the direction Sam! Justin, if you don't mind, can you share the patches you have to add the support? This actually would solve a few other things we are trying to accomplish.</div> </span></font></div> </div> </div> </body> </html>
> > As I understand Justin's version adds a generic handler, using the NCSI > > Netlink interface to pass OEM commands and responses to and from > > userspace, which does the actual packet handling. > Thanks for the direction Sam! Justin, if you don't mind, can you share the patches you have to add the support? This actually would solve a few other things we are trying to accomplish. Basically, I add a new flag to indicate the request is coming from the Netlink interface to allow the command handler and response handler to react. #define NCSI_REQ_FLAG_NETLINK_DRIVEN 2 The work flow is as below. Request: User space application -> Netlink interface (msg) -> new Netlink handler - ncsi_send_cmd_nl() - ncsi_xmit_cmd() Response: Response received - ncsi_rcv_rsp() -> internal response handler - ncsi_rsp_handler_xxx() -> ncsi_send_netlink_rsp () -> Netlink interface (msg) -> user space application Command timeout - ncsi_request_timeout() -> ncsi_send_netlink_timeout () -> Netlink interface (msg with zero data length) -> user space application Error: Detected error -> ncsi_send_netlink_err () -> Netlink interface (err msg) -> user space application I will clean up some code and send out the patch. Thanks, Justin
On Thu, 27 Sep 2018 at 00:39, <Justin.Lee1@dell.com> wrote: > > > > As I understand Justin's version adds a generic handler, using the NCSI > > > Netlink interface to pass OEM commands and responses to and from > > > userspace, which does the actual packet handling. > > Thanks for the direction Sam! Justin, if you don't mind, can you share the patches you have to add the support? This actually would solve a few other things we are trying to accomplish. > > > Basically, I add a new flag to indicate the request is coming from the Netlink interface to allow the command handler and response handler to react. > #define NCSI_REQ_FLAG_NETLINK_DRIVEN 2 > > The work flow is as below. > > Request: > User space application -> Netlink interface (msg) -> new Netlink handler - ncsi_send_cmd_nl() - ncsi_xmit_cmd() > > Response: > Response received - ncsi_rcv_rsp() -> internal response handler - ncsi_rsp_handler_xxx() -> ncsi_send_netlink_rsp () -> Netlink interface (msg) -> user space application > Command timeout - ncsi_request_timeout() -> ncsi_send_netlink_timeout () -> Netlink interface (msg with zero data length) -> user space application > > Error: > Detected error -> ncsi_send_netlink_err () -> Netlink interface (err msg) -> user space application > > I will clean up some code and send out the patch. Thanks for the overview. We look forward to your patches; please include the same cc list as this series. I think it makes sense to have some OEM NCSI handing purely in the kenrel. This would allow eg. the MAC address of an interface to be correct at boot, without requiring userspace to come up first. I have also heard stories of temperature sensors over NCSI. Those would make sense to be hwmon drivers, which again would mean handling them in the kernel. Justin, Vijay, can you please list the known NCSI OEM commands/extensions that we plan on implementing? Cheers, Joel
On Mon, 2018-09-24 at 17:08 -0700, Vijay Khemka wrote: > This patch adds OEM command to get mac address from NCSI device and and > configure the same to the network card. > > ncsi_cmd_arg - Modified this structure to include bigger payload data. > ncsi_cmd_handler_oem: This function handles oem command request > ncsi_rsp_handler_oem: This function handles response for OEM command. > get_mac_address_oem_mlx: This function will send OEM command to get > mac address for Mellanox card > set_mac_affinity_mlx: This will send OEM command to set Mac affinity > for Mellanox card > > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com> Hi Vijay, Having had a chance to take a closer look, there is probably room for both this patchset and Justin's potential changes to coexist; while Justin's is a more general solution for sending arbitrary commands, the approach of this patch is also useful for handling commands we want included in the configure process (such as get-mac-address). Some comments below: > --- > net/ncsi/Kconfig | 3 ++ > net/ncsi/internal.h | 11 +++++-- > net/ncsi/ncsi-cmd.c | 24 +++++++++++++-- > net/ncsi/ncsi-manage.c | 68 ++++++++++++++++++++++++++++++++++++++++++ > net/ncsi/ncsi-pkt.h | 16 ++++++++++ > net/ncsi/ncsi-rsp.c | 33 +++++++++++++++++++- > 6 files changed, 149 insertions(+), 6 deletions(-) > > diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig > index 08a8a6031fd7..b8bf89fea7c8 100644 > --- a/net/ncsi/Kconfig > +++ b/net/ncsi/Kconfig > @@ -10,3 +10,6 @@ config NET_NCSI > support. Enable this only if your system connects to a network > device via NCSI and the ethernet driver you're using supports > the protocol explicitly. > +config NCSI_OEM_CMD_GET_MAC > + bool "Get NCSI OEM MAC Address" > + depends on NET_NCSI For the moment this isn't too bad but I wonder if in the future it would be more flexible to have something like NCSI_OEM_CMD_MELLANOX etc, so we could selectively enable a class of OEM commands based on vendor rather than per-command. > diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h > index 8055e3965cef..da17958e6a4b 100644 > --- a/net/ncsi/internal.h > +++ b/net/ncsi/internal.h > @@ -68,6 +68,10 @@ enum { > NCSI_MODE_MAX > }; > > +#define NCSI_OEM_MFR_MLX_ID 0x8119 > +#define NCSI_OEM_MLX_CMD_GET_MAC 0x1b00 > +#define NCSI_OEM_MLX_CMD_SET_AFFINITY 0x010700 I gather this is part of the OEM command but it would be good to describe what these bits mean. Is this command documented anywhere by Mellanox? > + > struct ncsi_channel_version { > u32 version; /* Supported BCD encoded NCSI version */ > u32 alpha2; /* Supported BCD encoded NCSI version */ > @@ -236,6 +240,7 @@ enum { > ncsi_dev_state_probe_dp, > ncsi_dev_state_config_sp = 0x0301, > ncsi_dev_state_config_cis, > + ncsi_dev_state_config_oem_gma, > ncsi_dev_state_config_clear_vids, > ncsi_dev_state_config_svf, > ncsi_dev_state_config_ev, > @@ -301,9 +306,9 @@ struct ncsi_cmd_arg { > unsigned short payload; /* Command packet payload length */ > unsigned int req_flags; /* NCSI request properties */ > union { > - unsigned char bytes[16]; /* Command packet specific data */ > - unsigned short words[8]; > - unsigned int dwords[4]; > + unsigned char bytes[64]; /* Command packet specific data */ > + unsigned short words[32]; > + unsigned int dwords[16]; > }; > }; > > diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c > index 7567ca63aae2..3205e22c1734 100644 > --- a/net/ncsi/ncsi-cmd.c > +++ b/net/ncsi/ncsi-cmd.c > @@ -211,6 +211,25 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb, > return 0; > } > > +static int ncsi_cmd_handler_oem(struct sk_buff *skb, > + struct ncsi_cmd_arg *nca) > +{ > + struct ncsi_cmd_oem_pkt *cmd; > + unsigned int len; > + > + len = sizeof(struct ncsi_cmd_pkt_hdr) + 4; > + if (nca->payload < 26) > + len += 26; This will have already happened in ncsi_alloc_command(), is this check needed? > + else > + len += nca->payload; > + > + cmd = skb_put_zero(skb, len); > + memcpy(cmd->data, nca->bytes, nca->payload); > + ncsi_cmd_build_header(&cmd->cmd.common, nca); > + > + return 0; > +} > + > static struct ncsi_cmd_handler { > unsigned char type; > int payload; > @@ -244,7 +263,7 @@ static struct ncsi_cmd_handler { > { NCSI_PKT_CMD_GNS, 0, ncsi_cmd_handler_default }, > { NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default }, > { NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default }, > - { NCSI_PKT_CMD_OEM, 0, NULL }, > + { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem }, > { NCSI_PKT_CMD_PLDM, 0, NULL }, > { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default } > }; > @@ -317,7 +336,8 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca) > } > > /* Get packet payload length and allocate the request */ > - nca->payload = nch->payload; > + if (nch->payload >= 0) > + nca->payload = nch->payload; I think with this there is a chance of nca->payload being uninitialised and then used in ncsi_alloc_command(). Can you describe what the aim here is? > nr = ncsi_alloc_command(nca); > if (!nr) > return -ENOMEM; > diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c > index 091284760d21..3b2b86560cc8 100644 > --- a/net/ncsi/ncsi-manage.c > +++ b/net/ncsi/ncsi-manage.c > @@ -635,6 +635,58 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc, > return 0; > } > > +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC) > +/* NCSI Facebook OEM APIs */ Are these Facebook OEM commands or Mellanox OEM commands? > +static void get_mac_address_oem_mlx(struct ncsi_dev_priv *ndp) > +{ > + struct ncsi_cmd_arg nca; > + int ret = 0; > + > + memset(&nca, 0, sizeof(struct ncsi_cmd_arg)); > + nca.ndp = ndp; > + nca.channel = ndp->active_channel->id; > + nca.package = ndp->active_package->id; > + nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN; > + nca.type = NCSI_PKT_CMD_OEM; > + nca.payload = 8; > + > + nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID); > + nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_GET_MAC); > + > + ret = ncsi_xmit_cmd(&nca); > + if (ret) > + netdev_err(ndp->ndev.dev, > + "NCSI: Failed to transmit cmd 0x%x during probe\n", > + nca.type); > +} > + > +static void set_mac_affinity_mlx(struct ncsi_dev_priv *ndp) > +{ > + struct ncsi_cmd_arg nca; > + int ret = 0; > + > + memset(&nca, 0, sizeof(struct ncsi_cmd_arg)); Ah I see we initialise nca, and thus payload here - the path between these two points in the driver isn't super obvious (not this patch's fault :) ), it might be better to explicitly mention/handle this where we use payload later on. > + nca.ndp = ndp; > + nca.channel = ndp->active_channel->id; > + nca.package = ndp->active_package->id; These should probably be set in ncsi_configure_channel (eg. see the CIS state before these are called). > + nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN; > + nca.type = NCSI_PKT_CMD_OEM; > + nca.payload = 60; > + > + nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID); > + nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_SET_AFFINITY); > + > + memcpy(&(nca.bytes[8]), ndp->ndev.dev->dev_addr, ETH_ALEN); > + nca.bytes[14] = 0x09; > + > + ret = ncsi_xmit_cmd(&nca); > + if (ret) > + netdev_err(ndp->ndev.dev, > + "NCSI: Failed to transmit cmd 0x%x during probe\n", > + nca.type); > +} > +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */ > + > static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) > { > struct ncsi_dev *nd = &ndp->ndev; > @@ -685,6 +737,22 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) > goto error; > } > > +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC) > + /* Check Manufacture id if it is Mellanox then > + * get and set mac address. To Do: Add code for > + * other types of card if required > + */ > + if (nc->version.mf_id == NCSI_OEM_MFR_MLX_ID) > + nd->state = ncsi_dev_state_config_oem_gma; > + else > + nd->state = ncsi_dev_state_config_clear_vids; > + break; > + case ncsi_dev_state_config_oem_gma: > + ndp->pending_req_num = 2; > + get_mac_address_oem_mlx(ndp); > + msleep(500); Is this msleep() required? > + set_mac_affinity_mlx(ndp); Since this *sets* the MAC address, should we name the state and config option more accurately? How does this OEM command interact with the NCSI set-mac-address command? Does this command depend on the previous get_mac_address_oem_mlx() command succeeding? > +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */ > nd->state = ncsi_dev_state_config_clear_vids; > break; > case ncsi_dev_state_config_clear_vids: > diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h > index 91b4b66438df..0653a893eb12 100644 > --- a/net/ncsi/ncsi-pkt.h > +++ b/net/ncsi/ncsi-pkt.h > @@ -151,6 +151,22 @@ struct ncsi_cmd_snfc_pkt { > unsigned char pad[22]; > }; > > +/* Oem Request Command */ In general, s/Oem/OEM > +struct ncsi_cmd_oem_pkt { > + struct ncsi_cmd_pkt_hdr cmd; /* Command header */ > + unsigned char data[64]; /* OEM Payload Data */ > + __be32 checksum; /* Checksum */ > +}; > + > +/* Oem Response Packet */ > +struct ncsi_rsp_oem_pkt { > + struct ncsi_rsp_pkt_hdr rsp; /* Command header */ > + __be32 mfr_id; /* Manufacture ID */ > + __be32 oem_cmd; /* oem command */ > + unsigned char data[32]; /* Payload data */ > + __be32 checksum; /* Checksum */ > +}; > + > /* Get Link Status */ > struct ncsi_rsp_gls_pkt { > struct ncsi_rsp_pkt_hdr rsp; /* Response header */ > diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c > index 930c1d3796f0..3b94c96b9c7f 100644 > --- a/net/ncsi/ncsi-rsp.c > +++ b/net/ncsi/ncsi-rsp.c > @@ -596,6 +596,37 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr) > return 0; > } > > +static int ncsi_rsp_handler_oem(struct ncsi_request *nr) > +{ > + struct ncsi_rsp_oem_pkt *rsp; > + struct ncsi_dev_priv *ndp = nr->ndp; > + struct net_device *ndev = ndp->ndev.dev; > + int ret = 0; > + unsigned int oem_cmd, mfr_id; > + const struct net_device_ops *ops = ndev->netdev_ops; > + struct sockaddr saddr; > + > + /* Get the response header */ > + rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp); > + > + oem_cmd = ntohl(rsp->oem_cmd); > + mfr_id = ntohl(rsp->mfr_id); > + > + /* Check for Mellanox manufacturer id */ > + if (mfr_id != NCSI_OEM_MFR_MLX_ID) > + return 0; > + > + if (oem_cmd == NCSI_OEM_MLX_CMD_GET_MAC) { > + saddr.sa_family = ndev->type; > + ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE; > + memcpy(saddr.sa_data, &(rsp->data[4]), ETH_ALEN); > + ret = ops->ndo_set_mac_address(ndev, &saddr); > + if (ret < 0) > + netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n"); > + } > + return ret; > +} > + > static int ncsi_rsp_handler_gvi(struct ncsi_request *nr) > { > struct ncsi_rsp_gvi_pkt *rsp; > @@ -932,7 +963,7 @@ static struct ncsi_rsp_handler { > { NCSI_PKT_RSP_GNS, 172, ncsi_rsp_handler_gns }, > { NCSI_PKT_RSP_GNPTS, 172, ncsi_rsp_handler_gnpts }, > { NCSI_PKT_RSP_GPS, 8, ncsi_rsp_handler_gps }, > - { NCSI_PKT_RSP_OEM, 0, NULL }, > + { NCSI_PKT_RSP_OEM, -1, ncsi_rsp_handler_oem }, > { NCSI_PKT_RSP_PLDM, 0, NULL }, > { NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid } > };
On Thu, 2018-09-27 at 13:43 +1000, Samuel Mendoza-Jonas wrote: > On Mon, 2018-09-24 at 17:08 -0700, Vijay Khemka wrote: > > This patch adds OEM command to get mac address from NCSI device and and > > configure the same to the network card. > > > > ncsi_cmd_arg - Modified this structure to include bigger payload data. > > ncsi_cmd_handler_oem: This function handles oem command request > > ncsi_rsp_handler_oem: This function handles response for OEM command. > > get_mac_address_oem_mlx: This function will send OEM command to get > > mac address for Mellanox card > > set_mac_affinity_mlx: This will send OEM command to set Mac affinity > > for Mellanox card > > > > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com> > > Hi Vijay, > > Having had a chance to take a closer look, there is probably room for > both this patchset and Justin's potential changes to coexist; while > Justin's is a more general solution for sending arbitrary commands, the > approach of this patch is also useful for handling commands we want > included in the configure process (such as get-mac-address). > > Some comments below: Whoops, forgot to re-add netdev. > > > --- > > net/ncsi/Kconfig | 3 ++ > > net/ncsi/internal.h | 11 +++++-- > > net/ncsi/ncsi-cmd.c | 24 +++++++++++++-- > > net/ncsi/ncsi-manage.c | 68 ++++++++++++++++++++++++++++++++++++++++++ > > net/ncsi/ncsi-pkt.h | 16 ++++++++++ > > net/ncsi/ncsi-rsp.c | 33 +++++++++++++++++++- > > 6 files changed, 149 insertions(+), 6 deletions(-) > > > > diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig > > index 08a8a6031fd7..b8bf89fea7c8 100644 > > --- a/net/ncsi/Kconfig > > +++ b/net/ncsi/Kconfig > > @@ -10,3 +10,6 @@ config NET_NCSI > > support. Enable this only if your system connects to a network > > device via NCSI and the ethernet driver you're using supports > > the protocol explicitly. > > +config NCSI_OEM_CMD_GET_MAC > > + bool "Get NCSI OEM MAC Address" > > + depends on NET_NCSI > > For the moment this isn't too bad but I wonder if in the future it would > be more flexible to have something like NCSI_OEM_CMD_MELLANOX etc, so we > could selectively enable a class of OEM commands based on vendor rather > than per-command. > > > diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h > > index 8055e3965cef..da17958e6a4b 100644 > > --- a/net/ncsi/internal.h > > +++ b/net/ncsi/internal.h > > @@ -68,6 +68,10 @@ enum { > > NCSI_MODE_MAX > > }; > > > > +#define NCSI_OEM_MFR_MLX_ID 0x8119 > > +#define NCSI_OEM_MLX_CMD_GET_MAC 0x1b00 > > +#define NCSI_OEM_MLX_CMD_SET_AFFINITY 0x010700 > > I gather this is part of the OEM command but it would be good to describe > what these bits mean. Is this command documented anywhere by Mellanox? > > > + > > struct ncsi_channel_version { > > u32 version; /* Supported BCD encoded NCSI version */ > > u32 alpha2; /* Supported BCD encoded NCSI version */ > > @@ -236,6 +240,7 @@ enum { > > ncsi_dev_state_probe_dp, > > ncsi_dev_state_config_sp = 0x0301, > > ncsi_dev_state_config_cis, > > + ncsi_dev_state_config_oem_gma, > > ncsi_dev_state_config_clear_vids, > > ncsi_dev_state_config_svf, > > ncsi_dev_state_config_ev, > > @@ -301,9 +306,9 @@ struct ncsi_cmd_arg { > > unsigned short payload; /* Command packet payload length */ > > unsigned int req_flags; /* NCSI request properties */ > > union { > > - unsigned char bytes[16]; /* Command packet specific data */ > > - unsigned short words[8]; > > - unsigned int dwords[4]; > > + unsigned char bytes[64]; /* Command packet specific data */ > > + unsigned short words[32]; > > + unsigned int dwords[16]; > > }; > > }; > > > > diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c > > index 7567ca63aae2..3205e22c1734 100644 > > --- a/net/ncsi/ncsi-cmd.c > > +++ b/net/ncsi/ncsi-cmd.c > > @@ -211,6 +211,25 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb, > > return 0; > > } > > > > +static int ncsi_cmd_handler_oem(struct sk_buff *skb, > > + struct ncsi_cmd_arg *nca) > > +{ > > + struct ncsi_cmd_oem_pkt *cmd; > > + unsigned int len; > > + > > + len = sizeof(struct ncsi_cmd_pkt_hdr) + 4; > > + if (nca->payload < 26) > > + len += 26; > > This will have already happened in ncsi_alloc_command(), is this check > needed? > > > + else > > + len += nca->payload; > > + > > + cmd = skb_put_zero(skb, len); > > + memcpy(cmd->data, nca->bytes, nca->payload); > > + ncsi_cmd_build_header(&cmd->cmd.common, nca); > > + > > + return 0; > > +} > > + > > static struct ncsi_cmd_handler { > > unsigned char type; > > int payload; > > @@ -244,7 +263,7 @@ static struct ncsi_cmd_handler { > > { NCSI_PKT_CMD_GNS, 0, ncsi_cmd_handler_default }, > > { NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default }, > > { NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default }, > > - { NCSI_PKT_CMD_OEM, 0, NULL }, > > + { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem }, > > { NCSI_PKT_CMD_PLDM, 0, NULL }, > > { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default } > > }; > > @@ -317,7 +336,8 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca) > > } > > > > /* Get packet payload length and allocate the request */ > > - nca->payload = nch->payload; > > + if (nch->payload >= 0) > > + nca->payload = nch->payload; > > I think with this there is a chance of nca->payload being uninitialised > and then used in ncsi_alloc_command(). Can you describe what the aim here > is? > > > nr = ncsi_alloc_command(nca); > > if (!nr) > > return -ENOMEM; > > diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c > > index 091284760d21..3b2b86560cc8 100644 > > --- a/net/ncsi/ncsi-manage.c > > +++ b/net/ncsi/ncsi-manage.c > > @@ -635,6 +635,58 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc, > > return 0; > > } > > > > +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC) > > +/* NCSI Facebook OEM APIs */ > > Are these Facebook OEM commands or Mellanox OEM commands? > > > +static void get_mac_address_oem_mlx(struct ncsi_dev_priv *ndp) > > +{ > > + struct ncsi_cmd_arg nca; > > + int ret = 0; > > + > > + memset(&nca, 0, sizeof(struct ncsi_cmd_arg)); > > + nca.ndp = ndp; > > + nca.channel = ndp->active_channel->id; > > + nca.package = ndp->active_package->id; > > + nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN; > > + nca.type = NCSI_PKT_CMD_OEM; > > + nca.payload = 8; > > + > > + nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID); > > + nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_GET_MAC); > > + > > + ret = ncsi_xmit_cmd(&nca); > > + if (ret) > > + netdev_err(ndp->ndev.dev, > > + "NCSI: Failed to transmit cmd 0x%x during probe\n", > > + nca.type); > > +} > > + > > +static void set_mac_affinity_mlx(struct ncsi_dev_priv *ndp) > > +{ > > + struct ncsi_cmd_arg nca; > > + int ret = 0; > > + > > + memset(&nca, 0, sizeof(struct ncsi_cmd_arg)); > > Ah I see we initialise nca, and thus payload here - the path between > these two points in the driver isn't super obvious (not this patch's > fault :) ), it might be better to explicitly mention/handle this where we > use payload later on. > > > + nca.ndp = ndp; > > + nca.channel = ndp->active_channel->id; > > + nca.package = ndp->active_package->id; > > These should probably be set in ncsi_configure_channel (eg. see the CIS > state before these are called). > > > + nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN; > > + nca.type = NCSI_PKT_CMD_OEM; > > + nca.payload = 60; > > + > > + nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID); > > + nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_SET_AFFINITY); > > + > > + memcpy(&(nca.bytes[8]), ndp->ndev.dev->dev_addr, ETH_ALEN); > > + nca.bytes[14] = 0x09; > > + > > + ret = ncsi_xmit_cmd(&nca); > > + if (ret) > > + netdev_err(ndp->ndev.dev, > > + "NCSI: Failed to transmit cmd 0x%x during probe\n", > > + nca.type); > > +} > > +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */ > > + > > static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) > > { > > struct ncsi_dev *nd = &ndp->ndev; > > @@ -685,6 +737,22 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) > > goto error; > > } > > > > +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC) > > + /* Check Manufacture id if it is Mellanox then > > + * get and set mac address. To Do: Add code for > > + * other types of card if required > > + */ > > + if (nc->version.mf_id == NCSI_OEM_MFR_MLX_ID) > > + nd->state = ncsi_dev_state_config_oem_gma; > > + else > > + nd->state = ncsi_dev_state_config_clear_vids; > > + break; > > + case ncsi_dev_state_config_oem_gma: > > + ndp->pending_req_num = 2; > > + get_mac_address_oem_mlx(ndp); > > + msleep(500); > > Is this msleep() required? > > > + set_mac_affinity_mlx(ndp); > > Since this *sets* the MAC address, should we name the state and config > option more accurately? How does this OEM command interact with the NCSI > set-mac-address command? > Does this command depend on the previous get_mac_address_oem_mlx() > command succeeding? > > > +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */ > > nd->state = ncsi_dev_state_config_clear_vids; > > break; > > case ncsi_dev_state_config_clear_vids: > > diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h > > index 91b4b66438df..0653a893eb12 100644 > > --- a/net/ncsi/ncsi-pkt.h > > +++ b/net/ncsi/ncsi-pkt.h > > @@ -151,6 +151,22 @@ struct ncsi_cmd_snfc_pkt { > > unsigned char pad[22]; > > }; > > > > +/* Oem Request Command */ > > In general, s/Oem/OEM > > > +struct ncsi_cmd_oem_pkt { > > + struct ncsi_cmd_pkt_hdr cmd; /* Command header */ > > + unsigned char data[64]; /* OEM Payload Data */ > > + __be32 checksum; /* Checksum */ > > +}; > > + > > +/* Oem Response Packet */ > > +struct ncsi_rsp_oem_pkt { > > + struct ncsi_rsp_pkt_hdr rsp; /* Command header */ > > + __be32 mfr_id; /* Manufacture ID */ > > + __be32 oem_cmd; /* oem command */ > > + unsigned char data[32]; /* Payload data */ > > + __be32 checksum; /* Checksum */ > > +}; > > + > > /* Get Link Status */ > > struct ncsi_rsp_gls_pkt { > > struct ncsi_rsp_pkt_hdr rsp; /* Response header */ > > diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c > > index 930c1d3796f0..3b94c96b9c7f 100644 > > --- a/net/ncsi/ncsi-rsp.c > > +++ b/net/ncsi/ncsi-rsp.c > > @@ -596,6 +596,37 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr) > > return 0; > > } > > > > +static int ncsi_rsp_handler_oem(struct ncsi_request *nr) > > +{ > > + struct ncsi_rsp_oem_pkt *rsp; > > + struct ncsi_dev_priv *ndp = nr->ndp; > > + struct net_device *ndev = ndp->ndev.dev; > > + int ret = 0; > > + unsigned int oem_cmd, mfr_id; > > + const struct net_device_ops *ops = ndev->netdev_ops; > > + struct sockaddr saddr; > > + > > + /* Get the response header */ > > + rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp); > > + > > + oem_cmd = ntohl(rsp->oem_cmd); > > + mfr_id = ntohl(rsp->mfr_id); > > + > > + /* Check for Mellanox manufacturer id */ > > + if (mfr_id != NCSI_OEM_MFR_MLX_ID) > > + return 0; > > + > > + if (oem_cmd == NCSI_OEM_MLX_CMD_GET_MAC) { > > + saddr.sa_family = ndev->type; > > + ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE; > > + memcpy(saddr.sa_data, &(rsp->data[4]), ETH_ALEN); > > + ret = ops->ndo_set_mac_address(ndev, &saddr); > > + if (ret < 0) > > + netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n"); > > + } > > + return ret; > > +} > > + > > static int ncsi_rsp_handler_gvi(struct ncsi_request *nr) > > { > > struct ncsi_rsp_gvi_pkt *rsp; > > @@ -932,7 +963,7 @@ static struct ncsi_rsp_handler { > > { NCSI_PKT_RSP_GNS, 172, ncsi_rsp_handler_gns }, > > { NCSI_PKT_RSP_GNPTS, 172, ncsi_rsp_handler_gnpts }, > > { NCSI_PKT_RSP_GPS, 8, ncsi_rsp_handler_gps }, > > - { NCSI_PKT_RSP_OEM, 0, NULL }, > > + { NCSI_PKT_RSP_OEM, -1, ncsi_rsp_handler_oem }, > > { NCSI_PKT_RSP_PLDM, 0, NULL }, > > { NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid } > > };
> Thanks for the overview. We look forward to your patches; please > include the same cc list as this series. > I think it makes sense to have some OEM NCSI handing purely in the > kenrel. This would allow eg. the MAC address of an interface to be > correct at boot, without requiring userspace to come up first. > I have also heard stories of temperature sensors over NCSI. Those > would make sense to be hwmon drivers, which again would mean handling > them in the kernel. > Justin, Vijay, can you please list the known NCSI OEM > commands/extensions that we plan on implementing? In our implementation, All OEM commands are transparent to Kernel. We don't plan to add any specific handler in the Kernel. Thanks, Justin
On 9/26/18, 8:44 PM, "Samuel Mendoza-Jonas" <sam@mendozajonas.com> wrote: > Hi Vijay, > Having had a chance to take a closer look, there is probably room for > both this patchset and Justin's potential changes to coexist; while > Justin's is a more general solution for sending arbitrary commands, the > approach of this patch is also useful for handling commands we want > included in the configure process (such as get-mac-address). Hi Sam, I have created a more generic approach based patch, will send you after clean up. I agree we need both Justin patch as well as mine to handle OEM specific command. I am creating 2 patches one for generic OEM command and response handling and another is OEM vendor specific command handling. Please see my responses to your comments below. Some comments below: > --- > net/ncsi/Kconfig | 3 ++ > net/ncsi/internal.h | 11 +++++-- > net/ncsi/ncsi-cmd.c | 24 +++++++++++++-- > net/ncsi/ncsi-manage.c | 68 ++++++++++++++++++++++++++++++++++++++++++ > net/ncsi/ncsi-pkt.h | 16 ++++++++++ > net/ncsi/ncsi-rsp.c | 33 +++++++++++++++++++- > 6 files changed, 149 insertions(+), 6 deletions(-) > > diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig > index 08a8a6031fd7..b8bf89fea7c8 100644 > --- a/net/ncsi/Kconfig > +++ b/net/ncsi/Kconfig > @@ -10,3 +10,6 @@ config NET_NCSI > support. Enable this only if your system connects to a network > device via NCSI and the ethernet driver you're using supports > the protocol explicitly. > +config NCSI_OEM_CMD_GET_MAC > + bool "Get NCSI OEM MAC Address" > + depends on NET_NCSI For the moment this isn't too bad but I wonder if in the future it would be more flexible to have something like NCSI_OEM_CMD_MELLANOX etc, so we could selectively enable a class of OEM commands based on vendor rather than per-command. Certainly, we can have like that and will be an addition to above config. Above config is to enable retrieving and setting mac address from device during channel configuration. And then we can have vendor selection like MELLANOX, Broadcom etc. But I will rather check GV ID under this config and see if there is available function for specific vendor. This can be revised and made more generic based on vendor. > diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h > index 8055e3965cef..da17958e6a4b 100644 > --- a/net/ncsi/internal.h > +++ b/net/ncsi/internal.h > @@ -68,6 +68,10 @@ enum { > NCSI_MODE_MAX > }; > > +#define NCSI_OEM_MFR_MLX_ID 0x8119 > +#define NCSI_OEM_MLX_CMD_GET_MAC 0x1b00 > +#define NCSI_OEM_MLX_CMD_SET_AFFINITY 0x010700 I gather this is part of the OEM command but it would be good to describe what these bits mean. Is this command documented anywhere by Mellanox? I will add more description here, please see in next patch. Yes Mellanox specification describes these commands. > + > struct ncsi_channel_version { > u32 version; /* Supported BCD encoded NCSI version */ > u32 alpha2; /* Supported BCD encoded NCSI version */ > @@ -236,6 +240,7 @@ enum { > ncsi_dev_state_probe_dp, > ncsi_dev_state_config_sp = 0x0301, > ncsi_dev_state_config_cis, > + ncsi_dev_state_config_oem_gma, > ncsi_dev_state_config_clear_vids, > ncsi_dev_state_config_svf, > ncsi_dev_state_config_ev, > @@ -301,9 +306,9 @@ struct ncsi_cmd_arg { > unsigned short payload; /* Command packet payload length */ > unsigned int req_flags; /* NCSI request properties */ > union { > - unsigned char bytes[16]; /* Command packet specific data */ > - unsigned short words[8]; > - unsigned int dwords[4]; > + unsigned char bytes[64]; /* Command packet specific data */ > + unsigned short words[32]; > + unsigned int dwords[16]; > }; > }; > > diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c > index 7567ca63aae2..3205e22c1734 100644 > --- a/net/ncsi/ncsi-cmd.c > +++ b/net/ncsi/ncsi-cmd.c > @@ -211,6 +211,25 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb, > return 0; > } > > +static int ncsi_cmd_handler_oem(struct sk_buff *skb, > + struct ncsi_cmd_arg *nca) > +{ > + struct ncsi_cmd_oem_pkt *cmd; > + unsigned int len; > + > + len = sizeof(struct ncsi_cmd_pkt_hdr) + 4; > + if (nca->payload < 26) > + len += 26; This will have already happened in ncsi_alloc_command(), is this check needed? Yes it is needed to find length for skbuff data to initialize. ncsi_alloc_command() does the same and allocate skbuff. > + else > + len += nca->payload; > + > + cmd = skb_put_zero(skb, len); > + memcpy(cmd->data, nca->bytes, nca->payload); > + ncsi_cmd_build_header(&cmd->cmd.common, nca); > + > + return 0; > +} > + > static struct ncsi_cmd_handler { > unsigned char type; > int payload; > @@ -244,7 +263,7 @@ static struct ncsi_cmd_handler { > { NCSI_PKT_CMD_GNS, 0, ncsi_cmd_handler_default }, > { NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default }, > { NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default }, > - { NCSI_PKT_CMD_OEM, 0, NULL }, > + { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem }, > { NCSI_PKT_CMD_PLDM, 0, NULL }, > { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default } > }; > @@ -317,7 +336,8 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca) > } > > /* Get packet payload length and allocate the request */ > - nca->payload = nch->payload; > + if (nch->payload >= 0) > + nca->payload = nch->payload; I think with this there is a chance of nca->payload being uninitialised and then used in ncsi_alloc_command(). Can you describe what the aim here is? I will add more description here. > nr = ncsi_alloc_command(nca); > if (!nr) > return -ENOMEM; > diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c > index 091284760d21..3b2b86560cc8 100644 > --- a/net/ncsi/ncsi-manage.c > +++ b/net/ncsi/ncsi-manage.c > @@ -635,6 +635,58 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc, > return 0; > } > > +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC) > +/* NCSI Facebook OEM APIs */ Are these Facebook OEM commands or Mellanox OEM commands? Will be taken care in next patch > +static void get_mac_address_oem_mlx(struct ncsi_dev_priv *ndp) > +{ > + struct ncsi_cmd_arg nca; > + int ret = 0; > + > + memset(&nca, 0, sizeof(struct ncsi_cmd_arg)); > + nca.ndp = ndp; > + nca.channel = ndp->active_channel->id; > + nca.package = ndp->active_package->id; > + nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN; > + nca.type = NCSI_PKT_CMD_OEM; > + nca.payload = 8; > + > + nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID); > + nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_GET_MAC); > + > + ret = ncsi_xmit_cmd(&nca); > + if (ret) > + netdev_err(ndp->ndev.dev, > + "NCSI: Failed to transmit cmd 0x%x during probe\n", > + nca.type); > +} > + > +static void set_mac_affinity_mlx(struct ncsi_dev_priv *ndp) > +{ > + struct ncsi_cmd_arg nca; > + int ret = 0; > + > + memset(&nca, 0, sizeof(struct ncsi_cmd_arg)); Ah I see we initialise nca, and thus payload here - the path between these two points in the driver isn't super obvious (not this patch's fault :) ), it might be better to explicitly mention/handle this where we use payload later on. Yes, I agree, will add more details. > + nca.ndp = ndp; > + nca.channel = ndp->active_channel->id; > + nca.package = ndp->active_package->id; These should probably be set in ncsi_configure_channel (eg. see the CIS state before these are called). These functions are being called from ncsi_configure_channel() only. We can either initialize nca there in ncsi_configure_channel() function and pass nca as function parameter or initialize nca inside function and populate required field. > + nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN; > + nca.type = NCSI_PKT_CMD_OEM; > + nca.payload = 60; > + > + nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID); > + nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_SET_AFFINITY); > + > + memcpy(&(nca.bytes[8]), ndp->ndev.dev->dev_addr, ETH_ALEN); > + nca.bytes[14] = 0x09; > + > + ret = ncsi_xmit_cmd(&nca); > + if (ret) > + netdev_err(ndp->ndev.dev, > + "NCSI: Failed to transmit cmd 0x%x during probe\n", > + nca.type); > +} > +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */ > + > static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) > { > struct ncsi_dev *nd = &ndp->ndev; > @@ -685,6 +737,22 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) > goto error; > } > > +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC) > + /* Check Manufacture id if it is Mellanox then > + * get and set mac address. To Do: Add code for > + * other types of card if required > + */ > + if (nc->version.mf_id == NCSI_OEM_MFR_MLX_ID) > + nd->state = ncsi_dev_state_config_oem_gma; > + else > + nd->state = ncsi_dev_state_config_clear_vids; > + break; > + case ncsi_dev_state_config_oem_gma: > + ndp->pending_req_num = 2; > + get_mac_address_oem_mlx(ndp); > + msleep(500); Is this msleep() required? It is required to make sure previous command completed. Will try to handle it differently > + set_mac_affinity_mlx(ndp); Since this *sets* the MAC address, should we name the state and config option more accurately? How does this OEM command interact with the NCSI set-mac-address command? Does this command depend on the previous get_mac_address_oem_mlx() command succeeding? It is independent of Set_mac_address. Yes it depends on succession of get_mac_address_oem_mlx() that's why msleep was put there. > +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */ > nd->state = ncsi_dev_state_config_clear_vids; > break; > case ncsi_dev_state_config_clear_vids: > diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h > index 91b4b66438df..0653a893eb12 100644 > --- a/net/ncsi/ncsi-pkt.h > +++ b/net/ncsi/ncsi-pkt.h > @@ -151,6 +151,22 @@ struct ncsi_cmd_snfc_pkt { > unsigned char pad[22]; > }; > > +/* Oem Request Command */ In general, s/Oem/OEM Sure, will be done. > +struct ncsi_cmd_oem_pkt { > + struct ncsi_cmd_pkt_hdr cmd; /* Command header */ > + unsigned char data[64]; /* OEM Payload Data */ > + __be32 checksum; /* Checksum */ > +}; > + > +/* Oem Response Packet */ > +struct ncsi_rsp_oem_pkt { > + struct ncsi_rsp_pkt_hdr rsp; /* Command header */ > + __be32 mfr_id; /* Manufacture ID */ > + __be32 oem_cmd; /* oem command */ > + unsigned char data[32]; /* Payload data */ > + __be32 checksum; /* Checksum */ > +}; > + > /* Get Link Status */ > struct ncsi_rsp_gls_pkt { > struct ncsi_rsp_pkt_hdr rsp; /* Response header */ > diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c > index 930c1d3796f0..3b94c96b9c7f 100644 > --- a/net/ncsi/ncsi-rsp.c > +++ b/net/ncsi/ncsi-rsp.c > @@ -596,6 +596,37 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr) > return 0; > } > > +static int ncsi_rsp_handler_oem(struct ncsi_request *nr) > +{ > + struct ncsi_rsp_oem_pkt *rsp; > + struct ncsi_dev_priv *ndp = nr->ndp; > + struct net_device *ndev = ndp->ndev.dev; > + int ret = 0; > + unsigned int oem_cmd, mfr_id; > + const struct net_device_ops *ops = ndev->netdev_ops; > + struct sockaddr saddr; > + > + /* Get the response header */ > + rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp); > + > + oem_cmd = ntohl(rsp->oem_cmd); > + mfr_id = ntohl(rsp->mfr_id); > + > + /* Check for Mellanox manufacturer id */ > + if (mfr_id != NCSI_OEM_MFR_MLX_ID) > + return 0; > + > + if (oem_cmd == NCSI_OEM_MLX_CMD_GET_MAC) { > + saddr.sa_family = ndev->type; > + ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE; > + memcpy(saddr.sa_data, &(rsp->data[4]), ETH_ALEN); > + ret = ops->ndo_set_mac_address(ndev, &saddr); > + if (ret < 0) > + netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n"); > + } > + return ret; > +} > + > static int ncsi_rsp_handler_gvi(struct ncsi_request *nr) > { > struct ncsi_rsp_gvi_pkt *rsp; > @@ -932,7 +963,7 @@ static struct ncsi_rsp_handler { > { NCSI_PKT_RSP_GNS, 172, ncsi_rsp_handler_gns }, > { NCSI_PKT_RSP_GNPTS, 172, ncsi_rsp_handler_gnpts }, > { NCSI_PKT_RSP_GPS, 8, ncsi_rsp_handler_gps }, > - { NCSI_PKT_RSP_OEM, 0, NULL }, > + { NCSI_PKT_RSP_OEM, -1, ncsi_rsp_handler_oem }, > { NCSI_PKT_RSP_PLDM, 0, NULL }, > { NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid } > };
diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig index 08a8a6031fd7..b8bf89fea7c8 100644 --- a/net/ncsi/Kconfig +++ b/net/ncsi/Kconfig @@ -10,3 +10,6 @@ config NET_NCSI support. Enable this only if your system connects to a network device via NCSI and the ethernet driver you're using supports the protocol explicitly. +config NCSI_OEM_CMD_GET_MAC + bool "Get NCSI OEM MAC Address" + depends on NET_NCSI diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h index 8055e3965cef..da17958e6a4b 100644 --- a/net/ncsi/internal.h +++ b/net/ncsi/internal.h @@ -68,6 +68,10 @@ enum { NCSI_MODE_MAX }; +#define NCSI_OEM_MFR_MLX_ID 0x8119 +#define NCSI_OEM_MLX_CMD_GET_MAC 0x1b00 +#define NCSI_OEM_MLX_CMD_SET_AFFINITY 0x010700 + struct ncsi_channel_version { u32 version; /* Supported BCD encoded NCSI version */ u32 alpha2; /* Supported BCD encoded NCSI version */ @@ -236,6 +240,7 @@ enum { ncsi_dev_state_probe_dp, ncsi_dev_state_config_sp = 0x0301, ncsi_dev_state_config_cis, + ncsi_dev_state_config_oem_gma, ncsi_dev_state_config_clear_vids, ncsi_dev_state_config_svf, ncsi_dev_state_config_ev, @@ -301,9 +306,9 @@ struct ncsi_cmd_arg { unsigned short payload; /* Command packet payload length */ unsigned int req_flags; /* NCSI request properties */ union { - unsigned char bytes[16]; /* Command packet specific data */ - unsigned short words[8]; - unsigned int dwords[4]; + unsigned char bytes[64]; /* Command packet specific data */ + unsigned short words[32]; + unsigned int dwords[16]; }; }; diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c index 7567ca63aae2..3205e22c1734 100644 --- a/net/ncsi/ncsi-cmd.c +++ b/net/ncsi/ncsi-cmd.c @@ -211,6 +211,25 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb, return 0; } +static int ncsi_cmd_handler_oem(struct sk_buff *skb, + struct ncsi_cmd_arg *nca) +{ + struct ncsi_cmd_oem_pkt *cmd; + unsigned int len; + + len = sizeof(struct ncsi_cmd_pkt_hdr) + 4; + if (nca->payload < 26) + len += 26; + else + len += nca->payload; + + cmd = skb_put_zero(skb, len); + memcpy(cmd->data, nca->bytes, nca->payload); + ncsi_cmd_build_header(&cmd->cmd.common, nca); + + return 0; +} + static struct ncsi_cmd_handler { unsigned char type; int payload; @@ -244,7 +263,7 @@ static struct ncsi_cmd_handler { { NCSI_PKT_CMD_GNS, 0, ncsi_cmd_handler_default }, { NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default }, { NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default }, - { NCSI_PKT_CMD_OEM, 0, NULL }, + { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem }, { NCSI_PKT_CMD_PLDM, 0, NULL }, { NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default } }; @@ -317,7 +336,8 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca) } /* Get packet payload length and allocate the request */ - nca->payload = nch->payload; + if (nch->payload >= 0) + nca->payload = nch->payload; nr = ncsi_alloc_command(nca); if (!nr) return -ENOMEM; diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c index 091284760d21..3b2b86560cc8 100644 --- a/net/ncsi/ncsi-manage.c +++ b/net/ncsi/ncsi-manage.c @@ -635,6 +635,58 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc, return 0; } +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC) +/* NCSI Facebook OEM APIs */ +static void get_mac_address_oem_mlx(struct ncsi_dev_priv *ndp) +{ + struct ncsi_cmd_arg nca; + int ret = 0; + + memset(&nca, 0, sizeof(struct ncsi_cmd_arg)); + nca.ndp = ndp; + nca.channel = ndp->active_channel->id; + nca.package = ndp->active_package->id; + nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN; + nca.type = NCSI_PKT_CMD_OEM; + nca.payload = 8; + + nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID); + nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_GET_MAC); + + ret = ncsi_xmit_cmd(&nca); + if (ret) + netdev_err(ndp->ndev.dev, + "NCSI: Failed to transmit cmd 0x%x during probe\n", + nca.type); +} + +static void set_mac_affinity_mlx(struct ncsi_dev_priv *ndp) +{ + struct ncsi_cmd_arg nca; + int ret = 0; + + memset(&nca, 0, sizeof(struct ncsi_cmd_arg)); + nca.ndp = ndp; + nca.channel = ndp->active_channel->id; + nca.package = ndp->active_package->id; + nca.req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN; + nca.type = NCSI_PKT_CMD_OEM; + nca.payload = 60; + + nca.dwords[0] = ntohl(NCSI_OEM_MFR_MLX_ID); + nca.dwords[1] = ntohl(NCSI_OEM_MLX_CMD_SET_AFFINITY); + + memcpy(&(nca.bytes[8]), ndp->ndev.dev->dev_addr, ETH_ALEN); + nca.bytes[14] = 0x09; + + ret = ncsi_xmit_cmd(&nca); + if (ret) + netdev_err(ndp->ndev.dev, + "NCSI: Failed to transmit cmd 0x%x during probe\n", + nca.type); +} +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */ + static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) { struct ncsi_dev *nd = &ndp->ndev; @@ -685,6 +737,22 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp) goto error; } +#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC) + /* Check Manufacture id if it is Mellanox then + * get and set mac address. To Do: Add code for + * other types of card if required + */ + if (nc->version.mf_id == NCSI_OEM_MFR_MLX_ID) + nd->state = ncsi_dev_state_config_oem_gma; + else + nd->state = ncsi_dev_state_config_clear_vids; + break; + case ncsi_dev_state_config_oem_gma: + ndp->pending_req_num = 2; + get_mac_address_oem_mlx(ndp); + msleep(500); + set_mac_affinity_mlx(ndp); +#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */ nd->state = ncsi_dev_state_config_clear_vids; break; case ncsi_dev_state_config_clear_vids: diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h index 91b4b66438df..0653a893eb12 100644 --- a/net/ncsi/ncsi-pkt.h +++ b/net/ncsi/ncsi-pkt.h @@ -151,6 +151,22 @@ struct ncsi_cmd_snfc_pkt { unsigned char pad[22]; }; +/* Oem Request Command */ +struct ncsi_cmd_oem_pkt { + struct ncsi_cmd_pkt_hdr cmd; /* Command header */ + unsigned char data[64]; /* OEM Payload Data */ + __be32 checksum; /* Checksum */ +}; + +/* Oem Response Packet */ +struct ncsi_rsp_oem_pkt { + struct ncsi_rsp_pkt_hdr rsp; /* Command header */ + __be32 mfr_id; /* Manufacture ID */ + __be32 oem_cmd; /* oem command */ + unsigned char data[32]; /* Payload data */ + __be32 checksum; /* Checksum */ +}; + /* Get Link Status */ struct ncsi_rsp_gls_pkt { struct ncsi_rsp_pkt_hdr rsp; /* Response header */ diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c index 930c1d3796f0..3b94c96b9c7f 100644 --- a/net/ncsi/ncsi-rsp.c +++ b/net/ncsi/ncsi-rsp.c @@ -596,6 +596,37 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr) return 0; } +static int ncsi_rsp_handler_oem(struct ncsi_request *nr) +{ + struct ncsi_rsp_oem_pkt *rsp; + struct ncsi_dev_priv *ndp = nr->ndp; + struct net_device *ndev = ndp->ndev.dev; + int ret = 0; + unsigned int oem_cmd, mfr_id; + const struct net_device_ops *ops = ndev->netdev_ops; + struct sockaddr saddr; + + /* Get the response header */ + rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp); + + oem_cmd = ntohl(rsp->oem_cmd); + mfr_id = ntohl(rsp->mfr_id); + + /* Check for Mellanox manufacturer id */ + if (mfr_id != NCSI_OEM_MFR_MLX_ID) + return 0; + + if (oem_cmd == NCSI_OEM_MLX_CMD_GET_MAC) { + saddr.sa_family = ndev->type; + ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE; + memcpy(saddr.sa_data, &(rsp->data[4]), ETH_ALEN); + ret = ops->ndo_set_mac_address(ndev, &saddr); + if (ret < 0) + netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n"); + } + return ret; +} + static int ncsi_rsp_handler_gvi(struct ncsi_request *nr) { struct ncsi_rsp_gvi_pkt *rsp; @@ -932,7 +963,7 @@ static struct ncsi_rsp_handler { { NCSI_PKT_RSP_GNS, 172, ncsi_rsp_handler_gns }, { NCSI_PKT_RSP_GNPTS, 172, ncsi_rsp_handler_gnpts }, { NCSI_PKT_RSP_GPS, 8, ncsi_rsp_handler_gps }, - { NCSI_PKT_RSP_OEM, 0, NULL }, + { NCSI_PKT_RSP_OEM, -1, ncsi_rsp_handler_oem }, { NCSI_PKT_RSP_PLDM, 0, NULL }, { NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid } };
This patch adds OEM command to get mac address from NCSI device and and configure the same to the network card. ncsi_cmd_arg - Modified this structure to include bigger payload data. ncsi_cmd_handler_oem: This function handles oem command request ncsi_rsp_handler_oem: This function handles response for OEM command. get_mac_address_oem_mlx: This function will send OEM command to get mac address for Mellanox card set_mac_affinity_mlx: This will send OEM command to set Mac affinity for Mellanox card Signed-off-by: Vijay Khemka <vijaykhemka@fb.com> --- net/ncsi/Kconfig | 3 ++ net/ncsi/internal.h | 11 +++++-- net/ncsi/ncsi-cmd.c | 24 +++++++++++++-- net/ncsi/ncsi-manage.c | 68 ++++++++++++++++++++++++++++++++++++++++++ net/ncsi/ncsi-pkt.h | 16 ++++++++++ net/ncsi/ncsi-rsp.c | 33 +++++++++++++++++++- 6 files changed, 149 insertions(+), 6 deletions(-)