DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] ethdev: return diagnostic when setting MAC address
@ 2018-02-27 15:11 Olivier Matz
  2018-03-05 10:29 ` Adrien Mazarguil
                   ` (4 more replies)
  0 siblings, 5 replies; 21+ messages in thread
From: Olivier Matz @ 2018-02-27 15:11 UTC (permalink / raw)
  To: dev; +Cc: Thomas Monjalon, Ferruh Yigit

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>
---

Hi,

This patch is the following of the discussion we had in this thread:
https://dpdk.org/dev/patchwork/patch/32284/

I did my best to keep the consistency inside the PMDs. The behavior
of eth_mac_addr_set() is inspired from other fonctions in the same
PMD, usually eth_mac_addr_add(). For instance:
- dpaa and dpaa2 return 0 on error.
- some PMDs (bnxt, mlx5, ...?) do not return a -errno code (-1 or
  positive values).
- some PMDs (avf, tap) check if the address is the same and return 0
  in that case. This could go in generic code?

I tried to use the following errors when relevant:
- -EPERM when a VF is not allowed to do a change
- -ENOTSUP if the function is not supported
- -EIO if this is an unknown error from lower layer (hw or sdk)
- -EINVAL for other unknown errors

Please, PMD maintainers, feel free to comment if you ahve specific
needs for your driver.

Thanks
Olivier


 doc/guides/rel_notes/deprecation.rst    |  8 --------
 drivers/net/ark/ark_ethdev.c            |  9 ++++++---
 drivers/net/avf/avf_ethdev.c            | 12 ++++++++----
 drivers/net/bnxt/bnxt_ethdev.c          | 10 ++++++----
 drivers/net/bonding/rte_eth_bond_pmd.c  |  8 ++++++--
 drivers/net/dpaa/dpaa_ethdev.c          |  4 +++-
 drivers/net/dpaa2/dpaa2_ethdev.c        |  6 ++++--
 drivers/net/e1000/igb_ethdev.c          | 12 +++++++-----
 drivers/net/failsafe/failsafe_ops.c     | 16 +++++++++++++---
 drivers/net/i40e/i40e_ethdev.c          | 24 ++++++++++++++---------
 drivers/net/i40e/i40e_ethdev_vf.c       | 12 +++++++-----
 drivers/net/ixgbe/ixgbe_ethdev.c        | 13 ++++++++-----
 drivers/net/mlx4/mlx4.h                 |  2 +-
 drivers/net/mlx4/mlx4_ethdev.c          |  7 +++++--
 drivers/net/mlx5/mlx5.h                 |  2 +-
 drivers/net/mlx5/mlx5_mac.c             |  7 +++++--
 drivers/net/mrvl/mrvl_ethdev.c          |  7 ++++++-
 drivers/net/null/rte_eth_null.c         |  3 ++-
 drivers/net/octeontx/octeontx_ethdev.c  |  4 +++-
 drivers/net/qede/qede_ethdev.c          |  7 +++----
 drivers/net/sfc/sfc_ethdev.c            | 14 +++++---------
 drivers/net/szedata2/rte_eth_szedata2.c |  3 ++-
 drivers/net/tap/rte_eth_tap.c           | 34 +++++++++++++++++++++------------
 drivers/net/virtio/virtio_ethdev.c      | 15 ++++++++++-----
 drivers/net/vmxnet3/vmxnet3_ethdev.c    |  5 +++--
 lib/librte_ether/rte_ethdev.c           |  7 +++++--
 lib/librte_ether/rte_ethdev_core.h      |  2 +-
 test/test/virtual_pmd.c                 |  3 ++-
 28 files changed, 159 insertions(+), 97 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 74c18ed7c..2bf360f0d 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -134,14 +134,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.
-
 * ethdev: functions add rx/tx callback will return named opaque type
   ``rte_eth_add_rx_callback()``, ``rte_eth_add_first_rx_callback()`` and
   ``rte_eth_add_tx_callback()`` functions currently return callback object as
