On 3/12/2018 5:58 PM, Shahaf Shuler wrote:
> Monday, March 12, 2018 7:00 PM, Ferruh Yigit:
>> There are some devices supports queue level offloads and there are some
>> devices supports port level offloads.
>>
>> Values queue offload = 0x0 and port offload = 0x1000, for the device that
>> support queue level offloads may mean disabling all offloads for that 
>> specific
>> queue and this is valid value. For the device that support port level 
>> offloads
>> this is an error.
> 
> device which don't support port level offloads should not be configured with 
> port offloads. 
> Well implemented PMDs will fail the configuration with port offload = 0x1000.

For this particular error in tap:
Rx queue offloads = 0x0,
requested port offloads = 0x1000,
supported offloads = 0x300e

Since supported offloads reported, and requested is subset of it I think port
level offloads are OK, problem is in queue level offloads.

> 
>>
>> What about using "rx/tx_queue_offload_capa" field to help application to
>> detect if device supports queue level or port level offloads?
> 
> Yes. this is their purpose. 
> 
>>
>> If device reports 0x0 "rx_queue_offload_capa", application will know device
>> doesn't support queue level offloads and PMD just ignore all provided queue
>> offloads.
>>
>> If device reports a "rx_queue_offload_capa" other than 0x0, app will know
>> that device _supports_ queue offloads and will set offloads according and
>> PMD will verify and apply queue specific offloads according.
>>
>> For above tap specific case, is tap PMD supports queue specific offloads?
> 
> Am not sure. Moty? 
> 
>>
>> And there should be some restrictions on offloading values:
>> 1- requested port offloads should be subset of supported port offloads
>> 2- supported queue offloads should be subset of supported port offloads
>> 3- requested queue offloads should be subset of supported queue offloads
> 
> This is correct. 
> 
>>
>> And since these information is part of dev_info, these can be managed in the
>> ethdev layer, instead of checked in each PMD (as done in tap).
>>
>>
>> According above, would you mind walk-trough how application can set
>> offloads:
>>
>> A) New application that implements new offloading APIs:
>> - Get dev_info
>> - Configure Rx/Tx offloads based on rx_offload_capa / tx_offload_capa
>> - If rx_queue_offload_capa / tx_queue_offload_capa is other than 0, setup
>> RxQ / TxQ offloads based on these values.
>> - If rx_queue_offload_capa / tx_queue_offload_capa is 0, queue level
>> offlaods are not supported by this device, ignore offloads during RxQ / TxQ
>> setup.
> 
> With the current API it is not correct. queue offloads should be at least the 
> port offloads. If the application wants to enable more queue specific 
> offloads it can as long as those are supported.

So above statement 2 is wrong?

Just to confirm, a queue can not disable an offload configured for port but can
enable more offloads?

> The following pseudo code demonstrate above:
> 
> Dev_configure(port, port_offloads)
> Rx_queue_offloads = port_offloads
> If (rx_queue_offload_capa != 0)
>       Rx_queue_offloads |= rx_queue_offload_capa

Again to confirm, a device that supports port level offloads free to ignore TxQ
/ RxQ offload values, right? So why we need "Rx_queue_offloads = port_offloads",
will following be true?

Dev_configure(port, port_offloads)
If (rx_queue_offload_capa != 0)
        Rx_queue_offloads = port_offloads | rx_queue_offload_capa

> 
>>
>> All look OK here.
>>
>>
>> B) Old application with old offloading API
>> - Get dev_info, which provides only rx_offload_capa, tx_offload_capa and
>> txq_flags
>> - set rte_eth_rxmode->bitfield_values  ==> ethdev will convert them to port
>> level Rx offloads.
>> - port level offloads are empty!!
>> - ethdev will set queue level Rx offloads to be same as port level Rx 
>> offloads.
>> - ethdev will set txq_flags values for Tx offloads to queue level Tx 
>> offloads.
>>
>> Things should work well for PMDs with old offloading API.
>>
>> For the PMDs that support new offloading API, port level Tx offload values
>> are missing and Queue level and Port level Tx offloads mismatch. Am I
>> missing something here, if not how can we solve this issue in PMDs?
> 
> Those PMDs (new PMD for old application) can use the  ETH_TXQ_FLAGS_IGNORE 
> which must be set for application which uses the new API..
> see snipped code from mlx5 PMD:
> 
> [1]
> /*                                                                      
>  * Don't verify port offloads for application which                     
>  * use the old API.                                                     
>  */                                                                     
> if (!!(conf->txq_flags & ETH_TXQ_FLAGS_IGNORE) &&                       
>     !priv_is_tx_queue_offloads_allowed(priv, conf->offloads)) {         
>         ret = ENOTSUP;                                                  
>         ERROR("%p: Tx queue offloads 0x%" PRIx64 " don't match port "   
>               "offloads 0x%" PRIx64 " or supported offloads 0x%" PRIx64,
>               (void *)dev, conf->offloads,                              
>               dev->data->dev_conf.txmode.offloads,                      
>               mlx5_priv_get_tx_port_offloads(priv));                    
>         goto out;                                                       
> } 

What this code does is: "if new offload API is used and queue offload is not
valid, return error", this is completely different than what I say.

My concern is how new PMD will handle old application because of missing port
level Tx offloads.

I guess each time PMD needs to use a Tx offload, it needs to check
ETH_TXQ_FLAGS_IGNORE. If IGNORE is set PMD will use  txmode->offloads, if not it
will use txq->offloads. Do you think this solves the issue?

> 
> 
> 
> 
> 
>>
>>
>>>
>>>>
>>>> I believe other way around makes sense, to be able to set queue offload
>> param, device should announce that offloading as supported. Queue is
>> subset of the device, why you ignore device offload param to set queue
>> offload param?
>>>>
>>>> What makes sense to me:
>>>> "queue offload" is subset of "device offload" is subset of "device
>> supported offload"
>>>
>>>
>>>
>>>
>>> [1]
>>>
>> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdpd
>> k
>>>
>> .org%2Fdoc%2Fguides%2Fprog_guide%2Fpoll_mode_drv.html&data=02%7C
>> 01%7Cs
>>>
>> hahafs%40mellanox.com%7C2fc3763767f041846bdd08d5883aa76a%7Ca65297
>> 1c7d2
>>>
>> e4d9ba6a4d149256f461b%7C0%7C0%7C636564707857610335&sdata=Uavzne
>> YAsXf2M
>>> SdJWSp6i1fvRmCRyx6pWwLuzUCqOLA%3D&reserved=0
>>>
>>>
>>>>
>>>> <...>
> 

Reply via email to