Re: [dpdk-dev] [PATCH 4/7] ethdev: change allmulticast callbacks to return status

[Andrew Rybchenko]

Hi Hyong,

On 9/16/19 10:03 AM, Hyong Youb Kim (hyonkim) wrote:

-----Original Message-----
From: Andrew Rybchenko <arybche...@solarflare.com>
Sent: Monday, September 9, 2019 9:13 PM

[...]

Subject: [PATCH 4/7] ethdev: change allmulticast callbacks to return status

From: Ivan Ilchenko <ivan.ilche...@oktetlabs.ru>

Enabling/disabling of allmulticast mode is not always successful and
it should be taken into account to be able to handle it properly.

When correct return status is unclear from driver code, -EAGAIN is used.

Signed-off-by: Ivan Ilchenko <ivan.ilche...@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybche...@solarflare.com>
---

[...]

  drivers/net/enic/enic_ethdev.c          | 22 +++++++---

[...]

diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 5d48930a9d..e12ca213ae 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -638,28 +638,38 @@ static int enicpmd_dev_promiscuous_disable(struct
rte_eth_dev *eth_dev)
        return ret;
  }

-static void enicpmd_dev_allmulticast_enable(struct rte_eth_dev *eth_dev)
+static int enicpmd_dev_allmulticast_enable(struct rte_eth_dev *eth_dev)
  {
        struct enic *enic = pmd_priv(eth_dev);
+       int ret;

        if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-               return;
+               return -ENOTSUP;

Hi Andrew,

If you are making v2, could you make this to return -E_RTE_SECONDARY,
as in the promisc patch?

Yes, of course. I'll do in v2.

        ENICPMD_FUNC_TRACE();
        enic->allmulti = 1;
-       enic_add_packet_filter(enic);
+       ret = enic_add_packet_filter(enic);
+       if (ret != 0)
+               enic->allmulti = 0;
+
+       return ret;
  }

-static void enicpmd_dev_allmulticast_disable(struct rte_eth_dev *eth_dev)
+static int enicpmd_dev_allmulticast_disable(struct rte_eth_dev *eth_dev)
  {
        struct enic *enic = pmd_priv(eth_dev);
+       int ret;

        if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-               return;
+               return -ENOTSUP;

Here too.

I tested all 5 series (promisc, allmulti, ...) with enic, and my test
cases all passed.

Acked-by: Hyong Youb Kim <hyon...@cisco.com>

Thanks for the testing and review,
Andrew.

Thanks.
-Hyong

        ENICPMD_FUNC_TRACE();
        enic->allmulti = 0;
-       enic_add_packet_filter(enic);
+       ret = enic_add_packet_filter(enic);
+       if (ret != 0)
+               enic->allmulti = 1;
+
+       return ret;
  }

  static int enicpmd_add_mac_addr(struct rte_eth_dev *eth_dev,