[PATCH v11 0/4] virtio-net: define UDP tunnel offload

[PATCH v11 0/4] virtio-net: define UDP tunnel offload

[Paolo Abeni]

UDP tunnel usage is ubiquitous in container deployment, and the ability
to offload UDP encapsulated GSO traffic impacts greatly the performances
and the CPU utilization of such use cases.

This series includes:
- clarifications on the current semantic of NEEDS_CSUM offload
(patch 1) and DATA_VALID offload (patch 2) to make later changes easier
to follow,
- new features definition to handle both UDP tunnel segmentation
offload (patch 3).
- new features definition to handle UDP tunnel outer checksum offload
(patch 4).

A PoC implementation is available here:

https://github.com/pabeni/linux-devel/commits/virtio_tunnel_gso_v10/
https://github.com/pabeni/qemu/tree/virtio_tunnel_gso_v9

Note that both implementations has not been updated since the previous
iteration, since there are no additional functional changes in v11.

Changes from v10:
- added patch 2
- clarified only a single encapsulation level is supported (patch 3)
- s/TCP or UDP GSO packets/GSO packets/ (patch 3)
- 'to the outer UDP pseudo header' -> 'to the outer UDP pseudo header
  checksum' (patch 4)
https://lore.kernel.org/virtio-comment/cover.1731084277.git.pabeni@redhat.com/

Changes from v9:
- added patch 1
- clarified that 'hdr_len' accounts for the inner header for GSO over
UDP tunnel packets
- DATA_VALID is not be allowed for GSO over UDP tunnel packets.
https://lore.kernel.org/virtio-comment/cover.1728029499.git.pabeni@redhat.com/

Changes from v8:
- rebased on top of virtio-1.4, changed udp-tunnel features number
  to avoid conflicts/duplications
- Clarified the usage of DATA_VALID flag with UDP tunneled GSO packets
- mandate strict hdr validation on the rx side, too - for UDP tunneled
  GSO packets only
https://lore.kernel.org/virtio-comment/cover.1725463935.git.pabeni@redhat.com/

Changes from v7:
- minor cleanup in patch 1/2
- dropped confusing wording about csum validation in patch 2/2

Changes from v6:
- replaced inner_protocol_type with
  VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV{4,6}
- many clarifications and cleanup, see the individual patches changelog
  for the details

Changes from v5:
- split in 2 patches
- dropped outer_mh_offset field
- many csum related clarification
---
Paolo Abeni (4):
  virtio-net: clarify NEEDS_CSUM semantic for GSO packats.
  virtio-net: clarify DATA_VALID  semantic for encap protos.
  virtio-net: define UDP tunnel segmentation offload feature
  virtio-net: define UDP tunnel checksum offload feature

 device-types/net/description.tex | 355 ++++++++++++++++++++++++++++++-
 1 file changed, 345 insertions(+), 10 deletions(-)

-- 
2.45.2

[PATCH v11 1/4] virtio-net: clarify NEEDS_CSUM semantic for GSO packats.

[Paolo Abeni]

The current wording is a bit unclear hinting to possible additional
nested headers. For GSO packets virtio net (currently) supports
offload for the checksum of single transport header, explicitly state
that in both the driver and device sections.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 device-types/net/description.tex | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 4e4ffdb..3434a31 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -695,6 +695,11 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 \item the driver MUST validate the packet checksum at
 	offset \field{csum_offset} from \field{csum_start} as well as all
 	preceding offsets;
+\begin{note}
+If \field{gso_type} differs from VIRTIO_NET_HDR_GSO_NONE, \field{csum_offset}
+points to the only transport header present in the packet, and there are no
+additional preceding checksums validated by VIRTIO_NET_HDR_F_NEEDS_CSUM.
+\end{note}
 \item the driver MUST set the packet checksum stored in the
 	buffer to the TCP/UDP pseudo header;
 \item the driver MUST set \field{csum_start} and
@@ -895,7 +900,11 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
   if bit VIRTIO_NET_HDR_F_RSC_INFO is not set in \field{flags},
   then \field{csum_start} and \field{csum_offset} indicate how to calculate it
   (see Packet Transmission point 1).
-
+\begin{note}
+If \field{gso_type} differs from VIRTIO_NET_HDR_GSO_NONE, \field{csum_offset}
+points to the only transport header present in the packet, and there are no
+additional preceding checksums validated by VIRTIO_NET_HDR_F_NEEDS_CSUM.
+\end{note}
 \end{enumerate}
 
 If applicable, the device calculates per-packet hash for incoming packets as
-- 
2.45.2

Re: [PATCH v11 1/4] virtio-net: clarify NEEDS_CSUM semantic for GSO packats.

[Jason Wang]

On Wed, Nov 27, 2024 at 5:37 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> The current wording is a bit unclear hinting to possible additional
> nested headers. For GSO packets virtio net (currently) supports
> offload for the checksum of single transport header, explicitly state
> that in both the driver and device sections.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---

Reviewed-by: Jason Wang <jasowang@redhat.com>

Thanks

RE: [PATCH v11 1/4] virtio-net: clarify NEEDS_CSUM semantic for GSO packats.

[Parav Pandit]

> From: Paolo Abeni <pabeni@redhat.com>
> Sent: Wednesday, November 27, 2024 3:07 PM
> 
> The current wording is a bit unclear hinting to possible additional nested
> headers. For GSO packets virtio net (currently) supports offload for the
> checksum of single transport header, explicitly state that in both the driver
> and device sections.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  device-types/net/description.tex | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/device-types/net/description.tex b/device-
> types/net/description.tex
> index 4e4ffdb..3434a31 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -695,6 +695,11 @@ \subsubsection{Packet
> Transmission}\label{sec:Device Types / Network Device / De  \item the driver
> MUST validate the packet checksum at
>  	offset \field{csum_offset} from \field{csum_start} as well as all
>  	preceding offsets;
> +\begin{note}
> +If \field{gso_type} differs from VIRTIO_NET_HDR_GSO_NONE,
> +\field{csum_offset} points to the only transport header present in the
> +packet, and there are no additional preceding checksums validated by
> VIRTIO_NET_HDR_F_NEEDS_CSUM.
> +\end{note}
>  \item the driver MUST set the packet checksum stored in the
>  	buffer to the TCP/UDP pseudo header;
>  \item the driver MUST set \field{csum_start} and @@ -895,7 +900,11 @@
> \subsubsection{Processing of Incoming Packets}\label{sec:Device Types /
> Network
>    if bit VIRTIO_NET_HDR_F_RSC_INFO is not set in \field{flags},
>    then \field{csum_start} and \field{csum_offset} indicate how to calculate it
>    (see Packet Transmission point 1).
> -
> +\begin{note}
> +If \field{gso_type} differs from VIRTIO_NET_HDR_GSO_NONE,
> +\field{csum_offset} points to the only transport header present in the
> +packet, and there are no additional preceding checksums validated by
> VIRTIO_NET_HDR_F_NEEDS_CSUM.
> +\end{note}
>  \end{enumerate}
> 
>  If applicable, the device calculates per-packet hash for incoming packets as
> --
> 2.45.2
> 

Reviewed-by: Parav Pandit <parav@nvidia.com>

[PATCH v11 2/4] virtio-net: clarify DATA_VALID semantic for encap protos.

[Paolo Abeni]

DATA_VALID allows offloading a single checksum level, leaving
unspecified which header checksum is offloaded when one or more
encapsulated protocols are present.

In such a case, the only option usable from the guest OS is
offloading the outermost checksum. That also matches the existing
implementation.

Explicitly state such the constraint, to remove any ambiguity and
make later changes more straightforward.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 device-types/net/description.tex | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 3434a31..77bd847 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -994,8 +994,8 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 If the VIRTIO_NET_F_GUEST_CSUM feature has been negotiated, the
 device MAY set the VIRTIO_NET_HDR_F_DATA_VALID bit in
 \field{flags}, if so, the device MUST validate the packet
-checksum (in case of multiple encapsulated protocols, one level
-of checksums is validated).
+checksum. One level of checksum is validated: in case of multiple
+encapsulated protocols the outermost one.
 
 \drivernormative{\paragraph}{Processing of Incoming
 Packets}{Device Types / Network Device / Device Operation /
-- 
2.45.2

Re: [PATCH v11 2/4] virtio-net: clarify DATA_VALID semantic for encap protos.

[Jason Wang]

On Wed, Nov 27, 2024 at 5:37 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> DATA_VALID allows offloading a single checksum level, leaving
> unspecified which header checksum is offloaded when one or more
> encapsulated protocols are present.
>
> In such a case, the only option usable from the guest OS is
> offloading the outermost checksum. That also matches the existing
> implementation.
>
> Explicitly state such the constraint, to remove any ambiguity and
> make later changes more straightforward.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Reviewed-by: Jason Wang <jasowang@redhat.com>

Thanks

RE: [PATCH v11 2/4] virtio-net: clarify DATA_VALID semantic for encap protos.

[Parav Pandit]

> From: Paolo Abeni <pabeni@redhat.com>
> Sent: Wednesday, November 27, 2024 3:07 PM


> DATA_VALID allows offloading a single checksum level, leaving unspecified
> which header checksum is offloaded when one or more encapsulated
> protocols are present.
> 
> In such a case, the only option usable from the guest OS is offloading the
> outermost checksum. That also matches the existing implementation.
> 
> Explicitly state such the constraint, to remove any ambiguity and make later
> changes more straightforward.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  device-types/net/description.tex | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/device-types/net/description.tex b/device-
> types/net/description.tex
> index 3434a31..77bd847 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -994,8 +994,8 @@ \subsubsection{Processing of Incoming
> Packets}\label{sec:Device Types / Network  If the
> VIRTIO_NET_F_GUEST_CSUM feature has been negotiated, the  device MAY
> set the VIRTIO_NET_HDR_F_DATA_VALID bit in  \field{flags}, if so, the device
> MUST validate the packet -checksum (in case of multiple encapsulated
> protocols, one level -of checksums is validated).
> +checksum. One level of checksum is validated: in case of multiple
> +encapsulated protocols the outermost one.
> 
>  \drivernormative{\paragraph}{Processing of Incoming  Packets}{Device Types
> / Network Device / Device Operation /
> --
> 2.45.2
> 

Reviewed-by: Parav Pandit <parav@nvidia.com>

[PATCH v11 3/4] virtio-net: define UDP tunnel segmentation offload feature

[Paolo Abeni]

The VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV{4,6} are two gso_type flags
allowing respectively GSO over UDPv4 tunnel and GSO over UDPv6 tunnel.
They can be negotiated on both the host and guest sides.

One constraint addressed here is that the virtio side (either device or
driver) receiving a UDP tunneled GSO packet must be able to reconstruct
completely the inner and outer headers offset - to allow for later GSO.

To accommodate such need, new fields are introduced in the virtio_net
header: outer_th_offset and inner_nh_offset.
They map directly to the corresponding header information. The inner
transport header is implied by the (inner) checksum offload.

Those fields are required because a virtio device H/W implementation
performing segmentation for UDP tunneled packet will need to touch
the outer transport protocol (for the UDP length filed), the
inner network protocol (for the total length field, in the IPv4
case).

Note that segmentation will additionally need to touch
the outer network protocol and the inner transport protocol. The first
is implied/easily found with trivial parsing, the latter is identified
by the existing csum_start field.

Note that there is no concept of UDP tunnel type negotiation (e.g.
vxlan, geneve, vxlan-gpe, etc.), as a virtio device H/W implementation
can perform segmentation for every possible UDP-tunnel given the
specified new fields.
In the reverse direction, if a virtio device H/W implementation receives
some traffic for an unknown or unsupported UDP tunnel, it will simply
not aggregate the wire packet in a GSO one.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v10 -> v11:
 - clarified only a single encapsulation level is supported
 - s/TCP or UDP GSO packets/GSO packets/

v9 -> v10:
 - mandate DATA_VALID and VIRTIO_NET_HDR_F_NEEDS_CSUM as mutually exclusive
   for GSO over UDP tunnels packets.
 - clarified that hdr_len includes the inner transport header for GSO over
   UDP tunnels packets
 - highlighted that csum_offset refers to the inner header in a specific
   note

v8 -> v9:
 - rebased on top of virtio-1.4, changed udp-tunnel features number
   to avoid conflicts/duplications
 - fixed typos:
   VIRTIO_NET_HDR_F_UDP_TUNNEL_IPV4 -> VIRTIO_NET_HDR_F_GSO_UDP_TUNNEL_IPV4
   VIRTIO_NET_HDR_F_UDP_TUNNEL_IPV6 -> VIRTIO_NET_HDR_F_GSO_UDP_TUNNEL_IPV6
 - specified strict validation on RX side (both driver and device)
   for UDP tunneled packet: NEEDS_CSUM and plain GSO are mandatory for UDP
   tunnel GSO packets. The driver is not allowed to set DATA_VALID for UDP
   tunnel GSO packets, the driver can set both DATA_VALID & NEEDS_CSUM.

v7 -> v8:
 - dropped 'Note that' in note
 - NOT set neither A nor B -> NOT set either A or B
 - specify that 'gso_partial' needs uh->len set to the wire one

v6 -> v7:
 - replaced inner_protocol_type with VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 &&
   VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4
 - UDP_TUNNEL now depends on all the TCP and USO segmentation features
 - reworded the 'VIRTIO_NET_HDR_GSO_UDP_TUNNEL' bit driver normative
 - specified that  csum_start/offset points to the inner header in the
   driver normative, too
 - new fields presence depends on either VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO or
   VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO
 - e.g. ->  i.e.
 - zero -> positive zero (0x0)
 - checksumming -> checksum validation
 - explain why we the outer UDP csum can be precomputed only
   when the GSO packet length is a multiple of gso_size
 - added an example note for the tunnel-related fields value
 https://lore.kernel.org/virtio-comment/cover.1722252302.git.pabeni@redhat.com/T/#t

v5 -> v6:
 - split in 2 patches
 - dropped the unneeded outer_mh_offset field
 https://lore.kernel.org/virtio-comment/56ccba136272593d0d2af0303600c06f26bd84a1.1718621522.git.pabeni@redhat.com/

v4 -> v5:
 - add separate negotiation for UDP_TUNNEL_CSUM
 - much more verbose specification of outer csum handling
 - UDP_TUNNEL_CSUM requires GSO_UDP_TUNNEL
 https://lore.kernel.org/virtio-comment/cb89d3d6b6a7e4a7f13e60dd3fded5e950727890.1716448766.git.pabeni@redhat.com/

v3 -> v4:
 - new virtio_net_hdr fields depends on F_HOST_UDP_TUNNEL_GSO or
   F_GUEST_UDP_TUNNEL_GSO (Stefano)
 - Extended the changelog to answer Jason's questions.
 - Clarified outer csum handling (Jason)
 https://lore.kernel.org/virtio-dev/f0bf2db203f8061a3ecb50b7a48cf26d8f3c7f68.1716193086.git.pabeni@redhat.com

v2 -> v3:
 - UDP_TUNNEL -> UDP_TUNNEL_GSO
 - add explicit fields for the inner meta-data
 - more verbose changelog
 https://lists.oasis-open.org/archives/virtio-dev/202206/msg00026.html

v1 -> v2:
 - explicitly state that the outer header probing is mandatory
 - explicitly state that GSO_UDP is not allowed with GSO_UDP_TUNNEL
 - clarify hdr_len usage
 - clarify UDP_TUNNEL_CSUM bit usage
 - fix a few typos
 https://lists.oasis-open.org/archives/virtio-dev/202205/msg00037.html
---
 device-types/net/description.tex | 223 +++++++++++++++++++++++++++++--
 1 file changed, 215 insertions(+), 8 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 77bd847..67bbccf 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -88,6 +88,12 @@ \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_GUEST_UDP_TUNNEL_GSO (46)] Driver can receive GSO packets
+  carried by a UDP tunnel.
+
+\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO (48)] Device can receive GSO packets
+  carried by a UDP tunnel.
+
 \item[VIRTIO_NET_F_DEVICE_STATS(50)] Device can provide device-level statistics
     to the driver through the control virtqueue.
 
@@ -138,12 +144,16 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
 \item[VIRTIO_NET_F_GUEST_UFO] Requires VIRTIO_NET_F_GUEST_CSUM.
 \item[VIRTIO_NET_F_GUEST_USO4] Requires VIRTIO_NET_F_GUEST_CSUM.
 \item[VIRTIO_NET_F_GUEST_USO6] Requires VIRTIO_NET_F_GUEST_CSUM.
+\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO] Requires VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
+   VIRTIO_NET_F_GUEST_USO4 and VIRTIO_NET_F_GUEST_USO6.
 
 \item[VIRTIO_NET_F_HOST_TSO4] Requires VIRTIO_NET_F_CSUM.
 \item[VIRTIO_NET_F_HOST_TSO6] Requires VIRTIO_NET_F_CSUM.
 \item[VIRTIO_NET_F_HOST_ECN] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
 \item[VIRTIO_NET_F_HOST_UFO] Requires VIRTIO_NET_F_CSUM.
 \item[VIRTIO_NET_F_HOST_USO] Requires VIRTIO_NET_F_CSUM.
+\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO] Requires VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_TSO6
+   and VIRTIO_NET_F_HOST_USO.
 
 \item[VIRTIO_NET_F_CTRL_RX] Requires VIRTIO_NET_F_CTRL_VQ.
 \item[VIRTIO_NET_F_CTRL_VLAN] Requires VIRTIO_NET_F_CTRL_VQ.
@@ -381,6 +391,17 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
   TCP), VIRTIO_NET_F_HOST_TSO6 (IPv6 TCP), VIRTIO_NET_F_HOST_UFO
   (UDP fragmentation) and VIRTIO_NET_F_HOST_USO (UDP segmentation) features.
 