diff --git a/drivers/net/ark/ark_ethdev.c b/drivers/net/ark/ark_ethdev.c
index ff87c20e2..3fc40cd74 100644
--- a/drivers/net/ark/ark_ethdev.c
+++ b/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,
@@ -887,16 +887,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
diff --git a/drivers/net/avf/avf_ethdev.c b/drivers/net/avf/avf_ethdev.c
index 4df661705..0866f0bed 100644
--- a/drivers/net/avf/avf_ethdev.c
+++ b/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);
@@ -926,7 +926,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)
 {
@@ -940,11 +940,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)
@@ -968,7 +968,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
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 21c46f833..8eb923ebb 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1398,7 +1398,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;
@@ -1408,7 +1408,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));
 
@@ -1418,7 +1418,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;
@@ -1427,10 +1427,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
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index c34c3251f..979d8efcd 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2851,11 +2851,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 = {
diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index 9b69ef456..8227446e4 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -813,7 +813,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)
 {
@@ -825,6 +825,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 0;
 }
 
 static struct eth_dev_ops dpaa_devops = {
diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index 09a11d65a..2d092e72c 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -1074,7 +1074,7 @@ dpaa2_dev_remove_mac_addr(struct rte_eth_dev *dev,
 			"error: Removing the MAC ADDR failed: err = %d\n", ret);
 }
 
-static void
+static int
 dpaa2_dev_set_mac_addr(struct rte_eth_dev *dev,
 		       struct ether_addr *addr)
 {
@@ -1086,7 +1086,7 @@ dpaa2_dev_set_mac_addr(struct rte_eth_dev *dev,
 
 	if (dpni == NULL) {
 		RTE_LOG(ERR, PMD, "dpni is NULL\n");
-		return;
+		return -EINVAL;
 	}
 
 	ret = dpni_set_primary_mac_addr(dpni, CMD_PRI_LOW,
@@ -1095,6 +1095,8 @@ dpaa2_dev_set_mac_addr(struct rte_eth_dev *dev,
 	if (ret)
 		RTE_LOG(ERR, PMD,
 			"error: Setting the MAC ADDR failed %d\n", ret);
+
+	return 0;
 }
 static
 int dpaa2_dev_stats_get(struct rte_eth_dev *dev,
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 3c5138dea..e5be0d4a8 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -146,7 +146,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);
@@ -171,7 +171,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,
@@ -3146,13 +3146,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
@@ -3504,7 +3505,7 @@ 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 =
@@ -3512,6 +3513,7 @@ igbvf_default_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *addr)
 
 	/* index is not used by rar_set() */
 	hw->mac.ops.rar_set(hw, (void *)addr, 0);
+	return 0;
 }
 
 
diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
index 057e435cf..d2f302ffb 100644
--- a/drivers/net/failsafe/failsafe_ops.c
+++ b/drivers/net/failsafe/failsafe_ops.c
@@ -997,16 +997,26 @@ 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);
+		if ((ret = fs_err(sdev, ret))) {
+			ERROR("Operation rte_eth_dev_mac_addr_set failed for sub_device %"
+			      PRIu8 " with error %d", i, ret);
+			fs_unlock(dev, 0);
+			return ret;
+		}
+	}
 	fs_unlock(dev, 0);
+
+	return 0;
 }
 
 static int
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 508b4171c..455b1d0be 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/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);
@@ -11249,8 +11249,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);
@@ -11261,7 +11261,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) {
@@ -11271,25 +11271,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
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index fd003fe01..df9709f80 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -120,7 +120,7 @@ static int i40evf_dev_rss_hash_update(struct rte_eth_dev *dev,
 static int i40evf_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 					struct rte_eth_rss_conf *rss_conf);
 static int i40evf_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu);
-static void i40evf_set_default_mac_addr(struct rte_eth_dev *dev,
+static int i40evf_set_default_mac_addr(struct rte_eth_dev *dev,
 					struct ether_addr *mac_addr);
 static int
 i40evf_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id);
@@ -2658,7 +2658,7 @@ i40evf_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
 	return ret;
 }
 
