Thread (37 messages) 37 messages, 3 authors, 2025-06-17

Re: [PATCH RFC v3 6/8] virtio_net: enable gso over UDP tunnel support.

From: Jason Wang <jasowang@redhat.com>
Date: 2025-06-16 03:20:58

On Thu, Jun 12, 2025 at 6:18 PM Paolo Abeni [off-list ref] wrote:
On 6/12/25 6:05 AM, Jason Wang wrote:
quoted
On Fri, Jun 6, 2025 at 7:46 PM Paolo Abeni [off-list ref] wrote:
quoted
If the related virtio feature is set, enable transmission and reception
of gso over UDP tunnel packets.

Most of the work is done by the previously introduced helper, just need
to determine the UDP tunnel features inside the virtio_net_hdr and
update accordingly the virtio net hdr size.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v2 -> v3:
  - drop the VIRTIO_HAS_EXTENDED_FEATURES conditionals

v1 -> v2:
  - test for UDP_TUNNEL_GSO* only on builds with extended features support
  - comment indentation cleanup
  - rebased on top of virtio helpers changes
  - dump more information in case of bad offloads
---
 drivers/net/virtio_net.c | 70 +++++++++++++++++++++++++++++++---------
 1 file changed, 54 insertions(+), 16 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 18ad50de4928..0b234f318e39 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -78,15 +78,19 @@ static const unsigned long guest_offloads[] = {
        VIRTIO_NET_F_GUEST_CSUM,
        VIRTIO_NET_F_GUEST_USO4,
        VIRTIO_NET_F_GUEST_USO6,
-       VIRTIO_NET_F_GUEST_HDRLEN
+       VIRTIO_NET_F_GUEST_HDRLEN,
+       VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_MAPPED,
+       VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM_MAPPED,
 };

 #define GUEST_OFFLOAD_GRO_HW_MASK ((1ULL << VIRTIO_NET_F_GUEST_TSO4) | \
-                               (1ULL << VIRTIO_NET_F_GUEST_TSO6) | \
-                               (1ULL << VIRTIO_NET_F_GUEST_ECN)  | \
-                               (1ULL << VIRTIO_NET_F_GUEST_UFO)  | \
-                               (1ULL << VIRTIO_NET_F_GUEST_USO4) | \
-                               (1ULL << VIRTIO_NET_F_GUEST_USO6))
+                       (1ULL << VIRTIO_NET_F_GUEST_TSO6) | \
+                       (1ULL << VIRTIO_NET_F_GUEST_ECN)  | \
+                       (1ULL << VIRTIO_NET_F_GUEST_UFO)  | \
+                       (1ULL << VIRTIO_NET_F_GUEST_USO4) | \
+                       (1ULL << VIRTIO_NET_F_GUEST_USO6) | \
+                       (1ULL << VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_MAPPED) | \
+                       (1ULL << VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM_MAPPED))

 struct virtnet_stat_desc {
        char desc[ETH_GSTRING_LEN];
@@ -436,9 +440,14 @@ struct virtnet_info {
        /* Packet virtio header size */
        u8 hdr_len;

+       /* UDP tunnel support */
+       u8 tnl_offset;
+
        /* Work struct for delayed refilling if we run low on memory. */
        struct delayed_work refill;

+       bool rx_tnl_csum;
+
        /* Is delayed refill enabled? */
        bool refill_enabled;
@@ -2531,14 +2540,22 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
        if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report)
                virtio_skb_set_hash(&hdr->hash_v1_hdr, skb);

-       if (flags & VIRTIO_NET_HDR_F_DATA_VALID)
-               skb->ip_summed = CHECKSUM_UNNECESSARY;
+       /* restore the received value */
Nit: this comment seems to be redundant
quoted
+       hdr->hdr.flags = flags;
+       if (virtio_net_chk_data_valid(skb, &hdr->hdr, vi->rx_tnl_csum)) {
Nit: this function did more than just check DATA_VALID, we probably
need a better name.
What about virtio_net_handle_csum_offload()?
Works for me.
quoted
quoted
+               net_warn_ratelimited("%s: bad csum: flags: %x, gso_type: %x rx_tnl_csum %d\n",
+                                    dev->name, hdr->hdr.flags,
+                                    hdr->hdr.gso_type, vi->rx_tnl_csum);
+               goto frame_err;
+       }

-       if (virtio_net_hdr_to_skb(skb, &hdr->hdr,
-                                 virtio_is_little_endian(vi->vdev))) {
-               net_warn_ratelimited("%s: bad gso: type: %u, size: %u\n",
+       if (virtio_net_hdr_tnl_to_skb(skb, &hdr->hdr, vi->tnl_offset,
I wonder why virtio_net_chk_data_valid() is not part of the
virtio_net_hdr_tnl_to_skb().
It can't be part of virtio_net_hdr_tnl_to_skb(), as hdr to skb
conversion is actually not symmetric with respect to the checksum - only
the driver handles DATA_VALID.

Tun must not call virtio_net_chk_data_valid()  (or whatever different
name will use).
Note that we've already dealt with this via introducing a boolean
has_data_valid:

static inline int virtio_net_hdr_from_skb(const struct sk_buff *skb,
                                          struct virtio_net_hdr *hdr,
                                          bool little_endian,
                                          bool has_data_valid,
                                          int vlan_hlen)
/P
Thanks
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help