+\item If the VIRTIO_NET_F_HOST_TSO6, VIRTIO_NET_F_HOST_TSO4 and VIRTIO_NET_F_HOST_USO
+  segmentation features are negotiated, a driver can
+  use TCP segmentation or UDP segmentation on top of UDP encapsulation
+  offload, when the outer header does not require checksumming - e.g.
+  the outer UDP checksum is zero - by negotiating the
+  VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO feature.
+  GSO over UDP tunnels packets carry two sets of headers: the outer ones
+  and the inner ones. The outer transport protocol is UDP, the inner
+  could be either TCP or UDP. Only a single level of encapsulation
+  offload is supported.
+
 \item The converse features are also available: a driver can save
   the virtual device some work by negotiating these features.\note{For example, a network packet transported between two guests on
 the same system might not need checksumming at all, nor segmentation,
@@ -388,8 +409,9 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
    The VIRTIO_NET_F_GUEST_CSUM feature indicates that partially
   checksummed packets can be received, and if it can do that then
   the VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
-  VIRTIO_NET_F_GUEST_UFO, VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_USO4
-  and VIRTIO_NET_F_GUEST_USO6 are the input equivalents of the features described above.
+  VIRTIO_NET_F_GUEST_UFO, VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_USO4,
+  VIRTIO_NET_F_GUEST_USO6 and VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO
+  are the input equivalents of the features described above.
   See \ref{sec:Device Types / Network Device / Device Operation /
 Setting Up Receive Buffers}~\nameref{sec:Device Types / Network
 Device / Device Operation / Setting Up Receive Buffers} and
@@ -541,6 +563,8 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
 #define VIRTIO_NET_HDR_GSO_UDP         3
 #define VIRTIO_NET_HDR_GSO_TCPV6       4
 #define VIRTIO_NET_HDR_GSO_UDP_L4      5
+#define VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 0x20
+#define VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 0x40
 #define VIRTIO_NET_HDR_GSO_ECN      0x80
         u8 gso_type;
         le16 hdr_len;
@@ -551,6 +575,8 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
         le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
         le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
         le16 padding_reserved;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
+        le16 outer_th_offset    (Only if VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO or VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO negotiated)
+        le16 inner_nh_offset;   (Only if VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO or VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO negotiated)
 };
 \end{lstlisting}
 
@@ -608,6 +634,10 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 followed by the TCP header (with the TCP checksum field 16 bytes
 into that header). \field{csum_start} will be 14+20 = 34 (the TCP
 checksum includes the header), and \field{csum_offset} will be 16.
+If the given packet has the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit set,
+the above checksum fields refer to the inner header checksum, see
+the example below.
 \end{note}
 
 \item If the driver negotiated
@@ -624,6 +654,9 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
     \field{hdr_len} indicates the header length that needs to be replicated
     for each packet. It's the number of bytes from the beginning of the packet
     to the beginning of the transport payload.
+    If the \field{gso_type} has the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
+    VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit set, \field{hdr_len} accounts for
+    all the headers up to and including the inner transport.
     Otherwise, if the VIRTIO_NET_F_GUEST_HDRLEN feature has not been negotiated,
     \field{hdr_len} is a hint to the device as to how much of the header
     needs to be kept to copy into each packet, usually set to the
@@ -644,6 +677,39 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 specifically in the protocol.}.
    \end{itemize}
 
+\item If the driver negotiated the VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO feature and the
+  \field{gso_type} has the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
+  VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit set, the GSO protocol is encapsulated
+  in a UDP tunnel.
+  The outer UDP header must not require checksum validation, i.e. the outer UDP
+  checksum must be a positive zero (0x0) as defined in UDP RFC 768.
+  The other tunnel-related fields indicate how to replicate the packet
+  headers to cut it into smaller packets:
+
+  \begin{itemize}
+  \item \field{outer_th_offset} field indicates the outer transport header within
+      the packet. This field differs from \field{csum_start} as the latter
+      points to the inner transport header within the packet.
+
+  \item \field{inner_nh_offset} field indicates the inner network header within
+      the packet.
+  \end{itemize}
+
+\begin{note}
+For example, consider a partially checksummed TCP (IPv4) packet carried over a
+Geneve UDP tunnel (again IPv4) with no tunnel options. The
+only relevant variable related to the tunnel type is the tunnel header length.
+The packet will have a 14 byte outer ethernet header, 20 byte outer IP header
+followed by the 8 byte UDP header (with a 0 checksum value), 8 byte Geneve header,
+14 byte inner ethernet header, 20 byte inner IP header
+and the TCP header (with the TCP checksum field 16 bytes
+into that header). \field{csum_start} will be 14+20+8+8+14+20 = 84 (the TCP
+checksum includes the header), \field{csum_offset} will be 16.
+\field{inner_nh_offset} will be 14+20+8+8+14 = 62, \field{outer_th_offset} will be
+14+20+8 = 42 and \field{gso_type} will be
+VIRTIO_NET_HDR_GSO_TCPV4 | VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 = 0x21
+\end{note}
+
 \item \field{num_buffers} is set to zero.  This field is unused on transmitted packets.
 
 \item The header and packet are added as one output descriptor to the
@@ -688,6 +754,29 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 driver MUST set the VIRTIO_NET_HDR_GSO_ECN bit in
 \field{gso_type}.
 
+If VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO is negotiated, the driver MAY set
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit
+in \field{gso_type} according to the inner network header protocol type
+to request GSO packets over UDPv4 or UDPv6 tunnel segmentation,
+otherwise the driver MUST NOT set either the
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit
+in \field{gso_type}.
+
+When requesting GSO segmentation over UDP tunnel, the driver MUST SET the
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit if the inner network header is IPv4, i.e. the
+packet is a TCPv4 GSO one, otherwise, if the inner network header is IPv6, the driver
+MUST SET the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit.
+
+The driver MUST NOT send to the device GSO packets over UDP tunnel
+requiring segmentation and outer UDP checksum offload.
+
+The driver MUST NOT set the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit together with VIRTIO_NET_HDR_GSO_UDP, as the
+latter is deprecated in favor of UDP_L4 and no new feature will support it.
+
+The driver MUST NOT set the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit and the
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit together.
+
 If the VIRTIO_NET_F_CSUM feature has been negotiated, the
 driver MAY set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in
 \field{flags}, if so:
@@ -696,7 +785,9 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 	offset \field{csum_offset} from \field{csum_start} as well as all
 	preceding offsets;
 \begin{note}
-If \field{gso_type} differs from VIRTIO_NET_HDR_GSO_NONE, \field{csum_offset}
+If \field{gso_type} differs from VIRTIO_NET_HDR_GSO_NONE and the
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
+bit are not set in \field{gso_type}, \field{csum_offset}
 points to the only transport header present in the packet, and there are no
 additional preceding checksums validated by VIRTIO_NET_HDR_F_NEEDS_CSUM.
 \end{note}
@@ -725,7 +816,10 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 \item If the VIRTIO_NET_F_GUEST_HDRLEN feature has been negotiated,
 	and \field{gso_type} differs from VIRTIO_NET_HDR_GSO_NONE,
 	the driver MUST set \field{hdr_len} to a value equal to the length
-	of the headers, including the transport header.
+	of the headers, including the transport header. If \field{gso_type}
+	has the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
+	VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit set, \field{hdr_len} includes
+	the inner transport header.
 
 \item If the VIRTIO_NET_F_GUEST_HDRLEN feature has not been negotiated,
 	or \field{gso_type} is VIRTIO_NET_HDR_GSO_NONE,
@@ -734,12 +828,48 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 	header.
 \end{itemize}
 
+If the VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO option has been negotiated, the
+driver MAY set the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit in \field{gso_type}, if so:
+\begin{itemize}
+\item the driver MUST set \field{outer_th_offset} to the outer UDP header
+  offset and \field{inner_nh_offset} to the inner network header offset.
+  The \field{csum_start} and \field{csum_offset} fields point respectively
+  to the inner transport header and inner transport checksum field.
+\end{itemize}
+
+If the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit in \field{gso_type} are set, the
+outer UDP header MUST NOT require checksum validation. That is, the
+outer UDP checksum value MUST be 0 or the validated complete checksum
+for such header.
+
+\begin{note}
+The valid complete checksum of the outer UDP header of individual segments
+can be computed by the driver prior to segmentation only if the GSO packet
+size is a multiple of \field{gso_size}, because then all segments
+have the same size and thus all data included in the outer UDP
+checksum is the same for every segment. These pre-computed segment
+length and checksum fields are different from those of the GSO
+packet.
+In this scenario the outer UDP header of the GSO packet must carry the
+segmented UDP packet length.
+\end{note}
+
+If the VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO option has not
+been negotiated, the driver MUST NOT set either the VIRTIO_NET_HDR_F_GSO_UDP_TUNNEL_IPV4
+bit or the VIRTIO_NET_HDR_F_GSO_UDP_TUNNEL_IPV6 in \field{gso_type}.
+
 The driver SHOULD accept the VIRTIO_NET_F_GUEST_HDRLEN feature if it has
 been offered, and if it's able to provide the exact header length.
 
 The driver MUST NOT set the VIRTIO_NET_HDR_F_DATA_VALID and
 VIRTIO_NET_HDR_F_RSC_INFO bits in \field{flags}.
 
+The driver MUST NOT set the VIRTIO_NET_HDR_F_DATA_VALID bit in \field{flags}
+together with the VIRTIO_NET_HDR_F_GSO_UDP_TUNNEL_IPV4 bit or the
+VIRTIO_NET_HDR_F_GSO_UDP_TUNNEL_IPV6 bit in \field{gso_type}.
+
 \devicenormative{\paragraph}{Packet Transmission}{Device Types / Network Device / Device Operation / Packet Transmission}
 The device MUST ignore \field{flag} bits that it does not recognize.
 
@@ -769,6 +899,25 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 	\end{note}
 \end{itemize}
 
+If both the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit and
+the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit in in \field{gso_type} are set,
+the device MUST NOT accept the packet.
+
+If the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit and the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
+bit in \field{gso_type} are not set, the device MUST NOT use the
+\field{outer_th_offset} and \field{inner_nh_offset}.
+
+If either the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
+the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit in \field{gso_type} are set, and any of
+the following is true:
+\begin{itemize}
+\item the VIRTIO_NET_HDR_F_NEEDS_CSUM is not set in \field{flags}
+\item the VIRTIO_NET_HDR_F_DATA_VALID is set in \field{flags}
+\item the \field{gso_type} excluding the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4
+bit and the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit is VIRTIO_NET_HDR_GSO_NONE
+\end{itemize}
+the device MUST NOT accept the packet.
+
 If VIRTIO_NET_HDR_F_NEEDS_CSUM is not set, the device MUST NOT
 rely on the packet checksum being correct.
 \paragraph{Packet Transmission Interrupt}\label{sec:Device Types / Network Device / Device Operation / Packet Transmission / Packet Transmission Interrupt}
