Re: virtio: RSS support capa

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 9/11/2024 3:59 PM, Morten Brørup wrote:
>> From: Ferruh Yigit [mailto:ferruh.yigit@amd.com]
>> Sent: Wednesday, 11 September 2024 16.02
>>
>> On 9/11/2024 2:20 PM, Thomas Monjalon wrote:
>>> 11/09/2024 14:17, Morten Brørup:
>>>> From: Andrew Rybchenko [mailto:andrew.rybchenko@oktetlabs.ru]
>>>>> On 9/7/24 23:55, Morten Brørup wrote:
>>>>>>> From: Morten Brørup [mailto:mb@smartsharesystems.com]
>>>>>>> 		dev_info->rx_offload_capa |=
>> RTE_ETH_RX_OFFLOAD_RSS_HASH;
>>>>>>
>>>>>> Or perhaps I'm misunderstanding this capability flag.
>>>>>>
>>>>>> I thought it indicated RSS ability, i.e. multi-queue, effectively
>> shadowing
>>>>> rte_eth_conf.rxmode.mq_mode RTE_ETH_MQ_RX_RSS_FLAG.
>>>>>> But maybe it doesn't. Maybe it indicates the ability to store the
>> RSS hash
>>>>> value in the mbuf.
>>>>>>
>>>>>> The RTE_ETH_RX_OFFLOAD_RSS_HASH flag is completely undocumented.
>>>>>>
>>>>>> Can someone please clarify?
>>>>>>
>>>>> RTE_ETH_RX_OFFLOAD_RSS_HASH means that the driver can provide RSS
>> hash
>>>>> value in mbuf (it makes sense if HW can provide it to the driver).
>>>>
>>>> OK, thanks.
>>>>
>>>> Then what indicates this ethdev capability: Use RSS to distribute
>> packets into multiple RX queues?
>>>> I.e. what to check before setting
>> rte_eth_conf.rxmode.mq_mode=RTE_ETH_MQ_RX_RSS_FLAG?
>>>
>>> It is supposed to be implemented by all DPDK drivers I think.
>>> Is there any case where RSS is not supported?
> 
> Depending on the KVM host, virtio in the KVM guest might not support RSS:
> https://elixir.bootlin.com/dpdk/v24.07/source/drivers/net/virtio/virtio_ethdev.c#L2660
> 
> So we're currently checking if rte_eth_dev_info.flow_type_rss_offloads != 0.
> And I was hoping for an official way to check, which we could generally rely on (e.g. for future drivers).
> Apparently not available.
> 

Indeed that check can work, although this is not explicitly documented,
not having any RSS offload type implies that there is no RSS support.

Perhaps short term solution can be to document this as a way to detect
RSS support.

>>>
>>
>> Hi Morten,
>>
>> There is no formal capability reporting support in ethdev [1].
>>
>> `struct rte_eth_dev_info::[rt]x_[queue]_offload_capa` uses
>> RTE_ETH_RX_OFFLOAD_* flags, and they are for limited scope, only to
>> report the offloading capability of device.
>>
>> Also some capability reporting distributed to
>> - struct rte_eth_dev_info::dev_capa (RTE_ETH_DEV_CAPA_*)
>> - struct rte_eth_dev_info (various files like speed_capa)
>> - struct rte_eth_dev_data::dev_flags (RTE_ETH_DEV_*)
>>
>>
>> Programmatically there is no way for device to report RSS support.
> 
> OK, that answers my question.
> Not the answer I was hoping for, but much better than not knowing.
> 
>> The .ini file has "RSS hash" feature, which covers RSS support, please
>> check 'features.rst'.
> 
> Yeah, perhaps the lack of capability reporting and inconsistent interpretation of features (and no conformance testing to detect/reveal this) means that the application must maintain its own list of drivers it supports with details about capabilities and feature deviations.
> 
>>
>> [1]
>> We mentioned formal capability reporting a few times in the past, but
>> this is a big task to take, specially with all historical usages.
> 
> So instead of fixing this omission, because it is a big task, we pass on the challenge to the application developers to find out how the various drivers behave.
> 
>> Two things specially bothers me:
>> 1. When we need a capability, since there is no dedicated place for it,
>> it finds itself one of above locations, mostly in dev_info.
>> 2. 'struct rte_eth_dev_info' has not defined role, it is combination of
>> the status, configuration and capability reporting. With more
>> capabilities hitting it, it is becoming a more mixed, central struct
>> that many features depends on.
> 
> Some of it could be fixed by documenting what we already have.
> E.g. the RTE_ETH_RX_OFFLOAD_RSS_HASH is undocumented, and my initial guess at its purpose was wrong.
> 

This is added later as an optimization for some NICs.
Although these NICs support RSS, accessing to the internally calculated
hash value has performance impact, so this flag enables them to escape
from reporting RSS hash value to application.

I was thinking we documented it in 'features.rst', but looking again
that information is not much useful.

Would you mind adding a comment to macro in 'rte_ethdev.h' as documentation?


> Long term, we really need good feature descriptions and conformance testing, so all features behave the same way on all drivers.
> 

Ack.

> Short term I got my specific RSS questions answered. :-)
>