[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 <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?

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 <hyonkim at 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,