* [dpdk-stable] [PATCH 1/2] ethdev: fix adding invalid mac addr [not found] <1490979887-27827-1-git-send-email-wei.dai@intel.com> @ 2017-03-31 17:04 ` Wei Dai 0 siblings, 0 replies; 4+ messages in thread From: Wei Dai @ 2017-03-31 17:04 UTC (permalink / raw) To: dev; +Cc: 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/bnx2x/bnx2x_ethdev.c | 7 +++++-- drivers/net/bnxt/bnxt_ethdev.c | 12 ++++++------ drivers/net/e1000/base/e1000_api.c | 2 +- drivers/net/e1000/em_ethdev.c | 6 +++--- drivers/net/e1000/igb_ethdev.c | 5 +++-- drivers/net/enic/enic.h | 2 +- drivers/net/enic/enic_ethdev.c | 4 ++-- drivers/net/enic/enic_main.c | 6 +++--- drivers/net/fm10k/fm10k_ethdev.c | 3 ++- drivers/net/i40e/i40e_ethdev.c | 11 ++++++----- drivers/net/i40e/i40e_ethdev_vf.c | 8 ++++---- drivers/net/ixgbe/ixgbe_ethdev.c | 26 +++++++++++++++++--------- drivers/net/mlx4/mlx4.c | 14 +++++++++----- drivers/net/mlx5/mlx5.h | 2 +- drivers/net/mlx5/mlx5_mac.c | 12 ++++++++---- drivers/net/qede/qede_ethdev.c | 3 ++- 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, 92 insertions(+), 64 deletions(-) diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c index a0b0dfa..365a289 100644 --- a/drivers/net/bnx2x/bnx2x_ethdev.c +++ b/drivers/net/bnx2x/bnx2x_ethdev.c @@ -449,14 +449,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 -1; } static void diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index 6167443..95eb78d 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -613,7 +613,7 @@ 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, +static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev, struct ether_addr *mac_addr, uint32_t index, uint32_t pool) { @@ -623,30 +623,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 -1; } if (!vnic) { RTE_LOG(ERR, PMD, "VNIC not found for pool %d!\n", pool); - return; + return -2; } /* 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 -3; } } filter = bnxt_alloc_filter(bp); if (!filter) { RTE_LOG(ERR, PMD, "L2 filter alloc failed\n"); - return; + return -4; } 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 4f34c14..7d2cd79 100644 --- a/drivers/net/e1000/em_ethdev.c +++ b/drivers/net/e1000/em_ethdev.c @@ -120,7 +120,7 @@ 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, +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); @@ -1778,13 +1778,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 a73cd7a..35225ba 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -165,7 +165,7 @@ 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, +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); @@ -2976,7 +2976,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) { @@ -2987,6 +2987,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 2c2e29e..9ef9c31 100644 --- a/drivers/net/enic/enic_ethdev.c +++ b/drivers/net/enic/enic_ethdev.c @@ -532,14 +532,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 570b7b6..85490ab 100644 --- a/drivers/net/enic/enic_main.c +++ b/drivers/net/enic/enic_main.c @@ -202,20 +202,20 @@ 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 -1; } err = vnic_dev_add_addr(enic->vdev, mac_addr); 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 c4fe746..8f09b8c 100644 --- a/drivers/net/fm10k/fm10k_ethdev.c +++ b/drivers/net/fm10k/fm10k_ethdev.c @@ -1689,7 +1689,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, @@ -1700,6 +1700,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 8b5fd54..f23a4fa 100644 --- a/drivers/net/i40e/i40e_ethdev.c +++ b/drivers/net/i40e/i40e_ethdev.c @@ -290,7 +290,7 @@ 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, +static int i40e_macaddr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr, uint32_t index, uint32_t pool); @@ -3229,7 +3229,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, @@ -3246,13 +3246,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 -1; } 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 -2; } (void)rte_memcpy(&mac_filter.mac_addr, mac_addr, ETHER_ADDR_LEN); @@ -3269,8 +3269,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 -3; } + 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 d3659c9..b7469e4 100644 --- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c @@ -135,7 +135,7 @@ 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, +static int i40evf_add_mac_addr(struct rte_eth_dev *dev, struct ether_addr *addr, uint32_t index, uint32_t pool); @@ -854,7 +854,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, @@ -872,7 +872,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 -1; } list = (struct i40e_virtchnl_ether_addr_list *)cmd_buffer; @@ -891,7 +891,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 34bd681..a31902f 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -247,7 +247,7 @@ static int ixgbe_dev_interrupt_action(struct rte_eth_dev *dev, static void ixgbe_dev_interrupt_handler(struct rte_intr_handle *handle, 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, +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, @@ -304,7 +304,7 @@ 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, +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); @@ -4359,14 +4359,14 @@ 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 @@ -5884,7 +5884,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) @@ -5898,11 +5898,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 79efaaa..d61bb27 100644 --- a/drivers/net/mlx4/mlx4.c +++ b/drivers/net/mlx4/mlx4.c @@ -4630,26 +4630,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 -1; (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)) - goto end; - priv_mac_addr_add(priv, index, + if (index >= (elemof(priv->mac) - 1)) { + priv_unlock(priv); + return -2; + } + 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 879da5e..06b7431 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -229,7 +229,7 @@ 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, +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 *); diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c index 4fcfd3b..db93f4e 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 -1; (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 = -2; goto end; - priv_mac_addr_add(priv, index, + } + 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 0494dbd..dff257d 100644 --- a/drivers/net/qede/qede_ethdev.c +++ b/drivers/net/qede/qede_ethdev.c @@ -505,7 +505,7 @@ 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) { @@ -515,6 +515,7 @@ qede_mac_addr_add(struct rte_eth_dev *eth_dev, struct ether_addr *mac_addr, ucast.type = ECORE_FILTER_MAC; ether_addr_copy(mac_addr, (struct ether_addr *)&ucast.mac); (void)qede_mac_int_ops(eth_dev, &ucast, 1); + return 0; } static void diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c index 77ef3a1..4ca4ca8 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 -1; } static void diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index 4dc03b9..e242702 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -86,7 +86,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); @@ -1016,7 +1016,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) @@ -1026,7 +1026,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; @@ -1041,9 +1041,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) { @@ -1054,7 +1055,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 -1; } uc = alloca(VIRTIO_MAX_MAC_ADDRS * ETHER_ADDR_LEN + sizeof(uc->entries)); @@ -1071,7 +1072,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 c327c22..8735c02 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -2167,6 +2167,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]; @@ -2199,15 +2200,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 71a35f2..1eba177 100644 --- a/lib/librte_ether/rte_ethdev.h +++ b/lib/librte_ether/rte_ethdev.h @@ -1277,7 +1277,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] 4+ messages in thread
[parent not found: <1491033797-14186-1-git-send-email-wei.dai@intel.com>]
* [dpdk-stable] [PATCH 1/2] ethdev: fix adding invalid MAC addr [not found] <1491033797-14186-1-git-send-email-wei.dai@intel.com> @ 2017-04-01 8:03 ` Wei Dai 2017-04-03 9:18 ` Nélio Laranjeiro 0 siblings, 1 reply; 4+ messages in thread From: Wei Dai @ 2017-04-01 8:03 UTC (permalink / raw) To: dev, thomas.monjalon, harish.patil, rasesh.mody, stephen.hurd, ajit.khaparde, wenzhuo.lu, helin.zhang, konstantin.ananyev, jingjing.wu, jing.d.chen, adrien.mazarguil, nelio.laranjeiro, bruce.richardson, yuanhan.liu, maxime.coquelin Cc: 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/bnx2x/bnx2x_ethdev.c | 7 +++++-- drivers/net/bnxt/bnxt_ethdev.c | 12 ++++++------ drivers/net/e1000/base/e1000_api.c | 2 +- drivers/net/e1000/em_ethdev.c | 6 +++--- drivers/net/e1000/igb_ethdev.c | 5 +++-- drivers/net/enic/enic.h | 2 +- drivers/net/enic/enic_ethdev.c | 4 ++-- drivers/net/enic/enic_main.c | 6 +++--- drivers/net/fm10k/fm10k_ethdev.c | 3 ++- drivers/net/i40e/i40e_ethdev.c | 11 ++++++----- drivers/net/i40e/i40e_ethdev_vf.c | 8 ++++---- drivers/net/ixgbe/ixgbe_ethdev.c | 27 ++++++++++++++++++--------- drivers/net/mlx4/mlx4.c | 14 +++++++++----- drivers/net/mlx5/mlx5.h | 4 ++-- drivers/net/mlx5/mlx5_mac.c | 12 ++++++++---- drivers/net/qede/qede_ethdev.c | 3 ++- 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, 94 insertions(+), 65 deletions(-) diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c index a0b0dfa..365a289 100644 --- a/drivers/net/bnx2x/bnx2x_ethdev.c +++ b/drivers/net/bnx2x/bnx2x_ethdev.c @@ -449,14 +449,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 -1; } static void diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c index 6167443..95eb78d 100644 --- a/drivers/net/bnxt/bnxt_ethdev.c +++ b/drivers/net/bnxt/bnxt_ethdev.c @@ -613,7 +613,7 @@ 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, +static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev, struct ether_addr *mac_addr, uint32_t index, uint32_t pool) { @@ -623,30 +623,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 -1; } if (!vnic) { RTE_LOG(ERR, PMD, "VNIC not found for pool %d!\n", pool); - return; + return -2; } /* 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 -3; } } filter = bnxt_alloc_filter(bp); if (!filter) { RTE_LOG(ERR, PMD, "L2 filter alloc failed\n"); - return; + return -4; } 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 4f34c14..7d2cd79 100644 --- a/drivers/net/e1000/em_ethdev.c +++ b/drivers/net/e1000/em_ethdev.c @@ -120,7 +120,7 @@ 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, +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); @@ -1778,13 +1778,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 a73cd7a..35225ba 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -165,7 +165,7 @@ 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, +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); @@ -2976,7 +2976,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) { @@ -2987,6 +2987,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 2c2e29e..9ef9c31 100644 --- a/drivers/net/enic/enic_ethdev.c +++ b/drivers/net/enic/enic_ethdev.c @@ -532,14 +532,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 570b7b6..85490ab 100644 --- a/drivers/net/enic/enic_main.c +++ b/drivers/net/enic/enic_main.c @@ -202,20 +202,20 @@ 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 -1; } err = vnic_dev_add_addr(enic->vdev, mac_addr); 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 c4fe746..8f09b8c 100644 --- a/drivers/net/fm10k/fm10k_ethdev.c +++ b/drivers/net/fm10k/fm10k_ethdev.c @@ -1689,7 +1689,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, @@ -1700,6 +1700,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 8b5fd54..f23a4fa 100644 --- a/drivers/net/i40e/i40e_ethdev.c +++ b/drivers/net/i40e/i40e_ethdev.c @@ -290,7 +290,7 @@ 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, +static int i40e_macaddr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr, uint32_t index, uint32_t pool); @@ -3229,7 +3229,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, @@ -3246,13 +3246,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 -1; } 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 -2; } (void)rte_memcpy(&mac_filter.mac_addr, mac_addr, ETHER_ADDR_LEN); @@ -3269,8 +3269,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 -3; } + 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 d3659c9..b7469e4 100644 --- a/drivers/net/i40e/i40e_ethdev_vf.c +++ b/drivers/net/i40e/i40e_ethdev_vf.c @@ -135,7 +135,7 @@ 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, +static int i40evf_add_mac_addr(struct rte_eth_dev *dev, struct ether_addr *addr, uint32_t index, uint32_t pool); @@ -854,7 +854,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, @@ -872,7 +872,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 -1; } list = (struct i40e_virtchnl_ether_addr_list *)cmd_buffer; @@ -891,7 +891,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 34bd681..f9bdfce 100644 --- a/drivers/net/ixgbe/ixgbe_ethdev.c +++ b/drivers/net/ixgbe/ixgbe_ethdev.c @@ -247,7 +247,7 @@ static int ixgbe_dev_interrupt_action(struct rte_eth_dev *dev, static void ixgbe_dev_interrupt_handler(struct rte_intr_handle *handle, 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, +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, @@ -304,7 +304,7 @@ 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, +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); @@ -4359,14 +4359,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 @@ -5884,7 +5885,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) @@ -5898,11 +5899,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 79efaaa..d61bb27 100644 --- a/drivers/net/mlx4/mlx4.c +++ b/drivers/net/mlx4/mlx4.c @@ -4630,26 +4630,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 -1; (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)) - goto end; - priv_mac_addr_add(priv, index, + if (index >= (elemof(priv->mac) - 1)) { + priv_unlock(priv); + return -2; + } + 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 879da5e..cca7a36 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -229,8 +229,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 *dev, struct ether_addr *mac_addr, + uint32_t index, uint32_t vmdq); 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..db93f4e 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 -1; (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 = -2; goto end; - priv_mac_addr_add(priv, index, + } + 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 0494dbd..dff257d 100644 --- a/drivers/net/qede/qede_ethdev.c +++ b/drivers/net/qede/qede_ethdev.c @@ -505,7 +505,7 @@ 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) { @@ -515,6 +515,7 @@ qede_mac_addr_add(struct rte_eth_dev *eth_dev, struct ether_addr *mac_addr, ucast.type = ECORE_FILTER_MAC; ether_addr_copy(mac_addr, (struct ether_addr *)&ucast.mac); (void)qede_mac_int_ops(eth_dev, &ucast, 1); + return 0; } static void diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c index 77ef3a1..4ca4ca8 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 -1; } static void diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index 4dc03b9..e242702 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -86,7 +86,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); @@ -1016,7 +1016,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) @@ -1026,7 +1026,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; @@ -1041,9 +1041,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) { @@ -1054,7 +1055,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 -1; } uc = alloca(VIRTIO_MAX_MAC_ADDRS * ETHER_ADDR_LEN + sizeof(uc->entries)); @@ -1071,7 +1072,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 c327c22..8735c02 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -2167,6 +2167,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]; @@ -2199,15 +2200,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 71a35f2..1eba177 100644 --- a/lib/librte_ether/rte_ethdev.h +++ b/lib/librte_ether/rte_ethdev.h @@ -1277,7 +1277,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] 4+ messages in thread
* Re: [dpdk-stable] [PATCH 1/2] ethdev: fix adding invalid MAC addr 2017-04-01 8:03 ` [dpdk-stable] [PATCH 1/2] ethdev: fix adding invalid MAC addr Wei Dai @ 2017-04-03 9:18 ` Nélio Laranjeiro 2017-04-07 1:38 ` Dai, Wei 0 siblings, 1 reply; 4+ messages in thread From: Nélio Laranjeiro @ 2017-04-03 9:18 UTC (permalink / raw) To: Wei Dai Cc: dev, thomas.monjalon, harish.patil, rasesh.mody, stephen.hurd, ajit.khaparde, wenzhuo.lu, helin.zhang, konstantin.ananyev, jingjing.wu, jing.d.chen, adrien.mazarguil, bruce.richardson, yuanhan.liu, maxime.coquelin, stable Hi, Please see below some comments, On Sat, Apr 01, 2017 at 04:03:16PM +0800, Wei Dai wrote: >[...] > diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c > index 79efaaa..d61bb27 100644 > --- a/drivers/net/mlx4/mlx4.c > +++ b/drivers/net/mlx4/mlx4.c > @@ -4630,26 +4630,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 Please keep function documentation up to date. > 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 -1; Should not it return ENOSUP instead? > (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)) > - goto end; > - priv_mac_addr_add(priv, index, > + if (index >= (elemof(priv->mac) - 1)) { > + priv_unlock(priv); > + return -2; > + } > + re = priv_mac_addr_add(priv, index, > (const uint8_t (*)[ETHER_ADDR_LEN]) > mac_addr->addr_bytes); You have an issue here, internal function of mlx drivers return positives errno (as described in their functions documentation). It should be negated before returning. > end: > priv_unlock(priv); > + return re; > } > > /** > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h > index 879da5e..cca7a36 100644 > --- a/drivers/net/mlx5/mlx5.h > +++ b/drivers/net/mlx5/mlx5.h > @@ -229,8 +229,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 *dev, struct ether_addr *mac_addr, > + uint32_t index, uint32_t vmdq); > 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..db93f4e 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 -1; > > (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 = -2; > goto end; > - priv_mac_addr_add(priv, index, > + } > + re = priv_mac_addr_add(priv, index, > (const uint8_t (*)[ETHER_ADDR_LEN]) > mac_addr->addr_bytes); > end: > priv_unlock(priv); > + return re; > } >[...] Same remarks here, Thanks, -- Nélio Laranjeiro 6WIND ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-stable] [PATCH 1/2] ethdev: fix adding invalid MAC addr 2017-04-03 9:18 ` Nélio Laranjeiro @ 2017-04-07 1:38 ` Dai, Wei 0 siblings, 0 replies; 4+ messages in thread From: Dai, Wei @ 2017-04-07 1:38 UTC (permalink / raw) To: Nélio Laranjeiro Cc: dev, thomas.monjalon, harish.patil, rasesh.mody, stephen.hurd, ajit.khaparde, Lu, Wenzhuo, Zhang, Helin, Ananyev, Konstantin, Wu, Jingjing, Chen, Jing D, adrien.mazarguil, Richardson, Bruce, yuanhan.liu, maxime.coquelin, stable Hi, Nélio Laranjeiro Thanks for your feedback. I will change them in my next version of patch set. -Wei > -----Original Message----- > From: Nélio Laranjeiro [mailto:nelio.laranjeiro@6wind.com] > Sent: Monday, April 3, 2017 5:18 PM > To: Dai, Wei <wei.dai@intel.com> > Cc: dev@dpdk.org; thomas.monjalon@6wind.com; harish.patil@cavium.com; > rasesh.mody@cavium.com; stephen.hurd@broadcom.com; > ajit.khaparde@broadcom.com; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Zhang, > Helin <helin.zhang@intel.com>; Ananyev, Konstantin > <konstantin.ananyev@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Chen, > Jing D <jing.d.chen@intel.com>; adrien.mazarguil@6wind.com; Richardson, > Bruce <bruce.richardson@intel.com>; yuanhan.liu@linux.intel.com; > maxime.coquelin@redhat.com; stable@dpdk.org > Subject: Re: [PATCH 1/2] ethdev: fix adding invalid MAC addr > > Hi, > > Please see below some comments, > > On Sat, Apr 01, 2017 at 04:03:16PM +0800, Wei Dai wrote: > >[...] > > diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c index > >79efaaa..d61bb27 100644 > > --- a/drivers/net/mlx4/mlx4.c > > +++ b/drivers/net/mlx4/mlx4.c > > @@ -4630,26 +4630,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 > > Please keep function documentation up to date. > > > 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 -1; > > Should not it return ENOSUP instead? > > > (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)) > > - goto end; > > - priv_mac_addr_add(priv, index, > > + if (index >= (elemof(priv->mac) - 1)) { > > + priv_unlock(priv); > > + return -2; > > + } > > + re = priv_mac_addr_add(priv, index, > > (const uint8_t (*)[ETHER_ADDR_LEN]) > > mac_addr->addr_bytes); > > You have an issue here, internal function of mlx drivers return positives errno > (as described in their functions documentation). > It should be negated before returning. > > > end: > > priv_unlock(priv); > > + return re; > > } > > > > /** > > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index > > 879da5e..cca7a36 100644 > > --- a/drivers/net/mlx5/mlx5.h > > +++ b/drivers/net/mlx5/mlx5.h > > @@ -229,8 +229,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 *dev, struct ether_addr > *mac_addr, > > + uint32_t index, uint32_t vmdq); > > 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..db93f4e 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 -1; > > > > (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 = -2; > > goto end; > > - priv_mac_addr_add(priv, index, > > + } > > + re = priv_mac_addr_add(priv, index, > > (const uint8_t (*)[ETHER_ADDR_LEN]) > > mac_addr->addr_bytes); > > end: > > priv_unlock(priv); > > + return re; > > } > >[...] > > Same remarks here, > > Thanks, > > -- > Nélio Laranjeiro > 6WIND ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-04-07 1:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1490979887-27827-1-git-send-email-wei.dai@intel.com>
2017-03-31 17:04 ` [dpdk-stable] [PATCH 1/2] ethdev: fix adding invalid mac addr Wei Dai
[not found] <1491033797-14186-1-git-send-email-wei.dai@intel.com>
2017-04-01 8:03 ` [dpdk-stable] [PATCH 1/2] ethdev: fix adding invalid MAC addr Wei Dai
2017-04-03 9:18 ` Nélio Laranjeiro
2017-04-07 1:38 ` 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).