[PATCH dev-5.6 1/3] net/ncsi: Simplify Kconfig/dts control flow

[PATCH dev-5.6 1/3] net/ncsi: Simplify Kconfig/dts control flow

[Patrick Williams]

From: Peter Delevoryas <peter@pjd.dev>

Background:

1. CONFIG_NCSI_OEM_CMD_KEEP_PHY

If this is enabled, we send an extra OEM Intel command in the probe
sequence immediately after discovering a channel (e.g. after "Clear
Initial State").

2. CONFIG_NCSI_OEM_CMD_GET_MAC

If this is enabled, we send one of 3 OEM "Get MAC Address" commands from
Broadcom, Mellanox (Nvidida), and Intel in the *configuration* sequence
for a channel.

3. mellanox,multi-host (or mlx,multi-host)

Introduced by this patch:

https://lore.kernel.org/all/20200108234341.2590674-1-vijaykhemka@fb.com/

Which was actually originally from cosmo.chou@quantatw.com:

https://github.com/facebook/openbmc-linux/commit/9f132a10ec48db84613519258cd8a317fb9c8f1b

Cosmo claimed that the Nvidia ConnectX-4 and ConnectX-6 NIC's don't
respond to Get Version ID, et. al in the probe sequence unless you send
the Set MC Affinity command first.

Problem Statement:

We've been using a combination of #ifdef code blocks and IS_ENABLED()
conditions to conditionally send these OEM commands.

It makes adding any new code around these commands hard to understand.

Solution:

In this patch, I just want to remove the conditionally compiled blocks
of code, and always use IS_ENABLED(...) to do dynamic control flow.

I don't think the small amount of code this adds to non-users of the OEM
Kconfigs is a big deal.

Signed-off-by: Peter Delevoryas <peter@pjd.dev>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit c797ce168930ce3d62a9b7fc4d7040963ee6a01e)
---
 net/ncsi/ncsi-manage.c | 20 +++-----------------
 1 file changed, 3 insertions(+), 17 deletions(-)

diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index d9da942ad53d..f3d7fe86fea1 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -689,8 +689,6 @@ static int set_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc,
 	return 0;
 }
 
-#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_KEEP_PHY)
-
 static int ncsi_oem_keep_phy_intel(struct ncsi_cmd_arg *nca)
 {
 	unsigned char data[NCSI_OEM_INTEL_CMD_KEEP_PHY_LEN];
@@ -716,10 +714,6 @@ static int ncsi_oem_keep_phy_intel(struct ncsi_cmd_arg *nca)
 	return ret;
 }
 
-#endif
-
-#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
-
 /* NCSI OEM Command APIs */
 static int ncsi_oem_gma_handler_bcm(struct ncsi_cmd_arg *nca)
 {
@@ -856,8 +850,6 @@ static int ncsi_gma_handler(struct ncsi_cmd_arg *nca, unsigned int mf_id)
 	return nch->handler(nca);
 }
 
-#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
-
 /* Determine if a given channel from the channel_queue should be used for Tx */
 static bool ncsi_channel_is_tx(struct ncsi_dev_priv *ndp,
 			       struct ncsi_channel *nc)
@@ -1039,20 +1031,18 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
 			goto error;
 		}
 
-		nd->state = ncsi_dev_state_config_oem_gma;
+		nd->state = IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
+			  ? ncsi_dev_state_config_oem_gma
+			  : ncsi_dev_state_config_clear_vids;
 		break;
 	case ncsi_dev_state_config_oem_gma:
 		nd->state = ncsi_dev_state_config_clear_vids;
-		ret = -1;
 
-#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
 		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);
-#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
-
 		if (ret < 0)
 			schedule_work(&ndp->work);
 
@@ -1404,7 +1394,6 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
 
 		schedule_work(&ndp->work);
 		break;
-#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_GET_MAC)
 	case ncsi_dev_state_probe_mlx_gma:
 		ndp->pending_req_num = 1;
 
@@ -1429,7 +1418,6 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
 
 		nd->state = ncsi_dev_state_probe_cis;
 		break;
-#endif /* CONFIG_NCSI_OEM_CMD_GET_MAC */
 	case ncsi_dev_state_probe_cis:
 		ndp->pending_req_num = NCSI_RESERVED_CHANNEL;
 
@@ -1447,7 +1435,6 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
 		if (IS_ENABLED(CONFIG_NCSI_OEM_CMD_KEEP_PHY))
 			nd->state = ncsi_dev_state_probe_keep_phy;
 		break;
-#if IS_ENABLED(CONFIG_NCSI_OEM_CMD_KEEP_PHY)
 	case ncsi_dev_state_probe_keep_phy:
 		ndp->pending_req_num = 1;
 
@@ -1460,7 +1447,6 @@ static void ncsi_probe_channel(struct ncsi_dev_priv *ndp)
 
 		nd->state = ncsi_dev_state_probe_gvi;
 		break;