-static void
+static int
 i40evf_set_default_mac_addr(struct rte_eth_dev *dev,
 			    struct ether_addr *mac_addr)
 {
@@ -2667,15 +2667,17 @@ i40evf_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;
 	}
 
 	if (vf->flags & I40E_FLAG_VF_MAC_BY_PF)
-		return;
+		return -EPERM;
 
 	i40evf_del_mac_addr_by_addr(dev, (struct ether_addr *)hw->mac.addr);
 
-	i40evf_add_mac_addr(dev, mac_addr, 0, 0);
+	if (i40evf_add_mac_addr(dev, mac_addr, 0, 0) != 0)
+		return -EIO;
 
 	ether_addr_copy(mac_addr, (struct ether_addr *)hw->mac.addr);
+	return 0;
 }
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 448325857..4167b57e9 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -228,7 +228,7 @@ static void ixgbe_dev_interrupt_delayed_handler(void *param);
 static int ixgbe_add_rar(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 			 uint32_t index, uint32_t pool);
 static void ixgbe_remove_rar(struct rte_eth_dev *dev, uint32_t index);
-static void ixgbe_set_default_mac_addr(struct rte_eth_dev *dev,
+static int ixgbe_set_default_mac_addr(struct rte_eth_dev *dev,
 					   struct ether_addr *mac_addr);
 static void ixgbe_dcb_init(struct ixgbe_hw *hw, struct ixgbe_dcb_config *dcb_config);
 static bool is_device_supported(struct rte_eth_dev *dev,
@@ -286,7 +286,7 @@ static int ixgbevf_add_mac_addr(struct rte_eth_dev *dev,
 				struct ether_addr *mac_addr,
 				uint32_t index, uint32_t pool);
 static void ixgbevf_remove_mac_addr(struct rte_eth_dev *dev, uint32_t index);
-static void ixgbevf_set_default_mac_addr(struct rte_eth_dev *dev,
+static int ixgbevf_set_default_mac_addr(struct rte_eth_dev *dev,
 					     struct ether_addr *mac_addr);
 static int ixgbe_syn_filter_get(struct rte_eth_dev *dev,
 			struct rte_eth_syn_filter *filter);
@@ -4853,14 +4853,15 @@ ixgbe_remove_rar(struct rte_eth_dev *dev, uint32_t index)
 	ixgbe_clear_rar(hw, index);
 }
 
-static void
+static int
 ixgbe_set_default_mac_addr(struct rte_eth_dev *dev, struct ether_addr *addr)
 {
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 
 	ixgbe_remove_rar(dev, 0);
-
 	ixgbe_add_rar(dev, addr, 0, pci_dev->max_vfs);
+
+	return 0;
 }
 
 static bool
@@ -5983,12 +5984,14 @@ ixgbevf_remove_mac_addr(struct rte_eth_dev *dev, uint32_t index)
 	}
 }
 
-static void
+static int
 ixgbevf_set_default_mac_addr(struct rte_eth_dev *dev, struct ether_addr *addr)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	hw->mac.ops.set_rar(hw, 0, (void *)addr, 0, 0);
+
+	return 0;
 }
 
 int
diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h
index 19c8a223d..c107794ce 100644
--- a/drivers/net/mlx4/mlx4.h
+++ b/drivers/net/mlx4/mlx4.h
@@ -131,7 +131,7 @@ void mlx4_allmulticast_disable(struct rte_eth_dev *dev);
 void mlx4_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index);
 int mlx4_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		      uint32_t index, uint32_t vmdq);
-void mlx4_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr);
+int mlx4_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr);
 int mlx4_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on);
 int mlx4_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats);
 void mlx4_stats_reset(struct rte_eth_dev *dev);
diff --git a/drivers/net/mlx4/mlx4_ethdev.c b/drivers/net/mlx4/mlx4_ethdev.c
index 3bc692731..2442e16a6 100644
--- a/drivers/net/mlx4/mlx4_ethdev.c
+++ b/drivers/net/mlx4/mlx4_ethdev.c
@@ -701,11 +701,14 @@ mlx4_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
  *   Pointer to Ethernet device structure.
  * @param mac_addr
  *   MAC address to register.
+ *
+ * @return
+ *   0 on success, negative errno value otherwise and rte_errno is set.
  */
