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
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.
/* 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
if (can_push)
- csum_start offset below. */
@@ -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))
vi->hdr_len = sizeof(struct virtio_net_hdr_v1_hash_tunnel);virtio_has_feature(vdev, VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO)) {
- 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) ||
vi->hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
- 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