@@ -875,8 +1024,8 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
   has been validated.
 \end{enumerate}
 
-Additionally, VIRTIO_NET_F_GUEST_CSUM, TSO4, TSO6, UDP and ECN
-features enable receive checksum, large receive offload and ECN
+Additionally, VIRTIO_NET_F_GUEST_CSUM, TSO4, TSO6, UDP, UDP_TUNNEL
+and ECN features enable receive checksum, large receive offload and ECN
 support which are the input equivalents of the transmit checksum,
 transmit segmentation offloading and ECN features, as described
 in \ref{sec:Device Types / Network Device / Device Operation /
@@ -901,10 +1050,25 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
   then \field{csum_start} and \field{csum_offset} indicate how to calculate it
   (see Packet Transmission point 1).
 \begin{note}
-If \field{gso_type} differs from VIRTIO_NET_HDR_GSO_NONE, \field{csum_offset}
+If \field{gso_type} differs from VIRTIO_NET_HDR_GSO_NONE and the
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
+bit are not set, \field{csum_offset}
 points to the only transport header present in the packet, and there are no
 additional preceding checksums validated by VIRTIO_NET_HDR_F_NEEDS_CSUM.
 \end{note}
+\item If the VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO option was negotiated and
+  \field{gso_type} is not VIRTIO_NET_HDR_GSO_NONE, the
+  VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
+  bit MAY be set. In such case the \field{outer_th_offset} and
+  \field{inner_nh_offset} fields indicate the corresponding
+  headers information.
+\begin{note}
+If the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
+bit are set in \field{gso_type}, the \field{csum_start} field refers to
+the inner transport header offset (see Packet Transmission point 1)
+and only the inner transport header checksum has been validated by
+VIRTIO_NET_HDR_F_NEEDS_CSUM.
+\end{note}
 \end{enumerate}
 
 If applicable, the device calculates per-packet hash for incoming packets as
@@ -948,6 +1112,10 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 If none of VIRTIO_NET_F_GUEST_USO4 or VIRTIO_NET_F_GUEST_USO6 have been negotiated,
 the device MUST NOT set \field{gso_type} to VIRTIO_NET_HDR_GSO_UDP_L4.
 
+If VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO is not negotiated, the device MUST NOT set
+either the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit in \field{gso_type}.
+
 The device SHOULD NOT send to the driver TCP packets requiring segmentation offload
 which have the Explicit Congestion Notification bit set, unless the
 VIRTIO_NET_F_GUEST_ECN feature is negotiated, in which case the
@@ -971,6 +1139,18 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 	fully checksummed packet;
 \end{enumerate}
 
+The device MUST NOT send to the driver GSO packets encapsulated in UDP
+tunnel and requiring segmentation offload, unless the
+VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO is negotiated, in which case the device MUST set
+the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
+bit in \field{gso_type} according to the inner network header protocol type,
+MUST set the \field{outer_th_offset} and \field{inner_nh_offset} fields
+to the corresponding header information, and the outer UDP header MUST NOT
+require checksum offload.
+
+The device MUST NOT send the driver GSO packets encapsulated in UDP
+tunnel and requiring segmentation and outer checksum offload.
+
 If none of the VIRTIO_NET_F_GUEST_TSO4, TSO6, UFO, USO4 or USO6 options have
 been negotiated, the device MUST set \field{gso_type} to
 VIRTIO_NET_HDR_GSO_NONE.
@@ -989,7 +1169,9 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 If one of the VIRTIO_NET_F_GUEST_TSO4, TSO6, UFO, USO4 or USO6 options have
 been negotiated, the device SHOULD set \field{hdr_len} to a value
 not less than the length of the headers, including the transport
-header.
+header. If \field{gso_type} has the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit
+or the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit set, the referenced transport
+header is the inner one.
 
 If the VIRTIO_NET_F_GUEST_CSUM feature has been negotiated, the
 device MAY set the VIRTIO_NET_HDR_F_DATA_VALID bit in
@@ -997,6 +1179,11 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 checksum. One level of checksum is validated: in case of multiple
 encapsulated protocols the outermost one.
 
+If either the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit in \field{gso_type} are set,
+the device MUST NOT set the VIRTIO_NET_HDR_F_DATA_VALID bit in
+\field{flags}.
+
 \drivernormative{\paragraph}{Processing of Incoming
 Packets}{Device Types / Network Device / Device Operation /
 Processing of Incoming Packets}
@@ -1019,6 +1206,25 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 VIRTIO_NET_HDR_F_DATA_VALID is set, the driver MUST NOT
 rely on the packet checksum being correct.
 
+If both the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit and
+the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit in in \field{gso_type} are set,
+the driver MUST NOT accept the packet.
+
+If the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
+bit in \field{gso_type} are not set, the driver MUST NOT use the
+\field{outer_th_offset} and \field{inner_nh_offset}.
+
+If either the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
+the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit in \field{gso_type} are set, and any of
+the following is true:
+\begin{itemize}
+\item the VIRTIO_NET_HDR_F_NEEDS_CSUM bit is not set in \field{flags}
+\item the VIRTIO_NET_HDR_F_DATA_VALID bit is set in \field{flags}
+\item the \field{gso_type} excluding the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4
+bit and the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit is VIRTIO_NET_HDR_GSO_NONE
+\end{itemize}
+the driver MUST NOT accept the packet.
+
 \paragraph{Hash calculation for incoming packets}
 \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets}
 
@@ -1862,6 +2068,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 #define VIRTIO_NET_F_GUEST_TSO6       8
 #define VIRTIO_NET_F_GUEST_ECN        9
 #define VIRTIO_NET_F_GUEST_UFO        10
+#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO  46
 #define VIRTIO_NET_F_GUEST_USO4       54
 #define VIRTIO_NET_F_GUEST_USO6       55
 
-- 
2.45.2

Re: [PATCH v11 3/4] virtio-net: define UDP tunnel segmentation offload feature

[Jason Wang]

On Wed, Nov 27, 2024 at 5:37 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> The VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV{4,6} are two gso_type flags
> allowing respectively GSO over UDPv4 tunnel and GSO over UDPv6 tunnel.
> They can be negotiated on both the host and guest sides.
>
> One constraint addressed here is that the virtio side (either device or
> driver) receiving a UDP tunneled GSO packet must be able to reconstruct
> completely the inner and outer headers offset - to allow for later GSO.
>
> To accommodate such need, new fields are introduced in the virtio_net
> header: outer_th_offset and inner_nh_offset.
> They map directly to the corresponding header information. The inner
> transport header is implied by the (inner) checksum offload.
>
> Those fields are required because a virtio device H/W implementation
> performing segmentation for UDP tunneled packet will need to touch
> the outer transport protocol (for the UDP length filed), the
> inner network protocol (for the total length field, in the IPv4
> case).
>
> Note that segmentation will additionally need to touch
> the outer network protocol and the inner transport protocol. The first
> is implied/easily found with trivial parsing, the latter is identified
> by the existing csum_start field.
>
> Note that there is no concept of UDP tunnel type negotiation (e.g.
> vxlan, geneve, vxlan-gpe, etc.), as a virtio device H/W implementation
> can perform segmentation for every possible UDP-tunnel given the
> specified new fields.
> In the reverse direction, if a virtio device H/W implementation receives
> some traffic for an unknown or unsupported UDP tunnel, it will simply
> not aggregate the wire packet in a GSO one.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v10 -> v11:
>  - clarified only a single encapsulation level is supported
>  - s/TCP or UDP GSO packets/GSO packets/
>
> v9 -> v10:
>  - mandate DATA_VALID and VIRTIO_NET_HDR_F_NEEDS_CSUM as mutually exclusive
>    for GSO over UDP tunnels packets.
>  - clarified that hdr_len includes the inner transport header for GSO over
>    UDP tunnels packets
>  - highlighted that csum_offset refers to the inner header in a specific
>    note
>
> v8 -> v9:
>  - rebased on top of virtio-1.4, changed udp-tunnel features number
>    to avoid conflicts/duplications
>  - fixed typos:
>    VIRTIO_NET_HDR_F_UDP_TUNNEL_IPV4 -> VIRTIO_NET_HDR_F_GSO_UDP_TUNNEL_IPV4
>    VIRTIO_NET_HDR_F_UDP_TUNNEL_IPV6 -> VIRTIO_NET_HDR_F_GSO_UDP_TUNNEL_IPV6
>  - specified strict validation on RX side (both driver and device)
>    for UDP tunneled packet: NEEDS_CSUM and plain GSO are mandatory for UDP
>    tunnel GSO packets. The driver is not allowed to set DATA_VALID for UDP
>    tunnel GSO packets, the driver can set both DATA_VALID & NEEDS_CSUM.
>
> v7 -> v8:
>  - dropped 'Note that' in note
>  - NOT set neither A nor B -> NOT set either A or B
>  - specify that 'gso_partial' needs uh->len set to the wire one
>
> v6 -> v7:
>  - replaced inner_protocol_type with VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 &&
>    VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4
>  - UDP_TUNNEL now depends on all the TCP and USO segmentation features
>  - reworded the 'VIRTIO_NET_HDR_GSO_UDP_TUNNEL' bit driver normative
>  - specified that  csum_start/offset points to the inner header in the
>    driver normative, too
>  - new fields presence depends on either VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO or
>    VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO
>  - e.g. ->  i.e.
>  - zero -> positive zero (0x0)
>  - checksumming -> checksum validation
>  - explain why we the outer UDP csum can be precomputed only
>    when the GSO packet length is a multiple of gso_size
>  - added an example note for the tunnel-related fields value
>  https://lore.kernel.org/virtio-comment/cover.1722252302.git.pabeni@redhat.com/T/#t
>
> v5 -> v6:
>  - split in 2 patches
>  - dropped the unneeded outer_mh_offset field
>  https://lore.kernel.org/virtio-comment/56ccba136272593d0d2af0303600c06f26bd84a1.1718621522.git.pabeni@redhat.com/
>
> v4 -> v5:
>  - add separate negotiation for UDP_TUNNEL_CSUM
>  - much more verbose specification of outer csum handling
>  - UDP_TUNNEL_CSUM requires GSO_UDP_TUNNEL
>  https://lore.kernel.org/virtio-comment/cb89d3d6b6a7e4a7f13e60dd3fded5e950727890.1716448766.git.pabeni@redhat.com/
>
> v3 -> v4:
>  - new virtio_net_hdr fields depends on F_HOST_UDP_TUNNEL_GSO or
>    F_GUEST_UDP_TUNNEL_GSO (Stefano)
>  - Extended the changelog to answer Jason's questions.
>  - Clarified outer csum handling (Jason)
>  https://lore.kernel.org/virtio-dev/f0bf2db203f8061a3ecb50b7a48cf26d8f3c7f68.1716193086.git.pabeni@redhat.com
>
> v2 -> v3:
>  - UDP_TUNNEL -> UDP_TUNNEL_GSO
>  - add explicit fields for the inner meta-data
>  - more verbose changelog
>  https://lists.oasis-open.org/archives/virtio-dev/202206/msg00026.html
>
> v1 -> v2:
>  - explicitly state that the outer header probing is mandatory
>  - explicitly state that GSO_UDP is not allowed with GSO_UDP_TUNNEL
>  - clarify hdr_len usage
>  - clarify UDP_TUNNEL_CSUM bit usage
>  - fix a few typos
>  https://lists.oasis-open.org/archives/virtio-dev/202205/msg00037.html
> ---
>  device-types/net/description.tex | 223 +++++++++++++++++++++++++++++--
>  1 file changed, 215 insertions(+), 8 deletions(-)

Reviewed-by: Jason Wang <jasowang@redhat.com>

Thanks

RE: [PATCH v11 3/4] virtio-net: define UDP tunnel segmentation offload feature

[Parav Pandit]

Hi Paolo, Michael,

> From: Paolo Abeni <pabeni@redhat.com>
> Sent: Wednesday, November 27, 2024 3:07 PM
> 
> 
> +\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO (46)] Driver can receive
> GSO
> +packets
> +  carried by a UDP tunnel.
> +
> +\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO (48)] Device can receive
> GSO
> +packets
> +  carried by a UDP tunnel.

Feature bits 42 to 49 are not applicable to device specific type. They are reserved.
Please see the note. 

Current spec description is:

0 to 23, and 50 to 127 Feature bits for the specific device type
24 to 41 Feature bits reserved for extensions to the queue and feature negotiation mechanisms, see 6
42 to 49, and 128 and above Feature bits reserved for future extensions.

So 2 bits of these patch and of patch 4, 46 to 49 to be shifted to bits 65 to 68.

Or an alternative would be to amend,

Bits 46 to 127 for specific device type,
And 42 to 45 and 128 reserved for future extensions.

Michael,
Please let me know so that I can have the follow up patch to this one as voting is started.
And the fix is required for it.

Is there any historic reason for 42-49 as reserved, or it can be used as proposed in this patch?
If it was only kept as elastic buffer for device generic, and device type to grow it is probably ok.
Using 46 to 49 has small advantage to reuse in the offloads bit as below.
Please confirm.


> @@ -1862,6 +2068,7 @@ \subsubsection{Control
> Virtqueue}\label{sec:Device Types / Network Device / Devi
>  #define VIRTIO_NET_F_GUEST_TSO6       8
>  #define VIRTIO_NET_F_GUEST_ECN        9
>  #define VIRTIO_NET_F_GUEST_UFO        10
> +#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO  46
>  #define VIRTIO_NET_F_GUEST_USO4       54
>  #define VIRTIO_NET_F_GUEST_USO6       55

Re: [PATCH v11 3/4] virtio-net: define UDP tunnel segmentation offload feature

[Paolo Abeni]

On 1/19/25 9:06 AM, Parav Pandit wrote:
> Hi Paolo, Michael,
> 
>> From: Paolo Abeni <pabeni@redhat.com>
>> Sent: Wednesday, November 27, 2024 3:07 PM
>>
>>
>> +\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO (46)] Driver can receive
>> GSO
>> +packets
>> +  carried by a UDP tunnel.
>> +
>> +\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO (48)] Device can receive
>> GSO
>> +packets
>> +  carried by a UDP tunnel.
> 
> Feature bits 42 to 49 are not applicable to device specific type. They are reserved.
> Please see the note. 
> 
> Current spec description is:
> 
> 0 to 23, and 50 to 127 Feature bits for the specific device type
> 24 to 41 Feature bits reserved for extensions to the queue and feature negotiation mechanisms, see 6
> 42 to 49, and 128 and above Feature bits reserved for future extensions.

