patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH v5 1/3] ethdev: fix adding invalid MAC addr
       [not found] <1493525507-56304-1-git-send-email-wei.dai@intel.com>
@ 2017-04-30  4:11 ` Wei Dai
       [not found] ` <1493729065-17090-1-git-send-email-wei.dai@intel.com>
  1 sibling, 0 replies; 6+ messages in thread
From: Wei Dai @ 2017-04-30  4:11 UTC (permalink / raw)
  To: wenzhuo.lu, thomas, harish.patil, rasesh.mody, stephen.hurd,
	ajit.khaparde, helin.zhang, konstantin.ananyev, jingjing.wu,
	jing.d.chen, adrien.mazarguil, nelio.laranjeiro,
	bruce.richardson, yuanhan.liu, maxime.coquelin, shepard.siegel,
	ed.czeck, john.miller
  Cc: dev, Wei Dai, stable

Some customers find adding MAC addr to VF sometimes can fail,
but it is still stored in dev->data->mac_addrs[ ]. So this
can lead to some errors that assumes the non-zero entry in
dev->data->mac_addrs[ ] is valid.
Following acknowledgements are from specific NIC PMD
maintainer for their managing part.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 drivers/net/ark/ark_ethdev.c       | 15 +++++++++------
 drivers/net/bnx2x/bnx2x_ethdev.c   |  7 +++++--
 drivers/net/bnxt/bnxt_ethdev.c     | 16 ++++++++--------
 drivers/net/e1000/base/e1000_api.c |  2 +-
 drivers/net/e1000/em_ethdev.c      |  8 ++++----
 drivers/net/e1000/igb_ethdev.c     |  9 +++++----
 drivers/net/enic/enic.h            |  2 +-
 drivers/net/enic/enic_ethdev.c     |  4 ++--
 drivers/net/enic/enic_main.c       |  9 ++++-----
 drivers/net/fm10k/fm10k_ethdev.c   |  3 ++-
 drivers/net/i40e/i40e_ethdev.c     | 17 +++++++++--------
 drivers/net/i40e/i40e_ethdev_vf.c  | 14 +++++++-------
 drivers/net/ixgbe/ixgbe_ethdev.c   | 33 +++++++++++++++++++++------------
 drivers/net/mlx4/mlx4.c            | 16 ++++++++++------
 drivers/net/mlx5/mlx5.h            |  4 ++--
 drivers/net/mlx5/mlx5_mac.c        | 16 ++++++++++------
 drivers/net/qede/qede_ethdev.c     |  6 ++++--
 drivers/net/ring/rte_eth_ring.c    |  3 ++-
 drivers/net/virtio/virtio_ethdev.c | 13 +++++++------
 lib/librte_ether/rte_ethdev.c      | 15 +++++++++------
 lib/librte_ether/rte_ethdev.h      |  2 +-
 21 files changed, 123 insertions(+), 91 deletions(-)

diff --git a/drivers/net/ark/ark_ethdev.c b/drivers/net/ark/ark_ethdev.c
index 4caad98..0c3fa42 100644
--- a/drivers/net/ark/ark_ethdev.c
+++ b/drivers/net/ark/ark_ethdev.c
@@ -71,10 +71,10 @@ static void eth_ark_dev_stats_get(struct rte_eth_dev *dev,
 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,
 					 struct ether_addr *mac_addr);
-static void eth_ark_macaddr_add(struct rte_eth_dev *dev,
-				struct ether_addr *mac_addr,
-				uint32_t index,
-				uint32_t pool);
+static int eth_ark_macaddr_add(struct rte_eth_dev *dev,
+			       struct ether_addr *mac_addr,
+			       uint32_t index,
+			       uint32_t pool);
 static void eth_ark_macaddr_remove(struct rte_eth_dev *dev,
 				   uint32_t index);
 
@@ -831,7 +831,7 @@ eth_ark_dev_stats_reset(struct rte_eth_dev *dev)
 		ark->user_ext.stats_reset(dev, ark->user_data);
 }
 
-static void
+static int
 eth_ark_macaddr_add(struct rte_eth_dev *dev,
 		    struct ether_addr *mac_addr,
 		    uint32_t index,
@@ -840,12 +840,15 @@ eth_ark_macaddr_add(struct rte_eth_dev *dev,
 	struct ark_adapter *ark =
 		(struct ark_adapter *)dev->data->dev_private;
 
-	if (ark->user_ext.mac_addr_add)
+	if (ark->user_ext.mac_addr_add) {
 		ark->user_ext.mac_addr_add(dev,
 					   mac_addr,
 					   index,
 					   pool,
 					   ark->user_data);
+		return 0;
+	}
+	return -ENOTSUP;
 }
 
 static void
diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index 314e5ea..b79cfdb 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -451,14 +451,17 @@ bnx2x_dev_infos_get(struct rte_eth_dev *dev, __rte_unused struct rte_eth_dev_inf
 	dev_info->speed_capa = ETH_LINK_SPEED_10G | ETH_LINK_SPEED_20G;
 }
 
-static void
+static int
 bnx2x_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		uint32_t index, uint32_t pool)
 {
 	struct bnx2x_softc *sc = dev->data->dev_private;
 
-	if (sc->mac_ops.mac_addr_add)
+	if (sc->mac_ops.mac_addr_add) {
 		sc->mac_ops.mac_addr_add(dev, mac_addr, index, pool);
+		return 0;
+	}
+	return -ENOTSUP;
 }
 
 static void
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 5dc3ff0..c18555f 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -614,9 +614,9 @@ static void bnxt_mac_addr_remove_op(struct rte_eth_dev *eth_dev,
 	}
 }
 
-static void bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
-				 struct ether_addr *mac_addr,
-				 uint32_t index, uint32_t pool)
+static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
+				struct ether_addr *mac_addr,
+				uint32_t index, uint32_t pool)
 {
 	struct bnxt *bp = (struct bnxt *)eth_dev->data->dev_private;
 	struct bnxt_vnic_info *vnic = STAILQ_FIRST(&bp->ff_pool[pool]);
@@ -624,30 +624,30 @@ static void bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 
 	if (BNXT_VF(bp)) {
 		RTE_LOG(ERR, PMD, "Cannot add MAC address to a VF interface\n");
-		return;
+		return -ENOTSUP;
 	}
 
 	if (!vnic) {
 		RTE_LOG(ERR, PMD, "VNIC not found for pool %d!\n", pool);
-		return;
+		return -EINVAL;
 	}
 	/* Attach requested MAC address to the new l2_filter */
 	STAILQ_FOREACH(filter, &vnic->filter, next) {
 		if (filter->mac_index == index) {
 			RTE_LOG(ERR, PMD,
 				"MAC addr already existed for pool %d\n", pool);
-			return;
+			return -EINVAL;
 		}
 	}
 	filter = bnxt_alloc_filter(bp);
 	if (!filter) {
 		RTE_LOG(ERR, PMD, "L2 filter alloc failed\n");
-		return;
+		return -ENODEV;
 	}
 	STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
 	filter->mac_index = index;
 	memcpy(filter->l2_addr, mac_addr, ETHER_ADDR_LEN);
-	bnxt_hwrm_set_filter(bp, vnic, filter);
+	return bnxt_hwrm_set_filter(bp, vnic, filter);
 }
 
 int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
diff --git a/drivers/net/e1000/base/e1000_api.c b/drivers/net/e1000/base/e1000_api.c
index f7cf83b..dcb53f7 100644
--- a/drivers/net/e1000/base/e1000_api.c
+++ b/drivers/net/e1000/base/e1000_api.c
@@ -855,7 +855,7 @@ int e1000_rar_set(struct e1000_hw *hw, u8 *addr, u32 index)
 	if (hw->mac.ops.rar_set)
 		return hw->mac.ops.rar_set(hw, addr, index);
 
-	return E1000_SUCCESS;
+	return E1000_ERR_NO_SPACE;
 }
 
 /**
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 599b9e0..57eb017 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -120,8 +120,8 @@ static int eth_em_led_on(struct rte_eth_dev *dev);
 static int eth_em_led_off(struct rte_eth_dev *dev);
 
 static int em_get_rx_buffer_size(struct e1000_hw *hw);
-static void eth_em_rar_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
-		uint32_t index, uint32_t pool);
+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 int eth_em_set_mc_addr_list(struct rte_eth_dev *dev,
@@ -1794,13 +1794,13 @@ eth_em_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 	return -EIO;
 }
 
-static void
+static int
 eth_em_rar_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		uint32_t index, __rte_unused uint32_t pool)
 {
 	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-	e1000_rar_set(hw, mac_addr->addr_bytes, index);
+	return e1000_rar_set(hw, mac_addr->addr_bytes, index);
 }
 
 static void
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index ca9f98c..7574c26 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -169,9 +169,9 @@ static int eth_igb_led_off(struct rte_eth_dev *dev);
 
 static void igb_intr_disable(struct e1000_hw *hw);
 static int  igb_get_rx_buffer_size(struct e1000_hw *hw);
-static void eth_igb_rar_set(struct rte_eth_dev *dev,
-		struct ether_addr *mac_addr,
-		uint32_t index, uint32_t pool);
+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,
 		struct ether_addr *addr);
@@ -3077,7 +3077,7 @@ eth_igb_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 }
 
 #define E1000_RAH_POOLSEL_SHIFT      (18)
-static void
+static int
 eth_igb_rar_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 	        uint32_t index, __rte_unused uint32_t pool)
 {
@@ -3088,6 +3088,7 @@ eth_igb_rar_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 	rah = E1000_READ_REG(hw, E1000_RAH(index));
 	rah |= (0x1 << (E1000_RAH_POOLSEL_SHIFT + pool));
 	E1000_WRITE_REG(hw, E1000_RAH(index), rah);
+	return 0;
 }
 
 static void
diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index e921de4..d17a35f 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -279,7 +279,7 @@ extern void enic_dev_stats_get(struct enic *enic,
 	struct rte_eth_stats *r_stats);
 extern void enic_dev_stats_clear(struct enic *enic);
 extern void enic_add_packet_filter(struct enic *enic);
-void enic_set_mac_address(struct enic *enic, uint8_t *mac_addr);
+int enic_set_mac_address(struct enic *enic, uint8_t *mac_addr);
 void enic_del_mac_address(struct enic *enic, int mac_index);
 extern unsigned int enic_cleanup_wq(struct enic *enic, struct vnic_wq *wq);
 extern void enic_send_pkt(struct enic *enic, struct vnic_wq *wq,
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 7579696..372bae7 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -533,14 +533,14 @@ static void enicpmd_dev_allmulticast_disable(struct rte_eth_dev *eth_dev)
 	enic_add_packet_filter(enic);
 }
 
-static void enicpmd_add_mac_addr(struct rte_eth_dev *eth_dev,
+static int enicpmd_add_mac_addr(struct rte_eth_dev *eth_dev,
 	struct ether_addr *mac_addr,
 	__rte_unused uint32_t index, __rte_unused uint32_t pool)
 {
 	struct enic *enic = pmd_priv(eth_dev);
 
 	ENICPMD_FUNC_TRACE();
-	enic_set_mac_address(enic, mac_addr->addr_bytes);
+	return enic_set_mac_address(enic, mac_addr->addr_bytes);
 }
 
 static void enicpmd_remove_mac_addr(struct rte_eth_dev *eth_dev, uint32_t index)
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 5f2188b..d026241 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -202,20 +202,19 @@ void enic_del_mac_address(struct enic *enic, int mac_index)
 		dev_err(enic, "del mac addr failed\n");
 }
 
-void enic_set_mac_address(struct enic *enic, uint8_t *mac_addr)
+int enic_set_mac_address(struct enic *enic, uint8_t *mac_addr)
 {
 	int err;
 
 	if (!is_eth_addr_valid(mac_addr)) {
 		dev_err(enic, "invalid mac address\n");
-		return;
+		return -EINVAL;
 	}
 
 	err = vnic_dev_add_addr(enic->vdev, mac_addr);
-	if (err) {
+	if (err)
 		dev_err(enic, "add mac addr failed\n");
-		return;
-	}
+	return err;
 }
 
 static void
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 94b4d40..a742eec 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1690,7 +1690,7 @@ static void fm10k_MAC_filter_set(struct rte_eth_dev *dev,
 }
 
 /* Add a MAC address, and update filters */
