[PATCH v10 0/3] virtio-net: define UDP tunnel offload

[PATCH v10 0/3] 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:
- a clarification on the current semantic of
NEEDS_CSUM offload (patch 1) to make later changes easier to follow,
- new features definition to handle both UDP tunnel segmentation
offload (patch 2).
- new features definition to handleUDP tunnel outer checksum offload
(patch 2).

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 the user-space implementation has not been updated since
the previous iteration, since the kernel changes are transparent to it.

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 (3):
  virtio-net: clarify NEEDS_CSUM semantic for GSO packats.
  virtio-net: define UDP tunnel segmentation offload feature
  virtio-net: define UDP tunnel checksum offload feature

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

-- 
2.45.2

[PATCH v10 1/3] 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 b2a0d39..3099c6f 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -605,6 +605,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
@@ -805,7 +810,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 v10 1/3] virtio-net: clarify NEEDS_CSUM semantic for GSO packats.

[Jason Wang]

On Sat, Nov 9, 2024 at 12:53 AM 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>
> ---
>  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 b2a0d39..3099c6f 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -605,6 +605,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}

I wonder if this defeats the purpose of LCO where the outer checksum
could be deduced.

>  \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
> @@ -805,7 +810,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}

This looks fine.

Thanks

>
>  If applicable, the device calculates per-packet hash for incoming packets as
> --
> 2.45.2
>

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

[Paolo Abeni]

On 11/25/24 03:51, Jason Wang wrote:
> On Sat, Nov 9, 2024 at 12:53 AM 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>
>> ---
>>  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 b2a0d39..3099c6f 100644
>> --- a/device-types/net/description.tex
>> +++ b/device-types/net/description.tex
>> @@ -605,6 +605,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}
> 
> I wonder if this defeats the purpose of LCO where the outer checksum
> could be deduced.

I don't see how?!? The added statement is not a behavior change, it just
clarifies the existing text. I don't see any implications WRT LCO.

Could you please describe in details the eventual problem?

Thanks,

Paolo

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

[Jason Wang]

On Tue, Nov 26, 2024 at 12:01 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 11/25/24 03:51, Jason Wang wrote:
> > On Sat, Nov 9, 2024 at 12:53 AM 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>
> >> ---
> >>  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 b2a0d39..3099c6f 100644
> >> --- a/device-types/net/description.tex
> >> +++ b/device-types/net/description.tex
> >> @@ -605,6 +605,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}
> >
> > I wonder if this defeats the purpose of LCO where the outer checksum
> > could be deduced.
>
> I don't see how?!? The added statement is not a behavior change, it just
> clarifies the existing text. I don't see any implications WRT LCO.
>
> Could you please describe in details the eventual problem?

Not a native speaker, just want to check if "no additional preceding
checksums validated by VIRTIO_NET_HDR_F_NEEDS_CSUM" is in conflict
with the case when we apply LCO for the outer. Or do we treat outer
checksum as "preceding checksums" here.

Thanks

>
> Thanks,
>
> Paolo
>

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

[Paolo Abeni]

On 11/26/24 04:01, Jason Wang wrote:
> On Tue, Nov 26, 2024 at 12:01 AM Paolo Abeni <pabeni@redhat.com> wrote:
>> On 11/25/24 03:51, Jason Wang wrote:
>>> On Sat, Nov 9, 2024 at 12:53 AM 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>
>>>> ---
>>>>  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 b2a0d39..3099c6f 100644
>>>> --- a/device-types/net/description.tex
>>>> +++ b/device-types/net/description.tex
>>>> @@ -605,6 +605,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}
>>>
>>> I wonder if this defeats the purpose of LCO where the outer checksum
>>> could be deduced.
>>
>> I don't see how?!? The added statement is not a behavior change, it just
>> clarifies the existing text. I don't see any implications WRT LCO.
>>
>> Could you please describe in details the eventual problem?
> 
> Not a native speaker, just want to check if "no additional preceding
> checksums validated by VIRTIO_NET_HDR_F_NEEDS_CSUM" is in conflict
> with the case when we apply LCO for the outer. Or do we treat outer
> checksum as "preceding checksums" here.

My take is that with LCO virtio is not offloading anything. The header
subject to LCO, from virtio perspective, is just constant header, not
offloaded via any virtio hdr field. The above spec statement applies/is
valid verbatim even in presence of LCO.

Cheers,

Paolo

[PATCH v10 2/3] 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>
---
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 | 219 +++++++++++++++++++++++++++++--
 1 file changed, 211 insertions(+), 8 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 3099c6f..0279568 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,13 @@ \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.
