* [dpdk-dev] [PATCH v5 0/3] MAC address fail to be added shouldn't be stored
@ 2017-04-30 4:11 Wei Dai
2017-04-30 4:11 ` [dpdk-dev] [PATCH v5 1/3] ethdev: fix adding invalid MAC addr Wei Dai
` (4 more replies)
0 siblings, 5 replies; 25+ 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
Current ethdev always stores MAC address even it fails to be added.
Other function may regard the failed MAC address valid and lead to
some errors. So There is a need to check if the addr is added
successfully or not and discard it if it fails.
In 3rd patch, add a command "add_more_mac_addr port_id base_mac_addr count"
to add more than one MAC address one time.
This command can simplify the test for the first patch.
Normally a MAC address may fails to be added only after many MAC
addresses have been added.
Without this command, a tester may only trigger failed MAC address
by running many times of testpmd command 'mac_addr add' .
For v4 patch set, have got acknowledgement from
Nelio Laranjeiro <nelio.laranjeiro@6wind.com> for mlx changes
Yuanhan Liu <yuanhan.liu@linux.intel.com> for virtio changes
---
Changes
v5:
1. rebase master branch
2. add support to drivers/net/ark
3. fix some minor defects
v4:
1. rebase master branch
2. follow code style
v3:
1. Change return value for some specific NIC according to feedbacks
from the community;
2. Add ABI change in release note;
3. Add more detailed commit message.
v2:
fix warnings and erros from check-git-log.sh and checkpatch.pl
Wei Dai (3):
ethdev: fix adding invalid MAC addr
doc: change type of return value of adding MAC addr
app/testpmd: add a command to add many MAC addrs
app/test-pmd/cmdline.c | 55 ++++++++++++++++++++++++++++++++++
doc/guides/rel_notes/release_17_05.rst | 7 +++++
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 +-
23 files changed, 185 insertions(+), 91 deletions(-)
mode change 100644 => 100755 app/test-pmd/cmdline.c
mode change 100644 => 100755 doc/guides/rel_notes/release_17_05.rst
--
2.7.4
^ permalink raw reply [flat|nested] 25+ messages in thread* [dpdk-dev] [PATCH v5 1/3] ethdev: fix adding invalid MAC addr 2017-04-30 4:11 [dpdk-dev] [PATCH v5 0/3] MAC address fail to be added shouldn't be stored Wei Dai @ 2017-04-30 4:11 ` Wei Dai 2017-04-30 4:11 ` [dpdk-dev] [PATCH v5 2/3] doc: change type of return value of adding " Wei Dai ` (3 subsequent siblings) 4 siblings, 0 replies; 25+ 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] 25+ messages in thread
* [dpdk-dev] [PATCH v5 2/3] doc: change type of return value of adding MAC addr 2017-04-30 4:11 [dpdk-dev] [PATCH v5 0/3] MAC address fail to be added shouldn't be stored Wei Dai 2017-04-30 4:11 ` [dpdk-dev] [PATCH v5 1/3] ethdev: fix adding invalid MAC addr Wei Dai @ 2017-04-30 4:11 ` Wei Dai 2017-04-30 4:11 ` [dpdk-dev] [PATCH v5 3/3] app/testpmd: add a command to add many MAC addrs Wei Dai ` (2 subsequent siblings) 4 siblings, 0 replies; 25+ 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 Add following lines in section of API change in release note. If a MAC address fails to be added without this change, it is still stored and may be regarded as a valid one. This may lead to errors in application. The type of return value of eth_mac_addr_add_t in rte_ethdev.h is changed. Any specific NIC also follows this change. Signed-off-by: Wei Dai <wei.dai@intel.com> Acked-by: John McNamara <john.mcnamara@intel.com> --- doc/guides/rel_notes/release_17_05.rst | 7 +++++++ 1 file changed, 7 insertions(+) mode change 100644 => 100755 doc/guides/rel_notes/release_17_05.rst diff --git a/doc/guides/rel_notes/release_17_05.rst b/doc/guides/rel_notes/release_17_05.rst old mode 100644 new mode 100755 index ad20e86..8850fbe --- a/doc/guides/rel_notes/release_17_05.rst +++ b/doc/guides/rel_notes/release_17_05.rst @@ -481,6 +481,13 @@ ABI Changes * The ``rte_cryptodev_info.sym`` structure has new field ``max_nb_sessions_per_qp`` to support drivers which may support limited number of sessions per queue_pair. +* **Return if the MAC address is added successfully or not.** + + If a MAC address fails to be added without this change, it is still stored + and may be regarded as a valid one. This may lead to errors in application. + The type of return value of eth_mac_addr_add_t in rte_ethdev.h is changed. + Any specific NIC also follows this change. + Removed Items ------------- -- 2.7.4 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [dpdk-dev] [PATCH v5 3/3] app/testpmd: add a command to add many MAC addrs 2017-04-30 4:11 [dpdk-dev] [PATCH v5 0/3] MAC address fail to be added shouldn't be stored Wei Dai 2017-04-30 4:11 ` [dpdk-dev] [PATCH v5 1/3] ethdev: fix adding invalid MAC addr Wei Dai 2017-04-30 4:11 ` [dpdk-dev] [PATCH v5 2/3] doc: change type of return value of adding " Wei Dai @ 2017-04-30 4:11 ` Wei Dai 2017-04-30 22:59 ` [dpdk-dev] [PATCH v5 0/3] MAC address fail to be added shouldn't be stored Thomas Monjalon 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 " Wei Dai 4 siblings, 0 replies; 25+ 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 This patch is added to introduce a testpmd command which is used to add more than one MAC addresses one time. This command can simplify the test for the change where the type of return value of adding MAC address. Normally a MAC address may fails to be added only after many MAC addresses have been added. Without this command, a tester may only trigger failed MAC address by running many times of testpmd command 'mac_addr add' . Signed-off-by: Wei Dai <wei.dai@intel.com> --- app/test-pmd/cmdline.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) mode change 100644 => 100755 app/test-pmd/cmdline.c diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c old mode 100644 new mode 100755 index f6bd75b..a65f457 --- a/app/test-pmd/cmdline.c +++ b/app/test-pmd/cmdline.c @@ -6436,6 +6436,60 @@ cmdline_parse_inst_t cmd_mac_addr = { }, }; +/* *** ADD MORE THAN ONE MAC ADDRESS FROM A PORT *** */ +struct cmd_add_more_mac_addr_result { + cmdline_fixed_string_t mac_addr_cmd; + uint8_t port_num; + struct ether_addr address; + uint8_t cnt_addr; +}; + +static void cmd_add_more_mac_addr_parsed(void *parsed_result, + __attribute__((unused)) struct cmdline *cl, + __attribute__((unused)) void *data) +{ + struct cmd_add_more_mac_addr_result *res = parsed_result; + int ret; + int k; + + for (k = 0; k < res->cnt_addr; k++) { + ret = rte_eth_dev_mac_addr_add(res->port_num, &res->address, 0); + if (ret < 0) { + printf("Fail to add mac addr : (%s) after adding %u addresses\n", + strerror(-ret), k); + return; + } + res->address.addr_bytes[5]++; + } + printf("Success to add %u mac addresses\n", k); +} + +cmdline_parse_token_string_t cmd_add_more_mac_addr_cmd = + TOKEN_STRING_INITIALIZER(struct cmd_add_more_mac_addr_result, + mac_addr_cmd, "add_more_mac_addr"); +cmdline_parse_token_num_t cmd_add_more_mac_addr_portnum = + TOKEN_NUM_INITIALIZER(struct cmd_add_more_mac_addr_result, + port_num, UINT8); +cmdline_parse_token_etheraddr_t cmd_add_more_mac_addr_addr = + TOKEN_ETHERADDR_INITIALIZER(struct cmd_add_more_mac_addr_result, + address); +cmdline_parse_token_num_t cmd_add_more_mac_addr_cnt_addr = + TOKEN_NUM_INITIALIZER(struct cmd_add_more_mac_addr_result, + cnt_addr, UINT8); + +cmdline_parse_inst_t cmd_add_more_mac_addr = { + .f = cmd_add_more_mac_addr_parsed, + .data = (void *)0, + .help_str = "add_more_mac_addr <port_id> <mac_addr> <cnt_addr>: " + "Add cnt_addr MAC addresses on port_id", + .tokens = { + (void *)&cmd_add_more_mac_addr_cmd, + (void *)&cmd_add_more_mac_addr_portnum, + (void *)&cmd_add_more_mac_addr_addr, + (void *)&cmd_add_more_mac_addr_cnt_addr, + NULL, + }, +}; /* *** CONFIGURE QUEUE STATS COUNTER MAPPINGS *** */ struct cmd_set_qmap_result { @@ -13612,6 +13666,7 @@ cmdline_parse_ctx_t main_ctx[] = { (cmdline_parse_inst_t *)&cmd_read_rxd_txd, (cmdline_parse_inst_t *)&cmd_stop, (cmdline_parse_inst_t *)&cmd_mac_addr, + (cmdline_parse_inst_t *)&cmd_add_more_mac_addr, (cmdline_parse_inst_t *)&cmd_set_qmap, (cmdline_parse_inst_t *)&cmd_operate_port, (cmdline_parse_inst_t *)&cmd_operate_specific_port, -- 2.7.4 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [dpdk-dev] [PATCH v5 0/3] MAC address fail to be added shouldn't be stored 2017-04-30 4:11 [dpdk-dev] [PATCH v5 0/3] MAC address fail to be added shouldn't be stored Wei Dai ` (2 preceding siblings ...) 2017-04-30 4:11 ` [dpdk-dev] [PATCH v5 3/3] app/testpmd: add a command to add many MAC addrs Wei Dai @ 2017-04-30 22:59 ` Thomas Monjalon 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 " Wei Dai 4 siblings, 0 replies; 25+ messages in thread From: Thomas Monjalon @ 2017-04-30 22:59 UTC (permalink / raw) To: Wei Dai Cc: dev, wenzhuo.lu, 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 30/04/2017 06:11, Wei Dai: > Current ethdev always stores MAC address even it fails to be added. > Other function may regard the failed MAC address valid and lead to > some errors. So There is a need to check if the addr is added > successfully or not and discard it if it fails. > > In 3rd patch, add a command "add_more_mac_addr port_id base_mac_addr count" > to add more than one MAC address one time. > This command can simplify the test for the first patch. > Normally a MAC address may fails to be added only after many MAC > addresses have been added. > Without this command, a tester may only trigger failed MAC address > by running many times of testpmd command 'mac_addr add' . > > For v4 patch set, have got acknowledgement from > Nelio Laranjeiro <nelio.laranjeiro@6wind.com> for mlx changes > Yuanhan Liu <yuanhan.liu@linux.intel.com> for virtio changes You have not taken the ack from Konstantin, and you have again forgot --in-reply-to, so review is hard. Checkpatch reports: ERROR:EXECUTE_PERMISSIONS: do not set execute permissions for source files Please make a perfect v6. ^ permalink raw reply [flat|nested] 25+ messages in thread
* [dpdk-dev] [PATCH v6 0/3] MAC address fail to be added shouldn't be stored 2017-04-30 4:11 [dpdk-dev] [PATCH v5 0/3] MAC address fail to be added shouldn't be stored Wei Dai ` (3 preceding siblings ...) 2017-04-30 22:59 ` [dpdk-dev] [PATCH v5 0/3] MAC address fail to be added shouldn't be stored Thomas Monjalon @ 2017-05-02 12:44 ` Wei Dai 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 1/3] ethdev: fix adding invalid MAC addr Wei Dai ` (3 more replies) 4 siblings, 4 replies; 25+ 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 Current ethdev always stores MAC address even it fails to be added. Other function may regard the failed MAC address valid and lead to some errors. So There is a need to check if the addr is added successfully or not and discard it if it fails. In 3rd patch, add a command "add_more_mac_addr port_id base_mac_addr count" to add more than one MAC address one time. This command can simplify the test for the first patch. Normally a MAC address may fails to be added only after many MAC addresses have been added. Without this command, a tester may only trigger failed MAC address by running many times of testpmd command 'mac_addr add' . For v4 patch set, have got acknowledgements from Nelio Laranjeiro <nelio.laranjeiro@6wind.com> for mlx changes Yuanhan Liu <yuanhan.liu@linux.intel.com> for virtio changes --- Changes v6: 1. rebase master branch to v17.05-rc3 2. not touch e1000 base driver code 3. fix some minor defects v5: 1. rebase master branch 2. add support to drivers/net/ark 3. fix some minor defects v4: 1. rebase master branch 2. follow code style v3: 1. Change return value for some specific NIC according to feedbacks from the community; 2. Add ABI change in release note; 3. Add more detailed commit message. v2: fix warnings and erros from check-git-log.sh and checkpatch.pl Wei Dai (3): ethdev: fix adding invalid MAC addr doc: change type of return value of adding MAC addr app/testpmd: add a command to add many MAC addrs app/test-pmd/cmdline.c | 55 ++++++++++++++++++++++++++++++++++ doc/guides/rel_notes/release_17_05.rst | 7 +++++ 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 +- 22 files changed, 184 insertions(+), 90 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [dpdk-dev] [PATCH v6 1/3] ethdev: fix adding invalid MAC addr 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 " Wei Dai @ 2017-05-02 12:44 ` Wei Dai 2017-05-03 0:57 ` Lu, Wenzhuo 2017-05-03 1:46 ` Yuanhan Liu 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 2/3] doc: change type of return value of adding " Wei Dai ` (2 subsequent siblings) 3 siblings, 2 replies; 25+ 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] 25+ messages in thread
* Re: [dpdk-dev] [PATCH v6 1/3] ethdev: fix adding invalid MAC addr 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 1/3] ethdev: fix adding invalid MAC addr Wei Dai @ 2017-05-03 0:57 ` Lu, Wenzhuo 2017-05-03 1:46 ` Yuanhan Liu 1 sibling, 0 replies; 25+ 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] 25+ messages in thread
* Re: [dpdk-dev] [PATCH v6 1/3] ethdev: fix adding invalid MAC addr 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 1/3] ethdev: fix adding invalid MAC addr 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; 25+ 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] 25+ messages in thread
* Re: [dpdk-dev] [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; 25+ 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] 25+ messages in thread
* [dpdk-dev] [PATCH v6 2/3] doc: change type of return value of adding MAC addr 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 " Wei Dai 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 1/3] ethdev: fix adding invalid MAC addr Wei Dai @ 2017-05-02 12:44 ` Wei Dai 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 3/3] app/testpmd: add a command to add many MAC addrs Wei Dai 2017-05-05 0:39 ` [dpdk-dev] [PATCH v7 0/3] MAC address fail to be added shouldn't be stored Wei Dai 3 siblings, 0 replies; 25+ 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 Add following lines in section of API change in release note. If a MAC address fails to be added without this change, it is still stored and may be regarded as a valid one. This may lead to errors in application. The type of return value of eth_mac_addr_add_t in rte_ethdev.h is changed. Any specific NIC also follows this change. Signed-off-by: Wei Dai <wei.dai@intel.com> Acked-by: John McNamara <john.mcnamara@intel.com> --- doc/guides/rel_notes/release_17_05.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/guides/rel_notes/release_17_05.rst b/doc/guides/rel_notes/release_17_05.rst index 4b47ae1..0bd07f1 100644 --- a/doc/guides/rel_notes/release_17_05.rst +++ b/doc/guides/rel_notes/release_17_05.rst @@ -489,6 +489,13 @@ ABI Changes * The ``rte_cryptodev_info.sym`` structure has new field ``max_nb_sessions_per_qp`` to support drivers which may support limited number of sessions per queue_pair. +* **Return if the MAC address is added successfully or not.** + + If a MAC address fails to be added without this change, it is still stored + and may be regarded as a valid one. This may lead to errors in application. + The type of return value of eth_mac_addr_add_t in rte_ethdev.h is changed. + Any specific NIC also follows this change. + Removed Items ------------- -- 2.7.4 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [dpdk-dev] [PATCH v6 3/3] app/testpmd: add a command to add many MAC addrs 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 " Wei Dai 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 1/3] ethdev: fix adding invalid MAC addr Wei Dai 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 2/3] doc: change type of return value of adding " Wei Dai @ 2017-05-02 12:44 ` Wei Dai 2017-05-05 0:39 ` [dpdk-dev] [PATCH v7 0/3] MAC address fail to be added shouldn't be stored Wei Dai 3 siblings, 0 replies; 25+ 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 This patch is added to introduce a testpmd command which is used to add more than one MAC addresses one time. This command can simplify the test for the change where the type of return value of adding MAC address. Normally a MAC address may fails to be added only after many MAC addresses have been added. Without this command, a tester may only trigger failed MAC address by running many times of testpmd command 'mac_addr add' . Signed-off-by: Wei Dai <wei.dai@intel.com> --- app/test-pmd/cmdline.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index 06c1ce2..f73bb83 100644 --- a/app/test-pmd/cmdline.c +++ b/app/test-pmd/cmdline.c @@ -6436,6 +6436,60 @@ cmdline_parse_inst_t cmd_mac_addr = { }, }; +/* *** ADD MORE THAN ONE MAC ADDRESS FROM A PORT *** */ +struct cmd_add_more_mac_addr_result { + cmdline_fixed_string_t mac_addr_cmd; + uint8_t port_num; + struct ether_addr address; + uint8_t cnt_addr; +}; + +static void cmd_add_more_mac_addr_parsed(void *parsed_result, + __attribute__((unused)) struct cmdline *cl, + __attribute__((unused)) void *data) +{ + struct cmd_add_more_mac_addr_result *res = parsed_result; + int ret; + int k; + + for (k = 0; k < res->cnt_addr; k++) { + ret = rte_eth_dev_mac_addr_add(res->port_num, &res->address, 0); + if (ret < 0) { + printf("Fail to add mac addr : (%s) after adding %u addresses\n", + strerror(-ret), k); + return; + } + res->address.addr_bytes[5]++; + } + printf("Success to add %u mac addresses\n", k); +} + +cmdline_parse_token_string_t cmd_add_more_mac_addr_cmd = + TOKEN_STRING_INITIALIZER(struct cmd_add_more_mac_addr_result, + mac_addr_cmd, "add_more_mac_addr"); +cmdline_parse_token_num_t cmd_add_more_mac_addr_portnum = + TOKEN_NUM_INITIALIZER(struct cmd_add_more_mac_addr_result, + port_num, UINT8); +cmdline_parse_token_etheraddr_t cmd_add_more_mac_addr_addr = + TOKEN_ETHERADDR_INITIALIZER(struct cmd_add_more_mac_addr_result, + address); +cmdline_parse_token_num_t cmd_add_more_mac_addr_cnt_addr = + TOKEN_NUM_INITIALIZER(struct cmd_add_more_mac_addr_result, + cnt_addr, UINT8); + +cmdline_parse_inst_t cmd_add_more_mac_addr = { + .f = cmd_add_more_mac_addr_parsed, + .data = (void *)0, + .help_str = "add_more_mac_addr <port_id> <mac_addr> <cnt_addr>: " + "Add cnt_addr MAC addresses on port_id", + .tokens = { + (void *)&cmd_add_more_mac_addr_cmd, + (void *)&cmd_add_more_mac_addr_portnum, + (void *)&cmd_add_more_mac_addr_addr, + (void *)&cmd_add_more_mac_addr_cnt_addr, + NULL, + }, +}; /* *** CONFIGURE QUEUE STATS COUNTER MAPPINGS *** */ struct cmd_set_qmap_result { @@ -13647,6 +13701,7 @@ cmdline_parse_ctx_t main_ctx[] = { (cmdline_parse_inst_t *)&cmd_read_rxd_txd, (cmdline_parse_inst_t *)&cmd_stop, (cmdline_parse_inst_t *)&cmd_mac_addr, + (cmdline_parse_inst_t *)&cmd_add_more_mac_addr, (cmdline_parse_inst_t *)&cmd_set_qmap, (cmdline_parse_inst_t *)&cmd_operate_port, (cmdline_parse_inst_t *)&cmd_operate_specific_port, -- 2.7.4 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [dpdk-dev] [PATCH v7 0/3] MAC address fail to be added shouldn't be stored 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 " Wei Dai ` (2 preceding siblings ...) 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 3/3] app/testpmd: add a command to add many MAC addrs Wei Dai @ 2017-05-05 0:39 ` Wei Dai 2017-05-05 0:40 ` [dpdk-dev] [PATCH v7 1/3] ethdev: fix adding invalid MAC addr Wei Dai ` (2 more replies) 3 siblings, 3 replies; 25+ messages in thread From: Wei Dai @ 2017-05-05 0:39 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 Current ethdev always stores the MAC address even it fails to be added. Other function may regard the failed MAC address valid and lead to some errors. So There is a need to check if the addr is added successfully or not and discard it if it fails. In 3rd patch, add a command "add_more_mac_addr port_id base_mac_addr count" to add more than one MAC address one time. This command can simplify the test for the first patch. Normally a MAC address may fails to be added only after many MAC addresses have been added. Without this command, a tester may only trigger failed MAC address by running many times of testpmd command 'mac_addr add' . This patch set has got acknowledgements from Nelio Laranjeiro <nelio.laranjeiro@6wind.com> for mlx changes Yuanhan Liu <yuanhan.liu@linux.intel.com> for virtio changes Wenzhuo Lu <wenzhuo.lu@intel.com> for igb, e1000 and ixgbe changes --- Changes v7: 1. remove "Cc: stable@dpdk.org" in patch 1/3 2. add "Acked by: Wenzhuo.Lu <wenzhuo.lu@intel.com>" in patch 1/3 v6: 1. rebase master branch to v17.05-rc3 2. not touch e1000 base driver code 3. fix some minor defects v5: 1. rebase master branch 2. add support to drivers/net/ark 3. fix some minor defects v4: 1. rebase master branch 2. follow code style v3: 1. Change return value for some specific NIC according to feedbacks from the community; 2. Add ABI change in release note; 3. Add more detailed commit message. v2: fix warnings and erros from check-git-log.sh and checkpatch.pl Wei Dai (3): ethdev: fix adding invalid MAC addr doc: change type of return value of adding MAC addr app/testpmd: add a command to add many MAC addrs app/test-pmd/cmdline.c | 55 ++++++++++++++++++++++++++++++++++ doc/guides/rel_notes/release_17_05.rst | 7 +++++ 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 +- 22 files changed, 184 insertions(+), 90 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [dpdk-dev] [PATCH v7 1/3] ethdev: fix adding invalid MAC addr 2017-05-05 0:39 ` [dpdk-dev] [PATCH v7 0/3] MAC address fail to be added shouldn't be stored Wei Dai @ 2017-05-05 0:40 ` Wei Dai 2017-05-05 1:46 ` Yang, Qiming ` (2 more replies) 2017-05-05 0:40 ` [dpdk-dev] [PATCH v7 2/3] doc: change type of return value of adding " Wei Dai 2017-05-05 0:40 ` [dpdk-dev] [PATCH v7 3/3] app/testpmd: add a command to add many MAC addrs Wei Dai 2 siblings, 3 replies; 25+ messages in thread From: Wei Dai @ 2017-05-05 0:40 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 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. This patch changes the ethdev API, it should not be backported to a stable/LTS release so far. Fixes: af75078fece3 ("first public release") 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> --- 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] 25+ messages in thread
* Re: [dpdk-dev] [PATCH v7 1/3] ethdev: fix adding invalid MAC addr 2017-05-05 0:40 ` [dpdk-dev] [PATCH v7 1/3] ethdev: fix adding invalid MAC addr Wei Dai @ 2017-05-05 1:46 ` Yang, Qiming 2017-05-05 14:07 ` Thomas Monjalon 2017-05-05 14:21 ` Thomas Monjalon 2017-05-05 14:24 ` Thomas Monjalon 2 siblings, 1 reply; 25+ messages in thread From: Yang, Qiming @ 2017-05-05 1:46 UTC (permalink / raw) To: Dai, Wei; +Cc: dev Hi, Dai wei > 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; > } What's the meaning to add a return here? Return 0 can't represent adding an invalid or valid address, it's meaningless. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [dpdk-dev] [PATCH v7 1/3] ethdev: fix adding invalid MAC addr 2017-05-05 1:46 ` Yang, Qiming @ 2017-05-05 14:07 ` Thomas Monjalon 0 siblings, 0 replies; 25+ messages in thread From: Thomas Monjalon @ 2017-05-05 14:07 UTC (permalink / raw) To: Yang, Qiming; +Cc: dev, Dai, Wei 05/05/2017 03:46, Yang, Qiming: > Hi, Dai wei > > > 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; > > } > What's the meaning to add a return here? Return 0 can't represent adding an invalid or valid address, it's meaningless. Because drivers/net/e1000/igb_ethdev.c: .mac_addr_add = eth_igb_rar_set, and .mac_addr_add is changed to return an error code. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [dpdk-dev] [PATCH v7 1/3] ethdev: fix adding invalid MAC addr 2017-05-05 0:40 ` [dpdk-dev] [PATCH v7 1/3] ethdev: fix adding invalid MAC addr Wei Dai 2017-05-05 1:46 ` Yang, Qiming @ 2017-05-05 14:21 ` Thomas Monjalon 2017-05-07 4:39 ` Yuanhan Liu 2017-05-05 14:24 ` Thomas Monjalon 2 siblings, 1 reply; 25+ messages in thread From: Thomas Monjalon @ 2017-05-05 14:21 UTC (permalink / raw) To: Wei Dai Cc: dev, wenzhuo.lu, 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 05/05/2017 02:40, Wei Dai: > 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. > > This patch changes the ethdev API, it should not be > backported to a stable/LTS release so far. It changes only the internal function pointer used by drivers, not the API. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [dpdk-dev] [PATCH v7 1/3] ethdev: fix adding invalid MAC addr 2017-05-05 14:21 ` Thomas Monjalon @ 2017-05-07 4:39 ` Yuanhan Liu 0 siblings, 0 replies; 25+ messages in thread From: Yuanhan Liu @ 2017-05-07 4:39 UTC (permalink / raw) To: Thomas Monjalon Cc: Wei Dai, dev, wenzhuo.lu, 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 On Fri, May 05, 2017 at 04:21:53PM +0200, Thomas Monjalon wrote: > 05/05/2017 02:40, Wei Dai: > > 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. > > > > This patch changes the ethdev API, it should not be > > backported to a stable/LTS release so far. > > It changes only the internal function pointer used by drivers, > not the API. Oh, right. I was thinking the eth API rte_eth_dev_mac_addr also has to be changed. It turned out that it's already been designed with returning "int". Sorry for the noise. --yliu ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [dpdk-dev] [PATCH v7 1/3] ethdev: fix adding invalid MAC addr 2017-05-05 0:40 ` [dpdk-dev] [PATCH v7 1/3] ethdev: fix adding invalid MAC addr Wei Dai 2017-05-05 1:46 ` Yang, Qiming 2017-05-05 14:21 ` Thomas Monjalon @ 2017-05-05 14:24 ` Thomas Monjalon 2 siblings, 0 replies; 25+ messages in thread From: Thomas Monjalon @ 2017-05-05 14:24 UTC (permalink / raw) To: Wei Dai Cc: dev, wenzhuo.lu, 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 05/05/2017 02:40, Wei Dai: > 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. > > This patch changes the ethdev API, it should not be > backported to a stable/LTS release so far. > > Fixes: af75078fece3 ("first public release") > > 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> Applied without the rest of the series, thanks ^ permalink raw reply [flat|nested] 25+ messages in thread
* [dpdk-dev] [PATCH v7 2/3] doc: change type of return value of adding MAC addr 2017-05-05 0:39 ` [dpdk-dev] [PATCH v7 0/3] MAC address fail to be added shouldn't be stored Wei Dai 2017-05-05 0:40 ` [dpdk-dev] [PATCH v7 1/3] ethdev: fix adding invalid MAC addr Wei Dai @ 2017-05-05 0:40 ` Wei Dai 2017-05-05 14:23 ` Thomas Monjalon 2017-05-05 0:40 ` [dpdk-dev] [PATCH v7 3/3] app/testpmd: add a command to add many MAC addrs Wei Dai 2 siblings, 1 reply; 25+ messages in thread From: Wei Dai @ 2017-05-05 0:40 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 Add following lines in section of API change in release note. If a MAC address fails to be added without this change, it is still stored and may be regarded as a valid one. This may lead to errors in application. The type of return value of eth_mac_addr_add_t in rte_ethdev.h is changed. Any specific NIC also follows this change. Signed-off-by: Wei Dai <wei.dai@intel.com> Acked-by: John McNamara <john.mcnamara@intel.com> --- doc/guides/rel_notes/release_17_05.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/guides/rel_notes/release_17_05.rst b/doc/guides/rel_notes/release_17_05.rst index 4b47ae1..0bd07f1 100644 --- a/doc/guides/rel_notes/release_17_05.rst +++ b/doc/guides/rel_notes/release_17_05.rst @@ -489,6 +489,13 @@ ABI Changes * The ``rte_cryptodev_info.sym`` structure has new field ``max_nb_sessions_per_qp`` to support drivers which may support limited number of sessions per queue_pair. +* **Return if the MAC address is added successfully or not.** + + If a MAC address fails to be added without this change, it is still stored + and may be regarded as a valid one. This may lead to errors in application. + The type of return value of eth_mac_addr_add_t in rte_ethdev.h is changed. + Any specific NIC also follows this change. + Removed Items ------------- -- 2.7.4 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [dpdk-dev] [PATCH v7 2/3] doc: change type of return value of adding MAC addr 2017-05-05 0:40 ` [dpdk-dev] [PATCH v7 2/3] doc: change type of return value of adding " Wei Dai @ 2017-05-05 14:23 ` Thomas Monjalon 0 siblings, 0 replies; 25+ messages in thread From: Thomas Monjalon @ 2017-05-05 14:23 UTC (permalink / raw) To: Wei Dai Cc: dev, wenzhuo.lu, 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 05/05/2017 02:40, Wei Dai: > Add following lines in section of API change in release note. > > If a MAC address fails to be added without this change, it is still > stored and may be regarded as a valid one. This may lead to errors > in application. The type of return value of eth_mac_addr_add_t in > rte_ethdev.h is changed. Any specific NIC also follows this change. > > Signed-off-by: Wei Dai <wei.dai@intel.com> > Acked-by: John McNamara <john.mcnamara@intel.com> This patch is rejected because it is neither an API nor an ABI change. ^ permalink raw reply [flat|nested] 25+ messages in thread
* [dpdk-dev] [PATCH v7 3/3] app/testpmd: add a command to add many MAC addrs 2017-05-05 0:39 ` [dpdk-dev] [PATCH v7 0/3] MAC address fail to be added shouldn't be stored Wei Dai 2017-05-05 0:40 ` [dpdk-dev] [PATCH v7 1/3] ethdev: fix adding invalid MAC addr Wei Dai 2017-05-05 0:40 ` [dpdk-dev] [PATCH v7 2/3] doc: change type of return value of adding " Wei Dai @ 2017-05-05 0:40 ` Wei Dai 2017-05-05 14:24 ` Thomas Monjalon 2 siblings, 1 reply; 25+ messages in thread From: Wei Dai @ 2017-05-05 0:40 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 This patch is added to introduce a testpmd command which is used to add more than one MAC addresses one time. This command can simplify the test for the change where the type of return value of adding MAC address. Normally a MAC address may fails to be added only after many MAC addresses have been added. Without this command, a tester may only trigger failed MAC address by running many times of testpmd command 'mac_addr add' . Signed-off-by: Wei Dai <wei.dai@intel.com> --- app/test-pmd/cmdline.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index 06c1ce2..f73bb83 100644 --- a/app/test-pmd/cmdline.c +++ b/app/test-pmd/cmdline.c @@ -6436,6 +6436,60 @@ cmdline_parse_inst_t cmd_mac_addr = { }, }; +/* *** ADD MORE THAN ONE MAC ADDRESS FROM A PORT *** */ +struct cmd_add_more_mac_addr_result { + cmdline_fixed_string_t mac_addr_cmd; + uint8_t port_num; + struct ether_addr address; + uint8_t cnt_addr; +}; + +static void cmd_add_more_mac_addr_parsed(void *parsed_result, + __attribute__((unused)) struct cmdline *cl, + __attribute__((unused)) void *data) +{ + struct cmd_add_more_mac_addr_result *res = parsed_result; + int ret; + int k; + + for (k = 0; k < res->cnt_addr; k++) { + ret = rte_eth_dev_mac_addr_add(res->port_num, &res->address, 0); + if (ret < 0) { + printf("Fail to add mac addr : (%s) after adding %u addresses\n", + strerror(-ret), k); + return; + } + res->address.addr_bytes[5]++; + } + printf("Success to add %u mac addresses\n", k); +} + +cmdline_parse_token_string_t cmd_add_more_mac_addr_cmd = + TOKEN_STRING_INITIALIZER(struct cmd_add_more_mac_addr_result, + mac_addr_cmd, "add_more_mac_addr"); +cmdline_parse_token_num_t cmd_add_more_mac_addr_portnum = + TOKEN_NUM_INITIALIZER(struct cmd_add_more_mac_addr_result, + port_num, UINT8); +cmdline_parse_token_etheraddr_t cmd_add_more_mac_addr_addr = + TOKEN_ETHERADDR_INITIALIZER(struct cmd_add_more_mac_addr_result, + address); +cmdline_parse_token_num_t cmd_add_more_mac_addr_cnt_addr = + TOKEN_NUM_INITIALIZER(struct cmd_add_more_mac_addr_result, + cnt_addr, UINT8); + +cmdline_parse_inst_t cmd_add_more_mac_addr = { + .f = cmd_add_more_mac_addr_parsed, + .data = (void *)0, + .help_str = "add_more_mac_addr <port_id> <mac_addr> <cnt_addr>: " + "Add cnt_addr MAC addresses on port_id", + .tokens = { + (void *)&cmd_add_more_mac_addr_cmd, + (void *)&cmd_add_more_mac_addr_portnum, + (void *)&cmd_add_more_mac_addr_addr, + (void *)&cmd_add_more_mac_addr_cnt_addr, + NULL, + }, +}; /* *** CONFIGURE QUEUE STATS COUNTER MAPPINGS *** */ struct cmd_set_qmap_result { @@ -13647,6 +13701,7 @@ cmdline_parse_ctx_t main_ctx[] = { (cmdline_parse_inst_t *)&cmd_read_rxd_txd, (cmdline_parse_inst_t *)&cmd_stop, (cmdline_parse_inst_t *)&cmd_mac_addr, + (cmdline_parse_inst_t *)&cmd_add_more_mac_addr, (cmdline_parse_inst_t *)&cmd_set_qmap, (cmdline_parse_inst_t *)&cmd_operate_port, (cmdline_parse_inst_t *)&cmd_operate_specific_port, -- 2.7.4 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [dpdk-dev] [PATCH v7 3/3] app/testpmd: add a command to add many MAC addrs 2017-05-05 0:40 ` [dpdk-dev] [PATCH v7 3/3] app/testpmd: add a command to add many MAC addrs Wei Dai @ 2017-05-05 14:24 ` Thomas Monjalon 2017-05-08 1:01 ` Wu, Jingjing 0 siblings, 1 reply; 25+ messages in thread From: Thomas Monjalon @ 2017-05-05 14:24 UTC (permalink / raw) To: Wei Dai Cc: dev, wenzhuo.lu, 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 05/05/2017 02:40, Wei Dai: > This patch is added to introduce a testpmd command which > is used to add more than one MAC addresses one time. > This command can simplify the test for the change where > the type of return value of adding MAC address. > Normally a MAC address may fails to be added only after > many MAC addresses have been added. > Without this command, a tester may only trigger failed > MAC address by running many times of testpmd command > 'mac_addr add' . > > Signed-off-by: Wei Dai <wei.dai@intel.com> > --- > app/test-pmd/cmdline.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 55 insertions(+) This patch has not been reviewed. It is not a fix and I am not sure it is needed. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [dpdk-dev] [PATCH v7 3/3] app/testpmd: add a command to add many MAC addrs 2017-05-05 14:24 ` Thomas Monjalon @ 2017-05-08 1:01 ` Wu, Jingjing 2017-05-08 6:17 ` Wu, Jingjing 0 siblings, 1 reply; 25+ messages in thread From: Wu, Jingjing @ 2017-05-08 1:01 UTC (permalink / raw) To: Thomas Monjalon, Dai, Wei Cc: dev, Lu, Wenzhuo, harish.patil, rasesh.mody, stephen.hurd, ajit.khaparde, Zhang, Helin, Ananyev, Konstantin, Chen, Jing D, adrien.mazarguil, nelio.laranjeiro, Richardson, Bruce, yuanhan.liu, maxime.coquelin, shepard.siegel, ed.czeck, john.miller > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Friday, May 5, 2017 10:24 PM > To: Dai, Wei <wei.dai@intel.com> > Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; > harish.patil@cavium.com; rasesh.mody@cavium.com; > stephen.hurd@broadcom.com; ajit.khaparde@broadcom.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; > nelio.laranjeiro@6wind.com; Richardson, Bruce <bruce.richardson@intel.com>; > yuanhan.liu@linux.intel.com; maxime.coquelin@redhat.com; > shepard.siegel@atomicrules.com; ed.czeck@atomicrules.com; > john.miller@atomicrules.com > Subject: Re: [dpdk-dev] [PATCH v7 3/3] app/testpmd: add a command to add > many MAC addrs > > 05/05/2017 02:40, Wei Dai: > > This patch is added to introduce a testpmd command which is used to > > add more than one MAC addresses one time. > > This command can simplify the test for the change where the type of > > return value of adding MAC address. > > Normally a MAC address may fails to be added only after many MAC > > addresses have been added. > > Without this command, a tester may only trigger failed MAC address by > > running many times of testpmd command 'mac_addr add' . > > > > Signed-off-by: Wei Dai <wei.dai@intel.com> > > --- > > app/test-pmd/cmdline.c | 55 > > ++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 55 insertions(+) > > This patch has not been reviewed. > It is not a fix and I am not sure it is needed. The same doubt, what is this relationship with your fix in Patch 1/3? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [dpdk-dev] [PATCH v7 3/3] app/testpmd: add a command to add many MAC addrs 2017-05-08 1:01 ` Wu, Jingjing @ 2017-05-08 6:17 ` Wu, Jingjing 0 siblings, 0 replies; 25+ messages in thread From: Wu, Jingjing @ 2017-05-08 6:17 UTC (permalink / raw) To: Wu, Jingjing, Thomas Monjalon, Dai, Wei Cc: dev, Lu, Wenzhuo, harish.patil, rasesh.mody, stephen.hurd, ajit.khaparde, Zhang, Helin, Ananyev, Konstantin, Chen, Jing D, adrien.mazarguil, nelio.laranjeiro, Richardson, Bruce, yuanhan.liu, maxime.coquelin, shepard.siegel, ed.czeck, john.miller > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wu, Jingjing > Sent: Monday, May 8, 2017 9:02 AM > To: Thomas Monjalon <thomas@monjalon.net>; Dai, Wei <wei.dai@intel.com> > Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; > harish.patil@cavium.com; rasesh.mody@cavium.com; > stephen.hurd@broadcom.com; ajit.khaparde@broadcom.com; Zhang, Helin > <helin.zhang@intel.com>; Ananyev, Konstantin > <konstantin.ananyev@intel.com>; Chen, Jing D <jing.d.chen@intel.com>; > adrien.mazarguil@6wind.com; nelio.laranjeiro@6wind.com; Richardson, Bruce > <bruce.richardson@intel.com>; yuanhan.liu@linux.intel.com; > maxime.coquelin@redhat.com; shepard.siegel@atomicrules.com; > ed.czeck@atomicrules.com; john.miller@atomicrules.com > Subject: Re: [dpdk-dev] [PATCH v7 3/3] app/testpmd: add a command to add > many MAC addrs > > > > > -----Original Message----- > > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > Sent: Friday, May 5, 2017 10:24 PM > > To: Dai, Wei <wei.dai@intel.com> > > Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; > > harish.patil@cavium.com; rasesh.mody@cavium.com; > > stephen.hurd@broadcom.com; ajit.khaparde@broadcom.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; > > nelio.laranjeiro@6wind.com; Richardson, Bruce > > <bruce.richardson@intel.com>; yuanhan.liu@linux.intel.com; > > maxime.coquelin@redhat.com; shepard.siegel@atomicrules.com; > > ed.czeck@atomicrules.com; john.miller@atomicrules.com > > Subject: Re: [dpdk-dev] [PATCH v7 3/3] app/testpmd: add a command to > > add many MAC addrs > > > > 05/05/2017 02:40, Wei Dai: > > > This patch is added to introduce a testpmd command which is used to > > > add more than one MAC addresses one time. > > > This command can simplify the test for the change where the type of > > > return value of adding MAC address. > > > Normally a MAC address may fails to be added only after many MAC > > > addresses have been added. > > > Without this command, a tester may only trigger failed MAC address > > > by running many times of testpmd command 'mac_addr add' . > > > > > > Signed-off-by: Wei Dai <wei.dai@intel.com> > > > --- > > > app/test-pmd/cmdline.c | 55 > > > ++++++++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 55 insertions(+) > > > > This patch has not been reviewed. > > It is not a fix and I am not sure it is needed. > > The same doubt, what is this relationship with your fix in Patch 1/3? > If it is only for testing for special case, and it can be done through calling 'mac_addr add' by multi times. I'd like to Nack it. Thanks Jingjing ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2017-05-08 6:17 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-30 4:11 [dpdk-dev] [PATCH v5 0/3] MAC address fail to be added shouldn't be stored Wei Dai 2017-04-30 4:11 ` [dpdk-dev] [PATCH v5 1/3] ethdev: fix adding invalid MAC addr Wei Dai 2017-04-30 4:11 ` [dpdk-dev] [PATCH v5 2/3] doc: change type of return value of adding " Wei Dai 2017-04-30 4:11 ` [dpdk-dev] [PATCH v5 3/3] app/testpmd: add a command to add many MAC addrs Wei Dai 2017-04-30 22:59 ` [dpdk-dev] [PATCH v5 0/3] MAC address fail to be added shouldn't be stored Thomas Monjalon 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 " Wei Dai 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 1/3] ethdev: fix adding invalid MAC addr Wei Dai 2017-05-03 0:57 ` Lu, Wenzhuo 2017-05-03 1:46 ` Yuanhan Liu 2017-05-05 0:16 ` Dai, Wei 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 2/3] doc: change type of return value of adding " Wei Dai 2017-05-02 12:44 ` [dpdk-dev] [PATCH v6 3/3] app/testpmd: add a command to add many MAC addrs Wei Dai 2017-05-05 0:39 ` [dpdk-dev] [PATCH v7 0/3] MAC address fail to be added shouldn't be stored Wei Dai 2017-05-05 0:40 ` [dpdk-dev] [PATCH v7 1/3] ethdev: fix adding invalid MAC addr Wei Dai 2017-05-05 1:46 ` Yang, Qiming 2017-05-05 14:07 ` Thomas Monjalon 2017-05-05 14:21 ` Thomas Monjalon 2017-05-07 4:39 ` Yuanhan Liu 2017-05-05 14:24 ` Thomas Monjalon 2017-05-05 0:40 ` [dpdk-dev] [PATCH v7 2/3] doc: change type of return value of adding " Wei Dai 2017-05-05 14:23 ` Thomas Monjalon 2017-05-05 0:40 ` [dpdk-dev] [PATCH v7 3/3] app/testpmd: add a command to add many MAC addrs Wei Dai 2017-05-05 14:24 ` Thomas Monjalon 2017-05-08 1:01 ` Wu, Jingjing 2017-05-08 6:17 ` Wu, Jingjing
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).