-static void
+static int
 fm10k_macaddr_add(struct rte_eth_dev *dev,
 		struct ether_addr *mac_addr,
 		uint32_t index,
@@ -1701,6 +1701,7 @@ fm10k_macaddr_add(struct rte_eth_dev *dev,
 	macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private);
 	fm10k_MAC_filter_set(dev, mac_addr->addr_bytes, TRUE, pool);
 	macvlan->mac_vmdq_id[index] = pool;
+	return 0;
 }
 
 /* Remove a MAC address, and update filters */
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3fa25dc..986031d 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -291,10 +291,10 @@ static int i40e_flow_ctrl_set(struct rte_eth_dev *dev,
 			      struct rte_eth_fc_conf *fc_conf);
 static int i40e_priority_flow_ctrl_set(struct rte_eth_dev *dev,
 				       struct rte_eth_pfc_conf *pfc_conf);
-static void i40e_macaddr_add(struct rte_eth_dev *dev,
-			  struct ether_addr *mac_addr,
-			  uint32_t index,
-			  uint32_t pool);
+static int i40e_macaddr_add(struct rte_eth_dev *dev,
+			    struct ether_addr *mac_addr,
+			    uint32_t index,
+			    uint32_t pool);
 static void i40e_macaddr_remove(struct rte_eth_dev *dev, uint32_t index);
 static int i40e_dev_rss_reta_update(struct rte_eth_dev *dev,
 				    struct rte_eth_rss_reta_entry64 *reta_conf,
@@ -3267,7 +3267,7 @@ i40e_priority_flow_ctrl_set(__rte_unused struct rte_eth_dev *dev,
 }
 
 /* Add a MAC address, and update filters */
-static void
+static int
 i40e_macaddr_add(struct rte_eth_dev *dev,
 		 struct ether_addr *mac_addr,
 		 __rte_unused uint32_t index,
@@ -3284,13 +3284,13 @@ i40e_macaddr_add(struct rte_eth_dev *dev,
 		PMD_DRV_LOG(ERR, "VMDQ not %s, can't set mac to pool %u",
 			pf->flags & I40E_FLAG_VMDQ ? "configured" : "enabled",
 			pool);
-		return;
+		return -ENOTSUP;
 	}
 
 	if (pool > pf->nb_cfg_vmdq_vsi) {
 		PMD_DRV_LOG(ERR, "Pool number %u invalid. Max pool is %u",
 				pool, pf->nb_cfg_vmdq_vsi);
-		return;
+		return -EINVAL;
 	}
 
 	(void)rte_memcpy(&mac_filter.mac_addr, mac_addr, ETHER_ADDR_LEN);
@@ -3307,8 +3307,9 @@ i40e_macaddr_add(struct rte_eth_dev *dev,
 	ret = i40e_vsi_add_mac(vsi, &mac_filter);
 	if (ret != I40E_SUCCESS) {
 		PMD_DRV_LOG(ERR, "Failed to add MACVLAN filter");
-		return;
+		return -ENODEV;
 	}
+	return 0;
 }
 
 /* Remove a MAC address, and update filters */
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index cb34023..6c081f0 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -136,10 +136,10 @@ static int i40evf_dev_tx_queue_start(struct rte_eth_dev *dev,
 				     uint16_t tx_queue_id);
 static int i40evf_dev_tx_queue_stop(struct rte_eth_dev *dev,
 				    uint16_t tx_queue_id);
-static void i40evf_add_mac_addr(struct rte_eth_dev *dev,
-				struct ether_addr *addr,
-				uint32_t index,
-				uint32_t pool);
+static int i40evf_add_mac_addr(struct rte_eth_dev *dev,
+			       struct ether_addr *addr,
+			       uint32_t index,
+			       uint32_t pool);
 static void i40evf_del_mac_addr(struct rte_eth_dev *dev, uint32_t index);
 static int i40evf_dev_rss_reta_update(struct rte_eth_dev *dev,
 			struct rte_eth_rss_reta_entry64 *reta_conf,
@@ -855,7 +855,7 @@ i40evf_stop_queues(struct rte_eth_dev *dev)
 	return 0;
 }
 
-static void
+static int
 i40evf_add_mac_addr(struct rte_eth_dev *dev,
 		    struct ether_addr *addr,
 		    __rte_unused uint32_t index,
@@ -873,7 +873,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
 			    addr->addr_bytes[0], addr->addr_bytes[1],
 			    addr->addr_bytes[2], addr->addr_bytes[3],
 			    addr->addr_bytes[4], addr->addr_bytes[5]);
-		return;
+		return I40E_ERR_INVALID_MAC_ADDR;
 	}
 
 	list = (struct i40e_virtchnl_ether_addr_list *)cmd_buffer;
@@ -892,7 +892,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
 		PMD_DRV_LOG(ERR, "fail to execute command "
 			    "OP_ADD_ETHER_ADDRESS");
 
-	return;
+	return err;
 }
 
 static void
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 7b856bb..e134bcd 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -247,8 +247,8 @@ static int ixgbe_dev_interrupt_action(struct rte_eth_dev *dev,
 				      struct rte_intr_handle *handle);
 static void ixgbe_dev_interrupt_handler(void *param);
 static void ixgbe_dev_interrupt_delayed_handler(void *param);
-static void ixgbe_add_rar(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
-		uint32_t index, uint32_t pool);
+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,
 					   struct ether_addr *mac_addr);
@@ -304,9 +304,9 @@ static void ixgbe_configure_msix(struct rte_eth_dev *dev);
 static int ixgbe_set_queue_rate_limit(struct rte_eth_dev *dev,
 		uint16_t queue_idx, uint16_t tx_rate);
 
-static void ixgbevf_add_mac_addr(struct rte_eth_dev *dev,
-				 struct ether_addr *mac_addr,
-				 uint32_t index, uint32_t pool);
+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,
 					     struct ether_addr *mac_addr);
@@ -4622,14 +4622,15 @@ ixgbe_dev_rss_reta_query(struct rte_eth_dev *dev,
 	return 0;
 }
 
-static void
+static int
 ixgbe_add_rar(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 				uint32_t index, uint32_t pool)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	uint32_t enable_addr = 1;
 
-	ixgbe_set_rar(hw, index, mac_addr->addr_bytes, pool, enable_addr);
+	return ixgbe_set_rar(hw, index, mac_addr->addr_bytes,
+			     pool, enable_addr);
 }
 
 static void
@@ -5625,7 +5626,7 @@ static int ixgbe_set_queue_rate_limit(struct rte_eth_dev *dev,
 	return 0;
 }
 
-static void
+static int
 ixgbevf_add_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		     __attribute__((unused)) uint32_t index,
 		     __attribute__((unused)) uint32_t pool)
@@ -5639,11 +5640,19 @@ ixgbevf_add_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 	 * set of PF resources used to store VF MAC addresses.
 	 */
 	if (memcmp(hw->mac.perm_addr, mac_addr, sizeof(struct ether_addr)) == 0)
-		return;
+		return -1;
 	diag = ixgbevf_set_uc_addr_vf(hw, 2, mac_addr->addr_bytes);
-	if (diag == 0)
-		return;
-	PMD_DRV_LOG(ERR, "Unable to add MAC address - diag=%d", diag);
+	if (diag != 0)
+		PMD_DRV_LOG(ERR, "Unable to add MAC address "
+			    "%02x:%02x:%02x:%02x:%02x:%02x- diag=%d",
+			    mac_addr->addr_bytes[0],
+			    mac_addr->addr_bytes[1],
+			    mac_addr->addr_bytes[2],
+			    mac_addr->addr_bytes[3],
+			    mac_addr->addr_bytes[4],
+			    mac_addr->addr_bytes[5],
+			    diag);
+	return diag;
 }
 
 static void
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 0fdba80..2bb9645 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -4503,26 +4503,30 @@ mlx4_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
  * @param vmdq
  *   VMDq pool index to associate address with (ignored).
  */
-static void
+static int
 mlx4_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		  uint32_t index, uint32_t vmdq)
 {
 	struct priv *priv = dev->data->dev_private;
+	int re;
 
 	if (mlx4_is_secondary())
-		return;
+		return -ENOTSUP;
 	(void)vmdq;
 	priv_lock(priv);
 	DEBUG("%p: adding MAC address at index %" PRIu32,
 	      (void *)dev, index);
 	/* Last array entry is reserved for broadcast. */
-	if (index >= (elemof(priv->mac) - 1))
+	if (index >= (elemof(priv->mac) - 1)) {
+		re = EINVAL;
 		goto end;
-	priv_mac_addr_add(priv, index,
-			  (const uint8_t (*)[ETHER_ADDR_LEN])
-			  mac_addr->addr_bytes);
+	}
+	re = priv_mac_addr_add(priv, index,
+			       (const uint8_t (*)[ETHER_ADDR_LEN])
+			       mac_addr->addr_bytes);
 end:
 	priv_unlock(priv);
+	return -re;
 }
 
 /**
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 71e19ec..67fd742 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -238,8 +238,8 @@ int hash_rxq_mac_addrs_add(struct hash_rxq *);
 int priv_mac_addr_add(struct priv *, unsigned int,
 		      const uint8_t (*)[ETHER_ADDR_LEN]);
 int priv_mac_addrs_enable(struct priv *);
-void mlx5_mac_addr_add(struct rte_eth_dev *, struct ether_addr *, uint32_t,
-		       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 *);
 
 /* mlx5_rss.c */
diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
index 4fcfd3b..79e0c41 100644
--- a/drivers/net/mlx5/mlx5_mac.c
+++ b/drivers/net/mlx5/mlx5_mac.c
@@ -470,26 +470,30 @@ priv_mac_addrs_enable(struct priv *priv)
  * @param vmdq
  *   VMDq pool index to associate address with (ignored).
  */