+
 \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 +405,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
@@ -451,6 +469,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;
@@ -461,6 +481,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}
 
@@ -518,6 +540,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
@@ -534,6 +560,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
@@ -554,6 +583,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
@@ -598,6 +660,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 TCP or UDP 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
+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
+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:
@@ -606,7 +691,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}
@@ -635,7 +722,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,
@@ -644,12 +734,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.
 
@@ -679,6 +805,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}
@@ -785,8 +930,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 /
@@ -811,10 +956,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
@@ -858,6 +1018,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
@@ -881,6 +1045,18 @@ \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
+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 TCP or UDP 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.
@@ -899,7 +1075,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
@@ -907,6 +1085,11 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 checksum (in case of multiple encapsulated protocols, one level
 of checksums is validated).
 
+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}
@@ -929,6 +1112,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}
 
@@ -1772,6 +1974,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 v10 2/3] virtio-net: define UDP tunnel segmentation offload feature

[Jason Wang]

On Sat, Nov 9, 2024 at 12:53 AM 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>
> ---
> 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 | 219 +++++++++++++++++++++++++++++--
>  1 file changed, 211 insertions(+), 8 deletions(-)
>
> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> index 3099c6f..0279568 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,13 @@ \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.
> +
>  \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 +405,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
> @@ -451,6 +469,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;
> @@ -461,6 +481,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}
>
> @@ -518,6 +540,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
> @@ -534,6 +560,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
> @@ -554,6 +583,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

Do we need to clarify if "outer" here means outmost? Or we are
assuming a 1 level of encapsulation here. We probably need more
clarification in either cases.

> +      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
> @@ -598,6 +660,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 TCP or UDP 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,

I wonder if we can just remove this as

1) if we want to support other type of GSO in the future, we need to
add those types back to here
2) it only matters with the inner network protocol type

Other looks good.

Thanks

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

[Paolo Abeni]

On 11/25/24 03:50, Jason Wang wrote:
>> @@ -554,6 +583,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
> 
> Do we need to clarify if "outer" here means outmost? Or we are
> assuming a 1 level of encapsulation here. We probably need more
> clarification in either cases.

Note that this is not the first place referring to the 'outer header'.

What if I extend the paragraph introducing
'VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO' a few lines above with:

"""
  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.
"""

The whole paragraph will read as:

"""
\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.
"""

WDYT?

>> @@ -598,6 +660,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 TCP or UDP 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,
> 
> I wonder if we can just remove this as
> 
> 1) if we want to support other type of GSO in the future, we need to
> add those types back to here
> 2) it only matters with the inner network protocol type

It's not clear to me which part would you prefer to remove. Do you mean
to rephrase the above as something alike:

"""
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,
"""

basically I applied: "sed -e 's/TCP or UDP GSO/GSO/'"

Or did you mean something different?

Thanks,

Paolo

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

[Jason Wang]

On Tue, Nov 26, 2024 at 12:31 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 11/25/24 03:50, Jason Wang wrote:
> >> @@ -554,6 +583,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
> >
> > Do we need to clarify if "outer" here means outmost? Or we are
> > assuming a 1 level of encapsulation here. We probably need more
> > clarification in either cases.
>
> Note that this is not the first place referring to the 'outer header'.
>
> What if I extend the paragraph introducing
> 'VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO' a few lines above with:
>
> """
>   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.
> """
>
> The whole paragraph will read as:
>
> """
> \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.
> """
>
> WDYT?

Looks good to me.

>
> >> @@ -598,6 +660,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 TCP or UDP 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,
> >
> > I wonder if we can just remove this as
> >
> > 1) if we want to support other type of GSO in the future, we need to
> > add those types back to here
> > 2) it only matters with the inner network protocol type
>
> It's not clear to me which part would you prefer to remove.

I meant "When requesting TCP or UDP GSO segmentation over UDP tunnel,".

> Do you mean
> to rephrase the above as something alike:
>
> """
> 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,
> """

Yes, something like this.

>
> basically I applied: "sed -e 's/TCP or UDP GSO/GSO/'"
>
> Or did you mean something different?
>
> Thanks,
>
> Paolo

Thanks

>
>
>
>

[PATCH v10 3/3] 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>
---
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 | 146 ++++++++++++++++++++++++++++---
 1 file changed, 132 insertions(+), 14 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 0279568..7ca0fc2 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
@@ -398,6 +413,11 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
   the outer UDP checksum is zero - by negotiating the
   VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO feature.
 
+\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,
@@ -406,8 +426,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
@@ -463,6 +484,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
@@ -587,8 +609,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:
 
