[dpdk-dev] [PATCH v2] ethdev: fix MAC address replay

Igor Ryzhov iryzhov at nfware.com
Mon Jan 23 09:50:23 CET 2017


Hello Steve,

Thank you for all the fixes.
But I think I noticed another one issue in MAC replay process.

Pool number is extracted only once:

if (RTE_ETH_DEV_SRIOV(dev).active)
pool = RTE_ETH_DEV_SRIOV(dev).def_vmdq_idx;

But when MAC address is added using rte_eth_dev_mac_addr_add several
different pool numbers can be used.
Shouldn't we extract pool number for each MAC address separately from
mac_pool_sel array during restoration process?

Best regards,
Igor

On Sat, Jan 21, 2017 at 1:23 AM, Steve Shin <jonshin at cisco.com> wrote:

> This patch fixes a bug in replaying MAC address to the hardware
> in rte_eth_dev_config_restore() routine. Added default MAC replay as well.
>
> Fixes: 4bdefaade6d1 ("ethdev: VMDQ enhancements")
>
> ---
> v2: Added default MAC replay & Code optimization
>
> Signed-off-by: Steve Shin <jonshin at cisco.com>
> ---
>  lib/librte_ether/rte_ethdev.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 4790faf..150f350 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -931,7 +931,7 @@ rte_eth_dev_config_restore(uint8_t port_id)
>  {
>         struct rte_eth_dev *dev;
>         struct rte_eth_dev_info dev_info;
> -       struct ether_addr addr;
> +       struct ether_addr *addr;
>         uint16_t i;
>         uint32_t pool = 0;
>
> @@ -942,23 +942,23 @@ rte_eth_dev_config_restore(uint8_t port_id)
>         if (RTE_ETH_DEV_SRIOV(dev).active)
>                 pool = RTE_ETH_DEV_SRIOV(dev).def_vmdq_idx;
>
> -       /* replay MAC address configuration */
> -       for (i = 0; i < dev_info.max_mac_addrs; i++) {
> -               addr = dev->data->mac_addrs[i];
> +       /* replay MAC address configuration including default MAC */
> +       if (*dev->dev_ops->mac_addr_set != NULL) {
> +               addr = &dev->data->mac_addrs[0];
> +               (*dev->dev_ops->mac_addr_set)(dev, addr);
> +       }
>
> -               /* skip zero address */
> -               if (is_zero_ether_addr(&addr))
> -                       continue;
> +       if (*dev->dev_ops->mac_addr_add != NULL) {
> +               for (i = 1; i < dev_info.max_mac_addrs; i++) {
> +                       addr = &dev->data->mac_addrs[i];
>
> -               /* add address to the hardware */
> -               if  (*dev->dev_ops->mac_addr_add &&
> -                       (dev->data->mac_pool_sel[i] & (1ULL << pool)))
> -                       (*dev->dev_ops->mac_addr_add)(dev, &addr, i,
> pool);
> -               else {
> -                       RTE_PMD_DEBUG_TRACE("port %d: MAC address array
> not supported\n",
> -                                       port_id);
> -                       /* exit the loop but not return an error */
> -                       break;
> +                       /* skip zero address */
> +                       if (is_zero_ether_addr(addr))
> +                               continue;
> +
> +                       /* add address to the hardware */
> +                       if (dev->data->mac_pool_sel[i] & (1ULL << pool))
> +                               (*dev->dev_ops->mac_addr_add)(dev, addr,
> i, pool);
>                 }
>         }
>
> --
> 2.9.3
>
>


More information about the dev mailing list