-void
+int
 mlx5_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		  uint32_t index, uint32_t vmdq)
 {
 	struct priv *priv = dev->data->dev_private;
+	int re;
 
 	if (mlx5_is_secondary())
-		return;
+		return -ENOTSUP;
 
 	(void)vmdq;
 	priv_lock(priv);
 	DEBUG("%p: adding MAC address at index %" PRIu32,
 	      (void *)dev, index);
-	if (index >= RTE_DIM(priv->mac))
+	if (index >= RTE_DIM(priv->mac)) {
+		re = EINVAL;
 		goto end;
-	priv_mac_addr_add(priv, index,
-			  (const uint8_t (*)[ETHER_ADDR_LEN])
-			  mac_addr->addr_bytes);
+	}
+	re = priv_mac_addr_add(priv, index,
+			       (const uint8_t (*)[ETHER_ADDR_LEN])
+			       mac_addr->addr_bytes);
 end:
 	priv_unlock(priv);
+	return -re;
 }
 
 /**
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index fbad2a6..e2e963e 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -506,16 +506,18 @@ qede_mac_int_ops(struct rte_eth_dev *eth_dev, struct ecore_filter_ucast *ucast,
 	return rc;
 }
 
-static void
+static int
 qede_mac_addr_add(struct rte_eth_dev *eth_dev, struct ether_addr *mac_addr,
 		  uint32_t index, __rte_unused uint32_t pool)
 {
 	struct ecore_filter_ucast ucast;
+	int re;
 
 	qede_set_ucast_cmn_params(&ucast);
 	ucast.type = ECORE_FILTER_MAC;
 	ether_addr_copy(mac_addr, (struct ether_addr *)&ucast.mac);
-	(void)qede_mac_int_ops(eth_dev, &ucast, 1);
+	re = (int)qede_mac_int_ops(eth_dev, &ucast, 1);
+	return re;
 }
 
 static void
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 36867e6..87d2258 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -224,12 +224,13 @@ eth_mac_addr_remove(struct rte_eth_dev *dev __rte_unused,
 {
 }
 
-static void
+static int
 eth_mac_addr_add(struct rte_eth_dev *dev __rte_unused,
 	struct ether_addr *mac_addr __rte_unused,
 	uint32_t index __rte_unused,
 	uint32_t vmdq __rte_unused)
 {
+	return -ENOTSUP;
 }
 
 static void
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index e6c57b3..a042b31 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -87,7 +87,7 @@ static void virtio_dev_stats_reset(struct rte_eth_dev *dev);
 static void virtio_dev_free_mbufs(struct rte_eth_dev *dev);
 static int virtio_vlan_filter_set(struct rte_eth_dev *dev,
 				uint16_t vlan_id, int on);
-static void virtio_mac_addr_add(struct rte_eth_dev *dev,
+static int virtio_mac_addr_add(struct rte_eth_dev *dev,
 				struct ether_addr *mac_addr,
 				uint32_t index, uint32_t vmdq __rte_unused);
 static void virtio_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index);
@@ -1020,7 +1020,7 @@ virtio_get_hwaddr(struct virtio_hw *hw)
 	}
 }
 
-static void
+static int
 virtio_mac_table_set(struct virtio_hw *hw,
 		     const struct virtio_net_ctrl_mac *uc,
 		     const struct virtio_net_ctrl_mac *mc)
@@ -1030,7 +1030,7 @@ virtio_mac_table_set(struct virtio_hw *hw,
 
 	if (!vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
 		PMD_DRV_LOG(INFO, "host does not support mac table");
-		return;
+		return -1;
 	}
 
 	ctrl.hdr.class = VIRTIO_NET_CTRL_MAC;
@@ -1045,9 +1045,10 @@ virtio_mac_table_set(struct virtio_hw *hw,
 	err = virtio_send_command(hw->cvq, &ctrl, len, 2);
 	if (err != 0)
 		PMD_DRV_LOG(NOTICE, "mac table set failed: %d", err);
+	return err;
 }
 
-static void
+static int
 virtio_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		    uint32_t index, uint32_t vmdq __rte_unused)
 {
@@ -1058,7 +1059,7 @@ virtio_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 
 	if (index >= VIRTIO_MAX_MAC_ADDRS) {
 		PMD_DRV_LOG(ERR, "mac address index %u out of range", index);
-		return;
+		return -EINVAL;
 	}
 
 	uc = alloca(VIRTIO_MAX_MAC_ADDRS * ETHER_ADDR_LEN + sizeof(uc->entries));
@@ -1075,7 +1076,7 @@ virtio_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		memcpy(&tbl->macs[tbl->entries++], addr, ETHER_ADDR_LEN);
 	}
 
-	virtio_mac_table_set(hw, uc, mc);
+	return virtio_mac_table_set(hw, uc, mc);
 }
 
 static void
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 89f6514..59bed5d 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -2297,6 +2297,7 @@ rte_eth_dev_mac_addr_add(uint8_t port_id, struct ether_addr *addr,
 	struct rte_eth_dev *dev;
 	int index;
 	uint64_t pool_mask;
+	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
@@ -2329,15 +2330,17 @@ rte_eth_dev_mac_addr_add(uint8_t port_id, struct ether_addr *addr,
 	}
 
 	/* Update NIC */
-	(*dev->dev_ops->mac_addr_add)(dev, addr, index, pool);
+	ret = (*dev->dev_ops->mac_addr_add)(dev, addr, index, pool);
 
-	/* Update address in NIC data structure */
-	ether_addr_copy(addr, &dev->data->mac_addrs[index]);
+	if (ret == 0) {
+		/* Update address in NIC data structure */
+		ether_addr_copy(addr, &dev->data->mac_addrs[index]);
 
-	/* Update pool bitmap in NIC data structure */
-	dev->data->mac_pool_sel[index] |= (1ULL << pool);
+		/* Update pool bitmap in NIC data structure */
+		dev->data->mac_pool_sel[index] |= (1ULL << pool);
+	}
 
-	return 0;
+	return ret;
 }
 
 int
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index e0f7ee5..1917252 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1296,7 +1296,7 @@ typedef int (*eth_dev_led_off_t)(struct rte_eth_dev *dev);
 typedef void (*eth_mac_addr_remove_t)(struct rte_eth_dev *dev, uint32_t index);
 /**< @internal Remove MAC address from receive address register */
 
-typedef void (*eth_mac_addr_add_t)(struct rte_eth_dev *dev,
+typedef int (*eth_mac_addr_add_t)(struct rte_eth_dev *dev,
 				  struct ether_addr *mac_addr,
 				  uint32_t index,
 				  uint32_t vmdq);
-- 
2.7.4

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

* [dpdk-stable] [PATCH v6 1/3] ethdev: fix adding invalid MAC addr
       [not found] ` <1493729065-17090-1-git-send-email-wei.dai@intel.com>
@ 2017-05-02 12:44   ` Wei Dai
  2017-05-03  0:57     ` Lu, Wenzhuo
  2017-05-03  1:46     ` Yuanhan Liu
  0 siblings, 2 replies; 6+ messages in thread
From: Wei Dai @ 2017-05-02 12:44 UTC (permalink / raw)
  To: wenzhuo.lu, thomas, harish.patil, rasesh.mody, stephen.hurd,
	ajit.khaparde, helin.zhang, konstantin.ananyev, jingjing.wu,
	jing.d.chen, adrien.mazarguil, nelio.laranjeiro,
	bruce.richardson, yuanhan.liu, maxime.coquelin, shepard.siegel,
	ed.czeck, john.miller
  Cc: dev, Wei Dai, stable

Some customers find adding MAC addr to VF sometimes can fail,
but it is still stored in dev->data->mac_addrs[ ]. So this
can lead to some errors that assumes the non-zero entry in
dev->data->mac_addrs[ ] is valid.
Following acknowledgements are from specific NIC PMD
maintainer for their managing part.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
 drivers/net/ark/ark_ethdev.c       | 15 +++++++++------
 drivers/net/bnx2x/bnx2x_ethdev.c   |  7 +++++--
 drivers/net/bnxt/bnxt_ethdev.c     | 16 ++++++++--------
 drivers/net/e1000/em_ethdev.c      |  8 ++++----
 drivers/net/e1000/igb_ethdev.c     |  9 +++++----
 drivers/net/enic/enic.h            |  2 +-
 drivers/net/enic/enic_ethdev.c     |  4 ++--
 drivers/net/enic/enic_main.c       |  9 ++++-----
 drivers/net/fm10k/fm10k_ethdev.c   |  3 ++-
 drivers/net/i40e/i40e_ethdev.c     | 17 +++++++++--------
 drivers/net/i40e/i40e_ethdev_vf.c  | 14 +++++++-------
 drivers/net/ixgbe/ixgbe_ethdev.c   | 33 +++++++++++++++++++++------------
 drivers/net/mlx4/mlx4.c            | 16 ++++++++++------
 drivers/net/mlx5/mlx5.h            |  4 ++--
 drivers/net/mlx5/mlx5_mac.c        | 16 ++++++++++------
 drivers/net/qede/qede_ethdev.c     |  6 ++++--
 drivers/net/ring/rte_eth_ring.c    |  3 ++-
 drivers/net/virtio/virtio_ethdev.c | 13 +++++++------
 lib/librte_ether/rte_ethdev.c      | 15 +++++++++------
 lib/librte_ether/rte_ethdev.h      |  2 +-
 20 files changed, 122 insertions(+), 90 deletions(-)

diff --git a/drivers/net/ark/ark_ethdev.c b/drivers/net/ark/ark_ethdev.c
index 83961f5..995c93d 100644
--- a/drivers/net/ark/ark_ethdev.c
+++ b/drivers/net/ark/ark_ethdev.c
@@ -71,10 +71,10 @@ static void eth_ark_dev_stats_get(struct rte_eth_dev *dev,
 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,
 					 struct ether_addr *mac_addr);
-static void eth_ark_macaddr_add(struct rte_eth_dev *dev,
-				struct ether_addr *mac_addr,
-				uint32_t index,
-				uint32_t pool);
+static int eth_ark_macaddr_add(struct rte_eth_dev *dev,
+			       struct ether_addr *mac_addr,
+			       uint32_t index,
+			       uint32_t pool);
 static void eth_ark_macaddr_remove(struct rte_eth_dev *dev,
 				   uint32_t index);
 
@@ -831,7 +831,7 @@ eth_ark_dev_stats_reset(struct rte_eth_dev *dev)
 		ark->user_ext.stats_reset(dev, ark->user_data);
 }
 
-static void
+static int
 eth_ark_macaddr_add(struct rte_eth_dev *dev,
 		    struct ether_addr *mac_addr,
 		    uint32_t index,
@@ -840,12 +840,15 @@ eth_ark_macaddr_add(struct rte_eth_dev *dev,
 	struct ark_adapter *ark =
 		(struct ark_adapter *)dev->data->dev_private;
 
-	if (ark->user_ext.mac_addr_add)
+	if (ark->user_ext.mac_addr_add) {
 		ark->user_ext.mac_addr_add(dev,
 					   mac_addr,
 					   index,
 					   pool,
 					   ark->user_data);
+		return 0;
+	}
+	return -ENOTSUP;
 }
 
 static void
diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index 314e5ea..b79cfdb 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -451,14 +451,17 @@ bnx2x_dev_infos_get(struct rte_eth_dev *dev, __rte_unused struct rte_eth_dev_inf
 	dev_info->speed_capa = ETH_LINK_SPEED_10G | ETH_LINK_SPEED_20G;
 }
 
-static void
+static int
 bnx2x_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		uint32_t index, uint32_t pool)
 {
 	struct bnx2x_softc *sc = dev->data->dev_private;
 
-	if (sc->mac_ops.mac_addr_add)
+	if (sc->mac_ops.mac_addr_add) {
 		sc->mac_ops.mac_addr_add(dev, mac_addr, index, pool);
+		return 0;
+	}
+	return -ENOTSUP;
 }
 
 static void
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 7805221..bb87361 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -618,9 +618,9 @@ static void bnxt_mac_addr_remove_op(struct rte_eth_dev *eth_dev,
 	}
 }
 
