On Tue, Oct 28, 2025 at 10:39 PM Michael S. Tsirkin mst@redhat.com wrote:

On Tue, Oct 28, 2025 at 11:03:41AM +0800, Jason Wang wrote:

...

Commit 56a06bd40fab ("virtio_net: enable gso over UDP tunnel support.") switches to check the alignment of the virtio_net_hdr_v1_hash_tunnel even when doing the transmission even if the feature is not negotiated. This will cause

you mean this causes

...

a series performance degradation of pktgen as the skb->data can't satisfy the alignment requirement due to the increase of the header size then virtio-net must prepare at least 2 sgs with indirect descriptors which will introduce overheads in the

introduces, accordinglt

...

device.

Fixing this by calculate the header alignment during probe so when tunnel gso is not negotiated, we can less strict.

Pktgen in guest + XDP_DROP on TAP + vhost_net shows the TX PPS is recovered from 2.4Mpps to 4.45Mpps.

Note that we still need a way to recover the performance when tunnel gso is enabled, probably a new vnet header format.

you mean improve, not recover as such

The PPS drops to 2.4Mpps after the 56a06bd40fab, so this patch recovers it the number before 56a06bd40fab.

...

Fixes: 56a06bd40fab ("virtio_net: enable gso over UDP tunnel support.") Cc: stable@vger.kernel.org Signed-off-by: Jason Wang jasowang@redhat.com

---

drivers/net/virtio_net.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 31bd32bdecaf..5b851df749c0 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -441,6 +441,9 @@ struct virtnet_info { /* Packet virtio header size */ u8 hdr_len;

• /* header alignment */
  

• size_t hdr_align;
  

It makes no sense to have u8 for length but size_t for alignment, and u8 would fit in a memory hole we have, anyway.

Oh right.

...

  /* Work struct for delayed refilling if we run low on memory. */
  struct delayed_work refill;

@@ -3308,8 +3311,9 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb, bool orphan) pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);

  can_push = vi->any_header_sg &&

•         !((unsigned long)skb->data & (__alignof__(*hdr) - 1)) &&
  

•         !((unsigned long)skb->data & (vi->hdr_align - 1)) &&
  

So let me get it straight. We use the alignment check to be able to cast to the correct type. The issue is that alignment for the header changed.

virtio_net_hdr_v1 has 2 byte alignment, but:

struct virtio_net_hdr_v1_hash { struct virtio_net_hdr_v1 hdr; __le32 hash_value; #define VIRTIO_NET_HASH_REPORT_NONE 0 #define VIRTIO_NET_HASH_REPORT_IPv4 1 #define VIRTIO_NET_HASH_REPORT_TCPv4 2 #define VIRTIO_NET_HASH_REPORT_UDPv4 3 #define VIRTIO_NET_HASH_REPORT_IPv6 4 #define VIRTIO_NET_HASH_REPORT_TCPv6 5 #define VIRTIO_NET_HASH_REPORT_UDPv6 6 #define VIRTIO_NET_HASH_REPORT_IPv6_EX 7 #define VIRTIO_NET_HASH_REPORT_TCPv6_EX 8 #define VIRTIO_NET_HASH_REPORT_UDPv6_EX 9 __le16 hash_report; __le16 padding; };

has 4 byte due to hash_value, and accordingly:

struct virtio_net_hdr_v1_hash_tunnel { struct virtio_net_hdr_v1_hash hash_hdr; __le16 outer_th_offset; __le16 inner_nh_offset; };

now is 4 byte aligned so everything is messed up: net tends not to be 4 byte aligned.

...

          !skb_header_cloned(skb) && skb_headroom(skb) >= hdr_len;

• /* Even if we can, don't push here yet as this would skew
   * csum_start offset below. */
  if (can_push)
  

@@ -6926,15 +6930,20 @@ static int virtnet_probe(struct virtio_device *vdev) }

  if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO) ||

•     virtio_has_feature(vdev, VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO))
  