@@ -616,6 +641,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
@@ -674,6 +713,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 TCP or UDP 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 TCP or UDP 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
@@ -683,6 +732,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:
@@ -744,8 +797,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.
+
+\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.
@@ -766,6 +834,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.
 
@@ -824,6 +896,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}
@@ -926,8 +1003,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
@@ -968,12 +1048,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.
+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 
+VIRTIO_NET_HDR_F_NEEDS_CSUM, otherwise only the inner transport header
+checksum has been validated.
 \end{note}
 \end{enumerate}
 
@@ -1022,6 +1112,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
@@ -1054,7 +1147,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 TCP or UDP 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 TCP or UDP 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
@@ -1082,14 +1176,31 @@ \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. 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.
 
 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}.
 
+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}
@@ -1131,6 +1242,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}
 
@@ -1975,6 +2092,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 v10 3/3] virtio-net: define UDP tunnel checksum offload feature

[Jason Wang]

On Sat, Nov 9, 2024 at 12:53 AM 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>
> ---
> 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 | 146 ++++++++++++++++++++++++++++---
>  1 file changed, 132 insertions(+), 14 deletions(-)
>
> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> index 0279568..7ca0fc2 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
> @@ -398,6 +413,11 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
>    the outer UDP checksum is zero - by negotiating the
>    VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO feature.
>
> +\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,
> @@ -406,8 +426,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
> @@ -463,6 +484,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
> @@ -587,8 +609,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:
>
> @@ -616,6 +641,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
> @@ -674,6 +713,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 TCP or UDP 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 TCP or UDP 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
> @@ -683,6 +732,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:
> @@ -744,8 +797,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.

Is this "to the checksum of the outer UDP pseudo header."?

> +
> +\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.
> @@ -766,6 +834,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.
>
> @@ -824,6 +896,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}
> @@ -926,8 +1003,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.

Not a must but I'd suggest making this an independent patch. (I meant
the " (the outer one in case of tunnels) has been validated." part).

Or after some thought, maybe it's just better to leave it as is. This
means let the device choose which part has been validated:

1) more flexibility that it allows the device to choose the level it
can do the validation
2) no need to worry about the migration compatibility

But it would

1) complicate the implementation of both the device and driver

>  \end{enumerate}
>
>  Additionally, VIRTIO_NET_F_GUEST_CSUM, TSO4, TSO6, UDP, UDP_TUNNEL
> @@ -968,12 +1048,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.

For tunnel GSO, as replied in the patch 2, I think we need to clarify
the behavior for multiple levels of encapsulation.

> +
>  \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.
> +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
> +VIRTIO_NET_HDR_F_NEEDS_CSUM, otherwise only the inner transport header
> +checksum has been validated.
>  \end{note}
>  \end{enumerate}
>
> @@ -1022,6 +1112,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
> @@ -1054,7 +1147,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 TCP or UDP 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 TCP or UDP 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
> @@ -1082,14 +1176,31 @@ \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. 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.
>
>  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}.
>
> +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}
> @@ -1131,6 +1242,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}
>
> @@ -1975,6 +2092,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
>

Thanks

Re: [PATCH v10 3/3] virtio-net: define UDP tunnel checksum offload feature

[Paolo Abeni]

On 11/25/24 04:41, Jason Wang wrote:
>> @@ -744,8 +797,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.
> 
> Is this "to the checksum of the outer UDP pseudo header."?

Yes, good catch. I'll fix in the next version.

>> @@ -926,8 +1003,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.
> 
> Not a must but I'd suggest making this an independent patch. (I meant
> the " (the outer one in case of tunnels) has been validated." part).
> 
> Or after some thought, maybe it's just better to leave it as is. This
> means let the device choose which part has been validated:
> 
> 1) more flexibility that it allows the device to choose the level it
> can do the validation
> 2) no need to worry about the migration compatibility
> 
> But it would
> 
> 1) complicate the implementation of both the device and driver

Note that offloading the inner checksum only would be quite useless, as
AFAIK no OS as support for such a thing on RX.

I think it would be simpler to move the ' (the outer one in case of
tunnels)" in the previous patch. Why do you think it would be better to
have this single statement on a separate patch?

>>  \end{enumerate}
>>
>>  Additionally, VIRTIO_NET_F_GUEST_CSUM, TSO4, TSO6, UDP, UDP_TUNNEL
>> @@ -968,12 +1048,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.
> 
> For tunnel GSO, as replied in the patch 2, I think we need to clarify
> the behavior for multiple levels of encapsulation.

Please LMK if the reply on patch 1 suffices: this infra supports offload
for a single level of encapsulation.