Thank you for the head-up! I missed the above point.

> So 2 bits of these patch and of patch 4, 46 to 49 to be shifted to bits 65 to 68.

I can cook and send a new version of the series with the above change,
but it will take a few days as I can't start working on it before
Thursday and I want/intend to update and test the PoC code accordingly.

/P

RE: [PATCH v11 3/4] virtio-net: define UDP tunnel segmentation offload feature

[Parav Pandit]

> From: Paolo Abeni <pabeni@redhat.com>
> Sent: Monday, January 20, 2025 2:15 PM
> To: Parav Pandit <parav@nvidia.com>; virtio-comment@lists.linux.dev
> Cc: maxime.coquelin@redhat.com; Eelco Chaudron <echaudro@redhat.com>;
> Jason Wang <jasowang@redhat.com>; Stefano Garzarella
> <sgarzare@redhat.com>; Willem de Bruijn <willemb@google.com>;
> kshankar@marvell.com
> Subject: Re: [PATCH v11 3/4] virtio-net: define UDP tunnel segmentation
> offload feature
> 
> On 1/19/25 9:06 AM, Parav Pandit wrote:
> > Hi Paolo, Michael,
> >
> >> From: Paolo Abeni <pabeni@redhat.com>
> >> Sent: Wednesday, November 27, 2024 3:07 PM
> >>
> >>
> >> +\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO (46)] Driver can receive
> >> GSO
> >> +packets
> >> +  carried by a UDP tunnel.
> >> +
> >> +\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO (48)] Device can receive
> >> GSO
> >> +packets
> >> +  carried by a UDP tunnel.
> >
> > Feature bits 42 to 49 are not applicable to device specific type. They are
> reserved.
> > Please see the note.
> >
> > Current spec description is:
> >
> > 0 to 23, and 50 to 127 Feature bits for the specific device type
> > 24 to 41 Feature bits reserved for extensions to the queue and feature
> > negotiation mechanisms, see 6
> > 42 to 49, and 128 and above Feature bits reserved for future extensions.
> 
> Thank you for the head-up! I missed the above point.
> 
> > So 2 bits of these patch and of patch 4, 46 to 49 to be shifted to bits 65 to
> 68.
> 
> I can cook and send a new version of the series with the above change, but it
> will take a few days as I can't start working on it before Thursday and I
> want/intend to update and test the PoC code accordingly.
> 
Please do not send now as the voting has started.
We will be able to handle under a separate patch+vote in as fixup soon.
This will avoid voting related rework.

I would like to have Miachel's feedback if there was any past history for reserving bits 42-49, or it was just flexibility..

RE: [PATCH v11 3/4] virtio-net: define UDP tunnel segmentation offload feature

[Cornelia Huck]

On Sun, Jan 19 2025, Parav Pandit <parav@nvidia.com> wrote:

> Hi Paolo, Michael,
>
>> From: Paolo Abeni <pabeni@redhat.com>
>> Sent: Wednesday, November 27, 2024 3:07 PM
>> 
>> 
>> +\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO (46)] Driver can receive
>> GSO
>> +packets
>> +  carried by a UDP tunnel.
>> +
>> +\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO (48)] Device can receive
>> GSO
>> +packets
>> +  carried by a UDP tunnel.
>
> Feature bits 42 to 49 are not applicable to device specific type. They are reserved.
> Please see the note. 
>
> Current spec description is:
>
> 0 to 23, and 50 to 127 Feature bits for the specific device type
> 24 to 41 Feature bits reserved for extensions to the queue and feature negotiation mechanisms, see 6
> 42 to 49, and 128 and above Feature bits reserved for future extensions.
>
> So 2 bits of these patch and of patch 4, 46 to 49 to be shifted to bits 65 to 68.
>
> Or an alternative would be to amend,
>
> Bits 46 to 127 for specific device type,
> And 42 to 45 and 128 reserved for future extensions.
>
> Michael,
> Please let me know so that I can have the follow up patch to this one as voting is started.
> And the fix is required for it.
>
> Is there any historic reason for 42-49 as reserved, or it can be used as proposed in this patch?
> If it was only kept as elastic buffer for device generic, and device type to grow it is probably ok.
> Using 46 to 49 has small advantage to reuse in the offloads bit as below.
> Please confirm.

