ethdev: return diagnostic when setting MAC address

Change the prototype and the behavior of dev_ops->eth_mac_addr_set(): a
return code is added to notify the caller (librte_ether) if an error
occurred in the PMD.

The new default MAC address is now copied in dev->data->mac_addrs[0]
only if the operation is successful.

The patch also updates all the PMDs accordingly.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

doc/guides/rel_notes/deprecation.rst

@@ -100,14 +100,6 @@ Deprecation Notices
   between the VF representor and the VF or the parent PF. Those new fields
   are to be included in ``rte_eth_dev_info`` struct.
 
-* ethdev: The prototype and the behavior of
-  ``dev_ops->eth_mac_addr_set()`` will change in v18.05. A return code
-  will be added to notify the caller if an error occurred in the PMD. In
-  ``rte_eth_dev_default_mac_addr_set()``, the new default MAC address
-  will be copied in ``dev->data->mac_addrs[0]`` only if the operation is
-  successful. This modification will only impact the PMDs, not the
-  applications.
-
 * i40e: The default flexible payload configuration which extracts the first 16
   bytes of the payload for RSS will be deprecated starting from 18.02. If
   required the previous behavior can be configured using existing flow

drivers/net/ark/ark_ethdev.c

@@ -69,7 +69,7 @@ static int eth_ark_dev_set_link_down(struct rte_eth_dev *dev);
 static int eth_ark_dev_stats_get(struct rte_eth_dev *dev,
 				  struct rte_eth_stats *stats);
 static void eth_ark_dev_stats_reset(struct rte_eth_dev *dev);
-static void eth_ark_set_default_mac_addr(struct rte_eth_dev *dev,
+static int eth_ark_set_default_mac_addr(struct rte_eth_dev *dev,
 					 struct ether_addr *mac_addr);
 static int eth_ark_macaddr_add(struct rte_eth_dev *dev,
 			       struct ether_addr *mac_addr,
@@ -886,16 +886,19 @@ eth_ark_macaddr_remove(struct rte_eth_dev *dev, uint32_t index)
 			      ark->user_data[dev->data->port_id]);
 }
 
-static void
+static int
 eth_ark_set_default_mac_addr(struct rte_eth_dev *dev,
 			     struct ether_addr *mac_addr)
 {
 	struct ark_adapter *ark =
 		(struct ark_adapter *)dev->data->dev_private;
 
-	if (ark->user_ext.mac_addr_set)
+	if (ark->user_ext.mac_addr_set) {
 		ark->user_ext.mac_addr_set(dev, mac_addr,
 			   ark->user_data[dev->data->port_id]);
+		return 0;
+	}
+	return -ENOTSUP;
 }
 
 static int

drivers/net/avf/avf_ethdev.c

@@ -65,7 +65,7 @@ static int avf_dev_rss_hash_update(struct rte_eth_dev *dev,
 static int avf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 				     struct rte_eth_rss_conf *rss_conf);
 static int avf_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
-static void avf_dev_set_default_mac_addr(struct rte_eth_dev *dev,
+static int avf_dev_set_default_mac_addr(struct rte_eth_dev *dev,
 					 struct ether_addr *mac_addr);
 static int avf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev,
 					uint16_t queue_id);
@@ -925,7 +925,7 @@ avf_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	return ret;
 }
 
-static void
+static int
 avf_dev_set_default_mac_addr(struct rte_eth_dev *dev,
 			     struct ether_addr *mac_addr)
 {
@@ -939,11 +939,11 @@ avf_dev_set_default_mac_addr(struct rte_eth_dev *dev,
 	perm_addr = (struct ether_addr *)hw->mac.perm_addr;
 
 	if (is_same_ether_addr(mac_addr, old_addr))
-		return;
+		return 0;
 
 	/* If the MAC address is configured by host, skip the setting */
 	if (is_valid_assigned_ether_addr(perm_addr))
-		return;
+		return -EPERM;
 
 	ret = avf_add_del_eth_addr(adapter, old_addr, FALSE);
 	if (ret)
@@ -967,7 +967,11 @@ avf_dev_set_default_mac_addr(struct rte_eth_dev *dev,
 			    mac_addr->addr_bytes[4],
 			    mac_addr->addr_bytes[5]);
 
+	if (ret)
+		return -EIO;
+
 	ether_addr_copy(mac_addr, (struct ether_addr *)hw->mac.addr);
+	return 0;
 }
 
 static int

drivers/net/bnxt/bnxt_ethdev.c

@@ -1403,7 +1403,7 @@ bnxt_vlan_offload_set_op(struct rte_eth_dev *dev, int mask)
 	return 0;
 }
 
