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

[Hyong Youb Kim (hyonkim)]

> -----Original Message-----
> From: Andrew Rybchenko <arybchenko at 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.Ilchenko at 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.Ilchenko at oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko at 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?

> 
>  	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 <hyonkim at cisco.com>

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,