Skip to contentSkip to conversation

sfc: compat for lack of VADAPTOR_SET_MAC in adaptor_firmware <= 4.1.1.1023

PATCH In review Shradha Shah 2 participants 5 messages Jul 7, 2015 – Jul 8, 2015

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover sfc: compat for lack of VADAPTOR_SET_MAC in adaptor_firmware <= 4.1.1.1023

This patch series resolves an incompatibility with legacy firmware due to the lack of MC_CMD_VADAPTOR_SET_MAC in adaptor_firmware <= 4.1.1.1023

Unless this patch series is applied there will be a compatibility issue between the driver and Solarflare adapters running older firmware.

Tested with and without CONFIG_SFC_SRIOV

Daniel Pieczko (3):
sfc: refactor code in efx_ef10_set_mac_address()
sfc: add legacy method for changing a PF's MAC address
sfc: suppress handled MCDI failures when changing the MAC address

 drivers/net/ethernet/sfc/ef10.c       | 172 ++++++++++++++++++++++++++++------
 drivers/net/ethernet/sfc/ef10_sriov.c |  42 ---------
 drivers/net/ethernet/sfc/ef10_sriov.h |   6 ++
 3 files changed, 151 insertions(+), 69 deletions(-)
#1 lore raw

PATCH 1/3 sfc: refactor code in efx_ef10_set_mac_address()

From: Daniel Pieczko <[email protected]>

Re-organize the structure of error handling to avoid having to duplicate the netif_err() around the ifdefs.

The only change to the behaviour of the error-handling is that the PF's data structure to record VF details should only be updated if the original command succeeded.

---
 drivers/net/ethernet/sfc/ef10.c | 45 ++++++++++++++++++-----------------------
 1 file changed, 20 insertions(+), 25 deletions(-)
drivers/net/ethernet/sfc/ef10.c +20 -25
index 8476434..9740cd0 100644
@@ -3829,38 +3829,27 @@ static int efx_ef10_set_mac_address(struct efx_nic *efx)
 		efx_net_open(efx->net_dev);
 	netif_device_attach(efx->net_dev);
 
-#if !defined(CONFIG_SFC_SRIOV)
-	if (rc == -EPERM)
-		netif_err(efx, drv, efx->net_dev,
-			  "Cannot change MAC address; use sfboot to enable mac-spoofing"
-			  " on this interface\n");
-#else
-	if (rc == -EPERM) {
+#ifdef CONFIG_SFC_SRIOV
+	if (efx->pci_dev->is_virtfn && efx->pci_dev->physfn) {
 		struct pci_dev *pci_dev_pf = efx->pci_dev->physfn;
 
-		/* Switch to PF and change MAC address on vport */
-		if (efx->pci_dev->is_virtfn && pci_dev_pf) {
-			struct efx_nic *efx_pf = pci_get_drvdata(pci_dev_pf);
+		if (rc == -EPERM) {
+			struct efx_nic *efx_pf;
 
-			if (!efx_ef10_sriov_set_vf_mac(efx_pf,
-						       nic_data->vf_index,
-						       efx->net_dev->dev_addr))
-				return 0;
-		}
-		netif_err(efx, drv, efx->net_dev,
-			  "Cannot change MAC address; use sfboot to enable mac-spoofing"
-			  " on this interface\n");
-	} else if (efx->pci_dev->is_virtfn) {
-		/* Successfully changed by VF (with MAC spoofing), so update the
-		 * parent PF if possible.
-		 */
-		struct pci_dev *pci_dev_pf = efx->pci_dev->physfn;
+			/* Switch to PF and change MAC address on vport */
+			efx_pf = pci_get_drvdata(pci_dev_pf);
 
-		if (pci_dev_pf) {
+			rc = efx_ef10_sriov_set_vf_mac(efx_pf,
+						       nic_data->vf_index,
+						       efx->net_dev->dev_addr);
+		} else if (!rc) {
 			struct efx_nic *efx_pf = pci_get_drvdata(pci_dev_pf);
 			struct efx_ef10_nic_data *nic_data = efx_pf->nic_data;
 			unsigned int i;
 
+			/* MAC address successfully changed by VF (with MAC
+			 * spoofing) so update the parent PF if possible.
+			 */
 			for (i = 0; i < efx_pf->vf_count; ++i) {
 				struct ef10_vf *vf = nic_data->vf + i;
 
@@ -3871,8 +3860,14 @@ static int efx_ef10_set_mac_address(struct efx_nic *efx)
 				}
 			}
 		}
-	}
+	} else
 #endif
