On 5/25/23 11:25, Robin Jarry wrote:
> Hey Ilya,
> 
> Ilya Maximets, May 24, 2023 at 17:05:
>> I had a '+' because rss and lacp are two different entities and I looked
>> at it as a mode of operation. i.e. RSS plus special handling for LACP.
>> RSS looks strange in a comma-separated list, IMO.
> 
> For now, there is only LACP but if other protocols are added (e.g. BFD),
> wouldn't it be weird to have them separated as well?
> 
>     options:rx-steering=rss+lacp+bfd
> 
> Since lacp and bfd will most likely be put in the additional rxq, it
> would make sense to identify them as a group.
> 
> I also have other reserves about specifying rss here after thinking some
> more about it:
> 
> - rss shouldn't be disabled anyway, this forces users to always specify
>   it. This is not great from a usability point of view.
> 
> - When there is a single rxq configured by the user, there is no RSS
>   happening per-se since all other traffic will be put in a single
>   queue. The additional rxq being reserved for lacp and/or other special
>   traffic.
> 
> What do you think about removing "rss" altogether from the items?
> 
>     options:rx-steering=lacp,bfd,...


IMO, 'rx-steering=lacp' is not descriptive.  By looking at it users can't
tell what is going on without reading the documentation.  So, I think,
the 'rss' part is necessary.

Do you anticipate protocols other than lacp and bfd?  I'd actually just
treat them as enum ('rss', 'rss+lacp', 'rss+bfd' and 'rss+lacp+bfd') and
not a list.  And document them as a enum.  We can reconsider in the future
if we'll need more than 2 protocols.  It's experimental for now anyway.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to