-static void bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
-				 struct ether_addr *mac_addr,
-				 uint32_t index, uint32_t pool)
+static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
+				struct ether_addr *mac_addr,
+				uint32_t index, uint32_t pool)
 {
 	struct bnxt *bp = (struct bnxt *)eth_dev->data->dev_private;
 	struct bnxt_vnic_info *vnic = STAILQ_FIRST(&bp->ff_pool[pool]);
@@ -628,30 +628,30 @@ static void bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 
 	if (BNXT_VF(bp)) {
 		RTE_LOG(ERR, PMD, "Cannot add MAC address to a VF interface\n");
-		return;
+		return -ENOTSUP;
 	}
 
 	if (!vnic) {
 		RTE_LOG(ERR, PMD, "VNIC not found for pool %d!\n", pool);
-		return;
+		return -EINVAL;
 	}
 	/* Attach requested MAC address to the new l2_filter */
 	STAILQ_FOREACH(filter, &vnic->filter, next) {
 		if (filter->mac_index == index) {
 			RTE_LOG(ERR, PMD,
 				"MAC addr already existed for pool %d\n", pool);
-			return;
+			return -EINVAL;
 		}
 	}
 	filter = bnxt_alloc_filter(bp);
 	if (!filter) {
 		RTE_LOG(ERR, PMD, "L2 filter alloc failed\n");
-		return;
+		return -ENODEV;
 	}
 	STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
 	filter->mac_index = index;
 	memcpy(filter->l2_addr, mac_addr, ETHER_ADDR_LEN);
-	bnxt_hwrm_set_filter(bp, vnic, filter);
+	return bnxt_hwrm_set_filter(bp, vnic, filter);
 }
 
 int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 599b9e0..57eb017 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -120,8 +120,8 @@ static int eth_em_led_on(struct rte_eth_dev *dev);
 static int eth_em_led_off(struct rte_eth_dev *dev);
 
 static int em_get_rx_buffer_size(struct e1000_hw *hw);
-static void eth_em_rar_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
-		uint32_t index, uint32_t pool);
+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 int eth_em_set_mc_addr_list(struct rte_eth_dev *dev,
@@ -1794,13 +1794,13 @@ eth_em_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 	return -EIO;
 }
 
-static void
+static int
 eth_em_rar_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		uint32_t index, __rte_unused uint32_t pool)
 {
 	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-	e1000_rar_set(hw, mac_addr->addr_bytes, index);
+	return e1000_rar_set(hw, mac_addr->addr_bytes, index);
 }
 
 static void
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index b6b81cb..e8c6282 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -171,9 +171,9 @@ static int eth_igb_led_off(struct rte_eth_dev *dev);
 
 static void igb_intr_disable(struct e1000_hw *hw);
 static int  igb_get_rx_buffer_size(struct e1000_hw *hw);
-static void eth_igb_rar_set(struct rte_eth_dev *dev,
-		struct ether_addr *mac_addr,
-		uint32_t index, uint32_t pool);
+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,
 		struct ether_addr *addr);
@@ -3079,7 +3079,7 @@ eth_igb_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 }
 
 #define E1000_RAH_POOLSEL_SHIFT      (18)
-static void
+static int
 eth_igb_rar_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 	        uint32_t index, __rte_unused uint32_t pool)
 {
@@ -3090,6 +3090,7 @@ eth_igb_rar_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 	rah = E1000_READ_REG(hw, E1000_RAH(index));
 	rah |= (0x1 << (E1000_RAH_POOLSEL_SHIFT + pool));
 	E1000_WRITE_REG(hw, E1000_RAH(index), rah);
+	return 0;
 }
 
 static void
diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index e921de4..d17a35f 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -279,7 +279,7 @@ extern void enic_dev_stats_get(struct enic *enic,
 	struct rte_eth_stats *r_stats);
 extern void enic_dev_stats_clear(struct enic *enic);
 extern void enic_add_packet_filter(struct enic *enic);
-void enic_set_mac_address(struct enic *enic, uint8_t *mac_addr);
+int enic_set_mac_address(struct enic *enic, uint8_t *mac_addr);
 void enic_del_mac_address(struct enic *enic, int mac_index);
 extern unsigned int enic_cleanup_wq(struct enic *enic, struct vnic_wq *wq);
 extern void enic_send_pkt(struct enic *enic, struct vnic_wq *wq,
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 7579696..372bae7 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -533,14 +533,14 @@ static void enicpmd_dev_allmulticast_disable(struct rte_eth_dev *eth_dev)
 	enic_add_packet_filter(enic);
 }
 
-static void enicpmd_add_mac_addr(struct rte_eth_dev *eth_dev,
+static int enicpmd_add_mac_addr(struct rte_eth_dev *eth_dev,
 	struct ether_addr *mac_addr,
 	__rte_unused uint32_t index, __rte_unused uint32_t pool)
 {
 	struct enic *enic = pmd_priv(eth_dev);
 
 	ENICPMD_FUNC_TRACE();
-	enic_set_mac_address(enic, mac_addr->addr_bytes);
+	return enic_set_mac_address(enic, mac_addr->addr_bytes);
 }
 
 static void enicpmd_remove_mac_addr(struct rte_eth_dev *eth_dev, uint32_t index)
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 5f2188b..d026241 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -202,20 +202,19 @@ void enic_del_mac_address(struct enic *enic, int mac_index)
 		dev_err(enic, "del mac addr failed\n");
 }
 
-void enic_set_mac_address(struct enic *enic, uint8_t *mac_addr)
+int enic_set_mac_address(struct enic *enic, uint8_t *mac_addr)
 {
 	int err;
 
 	if (!is_eth_addr_valid(mac_addr)) {
 		dev_err(enic, "invalid mac address\n");
-		return;
+		return -EINVAL;
 	}
 
 	err = vnic_dev_add_addr(enic->vdev, mac_addr);
-	if (err) {
+	if (err)
 		dev_err(enic, "add mac addr failed\n");
-		return;
-	}
+	return err;
 }
 
 static void
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 94b4d40..a742eec 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1690,7 +1690,7 @@ static void fm10k_MAC_filter_set(struct rte_eth_dev *dev,
 }
 
 /* Add a MAC address, and update filters */
-static void
+static int
 fm10k_macaddr_add(struct rte_eth_dev *dev,
 		struct ether_addr *mac_addr,
 		uint32_t index,
@@ -1701,6 +1701,7 @@ fm10k_macaddr_add(struct rte_eth_dev *dev,
 	macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private);
 	fm10k_MAC_filter_set(dev, mac_addr->addr_bytes, TRUE, pool);
 	macvlan->mac_vmdq_id[index] = pool;
+	return 0;
 }
 
 /* Remove a MAC address, and update filters */
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3fa25dc..986031d 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -291,10 +291,10 @@ static int i40e_flow_ctrl_set(struct rte_eth_dev *dev,
 			      struct rte_eth_fc_conf *fc_conf);
 static int i40e_priority_flow_ctrl_set(struct rte_eth_dev *dev,
 				       struct rte_eth_pfc_conf *pfc_conf);
-static void i40e_macaddr_add(struct rte_eth_dev *dev,
-			  struct ether_addr *mac_addr,
-			  uint32_t index,
-			  uint32_t pool);
+static int i40e_macaddr_add(struct rte_eth_dev *dev,
+			    struct ether_addr *mac_addr,
+			    uint32_t index,
+			    uint32_t pool);
 static void i40e_macaddr_remove(struct rte_eth_dev *dev, uint32_t index);
 static int i40e_dev_rss_reta_update(struct rte_eth_dev *dev,
 				    struct rte_eth_rss_reta_entry64 *reta_conf,
@@ -3267,7 +3267,7 @@ i40e_priority_flow_ctrl_set(__rte_unused struct rte_eth_dev *dev,
 }
 
 /* Add a MAC address, and update filters */
-static void
+static int
 i40e_macaddr_add(struct rte_eth_dev *dev,
 		 struct ether_addr *mac_addr,
 		 __rte_unused uint32_t index,
@@ -3284,13 +3284,13 @@ i40e_macaddr_add(struct rte_eth_dev *dev,
 		PMD_DRV_LOG(ERR, "VMDQ not %s, can't set mac to pool %u",
 			pf->flags & I40E_FLAG_VMDQ ? "configured" : "enabled",
 			pool);
-		return;
+		return -ENOTSUP;
 	}
 
 	if (pool > pf->nb_cfg_vmdq_vsi) {
 		PMD_DRV_LOG(ERR, "Pool number %u invalid. Max pool is %u",
 				pool, pf->nb_cfg_vmdq_vsi);
-		return;
+		return -EINVAL;
 	}
 
 	(void)rte_memcpy(&mac_filter.mac_addr, mac_addr, ETHER_ADDR_LEN);
@@ -3307,8 +3307,9 @@ i40e_macaddr_add(struct rte_eth_dev *dev,
 	ret = i40e_vsi_add_mac(vsi, &mac_filter);
 	if (ret != I40E_SUCCESS) {
 		PMD_DRV_LOG(ERR, "Failed to add MACVLAN filter");
-		return;
+		return -ENODEV;
 	}
+	return 0;
 }
 
 /* Remove a MAC address, and update filters */
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index cb34023..6c081f0 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -136,10 +136,10 @@ static int i40evf_dev_tx_queue_start(struct rte_eth_dev *dev,
 				     uint16_t tx_queue_id);
 static int i40evf_dev_tx_queue_stop(struct rte_eth_dev *dev,
 				    uint16_t tx_queue_id);
-static void i40evf_add_mac_addr(struct rte_eth_dev *dev,
-				struct ether_addr *addr,
-				uint32_t index,
-				uint32_t pool);
+static int i40evf_add_mac_addr(struct rte_eth_dev *dev,
+			       struct ether_addr *addr,
+			       uint32_t index,
+			       uint32_t pool);
 static void i40evf_del_mac_addr(struct rte_eth_dev *dev, uint32_t index);
 static int i40evf_dev_rss_reta_update(struct rte_eth_dev *dev,
 			struct rte_eth_rss_reta_entry64 *reta_conf,
@@ -855,7 +855,7 @@ i40evf_stop_queues(struct rte_eth_dev *dev)
 	return 0;
 }
 
-static void
+static int
 i40evf_add_mac_addr(struct rte_eth_dev *dev,
 		    struct ether_addr *addr,
 		    __rte_unused uint32_t index,
@@ -873,7 +873,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
 			    addr->addr_bytes[0], addr->addr_bytes[1],
 			    addr->addr_bytes[2], addr->addr_bytes[3],
 			    addr->addr_bytes[4], addr->addr_bytes[5]);
-		return;
+		return I40E_ERR_INVALID_MAC_ADDR;
 	}
 
 	list = (struct i40e_virtchnl_ether_addr_list *)cmd_buffer;
@@ -892,7 +892,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
 		PMD_DRV_LOG(ERR, "fail to execute command "
 			    "OP_ADD_ETHER_ADDRESS");
 
-	return;
+	return err;
 }
 
 static void
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index bbae4f9..006203c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -248,8 +248,8 @@ static int ixgbe_dev_interrupt_action(struct rte_eth_dev *dev,
 				      struct rte_intr_handle *handle);
 static void ixgbe_dev_interrupt_handler(void *param);
 static void ixgbe_dev_interrupt_delayed_handler(void *param);
-static void ixgbe_add_rar(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
-		uint32_t index, uint32_t pool);
+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,
 					   struct ether_addr *mac_addr);