+	if (rc == -EPERM) {
+		netif_err(efx, drv, efx->net_dev,
+			  "Cannot change MAC address; use sfboot to enable"
+			  " mac-spoofing on this interface\n");
+	}
+
 	return rc;
 }
 
#2 lore raw

PATCH 2/3 sfc: add legacy method for changing a PF's MAC address

In reply to Shradha Shah

From: Daniel Pieczko <[email protected]>

Some versions of MCFW do not support the MC_CMD_VADAPTOR_SET_MAC command, and ENOSYS will be returned.

If the PF created its own vport, the function's datapath must be stopped and the vport can be reconfigured to reflect the new MAC address.

If the MCFW created the vport for the PF (which is the case when the nic_data->vport_mac is blank), nothing further needs to be done as the vport is not under the control of the PF.

This only applies to PFs because the MCFW in question does not support VFs.

---
 drivers/net/ethernet/sfc/ef10.c       | 120 ++++++++++++++++++++++++++++++++++
 drivers/net/ethernet/sfc/ef10_sriov.c |  42 ------------
 drivers/net/ethernet/sfc/ef10_sriov.h |   6 ++
 3 files changed, 126 insertions(+), 42 deletions(-)
drivers/net/ethernet/sfc/ef10.c +120 -0
index 9740cd0..e0cb361 100644
@@ -101,6 +101,11 @@ static unsigned int efx_ef10_mem_map_size(struct efx_nic *efx)
 	return resource_size(&efx->pci_dev->resource[bar]);
 }
 
+static bool efx_ef10_is_vf(struct efx_nic *efx)
+{
+	return efx->type->is_vf;
+}
+
 static int efx_ef10_get_pf_index(struct efx_nic *efx)
 {
 	MCDI_DECLARE_BUF(outbuf, MC_CMD_GET_FUNCTION_INFO_OUT_LEN);
@@ -677,6 +682,48 @@ static int efx_ef10_probe_pf(struct efx_nic *efx)
 	return efx_ef10_probe(efx);
 }
 
+int efx_ef10_vadaptor_alloc(struct efx_nic *efx, unsigned int port_id)
+{
+	MCDI_DECLARE_BUF(inbuf, MC_CMD_VADAPTOR_ALLOC_IN_LEN);
+
+	MCDI_SET_DWORD(inbuf, VADAPTOR_ALLOC_IN_UPSTREAM_PORT_ID, port_id);
+	return efx_mcdi_rpc(efx, MC_CMD_VADAPTOR_ALLOC, inbuf, sizeof(inbuf),
+			    NULL, 0, NULL);
+}
+
+int efx_ef10_vadaptor_free(struct efx_nic *efx, unsigned int port_id)
+{
+	MCDI_DECLARE_BUF(inbuf, MC_CMD_VADAPTOR_FREE_IN_LEN);
+
+	MCDI_SET_DWORD(inbuf, VADAPTOR_FREE_IN_UPSTREAM_PORT_ID, port_id);
+	return efx_mcdi_rpc(efx, MC_CMD_VADAPTOR_FREE, inbuf, sizeof(inbuf),
+			    NULL, 0, NULL);
+}
+
+int efx_ef10_vport_add_mac(struct efx_nic *efx,
+			   unsigned int port_id, u8 *mac)
+{
+	MCDI_DECLARE_BUF(inbuf, MC_CMD_VPORT_ADD_MAC_ADDRESS_IN_LEN);
+
+	MCDI_SET_DWORD(inbuf, VPORT_ADD_MAC_ADDRESS_IN_VPORT_ID, port_id);
+	ether_addr_copy(MCDI_PTR(inbuf, VPORT_ADD_MAC_ADDRESS_IN_MACADDR), mac);
+
+	return efx_mcdi_rpc(efx, MC_CMD_VPORT_ADD_MAC_ADDRESS, inbuf,
+			    sizeof(inbuf), NULL, 0, NULL);
+}
+
+int efx_ef10_vport_del_mac(struct efx_nic *efx,
+			   unsigned int port_id, u8 *mac)
+{
+	MCDI_DECLARE_BUF(inbuf, MC_CMD_VPORT_DEL_MAC_ADDRESS_IN_LEN);
+
+	MCDI_SET_DWORD(inbuf, VPORT_DEL_MAC_ADDRESS_IN_VPORT_ID, port_id);
+	ether_addr_copy(MCDI_PTR(inbuf, VPORT_DEL_MAC_ADDRESS_IN_MACADDR), mac);
+
+	return efx_mcdi_rpc(efx, MC_CMD_VPORT_DEL_MAC_ADDRESS, inbuf,
+			    sizeof(inbuf), NULL, 0, NULL);
+}
+
 #ifdef CONFIG_SFC_SRIOV
 static int efx_ef10_probe_vf(struct efx_nic *efx)
 {
@@ -3804,6 +3851,72 @@ static void efx_ef10_filter_sync_rx_mode(struct efx_nic *efx)
 	WARN_ON(remove_failed);
 }
 