See the reasoning given in 88895f56e642 ("Reserve more feature bits for
device type usage") -- the idea was to push out the point in time when
generic feature bits will have to be in the bit area beyond 63. Given
that we only used one of those generic bits in the meantime, I assume it
will still be some time before we run out of bits, even if we let the
net driver grab some bits there. Still, I'd probably prefer that the net
device used different bits. [And issues like this are easy to miss
during review unfortunately.]

However, if the device was to use different bits, the clean solution
would be to withdraw, respin, and revote. If we only changed the
reserved ranges, we'd be fine with a change on top, so that would be
less hassle in the short team. If we decide that we can live with the
smaller feature bit space below 63, I'd be fine with that solution.

>
>
>> @@ -1862,6 +2068,7 @@ \subsubsection{Control
>> Virtqueue}\label{sec:Device Types / Network Device / Devi
>>  #define VIRTIO_NET_F_GUEST_TSO6       8
>>  #define VIRTIO_NET_F_GUEST_ECN        9
>>  #define VIRTIO_NET_F_GUEST_UFO        10
>> +#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO  46
>>  #define VIRTIO_NET_F_GUEST_USO4       54
>>  #define VIRTIO_NET_F_GUEST_USO6       55

RE: [PATCH v11 3/4] virtio-net: define UDP tunnel segmentation offload feature

[Parav Pandit]

> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Monday, January 20, 2025 2:34 PM
> 
> On Sun, Jan 19 2025, Parav Pandit <parav@nvidia.com> wrote:
> 
> > Hi Paolo, Michael,
> >
> >> From: Paolo Abeni <pabeni@redhat.com>
> >> Sent: Wednesday, November 27, 2024 3:07 PM
> >>
> >>
> >> +\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO (46)] Driver can receive
> >> GSO
> >> +packets
> >> +  carried by a UDP tunnel.
> >> +
> >> +\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO (48)] Device can receive
> >> GSO
> >> +packets
> >> +  carried by a UDP tunnel.
> >
> > Feature bits 42 to 49 are not applicable to device specific type. They are
> reserved.
> > Please see the note.
> >
> > Current spec description is:
> >
> > 0 to 23, and 50 to 127 Feature bits for the specific device type
> > 24 to 41 Feature bits reserved for extensions to the queue and feature
> > negotiation mechanisms, see 6
> > 42 to 49, and 128 and above Feature bits reserved for future extensions.
> >
> > So 2 bits of these patch and of patch 4, 46 to 49 to be shifted to bits 65 to
> 68.
> >
> > Or an alternative would be to amend,
> >
> > Bits 46 to 127 for specific device type, And 42 to 45 and 128 reserved
> > for future extensions.
> >
> > Michael,
> > Please let me know so that I can have the follow up patch to this one as
> voting is started.
> > And the fix is required for it.
> >
> > Is there any historic reason for 42-49 as reserved, or it can be used as
> proposed in this patch?
> > If it was only kept as elastic buffer for device generic, and device type to
> grow it is probably ok.
> > Using 46 to 49 has small advantage to reuse in the offloads bit as below.
> > Please confirm.
> 
> See the reasoning given in 88895f56e642 ("Reserve more feature bits for
> device type usage") -- the idea was to push out the point in time when
> generic feature bits will have to be in the bit area beyond 63. Given that we
> only used one of those generic bits in the meantime, I assume it will still be
> some time before we run out of bits, even if we let the net driver grab some
> bits there. Still, I'd probably prefer that the net device used different bits.
> [And issues like this are easy to miss during review unfortunately.]
> 
I made assumption that one would have picked the device specific rage.
I missed to detect this early enough, my bad. :(

Given that we are only left with 4 bits in range 42 to 45, at generic level, it seems wise for device-type to consume feature bits in its defined range.
For virtio-net, bits 65 and higher are preferred.

> However, if the device was to use different bits, the clean solution would be
> to withdraw, respin, and revote. If we only changed the reserved ranges,
> we'd be fine with a change on top, so that would be less hassle in the short
> team. If we decide that we can live with the smaller feature bit space below
> 63, I'd be fine with that solution.
>

I am fine either way to merge this and fix immediately for using bits 65 and higher.
My preference is to use bits 65 and higher.

Or as you suggested to respin and revote right away to forward with bit 65.

Please let me know.
 
> >
> >
> >> @@ -1862,6 +2068,7 @@ \subsubsection{Control
> >> Virtqueue}\label{sec:Device Types / Network Device / Devi
> >>  #define VIRTIO_NET_F_GUEST_TSO6       8
> >>  #define VIRTIO_NET_F_GUEST_ECN        9
> >>  #define VIRTIO_NET_F_GUEST_UFO        10
> >> +#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO  46
> >>  #define VIRTIO_NET_F_GUEST_USO4       54
> >>  #define VIRTIO_NET_F_GUEST_USO6       55

RE: [PATCH v11 3/4] virtio-net: define UDP tunnel segmentation offload feature

[Cornelia Huck]

On Mon, Jan 20 2025, Parav Pandit <parav@nvidia.com> wrote:

>> From: Cornelia Huck <cohuck@redhat.com>
>> Sent: Monday, January 20, 2025 2:34 PM
>> 
>> On Sun, Jan 19 2025, Parav Pandit <parav@nvidia.com> wrote:
>> 
>> > Hi Paolo, Michael,
>> >
>> >> From: Paolo Abeni <pabeni@redhat.com>
>> >> Sent: Wednesday, November 27, 2024 3:07 PM
>> >>
>> >>
>> >> +\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO (46)] Driver can receive
>> >> GSO
>> >> +packets
>> >> +  carried by a UDP tunnel.
>> >> +
>> >> +\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO (48)] Device can receive
>> >> GSO
>> >> +packets
>> >> +  carried by a UDP tunnel.
>> >
>> > Feature bits 42 to 49 are not applicable to device specific type. They are
>> reserved.
>> > Please see the note.
>> >
>> > Current spec description is:
>> >
>> > 0 to 23, and 50 to 127 Feature bits for the specific device type
>> > 24 to 41 Feature bits reserved for extensions to the queue and feature
>> > negotiation mechanisms, see 6
>> > 42 to 49, and 128 and above Feature bits reserved for future extensions.
>> >
>> > So 2 bits of these patch and of patch 4, 46 to 49 to be shifted to bits 65 to
>> 68.
>> >
>> > Or an alternative would be to amend,
>> >
>> > Bits 46 to 127 for specific device type, And 42 to 45 and 128 reserved
>> > for future extensions.
>> >
>> > Michael,
>> > Please let me know so that I can have the follow up patch to this one as
>> voting is started.
>> > And the fix is required for it.
>> >
>> > Is there any historic reason for 42-49 as reserved, or it can be used as
>> proposed in this patch?
>> > If it was only kept as elastic buffer for device generic, and device type to
>> grow it is probably ok.
>> > Using 46 to 49 has small advantage to reuse in the offloads bit as below.
>> > Please confirm.
>> 
>> See the reasoning given in 88895f56e642 ("Reserve more feature bits for
>> device type usage") -- the idea was to push out the point in time when
>> generic feature bits will have to be in the bit area beyond 63. Given that we
>> only used one of those generic bits in the meantime, I assume it will still be
>> some time before we run out of bits, even if we let the net driver grab some
>> bits there. Still, I'd probably prefer that the net device used different bits.
>> [And issues like this are easy to miss during review unfortunately.]
>> 
> I made assumption that one would have picked the device specific rage.
> I missed to detect this early enough, my bad. :(
>
> Given that we are only left with 4 bits in range 42 to 45, at generic level, it seems wise for device-type to consume feature bits in its defined range.
> For virtio-net, bits 65 and higher are preferred.
>
>> However, if the device was to use different bits, the clean solution would be
>> to withdraw, respin, and revote. If we only changed the reserved ranges,
>> we'd be fine with a change on top, so that would be less hassle in the short
>> team. If we decide that we can live with the smaller feature bit space below
>> 63, I'd be fine with that solution.
>>
>
> I am fine either way to merge this and fix immediately for using bits 65 and higher.
> My preference is to use bits 65 and higher.
>
> Or as you suggested to respin and revote right away to forward with bit 65.
>
> Please let me know.

Personally, I'd prefer to handle this in an "atomic" manner, i.e. making
sure that no version ever has the wrong bit numbers... maybe if you did a
fixup patch that changes the numbers, and commit these patches + the
fixup all at once? Less churn than doing the respin and revote dance.

>  
>> >
>> >
>> >> @@ -1862,6 +2068,7 @@ \subsubsection{Control
>> >> Virtqueue}\label{sec:Device Types / Network Device / Devi
>> >>  #define VIRTIO_NET_F_GUEST_TSO6       8
>> >>  #define VIRTIO_NET_F_GUEST_ECN        9
>> >>  #define VIRTIO_NET_F_GUEST_UFO        10
>> >> +#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO  46
>> >>  #define VIRTIO_NET_F_GUEST_USO4       54
>> >>  #define VIRTIO_NET_F_GUEST_USO6       55

RE: [PATCH v11 3/4] virtio-net: define UDP tunnel segmentation offload feature

[Parav Pandit]

> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Monday, January 20, 2025 5:50 PM
> 
> On Mon, Jan 20 2025, Parav Pandit <parav@nvidia.com> wrote:
> 
> >> From: Cornelia Huck <cohuck@redhat.com>
> >> Sent: Monday, January 20, 2025 2:34 PM
> >>
> >> On Sun, Jan 19 2025, Parav Pandit <parav@nvidia.com> wrote:
> >>
> >> > Hi Paolo, Michael,
> >> >
> >> >> From: Paolo Abeni <pabeni@redhat.com>
> >> >> Sent: Wednesday, November 27, 2024 3:07 PM
> >> >>
> >> >>
> >> >> +\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO (46)] Driver can
> receive
> >> >> GSO
> >> >> +packets
> >> >> +  carried by a UDP tunnel.
> >> >> +
> >> >> +\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO (48)] Device can
> receive
> >> >> GSO
> >> >> +packets
> >> >> +  carried by a UDP tunnel.
> >> >
> >> > Feature bits 42 to 49 are not applicable to device specific type.
> >> > They are
> >> reserved.
> >> > Please see the note.
> >> >
> >> > Current spec description is:
> >> >
> >> > 0 to 23, and 50 to 127 Feature bits for the specific device type
> >> > 24 to 41 Feature bits reserved for extensions to the queue and
> >> > feature negotiation mechanisms, see 6
> >> > 42 to 49, and 128 and above Feature bits reserved for future extensions.
> >> >
> >> > So 2 bits of these patch and of patch 4, 46 to 49 to be shifted to
> >> > bits 65 to
> >> 68.
> >> >
> >> > Or an alternative would be to amend,
> >> >
> >> > Bits 46 to 127 for specific device type, And 42 to 45 and 128
> >> > reserved for future extensions.
> >> >
> >> > Michael,
> >> > Please let me know so that I can have the follow up patch to this
> >> > one as
> >> voting is started.
> >> > And the fix is required for it.
> >> >
> >> > Is there any historic reason for 42-49 as reserved, or it can be
> >> > used as
> >> proposed in this patch?
> >> > If it was only kept as elastic buffer for device generic, and
> >> > device type to
> >> grow it is probably ok.
> >> > Using 46 to 49 has small advantage to reuse in the offloads bit as below.
> >> > Please confirm.
> >>
> >> See the reasoning given in 88895f56e642 ("Reserve more feature bits
> >> for device type usage") -- the idea was to push out the point in time
> >> when generic feature bits will have to be in the bit area beyond 63.
> >> Given that we only used one of those generic bits in the meantime, I
> >> assume it will still be some time before we run out of bits, even if
> >> we let the net driver grab some bits there. Still, I'd probably prefer that the
> net device used different bits.
> >> [And issues like this are easy to miss during review unfortunately.]
> >>
> > I made assumption that one would have picked the device specific rage.
> > I missed to detect this early enough, my bad. :(
> >
> > Given that we are only left with 4 bits in range 42 to 45, at generic level, it
> seems wise for device-type to consume feature bits in its defined range.
> > For virtio-net, bits 65 and higher are preferred.
> >
> >> However, if the device was to use different bits, the clean solution
> >> would be to withdraw, respin, and revote. If we only changed the
> >> reserved ranges, we'd be fine with a change on top, so that would be
> >> less hassle in the short team. If we decide that we can live with the
> >> smaller feature bit space below 63, I'd be fine with that solution.
> >>
> >
> > I am fine either way to merge this and fix immediately for using bits 65 and
> higher.
> > My preference is to use bits 65 and higher.
> >
> > Or as you suggested to respin and revote right away to forward with bit 65.
> >
> > Please let me know.
> 
> Personally, I'd prefer to handle this in an "atomic" manner, i.e. making sure
> that no version ever has the wrong bit numbers... maybe if you did a fixup
> patch that changes the numbers, and commit these patches + the fixup all at
> once? Less churn than doing the respin and revote dance.
> 
Here it is.
https://lore.kernel.org/virtio-comment/20250120141052.877355-1-parav@nvidia.com/T/#u

Do we need to open a new voting request for it? yes?

> >
> >> >
> >> >
> >> >> @@ -1862,6 +2068,7 @@ \subsubsection{Control
> >> >> Virtqueue}\label{sec:Device Types / Network Device / Devi
> >> >>  #define VIRTIO_NET_F_GUEST_TSO6       8
> >> >>  #define VIRTIO_NET_F_GUEST_ECN        9
> >> >>  #define VIRTIO_NET_F_GUEST_UFO        10
> >> >> +#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO  46
> >> >>  #define VIRTIO_NET_F_GUEST_USO4       54
> >> >>  #define VIRTIO_NET_F_GUEST_USO6       55

RE: [PATCH v11 3/4] virtio-net: define UDP tunnel segmentation offload feature

[Cornelia Huck]

On Mon, Jan 20 2025, Parav Pandit <parav@nvidia.com> wrote:

>> From: Cornelia Huck <cohuck@redhat.com>
>> Sent: Monday, January 20, 2025 5:50 PM
>> Personally, I'd prefer to handle this in an "atomic" manner, i.e. making sure
>> that no version ever has the wrong bit numbers... maybe if you did a fixup
>> patch that changes the numbers, and commit these patches + the fixup all at
>> once? Less churn than doing the respin and revote dance.
>> 
> Here it is.
> https://lore.kernel.org/virtio-comment/20250120141052.877355-1-parav@nvidia.com/T/#u
>
> Do we need to open a new voting request for it? yes?

I guess if you get an ack by the patchset author you could do it as an
editorial update? Michael?

Re: [PATCH v11 3/4] virtio-net: define UDP tunnel segmentation offload feature

[Paolo Abeni]

On 1/20/25 3:19 PM, Parav Pandit wrote:
> 
>> From: Cornelia Huck <cohuck@redhat.com>
>> Sent: Monday, January 20, 2025 5:50 PM
>>
>> On Mon, Jan 20 2025, Parav Pandit <parav@nvidia.com> wrote:
>>
>>>> From: Cornelia Huck <cohuck@redhat.com>
>>>> Sent: Monday, January 20, 2025 2:34 PM
>>>>
>>>> On Sun, Jan 19 2025, Parav Pandit <parav@nvidia.com> wrote:
>>>>
>>>>> Hi Paolo, Michael,
>>>>>
>>>>>> From: Paolo Abeni <pabeni@redhat.com>
>>>>>> Sent: Wednesday, November 27, 2024 3:07 PM
>>>>>>
>>>>>>
>>>>>> +\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO (46)] Driver can
>> receive
>>>>>> GSO
>>>>>> +packets
>>>>>> +  carried by a UDP tunnel.
>>>>>> +
>>>>>> +\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO (48)] Device can
>> receive
>>>>>> GSO
>>>>>> +packets
>>>>>> +  carried by a UDP tunnel.
>>>>>
>>>>> Feature bits 42 to 49 are not applicable to device specific type.
>>>>> They are
>>>> reserved.
>>>>> Please see the note.
>>>>>
>>>>> Current spec description is:
>>>>>
>>>>> 0 to 23, and 50 to 127 Feature bits for the specific device type
>>>>> 24 to 41 Feature bits reserved for extensions to the queue and
>>>>> feature negotiation mechanisms, see 6
>>>>> 42 to 49, and 128 and above Feature bits reserved for future extensions.
>>>>>
>>>>> So 2 bits of these patch and of patch 4, 46 to 49 to be shifted to
>>>>> bits 65 to
>>>> 68.
>>>>>
>>>>> Or an alternative would be to amend,
>>>>>
>>>>> Bits 46 to 127 for specific device type, And 42 to 45 and 128
>>>>> reserved for future extensions.
>>>>>
>>>>> Michael,
>>>>> Please let me know so that I can have the follow up patch to this
>>>>> one as
>>>> voting is started.
>>>>> And the fix is required for it.
>>>>>
>>>>> Is there any historic reason for 42-49 as reserved, or it can be
>>>>> used as
>>>> proposed in this patch?
>>>>> If it was only kept as elastic buffer for device generic, and
>>>>> device type to
>>>> grow it is probably ok.
>>>>> Using 46 to 49 has small advantage to reuse in the offloads bit as below.
>>>>> Please confirm.
>>>>
>>>> See the reasoning given in 88895f56e642 ("Reserve more feature bits
>>>> for device type usage") -- the idea was to push out the point in time
>>>> when generic feature bits will have to be in the bit area beyond 63.
>>>> Given that we only used one of those generic bits in the meantime, I
>>>> assume it will still be some time before we run out of bits, even if
>>>> we let the net driver grab some bits there. Still, I'd probably prefer that the
>> net device used different bits.
>>>> [And issues like this are easy to miss during review unfortunately.]
>>>>
>>> I made assumption that one would have picked the device specific rage.
>>> I missed to detect this early enough, my bad. :(
>>>
>>> Given that we are only left with 4 bits in range 42 to 45, at generic level, it
>> seems wise for device-type to consume feature bits in its defined range.
>>> For virtio-net, bits 65 and higher are preferred.
>>>
>>>> However, if the device was to use different bits, the clean solution
>>>> would be to withdraw, respin, and revote. If we only changed the
>>>> reserved ranges, we'd be fine with a change on top, so that would be
>>>> less hassle in the short team. If we decide that we can live with the
>>>> smaller feature bit space below 63, I'd be fine with that solution.
>>>>
>>>
>>> I am fine either way to merge this and fix immediately for using bits 65 and
>> higher.
>>> My preference is to use bits 65 and higher.
>>>
>>> Or as you suggested to respin and revote right away to forward with bit 65.
>>>
>>> Please let me know.
>>
>> Personally, I'd prefer to handle this in an "atomic" manner, i.e. making sure
>> that no version ever has the wrong bit numbers... maybe if you did a fixup
>> patch that changes the numbers, and commit these patches + the fixup all at
>> once? Less churn than doing the respin and revote dance.
>>
> Here it is.
> https://lore.kernel.org/virtio-comment/20250120141052.877355-1-parav@nvidia.com/T/#u
> 
> Do we need to open a new voting request for it? yes?

I'm sorry for the latency.

Please be aware that VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO and
VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM values are reported a 2nd time in
the "Offloads State Configuration" paragraph. I guess even such chunk
will need patching.

/P

RE: [PATCH v11 3/4] virtio-net: define UDP tunnel segmentation offload feature

[Parav Pandit]

> From: Paolo Abeni <pabeni@redhat.com>
> Sent: Wednesday, January 22, 2025 12:10 AM
> 
> On 1/20/25 3:19 PM, Parav Pandit wrote:
> >
> >> From: Cornelia Huck <cohuck@redhat.com>
> >> Sent: Monday, January 20, 2025 5:50 PM
> >>
> >> On Mon, Jan 20 2025, Parav Pandit <parav@nvidia.com> wrote:
> >>
> >>>> From: Cornelia Huck <cohuck@redhat.com>
> >>>> Sent: Monday, January 20, 2025 2:34 PM
> >>>>
> >>>> On Sun, Jan 19 2025, Parav Pandit <parav@nvidia.com> wrote:
> >>>>
> >>>>> Hi Paolo, Michael,
> >>>>>
> >>>>>> From: Paolo Abeni <pabeni@redhat.com>
> >>>>>> Sent: Wednesday, November 27, 2024 3:07 PM
> >>>>>>
> >>>>>>
> >>>>>> +\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO (46)] Driver can
> >> receive
> >>>>>> GSO
> >>>>>> +packets
> >>>>>> +  carried by a UDP tunnel.
> >>>>>> +
> >>>>>> +\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO (48)] Device can
> >> receive
> >>>>>> GSO
> >>>>>> +packets
> >>>>>> +  carried by a UDP tunnel.
> >>>>>
> >>>>> Feature bits 42 to 49 are not applicable to device specific type.
> >>>>> They are
> >>>> reserved.
> >>>>> Please see the note.
> >>>>>
> >>>>> Current spec description is:
> >>>>>
> >>>>> 0 to 23, and 50 to 127 Feature bits for the specific device type
> >>>>> 24 to 41 Feature bits reserved for extensions to the queue and
> >>>>> feature negotiation mechanisms, see 6
> >>>>> 42 to 49, and 128 and above Feature bits reserved for future
> extensions.
> >>>>>
> >>>>> So 2 bits of these patch and of patch 4, 46 to 49 to be shifted to
> >>>>> bits 65 to
> >>>> 68.
> >>>>>
> >>>>> Or an alternative would be to amend,
> >>>>>
> >>>>> Bits 46 to 127 for specific device type, And 42 to 45 and 128
> >>>>> reserved for future extensions.
> >>>>>
> >>>>> Michael,
> >>>>> Please let me know so that I can have the follow up patch to this
> >>>>> one as
> >>>> voting is started.
> >>>>> And the fix is required for it.
> >>>>>
> >>>>> Is there any historic reason for 42-49 as reserved, or it can be
> >>>>> used as
> >>>> proposed in this patch?
> >>>>> If it was only kept as elastic buffer for device generic, and
> >>>>> device type to
> >>>> grow it is probably ok.
> >>>>> Using 46 to 49 has small advantage to reuse in the offloads bit as
> below.
> >>>>> Please confirm.
> >>>>
> >>>> See the reasoning given in 88895f56e642 ("Reserve more feature bits
> >>>> for device type usage") -- the idea was to push out the point in
> >>>> time when generic feature bits will have to be in the bit area beyond
> 63.
> >>>> Given that we only used one of those generic bits in the meantime,
> >>>> I assume it will still be some time before we run out of bits, even
> >>>> if we let the net driver grab some bits there. Still, I'd probably
> >>>> prefer that the
> >> net device used different bits.
> >>>> [And issues like this are easy to miss during review
> >>>> unfortunately.]
> >>>>
> >>> I made assumption that one would have picked the device specific rage.
> >>> I missed to detect this early enough, my bad. :(
> >>>
> >>> Given that we are only left with 4 bits in range 42 to 45, at
> >>> generic level, it
> >> seems wise for device-type to consume feature bits in its defined range.
> >>> For virtio-net, bits 65 and higher are preferred.
> >>>
> >>>> However, if the device was to use different bits, the clean
> >>>> solution would be to withdraw, respin, and revote. If we only
> >>>> changed the reserved ranges, we'd be fine with a change on top, so
> >>>> that would be less hassle in the short team. If we decide that we
> >>>> can live with the smaller feature bit space below 63, I'd be fine with
> that solution.
> >>>>
> >>>
> >>> I am fine either way to merge this and fix immediately for using
> >>> bits 65 and
> >> higher.
> >>> My preference is to use bits 65 and higher.
> >>>
> >>> Or as you suggested to respin and revote right away to forward with bit
> 65.
> >>>
> >>> Please let me know.
> >>
> >> Personally, I'd prefer to handle this in an "atomic" manner, i.e.
> >> making sure that no version ever has the wrong bit numbers... maybe
> >> if you did a fixup patch that changes the numbers, and commit these
> >> patches + the fixup all at once? Less churn than doing the respin and
> revote dance.
> >>
> > Here it is.
> > https://lore.kernel.org/virtio-comment/20250120141052.877355-1-
> parav@n
> > vidia.com/T/#u
> >
> > Do we need to open a new voting request for it? yes?
> 
> I'm sorry for the latency.
> 
> Please be aware that VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO and
> VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM values are reported a 2nd
> time in the "Offloads State Configuration" paragraph. I guess even such chunk
> will need patching.
> 
> /P
Ok. will fix and open github issue so that we can merge both the primary and fixes patches in one go after both the votings are done.

Re: [PATCH v11 3/4] virtio-net: define UDP tunnel segmentation offload feature

[Paolo Abeni]

On 1/21/25 8:38 PM, Parav Pandit wrote:
>> From: Paolo Abeni <pabeni@redhat.com>
>> Sent: Wednesday, January 22, 2025 12:10 AM
>>
>> On 1/20/25 3:19 PM, Parav Pandit wrote:
>>>
>>>> From: Cornelia Huck <cohuck@redhat.com>
>>>> Sent: Monday, January 20, 2025 5:50 PM
>>>>
>>>> On Mon, Jan 20 2025, Parav Pandit <parav@nvidia.com> wrote:
>>>>
>>>>>> From: Cornelia Huck <cohuck@redhat.com>
>>>>>> Sent: Monday, January 20, 2025 2:34 PM
>>>>>>
>>>>>> On Sun, Jan 19 2025, Parav Pandit <parav@nvidia.com> wrote:
>>>>>>
>>>>>>> Hi Paolo, Michael,
>>>>>>>
>>>>>>>> From: Paolo Abeni <pabeni@redhat.com>
>>>>>>>> Sent: Wednesday, November 27, 2024 3:07 PM
>>>>>>>>
>>>>>>>>
>>>>>>>> +\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO (46)] Driver can
>>>> receive
>>>>>>>> GSO
>>>>>>>> +packets
>>>>>>>> +  carried by a UDP tunnel.
>>>>>>>> +
>>>>>>>> +\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO (48)] Device can
>>>> receive
>>>>>>>> GSO
>>>>>>>> +packets
>>>>>>>> +  carried by a UDP tunnel.
>>>>>>>
>>>>>>> Feature bits 42 to 49 are not applicable to device specific type.
>>>>>>> They are
>>>>>> reserved.
>>>>>>> Please see the note.
>>>>>>>
>>>>>>> Current spec description is:
>>>>>>>
>>>>>>> 0 to 23, and 50 to 127 Feature bits for the specific device type
>>>>>>> 24 to 41 Feature bits reserved for extensions to the queue and
>>>>>>> feature negotiation mechanisms, see 6
>>>>>>> 42 to 49, and 128 and above Feature bits reserved for future
>> extensions.
>>>>>>>
>>>>>>> So 2 bits of these patch and of patch 4, 46 to 49 to be shifted to
>>>>>>> bits 65 to
>>>>>> 68.
>>>>>>>
>>>>>>> Or an alternative would be to amend,
>>>>>>>
>>>>>>> Bits 46 to 127 for specific device type, And 42 to 45 and 128
>>>>>>> reserved for future extensions.
>>>>>>>
>>>>>>> Michael,
>>>>>>> Please let me know so that I can have the follow up patch to this
>>>>>>> one as
>>>>>> voting is started.
>>>>>>> And the fix is required for it.
>>>>>>>
>>>>>>> Is there any historic reason for 42-49 as reserved, or it can be
>>>>>>> used as
>>>>>> proposed in this patch?
>>>>>>> If it was only kept as elastic buffer for device generic, and
>>>>>>> device type to
>>>>>> grow it is probably ok.
>>>>>>> Using 46 to 49 has small advantage to reuse in the offloads bit as
>> below.
>>>>>>> Please confirm.
>>>>>>
>>>>>> See the reasoning given in 88895f56e642 ("Reserve more feature bits
>>>>>> for device type usage") -- the idea was to push out the point in
>>>>>> time when generic feature bits will have to be in the bit area beyond
>> 63.
>>>>>> Given that we only used one of those generic bits in the meantime,
>>>>>> I assume it will still be some time before we run out of bits, even
>>>>>> if we let the net driver grab some bits there. Still, I'd probably
>>>>>> prefer that the
>>>> net device used different bits.
>>>>>> [And issues like this are easy to miss during review
>>>>>> unfortunately.]
>>>>>>
>>>>> I made assumption that one would have picked the device specific rage.
>>>>> I missed to detect this early enough, my bad. :(
>>>>>
>>>>> Given that we are only left with 4 bits in range 42 to 45, at
>>>>> generic level, it
>>>> seems wise for device-type to consume feature bits in its defined range.
>>>>> For virtio-net, bits 65 and higher are preferred.
>>>>>
>>>>>> However, if the device was to use different bits, the clean
>>>>>> solution would be to withdraw, respin, and revote. If we only
>>>>>> changed the reserved ranges, we'd be fine with a change on top, so
>>>>>> that would be less hassle in the short team. If we decide that we
>>>>>> can live with the smaller feature bit space below 63, I'd be fine with
>> that solution.
>>>>>>
>>>>>
>>>>> I am fine either way to merge this and fix immediately for using
>>>>> bits 65 and
>>>> higher.
>>>>> My preference is to use bits 65 and higher.
>>>>>
>>>>> Or as you suggested to respin and revote right away to forward with bit
>> 65.
>>>>>
>>>>> Please let me know.
>>>>
>>>> Personally, I'd prefer to handle this in an "atomic" manner, i.e.
>>>> making sure that no version ever has the wrong bit numbers... maybe
>>>> if you did a fixup patch that changes the numbers, and commit these
>>>> patches + the fixup all at once? Less churn than doing the respin and
>> revote dance.
>>>>
>>> Here it is.
>>> https://lore.kernel.org/virtio-comment/20250120141052.877355-1-
>> parav@n
>>> vidia.com/T/#u
>>>
>>> Do we need to open a new voting request for it? yes?
>>
>> I'm sorry for the latency.
>>
>> Please be aware that VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO and
>> VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM values are reported a 2nd
>> time in the "Offloads State Configuration" paragraph. I guess even such chunk
>> will need patching.
>>
>> /P
> Ok. will fix and open github issue so that we can merge both the primary and fixes patches in one go after both the votings are done.

Thanks!

Please consider my reply as a formal ack to such change just in case
it's needed by the process.

/P

RE: [PATCH v11 3/4] virtio-net: define UDP tunnel segmentation offload feature

[Parav Pandit]

> From: Paolo Abeni <pabeni@redhat.com>
> Sent: Wednesday, January 22, 2025 12:10 AM
> 
> On 1/20/25 3:19 PM, Parav Pandit wrote:
> >
> >> From: Cornelia Huck <cohuck@redhat.com>
> >> Sent: Monday, January 20, 2025 5:50 PM
> >>
> >> On Mon, Jan 20 2025, Parav Pandit <parav@nvidia.com> wrote:
> >>
> >>>> From: Cornelia Huck <cohuck@redhat.com>
> >>>> Sent: Monday, January 20, 2025 2:34 PM
> >>>>
> >>>> On Sun, Jan 19 2025, Parav Pandit <parav@nvidia.com> wrote:
> >>>>
> >>>>> Hi Paolo, Michael,
> >>>>>
> >>>>>> From: Paolo Abeni <pabeni@redhat.com>
> >>>>>> Sent: Wednesday, November 27, 2024 3:07 PM
> >>>>>>
> >>>>>>
> >>>>>> +\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO (46)] Driver can
> >> receive
> >>>>>> GSO
> >>>>>> +packets
> >>>>>> +  carried by a UDP tunnel.
> >>>>>> +
> >>>>>> +\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO (48)] Device can
> >> receive
> >>>>>> GSO
> >>>>>> +packets
> >>>>>> +  carried by a UDP tunnel.
> >>>>>
> >>>>> Feature bits 42 to 49 are not applicable to device specific type.
> >>>>> They are
> >>>> reserved.
> >>>>> Please see the note.
> >>>>>
> >>>>> Current spec description is:
> >>>>>
> >>>>> 0 to 23, and 50 to 127 Feature bits for the specific device type
> >>>>> 24 to 41 Feature bits reserved for extensions to the queue and
> >>>>> feature negotiation mechanisms, see 6
> >>>>> 42 to 49, and 128 and above Feature bits reserved for future
> extensions.
> >>>>>
> >>>>> So 2 bits of these patch and of patch 4, 46 to 49 to be shifted to
> >>>>> bits 65 to
> >>>> 68.
> >>>>>
> >>>>> Or an alternative would be to amend,
> >>>>>
> >>>>> Bits 46 to 127 for specific device type, And 42 to 45 and 128
> >>>>> reserved for future extensions.
> >>>>>
> >>>>> Michael,
> >>>>> Please let me know so that I can have the follow up patch to this
> >>>>> one as
> >>>> voting is started.
> >>>>> And the fix is required for it.
> >>>>>
> >>>>> Is there any historic reason for 42-49 as reserved, or it can be
> >>>>> used as
> >>>> proposed in this patch?
> >>>>> If it was only kept as elastic buffer for device generic, and
> >>>>> device type to
> >>>> grow it is probably ok.
> >>>>> Using 46 to 49 has small advantage to reuse in the offloads bit as
> below.
> >>>>> Please confirm.
> >>>>
> >>>> See the reasoning given in 88895f56e642 ("Reserve more feature bits
> >>>> for device type usage") -- the idea was to push out the point in
> >>>> time when generic feature bits will have to be in the bit area beyond
> 63.
> >>>> Given that we only used one of those generic bits in the meantime,
> >>>> I assume it will still be some time before we run out of bits, even
> >>>> if we let the net driver grab some bits there. Still, I'd probably
> >>>> prefer that the
> >> net device used different bits.
> >>>> [And issues like this are easy to miss during review
> >>>> unfortunately.]
> >>>>
> >>> I made assumption that one would have picked the device specific rage.
> >>> I missed to detect this early enough, my bad. :(
> >>>
> >>> Given that we are only left with 4 bits in range 42 to 45, at
> >>> generic level, it
> >> seems wise for device-type to consume feature bits in its defined range.
> >>> For virtio-net, bits 65 and higher are preferred.
> >>>
> >>>> However, if the device was to use different bits, the clean
> >>>> solution would be to withdraw, respin, and revote. If we only
> >>>> changed the reserved ranges, we'd be fine with a change on top, so
> >>>> that would be less hassle in the short team. If we decide that we
> >>>> can live with the smaller feature bit space below 63, I'd be fine with
> that solution.
> >>>>
> >>>
> >>> I am fine either way to merge this and fix immediately for using
> >>> bits 65 and
> >> higher.
> >>> My preference is to use bits 65 and higher.
> >>>
> >>> Or as you suggested to respin and revote right away to forward with bit
> 65.
> >>>
> >>> Please let me know.
> >>
> >> Personally, I'd prefer to handle this in an "atomic" manner, i.e.
> >> making sure that no version ever has the wrong bit numbers... maybe
> >> if you did a fixup patch that changes the numbers, and commit these
> >> patches + the fixup all at once? Less churn than doing the respin and
> revote dance.
> >>
> > Here it is.
> > https://lore.kernel.org/virtio-comment/20250120141052.877355-1-
> parav@n
> > vidia.com/T/#u
> >
> > Do we need to open a new voting request for it? yes?
> 
> I'm sorry for the latency.
> 
> Please be aware that VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO and
> VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM values are reported a 2nd
> time in the "Offloads State Configuration" paragraph. I guess even such chunk
> will need patching.
> 
Offloads state is only 64-bit in size.
So I kept it as is to reuse the reserved feature bits range.
This way there wont be any conflict.

Posted the v1 https://lore.kernel.org/virtio-comment/20250126062058.13695-1-parav@nvidia.com/T/#u

[PATCH v11 4/4] virtio-net: define UDP tunnel checksum offload feature

[Paolo Abeni]

This complements the previous change, additionally
introducing the UDP tunnel checksum offload feature.

Differently from the plain checksum offload feature, this
depends on UDP tunnel segmentation being available, as outer checksum
computation for non GSO packets is cheap and H/W implementation of
such a feature is complex.

UDP tunnel checksum offload does not introduce additional fields,
instead it leverages the outer transport offset introduced by the
UDP tunnel segmentation feature to locate the outer checksum
inside the packet.

When UDP tunnel checksum offload is negotiated:
- the driver requests the outer UDP checksum offload setting the
VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit in the flag field. Such bit
is not allocated inside the gso_type field to prevent space
exhaustion there.
- in the device -> driver direction, the VIRTIO_NET_HDR_F_DATA_VALID
bit semantic is extended, covering both the inner and the outer
checksum validation.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v10 -> v11:
 - 'to the outer UDP pseudo header' -> 'to the outer UDP pseudo header
   checksum'
 - rebased on top of patch 2/4
 - dropped trailing spaces

v9 -> v10:
 - DATA_VALID is not allowed for GSO over UDP tunnel packets
 - fixed more typos:
   VIRTIO_NET_HDR_GSO_UDP_TUNNEL -> either
VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 or VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6

v8 -> v9:
 - rebased on top of virtio-1.4, changed udp-tunnel features number
   to avoid conflicts/duplications
 - fixed typos:
   VIRTIO_NET_HDR_F_UDP_TUNNEL -> VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM
   VIRTIO_NET_HDR_GSO_UDP_TUNNEL -> either
VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 or VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
 - specified strict validation for the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM
   bit, only allowed with UDP tunnel GSO packets.
 - specified that VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM describe the outer
   header csum offload for both in presence and absence of
   VIRTIO_NET_HDR_F_DATA_VALID

v7 -> v8:
 - dropped confusing wording around csum validation
 - clarified VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM allocation

v6 -> v7:
 - rebased
 - VIRTIO_NET_F_{HOST,GUEST}_UDP_TUNNEL_CSUM -> ...UDP_TUNNEL_GSO_CSUM
 - dropped unintended change to existing TCP gso spec
---
 device-types/net/description.tex | 143 ++++++++++++++++++++++++++++---
 1 file changed, 131 insertions(+), 12 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 67bbccf..efb543f 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -91,9 +91,15 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
 \item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO (46)] Driver can receive GSO packets
   carried by a UDP tunnel.
 
+\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM (47)] Driver handles packets
+  carried by a UDP tunnel with partial csum for the outer header.
+
 \item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO (48)] Device can receive GSO packets
   carried by a UDP tunnel.
 
+\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM (49)] Device handles packets
+  carried by a UDP tunnel with partial csum for the outer header.
+
 \item[VIRTIO_NET_F_DEVICE_STATS(50)] Device can provide device-level statistics
     to the driver through the control virtqueue.
 
@@ -146,6 +152,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
 \item[VIRTIO_NET_F_GUEST_USO6] Requires VIRTIO_NET_F_GUEST_CSUM.
 \item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO] Requires VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
    VIRTIO_NET_F_GUEST_USO4 and VIRTIO_NET_F_GUEST_USO6.
