On 5/24/23 16:54, Kevin Traynor wrote: > On 24/05/2023 15:32, Robin Jarry wrote: >> Kevin Traynor, May 24, 2023 at 16:06: >>> Hmm, not sure on this one. I have a feeling that having a 'hash' mode >>> for tx-steering that only applies to vhost devices, and 'hash' mode >>> for rx-steering that only applies to NICs means people will miss the >>> subtlety and try to enable the wrong hash mode on the wrong device :-) >>> So 'rss' might make it more distinct. >> >> "rss" is fine then. >> >>> Any reason for '+' ? I think commas are used more frequently. If it >>> needed to be extended in future for some extra config, then adding >>> 'key:value' pairs would be seamlessly. e.g. =rss:<reta size?>, >>> lacp:<num of queues?> >> >> The "rss" item should be mandatory anyway. There should be no way to >> disable it. I assume that it is what Ilya meant with these "+" symbols. >> That would leave us with these examples: >> >> # lacp+bfd on separate queue, rss on other queues >> >> options:rx-steering=rss,lacp,bfd >> >> # same >> >> options:rx-steering=lacp,bfd,rss >> > > ^ It looks a little odd to me that some values are for single queues and > some are for the rest of the queues, with no way to distinguish. > > So maybe Ilya had placed significance in the order with his suggestion ? > i.e. [default]+[single queue proto]+[other single queue proto]+...
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. > > I don't want to bike shed too much on it, i guess with good docs, it can > be clear anyway. > >> # only regular rss, same as no config at all >> >> options:rx-steering=rss >> >> # invalid configurations >> >> options:rx-steering=lacp > >> options:rx-steering= > (^ This will be ok and use the default rss) > >> options:rx-steering=bfd,lacp >> >> What do you think? >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