@@ -305,9 +305,9 @@ static void ixgbe_configure_msix(struct rte_eth_dev *dev);
 static int ixgbe_set_queue_rate_limit(struct rte_eth_dev *dev,
 		uint16_t queue_idx, uint16_t tx_rate);
 
-static void ixgbevf_add_mac_addr(struct rte_eth_dev *dev,
-				 struct ether_addr *mac_addr,
-				 uint32_t index, uint32_t pool);
+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,
 					     struct ether_addr *mac_addr);
@@ -4637,14 +4637,15 @@ ixgbe_dev_rss_reta_query(struct rte_eth_dev *dev,
 	return 0;
 }
 
-static void
+static int
 ixgbe_add_rar(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 				uint32_t index, uint32_t pool)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	uint32_t enable_addr = 1;
 
-	ixgbe_set_rar(hw, index, mac_addr->addr_bytes, pool, enable_addr);
+	return ixgbe_set_rar(hw, index, mac_addr->addr_bytes,
+			     pool, enable_addr);
 }
 
 static void
@@ -5640,7 +5641,7 @@ static int ixgbe_set_queue_rate_limit(struct rte_eth_dev *dev,
 	return 0;
 }
 
-static void
+static int
 ixgbevf_add_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		     __attribute__((unused)) uint32_t index,
 		     __attribute__((unused)) uint32_t pool)
@@ -5654,11 +5655,19 @@ ixgbevf_add_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 	 * set of PF resources used to store VF MAC addresses.
 	 */
 	if (memcmp(hw->mac.perm_addr, mac_addr, sizeof(struct ether_addr)) == 0)
-		return;
+		return -1;
 	diag = ixgbevf_set_uc_addr_vf(hw, 2, mac_addr->addr_bytes);
-	if (diag == 0)
-		return;
-	PMD_DRV_LOG(ERR, "Unable to add MAC address - diag=%d", diag);
+	if (diag != 0)
+		PMD_DRV_LOG(ERR, "Unable to add MAC address "
+			    "%02x:%02x:%02x:%02x:%02x:%02x - diag=%d",
+			    mac_addr->addr_bytes[0],
+			    mac_addr->addr_bytes[1],
+			    mac_addr->addr_bytes[2],
+			    mac_addr->addr_bytes[3],
+			    mac_addr->addr_bytes[4],
+			    mac_addr->addr_bytes[5],
+			    diag);
+	return diag;
 }
 
 static void
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 0fdba80..2bb9645 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -4503,26 +4503,30 @@ mlx4_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
  * @param vmdq
  *   VMDq pool index to associate address with (ignored).
  */
-static void
+static int
 mlx4_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		  uint32_t index, uint32_t vmdq)
 {
 	struct priv *priv = dev->data->dev_private;
+	int re;
 
 	if (mlx4_is_secondary())
-		return;
+		return -ENOTSUP;
 	(void)vmdq;
 	priv_lock(priv);
 	DEBUG("%p: adding MAC address at index %" PRIu32,
 	      (void *)dev, index);
 	/* Last array entry is reserved for broadcast. */
-	if (index >= (elemof(priv->mac) - 1))
+	if (index >= (elemof(priv->mac) - 1)) {
+		re = EINVAL;
 		goto end;
-	priv_mac_addr_add(priv, index,
-			  (const uint8_t (*)[ETHER_ADDR_LEN])
-			  mac_addr->addr_bytes);
+	}
+	re = priv_mac_addr_add(priv, index,
+			       (const uint8_t (*)[ETHER_ADDR_LEN])
+			       mac_addr->addr_bytes);
 end:
 	priv_unlock(priv);
+	return -re;
 }
 
 /**
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 71e19ec..67fd742 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -238,8 +238,8 @@ int hash_rxq_mac_addrs_add(struct hash_rxq *);
 int priv_mac_addr_add(struct priv *, unsigned int,
 		      const uint8_t (*)[ETHER_ADDR_LEN]);
 int priv_mac_addrs_enable(struct priv *);
-void mlx5_mac_addr_add(struct rte_eth_dev *, struct ether_addr *, uint32_t,
-		       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 *);
 
 /* mlx5_rss.c */
diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
index 4fcfd3b..79e0c41 100644
--- a/drivers/net/mlx5/mlx5_mac.c
+++ b/drivers/net/mlx5/mlx5_mac.c
@@ -470,26 +470,30 @@ priv_mac_addrs_enable(struct priv *priv)
  * @param vmdq
  *   VMDq pool index to associate address with (ignored).
  */
-void
+int
 mlx5_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		  uint32_t index, uint32_t vmdq)
 {
 	struct priv *priv = dev->data->dev_private;
+	int re;
 
 	if (mlx5_is_secondary())
-		return;
+		return -ENOTSUP;
 
 	(void)vmdq;
 	priv_lock(priv);
 	DEBUG("%p: adding MAC address at index %" PRIu32,
 	      (void *)dev, index);
-	if (index >= RTE_DIM(priv->mac))
+	if (index >= RTE_DIM(priv->mac)) {
+		re = EINVAL;
 		goto end;
-	priv_mac_addr_add(priv, index,
-			  (const uint8_t (*)[ETHER_ADDR_LEN])
-			  mac_addr->addr_bytes);
+	}
+	re = priv_mac_addr_add(priv, index,
+			       (const uint8_t (*)[ETHER_ADDR_LEN])
+			       mac_addr->addr_bytes);
 end:
 	priv_unlock(priv);
+	return -re;
 }
 
 /**
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 7056fed..4ef5765 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -508,16 +508,18 @@ qede_mac_int_ops(struct rte_eth_dev *eth_dev, struct ecore_filter_ucast *ucast,
 	return rc;
 }
 
-static void
+static int
 qede_mac_addr_add(struct rte_eth_dev *eth_dev, struct ether_addr *mac_addr,
 		  __rte_unused uint32_t index, __rte_unused uint32_t pool)
 {
 	struct ecore_filter_ucast ucast;
+	int re;
 
 	qede_set_ucast_cmn_params(&ucast);
 	ucast.type = ECORE_FILTER_MAC;
 	ether_addr_copy(mac_addr, (struct ether_addr *)&ucast.mac);
-	(void)qede_mac_int_ops(eth_dev, &ucast, 1);
+	re = (int)qede_mac_int_ops(eth_dev, &ucast, 1);
+	return re;
 }
 
 static void
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 36867e6..87d2258 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -224,12 +224,13 @@ eth_mac_addr_remove(struct rte_eth_dev *dev __rte_unused,
 {
 }
 
-static void
+static int
 eth_mac_addr_add(struct rte_eth_dev *dev __rte_unused,
 	struct ether_addr *mac_addr __rte_unused,
 	uint32_t index __rte_unused,
 	uint32_t vmdq __rte_unused)
 {
+	return -ENOTSUP;
 }
 
 static void
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index b23f4c2..a4ed4cf 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -87,7 +87,7 @@ static void virtio_dev_stats_reset(struct rte_eth_dev *dev);
 static void virtio_dev_free_mbufs(struct rte_eth_dev *dev);
 static int virtio_vlan_filter_set(struct rte_eth_dev *dev,
 				uint16_t vlan_id, int on);
-static void virtio_mac_addr_add(struct rte_eth_dev *dev,
+static int virtio_mac_addr_add(struct rte_eth_dev *dev,
 				struct ether_addr *mac_addr,
 				uint32_t index, uint32_t vmdq __rte_unused);
 static void virtio_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index);
@@ -1025,7 +1025,7 @@ virtio_get_hwaddr(struct virtio_hw *hw)
 	}
 }
 
-static void
+static int
 virtio_mac_table_set(struct virtio_hw *hw,
 		     const struct virtio_net_ctrl_mac *uc,
 		     const struct virtio_net_ctrl_mac *mc)
@@ -1035,7 +1035,7 @@ virtio_mac_table_set(struct virtio_hw *hw,
 
 	if (!vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
 		PMD_DRV_LOG(INFO, "host does not support mac table");
-		return;
+		return -1;
 	}
 
 	ctrl.hdr.class = VIRTIO_NET_CTRL_MAC;
@@ -1050,9 +1050,10 @@ virtio_mac_table_set(struct virtio_hw *hw,
 	err = virtio_send_command(hw->cvq, &ctrl, len, 2);
 	if (err != 0)
 		PMD_DRV_LOG(NOTICE, "mac table set failed: %d", err);
+	return err;
 }
 
-static void
+static int
 virtio_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		    uint32_t index, uint32_t vmdq __rte_unused)
 {
@@ -1063,7 +1064,7 @@ virtio_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 
 	if (index >= VIRTIO_MAX_MAC_ADDRS) {
 		PMD_DRV_LOG(ERR, "mac address index %u out of range", index);
-		return;
+		return -EINVAL;
 	}
 
 	uc = alloca(VIRTIO_MAX_MAC_ADDRS * ETHER_ADDR_LEN + sizeof(uc->entries));
@@ -1080,7 +1081,7 @@ virtio_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		memcpy(&tbl->macs[tbl->entries++], addr, ETHER_ADDR_LEN);
 	}
 
-	virtio_mac_table_set(hw, uc, mc);
+	return virtio_mac_table_set(hw, uc, mc);
 }
 
 static void
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 5bc78c0..8cf8b65 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -2369,6 +2369,7 @@ rte_eth_dev_mac_addr_add(uint8_t port_id, struct ether_addr *addr,
 	struct rte_eth_dev *dev;
 	int index;
 	uint64_t pool_mask;
+	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
@@ -2401,15 +2402,17 @@ rte_eth_dev_mac_addr_add(uint8_t port_id, struct ether_addr *addr,
 	}
 
 	/* Update NIC */
-	(*dev->dev_ops->mac_addr_add)(dev, addr, index, pool);
+	ret = (*dev->dev_ops->mac_addr_add)(dev, addr, index, pool);
 
-	/* Update address in NIC data structure */
-	ether_addr_copy(addr, &dev->data->mac_addrs[index]);
+	if (ret == 0) {
+		/* Update address in NIC data structure */
+		ether_addr_copy(addr, &dev->data->mac_addrs[index]);
 
-	/* Update pool bitmap in NIC data structure */
-	dev->data->mac_pool_sel[index] |= (1ULL << pool);
+		/* Update pool bitmap in NIC data structure */
+		dev->data->mac_pool_sel[index] |= (1ULL << pool);
+	}
 
-	return 0;
+	return ret;
 }
 
 int
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index f8c8c13..0f38b45 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1282,7 +1282,7 @@ typedef int (*eth_dev_led_off_t)(struct rte_eth_dev *dev);
 typedef void (*eth_mac_addr_remove_t)(struct rte_eth_dev *dev, uint32_t index);
 /**< @internal Remove MAC address from receive address register */
 