+\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM] Requires VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO
 
 \item[VIRTIO_NET_F_HOST_TSO4] Requires VIRTIO_NET_F_CSUM.
 \item[VIRTIO_NET_F_HOST_TSO6] Requires VIRTIO_NET_F_CSUM.
@@ -154,6 +161,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
 \item[VIRTIO_NET_F_HOST_USO] Requires VIRTIO_NET_F_CSUM.
 \item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO] Requires VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_TSO6
    and VIRTIO_NET_F_HOST_USO.
+\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM] Requires VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO
 
 \item[VIRTIO_NET_F_CTRL_RX] Requires VIRTIO_NET_F_CTRL_VQ.
 \item[VIRTIO_NET_F_CTRL_VLAN] Requires VIRTIO_NET_F_CTRL_VQ.
@@ -168,6 +176,13 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
 \item[VIRTIO_NET_F_RSS_CONTEXT] Requires VIRTIO_NET_F_CTRL_VQ and VIRTIO_NET_F_RSS.
 \end{description}
 
+\begin{note}
+The dependency between UDP_TUNNEL_GSO_CSUM and UDP_TUNNEL_GSO is intentionally
+in the opposite direction with respect to the plain GSO features and the plain
+checksum offload because UDP tunnel checksum offload gives very little gain
+for non GSO packets and is quite complex to implement in H/W.
+\end{note}
+
 \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
 \begin{description}
 \item[VIRTIO_NET_F_GSO (6)] Device handles packets with any GSO type. This was supposed to indicate segmentation offload support, but
