linux-kernel - Re: [PATCH net V2] virtio_net: fix alignment for virtio_net_hdr_v1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPpAL=ybrp2kBCLivWpX_0NpV7tiCS26o6tzeDea6RWsAkS2Hg@mail.gmail.com>
Date: Wed, 5 Nov 2025 01:21:29 +0800
From: Lei Yang <leiyang@...hat.com>
To: Jason Wang <jasowang@...hat.com>
Cc: mst@...hat.com, xuanzhuo@...ux.alibaba.com, eperezma@...hat.com, 
	andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com, 
	kuba@...nel.org, pabeni@...hat.com, virtualization@...ts.linux.dev, 
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH net V2] virtio_net: fix alignment for virtio_net_hdr_v1_hash

Tested this patch with virtio-net regression tests, everything works fine.

Tested-by: Lei Yang <leiyang@...hat.com>

On Fri, Oct 31, 2025 at 2:06 PM Jason Wang <jasowang@...hat.com> wrote:
>
> From: "Michael S. Tsirkin" <mst@...hat.com>
>
> 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.
>
> This fixes the performance regression since
> commit ("virtio_net: enable gso over UDP tunnel support.") as it uses
> virtio_net_hdr_v1_hash_tunnel which embeds
> virtio_net_hdr_v1_hash. Pktgen in guest + XDP_DROP on TAP + vhost_net
> shows the TX PPS is recovered from 2.4Mpps to 4.45Mpps.
>
> Fixes: 56a06bd40fab ("virtio_net: enable gso over UDP tunnel support.")
> Cc: stable@...r.kernel.org
> Signed-off-by: Michael S. Tsirkin <mst@...hat.com>
> Signed-off-by: Jason Wang <jasowang@...hat.com>
> ---
> Changes since V1:
> - Fix build issues of virtio_net_hdr_tnl_from_skb()
> ---
>  drivers/net/virtio_net.c        | 15 +++++++++++++--
>  include/linux/virtio_net.h      |  3 ++-
>  include/uapi/linux/virtio_net.h |  3 ++-
>  3 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 8e8a179aaa49..e6e650bc3bc3 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2539,6 +2539,13 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>         return NULL;
>  }
>
> +static inline 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)
>  {
> @@ -2565,7 +2572,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,
> @@ -3311,6 +3318,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;
> @@ -6750,7 +6761,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/linux/virtio_net.h b/include/linux/virtio_net.h
> index 4d1780848d0e..b673c31569f3 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -401,7 +401,8 @@ virtio_net_hdr_tnl_from_skb(const struct sk_buff *skb,
>         if (!tnl_hdr_negotiated)
>                 return -EINVAL;
>
> -        vhdr->hash_hdr.hash_value = 0;
> +       vhdr->hash_hdr.hash_value_lo = 0;
> +       vhdr->hash_hdr.hash_value_hi = 0;
>          vhdr->hash_hdr.hash_report = 0;
>          vhdr->hash_hdr.padding = 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
> --
> 2.31.1
>
>