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

["Steve Shin (jonshin)" <jonshin@cisco.com>]

Dear Igor,

Yes, you’re right. We need to handle a case (ex, SR-IOV) where multiple pools exist in the mac_pool_sel array.
A new diff file will be uploaded with PATCH v3.

Thanks & Regards,
Steve

From: Igor Ryzhov <iryzhov@nfware.com>
Date: Monday, January 23, 2017 at 12:50 AM
To: Steve Shin <jonshin@cisco.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Ferruh Yigit <ferruh.yigit@intel.com>
Subject: Re: [PATCH v2] ethdev: fix MAC address replay

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@cisco.com<mailto:jonshin@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@cisco.com<mailto:jonshin@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