-#endif /* CONFIG_NCSI_OEM_CMD_KEEP_PHY */
 	case ncsi_dev_state_probe_gvi:
 	case ncsi_dev_state_probe_gc:
 	case ncsi_dev_state_probe_gls:
-- 
2.41.0

[PATCH dev-5.6 2/3] net/ncsi: Fix netlink major/minor version numbers

[Patrick Williams]

From: Peter Delevoryas <peter@pjd.dev>

The netlink interface for major and minor version numbers doesn't actually
return the major and minor version numbers.

It reports a u32 that contains the (major, minor, update, alpha1)
components as the major version number, and then alpha2 as the minor
version number.

For whatever reason, the u32 byte order was reversed (ntohl): maybe it was
assumed that the encoded value was a single big-endian u32, and alpha2 was
the minor version.

The correct way to get the supported NC-SI version from the network
controller is to parse the Get Version ID response as described in 8.4.44
of the NC-SI spec[1].

    Get Version ID Response Packet Format

              Bits
            +--------+--------+--------+--------+
     Bytes  | 31..24 | 23..16 | 15..8  | 7..0   |
    +-------+--------+--------+--------+--------+
    | 0..15 | NC-SI Header                      |
    +-------+--------+--------+--------+--------+
    | 16..19| Response code   | Reason code     |
    +-------+--------+--------+--------+--------+
    |20..23 | Major  | Minor  | Update | Alpha1 |
    +-------+--------+--------+--------+--------+
    |24..27 |         reserved         | Alpha2 |
    +-------+--------+--------+--------+--------+
    |            .... other stuff ....          |

The major, minor, and update fields are all binary-coded decimal (BCD)
encoded [2]. The spec provides examples below the Get Version ID response
format in section 8.4.44.1, but for practical purposes, this is an example
from a live network card:

    root@bmc:~# ncsi-util 0x15
    NC-SI Command Response:
    cmd: GET_VERSION_ID(0x15)
    Response: COMMAND_COMPLETED(0x0000)  Reason: NO_ERROR(0x0000)
    Payload length = 40

    20: 0xf1 0xf1 0xf0 0x00 <<<<<<<<< (major, minor, update, alpha1)
    24: 0x00 0x00 0x00 0x00 <<<<<<<<< (_, _, _, alpha2)

    28: 0x6d 0x6c 0x78 0x30
    32: 0x2e 0x31 0x00 0x00
    36: 0x00 0x00 0x00 0x00
    40: 0x16 0x1d 0x07 0xd2
    44: 0x10 0x1d 0x15 0xb3
    48: 0x00 0x17 0x15 0xb3
    52: 0x00 0x00 0x81 0x19

This should be parsed as "1.1.0".

"f" in the upper-nibble means to ignore it, contributing zero.

If both nibbles are "f", I think the whole field is supposed to be ignored.
Major and minor are "required", meaning they're not supposed to be "ff",
but the update field is "optional" so I think it can be ff. I think the
simplest thing to do is just set the major and minor to zero instead of
juggling some conditional logic or something.

bcd2bin() from "include/linux/bcd.h" seems to assume both nibbles are 0-9,
so I've provided a custom BCD decoding function.

Alpha1 and alpha2 are ISO/IEC 8859-1 encoded, which just means ASCII
characters as far as I can tell, although the full encoding table for
non-alphabetic characters is slightly different (I think).

I imagine the alpha fields are just supposed to be alphabetic characters,
but I haven't seen any network cards actually report a non-zero value for
either.

If people wrote software against this netlink behavior, and were parsing
the major and minor versions themselves from the u32, then this would
definitely break their code.

[1] https://www.dmtf.org/sites/default/files/standards/documents/DSP0222_1.0.0.pdf
[2] https://en.wikipedia.org/wiki/Binary-coded_decimal
[2] https://en.wikipedia.org/wiki/ISO/IEC_8859-1

