Re: [dpdk-dev] [PATCH 3/3] net/bonding: start ethdev prior to setting 8023ad flow

["Havlík Martin" <xhavli56@stud.fit.vutbr.cz>]

Dne 2021-06-23 09:04, Min Hu (Connor) napsal:
> 在 2021/6/22 17:25, Martin Havlik 写道:
>> When dedicated queues are enabled, mlx5 PMD fails to install RTE Flows
>> if the underlying ethdev is not started:
>> bond_ethdev_8023ad_flow_set(267) - bond_ethdev_8023ad_flow_set: port 
>> not started (slave_port=0 queue_id=1)
>> 
> Why mlx5 PMD doing flow create relys on port started ?
> I noticed other PMDs did not has that reliance.
> 
After looking into it, I really can't answer this mlx5 centered 
question.
Closest related info we found so far is the 5th point in 
https://doc.dpdk.org/guides/prog_guide/rte_flow.html#caveats
but it only specifies it's the application's responsibility and that
flow rules are assumed invalid after port stop/close/restart but doesn't 
say anything about
<stop - flow rule create - start> vs <stop - start - flow rule create>
where the former is the point of failure on mlx5.
I'm addressing the maintainers for mlx5, who might know a bit more on 
the topic.
>> Signed-off-by: Martin Havlik <xhavli56@stud.fit.vutbr.cz>
>> Cc: Jan Viktorin <viktorin@cesnet.cz>
>> ---
>>   drivers/net/bonding/rte_eth_bond_pmd.c | 26 
>> ++++++++++++++++++--------
>>   1 file changed, 18 insertions(+), 8 deletions(-)
>> 
>> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c 
>> b/drivers/net/bonding/rte_eth_bond_pmd.c
>> index a6755661c..fea3bc537 100644
>> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
>> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
>> @@ -1818,25 +1818,35 @@ slave_configure(struct rte_eth_dev 
>> *bonded_eth_dev,
>>   			rte_flow_destroy(slave_eth_dev->data->port_id,
>>   
>> 					internals->mode4.dedicated_queues.flow[slave_eth_dev->data->port_id],
>>   					&flow_error);
>> +	}
>>   +	/* Start device */
>> +	errval = rte_eth_dev_start(slave_eth_dev->data->port_id);
>> +	if (errval != 0) {
>> +		RTE_BOND_LOG(ERR, "rte_eth_dev_start: port=%u, err (%d)",
>> +				slave_eth_dev->data->port_id, errval);
>> +		return -1;
>> +	}
>> +
>> +	if (internals->mode == BONDING_MODE_8023AD &&
>> +			internals->mode4.dedicated_queues.enabled == 1) {
>>   		errval = bond_ethdev_8023ad_flow_set(bonded_eth_dev,
>>   				slave_eth_dev->data->port_id);
>>   		if (errval != 0) {
>>   			RTE_BOND_LOG(ERR,
>>   				"bond_ethdev_8023ad_flow_set: port=%d, err (%d)",
>>   				slave_eth_dev->data->port_id, errval);
>> +
>> +			errval = rte_eth_dev_stop(slave_eth_dev->data->port_id);
>> +			if (errval < 0) {
>> +				RTE_BOND_LOG(ERR,
>> +					"rte_eth_dev_stop: port=%d, err (%d)",
>> +					slave_eth_dev->data->port_id, errval);
>> +			}
>>   			return errval;
>>   		}
>>   	}
>>   -	/* Start device */
>> -	errval = rte_eth_dev_start(slave_eth_dev->data->port_id);
>> -	if (errval != 0) {
>> -		RTE_BOND_LOG(ERR, "rte_eth_dev_start: port=%u, err (%d)",
>> -				slave_eth_dev->data->port_id, errval);
>> -		return -1;
>> -	}
>> -
>>   	/* If RSS is enabled for bonding, synchronize RETA */
>>   	if (bonded_eth_dev->data->dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS) 
>> {
>>   		int i;
>>