[PATCH 0/1] net/ncsi: add get mac address command for Intel

[PATCH 0/1] net/ncsi: add get mac address command for Intel

[Ivan Mikhaylov]

Add NCSI Intel command to get MAC address from host and set it on interface.

Ivan Mikhaylov (1):
  net/ncsi: add get MAC address command to get Intel i210 MAC address

 net/ncsi/internal.h    |  3 +++
 net/ncsi/ncsi-manage.c | 25 ++++++++++++++++++++++++-
 net/ncsi/ncsi-pkt.h    |  6 ++++++
 net/ncsi/ncsi-rsp.c    | 42 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 75 insertions(+), 1 deletion(-)

-- 
2.31.1

[PATCH 0/1] net/ncsi: add get mac address command for Intel

[Ivan Mikhaylov]

Add NCSI Intel command to get MAC address from host and set it on interface.

Ivan Mikhaylov (1):
  net/ncsi: add get MAC address command to get Intel i210 MAC address

 net/ncsi/internal.h    |  3 +++
 net/ncsi/ncsi-manage.c | 25 ++++++++++++++++++++++++-
 net/ncsi/ncsi-pkt.h    |  6 ++++++
 net/ncsi/ncsi-rsp.c    | 42 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 75 insertions(+), 1 deletion(-)

-- 
2.31.1

[PATCH 1/1] net/ncsi: add get MAC address command to get Intel i210 MAC address

[Ivan Mikhaylov]

This patch adds OEM Intel GMA command and response handler for it.