-static void
+static int
 bnxt_set_default_mac_addr_op(struct rte_eth_dev *dev, struct ether_addr *addr)
 {
 	struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
@@ -1413,7 +1413,7 @@ bnxt_set_default_mac_addr_op(struct rte_eth_dev *dev, struct ether_addr *addr)
 	int rc;
 
 	if (BNXT_VF(bp))
-		return;
+		return -EPERM;
 
 	memcpy(bp->mac_addr, addr, sizeof(bp->mac_addr));
 
@@ -1423,7 +1423,7 @@ bnxt_set_default_mac_addr_op(struct rte_eth_dev *dev, struct ether_addr *addr)
 			continue;
 		rc = bnxt_hwrm_clear_l2_filter(bp, filter);
 		if (rc)
-			break;
+			return rc;
 		memcpy(filter->l2_addr, bp->mac_addr, ETHER_ADDR_LEN);
 		memset(filter->l2_addr_mask, 0xff, ETHER_ADDR_LEN);
 		filter->flags |= HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_PATH_RX;
@@ -1432,10 +1432,12 @@ bnxt_set_default_mac_addr_op(struct rte_eth_dev *dev, struct ether_addr *addr)
 			HWRM_CFA_L2_FILTER_ALLOC_INPUT_ENABLES_L2_ADDR_MASK;
 		rc = bnxt_hwrm_set_l2_filter(bp, vnic->fw_vnic_id, filter);
 		if (rc)
-			break;
+			return rc;
 		filter->mac_index = 0;
 		PMD_DRV_LOG(DEBUG, "Set MAC addr\n");
 	}
+
+	return 0;
 }
 
 static int

drivers/net/bonding/rte_eth_bond_pmd.c

@@ -2872,11 +2872,15 @@ bond_ethdev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	return 0;
 }
 
-static void
+static int
 bond_ethdev_mac_address_set(struct rte_eth_dev *dev, struct ether_addr *addr)
 {
-	if (mac_address_set(dev, addr))
+	if (mac_address_set(dev, addr)) {
 		RTE_BOND_LOG(ERR, "Failed to update MAC address");
+		return -EINVAL;
+	}
+
+	return 0;
 }
 
 const struct eth_dev_ops default_dev_ops = {

drivers/net/cxgbe/cxgbe_ethdev.c

@@ -1056,7 +1056,7 @@ static int cxgbe_get_regs(struct rte_eth_dev *eth_dev,
 	return 0;
 }
 
-void cxgbe_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *addr)
+int cxgbe_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *addr)
 {
 	struct port_info *pi = (struct port_info *)(dev->data->dev_private);
 	struct adapter *adapter = pi->adapter;
@@ -1067,9 +1067,10 @@ void cxgbe_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *addr)
 	if (ret < 0) {
 		dev_err(adapter, "failed to set mac addr; err = %d\n",
 			ret);
-		return;
+		return ret;
 	}
 	pi->xact_addr_filt = ret;