Cheers,

Paolo

Re: [PATCH v10 3/3] virtio-net: define UDP tunnel checksum offload feature

[Jason Wang]

On Tue, Nov 26, 2024 at 12:44 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 11/25/24 04:41, Jason Wang wrote:
> >> @@ -744,8 +797,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.
> >
> > Is this "to the checksum of the outer UDP pseudo header."?
>
> Yes, good catch. I'll fix in the next version.
>
> >> @@ -926,8 +1003,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.
> >
> > Not a must but I'd suggest making this an independent patch. (I meant
> > the " (the outer one in case of tunnels) has been validated." part).
> >
> > Or after some thought, maybe it's just better to leave it as is. This
> > means let the device choose which part has been validated:
> >
> > 1) more flexibility that it allows the device to choose the level it
> > can do the validation
> > 2) no need to worry about the migration compatibility
> >
> > But it would
> >
> > 1) complicate the implementation of both the device and driver
>
> Note that offloading the inner checksum only would be quite useless, as
> AFAIK no OS as support for such a thing on RX.

Yes it is, a small concern is that it might be too late to do such
clarification. Rethink this, I think it would be still better to have
a clarification like this.

>
> I think it would be simpler to move the ' (the outer one in case of
> tunnels)" in the previous patch. Why do you think it would be better to
> have this single statement on a separate patch?

Because it's an independent clarification not for GSO.

>
> >>  \end{enumerate}
> >>
> >>  Additionally, VIRTIO_NET_F_GUEST_CSUM, TSO4, TSO6, UDP, UDP_TUNNEL
> >> @@ -968,12 +1048,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.
> >
> > For tunnel GSO, as replied in the patch 2, I think we need to clarify
> > the behavior for multiple levels of encapsulation.
>
> Please LMK if the reply on patch 1 suffices: this infra supports offload
> for a single level of encapsulation.

Replied in another thread.

>
> Cheers,
>
> Paolo
>

Thanks

Re: [PATCH v10 3/3] virtio-net: define UDP tunnel checksum offload feature

[Paolo Abeni]

On 11/26/24 04:07, Jason Wang wrote:
> On Tue, Nov 26, 2024 at 12:44 AM Paolo Abeni <pabeni@redhat.com> wrote:
>> On 11/25/24 04:41, Jason Wang wrote:
>>>> @@ -926,8 +1003,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.
>>>
>>> Not a must but I'd suggest making this an independent patch. (I meant
>>> the " (the outer one in case of tunnels) has been validated." part).
>>>
>>> Or after some thought, maybe it's just better to leave it as is. This
>>> means let the device choose which part has been validated:
>>>
>>> 1) more flexibility that it allows the device to choose the level it
>>> can do the validation
>>> 2) no need to worry about the migration compatibility
>>>
>>> But it would
>>>
>>> 1) complicate the implementation of both the device and driver
>>
>> Note that offloading the inner checksum only would be quite useless, as
>> AFAIK no OS as support for such a thing on RX.
> 
> Yes it is, a small concern is that it might be too late to do such
> clarification. Rethink this, I think it would be still better to have
> a clarification like this.

I'm sorry, but I'm unsure what exactly is the clarification you mention
above.

Do you mean moving """(the outer one in case of tunnels)""" into a
separate patch?

I'll do that in next revision.

Thanks!

Paolo

Re: [PATCH v10 3/3] virtio-net: define UDP tunnel checksum offload feature

[Jason Wang]

On Tue, Nov 26, 2024 at 11:37 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 11/26/24 04:07, Jason Wang wrote:
> > On Tue, Nov 26, 2024 at 12:44 AM Paolo Abeni <pabeni@redhat.com> wrote:
> >> On 11/25/24 04:41, Jason Wang wrote:
> >>>> @@ -926,8 +1003,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.
> >>>
> >>> Not a must but I'd suggest making this an independent patch. (I meant
> >>> the " (the outer one in case of tunnels) has been validated." part).
> >>>
> >>> Or after some thought, maybe it's just better to leave it as is. This
> >>> means let the device choose which part has been validated:
> >>>
> >>> 1) more flexibility that it allows the device to choose the level it
> >>> can do the validation
> >>> 2) no need to worry about the migration compatibility
> >>>
> >>> But it would
> >>>
> >>> 1) complicate the implementation of both the device and driver
> >>
> >> Note that offloading the inner checksum only would be quite useless, as
> >> AFAIK no OS as support for such a thing on RX.
> >
> > Yes it is, a small concern is that it might be too late to do such
> > clarification. Rethink this, I think it would be still better to have
> > a clarification like this.
>
> I'm sorry, but I'm unsure what exactly is the clarification you mention
> above.
>
> Do you mean moving """(the outer one in case of tunnels)""" into a
> separate patch?

