On Mon, Jan 9, 2023 at 10:43 AM Heng Qi <[email protected]> wrote: > > On Fri, Jan 06, 2023 at 01:59:38AM -0500, Michael S. Tsirkin wrote: > > On Fri, Jan 06, 2023 at 02:42:21PM +0800, Heng Qi wrote: > > > On Fri, Jan 06, 2023 at 12:27:04AM -0500, Michael S. Tsirkin wrote: > > > > On Wed, Jan 04, 2023 at 03:14:01PM +0800, Heng Qi wrote: > > > > > If the tunnel is used to encapsulate the packets, the hash calculated > > > > > using the outer header of the receive packets is always fixed for the > > > > > same flow packets, i.e. they will be steered to the same receive > > > > > queue. > > > > > > > > > > We add a feature bit VIRTIO_NET_F_HASH_TUNNEL and related bitmasks > > > > > in \field{hash_types}, which instructs the device to calculate the > > > > > hash using the inner headers of tunnel-encapsulated packets. Besides, > > > > > values in \field{hash_report_tunnel} are added to report tunnel types. > > > > > > > > > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/151 > > > > > > > > > > Reviewed-by: Jason Wang <[email protected]> > > > > > Signed-off-by: Heng Qi <[email protected]> > > > > > Signed-off-by: Xuan Zhuo <[email protected]> > > > > > > > > > > > > ok close to being ready. a couple of minor comments. > > > > > > > > > --- > > > > > v6: > > > > > 1. Modify the wording of some sentences for clarity. @Michael > > > > > S. Tsirkin > > > > > 2. Fix some syntax issues. @Michael S. Tsirkin > > > > > > > > > > v5: > > > > > 1. Fix some syntax and capitalization issues. @Michael S. > > > > > Tsirkin > > > > > 2. Use encapsulated/encaptulation uniformly. @Michael S. > > > > > Tsirkin > > > > > 3. Move the links to introduction section. @Michael S. Tsirkin > > > > > 4. Clarify some sentences. @Michael S. Tsirkin > > > > > > > > > > v4: > > > > > 1. Clarify some paragraphs. @Cornelia Huck > > > > > 2. Fix the u8 type. @Cornelia Huck > > > > > > > > > > v3: > > > > > 1. Rename VIRTIO_NET_F_HASH_GRE_VXLAN_GENEVE_INNER to > > > > > VIRTIO_NET_F_HASH_TUNNEL. @Jason Wang > > > > > 2. Make things clearer. @Jason Wang @Michael S. Tsirkin > > > > > 3. Keep the possibility to use inner hash for automatic > > > > > receive steering. @Jason Wang > > > > > 4. Add the "Tunnel packet" paragraph to avoid repeating the > > > > > GRE etc. many times. @Michael S. Tsirkin > > > > > > > > > > v2: > > > > > 1. Add a feature bit for GRE/VXLAN/GENEVE inner hash. @Jason > > > > > Wang > > > > > 2. Chang \field{hash_tunnel} to \field{hash_report_tunnel}. > > > > > @Jason Wang, @Michael S. Tsirkin > > > > > > > > > > v1: > > > > > 1. Remove the patch for the bitmask fix. @Michael S. Tsirkin > > > > > 2. Clarify some paragraphs. @Jason Wang > > > > > 3. Add \field{hash_tunnel} and VIRTIO_NET_HASH_REPORT_GRE. > > > > > @Yuri Benditovich > > > > > > > > > > content.tex | 191 > > > > > +++++++++++++++++++++++++++++++++++++++++++++-- > > > > > introduction.tex | 19 +++++ > > > > > 2 files changed, 203 insertions(+), 7 deletions(-) > > > > > > > > > > diff --git a/content.tex b/content.tex > > > > > index e863709..7845f6c 100644 > > > > > --- a/content.tex > > > > > +++ b/content.tex > > > > > @@ -3084,6 +3084,9 @@ \subsection{Feature bits}\label{sec:Device > > > > > Types / Network Device / Feature bits > > > > > \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control > > > > > channel. > > > > > > > > > > +\item[VIRTIO_NET_F_HASH_TUNNEL(52)] Device supports inner > > > > > + header hash for GRE, VXLAN and GENEVE tunnel-encapsulated > > > > > packets. > > > > > > > > I would probably drop the list of tunnel types here. > > > > > > Do you mean to use "Device supports inner header hash for > > > tunnel-encapsulated packets." instead? Why? We do want to use this > > > feature bit to indicate that the device supports inner hashing of > > > GRE, VXLAN and GENEVE encapsulated packets. As in the v3 discussion > > > https://lists.oasis-open.org/archives/virtio-dev/202212/msg00024.html , > > > we discussed using VIRTIO_NET_F_HASH_TUNNEL to replace > > > VIRTIO_NET_F_HASH_GRE_VXLAN_GENEVE_INNER and plan to use > > > VIRTIO_NET_F_HASH_TUNNEL_XYZ for future extensions. > > > > So imagine we add a new tunnel type. Let's say there's VXLAN v2. > > why would we need a new feature bit? I think a new hash type > > will be sufficient. No? > > If the description for VIRTIO_NET_F_HASH_TUNNEL is as follows: > "[VIRTIO_NET_F_HASH_TUNNEL(52)] Device supports inner header hash for > tunnel-encapsulated packets.". > Then the following may happen > 1. For VXLANv2, if both device src and device dst have negotiated this > feature, it is assumed that > device src supports VXLAN and VXLANv2, but device dst may only support > VXLAN, not VXLANv2. > 2. For other encapsulation protocols such as ip in ip, after device src and > device dst have > negotiated this feature, it is assumed that device src supports GRE, > VXLAN, GENEVE and ip in ip, > but it is not clear that device dst also supports ip in ip. Especially > when migrating, this can > lead to inconsistencies in live migrations.
Yes, this looks like the only way that I can think of to keep migration compatibility in an easy way. Thanks > So, I think it's better to keep the original description: > "[VIRTIO_NET_F_HASH_TUNNEL(52)] Device supports inner header hash for GRE, > VXLAN and GENEVE tunnel-encapsulated packets." > > Thanks. > > > > > -- > > MST > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: [email protected] > > For additional commands, e-mail: [email protected] > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