-void
+int
 mlx4_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 {
-	mlx4_mac_addr_add(dev, mac_addr, 0, 0);
+	return mlx4_mac_addr_add(dev, mac_addr, 0, 0);
 }
 
 /**
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 965c19f21..42e58d7f7 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -241,7 +241,7 @@ int priv_get_mac(struct priv *, uint8_t (*)[ETHER_ADDR_LEN]);
 void mlx5_mac_addr_remove(struct rte_eth_dev *, uint32_t);
 int mlx5_mac_addr_add(struct rte_eth_dev *, struct ether_addr *, uint32_t,
 		      uint32_t);
-void mlx5_mac_addr_set(struct rte_eth_dev *, struct ether_addr *);
+int mlx5_mac_addr_set(struct rte_eth_dev *, struct ether_addr *);
 
 /* mlx5_rss.c */
 
diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
index e8a8d4594..0dc4bec46 100644
--- a/drivers/net/mlx5/mlx5_mac.c
+++ b/drivers/net/mlx5/mlx5_mac.c
@@ -118,10 +118,13 @@ mlx5_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac,
  *   Pointer to Ethernet device structure.
  * @param mac_addr
  *   MAC address to register.
+ *
+ * @return
+ *   0 on success.
  */
-void
+int
 mlx5_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 {
 	DEBUG("%p: setting primary MAC address", (void *)dev);
-	mlx5_mac_addr_add(dev, mac_addr, 0, 0);
+	return mlx5_mac_addr_add(dev, mac_addr, 0, 0);
 }
diff --git a/drivers/net/mrvl/mrvl_ethdev.c b/drivers/net/mrvl/mrvl_ethdev.c
index 705c4bd8b..8e14e54e2 100644
--- a/drivers/net/mrvl/mrvl_ethdev.c
+++ b/drivers/net/mrvl/mrvl_ethdev.c
@@ -952,6 +952,9 @@ mrvl_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
  *   Pointer to Ethernet device structure.
  * @param mac_addr
  *   MAC address to register.
+ *
+ * @return
+ *   0 on success, negative error value otherwise.
  */
 static void
 mrvl_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
@@ -960,7 +963,7 @@ mrvl_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 	int ret;
 
 	if (!priv->ppio)
-		return;
+		return -EPERM;
 
 	ret = pp2_ppio_set_mac_addr(priv->ppio, mac_addr->addr_bytes);
 	if (ret) {
@@ -968,6 +971,8 @@ mrvl_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 		ether_format_addr(buf, sizeof(buf), mac_addr);
 		RTE_LOG(ERR, PMD, "Failed to set mac to %s\n", buf);
 	}