+static int efx_ef10_vport_set_mac_address(struct efx_nic *efx)
+{
+	struct efx_ef10_nic_data *nic_data = efx->nic_data;
+	u8 mac_old[ETH_ALEN];
+	int rc, rc2;
+
+	/* Only reconfigure a PF-created vport */
+	if (is_zero_ether_addr(nic_data->vport_mac))
+		return 0;
+
+	efx_device_detach_sync(efx);
+	efx_net_stop(efx->net_dev);
+	down_write(&efx->filter_sem);
+	efx_ef10_filter_table_remove(efx);
+	up_write(&efx->filter_sem);
+
+	rc = efx_ef10_vadaptor_free(efx, nic_data->vport_id);
+	if (rc)
+		goto restore_filters;
+
+	ether_addr_copy(mac_old, nic_data->vport_mac);
+	rc = efx_ef10_vport_del_mac(efx, nic_data->vport_id,
+				    nic_data->vport_mac);
+	if (rc)
+		goto restore_vadaptor;
+
+	rc = efx_ef10_vport_add_mac(efx, nic_data->vport_id,
+				    efx->net_dev->dev_addr);
+	if (!rc) {
+		ether_addr_copy(nic_data->vport_mac, efx->net_dev->dev_addr);
+	} else {
+		rc2 = efx_ef10_vport_add_mac(efx, nic_data->vport_id, mac_old);
+		if (rc2) {
+			/* Failed to add original MAC, so clear vport_mac */
+			eth_zero_addr(nic_data->vport_mac);
+			goto reset_nic;
+		}
+	}
+
+restore_vadaptor:
+	rc2 = efx_ef10_vadaptor_alloc(efx, nic_data->vport_id);
+	if (rc2)
+		goto reset_nic;
+restore_filters:
+	down_write(&efx->filter_sem);
+	rc2 = efx_ef10_filter_table_probe(efx);
+	up_write(&efx->filter_sem);
+	if (rc2)
+		goto reset_nic;
+
+	rc2 = efx_net_open(efx->net_dev);
+	if (rc2)
+		goto reset_nic;
+
+	netif_device_attach(efx->net_dev);
+
+	return rc;
+
+reset_nic:
+	netif_err(efx, drv, efx->net_dev,
+		  "Failed to restore when changing MAC address - scheduling reset\n");
+	efx_schedule_reset(efx, RESET_TYPE_DATAPATH);
+
+	return rc ? rc : rc2;
+}
+
 static int efx_ef10_set_mac_address(struct efx_nic *efx)
 {
 	MCDI_DECLARE_BUF(inbuf, MC_CMD_VADAPTOR_SET_MAC_IN_LEN);
@@ -3866,6 +3979,13 @@ static int efx_ef10_set_mac_address(struct efx_nic *efx)
 		netif_err(efx, drv, efx->net_dev,
 			  "Cannot change MAC address; use sfboot to enable"
 			  " mac-spoofing on this interface\n");
+	} else if (rc == -ENOSYS && !efx_ef10_is_vf(efx)) {
+		/* If the active MCFW does not support MC_CMD_VADAPTOR_SET_MAC
+		 * fall-back to the method of changing the MAC address on the
+		 * vport.  This only applies to PFs because such versions of
+		 * MCFW do not support VFs.
+		 */
+		rc = efx_ef10_vport_set_mac_address(efx);
 	}
 
 	return rc;