Yes.

>
> I'll do that in next revision.
>
> Thanks!
>
> Paolo
>

Thanks

Re: [PATCH v10 0/3] virtio-net: define UDP tunnel offload

[Paolo Abeni]

On 11/8/24 17:52, 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:
> - a clarification on the current semantic of
> NEEDS_CSUM offload (patch 1) to make later changes easier to follow,
> - new features definition to handle both UDP tunnel segmentation
> offload (patch 2).
> - new features definition to handleUDP tunnel outer checksum offload
> (patch 2).
> 
> 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 the user-space implementation has not been updated since
> the previous iteration, since the kernel changes are transparent to it.
> 
> 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/

As a reference this is the cumulative diff WRT v9:

diff --git a/device-types/net/description.tex
b/device-types/net/description.tex
index 48e0f5e..7ca0fc2 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -582,6 +582,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
@@ -712,8 +715,10 @@ \subsubsection{Packet
Transmission}\label{sec:Device Types / Network Device / De
 The driver MUST NOT send to the device TCP or UDP 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 the
VIRTIO_NET_HDR_GSO_UDP_TUNNEL
-bit in the \field{gso_type} and the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM
bit in the \field{flags}.
+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
@@ -738,6 +743,13 @@ \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 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}
 \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
@@ -763,7 +775,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,
@@ -988,17 +1003,10 @@ \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.
-  If the VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO feature has been negotiated,
-  and 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 can additionally set the VIRTIO_NET_HDR_F_DATA_VALID bit
-  in \field{flags}, meaning that the device validated the checksum,
-  set the csum_start and csum_offset fields, but did not provide the
-  partial checksum for the transport header.
   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 inner one
+  have been validated, otherwise only one level of checksums (the outer one
   in case of tunnels) has been validated.
 \end{enumerate}

@@ -1025,10 +1033,15 @@ \subsubsection{Processing of Incoming
Packets}\label{sec:Device Types / Network
   from \field{csum_start} and any preceding checksums
   have been validated.  The checksum on the packet is incomplete and
   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.
-  If the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or
VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6
-  bit are set, the \field{csum_start} field
-  refers to the inner transport header offset (see Packet Transmission
point 1).
+  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 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
@@ -1037,13 +1050,21 @@ \subsubsection{Processing of Incoming
Packets}\label{sec:Device Types / Network
   headers information.
 \item If the VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM feature was
 negotiated, and
-  the VIRTIO_NET_HDR_GSO_UDP_TUNNEL bit is 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.
-  If the VIRTIO_NET_HDR_F_DATA_VALID bit in \field{flags} is not set,
the UDP
-  header checksum at offset 6 from from \field{outer_th_offset}
+  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).
+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}

 If applicable, the device calculates per-packet hash for incoming
packets as
@@ -1148,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
@@ -1157,19 +1180,19 @@ \subsubsection{Processing of Incoming
Packets}\label{sec:Device Types / Network
 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 inner one in case of tunnels) has been validated. \note{This implies
-that if either the VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
+(the outer one in case of tunnels) has been validated.
+
+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 the set the VIRTIO_NET_HDR_F_DATA_VALID in \field{flags} is set,
-the the VIRTIO_NET_F_GUEST_CSUM bit in \field{flags} is must be set, too}
+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 and the VIRTIO_NET_F_DATA_VALID bit in \field{flags}
-is not set, the device MUST set the packet outer UDP checksum stored in the
-receive buffer to the outer UDP pseudo header.
+\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
@@ -1212,13 +1235,15 @@ \subsubsection{Processing of Incoming
Packets}\label{sec:Device Types / Network
 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_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.

-If the VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit in \field{flags} is set,
+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.

Re: [PATCH v10 0/3] virtio-net: define UDP tunnel offload

[Willem de Bruijn]

On Fri, Nov 15, 2024 at 12:44 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 11/8/24 17:52, 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:
> > - a clarification on the current semantic of
> > NEEDS_CSUM offload (patch 1) to make later changes easier to follow,
> > - new features definition to handle both UDP tunnel segmentation
> > offload (patch 2).
> > - new features definition to handleUDP tunnel outer checksum offload
> > (patch 2).
> >
> > 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 the user-space implementation has not been updated since
> > the previous iteration, since the kernel changes are transparent to it.
> >
> > 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/
>
> As a reference this is the cumulative diff WRT v9:

Thanks Paolo. I think this matches our conversation and addresses the
open comments to v8.