•     virtio_has_feature(vdev, VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO)) {
          vi->hdr_len = sizeof(struct virtio_net_hdr_v1_hash_tunnel);
  

• else if (vi->has_rss_hash_report)
  

•         vi->hdr_align = __alignof__(struct virtio_net_hdr_v1_hash_tunnel);
  

• } else if (vi->has_rss_hash_report) {
          vi->hdr_len = sizeof(struct virtio_net_hdr_v1_hash);
  

• else if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
  

•          virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
  

•         vi->hdr_align = __alignof__(struct virtio_net_hdr_v1_hash);
  

• } else if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF) ||
  

•         virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
          vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
  

• else
  

•         vi->hdr_align = __alignof__(struct virtio_net_hdr_mrg_rxbuf);
  

• } else {
          vi->hdr_len = sizeof(struct virtio_net_hdr);
  

•         vi->hdr_align = __alignof__(struct virtio_net_hdr);
  

• }
  
  if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM))
          vi->rx_tnl_csum = true;
  

So how about just fixing the root cause then? Like this (untested, if you agree pls take over this):

---

virtio_net: fix alignment for virtio_net_hdr_v1_hash

changing alignment of header would mean it's no longer safe to cast a 2 byte aligned pointer between formats. Use two 16 bit fields to make it 2 byte aligned as previously.

Signed-off-by: Michael S. Tsirkin mst@redhat.com

This looks indeed better, will go this way.

--

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 31bd32bdecaf..02ce5316f47d 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -2535,6 +2535,13 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, return NULL; }

+static inline u16

Should be u32.

+virtio_net_hash_value(const struct virtio_net_hdr_v1_hash *hdr_hash) +{

•   return __le16_to_cpu(hdr_hash->hash_value_lo) |
  

•           (__le16_to_cpu(hdr_hash->hash_value_hi) << 16);
  

+}

static void virtio_skb_set_hash(const struct virtio_net_hdr_v1_hash *hdr_hash, struct sk_buff *skb) { @@ -2561,7 +2568,7 @@ static void virtio_skb_set_hash(const struct virtio_net_hdr_v1_hash *hdr_hash, default: rss_hash_type = PKT_HASH_TYPE_NONE; }

•   skb_set_hash(skb, __le32_to_cpu(hdr_hash->hash_value), rss_hash_type);
  

•   skb_set_hash(skb, virtio_net_hash_value(hdr_hash), rss_hash_type);
  

}

static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *rq, @@ -3307,6 +3314,10 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb, bool orphan)

    pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);

•   /* Make sure it's safe to cast between formats */
  

•   BUILD_BUG_ON(__alignof__(*hdr) != __alignof__(hdr->hash_hdr));
  

•   BUILD_BUG_ON(__alignof__(*hdr) != __alignof__(hdr->hash_hdr.hdr));
  

•   can_push = vi->any_header_sg &&
            !((unsigned long)skb->data & (__alignof__(*hdr) - 1)) &&
            !skb_header_cloned(skb) && skb_headroom(skb) >= hdr_len;
  

@@ -6755,7 +6766,7 @@ static int virtnet_xdp_rx_hash(const struct xdp_md *_ctx, u32 *hash, hash_report = VIRTIO_NET_HASH_REPORT_NONE;

    *rss_type = virtnet_xdp_rss_type[hash_report];

•   *hash = __le32_to_cpu(hdr_hash->hash_value);
  

•   *hash = virtio_net_hash_value(hdr_hash);
    return 0;
  

}

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h index 8bf27ab8bcb4..1db45b01532b 100644 --- a/include/uapi/linux/virtio_net.h +++ b/include/uapi/linux/virtio_net.h @@ -193,7 +193,8 @@ struct virtio_net_hdr_v1 {

struct virtio_net_hdr_v1_hash { struct virtio_net_hdr_v1 hdr;

•   __le32 hash_value;
  

•   __le16 hash_value_lo;
  

•   __le16 hash_value_hi;
  

#define VIRTIO_NET_HASH_REPORT_NONE 0 #define VIRTIO_NET_HASH_REPORT_IPv4 1 #define VIRTIO_NET_HASH_REPORT_TCPv4 2

Thanks