[dpdk-dev] [PATCH 02/13] net/failsafe: check code of promiscuous mode switch

Andrew Rybchenko arybchenko at solarflare.com
Thu Sep 5 18:32:20 CEST 2019


On 9/5/19 7:25 PM, Gaëtan Rivet wrote:
> Hi,
>
> On Thu, Sep 05, 2019 at 05:10:40PM +0100, Andrew Rybchenko wrote:
>> From: Ivan Ilchenko <Ivan.Ilchenko at oktetlabs.ru>
>>
>> rte_eth_promiscuous_enable()/rte_eth_promiscuous_disable() return
>> value was changed from void to int, so this patch modify usage
>> of these functions across net/failsafe according to new return type.
>>
>> Try to keep promiscuous mode consistent across all active
>> devices in the case of failure.
>>
>> Signed-off-by: Ivan Ilchenko <Ivan.Ilchenko at oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <arybchenko at solarflare.com>
>> ---
>>   drivers/net/failsafe/failsafe_ether.c |  8 +++--
>>   drivers/net/failsafe/failsafe_ops.c   | 44 ++++++++++++++++++++++++---
>>   2 files changed, 46 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
>> index 504c76edb0..bd38f1c1e4 100644
>> --- a/drivers/net/failsafe/failsafe_ether.c
>> +++ b/drivers/net/failsafe/failsafe_ether.c
>> @@ -126,9 +126,13 @@ fs_eth_dev_conf_apply(struct rte_eth_dev *dev,
>>   	if (dev->data->promiscuous != edev->data->promiscuous) {
>>   		DEBUG("Configuring promiscuous");
>>   		if (dev->data->promiscuous)
>> -			rte_eth_promiscuous_enable(PORT_ID(sdev));
>> +			ret = rte_eth_promiscuous_enable(PORT_ID(sdev));
>>   		else
>> -			rte_eth_promiscuous_disable(PORT_ID(sdev));
>> +			ret = rte_eth_promiscuous_disable(PORT_ID(sdev));
>> +		if (ret != 0) {
>> +			ERROR("Failed to apply promiscuous mode");
>> +			return ret;
>> +		}
>>   	} else {
>>   		DEBUG("promiscuous already set");
>>   	}
>> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
>> index cc14bc2bcc..cbf143fb3c 100644
>> --- a/drivers/net/failsafe/failsafe_ops.c
>> +++ b/drivers/net/failsafe/failsafe_ops.c
>> @@ -659,11 +659,29 @@ fs_promiscuous_enable(struct rte_eth_dev *dev)
>>   {
>>   	struct sub_device *sdev;
>>   	uint8_t i;
>> +	int ret = 0;
>>   
>>   	fs_lock(dev, 0);
>> -	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE)
>> -		rte_eth_promiscuous_enable(PORT_ID(sdev));
>> +	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
>> +		ret = rte_eth_promiscuous_enable(PORT_ID(sdev));
>> +		if (ret != 0) {
>> +			ERROR("Promiscuous mode enable failed for subdevice %d",
>> +				PORT_ID(sdev));
>> +			break;
>> +		}
>> +	}
>> +	if (ret != 0) {
>> +		/* Rollback in the case of failure */
>> +		FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) {
>> +			ret = rte_eth_promiscuous_disable(PORT_ID(sdev));
>> +			if (ret != 0)
>> +				ERROR("Promiscuous mode disable failed for subdevice %d",
>> +					PORT_ID(sdev));
>> +		}
>> +	}
>>   	fs_unlock(dev, 0);
>> +
>> +	return;
> This patch should be applied after the ethdev change to avoid breaking
> the build, I think?

As far as I tested it does not break the build. Sorry for confusing
return at the end of function without return value.
The function is still void and it is updated to forward ret when
callback has return value.

> You should be able to change the ethdev API, leaving the fail-safe
> internals ignore the return value, then apply this patch and fix it.
> This way the patchset should not break the build mid-series.
>
> Otherwise good implementation with the rollback.

Thanks.



More information about the dev mailing list