Signed-off-by: Peter Delevoryas <peter@pjd.dev>
Fixes: 138635cc27c9 ("net/ncsi: NCSI response packet handler")
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 3084b58bfd0b9e4b5e034f31f31b42977db35f12)
---
 net/ncsi/internal.h     |  7 +++++--
 net/ncsi/ncsi-netlink.c |  4 ++--
 net/ncsi/ncsi-pkt.h     |  7 +++++--
 net/ncsi/ncsi-rsp.c     | 26 ++++++++++++++++++++++++--
 4 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 03757e76bb6b..374412ed780b 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -105,8 +105,11 @@ enum {
 
 
 struct ncsi_channel_version {
-	u32 version;		/* Supported BCD encoded NCSI version */
-	u32 alpha2;		/* Supported BCD encoded NCSI version */
+	u8   major;		/* NCSI version major */
+	u8   minor;		/* NCSI version minor */
+	u8   update;		/* NCSI version update */
+	char alpha1;		/* NCSI version alpha1 */
+	char alpha2;		/* NCSI version alpha2 */
 	u8  fw_name[12];	/* Firmware name string                */
 	u32 fw_version;		/* Firmware version                   */
 	u16 pci_ids[4];		/* PCI identification                 */
diff --git a/net/ncsi/ncsi-netlink.c b/net/ncsi/ncsi-netlink.c
index d27f4eccce6d..fe681680b5d9 100644
--- a/net/ncsi/ncsi-netlink.c
+++ b/net/ncsi/ncsi-netlink.c
@@ -71,8 +71,8 @@ static int ncsi_write_channel_info(struct sk_buff *skb,
 	if (nc == nc->package->preferred_channel)
 		nla_put_flag(skb, NCSI_CHANNEL_ATTR_FORCED);
 
-	nla_put_u32(skb, NCSI_CHANNEL_ATTR_VERSION_MAJOR, nc->version.version);
-	nla_put_u32(skb, NCSI_CHANNEL_ATTR_VERSION_MINOR, nc->version.alpha2);
+	nla_put_u32(skb, NCSI_CHANNEL_ATTR_VERSION_MAJOR, nc->version.major);
+	nla_put_u32(skb, NCSI_CHANNEL_ATTR_VERSION_MINOR, nc->version.minor);
 	nla_put_string(skb, NCSI_CHANNEL_ATTR_VERSION_STR, nc->version.fw_name);
 
 	vid_nest = nla_nest_start_noflag(skb, NCSI_CHANNEL_ATTR_VLAN_LIST);
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index ba66c7dc3a21..c9d1da34dc4d 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -197,9 +197,12 @@ struct ncsi_rsp_gls_pkt {
 /* Get Version ID */
 struct ncsi_rsp_gvi_pkt {
 	struct ncsi_rsp_pkt_hdr rsp;          /* Response header */
-	__be32                  ncsi_version; /* NCSI version    */
+	unsigned char           major;        /* NCSI version major */
+	unsigned char           minor;        /* NCSI version minor */
+	unsigned char           update;       /* NCSI version update */
+	unsigned char           alpha1;       /* NCSI version alpha1 */
 	unsigned char           reserved[3];  /* Reserved        */
-	unsigned char           alpha2;       /* NCSI version    */
+	unsigned char           alpha2;       /* NCSI version alpha2 */
 	unsigned char           fw_name[12];  /* f/w name string */
 	__be32                  fw_version;   /* f/w version     */
 	__be16                  pci_ids[4];   /* PCI IDs         */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 069c2659074b..480e80e3c283 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -19,6 +19,19 @@
 #include "ncsi-pkt.h"
 #include "ncsi-netlink.h"
 
+/* Nibbles within [0xA, 0xF] add zero "0" to the returned value.
+ * Optional fields (encoded as 0xFF) will default to zero.
+ */
+static u8 decode_bcd_u8(u8 x)
+{
+	int lo = x & 0xF;
+	int hi = x >> 4;
+
+	lo = lo < 0xA ? lo : 0;
+	hi = hi < 0xA ? hi : 0;
+	return lo + hi * 10;
+}
+
 static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
 				 unsigned short payload)
 {
@@ -755,9 +768,18 @@ static int ncsi_rsp_handler_gvi(struct ncsi_request *nr)
 	if (!nc)
 		return -ENODEV;
 
-	/* Update to channel's version info */
+	/* Update channel's version info
+	 *
+	 * Major, minor, and update fields are supposed to be
+	 * unsigned integers encoded as packed BCD.
+	 *
+	 * Alpha1 and alpha2 are ISO/IEC 8859-1 characters.
+	 */
 	ncv = &nc->version;
-	ncv->version = ntohl(rsp->ncsi_version);
+	ncv->major = decode_bcd_u8(rsp->major);
+	ncv->minor = decode_bcd_u8(rsp->minor);
+	ncv->update = decode_bcd_u8(rsp->update);
+	ncv->alpha1 = rsp->alpha1;
 	ncv->alpha2 = rsp->alpha2;
 	memcpy(ncv->fw_name, rsp->fw_name, 12);
 	ncv->fw_version = ntohl(rsp->fw_version);
-- 
2.41.0

[PATCH dev-5.6 3/3] net/ncsi: Add NC-SI 1.2 Get MC MAC Address command

[Patrick Williams]

From: Peter Delevoryas <peter@pjd.dev>

This change adds support for the NC-SI 1.2 Get MC MAC Address command,
specified here:

https://www.dmtf.org/sites/default/files/standards/documents/DSP0222_1.2.0.pdf

It serves the exact same function as the existing OEM Get MAC Address
commands, so if a channel reports that it supports NC-SI 1.2, we prefer
to use the standard command rather than the OEM command.

Verified with an invalid MAC address and 2 valid ones:

[   55.137072] ftgmac100 1e690000.ftgmac eth0: NCSI: Received 3 provisioned MAC addresses
[   55.137614] ftgmac100 1e690000.ftgmac eth0: NCSI: MAC address 0: 00:00:00:00:00:00
[   55.138026] ftgmac100 1e690000.ftgmac eth0: NCSI: MAC address 1: fa:ce:b0:0c:20:22
[   55.138528] ftgmac100 1e690000.ftgmac eth0: NCSI: MAC address 2: fa:ce:b0:0c:20:23
[   55.139241] ftgmac100 1e690000.ftgmac eth0: NCSI: Unable to assign 00:00:00:00:00:00 to device
[   55.140098] ftgmac100 1e690000.ftgmac eth0: NCSI: Set MAC address to fa:ce:b0:0c:20:22

Signed-off-by: Peter Delevoryas <peter@pjd.dev>
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit b8291cf3d1180b5b61299922f17c9441616a805a)
---
 net/ncsi/ncsi-cmd.c    |  3 ++-
 net/ncsi/ncsi-manage.c |  9 +++++++--
 net/ncsi/ncsi-pkt.h    | 10 ++++++++++
 net/ncsi/ncsi-rsp.c    | 41 ++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index fd2236ee9a79..b3ff37a181d7 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -270,7 +270,8 @@ static struct ncsi_cmd_handler {
 	{ NCSI_PKT_CMD_GPS,    0, ncsi_cmd_handler_default },
 	{ NCSI_PKT_CMD_OEM,   -1, ncsi_cmd_handler_oem     },
 	{ NCSI_PKT_CMD_PLDM,   0, NULL                     },
-	{ NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }
+	{ NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default },
+	{ NCSI_PKT_CMD_GMCMA,  0, ncsi_cmd_handler_default }
 };
 
 static struct ncsi_request *ncsi_alloc_command(struct ncsi_cmd_arg *nca)
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index f3d7fe86fea1..745c788f1d1d 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1038,11 +1038,16 @@ static void ncsi_configure_channel(struct ncsi_dev_priv *ndp)
 	case ncsi_dev_state_config_oem_gma:
 		nd->state = ncsi_dev_state_config_clear_vids;
 
-		nca.type = NCSI_PKT_CMD_OEM;
 		nca.package = np->id;
 		nca.channel = nc->id;
 		ndp->pending_req_num = 1;
-		ret = ncsi_gma_handler(&nca, nc->version.mf_id);
+		if (nc->version.major >= 1 && nc->version.minor >= 2) {
+			nca.type = NCSI_PKT_CMD_GMCMA;
+			ret = ncsi_xmit_cmd(&nca);
+		} else {
+			nca.type = NCSI_PKT_CMD_OEM;
+			ret = ncsi_gma_handler(&nca, nc->version.mf_id);
+		}
 		if (ret < 0)
 			schedule_work(&ndp->work);
 
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index c9d1da34dc4d..f2f3b5c1b941 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -338,6 +338,14 @@ struct ncsi_rsp_gpuuid_pkt {
 	__be32                  checksum;
 };
 
+/* Get MC MAC Address */
+struct ncsi_rsp_gmcma_pkt {
+	struct ncsi_rsp_pkt_hdr rsp;
+	unsigned char           address_count;
+	unsigned char           reserved[3];
+	unsigned char           addresses[][ETH_ALEN];
+};
+
 /* AEN: Link State Change */
 struct ncsi_aen_lsc_pkt {
 	struct ncsi_aen_pkt_hdr aen;        /* AEN header      */
@@ -398,6 +406,7 @@ struct ncsi_aen_hncdsc_pkt {
 #define NCSI_PKT_CMD_GPUUID	0x52 /* Get package UUID                 */
 #define NCSI_PKT_CMD_QPNPR	0x56 /* Query Pending NC PLDM request */
 #define NCSI_PKT_CMD_SNPR	0x57 /* Send NC PLDM Reply  */
+#define NCSI_PKT_CMD_GMCMA	0x58 /* Get MC MAC Address */
 
 
 /* NCSI packet responses */
@@ -433,6 +442,7 @@ struct ncsi_aen_hncdsc_pkt {
 #define NCSI_PKT_RSP_GPUUID	(NCSI_PKT_CMD_GPUUID + 0x80)
 #define NCSI_PKT_RSP_QPNPR	(NCSI_PKT_CMD_QPNPR   + 0x80)
 #define NCSI_PKT_RSP_SNPR	(NCSI_PKT_CMD_SNPR   + 0x80)
+#define NCSI_PKT_RSP_GMCMA	(NCSI_PKT_CMD_GMCMA  + 0x80)
 
 /* NCSI response code/reason */
 #define NCSI_PKT_RSP_C_COMPLETED	0x0000 /* Command Completed        */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 480e80e3c283..bee290d0f48b 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -1091,6 +1091,44 @@ static int ncsi_rsp_handler_netlink(struct ncsi_request *nr)
 	return ret;
 }
 
+static int ncsi_rsp_handler_gmcma(struct ncsi_request *nr)
+{
+	struct ncsi_dev_priv *ndp = nr->ndp;
+	struct net_device *ndev = ndp->ndev.dev;
+	struct ncsi_rsp_gmcma_pkt *rsp;
+	struct sockaddr saddr;
+	int ret = -1;
+	int i;
+
+	rsp = (struct ncsi_rsp_gmcma_pkt *)skb_network_header(nr->rsp);
+	saddr.sa_family = ndev->type;
+	ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+
+	netdev_info(ndev, "NCSI: Received %d provisioned MAC addresses\n",
+		    rsp->address_count);
+	for (i = 0; i < rsp->address_count; i++) {
+		netdev_info(ndev, "NCSI: MAC address %d: %02x:%02x:%02x:%02x:%02x:%02x\n",
+			    i, rsp->addresses[i][0], rsp->addresses[i][1],
+			    rsp->addresses[i][2], rsp->addresses[i][3],
+			    rsp->addresses[i][4], rsp->addresses[i][5]);
+	}
+
+	for (i = 0; i < rsp->address_count; i++) {
+		memcpy(saddr.sa_data, &rsp->addresses[i], ETH_ALEN);
+		ret = ndev->netdev_ops->ndo_set_mac_address(ndev, &saddr);
+		if (ret < 0) {
+			netdev_warn(ndev, "NCSI: Unable to assign %pM to device\n",
+				    saddr.sa_data);
+			continue;
+		}
+		netdev_warn(ndev, "NCSI: Set MAC address to %pM\n", saddr.sa_data);
+		break;
+	}
+
+	ndp->gma_flag = ret == 0;
+	return ret;
+}
+
 static struct ncsi_rsp_handler {
 	unsigned char	type;
 	int             payload;
@@ -1127,7 +1165,8 @@ static struct ncsi_rsp_handler {
 	{ NCSI_PKT_RSP_PLDM,   -1, ncsi_rsp_handler_pldm    },
 	{ NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid  },
 	{ NCSI_PKT_RSP_QPNPR,  -1, ncsi_rsp_handler_pldm    },
-	{ NCSI_PKT_RSP_SNPR,   -1, ncsi_rsp_handler_pldm    }
+	{ NCSI_PKT_RSP_SNPR,   -1, ncsi_rsp_handler_pldm    },
+	{ NCSI_PKT_RSP_GMCMA,  -1, ncsi_rsp_handler_gmcma   },
 };
 
 int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
-- 
2.41.0

Re: [PATCH dev-5.6 1/3] net/ncsi: Simplify Kconfig/dts control flow

[Patrick Williams]

[-- Attachment #1: Type: text/plain, Size: 356 bytes --]

On Tue, Dec 05, 2023 at 05:48:41PM -0600, Patrick Williams wrote:
> (cherry picked from commit c797ce168930ce3d62a9b7fc4d7040963ee6a01e)

These patches were cherry-picked from the net-next tree and thus are
tentatively going into the 6.8 tree.  I've applied them onto our 6.5
tree, but they should apply cleanly to 6.6 also.

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH dev-5.6 3/3] net/ncsi: Add NC-SI 1.2 Get MC MAC Address command

[Ivan Mikhaylov]

On Tue, 2023-12-05 at 17:48 -0600, Patrick Williams wrote:
> From: Peter Delevoryas <peter@pjd.dev>
> 
> This change adds support for the NC-SI 1.2 Get MC MAC Address
> command,
> specified here:
> 
> https://www.dmtf.org/sites/default/files/standards/documents/DSP0222_1.2.0.pdf
> 
> It serves the exact same function as the existing OEM Get MAC Address
> commands, so if a channel reports that it supports NC-SI 1.2, we
> prefer
> to use the standard command rather than the OEM command.
> 
> Verified with an invalid MAC address and 2 valid ones:
> 
> [   55.137072] ftgmac100 1e690000.ftgmac eth0: NCSI: Received 3
> provisioned MAC addresses
> [   55.137614] ftgmac100 1e690000.ftgmac eth0: NCSI: MAC address 0:
> 00:00:00:00:00:00
> [   55.138026] ftgmac100 1e690000.ftgmac eth0: NCSI: MAC address 1:
> fa:ce:b0:0c:20:22
> [   55.138528] ftgmac100 1e690000.ftgmac eth0: NCSI: MAC address 2:
> fa:ce:b0:0c:20:23
> [   55.139241] ftgmac100 1e690000.ftgmac eth0: NCSI: Unable to assign
> 00:00:00:00:00:00 to device
> [   55.140098] ftgmac100 1e690000.ftgmac eth0: NCSI: Set MAC address
> to fa:ce:b0:0c:20:22
> 
> Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (cherry picked from commit b8291cf3d1180b5b61299922f17c9441616a805a)
> ---
>  net/ncsi/ncsi-cmd.c    |  3 ++-
>  net/ncsi/ncsi-manage.c |  9 +++++++--
>  net/ncsi/ncsi-pkt.h    | 10 ++++++++++
>  net/ncsi/ncsi-rsp.c    | 41
> ++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 59 insertions(+), 4 deletions(-)
> 
> diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
> index fd2236ee9a79..b3ff37a181d7 100644
> --- a/net/ncsi/ncsi-cmd.c
> +++ b/net/ncsi/ncsi-cmd.c
> @@ -270,7 +270,8 @@ static struct ncsi_cmd_handler {
>  	{ NCSI_PKT_CMD_GPS,    0, ncsi_cmd_handler_default },
>  	{ NCSI_PKT_CMD_OEM,   -1, ncsi_cmd_handler_oem     },
>  	{ NCSI_PKT_CMD_PLDM,   0, NULL                     },
> -	{ NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }
> +	{ NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default },
> +	{ NCSI_PKT_CMD_GMCMA,  0, ncsi_cmd_handler_default }
>  };
>  
>  static struct ncsi_request *ncsi_alloc_command(struct ncsi_cmd_arg
> *nca)
> diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
> index f3d7fe86fea1..745c788f1d1d 100644
> --- a/net/ncsi/ncsi-manage.c
> +++ b/net/ncsi/ncsi-manage.c
> @@ -1038,11 +1038,16 @@ static void ncsi_configure_channel(struct
> ncsi_dev_priv *ndp)
>  	case ncsi_dev_state_config_oem_gma:
>  		nd->state = ncsi_dev_state_config_clear_vids;
>  
> -		nca.type = NCSI_PKT_CMD_OEM;
>  		nca.package = np->id;
>  		nca.channel = nc->id;
>  		ndp->pending_req_num = 1;
> -		ret = ncsi_gma_handler(&nca, nc->version.mf_id);
> +		if (nc->version.major >= 1 && nc->version.minor >=
> 2) {
> +			nca.type = NCSI_PKT_CMD_GMCMA;
> +			ret = ncsi_xmit_cmd(&nca);
> +		} else {
> +			nca.type = NCSI_PKT_CMD_OEM;
> +			ret = ncsi_gma_handler(&nca, nc-
> >version.mf_id);
> +		}
>  		if (ret < 0)
>  			schedule_work(&ndp->work);
>  
> diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
> index c9d1da34dc4d..f2f3b5c1b941 100644
> --- a/net/ncsi/ncsi-pkt.h
> +++ b/net/ncsi/ncsi-pkt.h
> @@ -338,6 +338,14 @@ struct ncsi_rsp_gpuuid_pkt {
>  	__be32                  checksum;
>  };
>  
> +/* Get MC MAC Address */
> +struct ncsi_rsp_gmcma_pkt {
> +	struct ncsi_rsp_pkt_hdr rsp;
> +	unsigned char           address_count;
> +	unsigned char           reserved[3];
> +	unsigned char           addresses[][ETH_ALEN];
> +};
> +
>  /* AEN: Link State Change */
>  struct ncsi_aen_lsc_pkt {
>  	struct ncsi_aen_pkt_hdr aen;        /* AEN header      */
> @@ -398,6 +406,7 @@ struct ncsi_aen_hncdsc_pkt {
>  #define NCSI_PKT_CMD_GPUUID	0x52 /* Get package
> UUID                 */
>  #define NCSI_PKT_CMD_QPNPR	0x56 /* Query Pending NC PLDM
> request */
>  #define NCSI_PKT_CMD_SNPR	0x57 /* Send NC PLDM Reply  */
> +#define NCSI_PKT_CMD_GMCMA	0x58 /* Get MC MAC Address */
>  
>  
>  /* NCSI packet responses */
> @@ -433,6 +442,7 @@ struct ncsi_aen_hncdsc_pkt {
>  #define NCSI_PKT_RSP_GPUUID	(NCSI_PKT_CMD_GPUUID + 0x80)
>  #define NCSI_PKT_RSP_QPNPR	(NCSI_PKT_CMD_QPNPR   + 0x80)
>  #define NCSI_PKT_RSP_SNPR	(NCSI_PKT_CMD_SNPR   + 0x80)
> +#define NCSI_PKT_RSP_GMCMA	(NCSI_PKT_CMD_GMCMA  + 0x80)
>  
>  /* NCSI response code/reason */
>  #define NCSI_PKT_RSP_C_COMPLETED	0x0000 /* Command
> Completed        */
> diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> index 480e80e3c283..bee290d0f48b 100644
> --- a/net/ncsi/ncsi-rsp.c
> +++ b/net/ncsi/ncsi-rsp.c
> @@ -1091,6 +1091,44 @@ static int ncsi_rsp_handler_netlink(struct
> ncsi_request *nr)
>  	return ret;
>  }
>  
> +static int ncsi_rsp_handler_gmcma(struct ncsi_request *nr)
> +{
> +	struct ncsi_dev_priv *ndp = nr->ndp;
> +	struct net_device *ndev = ndp->ndev.dev;
> +	struct ncsi_rsp_gmcma_pkt *rsp;
> +	struct sockaddr saddr;
> +	int ret = -1;
> +	int i;
> +
> +	rsp = (struct ncsi_rsp_gmcma_pkt *)skb_network_header(nr-
> >rsp);
> +	saddr.sa_family = ndev->type;
> +	ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
> +
> +	netdev_info(ndev, "NCSI: Received %d provisioned MAC
> addresses\n",
> +		    rsp->address_count);
> +	for (i = 0; i < rsp->address_count; i++) {
> +		netdev_info(ndev, "NCSI: MAC address %d:
> %02x:%02x:%02x:%02x:%02x:%02x\n",
> +			    i, rsp->addresses[i][0], rsp-
> >addresses[i][1],
> +			    rsp->addresses[i][2], rsp-
> >addresses[i][3],
> +			    rsp->addresses[i][4], rsp-
> >addresses[i][5]);
> +	}
> +
> +	for (i = 0; i < rsp->address_count; i++) {
> +		memcpy(saddr.sa_data, &rsp->addresses[i], ETH_ALEN);
> +		ret = ndev->netdev_ops->ndo_set_mac_address(ndev,
> &saddr);
> +		if (ret < 0) {
> +			netdev_warn(ndev, "NCSI: Unable to assign
> %pM to device\n",
> +				    saddr.sa_data);
> +			continue;
> +		}
> +		netdev_warn(ndev, "NCSI: Set MAC address to %pM\n",
> saddr.sa_data);
> +		break;
> +	}
> +
> +	ndp->gma_flag = ret == 0;
> +	return ret;
> +}
> +
>  static struct ncsi_rsp_handler {
>  	unsigned char	type;
>  	int             payload;
> @@ -1127,7 +1165,8 @@ static struct ncsi_rsp_handler {
>  	{ NCSI_PKT_RSP_PLDM,   -1, ncsi_rsp_handler_pldm    },
>  	{ NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid  },
>  	{ NCSI_PKT_RSP_QPNPR,  -1, ncsi_rsp_handler_pldm    },
> -	{ NCSI_PKT_RSP_SNPR,   -1, ncsi_rsp_handler_pldm    }
> +	{ NCSI_PKT_RSP_SNPR,   -1, ncsi_rsp_handler_pldm    },
> +	{ NCSI_PKT_RSP_GMCMA,  -1, ncsi_rsp_handler_gmcma   },
>  };
>  
>  int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,


Patrick, I've the fix about ndo_set_mac_address not so long in the past
https://lore.kernel.org/all/20230828101151.684010399@linuxfoundation.org/

ndo_set_mac_address do not notify network layer about mac change.

Thanks.

Re: [PATCH dev-5.6 3/3] net/ncsi: Add NC-SI 1.2 Get MC MAC Address command

[Patrick Williams]

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]

On Thu, Dec 07, 2023 at 12:23:38AM +0300, Ivan Mikhaylov wrote:
> 
> 
> Patrick, I've the fix about ndo_set_mac_address not so long in the past
> https://lore.kernel.org/all/20230828101151.684010399@linuxfoundation.org/
> 
> ndo_set_mac_address do not notify network layer about mac change.

Hello Ivan,

I think you're suggesting there is a bug in the code that was applied to
net-next here?  If so, we'll need to get a fix into net-next.  These
commits are just a backport request to the OpenBMC tree of the code that
was already applied to net-next.

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH dev-5.6 3/3] net/ncsi: Add NC-SI 1.2 Get MC MAC Address command

[Ivan Mikhaylov]

On Wed, 2023-12-06 at 22:17 -0600, Patrick Williams wrote:
> On Thu, Dec 07, 2023 at 12:23:38AM +0300, Ivan Mikhaylov wrote:
> > 
> > 
> > Patrick, I've the fix about ndo_set_mac_address not so long in the
> > past
> > https://lore.kernel.org/all/20230828101151.684010399@linuxfoundation.org/
> > 
> > ndo_set_mac_address do not notify network layer about mac change.
> 
> Hello Ivan,
> 
> I think you're suggesting there is a bug in the code that was applied
> to
> net-next here?  If so, we'll need to get a fix into net-next.  These
> commits are just a backport request to the OpenBMC tree of the code
> that
> was already applied to net-next.
> 

Patrick, yes, there is a bug, I'll write to the thread today/tomorrow
with that commit about that problem. Need to think how to make a fix
for this problem, reverting and make it right until it in net-next or
fix above that commit.

Thanks.

Re: [PATCH dev-5.6 3/3] net/ncsi: Add NC-SI 1.2 Get MC MAC Address command

[Patrick Williams]

[-- Attachment #1: Type: text/plain, Size: 1798 bytes --]

On Thu, Dec 07, 2023 at 10:44:05AM +0300, Ivan Mikhaylov wrote:
> On Wed, 2023-12-06 at 22:17 -0600, Patrick Williams wrote:
> > On Thu, Dec 07, 2023 at 12:23:38AM +0300, Ivan Mikhaylov wrote:
> > > 
> > > 
> > > Patrick, I've the fix about ndo_set_mac_address not so long in the
> > > past
> > > https://lore.kernel.org/all/20230828101151.684010399@linuxfoundation.org/
> > > 
> > > ndo_set_mac_address do not notify network layer about mac change.
> > 
> > Hello Ivan,
> > 
> > I think you're suggesting there is a bug in the code that was applied
> > to
> > net-next here?  If so, we'll need to get a fix into net-next.  These
> > commits are just a backport request to the OpenBMC tree of the code
> > that
> > was already applied to net-next.
> > 
> 
> Patrick, yes, there is a bug, I'll write to the thread today/tomorrow
> with that commit about that problem. Need to think how to make a fix
> for this problem, reverting and make it right until it in net-next or
> fix above that commit.

Is this the fix?

diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index bee290d0f48b..b02e663e56dc 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -1115,7 +1115,9 @@ static int ncsi_rsp_handler_gmcma(struct ncsi_request *nr)
 
        for (i = 0; i < rsp->address_count; i++) {
                memcpy(saddr.sa_data, &rsp->addresses[i], ETH_ALEN);
-               ret = ndev->netdev_ops->ndo_set_mac_address(ndev, &saddr);
+               rtnl_lock();
+               ret = dev_set_mac_address(ndev, &saddr, NULL);
+               rtnl_unlock();
                if (ret < 0) {
                        netdev_warn(ndev, "NCSI: Unable to assign %pM to device\n",
                                    saddr.sa_data);

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH dev-5.6 3/3] net/ncsi: Add NC-SI 1.2 Get MC MAC Address command

[Ivan Mikhaylov]

On Thu, 2023-12-07 at 06:16 -0600, Patrick Williams wrote:
> On Thu, Dec 07, 2023 at 10:44:05AM +0300, Ivan Mikhaylov wrote:
> > On Wed, 2023-12-06 at 22:17 -0600, Patrick Williams wrote:
> > > On Thu, Dec 07, 2023 at 12:23:38AM +0300, Ivan Mikhaylov wrote:
> > > > 
> > > > 
> > > > Patrick, I've the fix about ndo_set_mac_address not so long in
> > > > the
> > > > past
> > > > https://lore.kernel.org/all/20230828101151.684010399@linuxfoundation.org/
> > > > 
> > > > ndo_set_mac_address do not notify network layer about mac
> > > > change.
> > > 
> > > Hello Ivan,
> > > 
> > > I think you're suggesting there is a bug in the code that was
> > > applied
> > > to
> > > net-next here?  If so, we'll need to get a fix into net-next. 
> > > These
> > > commits are just a backport request to the OpenBMC tree of the
> > > code
> > > that
> > > was already applied to net-next.
> > > 
> > 
> > Patrick, yes, there is a bug, I'll write to the thread
> > today/tomorrow
> > with that commit about that problem. Need to think how to make a
> > fix
> > for this problem, reverting and make it right until it in net-next
> > or
> > fix above that commit.
> 
> Is this the fix?
> 
> diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> index bee290d0f48b..b02e663e56dc 100644
> --- a/net/ncsi/ncsi-rsp.c
> +++ b/net/ncsi/ncsi-rsp.c
> @@ -1115,7 +1115,9 @@ static int ncsi_rsp_handler_gmcma(struct
> ncsi_request *nr)
>  
>         for (i = 0; i < rsp->address_count; i++) {
>                 memcpy(saddr.sa_data, &rsp->addresses[i], ETH_ALEN);
> -               ret = ndev->netdev_ops->ndo_set_mac_address(ndev,
> &saddr);
> +               rtnl_lock();
> +               ret = dev_set_mac_address(ndev, &saddr, NULL);
> +               rtnl_unlock();
>                 if (ret < 0) {
>                         netdev_warn(ndev, "NCSI: Unable to assign %pM
> to device\n",
>                                     saddr.sa_data);
> 

Patrick, yes, this is the fix. Difference in calling of
call_netdevice_notifiers from dev_set_mac_address to make sure that
everybody aware about mac change which using this interface.

Thanks.

Re: [PATCH dev-5.6 1/3] net/ncsi: Simplify Kconfig/dts control flow

[Patrick Williams]

Joel,

I realized I used "5.6" as the prefix on these when I meant "6.6".  Can
these be picked up as is or do you need me to resent?

On Tue, Dec 05, 2023 at 07:00:25PM -0600, Patrick Williams wrote:
> On Tue, Dec 05, 2023 at 05:48:41PM -0600, Patrick Williams wrote:
> > (cherry picked from commit c797ce168930ce3d62a9b7fc4d7040963ee6a01e)
> 
> These patches were cherry-picked from the net-next tree and thus are
> tentatively going into the 6.8 tree.  I've applied them onto our 6.5
> tree, but they should apply cleanly to 6.6 also.
> 
> -- 
> Patrick Williams



-- 
Patrick Williams