RE: [PATCH v9] virtio-net: support inner header hash

[Parav Pandit <parav@nvidia.com>]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Tuesday, February 21, 2023 6:18 PM

> > The question of discussion was,
> > Scenario:
> > 1. device advertises the ability to hash on the inner packet header.
> > 2. device prefers that driver enable it only when it needs to use this extra
> packet parser in hardware.
> >
> > There are 3 options.
> > a. Because the feature is negotiated, it means it is enabled for all the tunnel
> types.
> > Pros:
> > 1. No need to extend cvq cmd.
> > Cons:
> > 1. device parser is always enabled, and the driver never uses it. This may
> result in inferior rx performance.
> >
> > b. Since the feature is useful in a narrow case of sw-based vxlan etc driver,
> better not to enable hw for it.
> > Hence, have the knob to explicitly enable in hw.
> > So have the cvq command.
> > b.1 should it be combined with the existing command?
> > Cons:
> > a. when the driver wants to enable hash on inner, it needs to supply the exact
> same RSS config as before. Sw overhead with no gain.
> > b. device needs to parse new command value, compare with old config, and
> drop the RSS config, just enable inner hashing hw parser.
> > Or destroy the old rss config and re-apply. This results in weird behavior for
> the short interval with no apparent gain.
> >
> > b.2 should it be on its own command?
> > Pros:
> > a. device and driver doesn't need to bother about b.1.a and b.1.b.
> > b. still benefits from not always enabling hw parser, as this is not a common
> case.
> > c. has the ability to enable when needed.
> 
> I prefer b.1. With reporting of the tunnel type gone I don't see a fundamental
> difference between hashing over tunneling types and other protocol types we
> support.  It's just a flag telling device over which bits to calculate the hash. We
> don't have a separate command for hashing of TCPv6, why have it for vxlan?
b.1 to always enable hw for multi-level packet processing is not very optimal for actual device implementation.
The difference is one level of header vs second-level hashing.
And new hash type values have zero use of it in sw.

> Extending with more HASH_TYPE makes total sense to me, seems to fit better
> with the existing design and will make patch smaller.