+
+	return ret;
 }
 
 /**
diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
index d003b2839..3b659cd23 100644
--- a/drivers/net/null/rte_eth_null.c
+++ b/drivers/net/null/rte_eth_null.c
@@ -461,10 +461,11 @@ eth_rss_hash_conf_get(struct rte_eth_dev *dev,
 	return 0;
 }
 
-static void
+static int
 eth_mac_address_set(__rte_unused struct rte_eth_dev *dev,
 		    __rte_unused struct ether_addr *addr)
 {
+	return 0;
 }
 
 static const struct eth_dev_ops ops = {
diff --git a/drivers/net/octeontx/octeontx_ethdev.c b/drivers/net/octeontx/octeontx_ethdev.c
index b739c0b39..fabf86aa5 100644
--- a/drivers/net/octeontx/octeontx_ethdev.c
+++ b/drivers/net/octeontx/octeontx_ethdev.c
@@ -594,7 +594,7 @@ octeontx_dev_stats_reset(struct rte_eth_dev *dev)
 	octeontx_port_stats_clr(nic);
 }
 
-static void
+static int
 octeontx_dev_default_mac_addr_set(struct rte_eth_dev *dev,
 					struct ether_addr *addr)
 {
@@ -605,6 +605,8 @@ octeontx_dev_default_mac_addr_set(struct rte_eth_dev *dev,
 	if (ret != 0)
 		octeontx_log_err("failed to set MAC address on port %d",
 				nic->port_id);
+
+	return ret;
 }
 
 static void
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index a91f43683..e4cd89511 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -1001,7 +1001,7 @@ qede_mac_addr_remove(struct rte_eth_dev *eth_dev, uint32_t index)
 	ecore_filter_ucast_cmd(edev, &ucast, ECORE_SPQ_MODE_CB, NULL);
 }
 
-static void
+static int
 qede_mac_addr_set(struct rte_eth_dev *eth_dev, struct ether_addr *mac_addr)
 {
 	struct qede_dev *qdev = QEDE_INIT_QDEV(eth_dev);
@@ -1010,12 +1010,11 @@ qede_mac_addr_set(struct rte_eth_dev *eth_dev, struct ether_addr *mac_addr)
 	if (IS_VF(edev) && !ecore_vf_check_mac(ECORE_LEADING_HWFN(edev),
 					       mac_addr->addr_bytes)) {
 		DP_ERR(edev, "Setting MAC address is not allowed\n");
-		ether_addr_copy(&qdev->primary_mac,
-				&eth_dev->data->mac_addrs[0]);
-		return;
+		return -EPERM;
 	}
 
 	qede_mac_addr_add(eth_dev, mac_addr, 0, 0);
+	return 0;
 }
 
 static void qede_config_accept_any_vlan(struct qede_dev *qdev, bool flg)
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 89a452907..b76d4bb2c 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -920,7 +920,7 @@ sfc_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu)
 	SFC_ASSERT(rc > 0);
 	return -rc;
 }
-static void
+static int
 sfc_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 {
 	struct sfc_adapter *sa = dev->data->dev_private;
@@ -939,13 +939,14 @@ sfc_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 	if (port->isolated) {
 		sfc_err(sa, "isolated mode is active on the port");
 		sfc_err(sa, "will not set MAC address");
+		rc = ENOTSUP;
 		goto unlock;
 	}
 
 	if (sa->state != SFC_ADAPTER_STARTED) {
 		sfc_info(sa, "the port is not started");
 		sfc_info(sa, "the new MAC address will be set on port start");
-
+		rc = EINVAL;
 		goto unlock;
 	}
 
@@ -982,14 +983,9 @@ 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);
+	SFC_ASSERT(rc >= 0);
+	return -rc;
 }
 
 
diff --git a/drivers/net/szedata2/rte_eth_szedata2.c b/drivers/net/szedata2/rte_eth_szedata2.c
index e53c738db..1a1e2356b 100644
--- a/drivers/net/szedata2/rte_eth_szedata2.c
+++ b/drivers/net/szedata2/rte_eth_szedata2.c
@@ -1315,10 +1315,11 @@ eth_tx_queue_setup(struct rte_eth_dev *dev,
 	return 0;
 }
 
-static void
+static int
 eth_mac_addr_set(struct rte_eth_dev *dev __rte_unused,
 		struct ether_addr *mac_addr __rte_unused)
 {
+	return 0;
 }
 
 static void
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index f09db0ea9..ac99b233c 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -929,48 +929,58 @@ tap_allmulti_disable(struct rte_eth_dev *dev)
 		tap_flow_implicit_destroy(pmd, TAP_REMOTE_ALLMULTI);
 }
 
-static void
+static int
 tap_mac_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 {
 	struct pmd_internals *pmd = dev->data->dev_private;
 	enum ioctl_mode mode = LOCAL_ONLY;
 	struct ifreq ifr;
+	int ret;
 
 	if (is_zero_ether_addr(mac_addr)) {
 		RTE_LOG(ERR, PMD, "%s: can't set an empty MAC address\n",
 			dev->device->name);
-		return;
+		return -EINVAL;
 	}
 	/* Check the actual current MAC address on the tap netdevice */
-	if (tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0)
-		return;
+	ret = tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, LOCAL_ONLY);
+	if (ret < 0)
+		return ret;
 	if (is_same_ether_addr((struct ether_addr *)&ifr.ifr_hwaddr.sa_data,
 			       mac_addr))
-		return;
+		return 0;
 	/* Check the current MAC address on the remote */