Signed-off-by: Brad Ho <Brad_Ho@phoenix.com>
Signed-off-by: Paul Fertser <fercerpav@gmail.com>
Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
---
 net/ncsi/internal.h    |  3 +++
 net/ncsi/ncsi-manage.c | 25 ++++++++++++++++++++++++-
 net/ncsi/ncsi-pkt.h    |  6 ++++++
 net/ncsi/ncsi-rsp.c    | 42 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 0b6cfd3b31e0..03757e76bb6b 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -80,6 +80,7 @@ enum {
 #define NCSI_OEM_MFR_BCM_ID             0x113d
 #define NCSI_OEM_MFR_INTEL_ID           0x157
 /* Intel specific OEM command */
+#define NCSI_OEM_INTEL_CMD_GMA          0x06   /* CMD ID for Get MAC */
 #define NCSI_OEM_INTEL_CMD_KEEP_PHY     0x20   /* CMD ID for Keep PHY up */
 /* Broadcom specific OEM Command */
 #define NCSI_OEM_BCM_CMD_GMA            0x01   /* CMD ID for Get MAC */
@@ -89,6 +90,7 @@ enum {
 #define NCSI_OEM_MLX_CMD_SMAF           0x01   /* CMD ID for Set MC Affinity */
 #define NCSI_OEM_MLX_CMD_SMAF_PARAM     0x07   /* Parameter for SMAF         */
 /* OEM Command payload lengths*/
+#define NCSI_OEM_INTEL_CMD_GMA_LEN      5
 #define NCSI_OEM_INTEL_CMD_KEEP_PHY_LEN 7
 #define NCSI_OEM_BCM_CMD_GMA_LEN        12
 #define NCSI_OEM_MLX_CMD_GMA_LEN        8
@@ -99,6 +101,7 @@ enum {
 /* Mac address offset in OEM response */
 #define BCM_MAC_ADDR_OFFSET             28
 #define MLX_MAC_ADDR_OFFSET             8
+#define INTEL_MAC_ADDR_OFFSET           1
 
 
 struct ncsi_channel_version {
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 89c7742cd72e..7121ce2a47c0 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -795,13 +795,36 @@ static int ncsi_oem_smaf_mlx(struct ncsi_cmd_arg *nca)
 	return ret;
 }
 
+static int ncsi_oem_gma_handler_intel(struct ncsi_cmd_arg *nca)
+{
+	unsigned char data[NCSI_OEM_INTEL_CMD_GMA_LEN];
+	int ret = 0;
+
+	nca->payload = NCSI_OEM_INTEL_CMD_GMA_LEN;
+
+	memset(data, 0, NCSI_OEM_INTEL_CMD_GMA_LEN);
+	*(unsigned int *)data = ntohl((__force __be32)NCSI_OEM_MFR_INTEL_ID);
+	data[4] = NCSI_OEM_INTEL_CMD_GMA;
+
+	nca->data = data;
+
+	ret = ncsi_xmit_cmd(nca);
+	if (ret)
+		netdev_err(nca->ndp->ndev.dev,
+			   "NCSI: Failed to transmit cmd 0x%x during configure\n",
+			   nca->type);
+
+	return ret;
+}
+
 /* OEM Command handlers initialization */
 static struct ncsi_oem_gma_handler {
 	unsigned int	mfr_id;
 	int		(*handler)(struct ncsi_cmd_arg *nca);
 } ncsi_oem_gma_handlers[] = {
 	{ NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm },
-	{ NCSI_OEM_MFR_MLX_ID, ncsi_oem_gma_handler_mlx }
+	{ NCSI_OEM_MFR_MLX_ID, ncsi_oem_gma_handler_mlx },
+	{ NCSI_OEM_MFR_INTEL_ID, ncsi_oem_gma_handler_intel }
 };
 
 static int ncsi_gma_handler(struct ncsi_cmd_arg *nca, unsigned int mf_id)
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 80938b338fee..ba66c7dc3a21 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -178,6 +178,12 @@ struct ncsi_rsp_oem_bcm_pkt {
 	unsigned char           data[];      /* Cmd specific Data */
 };
 
+/* Intel Response Data */
+struct ncsi_rsp_oem_intel_pkt {
+	unsigned char           cmd;         /* OEM Command ID    */
+	unsigned char           data[];      /* Cmd specific Data */
+};
+
 /* 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 d48374894817..6447a09932f5 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -699,9 +699,51 @@ static int ncsi_rsp_handler_oem_bcm(struct ncsi_request *nr)
 	return 0;
 }
 
+/* Response handler for Intel command Get Mac Address */
+static int ncsi_rsp_handler_oem_intel_gma(struct ncsi_request *nr)
+{
+	struct ncsi_dev_priv *ndp = nr->ndp;
+	struct net_device *ndev = ndp->ndev.dev;
+	const struct net_device_ops *ops = ndev->netdev_ops;
+	struct ncsi_rsp_oem_pkt *rsp;
+	struct sockaddr saddr;
+	int ret = 0;
+
+	/* Get the response header */
+	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+
+	saddr.sa_family = ndev->type;
+	ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+	memcpy(saddr.sa_data, &rsp->data[INTEL_MAC_ADDR_OFFSET], ETH_ALEN);
+	/* Increase mac address by 1 for BMC's address */
+	eth_addr_inc((u8 *)saddr.sa_data);
+	if (!is_valid_ether_addr((const u8 *)saddr.sa_data))
+		return -ENXIO;
+
+	/* Set the flag for GMA command which should only be called once */
+	ndp->gma_flag = 1;
+
+	ret = ops->ndo_set_mac_address(ndev, &saddr);
+	if (ret < 0)
+		netdev_warn(ndev,
+			    "NCSI: 'Writing mac address to device failed\n");
+
+	return ret;
+}
+
 /* Response handler for Intel card */
 static int ncsi_rsp_handler_oem_intel(struct ncsi_request *nr)
 {
+	struct ncsi_rsp_oem_intel_pkt *intel;
+	struct ncsi_rsp_oem_pkt *rsp;
+
+	/* Get the response header */
+	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+	intel = (struct ncsi_rsp_oem_intel_pkt *)(rsp->data);
+
+	if (intel->cmd == NCSI_OEM_INTEL_CMD_GMA)
+		return ncsi_rsp_handler_oem_intel_gma(nr);
+
 	return 0;
 }
 
-- 
2.31.1

[PATCH 1/1] net/ncsi: add get MAC address command to get Intel i210 MAC address

[Ivan Mikhaylov]

This patch adds OEM Intel GMA command and response handler for it.

Signed-off-by: Brad Ho <Brad_Ho@phoenix.com>
Signed-off-by: Paul Fertser <fercerpav@gmail.com>
Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
---
 net/ncsi/internal.h    |  3 +++
 net/ncsi/ncsi-manage.c | 25 ++++++++++++++++++++++++-
 net/ncsi/ncsi-pkt.h    |  6 ++++++
 net/ncsi/ncsi-rsp.c    | 42 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 0b6cfd3b31e0..03757e76bb6b 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -80,6 +80,7 @@ enum {
 #define NCSI_OEM_MFR_BCM_ID             0x113d
 #define NCSI_OEM_MFR_INTEL_ID           0x157
 /* Intel specific OEM command */
+#define NCSI_OEM_INTEL_CMD_GMA          0x06   /* CMD ID for Get MAC */
 #define NCSI_OEM_INTEL_CMD_KEEP_PHY     0x20   /* CMD ID for Keep PHY up */
 /* Broadcom specific OEM Command */
 #define NCSI_OEM_BCM_CMD_GMA            0x01   /* CMD ID for Get MAC */
@@ -89,6 +90,7 @@ enum {
 #define NCSI_OEM_MLX_CMD_SMAF           0x01   /* CMD ID for Set MC Affinity */
 #define NCSI_OEM_MLX_CMD_SMAF_PARAM     0x07   /* Parameter for SMAF         */
 /* OEM Command payload lengths*/
+#define NCSI_OEM_INTEL_CMD_GMA_LEN      5
 #define NCSI_OEM_INTEL_CMD_KEEP_PHY_LEN 7
 #define NCSI_OEM_BCM_CMD_GMA_LEN        12
 #define NCSI_OEM_MLX_CMD_GMA_LEN        8
@@ -99,6 +101,7 @@ enum {
 /* Mac address offset in OEM response */
 #define BCM_MAC_ADDR_OFFSET             28
 #define MLX_MAC_ADDR_OFFSET             8
+#define INTEL_MAC_ADDR_OFFSET           1
 
 
 struct ncsi_channel_version {
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 89c7742cd72e..7121ce2a47c0 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -795,13 +795,36 @@ static int ncsi_oem_smaf_mlx(struct ncsi_cmd_arg *nca)
 	return ret;
 }
 
+static int ncsi_oem_gma_handler_intel(struct ncsi_cmd_arg *nca)
+{
+	unsigned char data[NCSI_OEM_INTEL_CMD_GMA_LEN];
+	int ret = 0;
+
+	nca->payload = NCSI_OEM_INTEL_CMD_GMA_LEN;
+
+	memset(data, 0, NCSI_OEM_INTEL_CMD_GMA_LEN);
+	*(unsigned int *)data = ntohl((__force __be32)NCSI_OEM_MFR_INTEL_ID);
+	data[4] = NCSI_OEM_INTEL_CMD_GMA;
+
+	nca->data = data;
+
+	ret = ncsi_xmit_cmd(nca);
+	if (ret)
+		netdev_err(nca->ndp->ndev.dev,
+			   "NCSI: Failed to transmit cmd 0x%x during configure\n",
+			   nca->type);
+
+	return ret;
+}
+
 /* OEM Command handlers initialization */
 static struct ncsi_oem_gma_handler {
 	unsigned int	mfr_id;
 	int		(*handler)(struct ncsi_cmd_arg *nca);
 } ncsi_oem_gma_handlers[] = {
 	{ NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm },
-	{ NCSI_OEM_MFR_MLX_ID, ncsi_oem_gma_handler_mlx }
+	{ NCSI_OEM_MFR_MLX_ID, ncsi_oem_gma_handler_mlx },
+	{ NCSI_OEM_MFR_INTEL_ID, ncsi_oem_gma_handler_intel }
 };
 
 static int ncsi_gma_handler(struct ncsi_cmd_arg *nca, unsigned int mf_id)
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 80938b338fee..ba66c7dc3a21 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -178,6 +178,12 @@ struct ncsi_rsp_oem_bcm_pkt {
 	unsigned char           data[];      /* Cmd specific Data */
 };
 
+/* Intel Response Data */
+struct ncsi_rsp_oem_intel_pkt {
+	unsigned char           cmd;         /* OEM Command ID    */
+	unsigned char           data[];      /* Cmd specific Data */
+};
+
 /* 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 d48374894817..6447a09932f5 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -699,9 +699,51 @@ static int ncsi_rsp_handler_oem_bcm(struct ncsi_request *nr)
 	return 0;
 }
 
+/* Response handler for Intel command Get Mac Address */
+static int ncsi_rsp_handler_oem_intel_gma(struct ncsi_request *nr)
+{
+	struct ncsi_dev_priv *ndp = nr->ndp;
+	struct net_device *ndev = ndp->ndev.dev;
+	const struct net_device_ops *ops = ndev->netdev_ops;
+	struct ncsi_rsp_oem_pkt *rsp;
+	struct sockaddr saddr;
+	int ret = 0;
+
+	/* Get the response header */
+	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+
+	saddr.sa_family = ndev->type;
+	ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+	memcpy(saddr.sa_data, &rsp->data[INTEL_MAC_ADDR_OFFSET], ETH_ALEN);
+	/* Increase mac address by 1 for BMC's address */
+	eth_addr_inc((u8 *)saddr.sa_data);
+	if (!is_valid_ether_addr((const u8 *)saddr.sa_data))
+		return -ENXIO;
+
+	/* Set the flag for GMA command which should only be called once */
+	ndp->gma_flag = 1;
+
+	ret = ops->ndo_set_mac_address(ndev, &saddr);
+	if (ret < 0)
+		netdev_warn(ndev,
+			    "NCSI: 'Writing mac address to device failed\n");
+
+	return ret;
+}
+
 /* Response handler for Intel card */
 static int ncsi_rsp_handler_oem_intel(struct ncsi_request *nr)
 {
+	struct ncsi_rsp_oem_intel_pkt *intel;
+	struct ncsi_rsp_oem_pkt *rsp;
+
+	/* Get the response header */
+	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+	intel = (struct ncsi_rsp_oem_intel_pkt *)(rsp->data);
+
+	if (intel->cmd == NCSI_OEM_INTEL_CMD_GMA)
+		return ncsi_rsp_handler_oem_intel_gma(nr);
+
 	return 0;
 }
 
-- 
2.31.1

Re: [PATCH 1/1] net/ncsi: add get MAC address command to get Intel i210 MAC address

[Jakub Kicinski]

On Mon, 30 Aug 2021 20:18:06 +0300 Ivan Mikhaylov wrote:
> This patch adds OEM Intel GMA command and response handler for it.
> 
> Signed-off-by: Brad Ho <Brad_Ho@phoenix.com>
> Signed-off-by: Paul Fertser <fercerpav@gmail.com>
> Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>

Applied, thanks, but there is a disconcerting lack of length checking.
How are truncated responses rejected?

Re: [PATCH 1/1] net/ncsi: add get MAC address command to get Intel i210 MAC address

[Jakub Kicinski]

On Mon, 30 Aug 2021 20:18:06 +0300 Ivan Mikhaylov wrote:
> This patch adds OEM Intel GMA command and response handler for it.
> 
> Signed-off-by: Brad Ho <Brad_Ho@phoenix.com>
> Signed-off-by: Paul Fertser <fercerpav@gmail.com>
> Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>

Applied, thanks, but there is a disconcerting lack of length checking.
How are truncated responses rejected?

Re: [PATCH 1/1] net/ncsi: add get MAC address command to get Intel i210 MAC address

[Milton Miller II]

On August 30, 2021, Ivan Mikhaylov" <i.mikhaylov@yadro.com> wrote:
>This patch adds OEM Intel GMA command and response handler for it.



>>>Signed-off-by: Brad Ho <Brad_Ho@phoenix.com
>>Signed-off-by: Paul Fertser <fercerpav@gmail.com>
>Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
>---
> net/ncsi/internal.h    |  3 +++
> net/ncsi/ncsi-manage.c | 25 ++++++++++++++++++++++++-
> net/ncsi/ncsi-pkt.h    |  6 ++++++
> net/ncsi/ncsi-rsp.c    | 42
>++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 75 insertions(+), 1 deletion(-)
>
>diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
>index 0b6cfd3b31e0..03757e76bb6b 100644
>--- a/net/ncsi/internal.h
>+++ b/net/ncsi/internal.h
>@@ -80,6 +80,7 @@ enum {
> #define NCSI_OEM_MFR_BCM_ID             0x113d
> #define NCSI_OEM_MFR_INTEL_ID           0x157
> /* Intel specific OEM command */
>+#define NCSI_OEM_INTEL_CMD_GMA          0x06   /* CMD ID for Get MAC
>*/
> #define NCSI_OEM_INTEL_CMD_KEEP_PHY     0x20   /* CMD ID for Keep
>PHY up */
> /* Broadcom specific OEM Command */
> #define NCSI_OEM_BCM_CMD_GMA            0x01   /* CMD ID for Get MAC
>*/
>@@ -89,6 +90,7 @@ enum {
> #define NCSI_OEM_MLX_CMD_SMAF           0x01   /* CMD ID for Set MC
>Affinity */
> #define NCSI_OEM_MLX_CMD_SMAF_PARAM     0x07   /* Parameter for SMAF
>        */
> /* OEM Command payload lengths*/
>+#define NCSI_OEM_INTEL_CMD_GMA_LEN      5
> #define NCSI_OEM_INTEL_CMD_KEEP_PHY_LEN 7
> #define NCSI_OEM_BCM_CMD_GMA_LEN        12
> #define NCSI_OEM_MLX_CMD_GMA_LEN        8
>@@ -99,6 +101,7 @@ enum {
> /* Mac address offset in OEM response */
> #define BCM_MAC_ADDR_OFFSET             28
> #define MLX_MAC_ADDR_OFFSET             8
>+#define INTEL_MAC_ADDR_OFFSET           1
> 
> 
> struct ncsi_channel_version {
>diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
>index 89c7742cd72e..7121ce2a47c0 100644
>--- a/net/ncsi/ncsi-manage.c
>+++ b/net/ncsi/ncsi-manage.c
>@@ -795,13 +795,36 @@ static int ncsi_oem_smaf_mlx(struct
>ncsi_cmd_arg *nca)
> 	return ret;
> }
> 
>+static int ncsi_oem_gma_handler_intel(struct ncsi_cmd_arg *nca)
>+{
>+	unsigned char data[NCSI_OEM_INTEL_CMD_GMA_LEN];
>+	int ret = 0;
>+
>+	nca->payload = NCSI_OEM_INTEL_CMD_GMA_LEN;
>+
>+	memset(data, 0, NCSI_OEM_INTEL_CMD_GMA_LEN);
>+	*(unsigned int *)data = ntohl((__force
>__be32)NCSI_OEM_MFR_INTEL_ID);
>+	data[4] = NCSI_OEM_INTEL_CMD_GMA;
>+
>+	nca->data = data;
>+
>+	ret = ncsi_xmit_cmd(nca);
>+	if (ret)
>+		netdev_err(nca->ndp->ndev.dev,
>+			   "NCSI: Failed to transmit cmd 0x%x during configure\n",
>+			   nca->type);
>+
>+	return ret;
>+}
>+
> /* OEM Command handlers initialization */
> static struct ncsi_oem_gma_handler {
> 	unsigned int	mfr_id;
> 	int		(*handler)(struct ncsi_cmd_arg *nca);
> } ncsi_oem_gma_handlers[] = {
> 	{ NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm },
>-	{ NCSI_OEM_MFR_MLX_ID, ncsi_oem_gma_handler_mlx }
>+	{ NCSI_OEM_MFR_MLX_ID, ncsi_oem_gma_handler_mlx },
>+	{ NCSI_OEM_MFR_INTEL_ID, ncsi_oem_gma_handler_intel }
> };
> 
> static int ncsi_gma_handler(struct ncsi_cmd_arg *nca, unsigned int
>mf_id)
>diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
>index 80938b338fee..ba66c7dc3a21 100644
>--- a/net/ncsi/ncsi-pkt.h
>+++ b/net/ncsi/ncsi-pkt.h
>@@ -178,6 +178,12 @@ struct ncsi_rsp_oem_bcm_pkt {
> 	unsigned char           data[];      /* Cmd specific Data */
> };
> 
>+/* Intel Response Data */
>+struct ncsi_rsp_oem_intel_pkt {
>+	unsigned char           cmd;         /* OEM Command ID    */
>+	unsigned char           data[];      /* Cmd specific Data */
>+};
>+
> /* 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 d48374894817..6447a09932f5 100644
>--- a/net/ncsi/ncsi-rsp.c
>+++ b/net/ncsi/ncsi-rsp.c
>@@ -699,9 +699,51 @@ static int ncsi_rsp_handler_oem_bcm(struct
>ncsi_request *nr)
> 	return 0;
> }
> 
>+/* Response handler for Intel command Get Mac Address */
>+static int ncsi_rsp_handler_oem_intel_gma(struct ncsi_request *nr)
>+{
>+	struct ncsi_dev_priv *ndp = nr->ndp;
>+	struct net_device *ndev = ndp->ndev.dev;
>+	const struct net_device_ops *ops = ndev->netdev_ops;
>+	struct ncsi_rsp_oem_pkt *rsp;
>+	struct sockaddr saddr;
>+	int ret = 0;
>+
>+	/* Get the response header */
>+	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
>+
>+	saddr.sa_family = ndev->type;
>+	ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
>+	memcpy(saddr.sa_data, &rsp->data[INTEL_MAC_ADDR_OFFSET], ETH_ALEN);
>+	/* Increase mac address by 1 for BMC's address */
>+	eth_addr_inc((u8 *)saddr.sa_data);
>+	if (!is_valid_ether_addr((const u8 *)saddr.sa_data))
>+		return -ENXIO;

The Intel GMA retireves the MAC address of the host, and the datasheet
anticipates the BMC will "share" the MAC by stealing specific TCP and 
UDP port traffic destined to the host.

This "add one" allocation of the MAC is therefore a policy, and one that 
is beyond the data sheet.

While this +1 policy may work for some OEM boards, there are other boards 
where the MAC address assigned to the BMC does not follow this pattern, 
but instead the MAC is stored in some platform dependent location obtained 
in a platform specific manner.

I suggest this BMC = ether_addr_inc(GMA) be opt in via a device tree property.  

The name of the property should be negotiated in the device tree mailing list, 
as it appears it would be generic to more than one vendor.

Unfortunately, we missed this when we added the broadcom and mellanox handlers.


>+>+	/* Set the flag for GMA command which should only be called once */
>+	ndp->gma_flag = 1;
>+
>+	ret = ops->ndo_set_mac_address(ndev, &saddr);
>+	if (ret < 0)
>+		netdev_warn(ndev,
>+			    "NCSI: 'Writing mac address to device failed\n");
>+
>+	return ret;
>+}
>+
> /* Response handler for Intel card */
> static int ncsi_rsp_handler_oem_intel(struct ncsi_request *nr)
> {
>+	struct ncsi_rsp_oem_intel_pkt *intel;
>+	struct ncsi_rsp_oem_pkt *rsp;
>+
>+	/* Get the response header */
>+	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
>+	intel = (struct ncsi_rsp_oem_intel_pkt *)(rsp->data);
>+
>+	if (intel->cmd == NCSI_OEM_INTEL_CMD_GMA)
>+		return ncsi_rsp_handler_oem_intel_gma(nr);
>+
> 	return 0;
> }
> 
>-- 
>2.31.1
>
>

Re: [PATCH 1/1] net/ncsi: add get MAC address command to get Intel i210 MAC address

[Milton Miller II]

On August 30, 2021, Ivan Mikhaylov" <i.mikhaylov@yadro.com> wrote:
>This patch adds OEM Intel GMA command and response handler for it.



>>>Signed-off-by: Brad Ho <Brad_Ho@phoenix.com
>>Signed-off-by: Paul Fertser <fercerpav@gmail.com>
>Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
>---
> net/ncsi/internal.h    |  3 +++
> net/ncsi/ncsi-manage.c | 25 ++++++++++++++++++++++++-
> net/ncsi/ncsi-pkt.h    |  6 ++++++
> net/ncsi/ncsi-rsp.c    | 42
>++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 75 insertions(+), 1 deletion(-)
>
>diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
>index 0b6cfd3b31e0..03757e76bb6b 100644
>--- a/net/ncsi/internal.h
>+++ b/net/ncsi/internal.h
>@@ -80,6 +80,7 @@ enum {
> #define NCSI_OEM_MFR_BCM_ID             0x113d
> #define NCSI_OEM_MFR_INTEL_ID           0x157
> /* Intel specific OEM command */
>+#define NCSI_OEM_INTEL_CMD_GMA          0x06   /* CMD ID for Get MAC
>*/
> #define NCSI_OEM_INTEL_CMD_KEEP_PHY     0x20   /* CMD ID for Keep
>PHY up */
> /* Broadcom specific OEM Command */
> #define NCSI_OEM_BCM_CMD_GMA            0x01   /* CMD ID for Get MAC
>*/
>@@ -89,6 +90,7 @@ enum {
> #define NCSI_OEM_MLX_CMD_SMAF           0x01   /* CMD ID for Set MC
>Affinity */
> #define NCSI_OEM_MLX_CMD_SMAF_PARAM     0x07   /* Parameter for SMAF
>        */
> /* OEM Command payload lengths*/
>+#define NCSI_OEM_INTEL_CMD_GMA_LEN      5
> #define NCSI_OEM_INTEL_CMD_KEEP_PHY_LEN 7
> #define NCSI_OEM_BCM_CMD_GMA_LEN        12
> #define NCSI_OEM_MLX_CMD_GMA_LEN        8
>@@ -99,6 +101,7 @@ enum {
> /* Mac address offset in OEM response */
> #define BCM_MAC_ADDR_OFFSET             28
> #define MLX_MAC_ADDR_OFFSET             8
>+#define INTEL_MAC_ADDR_OFFSET           1
> 
> 
> struct ncsi_channel_version {
>diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
>index 89c7742cd72e..7121ce2a47c0 100644
>--- a/net/ncsi/ncsi-manage.c
>+++ b/net/ncsi/ncsi-manage.c
>@@ -795,13 +795,36 @@ static int ncsi_oem_smaf_mlx(struct
>ncsi_cmd_arg *nca)
> 	return ret;
> }
> 
>+static int ncsi_oem_gma_handler_intel(struct ncsi_cmd_arg *nca)
>+{
>+	unsigned char data[NCSI_OEM_INTEL_CMD_GMA_LEN];
>+	int ret = 0;
>+
>+	nca->payload = NCSI_OEM_INTEL_CMD_GMA_LEN;
>+
>+	memset(data, 0, NCSI_OEM_INTEL_CMD_GMA_LEN);
>+	*(unsigned int *)data = ntohl((__force
>__be32)NCSI_OEM_MFR_INTEL_ID);
>+	data[4] = NCSI_OEM_INTEL_CMD_GMA;
>+
>+	nca->data = data;
>+
>+	ret = ncsi_xmit_cmd(nca);
>+	if (ret)
>+		netdev_err(nca->ndp->ndev.dev,
>+			   "NCSI: Failed to transmit cmd 0x%x during configure\n",
>+			   nca->type);
>+
>+	return ret;
>+}
>+
> /* OEM Command handlers initialization */
> static struct ncsi_oem_gma_handler {
> 	unsigned int	mfr_id;
> 	int		(*handler)(struct ncsi_cmd_arg *nca);
> } ncsi_oem_gma_handlers[] = {
> 	{ NCSI_OEM_MFR_BCM_ID, ncsi_oem_gma_handler_bcm },
>-	{ NCSI_OEM_MFR_MLX_ID, ncsi_oem_gma_handler_mlx }
>+	{ NCSI_OEM_MFR_MLX_ID, ncsi_oem_gma_handler_mlx },
>+	{ NCSI_OEM_MFR_INTEL_ID, ncsi_oem_gma_handler_intel }
> };
> 
> static int ncsi_gma_handler(struct ncsi_cmd_arg *nca, unsigned int
>mf_id)
>diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
>index 80938b338fee..ba66c7dc3a21 100644
>--- a/net/ncsi/ncsi-pkt.h
>+++ b/net/ncsi/ncsi-pkt.h
>@@ -178,6 +178,12 @@ struct ncsi_rsp_oem_bcm_pkt {
> 	unsigned char           data[];      /* Cmd specific Data */
> };
> 
>+/* Intel Response Data */
>+struct ncsi_rsp_oem_intel_pkt {
>+	unsigned char           cmd;         /* OEM Command ID    */
>+	unsigned char           data[];      /* Cmd specific Data */
>+};
>+
> /* 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 d48374894817..6447a09932f5 100644
>--- a/net/ncsi/ncsi-rsp.c
>+++ b/net/ncsi/ncsi-rsp.c
>@@ -699,9 +699,51 @@ static int ncsi_rsp_handler_oem_bcm(struct
>ncsi_request *nr)
> 	return 0;
> }
> 
>+/* Response handler for Intel command Get Mac Address */
>+static int ncsi_rsp_handler_oem_intel_gma(struct ncsi_request *nr)
>+{
>+	struct ncsi_dev_priv *ndp = nr->ndp;
>+	struct net_device *ndev = ndp->ndev.dev;
>+	const struct net_device_ops *ops = ndev->netdev_ops;
>+	struct ncsi_rsp_oem_pkt *rsp;
>+	struct sockaddr saddr;
>+	int ret = 0;
>+
>+	/* Get the response header */
>+	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
>+
>+	saddr.sa_family = ndev->type;
>+	ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
>+	memcpy(saddr.sa_data, &rsp->data[INTEL_MAC_ADDR_OFFSET], ETH_ALEN);
>+	/* Increase mac address by 1 for BMC's address */
>+	eth_addr_inc((u8 *)saddr.sa_data);
>+	if (!is_valid_ether_addr((const u8 *)saddr.sa_data))
>+		return -ENXIO;

The Intel GMA retireves the MAC address of the host, and the datasheet
anticipates the BMC will "share" the MAC by stealing specific TCP and 
UDP port traffic destined to the host.

This "add one" allocation of the MAC is therefore a policy, and one that 
is beyond the data sheet.

While this +1 policy may work for some OEM boards, there are other boards 
where the MAC address assigned to the BMC does not follow this pattern, 
but instead the MAC is stored in some platform dependent location obtained 
in a platform specific manner.

I suggest this BMC = ether_addr_inc(GMA) be opt in via a device tree property.  

The name of the property should be negotiated in the device tree mailing list, 
as it appears it would be generic to more than one vendor.

Unfortunately, we missed this when we added the broadcom and mellanox handlers.


>+>+	/* Set the flag for GMA command which should only be called once */
>+	ndp->gma_flag = 1;
>+
>+	ret = ops->ndo_set_mac_address(ndev, &saddr);
>+	if (ret < 0)
>+		netdev_warn(ndev,
>+			    "NCSI: 'Writing mac address to device failed\n");
>+
>+	return ret;
>+}
>+
> /* Response handler for Intel card */
> static int ncsi_rsp_handler_oem_intel(struct ncsi_request *nr)
> {
>+	struct ncsi_rsp_oem_intel_pkt *intel;
>+	struct ncsi_rsp_oem_pkt *rsp;
>+
>+	/* Get the response header */
>+	rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
>+	intel = (struct ncsi_rsp_oem_intel_pkt *)(rsp->data);
>+
>+	if (intel->cmd == NCSI_OEM_INTEL_CMD_GMA)
>+		return ncsi_rsp_handler_oem_intel_gma(nr);
>+
> 	return 0;
> }
> 
>-- 
>2.31.1
>
>

Re: [PATCH 1/1] net/ncsi: add get MAC address command to get Intel i210 MAC address

[Ivan Mikhaylov]

On Thu, 2021-09-02 at 05:48 +0000, Milton Miller II wrote:
> On August 30, 2021, Ivan Mikhaylov" <i.mikhaylov@yadro.com> wrote:
> > This patch adds OEM Intel GMA command and response handler for it.
> > 
> > /* 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 d48374894817..6447a09932f5 100644
> > --- a/net/ncsi/ncsi-rsp.c
> > +++ b/net/ncsi/ncsi-rsp.c
> > @@ -699,9 +699,51 @@ static int ncsi_rsp_handler_oem_bcm(struct
> > ncsi_request *nr)
> >         return 0;
> > }
> > 
> > +/* Response handler for Intel command Get Mac Address */
> > +static int ncsi_rsp_handler_oem_intel_gma(struct ncsi_request *nr)
> > +{
> > +       struct ncsi_dev_priv *ndp = nr->ndp;
> > +       struct net_device *ndev = ndp->ndev.dev;
> > +       const struct net_device_ops *ops = ndev->netdev_ops;
> > +       struct ncsi_rsp_oem_pkt *rsp;
> > +       struct sockaddr saddr;
> > +       int ret = 0;
> > +
> > +       /* Get the response header */
> > +       rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
> > +
> > +       saddr.sa_family = ndev->type;
> > +       ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
> > +       memcpy(saddr.sa_data, &rsp->data[INTEL_MAC_ADDR_OFFSET], ETH_ALEN);
> > +       /* Increase mac address by 1 for BMC's address */
> > +       eth_addr_inc((u8 *)saddr.sa_data);
> > +       if (!is_valid_ether_addr((const u8 *)saddr.sa_data))
> > +               return -ENXIO;
> 
> The Intel GMA retireves the MAC address of the host, and the datasheet
> anticipates the BMC will "share" the MAC by stealing specific TCP and 
> UDP port traffic destined to the host.
> 
> This "add one" allocation of the MAC is therefore a policy, and one that 
> is beyond the data sheet.
> 
> While this +1 policy may work for some OEM boards, there are other boards 
> where the MAC address assigned to the BMC does not follow this pattern, 
> but instead the MAC is stored in some platform dependent location obtained 
> in a platform specific manner.
> 
> I suggest this BMC = ether_addr_inc(GMA) be opt in via a device tree
> property.  
> 
> as it appears it would be generic to more than one vendor.
> 
> Unfortunately, we missed this when we added the broadcom and mellanox
> handlers.
> 
> 
> 

Milton,

maybe something like "mac_addr_inc" or "ncsi,mac_addr_inc"? Also those 3(intel,
mellanox, broadcom) functions even with handlers similar to each other, they
could be unified on idea, difference in addresses, payload lengths, ids only as
I see. Joel proposed in the past about dts option for Intel OEM keep_phy option,
maybe that's the right time to reorganize all OEM related parts to fit in one
direction with dts options for ethernet interface without Kconfig options?

Thanks.

Re: [PATCH 1/1] net/ncsi: add get MAC address command to get Intel i210 MAC address

[Ivan Mikhaylov]

On Thu, 2021-09-02 at 05:48 +0000, Milton Miller II wrote:
> On August 30, 2021, Ivan Mikhaylov" <i.mikhaylov@yadro.com> wrote:
> > This patch adds OEM Intel GMA command and response handler for it.
> > 
> > /* 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 d48374894817..6447a09932f5 100644
> > --- a/net/ncsi/ncsi-rsp.c
> > +++ b/net/ncsi/ncsi-rsp.c
> > @@ -699,9 +699,51 @@ static int ncsi_rsp_handler_oem_bcm(struct
> > ncsi_request *nr)
> >         return 0;
> > }
> > 
> > +/* Response handler for Intel command Get Mac Address */
> > +static int ncsi_rsp_handler_oem_intel_gma(struct ncsi_request *nr)
> > +{
> > +       struct ncsi_dev_priv *ndp = nr->ndp;
> > +       struct net_device *ndev = ndp->ndev.dev;
> > +       const struct net_device_ops *ops = ndev->netdev_ops;
> > +       struct ncsi_rsp_oem_pkt *rsp;
> > +       struct sockaddr saddr;
> > +       int ret = 0;
> > +
> > +       /* Get the response header */
> > +       rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
> > +
> > +       saddr.sa_family = ndev->type;
> > +       ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
> > +       memcpy(saddr.sa_data, &rsp->data[INTEL_MAC_ADDR_OFFSET], ETH_ALEN);
> > +       /* Increase mac address by 1 for BMC's address */
> > +       eth_addr_inc((u8 *)saddr.sa_data);
> > +       if (!is_valid_ether_addr((const u8 *)saddr.sa_data))
> > +               return -ENXIO;
> 
> The Intel GMA retireves the MAC address of the host, and the datasheet
> anticipates the BMC will "share" the MAC by stealing specific TCP and 
> UDP port traffic destined to the host.
> 
> This "add one" allocation of the MAC is therefore a policy, and one that 
> is beyond the data sheet.
> 
> While this +1 policy may work for some OEM boards, there are other boards 
> where the MAC address assigned to the BMC does not follow this pattern, 
> but instead the MAC is stored in some platform dependent location obtained 
> in a platform specific manner.
> 
> I suggest this BMC = ether_addr_inc(GMA) be opt in via a device tree
> property.  
> 
> as it appears it would be generic to more than one vendor.
> 
> Unfortunately, we missed this when we added the broadcom and mellanox
> handlers.
> 
> 
> 

Milton,

maybe something like "mac_addr_inc" or "ncsi,mac_addr_inc"? Also those 3(intel,
mellanox, broadcom) functions even with handlers similar to each other, they
could be unified on idea, difference in addresses, payload lengths, ids only as
I see. Joel proposed in the past about dts option for Intel OEM keep_phy option,
maybe that's the right time to reorganize all OEM related parts to fit in one
direction with dts options for ethernet interface without Kconfig options?

Thanks.

RE: [PATCH 1/1] net/ncsi: add get MAC address command to get Intel i210 MAC address

[Milton Miller II]

On September 2, 2021, Ivan Mikhaylov wrote:

>On Thu, 2021-09-02 at 05:48 +0000, Milton Miller II wrote:
>> On August 30, 2021, Ivan Mikhaylov" <i.mikhaylov@yadro.com> wrote:
>> > This patch adds OEM Intel GMA command and response handler for
>it.
>> > 
>> > /* 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 d48374894817..6447a09932f5 100644
>> > --- a/net/ncsi/ncsi-rsp.c
>> > +++ b/net/ncsi/ncsi-rsp.c
>> > @@ -699,9 +699,51 @@ static int ncsi_rsp_handler_oem_bcm(struct
>> > ncsi_request *nr)
>> >         return 0;
>> > }
>> > 
>> > +/* Response handler for Intel command Get Mac Address */
>> > +static int ncsi_rsp_handler_oem_intel_gma(struct ncsi_request
>*nr)
>> > +{
>> > +       struct ncsi_dev_priv *ndp = nr->ndp;
>> > +       struct net_device *ndev = ndp->ndev.dev;
>> > +       const struct net_device_ops *ops = ndev->netdev_ops;
>> > +       struct ncsi_rsp_oem_pkt *rsp;
>> > +       struct sockaddr saddr;
>> > +       int ret = 0;
>> > +
>> > +       /* Get the response header */
>> > +       rsp = (struct ncsi_rsp_oem_pkt
>*)skb_network_header(nr->rsp);
>> > +
>> > +       saddr.sa_family = ndev->type;
>> > +       ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
>> > +       memcpy(saddr.sa_data, &rsp->data[INTEL_MAC_ADDR_OFFSET],
>ETH_ALEN);
>> > +       /* Increase mac address by 1 for BMC's address */
>> > +       eth_addr_inc((u8 *)saddr.sa_data);
>> > +       if (!is_valid_ether_addr((const u8 *)saddr.sa_data))
>> > +               return -ENXIO;
>> 
>> The Intel GMA retireves the MAC address of the host, and the
>datasheet
>> anticipates the BMC will "share" the MAC by stealing specific TCP
>and 
>> UDP port traffic destined to the host.
>> 
>> This "add one" allocation of the MAC is therefore a policy, and one
>that 
>> is beyond the data sheet.
>> 
>> While this +1 policy may work for some OEM boards, there are other
>boards 
>> where the MAC address assigned to the BMC does not follow this
>pattern, 
>> but instead the MAC is stored in some platform dependent location
>obtained 
>> in a platform specific manner.
>> 
>> I suggest this BMC = ether_addr_inc(GMA) be opt in via a device
>tree
>> property.  
>> 
>> as it appears it would be generic to more than one vendor.
>> 
>> Unfortunately, we missed this when we added the broadcom and
>mellanox
>> handlers.
>> 
>> 
>> 
>
>Milton,
>
>maybe something like "mac_addr_inc" or "ncsi,mac_addr_inc"? Also
>those 3(intel,
>mellanox, broadcom) functions even with handlers similar to each
>other, they
>could be unified on idea, difference in addresses, payload lengths,
>ids only as
>I see. Joel proposed in the past about dts option for Intel OEM
>keep_phy option,
>maybe that's the right time to reorganize all OEM related parts to
>fit in one
>direction with dts options for ethernet interface without Kconfig
>options?

I was hopping to get some feed back from device tree maintainers. 
I hope we can get something decided before we have to ask for a 
revert.  

Since the existing properties are mac-address and local-mac-address, 
I feel the new property should build upon the former like the later.  
I think the most general would be to have an offset that could be 
positive or negative.  I don't think we necessarily need the full 
range of address offset as I expect the upper bytes would be remain 
the assigned block but maybe some would want a large offset in the 
administrativly set address space?  or buy 2 ranges and assign one 
from each?

Anyways, I propose one of

mac-address-host-offset
host-mac-address-offset

how do we make it clear its the offset from the host to the BMC not 
from the BMC to the host?   Is the description in the binding enough?

Do we need more than 3 bytes offset?  How should we represent a 
decrement vs an increment?  sign extend a u32?  two cells for u64?
treat the first byte as add or subtract and the rest the offset?  Do 
we need a separate property name to subtract?

My system stores the MAC for the BMC elsewhere but we need a 
way to opt out of using an offset from the host, hence the need of
at least some property to opt in.





Some background for Rob (and others):

DTMF spec DSP0222 NC-SI (network controller sideband interface)
is a method to provide a BMC (Baseboard management controller) shared
access to an external ethernet port for comunication to the management
network in the outside world.  The protocol describes ethernet packets 
that control selective bridging implemented in a host network controller
to share its phy.  Various NIC OEMs have added a query to find out the 
address the host is using, and some vendors have added code to query host
nic and set the BMC mac to a fixed offset (current hard coded +1 from
the host value).  If this is compiled in the kernel, the NIC OEM is 
recognised and the BMC doesn't miss the NIC response the address is set
once each time the NCSI stack reinitializes.  This mechanism overrides
any mac-address or local-mac-address or other assignment.

DSP0222 https://www.dmtf.org/documents/pmci/network-controller-sideband-interface-nc-si-specification-110

milton

RE: [PATCH 1/1] net/ncsi: add get MAC address command to get Intel i210 MAC address

[Milton Miller II]

On September 2, 2021, Ivan Mikhaylov wrote:

>On Thu, 2021-09-02 at 05:48 +0000, Milton Miller II wrote:
>> On August 30, 2021, Ivan Mikhaylov" <i.mikhaylov@yadro.com> wrote:
>> > This patch adds OEM Intel GMA command and response handler for
>it.
>> > 
>> > /* 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 d48374894817..6447a09932f5 100644
>> > --- a/net/ncsi/ncsi-rsp.c
>> > +++ b/net/ncsi/ncsi-rsp.c
>> > @@ -699,9 +699,51 @@ static int ncsi_rsp_handler_oem_bcm(struct
>> > ncsi_request *nr)
>> >         return 0;
>> > }
>> > 
>> > +/* Response handler for Intel command Get Mac Address */
>> > +static int ncsi_rsp_handler_oem_intel_gma(struct ncsi_request
>*nr)
>> > +{
>> > +       struct ncsi_dev_priv *ndp = nr->ndp;
>> > +       struct net_device *ndev = ndp->ndev.dev;
>> > +       const struct net_device_ops *ops = ndev->netdev_ops;
>> > +       struct ncsi_rsp_oem_pkt *rsp;
>> > +       struct sockaddr saddr;
>> > +       int ret = 0;
>> > +
>> > +       /* Get the response header */
>> > +       rsp = (struct ncsi_rsp_oem_pkt
>*)skb_network_header(nr->rsp);
>> > +
>> > +       saddr.sa_family = ndev->type;
>> > +       ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
>> > +       memcpy(saddr.sa_data, &rsp->data[INTEL_MAC_ADDR_OFFSET],
>ETH_ALEN);
>> > +       /* Increase mac address by 1 for BMC's address */
>> > +       eth_addr_inc((u8 *)saddr.sa_data);
>> > +       if (!is_valid_ether_addr((const u8 *)saddr.sa_data))
>> > +               return -ENXIO;
>> 
>> The Intel GMA retireves the MAC address of the host, and the
>datasheet
>> anticipates the BMC will "share" the MAC by stealing specific TCP
>and 
>> UDP port traffic destined to the host.
>> 
>> This "add one" allocation of the MAC is therefore a policy, and one
>that 
>> is beyond the data sheet.
>> 
>> While this +1 policy may work for some OEM boards, there are other
>boards 
>> where the MAC address assigned to the BMC does not follow this
>pattern, 
>> but instead the MAC is stored in some platform dependent location
>obtained 
>> in a platform specific manner.
>> 
>> I suggest this BMC = ether_addr_inc(GMA) be opt in via a device
>tree
>> property.  
>> 
>> as it appears it would be generic to more than one vendor.
>> 
>> Unfortunately, we missed this when we added the broadcom and
>mellanox
>> handlers.
>> 
>> 
>> 
>
>Milton,
>
>maybe something like "mac_addr_inc" or "ncsi,mac_addr_inc"? Also
>those 3(intel,
>mellanox, broadcom) functions even with handlers similar to each
>other, they
>could be unified on idea, difference in addresses, payload lengths,
>ids only as
>I see. Joel proposed in the past about dts option for Intel OEM
>keep_phy option,
>maybe that's the right time to reorganize all OEM related parts to
>fit in one
>direction with dts options for ethernet interface without Kconfig
>options?

I was hopping to get some feed back from device tree maintainers. 
I hope we can get something decided before we have to ask for a 
revert.  

Since the existing properties are mac-address and local-mac-address, 
I feel the new property should build upon the former like the later.  
I think the most general would be to have an offset that could be 
positive or negative.  I don't think we necessarily need the full 
range of address offset as I expect the upper bytes would be remain 
the assigned block but maybe some would want a large offset in the 
administrativly set address space?  or buy 2 ranges and assign one 
from each?

Anyways, I propose one of

mac-address-host-offset
host-mac-address-offset

how do we make it clear its the offset from the host to the BMC not 
from the BMC to the host?   Is the description in the binding enough?

Do we need more than 3 bytes offset?  How should we represent a 
decrement vs an increment?  sign extend a u32?  two cells for u64?
treat the first byte as add or subtract and the rest the offset?  Do 
we need a separate property name to subtract?

My system stores the MAC for the BMC elsewhere but we need a 
way to opt out of using an offset from the host, hence the need of
at least some property to opt in.





Some background for Rob (and others):

DTMF spec DSP0222 NC-SI (network controller sideband interface)
is a method to provide a BMC (Baseboard management controller) shared
access to an external ethernet port for comunication to the management
network in the outside world.  The protocol describes ethernet packets 
that control selective bridging implemented in a host network controller
to share its phy.  Various NIC OEMs have added a query to find out the 
address the host is using, and some vendors have added code to query host
nic and set the BMC mac to a fixed offset (current hard coded +1 from
the host value).  If this is compiled in the kernel, the NIC OEM is 
recognised and the BMC doesn't miss the NIC response the address is set
once each time the NCSI stack reinitializes.  This mechanism overrides
any mac-address or local-mac-address or other assignment.

DSP0222 https://www.dmtf.org/documents/pmci/network-controller-sideband-interface-nc-si-specification-110

milton