@@ -402,6 +417,11 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
   could be either TCP or UDP. Only a single level of encapsulation
   offload is supported.
 
+\item If VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO is negotiated, a driver can
+  additionally use TCP segmentation or UDP segmentation on top of UDP
+  encapsulation with the outer header requiring checksum offload,
+  negotiating the VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM feature.
+
 \item The converse features are also available: a driver can save
   the virtual device some work by negotiating these features.\note{For example, a network packet transported between two guests on
 the same system might not need checksumming at all, nor segmentation,
@@ -410,8 +430,9 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
   checksummed packets can be received, and if it can do that then
   the VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
   VIRTIO_NET_F_GUEST_UFO, VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_USO4,
-  VIRTIO_NET_F_GUEST_USO6 and VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO
-  are the input equivalents of the features described above.
+  VIRTIO_NET_F_GUEST_USO6 VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO and
+  VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM are the input equivalents of
+  the features described above.
   See \ref{sec:Device Types / Network Device / Device Operation /
 Setting Up Receive Buffers}~\nameref{sec:Device Types / Network
 Device / Device Operation / Setting Up Receive Buffers} and
@@ -557,6 +578,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
 #define VIRTIO_NET_HDR_F_NEEDS_CSUM    1
 #define VIRTIO_NET_HDR_F_DATA_VALID    2
 #define VIRTIO_NET_HDR_F_RSC_INFO      4
+#define VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM 8
         u8 flags;
 #define VIRTIO_NET_HDR_GSO_NONE        0
 #define VIRTIO_NET_HDR_GSO_TCPV4       1
@@ -681,8 +703,11 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
   \field{gso_type} has the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
   VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit set, the GSO protocol is encapsulated
   in a UDP tunnel.
-  The outer UDP header must not require checksum validation, i.e. the outer UDP
-  checksum must be a positive zero (0x0) as defined in UDP RFC 768.
+  If the outer UDP header requires checksumming, the driver must have
+  additionally negotiated the VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM feature
+  and offloaded the outer checksum accordingly, otherwise
+  the outer UDP header must not require checksum validation, i.e. the outer
+  UDP checksum must be positive zero (0x0) as defined in UDP RFC 768.
   The other tunnel-related fields indicate how to replicate the packet
   headers to cut it into smaller packets:
 
@@ -710,6 +735,20 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 VIRTIO_NET_HDR_GSO_TCPV4 | VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 = 0x21
 \end{note}
 
+\item If the driver negotiated the VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM feature,
+  the transmitted packet is a GSO one encapsulated in a UDP tunnel, and
+  the outer UDP header requires checksumming, the driver can skip checksumming
+  the outer header:
+
+  \begin{itemize}
+  \item \field{flags} has the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM set,
+
+  \item The outer UDP checksum field in the packet is set to the sum
+    of the UDP pseudo header, so that replacing it by the ones'
+    complement checksum of the outer UDP header and payload will give the
+    correct result.
+  \end{itemize}
+
 \item \field{num_buffers} is set to zero.  This field is unused on transmitted packets.
 
 \item The header and packet are added as one output descriptor to the
@@ -768,6 +807,16 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 MUST SET the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit.
 
 The driver MUST NOT send to the device GSO packets over UDP tunnel
+requiring segmentation and outer UDP checksum offload, unless both the
+VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO and VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM features
+are negotiated, in which case the driver MUST set either the
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
+bit in the \field{gso_type} and the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit in
+the \field{flags}.
+
+If VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM is not negotiated, the driver MUST not set
+the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit in the \field{flags} and
+MUST NOT send to the device GSO packets over UDP tunnel
 requiring segmentation and outer UDP checksum offload.
 
 The driver MUST NOT set the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
@@ -777,6 +826,10 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 The driver MUST NOT set the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit and the
 VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit together.
 
+The driver MUST NOT set the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit \field{flags}
+without setting either the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
+the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit in \field{gso_type}.
+
 If the VIRTIO_NET_F_CSUM feature has been negotiated, the
 driver MAY set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in
 \field{flags}, if so:
@@ -838,8 +891,23 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
   to the inner transport header and inner transport checksum field.
 \end{itemize}
 
+If the VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM feature has been negotiated,
+and the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit in \field{gso_type} are set,
+the driver MAY set the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit in
+\field{flags}, if so the driver MUST set the packet outer UDP header checksum
+to the outer UDP pseudo header checksum.
+
+\begin{note}
+calculating a ones' complement checksum from \field{outer_th_offset}
+up until the end of the packet and storing the result at offset 6
+from \field{outer_th_offset} will result in a fully checksummed outer UDP packet;
+\end{note}
+
 If the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
-VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit in \field{gso_type} are set, the
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit in \field{gso_type} are set
+and the VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM feature has not
+been negotiated, the
 outer UDP header MUST NOT require checksum validation. That is, the
 outer UDP checksum value MUST be 0 or the validated complete checksum
 for such header.
@@ -860,6 +928,10 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 been negotiated, the driver MUST NOT set either the VIRTIO_NET_HDR_F_GSO_UDP_TUNNEL_IPV4
 bit or the VIRTIO_NET_HDR_F_GSO_UDP_TUNNEL_IPV6 in \field{gso_type}.
 
+If the VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM option has not been negotiated,
+the driver MUST NOT set the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit
+in \field{flags}.
+
 The driver SHOULD accept the VIRTIO_NET_F_GUEST_HDRLEN feature if it has
 been offered, and if it's able to provide the exact header length.
 
@@ -918,6 +990,11 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 \end{itemize}
 the device MUST NOT accept the packet.
 
+If the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit in \field{flags} is set,
+and both the bits VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 and
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 in \field{gso_type} are not set,
+the device MOST NOT accept the packet.
+
 If VIRTIO_NET_HDR_F_NEEDS_CSUM is not set, the device MUST NOT
 rely on the packet checksum being correct.
 \paragraph{Packet Transmission Interrupt}\label{sec:Device Types / Network Device / Device Operation / Packet Transmission / Packet Transmission Interrupt}
@@ -1020,8 +1097,11 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 \item If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated, the
   VIRTIO_NET_HDR_F_DATA_VALID bit in \field{flags} can be
   set: if so, device has validated the packet checksum.
-  In case of multiple encapsulated protocols, one level of checksums
-  has been validated.
+  If the VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM feature has been negotiated,
+  and the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit is set in \field{flags},
+  both the outer UDP checksum and the inner transport checksum
+  have been validated, otherwise only one level of checksums (the outer one
+  in case of tunnels) has been validated.
 \end{enumerate}
 
 Additionally, VIRTIO_NET_F_GUEST_CSUM, TSO4, TSO6, UDP, UDP_TUNNEL
@@ -1062,12 +1142,22 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
   bit MAY be set. In such case the \field{outer_th_offset} and
   \field{inner_nh_offset} fields indicate the corresponding
   headers information.
+\item If the VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM feature was
+negotiated, and
+  the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
+  are set in \field{gso_type}, the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit in the
+  \field{flags} can be set: if so, the outer UDP checksum has been validated
+  and the UDP header checksum at offset 6 from from \field{outer_th_offset}
+  is set to the outer UDP pseudo header checksum.
+
 \begin{note}
 If the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
 bit are set in \field{gso_type}, the \field{csum_start} field refers to
-the inner transport header offset (see Packet Transmission point 1)
-and only the inner transport header checksum has been validated by
-VIRTIO_NET_HDR_F_NEEDS_CSUM.
+the inner transport header offset (see Packet Transmission point 1).
+If the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit in \field{flags} is set both
+the inner and the outer header checksums have been validated by
+VIRTIO_NET_HDR_F_NEEDS_CSUM, otherwise only the inner transport header
+checksum has been validated.
 \end{note}
 \end{enumerate}
 
@@ -1116,6 +1206,9 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 either the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
 VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit in \field{gso_type}.
 
+If VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM is not negotiated the device MUST NOT set
+the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit in \field{flags}.
+
 The device SHOULD NOT send to the driver TCP packets requiring segmentation offload
 which have the Explicit Congestion Notification bit set, unless the
 VIRTIO_NET_F_GUEST_ECN feature is negotiated, in which case the
@@ -1148,7 +1241,8 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 to the corresponding header information, and the outer UDP header MUST NOT
 require checksum offload.
 
-The device MUST NOT send the driver GSO packets encapsulated in UDP
+If the VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM feature has not been negotiated,
+the device MUST NOT send the driver GSO packets encapsulated in UDP
 tunnel and requiring segmentation and outer checksum offload.
 
 If none of the VIRTIO_NET_F_GUEST_TSO4, TSO6, UFO, USO4 or USO6 options have
@@ -1176,7 +1270,11 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 If the VIRTIO_NET_F_GUEST_CSUM feature has been negotiated, the
 device MAY set the VIRTIO_NET_HDR_F_DATA_VALID bit in
 \field{flags}, if so, the device MUST validate the packet
-checksum. One level of checksum is validated: in case of multiple
+checksum. If the VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM feature has
+been negotiated, and the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit set in
+\field{flags}, both the outer UDP checksum and the inner transport
+checksum have been validated.
+Otherwise level of checksum is validated: in case of multiple
 encapsulated protocols the outermost one.
 
 If either the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
@@ -1184,6 +1282,20 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 the device MUST NOT set the VIRTIO_NET_HDR_F_DATA_VALID bit in
 \field{flags}.
 
+If the VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM feature has been negotiated
+and either the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit is set or the
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit is set in \field{gso_type}, the
+device MAY set the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit in
+\field{flags}, if so the device MUST set the packet outer UDP checksum
+stored in the receive buffer to the outer UDP pseudo header.
+
+Otherwise, the VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM feature has been
+negotiated, either the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit is set or the
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit is set in \field{gso_type},
+and the bit VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM is not set in
+\field{flags}, the device MUST either provide a zero outer UDP header
+checksum or a fully checksummed outer UDP header.
+
 \drivernormative{\paragraph}{Processing of Incoming
 Packets}{Device Types / Network Device / Device Operation /
 Processing of Incoming Packets}
@@ -1225,6 +1337,12 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 \end{itemize}
 the driver MUST NOT accept the packet.
 
+If the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit and the VIRTIO_NET_HDR_F_NEEDS_CSUM
+bit in \field{flags} are set,
+and both the bits VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 and
+VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 in \field{gso_type} are not set,
+the driver MOST NOT accept the packet.
+
 \paragraph{Hash calculation for incoming packets}
 \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets}
 
@@ -2069,6 +2187,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 #define VIRTIO_NET_F_GUEST_ECN        9
 #define VIRTIO_NET_F_GUEST_UFO        10
 #define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO  46
+#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM 47
 #define VIRTIO_NET_F_GUEST_USO4       54
 #define VIRTIO_NET_F_GUEST_USO6       55
 
-- 
2.45.2

Re: [PATCH v11 4/4] virtio-net: define UDP tunnel checksum offload feature

[Jason Wang]