-	if (tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, REMOTE_ONLY) < 0)
-		return;
+	ret = tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, REMOTE_ONLY);
+	if (ret < 0)
+		return ret;
 	if (!is_same_ether_addr((struct ether_addr *)&ifr.ifr_hwaddr.sa_data,
 			       mac_addr))
 		mode = LOCAL_AND_REMOTE;
 	ifr.ifr_hwaddr.sa_family = AF_LOCAL;
 	rte_memcpy(ifr.ifr_hwaddr.sa_data, mac_addr, ETHER_ADDR_LEN);
-	if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 1, mode) < 0)
-		return;
+	ret = tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 1, mode);
+	if (ret < 0)
+		return ret;
 	rte_memcpy(&pmd->eth_addr, mac_addr, ETHER_ADDR_LEN);
 	if (pmd->remote_if_index && !pmd->flow_isolate) {
 		/* Replace MAC redirection rule after a MAC change */
-		if (tap_flow_implicit_destroy(pmd, TAP_REMOTE_LOCAL_MAC) < 0) {
+		ret = tap_flow_implicit_destroy(pmd, TAP_REMOTE_LOCAL_MAC);
+		if (ret < 0) {
 			RTE_LOG(ERR, PMD,
 				"%s: Couldn't delete MAC redirection rule\n",
 				dev->device->name);
-			return;
+			return ret;
 		}
-		if (tap_flow_implicit_create(pmd, TAP_REMOTE_LOCAL_MAC) < 0)
+		ret = tap_flow_implicit_create(pmd, TAP_REMOTE_LOCAL_MAC);
+		if (ret < 0) {
 			RTE_LOG(ERR, PMD,
 				"%s: Couldn't add MAC redirection rule\n",
 				dev->device->name);
+			return ret;
+		}
 	}
+
+	return 0;
 }
 
 static int
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 884f74ad0..38a7a14f7 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -68,7 +68,7 @@ static int virtio_mac_addr_add(struct rte_eth_dev *dev,
 				struct ether_addr *mac_addr,
 				uint32_t index, uint32_t vmdq);
 static void virtio_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index);
-static void virtio_mac_addr_set(struct rte_eth_dev *dev,
+static int virtio_mac_addr_set(struct rte_eth_dev *dev,
 				struct ether_addr *mac_addr);
 
 static int virtio_intr_enable(struct rte_eth_dev *dev);
@@ -1097,7 +1097,7 @@ virtio_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
 	virtio_mac_table_set(hw, uc, mc);
 }
 
-static void
+static int
 virtio_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 {
 	struct virtio_hw *hw = dev->data->dev_private;
@@ -1113,9 +1113,14 @@ virtio_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 		ctrl.hdr.cmd = VIRTIO_NET_CTRL_MAC_ADDR_SET;
 
 		memcpy(ctrl.data, mac_addr, ETHER_ADDR_LEN);
-		virtio_send_command(hw->cvq, &ctrl, &len, 1);
-	} else if (vtpci_with_feature(hw, VIRTIO_NET_F_MAC))
-		virtio_set_hwaddr(hw);
+		return virtio_send_command(hw->cvq, &ctrl, &len, 1);
+	}
+
+	if (!vtpci_with_feature(hw, VIRTIO_NET_F_MAC))
+		return -ENOTSUP;
+
+	virtio_set_hwaddr(hw);
+	return 0;
 }
 
 static int
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 4e68aae6b..13ee8f250 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -73,7 +73,7 @@ vmxnet3_dev_supported_ptypes_get(struct rte_eth_dev *dev);
 static int vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev,
 				       uint16_t vid, int on);
 static int vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask);
-static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev,
+static int vmxnet3_mac_addr_set(struct rte_eth_dev *dev,
 				 struct ether_addr *mac_addr);
 static void vmxnet3_interrupt_handler(void *param);
 
@@ -1117,13 +1117,14 @@ vmxnet3_dev_supported_ptypes_get(struct rte_eth_dev *dev)
 	return NULL;
 }
 
