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

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

å 2023/6/8 äå10:26, Parav Pandit åé:
> > From: Heng Qi <hengqi@linux.alibaba.com>
> > Sent: Thursday, June 8, 2023 2:41 AM
> [..]
> > > > +The device MUST calculate the inner header hash for the configured
> > > > \field{hash_tunnel_types} without the outer UDP header if
> > > > \field{hash_option} is 1.
> > > > +
> > > This I think conflicts with the current RSS where if this override (higher priority
> > than RSS) or not.
> >
> > VIRTIO_NET_F_HASH_TUNNEL instructs the device to calculate an inner header
> > hash for configured tunnel types.
> > Then, which fields are used for the calculation of the inner header hash is
> > determined by the RSS configuration.
> >
> > Therefore, if the inner header hash uses \field{hash_option} to indicate that the
> > outer source UDP port is used to calculate the hash, it overrides the RSS
> > configuration (higher priority than RSS). It now not only indicates for which
> > tunnel types the inner hash should be calculated, but also indicates which fields
> > participate in the RSS hash calculation.
> >
> > At this point, we can
> > 1. continue to allow this behavior. That is, if \field{hash_option} is set to 1, the
> > hash value calculated based on the outer source port indicated by
> > VIRTIO_NET_F_HASH_TUNNEL is provided to the indirection table to select the
> > rxq instead of the hash value provided by RSS.
> >
> > 2. hand over to RSS the ability to specify a hash based on the outer source port.
> > That is, we can add VIRTIO_NET_RSS_HASH_TUNNEL_SRC_PORT_UDP_{v4, v6,
> > EX} in \field{supported_hash_types} to indicate that the hash should be
> > calculated using the outer source port for all UDP tunneling protocols.
> > This approach removes the need for \field{hash_option} and makes the line
> > between VIRTIO_NET_F_HASH_TUNNEL and VIRTIO_NET_F_RSS clearer.
> #2 is preferred.

Ok.

> But I must be missing something.
> Without VIRTIO_NET_RSS_HASH_TUNNEL_SRC_PORT_UDP_X, today if one wants to do RSS based on the outer UDP ports,
> It is already supported by the RSS (even in presence of the inner header).
>
> So why do have to introduce VIRTIO_NET_RSS_HASH_TUNNEL_SRC_PORT_UDP_X?

For legacy tunneling protocols like IPIP/GRE_2784/GRE_2890 that do not 
have outer transport headers,
it is worth calculating the inner header hash for these protocols.

For modern tunneling protocols with outer transport headers, although 
they can provide entropy through the outer source udp port,
sometimes we need to perform symmetric hashing on these tunneling protocols.
Then,
1. We can achieve this through the inner header, but we need a way to 
cope with the addition of new tunneling protocols in the future;
2. The outer source udp port can identify the same flow in modern 
tunneling protocols such as VXLAN/VXLAN-GPE/GENEVE/NVGRE.
But this is marked as "SHOULD" or "RECOMMEND" in various RFCs, so it is 
not reliable. This method ( adding
VIRTIO_NET_RSS_HASH_TUNNEL_SRC_PORT_UDP_X) can deal with the needs of 
new tunneling protocols in the future for symmetric hashing.

If #2 is preferred, I'll update it in the next version.
(this also doesn't affect the migration, and maybe we shouldn't need a 
new feature to extend new values for \field{supported_hash_types}.)

Thanks!