-typedef void (*eth_mac_addr_add_t)(struct rte_eth_dev *dev,
+typedef int (*eth_mac_addr_add_t)(struct rte_eth_dev *dev,
 				  struct ether_addr *mac_addr,
 				  uint32_t index,
 				  uint32_t vmdq);
-- 
2.7.4

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

* Re: [dpdk-stable] [PATCH v6 1/3] ethdev: fix adding invalid MAC addr
  2017-05-02 12:44   ` [dpdk-stable] [PATCH v6 " Wei Dai
@ 2017-05-03  0:57     ` Lu, Wenzhuo
  2017-05-03  1:46     ` Yuanhan Liu
  1 sibling, 0 replies; 6+ messages in thread
From: Lu, Wenzhuo @ 2017-05-03  0:57 UTC (permalink / raw)
  To: Dai, Wei, thomas, harish.patil, rasesh.mody, stephen.hurd,
	ajit.khaparde, Zhang, Helin, Ananyev, Konstantin, Wu, Jingjing,
	Chen, Jing D, adrien.mazarguil, nelio.laranjeiro, Richardson,
	Bruce, yuanhan.liu, maxime.coquelin, shepard.siegel, ed.czeck,
	john.miller
  Cc: dev, stable

Hi,

> -----Original Message-----
> From: Dai, Wei
> Sent: Tuesday, May 2, 2017 8:44 PM
> To: Lu, Wenzhuo; thomas@monjalon.net; harish.patil@cavium.com;
> rasesh.mody@cavium.com; stephen.hurd@broadcom.com;
> ajit.khaparde@broadcom.com; Zhang, Helin; Ananyev, Konstantin; Wu,
> Jingjing; Chen, Jing D; adrien.mazarguil@6wind.com;
> nelio.laranjeiro@6wind.com; Richardson, Bruce;
> yuanhan.liu@linux.intel.com; maxime.coquelin@redhat.com;
> shepard.siegel@atomicrules.com; ed.czeck@atomicrules.com;
> john.miller@atomicrules.com
> Cc: dev@dpdk.org; Dai, Wei; stable@dpdk.org
> Subject: [PATCH v6 1/3] ethdev: fix adding invalid MAC addr
> 
> Some customers find adding MAC addr to VF sometimes can fail, but it is still
> stored in dev->data->mac_addrs[ ]. So this can lead to some errors that
> assumes the non-zero entry in
> dev->data->mac_addrs[ ] is valid.
> Following acknowledgements are from specific NIC PMD maintainer for their
> managing part.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

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

* Re: [dpdk-stable] [PATCH v6 1/3] ethdev: fix adding invalid MAC addr
  2017-05-02 12:44   ` [dpdk-stable] [PATCH v6 " Wei Dai
  2017-05-03  0:57     ` Lu, Wenzhuo
@ 2017-05-03  1:46     ` Yuanhan Liu
  2017-05-05  0:16       ` Dai, Wei
  1 sibling, 1 reply; 6+ messages in thread
From: Yuanhan Liu @ 2017-05-03  1:46 UTC (permalink / raw)
  To: Wei Dai
  Cc: wenzhuo.lu, thomas, harish.patil, rasesh.mody, stephen.hurd,
	ajit.khaparde, helin.zhang, konstantin.ananyev, jingjing.wu,
	jing.d.chen, adrien.mazarguil, nelio.laranjeiro,
	bruce.richardson, maxime.coquelin, shepard.siegel, ed.czeck,
	john.miller, dev, stable

On Tue, May 02, 2017 at 08:44:23PM +0800, Wei Dai wrote:
> Some customers find adding MAC addr to VF sometimes can fail,
> but it is still stored in dev->data->mac_addrs[ ]. So this
> can lead to some errors that assumes the non-zero entry in
> dev->data->mac_addrs[ ] is valid.
> Following acknowledgements are from specific NIC PMD
> maintainer for their managing part.
> 
> Fixes: af75078fece3 ("first public release")


> Cc: stable@dpdk.org

Just a note, this patch changes API. It should not be backported to a
stable/LTS release, even though it fixes something.

	--yliu

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

* Re: [dpdk-stable] [PATCH v6 1/3] ethdev: fix adding invalid MAC addr
  2017-05-03  1:46     ` Yuanhan Liu
@ 2017-05-05  0:16       ` Dai, Wei
  0 siblings, 0 replies; 6+ messages in thread
From: Dai, Wei @ 2017-05-05  0:16 UTC (permalink / raw)
  To: Yuanhan Liu
  Cc: Lu, Wenzhuo, thomas, harish.patil, rasesh.mody, stephen.hurd,
	ajit.khaparde, Zhang, Helin, Ananyev, Konstantin, Wu, Jingjing,
	Chen, Jing D, adrien.mazarguil, nelio.laranjeiro, Richardson,
	Bruce, maxime.coquelin, shepard.siegel, ed.czeck, john.miller,
	dev, stable

> Subject: Re: [PATCH v6 1/3] ethdev: fix adding invalid MAC addr
> 
> On Tue, May 02, 2017 at 08:44:23PM +0800, Wei Dai wrote:
> > Some customers find adding MAC addr to VF sometimes can fail, but it
> > is still stored in dev->data->mac_addrs[ ]. So this can lead to some
> > errors that assumes the non-zero entry in
> > dev->data->mac_addrs[ ] is valid.
> > Following acknowledgements are from specific NIC PMD maintainer for
> > their managing part.
> >
> > Fixes: af75078fece3 ("first public release")
> 
> 
> > Cc: stable@dpdk.org
> 
> Just a note, this patch changes API. It should not be backported to a stable/LTS
> release, even though it fixes something.
> 
> 	--yliu

Thank you, Yuanhan. I will drop the "Cc: stable@dpdk.org" in my v7 patch set.

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

* [dpdk-stable] [PATCH v5 1/3] ethdev: fix adding invalid MAC addr
       [not found] <1493446107-62078-1-git-send-email-wei.dai@intel.com>
@ 2017-04-29  6:08 ` Wei Dai
  0 siblings, 0 replies; 6+ messages in thread
From: Wei Dai @ 2017-04-29  6:08 UTC (permalink / raw)
  To: wenzhuo.lu, thomas, harish.patil, rasesh.mody, stephen.hurd,
	ajit.khaparde, helin.zhang, konstantin.ananyev, jingjing.wu,
	jing.d.chen, adrien.mazarguil, nelio.laranjeiro,
	bruce.richardson, yuanhan.liu, maxime.coquelin, shepard.siegel,
	ed.czeck, john.miller
  Cc: dev, Wei Dai, stable

Some customers find adding MAC addr to VF sometimes can fail,
but it is still stored in dev->data->mac_addrs[ ]. So this
can lead to some errors that assumes the non-zero entry in
dev->data->mac_addrs[ ] is valid.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ark/ark_ethdev.c       | 15 +++++++++------
 drivers/net/bnx2x/bnx2x_ethdev.c   |  7 +++++--
 drivers/net/bnxt/bnxt_ethdev.c     | 16 ++++++++--------
 drivers/net/e1000/base/e1000_api.c |  2 +-
 drivers/net/e1000/em_ethdev.c      |  8 ++++----
 drivers/net/e1000/igb_ethdev.c     |  9 +++++----
 drivers/net/enic/enic.h            |  2 +-
 drivers/net/enic/enic_ethdev.c     |  4 ++--
 drivers/net/enic/enic_main.c       |  9 ++++-----
 drivers/net/fm10k/fm10k_ethdev.c   |  3 ++-
 drivers/net/i40e/i40e_ethdev.c     | 17 +++++++++--------
 drivers/net/i40e/i40e_ethdev_vf.c  | 14 +++++++-------
 drivers/net/ixgbe/ixgbe_ethdev.c   | 33 +++++++++++++++++++++------------
 drivers/net/mlx4/mlx4.c            | 16 ++++++++++------
 drivers/net/mlx5/mlx5.h            |  4 ++--
 drivers/net/mlx5/mlx5_mac.c        | 16 ++++++++++------
 drivers/net/qede/qede_ethdev.c     |  6 ++++--
 drivers/net/ring/rte_eth_ring.c    |  3 ++-
 drivers/net/virtio/virtio_ethdev.c | 13 +++++++------
 lib/librte_ether/rte_ethdev.c      | 15 +++++++++------
 lib/librte_ether/rte_ethdev.h      |  2 +-
 21 files changed, 123 insertions(+), 91 deletions(-)

diff --git a/drivers/net/ark/ark_ethdev.c b/drivers/net/ark/ark_ethdev.c
index 4caad98..0c3fa42 100644
--- a/drivers/net/ark/ark_ethdev.c
+++ b/drivers/net/ark/ark_ethdev.c
@@ -71,10 +71,10 @@ static void eth_ark_dev_stats_get(struct rte_eth_dev *dev,
 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,
 					 struct ether_addr *mac_addr);
-static void eth_ark_macaddr_add(struct rte_eth_dev *dev,
-				struct ether_addr *mac_addr,
-				uint32_t index,
-				uint32_t pool);
+static int eth_ark_macaddr_add(struct rte_eth_dev *dev,
+			       struct ether_addr *mac_addr,
+			       uint32_t index,
+			       uint32_t pool);
 static void eth_ark_macaddr_remove(struct rte_eth_dev *dev,
 				   uint32_t index);
 
@@ -831,7 +831,7 @@ eth_ark_dev_stats_reset(struct rte_eth_dev *dev)
 		ark->user_ext.stats_reset(dev, ark->user_data);
 }
 
-static void
+static int
 eth_ark_macaddr_add(struct rte_eth_dev *dev,
 		    struct ether_addr *mac_addr,
 		    uint32_t index,
@@ -840,12 +840,15 @@ eth_ark_macaddr_add(struct rte_eth_dev *dev,
 	struct ark_adapter *ark =
 		(struct ark_adapter *)dev->data->dev_private;
 
-	if (ark->user_ext.mac_addr_add)
+	if (ark->user_ext.mac_addr_add) {
 		ark->user_ext.mac_addr_add(dev,
 					   mac_addr,
 					   index,
 					   pool,
 					   ark->user_data);
+		return 0;
+	}
+	return -ENOTSUP;
 }
 
 static void
diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index 314e5ea..b79cfdb 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -451,14 +451,17 @@ bnx2x_dev_infos_get(struct rte_eth_dev *dev, __rte_unused struct rte_eth_dev_inf
 	dev_info->speed_capa = ETH_LINK_SPEED_10G | ETH_LINK_SPEED_20G;
 }
 
-static void
+static int
 bnx2x_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		uint32_t index, uint32_t pool)
 {
 	struct bnx2x_softc *sc = dev->data->dev_private;
 
-	if (sc->mac_ops.mac_addr_add)
+	if (sc->mac_ops.mac_addr_add) {
 		sc->mac_ops.mac_addr_add(dev, mac_addr, index, pool);
+		return 0;
+	}
+	return -ENOTSUP;
 }
 
 static void
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 5dc3ff0..c18555f 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -614,9 +614,9 @@ static void bnxt_mac_addr_remove_op(struct rte_eth_dev *eth_dev,
 	}
 }
 
-static void bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
-				 struct ether_addr *mac_addr,
-				 uint32_t index, uint32_t pool)
+static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
+				struct ether_addr *mac_addr,
+				uint32_t index, uint32_t pool)
 {
 	struct bnxt *bp = (struct bnxt *)eth_dev->data->dev_private;
 	struct bnxt_vnic_info *vnic = STAILQ_FIRST(&bp->ff_pool[pool]);
@@ -624,30 +624,30 @@ static void bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 
 	if (BNXT_VF(bp)) {
 		RTE_LOG(ERR, PMD, "Cannot add MAC address to a VF interface\n");
-		return;
+		return -ENOTSUP;
 	}
 
 	if (!vnic) {
 		RTE_LOG(ERR, PMD, "VNIC not found for pool %d!\n", pool);
-		return;
+		return -EINVAL;
 	}
 	/* Attach requested MAC address to the new l2_filter */
 	STAILQ_FOREACH(filter, &vnic->filter, next) {
 		if (filter->mac_index == index) {
 			RTE_LOG(ERR, PMD,
 				"MAC addr already existed for pool %d\n", pool);
-			return;
+			return -EINVAL;
 		}
 	}
 	filter = bnxt_alloc_filter(bp);
 	if (!filter) {
 		RTE_LOG(ERR, PMD, "L2 filter alloc failed\n");
-		return;
+		return -ENODEV;
 	}
 	STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
 	filter->mac_index = index;
 	memcpy(filter->l2_addr, mac_addr, ETHER_ADDR_LEN);
-	bnxt_hwrm_set_filter(bp, vnic, filter);
+	return bnxt_hwrm_set_filter(bp, vnic, filter);
 }
 
 int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete)