drivers/net/ethernet/sfc/ef10_sriov.c +0 -42

54 lines not on this page · raw message

drivers/net/ethernet/sfc/ef10_sriov.h +6 -0
index db4ef53..6d25b92 100644
@@ -65,5 +65,11 @@ int efx_ef10_vswitching_restore_pf(struct efx_nic *efx);
 int efx_ef10_vswitching_restore_vf(struct efx_nic *efx);
 void efx_ef10_vswitching_remove_pf(struct efx_nic *efx);
 void efx_ef10_vswitching_remove_vf(struct efx_nic *efx);
+int efx_ef10_vport_add_mac(struct efx_nic *efx,
+			   unsigned int port_id, u8 *mac);
+int efx_ef10_vport_del_mac(struct efx_nic *efx,
+			   unsigned int port_id, u8 *mac);
+int efx_ef10_vadaptor_alloc(struct efx_nic *efx, unsigned int port_id);
+int efx_ef10_vadaptor_free(struct efx_nic *efx, unsigned int port_id);
 
 #endif /* EF10_SRIOV_H */
#3 lore raw

PATCH 3/3 sfc: suppress handled MCDI failures when changing the MAC address

In reply to Shradha Shah

From: Daniel Pieczko <[email protected]>

---
 drivers/net/ethernet/sfc/ef10.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/net/ethernet/sfc/ef10.c +5 -2
index e0cb361..605cc89 100644
@@ -3933,8 +3933,8 @@ static int efx_ef10_set_mac_address(struct efx_nic *efx)
 			efx->net_dev->dev_addr);
 	MCDI_SET_DWORD(inbuf, VADAPTOR_SET_MAC_IN_UPSTREAM_PORT_ID,
 		       nic_data->vport_id);
-	rc = efx_mcdi_rpc(efx, MC_CMD_VADAPTOR_SET_MAC, inbuf,
-			  sizeof(inbuf), NULL, 0, NULL);
+	rc = efx_mcdi_rpc_quiet(efx, MC_CMD_VADAPTOR_SET_MAC, inbuf,
+				sizeof(inbuf), NULL, 0, NULL);
 
 	efx_ef10_filter_table_probe(efx);
 	up_write(&efx->filter_sem);
@@ -3986,6 +3986,9 @@ static int efx_ef10_set_mac_address(struct efx_nic *efx)
 		 * MCFW do not support VFs.
 		 */
 		rc = efx_ef10_vport_set_mac_address(efx);
+	} else {
+		efx_mcdi_display_error(efx, MC_CMD_VADAPTOR_SET_MAC,
+				       sizeof(inbuf), NULL, 0, rc);
 	}
 
 	return rc;
maintainer
#4 lore raw
In reply to Shradha Shah

From: Shradha Shah <[email protected]>
Date: Tue, 7 Jul 2015 11:35:18 +0100

Quoted 9 lines

This patch series resolves an incompatibility with legacy firmware due to the lack of MC_CMD_VADAPTOR_SET_MAC in adaptor_firmware <= 4.1.1.1023

Unless this patch series is applied there will be a compatibility issue between the driver and Solarflare adapters running older firmware.

Tested with and without CONFIG_SFC_SRIOV

Series applied, thanks.