On Wed, Nov 27, 2024 at 5:37 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> This complements the previous change, additionally
> introducing the UDP tunnel checksum offload feature.
>
> Differently from the plain checksum offload feature, this
> depends on UDP tunnel segmentation being available, as outer checksum
> computation for non GSO packets is cheap and H/W implementation of
> such a feature is complex.
>
> UDP tunnel checksum offload does not introduce additional fields,
> instead it leverages the outer transport offset introduced by the
> UDP tunnel segmentation feature to locate the outer checksum
> inside the packet.
>
> When UDP tunnel checksum offload is negotiated:
> - the driver requests the outer UDP checksum offload setting the
> VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit in the flag field. Such bit
> is not allocated inside the gso_type field to prevent space
> exhaustion there.
> - in the device -> driver direction, the VIRTIO_NET_HDR_F_DATA_VALID
> bit semantic is extended, covering both the inner and the outer
> checksum validation.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v10 -> v11:
>  - 'to the outer UDP pseudo header' -> 'to the outer UDP pseudo header
>    checksum'
>  - rebased on top of patch 2/4
>  - dropped trailing spaces
>
> v9 -> v10:
>  - DATA_VALID is not allowed for GSO over UDP tunnel packets
>  - fixed more typos:
>    VIRTIO_NET_HDR_GSO_UDP_TUNNEL -> either
> VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 or VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
>
> v8 -> v9:
>  - rebased on top of virtio-1.4, changed udp-tunnel features number
>    to avoid conflicts/duplications
>  - fixed typos:
>    VIRTIO_NET_HDR_F_UDP_TUNNEL -> VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM
>    VIRTIO_NET_HDR_GSO_UDP_TUNNEL -> either
> VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 or VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
>  - specified strict validation for the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM
>    bit, only allowed with UDP tunnel GSO packets.
>  - specified that VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM describe the outer
>    header csum offload for both in presence and absence of
>    VIRTIO_NET_HDR_F_DATA_VALID
>
> v7 -> v8:
>  - dropped confusing wording around csum validation
>  - clarified VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM allocation
>
> v6 -> v7:
>  - rebased
>  - VIRTIO_NET_F_{HOST,GUEST}_UDP_TUNNEL_CSUM -> ...UDP_TUNNEL_GSO_CSUM
>  - dropped unintended change to existing TCP gso spec
> ---
>  device-types/net/description.tex | 143 ++++++++++++++++++++++++++++---
>  1 file changed, 131 insertions(+), 12 deletions(-)
>

Reviewed-by: Jason Wang <jasowang@redhat.com>

Thanks

Re: [PATCH v11 0/4] virtio-net: define UDP tunnel offload

[Paolo Abeni]

On 11/27/24 10:36, Paolo Abeni wrote:
> UDP tunnel usage is ubiquitous in container deployment, and the ability
> to offload UDP encapsulated GSO traffic impacts greatly the performances
> and the CPU utilization of such use cases.
> 
> This series includes:
> - clarifications on the current semantic of NEEDS_CSUM offload
> (patch 1) and DATA_VALID offload (patch 2) to make later changes easier
> to follow,
> - new features definition to handle both UDP tunnel segmentation
> offload (patch 3).
> - new features definition to handle UDP tunnel outer checksum offload
> (patch 4).
> 
> A PoC implementation is available here:
> 
> https://github.com/pabeni/linux-devel/commits/virtio_tunnel_gso_v10/
> https://github.com/pabeni/qemu/tree/virtio_tunnel_gso_v9
> 
> Note that both implementations has not been updated since the previous
> iteration, since there are no additional functional changes in v11.
> 
> Changes from v10:
> - added patch 2
> - clarified only a single encapsulation level is supported (patch 3)
> - s/TCP or UDP GSO packets/GSO packets/ (patch 3)
> - 'to the outer UDP pseudo header' -> 'to the outer UDP pseudo header
>   checksum' (patch 4)
> https://lore.kernel.org/virtio-comment/cover.1731084277.git.pabeni@redhat.com/

As a reference this is the complete diff from v10:
---
diff --git a/device-types/net/description.tex
b/device-types/net/description.tex
index 2d15ca8..efb543f 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -412,6 +412,10 @@ \subsection{Device Initialization}\label{sec:Device
Types / Network Device / Dev
   offload, when the outer header does not require checksumming - e.g.
   the outer UDP checksum is zero - by negotiating the
   VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO feature.
+  GSO over UDP tunnels packets carry two sets of headers: the outer ones
+  and the inner ones. The outer transport protocol is UDP, the inner
+  could be either TCP or UDP. Only a single level of encapsulation
+  offload is supported.

 \item If VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO is negotiated, a driver can
   additionally use TCP segmentation or UDP segmentation on top of UDP
@@ -792,17 +796,17 @@ \subsubsection{Packet
Transmission}\label{sec:Device Types / Network Device / De
 If VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO is negotiated, the driver MAY set
 VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit
 in \field{gso_type} according to the inner network header protocol type
-to request TCP or UDP GSO packets over UDPv4 or UDPv6 tunnel
-segmentation, otherwise the driver MUST NOT set either the
+to request GSO packets over UDPv4 or UDPv6 tunnel segmentation,
+otherwise the driver MUST NOT set either the
 VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit
 in \field{gso_type}.

-When requesting TCP or UDP GSO segmentation over UDP tunnel, the driver
MUST SET the
+When requesting GSO segmentation over UDP tunnel, the driver MUST SET the
 VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit if the inner network header is
IPv4, i.e. the
 packet is a TCPv4 GSO one, otherwise, if the inner network header is
IPv6, the driver
 MUST SET the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit.

-The driver MUST NOT send to the device TCP or UDP GSO packets over UDP
tunnel
+The driver MUST NOT send to the device GSO packets over UDP tunnel
 requiring segmentation and outer UDP checksum offload, unless both the
 VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO and
VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM features
 are negotiated, in which case the driver MUST set either the
@@ -812,7 +816,7 @@ \subsubsection{Packet Transmission}\label{sec:Device
Types / Network Device / De

 If VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM is not negotiated, the driver
MUST not set
 the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit in the \field{flags} and
-MUST NOT send to the device TCP or UDP GSO packets over UDP tunnel
+MUST NOT send to the device GSO packets over UDP tunnel
 requiring segmentation and outer UDP checksum offload.

 The driver MUST NOT set the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
@@ -892,7 +896,7 @@ \subsubsection{Packet Transmission}\label{sec:Device
Types / Network Device / De
 VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit in \field{gso_type} are set,
 the driver MAY set the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit in
 \field{flags}, if so the driver MUST set the packet outer UDP header
checksum
-to the outer UDP pseudo header.
+to the outer UDP pseudo header checksum.

 \begin{note}
 calculating a ones' complement checksum from \field{outer_th_offset}
@@ -1148,10 +1152,10 @@ \subsubsection{Processing of Incoming
Packets}\label{sec:Device Types / Network

 \begin{note}
 If the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
-bit are set in \field{gso_type}, the \field{csum_start} field refers to
+bit are set in \field{gso_type}, the \field{csum_start} field refers to
 the inner transport header offset (see Packet Transmission point 1).
 If the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit in \field{flags} is set both
-the inner and the outer header checksums have been validated by
+the inner and the outer header checksums have been validated by
 VIRTIO_NET_HDR_F_NEEDS_CSUM, otherwise only the inner transport header
 checksum has been validated.
 \end{note}
@@ -1228,7 +1232,7 @@ \subsubsection{Processing of Incoming
Packets}\label{sec:Device Types / Network
 	fully checksummed packet;
 \end{enumerate}

-The device MUST NOT send to the driver TCP or UDP GSO packets
encapsulated in UDP
+The device MUST NOT send to the driver GSO packets encapsulated in UDP
 tunnel and requiring segmentation offload, unless the
 VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO is negotiated, in which case the
device MUST set
 the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
@@ -1238,7 +1242,7 @@ \subsubsection{Processing of Incoming
Packets}\label{sec:Device Types / Network
 require checksum offload.

 If the VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM feature has not been
negotiated,
-the device MUST NOT send the driver TCP or UDP GSO packets encapsulated
in UDP
+the device MUST NOT send the driver GSO packets encapsulated in UDP
 tunnel and requiring segmentation and outer checksum offload.

 If none of the VIRTIO_NET_F_GUEST_TSO4, TSO6, UFO, USO4 or USO6 options
have
@@ -1269,8 +1273,9 @@ \subsubsection{Processing of Incoming
Packets}\label{sec:Device Types / Network
 checksum. If the VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM feature has
 been negotiated, and the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit set in
 \field{flags}, both the outer UDP checksum and the inner transport
-checksum have been validated, otherwise only one level of checksums
-(the outer one in case of tunnels) has been validated.
+checksum have been validated.
+Otherwise level of checksum is validated: in case of multiple
+encapsulated protocols the outermost one.

 If either the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
 VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit in \field{gso_type} are set,

Re: [PATCH v11 0/4] virtio-net: define UDP tunnel offload

[Paolo Abeni]

On 11/27/24 10:48 AM, Paolo Abeni wrote:
> On 11/27/24 10:36, Paolo Abeni wrote:
>> UDP tunnel usage is ubiquitous in container deployment, and the ability
>> to offload UDP encapsulated GSO traffic impacts greatly the performances
>> and the CPU utilization of such use cases.
>>
>> This series includes:
>> - clarifications on the current semantic of NEEDS_CSUM offload
>> (patch 1) and DATA_VALID offload (patch 2) to make later changes easier
>> to follow,
>> - new features definition to handle both UDP tunnel segmentation
>> offload (patch 3).
>> - new features definition to handle UDP tunnel outer checksum offload
>> (patch 4).
>>
>> A PoC implementation is available here:
>>
>> https://github.com/pabeni/linux-devel/commits/virtio_tunnel_gso_v10/
>> https://github.com/pabeni/qemu/tree/virtio_tunnel_gso_v9
>>
>> Note that both implementations has not been updated since the previous
>> iteration, since there are no additional functional changes in v11.
>>
>> Changes from v10:
>> - added patch 2
>> - clarified only a single encapsulation level is supported (patch 3)
>> - s/TCP or UDP GSO packets/GSO packets/ (patch 3)
>> - 'to the outer UDP pseudo header' -> 'to the outer UDP pseudo header
>>   checksum' (patch 4)
>> https://lore.kernel.org/virtio-comment/cover.1731084277.git.pabeni@redhat.com/

I understand all the points under discussion have been addressed. I'm
wondering the next steps are and what I could do to move things forward.

Thanks,

Paolo

Re: [PATCH v11 0/4] virtio-net: define UDP tunnel offload

[Michael S. Tsirkin]

On Tue, Jan 14, 2025 at 04:04:35PM +0100, Paolo Abeni wrote:
> On 11/27/24 10:48 AM, Paolo Abeni wrote:
> > On 11/27/24 10:36, Paolo Abeni wrote:
> >> UDP tunnel usage is ubiquitous in container deployment, and the ability
> >> to offload UDP encapsulated GSO traffic impacts greatly the performances
> >> and the CPU utilization of such use cases.
> >>
> >> This series includes:
> >> - clarifications on the current semantic of NEEDS_CSUM offload
> >> (patch 1) and DATA_VALID offload (patch 2) to make later changes easier
> >> to follow,
> >> - new features definition to handle both UDP tunnel segmentation
> >> offload (patch 3).
> >> - new features definition to handle UDP tunnel outer checksum offload
> >> (patch 4).
> >>
> >> A PoC implementation is available here:
> >>
> >> https://github.com/pabeni/linux-devel/commits/virtio_tunnel_gso_v10/
> >> https://github.com/pabeni/qemu/tree/virtio_tunnel_gso_v9
> >>
> >> Note that both implementations has not been updated since the previous
> >> iteration, since there are no additional functional changes in v11.
> >>
> >> Changes from v10:
> >> - added patch 2
> >> - clarified only a single encapsulation level is supported (patch 3)
> >> - s/TCP or UDP GSO packets/GSO packets/ (patch 3)
> >> - 'to the outer UDP pseudo header' -> 'to the outer UDP pseudo header
> >>   checksum' (patch 4)
> >> https://lore.kernel.org/virtio-comment/cover.1731084277.git.pabeni@redhat.com/
> 
> I understand all the points under discussion have been addressed. I'm
> wondering the next steps are and what I could do to move things forward.
> 
> Thanks,
> 
> Paolo


See
https://github.com/oasis-tcs/virtio-spec?tab=readme-ov-file#use-of-github-issues

for the process to get a change merged once there is concensus.

-- 
MST

Re: [PATCH v11 0/4] virtio-net: define UDP tunnel offload

[Paolo Abeni]

On 1/14/25 4:16 PM, Michael S. Tsirkin wrote:
> On Tue, Jan 14, 2025 at 04:04:35PM +0100, Paolo Abeni wrote:
>> On 11/27/24 10:48 AM, Paolo Abeni wrote:
>>> On 11/27/24 10:36, Paolo Abeni wrote:
>>>> UDP tunnel usage is ubiquitous in container deployment, and the ability
>>>> to offload UDP encapsulated GSO traffic impacts greatly the performances
>>>> and the CPU utilization of such use cases.
>>>>
>>>> This series includes:
>>>> - clarifications on the current semantic of NEEDS_CSUM offload
>>>> (patch 1) and DATA_VALID offload (patch 2) to make later changes easier
>>>> to follow,
>>>> - new features definition to handle both UDP tunnel segmentation
>>>> offload (patch 3).
>>>> - new features definition to handle UDP tunnel outer checksum offload
>>>> (patch 4).
>>>>
>>>> A PoC implementation is available here:
>>>>
>>>> https://github.com/pabeni/linux-devel/commits/virtio_tunnel_gso_v10/
>>>> https://github.com/pabeni/qemu/tree/virtio_tunnel_gso_v9
>>>>
>>>> Note that both implementations has not been updated since the previous
>>>> iteration, since there are no additional functional changes in v11.
>>>>
>>>> Changes from v10:
>>>> - added patch 2
>>>> - clarified only a single encapsulation level is supported (patch 3)
>>>> - s/TCP or UDP GSO packets/GSO packets/ (patch 3)
>>>> - 'to the outer UDP pseudo header' -> 'to the outer UDP pseudo header
>>>>   checksum' (patch 4)
>>>> https://lore.kernel.org/virtio-comment/cover.1731084277.git.pabeni@redhat.com/
>>
>> I understand all the points under discussion have been addressed. I'm
>> wondering the next steps are and what I could do to move things forward.
>>
>> Thanks,
>>
>> Paolo
>
> See
> https://github.com/oasis-tcs/virtio-spec?tab=readme-ov-file#use-of-github-issues
> 
> for the process to get a change merged once there is concensus.

Thank you for the prompt reply and useful guidance.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/212

Could please open a voting ballot for these changes?

Thanks,

Paolo