diff --git a/drivers/net/e1000/base/e1000_api.c b/drivers/net/e1000/base/e1000_api.c
index f7cf83b..dcb53f7 100644
--- a/drivers/net/e1000/base/e1000_api.c
+++ b/drivers/net/e1000/base/e1000_api.c
@@ -855,7 +855,7 @@ int e1000_rar_set(struct e1000_hw *hw, u8 *addr, u32 index)
 	if (hw->mac.ops.rar_set)
 		return hw->mac.ops.rar_set(hw, addr, index);
 
-	return E1000_SUCCESS;
+	return E1000_ERR_NO_SPACE;
 }
 
 /**
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 599b9e0..57eb017 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -120,8 +120,8 @@ static int eth_em_led_on(struct rte_eth_dev *dev);
 static int eth_em_led_off(struct rte_eth_dev *dev);
 
 static int em_get_rx_buffer_size(struct e1000_hw *hw);
-static void eth_em_rar_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
-		uint32_t index, uint32_t pool);
+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 int eth_em_set_mc_addr_list(struct rte_eth_dev *dev,
@@ -1794,13 +1794,13 @@ eth_em_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 	return -EIO;
 }
 
-static void
+static int
 eth_em_rar_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		uint32_t index, __rte_unused uint32_t pool)
 {
 	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-	e1000_rar_set(hw, mac_addr->addr_bytes, index);
+	return e1000_rar_set(hw, mac_addr->addr_bytes, index);
 }
 
 static void
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index ca9f98c..7574c26 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -169,9 +169,9 @@ static int eth_igb_led_off(struct rte_eth_dev *dev);
 
 static void igb_intr_disable(struct e1000_hw *hw);
 static int  igb_get_rx_buffer_size(struct e1000_hw *hw);
-static void eth_igb_rar_set(struct rte_eth_dev *dev,
-		struct ether_addr *mac_addr,
-		uint32_t index, uint32_t pool);
+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,
 		struct ether_addr *addr);
@@ -3077,7 +3077,7 @@ eth_igb_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 }
 
 #define E1000_RAH_POOLSEL_SHIFT      (18)
-static void
+static int
 eth_igb_rar_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 	        uint32_t index, __rte_unused uint32_t pool)
 {
@@ -3088,6 +3088,7 @@ eth_igb_rar_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 	rah = E1000_READ_REG(hw, E1000_RAH(index));
 	rah |= (0x1 << (E1000_RAH_POOLSEL_SHIFT + pool));
 	E1000_WRITE_REG(hw, E1000_RAH(index), rah);
+	return 0;
 }
 
 static void
diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index e921de4..d17a35f 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -279,7 +279,7 @@ extern void enic_dev_stats_get(struct enic *enic,
 	struct rte_eth_stats *r_stats);
 extern void enic_dev_stats_clear(struct enic *enic);
 extern void enic_add_packet_filter(struct enic *enic);
-void enic_set_mac_address(struct enic *enic, uint8_t *mac_addr);
+int enic_set_mac_address(struct enic *enic, uint8_t *mac_addr);
 void enic_del_mac_address(struct enic *enic, int mac_index);
 extern unsigned int enic_cleanup_wq(struct enic *enic, struct vnic_wq *wq);
 extern void enic_send_pkt(struct enic *enic, struct vnic_wq *wq,
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 7579696..372bae7 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -533,14 +533,14 @@ static void enicpmd_dev_allmulticast_disable(struct rte_eth_dev *eth_dev)
 	enic_add_packet_filter(enic);
 }
 
-static void enicpmd_add_mac_addr(struct rte_eth_dev *eth_dev,
+static int enicpmd_add_mac_addr(struct rte_eth_dev *eth_dev,
 	struct ether_addr *mac_addr,
 	__rte_unused uint32_t index, __rte_unused uint32_t pool)
 {
 	struct enic *enic = pmd_priv(eth_dev);
 
 	ENICPMD_FUNC_TRACE();
-	enic_set_mac_address(enic, mac_addr->addr_bytes);
+	return enic_set_mac_address(enic, mac_addr->addr_bytes);
 }
 
 static void enicpmd_remove_mac_addr(struct rte_eth_dev *eth_dev, uint32_t index)
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 5f2188b..d026241 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -202,20 +202,19 @@ void enic_del_mac_address(struct enic *enic, int mac_index)
 		dev_err(enic, "del mac addr failed\n");
 }
 
-void enic_set_mac_address(struct enic *enic, uint8_t *mac_addr)
+int enic_set_mac_address(struct enic *enic, uint8_t *mac_addr)
 {
 	int err;
 
 	if (!is_eth_addr_valid(mac_addr)) {
 		dev_err(enic, "invalid mac address\n");
-		return;
+		return -EINVAL;
 	}
 
 	err = vnic_dev_add_addr(enic->vdev, mac_addr);
-	if (err) {
+	if (err)
 		dev_err(enic, "add mac addr failed\n");
-		return;
-	}
+	return err;
 }
 
 static void
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 94b4d40..a742eec 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1690,7 +1690,7 @@ static void fm10k_MAC_filter_set(struct rte_eth_dev *dev,
 }
 
 /* Add a MAC address, and update filters */
-static void
+static int
 fm10k_macaddr_add(struct rte_eth_dev *dev,
 		struct ether_addr *mac_addr,
 		uint32_t index,
@@ -1701,6 +1701,7 @@ fm10k_macaddr_add(struct rte_eth_dev *dev,
 	macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private);
 	fm10k_MAC_filter_set(dev, mac_addr->addr_bytes, TRUE, pool);
 	macvlan->mac_vmdq_id[index] = pool;
+	return 0;
 }
 
 /* Remove a MAC address, and update filters */
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3fa25dc..986031d 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -291,10 +291,10 @@ static int i40e_flow_ctrl_set(struct rte_eth_dev *dev,
 			      struct rte_eth_fc_conf *fc_conf);
 static int i40e_priority_flow_ctrl_set(struct rte_eth_dev *dev,
 				       struct rte_eth_pfc_conf *pfc_conf);
-static void i40e_macaddr_add(struct rte_eth_dev *dev,
-			  struct ether_addr *mac_addr,
-			  uint32_t index,
-			  uint32_t pool);
+static int i40e_macaddr_add(struct rte_eth_dev *dev,
+			    struct ether_addr *mac_addr,
+			    uint32_t index,
+			    uint32_t pool);
 static void i40e_macaddr_remove(struct rte_eth_dev *dev, uint32_t index);
 static int i40e_dev_rss_reta_update(struct rte_eth_dev *dev,
 				    struct rte_eth_rss_reta_entry64 *reta_conf,
@@ -3267,7 +3267,7 @@ i40e_priority_flow_ctrl_set(__rte_unused struct rte_eth_dev *dev,
 }
 
 /* Add a MAC address, and update filters */
-static void
+static int
 i40e_macaddr_add(struct rte_eth_dev *dev,
 		 struct ether_addr *mac_addr,
 		 __rte_unused uint32_t index,
@@ -3284,13 +3284,13 @@ i40e_macaddr_add(struct rte_eth_dev *dev,
 		PMD_DRV_LOG(ERR, "VMDQ not %s, can't set mac to pool %u",
 			pf->flags & I40E_FLAG_VMDQ ? "configured" : "enabled",
 			pool);
-		return;
+		return -ENOTSUP;
 	}
 
 	if (pool > pf->nb_cfg_vmdq_vsi) {
 		PMD_DRV_LOG(ERR, "Pool number %u invalid. Max pool is %u",
 				pool, pf->nb_cfg_vmdq_vsi);
-		return;
+		return -EINVAL;
 	}
 
 	(void)rte_memcpy(&mac_filter.mac_addr, mac_addr, ETHER_ADDR_LEN);
@@ -3307,8 +3307,9 @@ i40e_macaddr_add(struct rte_eth_dev *dev,
 	ret = i40e_vsi_add_mac(vsi, &mac_filter);
 	if (ret != I40E_SUCCESS) {
 		PMD_DRV_LOG(ERR, "Failed to add MACVLAN filter");
-		return;
+		return -ENODEV;
 	}
+	return 0;
 }
 
 /* Remove a MAC address, and update filters */
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index cb34023..6c081f0 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -136,10 +136,10 @@ static int i40evf_dev_tx_queue_start(struct rte_eth_dev *dev,
 				     uint16_t tx_queue_id);
 static int i40evf_dev_tx_queue_stop(struct rte_eth_dev *dev,
 				    uint16_t tx_queue_id);
-static void i40evf_add_mac_addr(struct rte_eth_dev *dev,
-				struct ether_addr *addr,
-				uint32_t index,
-				uint32_t pool);
+static int i40evf_add_mac_addr(struct rte_eth_dev *dev,
+			       struct ether_addr *addr,
+			       uint32_t index,
+			       uint32_t pool);
 static void i40evf_del_mac_addr(struct rte_eth_dev *dev, uint32_t index);
 static int i40evf_dev_rss_reta_update(struct rte_eth_dev *dev,
 			struct rte_eth_rss_reta_entry64 *reta_conf,
@@ -855,7 +855,7 @@ i40evf_stop_queues(struct rte_eth_dev *dev)
 	return 0;
 }
 
-static void
+static int
 i40evf_add_mac_addr(struct rte_eth_dev *dev,
 		    struct ether_addr *addr,
 		    __rte_unused uint32_t index,
@@ -873,7 +873,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
 			    addr->addr_bytes[0], addr->addr_bytes[1],
 			    addr->addr_bytes[2], addr->addr_bytes[3],
 			    addr->addr_bytes[4], addr->addr_bytes[5]);
-		return;
+		return I40E_ERR_INVALID_MAC_ADDR;
 	}
 
 	list = (struct i40e_virtchnl_ether_addr_list *)cmd_buffer;
@@ -892,7 +892,7 @@ i40evf_add_mac_addr(struct rte_eth_dev *dev,
 		PMD_DRV_LOG(ERR, "fail to execute command "
 			    "OP_ADD_ETHER_ADDRESS");
 
-	return;
+	return err;
 }
 
 static void
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 7b856bb..e134bcd 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -247,8 +247,8 @@ static int ixgbe_dev_interrupt_action(struct rte_eth_dev *dev,
 				      struct rte_intr_handle *handle);
 static void ixgbe_dev_interrupt_handler(void *param);
 static void ixgbe_dev_interrupt_delayed_handler(void *param);
-static void ixgbe_add_rar(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
-		uint32_t index, uint32_t pool);
+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,
 					   struct ether_addr *mac_addr);
@@ -304,9 +304,9 @@ static void ixgbe_configure_msix(struct rte_eth_dev *dev);
 static int ixgbe_set_queue_rate_limit(struct rte_eth_dev *dev,
 		uint16_t queue_idx, uint16_t tx_rate);
 
-static void ixgbevf_add_mac_addr(struct rte_eth_dev *dev,
-				 struct ether_addr *mac_addr,
-				 uint32_t index, uint32_t pool);
+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,
 					     struct ether_addr *mac_addr);
