Re: [dpdk-dev] [PATCH 2/3] ethdev: fail if Tx queue offload is not supported at all

[Andrew Rybchenko <arybchenko@solarflare.com>]

On 05/13/2018 08:37 AM, Shahaf Shuler wrote:
>> Do not allow to request unsupported Tx offload since all checks are removed
>> from PMDs because of consistency check in ethdev.
>> Otherwise application may rely on offload which is not actually supported
>> and send traffic with, for example, wrong checksums, truncated packets or
>> packets with garbage.
>>
>> Fixes: d04dd6d4ed67 ("ethdev: new Rx/Tx offloads API")
>>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>>   lib/librte_ethdev/rte_ethdev.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>> index dd36e6270..60577efcf 100644
>> --- a/lib/librte_ethdev/rte_ethdev.c
>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> @@ -1744,6 +1744,16 @@ rte_eth_tx_queue_setup(uint16_t port_id,
>> uint16_t tx_queue_id,
>>   				local_conf.offloads,
>>   				dev_info.tx_queue_offload_capa,
>>   				__func__);
>> +		/*
>> +		 * Applications which are not converted yet to the new
>> +		 * Tx offload API may request device level offloads on
>> +		 * queue level (and nothing is requested on device level).
>> +		 * However, if the offload is not supported at all Tx
>> +		 * queue setup must fail.
>> +		 */
>> +		if ((local_conf.offloads & dev_info.tx_offload_capa) !=
>> +		    local_conf.offloads)
>> +			return -EINVAL;
> Not converted application doesn't have a clue what are per-queue offloads, and this is the error they will get when the Tx queue configuration will fail.
>
> How about using ETH_TXQ_FLAGS_IGNORE flag, which explicitly says "application converted to the new Tx offloads API". and have 2 different checks:
> 1. for converted application the already exist check[1] with the related error.
> 2. for not converted application your check with a related error.

Yes, I like the idea. Many thanks. I'll send v2 shortly.

> [1]
> if ((local_conf.offloads & dev_info.tx_queue_offload_capa) !=
>       local_conf.offloads) {
>
>>   	}
>>
>>   	return eth_err(port_id, (*dev->dev_ops->tx_queue_setup)(dev,
>> --
>> 2.17.0