-static void
+static int
 vmxnet3_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 {
 	struct vmxnet3_hw *hw = dev->data->dev_private;
 
 	ether_addr_copy(mac_addr, (struct ether_addr *)(hw->perm_addr));
 	vmxnet3_write_mac(hw, mac_addr->addr_bytes);
+	return 0;
 }
 
 /* return 0 means link status changed, -1 means not changed */
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 0590f0c10..6a7330d92 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3003,6 +3003,7 @@ int
 rte_eth_dev_default_mac_addr_set(uint16_t port_id, struct ether_addr *addr)
 {
 	struct rte_eth_dev *dev;
+	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 
@@ -3012,11 +3013,13 @@ rte_eth_dev_default_mac_addr_set(uint16_t port_id, struct ether_addr *addr)
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mac_addr_set, -ENOTSUP);
 
+	ret = (*dev->dev_ops->mac_addr_set)(dev, addr);
+	if (ret < 0)
+		return ret;
+
 	/* Update default address in NIC data structure */
 	ether_addr_copy(addr, &dev->data->mac_addrs[0]);
 
-	(*dev->dev_ops->mac_addr_set)(dev, addr);
-
 	return 0;
 }
 
diff --git a/lib/librte_ether/rte_ethdev_core.h b/lib/librte_ether/rte_ethdev_core.h
index e5681e466..55eb2b09e 100644
--- a/lib/librte_ether/rte_ethdev_core.h
+++ b/lib/librte_ether/rte_ethdev_core.h
@@ -255,7 +255,7 @@ typedef int (*eth_mac_addr_add_t)(struct rte_eth_dev *dev,
 				  uint32_t vmdq);
 /**< @internal Set a MAC address into Receive Address Address Register */
 
-typedef void (*eth_mac_addr_set_t)(struct rte_eth_dev *dev,
+typedef int (*eth_mac_addr_set_t)(struct rte_eth_dev *dev,
 				  struct ether_addr *mac_addr);
 /**< @internal Set a MAC address into Receive Address Address Register */
 
diff --git a/test/test/virtual_pmd.c b/test/test/virtual_pmd.c
index 2f5b31dba..69b4ba034 100644
--- a/test/test/virtual_pmd.c
+++ b/test/test/virtual_pmd.c
@@ -216,10 +216,11 @@ static void
 virtual_ethdev_promiscuous_mode_disable(struct rte_eth_dev *dev __rte_unused)
 {}
 
-static void
+static int
 virtual_ethdev_mac_address_set(__rte_unused struct rte_eth_dev *dev,
 			       __rte_unused struct ether_addr *addr)
 {
+	return 0;
 }
 
 static const struct eth_dev_ops virtual_ethdev_default_dev_ops = {
-- 
2.11.0

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2018-04-11 19:30 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-27 15:11 [dpdk-dev] [PATCH] ethdev: return diagnostic when setting MAC address Olivier Matz
2018-03-05 10:29 ` Adrien Mazarguil
2018-03-06  9:37 ` Tomasz Duszynski
2018-03-16 15:41 ` Andrew Rybchenko
2018-03-26 18:39 ` Ferruh Yigit
2018-03-28  8:24   ` Olivier Matz
2018-04-03 12:41 ` [dpdk-dev] [PATCH v2] " Olivier Matz
2018-04-03 13:02   ` Andrew Rybchenko
2018-04-03 14:57   ` Adrien Mazarguil
2018-04-03 16:26     ` Olivier Matz
2018-04-03 16:19   ` Ferruh Yigit
2018-04-03 16:25     ` Olivier Matz
2018-04-04  8:19   ` Shreyansh Jain
2018-04-06 15:21   ` [dpdk-dev] [PATCH] " Olivier Matz
2018-04-06 15:34     ` Olivier Matz
2018-04-09  8:57     ` Nélio Laranjeiro
2018-04-06 15:34   ` [dpdk-dev] [PATCH v3] " Olivier Matz
2018-04-06 16:03     ` Ferruh Yigit
2018-04-10 13:19       ` Thomas Monjalon
2018-04-11 16:32     ` [dpdk-dev] [PATCH v4] " Olivier Matz
2018-04-11 19:30       ` Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).