[dpdk-dev] [PATCH v2 04/13] ethdev: change promiscuous callbacks to return status

Andrew Rybchenko arybchenko at solarflare.com
Fri Sep 13 23:06:55 CEST 2019


On 9/11/19 11:46 AM, Hyong Youb Kim (hyonkim) wrote:
>> -----Original Message-----
>> From: Andrew Rybchenko <arybchenko at solarflare.com>
>> Sent: Monday, September 9, 2019 8:59 PM
> [...]
>> Subject: [PATCH v2 04/13] ethdev: change promiscuous callbacks to return
>> status
>>
>> Enabling/disabling of promiscuous 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: Andrew Rybchenko <arybchenko at solarflare.com>
>> ---
> [...]
>>   drivers/net/enic/enic.h                   |  2 +-
>>   drivers/net/enic/enic_ethdev.c            | 22 +++++++---
>>   drivers/net/enic/enic_main.c              |  4 +-
> [...]
>>   static void
>> diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
>> index 5a92508f00..72b1e7956b 100644
>> --- a/drivers/net/enic/enic.h
>> +++ b/drivers/net/enic/enic.h
>> @@ -305,7 +305,7 @@ int enic_get_link_status(struct enic *enic);
>>   int enic_dev_stats_get(struct enic *enic,
>>   		       struct rte_eth_stats *r_stats);
>>   void enic_dev_stats_clear(struct enic *enic);
>> -void enic_add_packet_filter(struct enic *enic);
>> +int enic_add_packet_filter(struct enic *enic);
>>   int enic_set_mac_address(struct enic *enic, uint8_t *mac_addr);
>>   int enic_del_mac_address(struct enic *enic, int mac_index);
>>   unsigned int enic_cleanup_wq(struct enic *enic, struct vnic_wq *wq);
>> diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
>> index 90fdeda901..5d48930a9d 100644
>> --- a/drivers/net/enic/enic_ethdev.c
>> +++ b/drivers/net/enic/enic_ethdev.c
>> @@ -603,29 +603,39 @@ static const uint32_t
>> *enicpmd_dev_supported_ptypes_get(struct rte_eth_dev *dev)
>>   	return NULL;
>>   }
>>
>> -static void enicpmd_dev_promiscuous_enable(struct rte_eth_dev
>> *eth_dev)
>> +static int enicpmd_dev_promiscuous_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;
> Should return -E_RTE_SECONDARY to be consistent with other handlers
> that check primary/secondary.

I'll fix in the next version, but please, note that -ENOTSUP has
special handling in ethdev patch on config restore. It looks like
different error code should not be a problem here, but please, check.

>>   	ENICPMD_FUNC_TRACE();
>>
>>   	enic->promisc = 1;
>> -	enic_add_packet_filter(enic);
>> +	ret = enic_add_packet_filter(enic);
>> +	if (ret != 0)
>> +		enic->promisc = 0;
>> +
>> +	return ret;
>>   }
>>
>> -static void enicpmd_dev_promiscuous_disable(struct rte_eth_dev
>> *eth_dev)
>> +static int enicpmd_dev_promiscuous_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;
> Should return -E_RTE_SECONDARY here too.
>
>>   	ENICPMD_FUNC_TRACE();
>>   	enic->promisc = 0;
>> -	enic_add_packet_filter(enic);
>> +	ret = enic_add_packet_filter(enic);
>> +	if (ret != 0)
>> +		enic->promisc = 1;
>> +
>> +	return ret;
>>   }
>>
>>   static void enicpmd_dev_allmulticast_enable(struct rte_eth_dev *eth_dev)
>> diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
>> index 40af3781b3..f4e76a057a 100644
>> --- a/drivers/net/enic/enic_main.c
>> +++ b/drivers/net/enic/enic_main.c
>> @@ -1364,10 +1364,10 @@ int enic_set_vlan_strip(struct enic *enic)
>>   			       enic->rss_enable);
>>   }
>>
>> -void enic_add_packet_filter(struct enic *enic)
>> +int enic_add_packet_filter(struct enic *enic)
>>   {
>>   	/* Args -> directed, multicast, broadcast, promisc, allmulti */
>> -	vnic_dev_packet_filter(enic->vdev, 1, 1, 1,
>> +	return vnic_dev_packet_filter(enic->vdev, 1, 1, 1,
>>   		enic->promisc, enic->allmulti);
>>   }
>>
> A couple minor comments above. Other than those, patch works fine for enic.
> Feel free to add my ack on v2..
>
> Acked-by: Hyong Youb Kim <hyonkim at cisco.com>
>

Thanks for review,
Andrew.


More information about the dev mailing list