All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.