[dpdk-dev] [PATCH] net/sfc: fix main MAC address handling

[Andrew Rybchenko <arybchenko@solarflare.com>]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

There is a school of thought that rte_eth_dev_default_mac_addr_set()
must call the PMD callback first and then save the new MAC address
in dev->data->mac_addrs[0]. If this concept gets approved, it will
break the current approach used in sfc driver as the latter relies
on the assumption that the new MAC address is already contained in
dev->data->mac_addrs[0], and, if adapter restart is needed to make
the HW apply the new address, the outdated value will be retrieved
from dev->data. In order to preclude any possible bugs, this patch
adds device private storage for the up-to-date copy of the address.

Fixes: c100fd464bb7 ("net/sfc: support main MAC address change")
Cc: stable@dpdk.org

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/sfc/sfc.h        |  2 ++
 drivers/net/sfc/sfc_ethdev.c | 19 ++++++++++++++++---
 drivers/net/sfc/sfc_port.c   | 10 ++++++++--
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 7f11bf2..b72eba0 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -158,6 +158,8 @@ struct sfc_port {
 	boolean_t			promisc;
 	boolean_t			allmulti;
 
+	struct ether_addr		default_mac_addr;
+
 	unsigned int			max_mcast_addrs;
 	unsigned int			nb_mcast_addrs;
 	uint8_t				*mcast_addrs;
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 2f5f86f..fabcc32 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -926,6 +926,12 @@ sfc_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 
 	sfc_adapter_lock(sa);
 
+	/*
+	 * Copy the address to the device private data so that
+	 * it could be recalled in the case of adapter restart.
+	 */
+	ether_addr_copy(mac_addr, &port->default_mac_addr);
+
 	if (port->isolated) {
 		sfc_err(sa, "isolated mode is active on the port");
 		sfc_err(sa, "will not set MAC address");
@@ -961,9 +967,9 @@ sfc_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 
 		/*
 		 * Since setting MAC address with filters installed is not
-		 * allowed on the adapter, one needs to simply restart adapter
-		 * so that the new MAC address will be taken from an outer
-		 * storage and set flawlessly by means of sfc_start() call
+		 * allowed on the adapter, the new MAC address will be set
+		 * by means of adapter restart. sfc_start() shall retrieve
+		 * the new address from the device private data and set it.
 		 */
 		sfc_stop(sa);
 		rc = sfc_start(sa);
@@ -972,6 +978,13 @@ sfc_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 	}
 
 unlock:
+	/*
+	 * In the case of failure sa->port->default_mac_addr does not
+	 * need rollback since no error code is returned, and the upper
+	 * API will anyway update the external MAC address storage.
+	 * To be consistent with that new value it is better to keep
+	 * the device private value the same.
+	 */
 	sfc_adapter_unlock(sa);
 }
 
diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
index 6cfd0e3..5254394 100644
--- a/drivers/net/sfc/sfc_port.c
+++ b/drivers/net/sfc/sfc_port.c
@@ -188,10 +188,10 @@ sfc_port_start(struct sfc_adapter *sa)
 		goto fail_mac_pdu_set;
 
 	if (!port->isolated) {
-		struct ether_addr *mac_addrs = sa->eth_dev->data->mac_addrs;
+		struct ether_addr *addr = &port->default_mac_addr;
 
 		sfc_log_init(sa, "set MAC address");
-		rc = efx_mac_addr_set(sa->nic, mac_addrs[0].addr_bytes);
+		rc = efx_mac_addr_set(sa->nic, addr->addr_bytes);
 		if (rc != 0)
 			goto fail_mac_addr_set;
 
@@ -342,6 +342,8 @@ int
 sfc_port_attach(struct sfc_adapter *sa)
 {
 	struct sfc_port *port = &sa->port;
+	const efx_nic_cfg_t *encp = efx_nic_cfg_get(sa->nic);
+	const struct ether_addr *from;
 	long kvarg_stats_update_period_ms;
 	int rc;
 
@@ -353,6 +355,10 @@ sfc_port_attach(struct sfc_adapter *sa)
 	port->flow_ctrl = EFX_FCNTL_RESPOND | EFX_FCNTL_GENERATE;
 	port->flow_ctrl_autoneg = B_TRUE;
 
+	RTE_BUILD_BUG_ON(sizeof(encp->enc_mac_addr) != sizeof(*from));
+	from = (const struct ether_addr *)(encp->enc_mac_addr);
+	ether_addr_copy(from, &port->default_mac_addr);
+
 	port->max_mcast_addrs = EFX_MAC_MULTICAST_LIST_MAX;
 	port->nb_mcast_addrs = 0;
 	port->mcast_addrs = rte_calloc_socket("mcast_addr_list_buf",
-- 
2.7.4