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

Jason Wang
2024-11-28T03:01:49Z

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

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

Thanks