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

[Heng Qi <hengqi@linux.alibaba.com>]

å 2023/2/8 äå8:21, Parav Pandit åé:
> > From: Heng Qi <hengqi@linux.alibaba.com>
> > Sent: Wednesday, February 8, 2023 1:11 AM
> >
> >
> > å 2023/2/8 äå1:18, Parav Pandit åé:
> > > > 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?
> > The driver may use it to do some statistical information, or do some rx
> > classification based on the rx hash, and we'd better not hide information from
> > the driver.
> Statistical information is better gathered via stats, instead of adding such code in driver data path.

A cautionary note : the source of hash_report_tunnel comes from the 
discussion here 
https://lists.oasis-open.org/archives/virtio-dev/202211/msg00063.html .
If I understand correctly, if virtio-net-hdr also has 
hash_report_tunnel, how does the driver do statistics?

CC Yuri for some opinions.

Thanks.

> It is not about hiding.
> It has onus on the data path to detect the tunnel type and place that virtio_net_hdr.
> So lets find one use of it in data path which sw entity will use it.