+	return 0;
 }
 
 static const struct eth_dev_ops cxgbe_eth_dev_ops = {

drivers/net/cxgbe/cxgbe_pfvf.h

@@ -16,7 +16,7 @@ void cxgbe_dev_promiscuous_enable(struct rte_eth_dev *eth_dev);
 void cxgbe_dev_promiscuous_disable(struct rte_eth_dev *eth_dev);
 void cxgbe_dev_allmulticast_enable(struct rte_eth_dev *eth_dev);
 void cxgbe_dev_allmulticast_disable(struct rte_eth_dev *eth_dev);
-void cxgbe_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *addr);
+int cxgbe_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *addr);
 int cxgbe_dev_configure(struct rte_eth_dev *eth_dev);
 int cxgbe_dev_tx_queue_setup(struct rte_eth_dev *eth_dev, uint16_t queue_idx,
 			     uint16_t nb_desc, unsigned int socket_id,

drivers/net/dpaa/dpaa_ethdev.c

@@ -816,7 +816,7 @@ dpaa_dev_remove_mac_addr(struct rte_eth_dev *dev,
 	fman_if_clear_mac_addr(dpaa_intf->fif, index);
 }
 
-static void
+static int
 dpaa_dev_set_mac_addr(struct rte_eth_dev *dev,
 		       struct ether_addr *addr)
 {
@@ -828,6 +828,8 @@ dpaa_dev_set_mac_addr(struct rte_eth_dev *dev,
 	ret = fman_if_add_mac_addr(dpaa_intf->fif, addr->addr_bytes, 0);
 	if (ret)
 		RTE_LOG(ERR, PMD, "error: Setting the MAC ADDR failed %d", ret);
+
+	return ret;
 }
 
 static struct eth_dev_ops dpaa_devops = {

drivers/net/dpaa2/dpaa2_ethdev.c

@@ -1019,7 +1019,7 @@ dpaa2_dev_remove_mac_addr(struct rte_eth_dev *dev,
 			"error: Removing the MAC ADDR failed: err = %d", ret);
 }
 
-static void
+static int
 dpaa2_dev_set_mac_addr(struct rte_eth_dev *dev,
 		       struct ether_addr *addr)
 {
@@ -1031,7 +1031,7 @@ dpaa2_dev_set_mac_addr(struct rte_eth_dev *dev,
 
 	if (dpni == NULL) {
 		DPAA2_PMD_ERR("dpni is NULL");
-		return;
+		return -EINVAL;
 	}
 
 	ret = dpni_set_primary_mac_addr(dpni, CMD_PRI_LOW,
@@ -1040,6 +1040,8 @@ dpaa2_dev_set_mac_addr(struct rte_eth_dev *dev,
 	if (ret)
 		DPAA2_PMD_ERR(
 			"error: Setting the MAC ADDR failed %d", ret);
+
+	return ret;
 }
 
 static

drivers/net/e1000/em_ethdev.c

@@ -93,7 +93,7 @@ static int em_get_rx_buffer_size(struct e1000_hw *hw);
 static int eth_em_rar_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 			  uint32_t index, uint32_t pool);
 static void eth_em_rar_clear(struct rte_eth_dev *dev, uint32_t index);
-static void eth_em_default_mac_addr_set(struct rte_eth_dev *dev,
+static int eth_em_default_mac_addr_set(struct rte_eth_dev *dev,
 					 struct ether_addr *addr);
 
 static int eth_em_set_mc_addr_list(struct rte_eth_dev *dev,
@@ -1779,13 +1779,13 @@ eth_em_rar_clear(struct rte_eth_dev *dev, uint32_t index)
 	e1000_rar_set(hw, addr, index);
 }
 
-static void
+static int
 eth_em_default_mac_addr_set(struct rte_eth_dev *dev,
 			    struct ether_addr *addr)
 {
 	eth_em_rar_clear(dev, 0);
 
-	eth_em_rar_set(dev, (void *)addr, 0, 0);
+	return eth_em_rar_set(dev, (void *)addr, 0, 0);
 }
 
 static int

drivers/net/e1000/igb_ethdev.c

@@ -145,7 +145,7 @@ static int eth_igb_rar_set(struct rte_eth_dev *dev,
 			   struct ether_addr *mac_addr,
 			   uint32_t index, uint32_t pool);
 static void eth_igb_rar_clear(struct rte_eth_dev *dev, uint32_t index);
-static void eth_igb_default_mac_addr_set(struct rte_eth_dev *dev,
+static int eth_igb_default_mac_addr_set(struct rte_eth_dev *dev,
 		struct ether_addr *addr);
 
 static void igbvf_intr_disable(struct e1000_hw *hw);
@@ -170,7 +170,7 @@ static int igbvf_vlan_filter_set(struct rte_eth_dev *dev,
 		uint16_t vlan_id, int on);
 static int igbvf_set_vfta(struct e1000_hw *hw, uint16_t vid, bool on);
 static void igbvf_set_vfta_all(struct rte_eth_dev *dev, bool on);
-static void igbvf_default_mac_addr_set(struct rte_eth_dev *dev,
+static int igbvf_default_mac_addr_set(struct rte_eth_dev *dev,
 		struct ether_addr *addr);
 static int igbvf_get_reg_length(struct rte_eth_dev *dev);
 static int igbvf_get_regs(struct rte_eth_dev *dev,
@@ -3087,13 +3087,14 @@ eth_igb_rar_clear(struct rte_eth_dev *dev, uint32_t index)
 	e1000_rar_set(hw, addr, index);
 }
 
-static void
+static int
 eth_igb_default_mac_addr_set(struct rte_eth_dev *dev,
 				struct ether_addr *addr)
 {
 	eth_igb_rar_clear(dev, 0);
-
 	eth_igb_rar_set(dev, (void *)addr, 0, 0);
+
+	return 0;
 }
 /*
  * Virtual Function operations
@@ -3445,14 +3446,15 @@ igbvf_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
 	return 0;
 }
 
-static void
+static int
 igbvf_default_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *addr)
 {
 	struct e1000_hw *hw =
 		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	/* index is not used by rar_set() */
 	hw->mac.ops.rar_set(hw, (void *)addr, 0);
+	return 0;
 }
 
 

drivers/net/failsafe/failsafe_ops.c

@@ -997,16 +997,27 @@ fs_mac_addr_add(struct rte_eth_dev *dev,
 	return 0;
 }
 
-static void
+static int
 fs_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 {
 	struct sub_device *sdev;
 	uint8_t i;
+	int ret;
 
 	fs_lock(dev, 0);
-	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
-		rte_eth_dev_default_mac_addr_set(PORT_ID(sdev), mac_addr);
+	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
+		ret = rte_eth_dev_default_mac_addr_set(PORT_ID(sdev), mac_addr);
+		ret = fs_err(sdev, ret);
+		if (ret) {
+			ERROR("Operation rte_eth_dev_mac_addr_set failed for sub_device %d with error %d",
+				i, ret);
+			fs_unlock(dev, 0);
+			return ret;
+		}
+	}
 	fs_unlock(dev, 0);
+
+	return 0;
 }
 
 static int

drivers/net/i40e/i40e_ethdev.c

@@ -369,7 +369,7 @@ static int i40e_get_eeprom_length(struct rte_eth_dev *dev);
 static int i40e_get_eeprom(struct rte_eth_dev *dev,
 			   struct rte_dev_eeprom_info *eeprom);
 
-static void i40e_set_default_mac_addr(struct rte_eth_dev *dev,
+static int i40e_set_default_mac_addr(struct rte_eth_dev *dev,
 				      struct ether_addr *mac_addr);
 
 static int i40e_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
@@ -11327,8 +11327,8 @@ static int i40e_get_eeprom(struct rte_eth_dev *dev,
 	return 0;
 }
 
-static void i40e_set_default_mac_addr(struct rte_eth_dev *dev,
-				      struct ether_addr *mac_addr)
+static int i40e_set_default_mac_addr(struct rte_eth_dev *dev,
+				     struct ether_addr *mac_addr)
 {
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
@@ -11339,7 +11339,7 @@ static void i40e_set_default_mac_addr(struct rte_eth_dev *dev,
 
 	if (!is_valid_assigned_ether_addr(mac_addr)) {
 		PMD_DRV_LOG(ERR, "Tried to set invalid MAC address.");
-		return;
+		return -EINVAL;
 	}
 
 	TAILQ_FOREACH(f, &vsi->mac_list, next) {
@@ -11349,25 +11349,31 @@ static void i40e_set_default_mac_addr(struct rte_eth_dev *dev,
 
 	if (f == NULL) {
 		PMD_DRV_LOG(ERR, "Failed to find filter for default mac");
-		return;
+		return -EIO;
 	}
 
 	mac_filter = f->mac_info;
 	ret = i40e_vsi_delete_mac(vsi, &mac_filter.mac_addr);
 	if (ret != I40E_SUCCESS) {
 		PMD_DRV_LOG(ERR, "Failed to delete mac filter");
-		return;
+		return -EIO;
 	}
 	memcpy(&mac_filter.mac_addr, mac_addr, ETH_ADDR_LEN);
 	ret = i40e_vsi_add_mac(vsi, &mac_filter);
 	if (ret != I40E_SUCCESS) {
 		PMD_DRV_LOG(ERR, "Failed to add mac filter");
-		return;
+		return -EIO;
 	}
 	memcpy(&pf->dev_addr, mac_addr, ETH_ADDR_LEN);
 
-	i40e_aq_mac_address_write(hw, I40E_AQC_WRITE_TYPE_LAA_WOL,
-				  mac_addr->addr_bytes, NULL);
+	ret = i40e_aq_mac_address_write(hw, I40E_AQC_WRITE_TYPE_LAA_WOL,
+					mac_addr->addr_bytes, NULL);
+	if (ret != I40E_SUCCESS) {
+		PMD_DRV_LOG(ERR, "Failed to change mac");
+		return -EIO;
+	}
+
+	return 0;
 }
 
 static int