RE: [virtio-comment] RE: [virtio-dev] Re: [virtio-comment] Re: [PATCH v7] virtio-net: support inner header hash

[Parav Pandit <parav@nvidia.com>]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, February 8, 2023 8:32 AM
> 
> On Wed, Feb 08, 2023 at 05:18:32AM +0000, Parav Pandit wrote:
> > > From: Heng Qi <hengqi@linux.alibaba.com>
> > > Sent: Tuesday, February 7, 2023 10:25 PM
> >
> > [..]
> > > >>
> > > >> Do you think we need both hash_types and hash_tunnel_types?
> > > > In struct virtio_net_config we need two fields.
> > > > a. supported_hash_types (already exists) b.
> > > > supported_hash_tunnel_type
> > > > -> bitmap indicating for which outer headers, inner hash
> > > > -> calculation is
> > > supported.
> > >
> > > Thanks for the suggestion, we seem to have reached an agreement.
> > >
> > > >
> > > > In struct virtio_net_hdr we need two fields.
> > > > a. hash_report (already exists)
> > > > b. hash_tunnel_type 8 bits -> absolute value indicating which
> > > > outer header
> > > exists when inner header hash calculated.
> > > > You already have it in your patch named as hash_report_tunnel.
> > > > May be better to name as hash_report_tunnel_type to make it
> > > > clearer that its
> > > type.
> > >
> > > Sure.
> > >
> > > Thanks for your reply.
> >
> > I had one last question. Why do we need to inform the
> hash_report_tunnel_type of the outer header in the virtio_net_hdr?
> > Is this for debug? Or is there a use case that will process this value?
> 
> Well we have hash_report which is kind of similar (and also kind of pointless
> but I think it's there because WHQL wants it). 
Hash_report is useful. It tells hash_value is in which namespace (ipv4-tcp/ipv4 udp etc).
OS can use this value to find tcp connection in a given namespace.

> Maybe we can steal some bits
> from there instead of a new field?
>
I do not have problem adding extra bits. I just don't find that just telling that its vxlan or nvgre to the OS is useful.
If OS needs to know about outer header details, it needs to know the VNI information than just telling vxlan.
 
> 
> I have a follow up question though: are we only hashing the inner header or
> both inner and outer header? Somewhat confused on this.
> 
I understood as inner header. But worth to describe it. May be there. Need to read v8 patch.