* [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; 5+ 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] 5+ messages in thread
[parent not found: <1493729065-17090-1-git-send-email-wei.dai@intel.com>]
* [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; 5+ 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] 5+ 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; 5+ 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] 5+ 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; 5+ 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] 5+ 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; 5+ 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] 5+ messages in thread
end of thread, other threads:[~2017-05-05 0:16 UTC | newest]
Thread overview: 5+ 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
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).