Netdev List
 help / color / mirror / Atom feed
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: Eric Dumazet <edumazet@kernel.org>,
	 Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: "David S . Miller" <davem@davemloft.net>,
	 Jakub Kicinski <kuba@kernel.org>,
	 Paolo Abeni <pabeni@redhat.com>,
	 Simon Horman <horms@kernel.org>,
	 Willem de Bruijn <willemb@google.com>,
	 "Michael S . Tsirkin" <mst@redhat.com>,
	 netdev@vger.kernel.org
Subject: Re: [PATCH v2 net 2/2] selftests: net: tun: add test for VLAN-tagged GSO without NEEDS_CSUM
Date: Mon, 28 Sep 2026 16:22:23 -0400	[thread overview]
Message-ID: <willemdebruijn.kernel.100d5c1377db@gmail.com> (raw)
In-Reply-To: <CAL4Wiip8iSAYT0NgFgEidvk4TD35aemRNfFVUjAnUoVaqips9g@mail.gmail.com>

Eric Dumazet wrote:
> On Mon, Sep 28, 2026 at 9:17 PM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > Eric Dumazet wrote:
> > > Add a selftest in tun.c verifying that a VLAN-tagged (802.1Q) TCPv4 GSO
> > > packet without VIRTIO_NET_HDR_F_NEEDS_CSUM (both flags = 0 and
> > > flags = VIRTIO_NET_HDR_F_DATA_VALID) is accepted when written to a TAP
> > > device (/dev/net/tun with IFF_TAP | IFF_NO_PI | IFF_VNET_HDR).
> > >
> > > Also verify that mismatched GSO types (e.g. VIRTIO_NET_HDR_GSO_TCPV6 on
> > > a VLAN-tagged IPv4 packet) and truncated headers without NEEDS_CSUM are
> > > rejected with -EINVAL.
> > >
> > > Based on a reproducer by Michael S. Tsirkin <mst@redhat.com>.
> > >
> > > Assisted-by: LLM
> > > Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> > > ---
> > >  tools/testing/selftests/net/tun.c | 107 ++++++++++++++++++++++++++++++
> > >  1 file changed, 107 insertions(+)
> > >
> > > diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
> > > index abe488bac50bb3b05df5c446836c1c3dfa9ea604..c6afafb7b957360e2873640e60a7d580c6564a66 100644
> > > --- a/tools/testing/selftests/net/tun.c
> > > +++ b/tools/testing/selftests/net/tun.c
> > > @@ -9,6 +9,7 @@
> > >  #include <string.h>
> > >  #include <unistd.h>
> > >  #include <linux/if_tun.h>
> > > +#include <netinet/tcp.h>
> > >  #include <sys/ioctl.h>
> > >  #include <sys/socket.h>
> > >
> > > @@ -542,6 +543,112 @@ TEST_F(tun, reattach_close_delete)
> > >       EXPECT_EQ(tun_delete(self->ifname), 0);
> > >  }
> > >
> > > +FIXTURE(tun_vnet_gso)
> > > +{
> > > +     char ifname[IFNAMSIZ];
> > > +     int fd;
> > > +};
> > > +
> > > +FIXTURE_SETUP(tun_vnet_gso)
> > > +{
> > > +     int flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
> > > +
> > > +     memset(self->ifname, 0, sizeof(self->ifname));
> > > +     self->fd = tun_open(self->ifname, flags, 0, 0, NULL);
> > > +     ASSERT_GE(self->fd, 0);
> > > +}
> > > +
> > > +FIXTURE_TEARDOWN(tun_vnet_gso)
> > > +{
> > > +     if (self->fd >= 0)
> > > +             close(self->fd);
> > > +}
> > > +
> > > +static int build_vlan_tcpv4_gso_packet(uint8_t *buf, int payload_len)
> > > +{
> > > +     uint16_t vlan_tag[2] = { htons(100), htons(ETH_P_IP) };
> > > +     uint8_t *cur = buf + sizeof(struct virtio_net_hdr);
> > > +     struct virtio_net_hdr vh = { 0 };
> > > +     struct tcphdr tcph = { 0 };
> > > +     uint32_t sum;
> > > +
> > > +     cur += build_eth(cur, ETH_P_8021Q, param_hwaddr_outer_src,
> > > +                      param_hwaddr_outer_dst);
> > > +
> > > +     /* 802.1Q tag: VID=100, inner protocol=ETH_P_IP */
> > > +     memcpy(cur, vlan_tag, sizeof(vlan_tag));
> > > +     cur += sizeof(vlan_tag);
> > > +
> > > +     cur += build_ipv4_header(cur, IPPROTO_TCP,
> > > +                              sizeof(tcph) + payload_len,
> > > +                              &param_ipaddr4_outer_src,
> > > +                              &param_ipaddr4_outer_dst);
> > > +
> > > +     tcph.source = htons(12345);
> > > +     tcph.dest = htons(80);
> > > +     tcph.seq = htonl(1);
> > > +     tcph.doff = sizeof(tcph) / 4;
> > > +     tcph.ack = 1;
> > > +     tcph.window = htons(65535);
> > > +     memcpy(cur, &tcph, sizeof(tcph));
> > > +     memset(cur + sizeof(tcph), PKT_DATA, payload_len);
> > > +
> > > +     sum = add_csum((const uint8_t *)&param_ipaddr4_outer_src,
> > > +                    sizeof(param_ipaddr4_outer_src));
> > > +     sum += add_csum((const uint8_t *)&param_ipaddr4_outer_dst,
> > > +                     sizeof(param_ipaddr4_outer_dst));
> > > +     sum += htons(IPPROTO_TCP) + htons(sizeof(tcph) + payload_len);
> > > +     sum += add_csum(cur, sizeof(tcph) + payload_len);
> > > +     tcph.check = finish_ip_csum(sum);
> > > +     memcpy(cur, &tcph, sizeof(tcph));
> > > +     cur += sizeof(tcph) + payload_len;
> > > +
> > > +     vh.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> > > +     vh.gso_size = 1400;
> > > +     vh.hdr_len = (cur - buf) - sizeof(vh) - payload_len;
> > > +     memcpy(buf, &vh, sizeof(vh));
> > > +
> > > +     return cur - buf;
> > > +}
> > > +
> > > +TEST_F(tun_vnet_gso, vlan_tcpv4_gso_no_csum)
> > > +{
> > > +     struct virtio_net_hdr vh;
> > > +     uint8_t pkt[4096] = { 0 };
> > > +     int len, ret;
> > > +
> > > +     len = build_vlan_tcpv4_gso_packet(pkt, 2800);
> > > +     memcpy(&vh, pkt, sizeof(vh));
> > > +
> > > +     /* Valid VLAN-tagged TCPv4 GSO with flags = 0 (no NEEDS_CSUM) */
> >
> > This is not a GSO packet if no vh.gso_type
> >
> > > +     vh.flags = 0;
> > > +     memcpy(pkt, &vh, sizeof(vh));
> > > +     ret = write(self->fd, pkt, len);
> > > +     ASSERT_EQ(ret, len);
> > > +
> > > +     /* Valid VLAN-tagged TCPv4 GSO with flags = DATA_VALID */
> >
> > Same
> 
> build_vlan_tcpv4_gso_packet() already sets:
> 
> vh.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> vh.gso_size = 1400;
> vh.hdr_len = (cur - buf) - sizeof(vh) - payload_len;
> memcpy(buf, &vh, sizeof(vh));

Missed that, sorry. I was only looking at the top of the function.

> and vlan_tcpv4_gso_no_csum() copies it back into the local vh right
> afterwards:
> 
> len = build_vlan_tcpv4_gso_packet(pkt, 2800);
> memcpy(&vh, pkt, sizeof(vh));
> 
> so vh.gso_type is VIRTIO_NET_HDR_GSO_TCPV4 (and vh.gso_size is 1400) for
> both the flags = 0 and flags = VIRTIO_NET_HDR_F_DATA_VALID cases.



  reply	other threads:[~2026-09-28 20:22 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-28 14:42 [PATCH v2 net 0/2] net: always dissect GSO packets in __virtio_net_hdr_to_skb() Eric Dumazet
2026-09-28 14:42 ` [PATCH v2 net 1/2] " Eric Dumazet
2026-10-01 17:52   ` netdev-bot+sashiko
2026-10-01 18:34     ` Eric Dumazet
2026-09-28 14:42 ` [PATCH v2 net 2/2] selftests: net: tun: add test for VLAN-tagged GSO without NEEDS_CSUM Eric Dumazet
2026-09-28 19:17   ` Willem de Bruijn
2026-09-28 19:43     ` Eric Dumazet
2026-09-28 20:22       ` Willem de Bruijn [this message]
2026-10-01 17:52   ` netdev-bot+sashiko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=willemdebruijn.kernel.100d5c1377db@gmail.com \
    --to=willemdebruijn.kernel@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@kernel.org \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=willemb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox