Re: [dpdk-dev] [PATCH 1/3] net/virtio: add initial RSS support

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 9/10/21 2:44 PM, Maxime Coquelin wrote:
> Thanks for the review!
> 
> On 9/10/21 12:06 PM, Andrew Rybchenko wrote:
>> On 9/10/21 12:17 PM, Maxime Coquelin wrote:
>>>
>> I've failed to find F_RSS definion in virtio spec.
>> Could you share the reference, please.
>>
>> Without the sepc it is almost impossible to review the code.
> 
> Sorry, I forgot to add the link, you can find it on the Virtio-spec
> github master branch:
> 
> https://github.com/oasis-tcs/virtio-spec/blob/master/content.tex
> 
> For readability, I would suggest you to clone it and build the HTML or
> PDF versions.

Many thanks.

>>> diff --git a/doc/guides/nics/features/virtio.ini b/doc/guides/nics/features/virtio.ini
>>> index 48f6f393b1..883dd1426c 100644
>>> --- a/doc/guides/nics/features/virtio.ini
>>> +++ b/doc/guides/nics/features/virtio.ini
>>> @@ -14,6 +14,8 @@ Promiscuous mode     = Y
>>>  Allmulticast mode    = Y
>>>  Unicast MAC filter   = Y
>>>  Multicast MAC filter = Y
>>
>> I'd say that
>> RSS has                = P
>> should be added here.
> 
> OK, as I mentionned in the cover letter, I was not sure becasue the doc
> says "RSS hash" uses DEV_RX_OFFLOAD_RSS_HASH and provides mbuf.rss,
> which this patch does not since it does not support (yet)
> VIRTIO_NET_F_HASH_REPORT.
> http://doc.dpdk.org/guides/nics/features.html#rss-hash

Yes, I've seen question in cover letter. It is some kind of gap
in features to claim overall RSS support - just hashing and
distribution across queues without hash forwarding to user.
The item definition covers both generic support and offload to
deliver RSS hash. That's why I suggest P (partial).

> I can try to add VIRTIO_NET_F_HASH_REPORT in v2, so that we can set RSS
> hash = Y without any doubts :)
> 
> Regards,
> Maxime
>