@@ -4622,14 +4622,15 @@ ixgbe_dev_rss_reta_query(struct rte_eth_dev *dev,
 	return 0;
 }
 
-static void
+static int
 ixgbe_add_rar(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 				uint32_t index, uint32_t pool)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	uint32_t enable_addr = 1;
 
-	ixgbe_set_rar(hw, index, mac_addr->addr_bytes, pool, enable_addr);
+	return ixgbe_set_rar(hw, index, mac_addr->addr_bytes,
+			     pool, enable_addr);
 }
 
 static void
@@ -5625,7 +5626,7 @@ static int ixgbe_set_queue_rate_limit(struct rte_eth_dev *dev,
 	return 0;
 }
 
-static void
+static int
 ixgbevf_add_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		     __attribute__((unused)) uint32_t index,
 		     __attribute__((unused)) uint32_t pool)
@@ -5639,11 +5640,19 @@ ixgbevf_add_mac_addr(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 	 * set of PF resources used to store VF MAC addresses.
 	 */
 	if (memcmp(hw->mac.perm_addr, mac_addr, sizeof(struct ether_addr)) == 0)
-		return;
+		return -1;
 	diag = ixgbevf_set_uc_addr_vf(hw, 2, mac_addr->addr_bytes);
-	if (diag == 0)
-		return;
-	PMD_DRV_LOG(ERR, "Unable to add MAC address - diag=%d", diag);
+	if (diag != 0)
+		PMD_DRV_LOG(ERR, "Unable to add MAC address "
+			    "%02x:%02x:%02x:%02x:%02x:%02x- diag=%d",
+			    mac_addr->addr_bytes[0],
+			    mac_addr->addr_bytes[1],
+			    mac_addr->addr_bytes[2],
+			    mac_addr->addr_bytes[3],
+			    mac_addr->addr_bytes[4],
+			    mac_addr->addr_bytes[5],
+			    diag);
+	return diag;
 }
 
 static void
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 0fdba80..2bb9645 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -4503,26 +4503,30 @@ mlx4_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index)
  * @param vmdq
  *   VMDq pool index to associate address with (ignored).
  */
-static void
+static int
 mlx4_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		  uint32_t index, uint32_t vmdq)
 {
 	struct priv *priv = dev->data->dev_private;
+	int re;
 
 	if (mlx4_is_secondary())
-		return;
+		return -ENOTSUP;
 	(void)vmdq;
 	priv_lock(priv);
 	DEBUG("%p: adding MAC address at index %" PRIu32,
 	      (void *)dev, index);
 	/* Last array entry is reserved for broadcast. */
-	if (index >= (elemof(priv->mac) - 1))
+	if (index >= (elemof(priv->mac) - 1)) {
+		re = EINVAL;
 		goto end;
-	priv_mac_addr_add(priv, index,
-			  (const uint8_t (*)[ETHER_ADDR_LEN])
-			  mac_addr->addr_bytes);
+	}
+	re = priv_mac_addr_add(priv, index,
+			       (const uint8_t (*)[ETHER_ADDR_LEN])
+			       mac_addr->addr_bytes);
 end:
 	priv_unlock(priv);
+	return -re;
 }
 
 /**
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 71e19ec..67fd742 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -238,8 +238,8 @@ int hash_rxq_mac_addrs_add(struct hash_rxq *);
 int priv_mac_addr_add(struct priv *, unsigned int,
 		      const uint8_t (*)[ETHER_ADDR_LEN]);
 int priv_mac_addrs_enable(struct priv *);
-void mlx5_mac_addr_add(struct rte_eth_dev *, struct ether_addr *, uint32_t,
-		       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 *);
 
 /* mlx5_rss.c */
diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c
index 4fcfd3b..79e0c41 100644
--- a/drivers/net/mlx5/mlx5_mac.c
+++ b/drivers/net/mlx5/mlx5_mac.c
@@ -470,26 +470,30 @@ priv_mac_addrs_enable(struct priv *priv)
  * @param vmdq
  *   VMDq pool index to associate address with (ignored).
  */
-void
+int
 mlx5_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		  uint32_t index, uint32_t vmdq)
 {
 	struct priv *priv = dev->data->dev_private;
+	int re;
 
 	if (mlx5_is_secondary())
-		return;
+		return -ENOTSUP;
 
 	(void)vmdq;
 	priv_lock(priv);
 	DEBUG("%p: adding MAC address at index %" PRIu32,
 	      (void *)dev, index);
-	if (index >= RTE_DIM(priv->mac))
+	if (index >= RTE_DIM(priv->mac)) {
+		re = EINVAL;
 		goto end;
-	priv_mac_addr_add(priv, index,
-			  (const uint8_t (*)[ETHER_ADDR_LEN])
-			  mac_addr->addr_bytes);
+	}
+	re = priv_mac_addr_add(priv, index,
+			       (const uint8_t (*)[ETHER_ADDR_LEN])
+			       mac_addr->addr_bytes);
 end:
 	priv_unlock(priv);
+	return -re;
 }
 
 /**
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index fbad2a6..e2e963e 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -506,16 +506,18 @@ qede_mac_int_ops(struct rte_eth_dev *eth_dev, struct ecore_filter_ucast *ucast,
 	return rc;
 }
 
-static void
+static int
 qede_mac_addr_add(struct rte_eth_dev *eth_dev, struct ether_addr *mac_addr,
 		  uint32_t index, __rte_unused uint32_t pool)
 {
 	struct ecore_filter_ucast ucast;
+	int re;
 
 	qede_set_ucast_cmn_params(&ucast);
 	ucast.type = ECORE_FILTER_MAC;
 	ether_addr_copy(mac_addr, (struct ether_addr *)&ucast.mac);
-	(void)qede_mac_int_ops(eth_dev, &ucast, 1);
+	re = (int)qede_mac_int_ops(eth_dev, &ucast, 1);
+	return re;
 }
 
 static void
diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 36867e6..87d2258 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -224,12 +224,13 @@ eth_mac_addr_remove(struct rte_eth_dev *dev __rte_unused,
 {
 }
 
-static void
+static int
 eth_mac_addr_add(struct rte_eth_dev *dev __rte_unused,
 	struct ether_addr *mac_addr __rte_unused,
 	uint32_t index __rte_unused,
 	uint32_t vmdq __rte_unused)
 {
+	return -ENOTSUP;
 }
 
 static void
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index e6c57b3..a042b31 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -87,7 +87,7 @@ static void virtio_dev_stats_reset(struct rte_eth_dev *dev);
 static void virtio_dev_free_mbufs(struct rte_eth_dev *dev);
 static int virtio_vlan_filter_set(struct rte_eth_dev *dev,
 				uint16_t vlan_id, int on);
-static void virtio_mac_addr_add(struct rte_eth_dev *dev,
+static int virtio_mac_addr_add(struct rte_eth_dev *dev,
 				struct ether_addr *mac_addr,
 				uint32_t index, uint32_t vmdq __rte_unused);
 static void virtio_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index);
@@ -1020,7 +1020,7 @@ virtio_get_hwaddr(struct virtio_hw *hw)
 	}
 }
 
-static void
+static int
 virtio_mac_table_set(struct virtio_hw *hw,
 		     const struct virtio_net_ctrl_mac *uc,
 		     const struct virtio_net_ctrl_mac *mc)
@@ -1030,7 +1030,7 @@ virtio_mac_table_set(struct virtio_hw *hw,
 
 	if (!vtpci_with_feature(hw, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
 		PMD_DRV_LOG(INFO, "host does not support mac table");
-		return;
+		return -1;
 	}
 
 	ctrl.hdr.class = VIRTIO_NET_CTRL_MAC;
@@ -1045,9 +1045,10 @@ virtio_mac_table_set(struct virtio_hw *hw,
 	err = virtio_send_command(hw->cvq, &ctrl, len, 2);
 	if (err != 0)
 		PMD_DRV_LOG(NOTICE, "mac table set failed: %d", err);
+	return err;
 }
 
-static void
+static int
 virtio_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		    uint32_t index, uint32_t vmdq __rte_unused)
 {
@@ -1058,7 +1059,7 @@ virtio_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 
 	if (index >= VIRTIO_MAX_MAC_ADDRS) {
 		PMD_DRV_LOG(ERR, "mac address index %u out of range", index);
-		return;
+		return -EINVAL;
 	}
 
 	uc = alloca(VIRTIO_MAX_MAC_ADDRS * ETHER_ADDR_LEN + sizeof(uc->entries));
@@ -1075,7 +1076,7 @@ virtio_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		memcpy(&tbl->macs[tbl->entries++], addr, ETHER_ADDR_LEN);
 	}
 
-	virtio_mac_table_set(hw, uc, mc);
+	return virtio_mac_table_set(hw, uc, mc);
 }
 
 static void
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 89f6514..59bed5d 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -2297,6 +2297,7 @@ rte_eth_dev_mac_addr_add(uint8_t port_id, struct ether_addr *addr,
 	struct rte_eth_dev *dev;
 	int index;
 	uint64_t pool_mask;
+	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
@@ -2329,15 +2330,17 @@ rte_eth_dev_mac_addr_add(uint8_t port_id, struct ether_addr *addr,
 	}
 
 	/* Update NIC */
-	(*dev->dev_ops->mac_addr_add)(dev, addr, index, pool);
+	ret = (*dev->dev_ops->mac_addr_add)(dev, addr, index, pool);
 
-	/* Update address in NIC data structure */
-	ether_addr_copy(addr, &dev->data->mac_addrs[index]);
+	if (ret == 0) {
+		/* Update address in NIC data structure */
+		ether_addr_copy(addr, &dev->data->mac_addrs[index]);
 
-	/* Update pool bitmap in NIC data structure */
-	dev->data->mac_pool_sel[index] |= (1ULL << pool);
+		/* Update pool bitmap in NIC data structure */
+		dev->data->mac_pool_sel[index] |= (1ULL << pool);
+	}
 
-	return 0;
+	return ret;
 }
 
 int
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index e0f7ee5..1917252 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1296,7 +1296,7 @@ typedef int (*eth_dev_led_off_t)(struct rte_eth_dev *dev);
 typedef void (*eth_mac_addr_remove_t)(struct rte_eth_dev *dev, uint32_t index);
 /**< @internal Remove MAC address from receive address register */
 
-typedef void (*eth_mac_addr_add_t)(struct rte_eth_dev *dev,
+typedef int (*eth_mac_addr_add_t)(struct rte_eth_dev *dev,
 				  struct ether_addr *mac_addr,
 				  uint32_t index,
 				  uint32_t vmdq);
-- 
2.7.4

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

end of thread, other threads:[~2017-05-05  0:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1493525507-56304-1-git-send-email-wei.dai@intel.com>
2017-04-30  4:11 ` [dpdk-stable] [PATCH v5 1/3] ethdev: fix adding invalid MAC addr Wei Dai
     [not found] ` <1493729065-17090-1-git-send-email-wei.dai@intel.com>
2017-05-02 12:44   ` [dpdk-stable] [PATCH v6 " Wei Dai
2017-05-03  0:57     ` Lu, Wenzhuo
2017-05-03  1:46     ` Yuanhan Liu
2017-05-05  0:16       ` Dai, Wei
     [not found] <1493446107-62078-1-git-send-email-wei.dai@intel.com>
2017-04-29  6:08 ` [dpdk-stable] [PATCH v5 " Wei Dai

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