[dpdk-dev] [PATCH] net/thunderx: Convert ThunderX VNIC PMD to new offload API

Maciej Czekaj maciej.czekaj at caviumnetworks.com
Mon Jan 15 15:49:24 CET 2018





-- Oryginal message --
> On 1/3/2018 1:12 PM, maciej.czekaj at caviumnetworks.com wrote:
>> From: Maciej Czekaj <maciej.czekaj at caviumnetworks.com>
>>
>> This patch removes all references to old-style offload API
>> replacing them with new offload flags.
>>
>> Signed-off-by: Maciej Czekaj <maciej.czekaj at caviumnetworks.com>
> <...>
>
>>   
>>   	dev_info->default_txconf = (struct rte_eth_txconf) {
>>   		.tx_free_thresh = NICVF_DEFAULT_TX_FREE_THRESH,
>> -		.txq_flags =
>> -			ETH_TXQ_FLAGS_NOMULTSEGS  |
>> -			ETH_TXQ_FLAGS_NOREFCOUNT  |
>> -			ETH_TXQ_FLAGS_NOMULTMEMP  |
>> -			ETH_TXQ_FLAGS_NOVLANOFFL  |
>> -			ETH_TXQ_FLAGS_NOXSUMSCTP,
>> +		.txq_flags = ETH_TXQ_FLAGS_IGNORE,
> I am not sure about this, Shahafs may comment better, shouldn't application
> decide to set "c" or not, instead of having this in default
> configuration?
I think of it as a safeguard against a legacy application that is using 
old-style txq_flags.

There is an assymetry in API, since
  - rte_eth_tx_queue setup() and rte_eth_rx_queue_setup() convert flags
  - rte_eth_dev_info_get() - does not convert them.

The scenario leading to issues is as follows:

1.Application reads default txq_flags from the rte_eth_dev_info_get().
  - dev_info.txconf.offloads contains flags but it is ignored by legacy code
  - dev_info.txq_flags may be:
       a) txq_flags == 0
       b) txq_flags ==  ETH_TXQ_FLAGS_IGNORE
       c) txq_flags == new-style flags converted to old-style flags


2. Application uses default txq_flags field to rte_eth_tx_queue_setup(). 
Now, depending on txq_flags:

   a) txq_flags == 0, ethdev layer __clears__ all offloads, see *
   b) txq_flags ==  ETH_TXQ_FLAGS_IGNORE, ethdev layer converts offloads 
to txq_flags, but leaves orignal offloads, so PMD can still use them
   c) txq_flags == old-style flags, ethdev layer converts txq_flags to 
offloads, destroying default offloads


* relevant code snippet from rte_eth_tx_queue_setup() with comments:

     if (tx_conf->txq_flags & ETH_TXQ_FLAGS_IGNORE) {
         // ==> converts offloads to txq_flags but LEAVES offloads, too
         rte_eth_convert_txq_offloads(tx_conf->offloads,
                          &local_conf.txq_flags);
         /* Keep the ignore flag. */
         local_conf.txq_flags |= ETH_TXQ_FLAGS_IGNORE;
     } else {
         // ==> converts txq_flags to offloads but DESTROYS original 
offloads
rte_eth_convert_txq_flags(tx_conf->txq_flags,
                       &local_conf.offloads);
     }


So, out of 3 options:

a) does not work for legacy code
b) will work for legacy code
c) will work but defeats the purpose of removing old-style flags, since 
dev_info() callback has to setup both old-style and new-style default flags

I chose b) as the simplest way to work-around the issue. I could post a 
patch to ethdev API if you think it is important.

>
> <...>
>
>> +	if ((conf_tx_offloads & tx_offload_capa) != conf_tx_offloads) {
>> +		PMD_INIT_LOG(ERR, "Some Tx offloads are not supported "
>> +		      "requested 0x%lx supported 0x%lx\n",
>> +		      conf_tx_offloads, tx_offload_capa);
> This is broken for 32bits, using PRIx64 instead of "lx" makes your code more
> portable.
Will fix in v2.





More information about the dev mailing list