Re: [PATCH net-next v9 5/6] selftest: tun: Add tests for virtio-net hashing

[Akihiko Odaki]

On 2025/03/10 13:03, Jason Wang wrote:
> On Fri, Mar 7, 2025 at 7:02 PM Akihiko Odaki <akihiko.odaki@xxxxxxxxxx> wrote:
> > The added tests confirm tun can perform RSS and hash reporting, and
> > reject invalid configurations for them.
>
> Let's be more verbose here. E.g what's the network topology used here.

The network topology doesn't matter because this only tests the rx of 
one device.

I can still add more details; it tests all supported hash types, and 
tests both the queue selection and reported hash values.

And this message is wrong in terms that it does not test validation of 
configuration so it also needs correction.

> > Signed-off-by: Akihiko Odaki <akihiko.odaki@xxxxxxxxxx>
> > Tested-by: Lei Yang <leiyang@xxxxxxxxxx>
> > ---
> >   tools/testing/selftests/net/Makefile |   2 +-
> >   tools/testing/selftests/net/tun.c    | 584 ++++++++++++++++++++++++++++++++++-
> >   2 files changed, 576 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> > index 73ee88d6b043004be23b444de667a1d99a6045de..9772f691a9a011d99212df32463cdb930cf0a1a0 100644
> > --- a/tools/testing/selftests/net/Makefile
> > +++ b/tools/testing/selftests/net/Makefile
> > @@ -123,6 +123,6 @@ $(OUTPUT)/reuseport_bpf_numa: LDLIBS += -lnuma
> >   $(OUTPUT)/tcp_mmap: LDLIBS += -lpthread -lcrypto
> >   $(OUTPUT)/tcp_inq: LDLIBS += -lpthread
> >   $(OUTPUT)/bind_bhash: LDLIBS += -lpthread
> > -$(OUTPUT)/io_uring_zerocopy_tx: CFLAGS += -I../../../include/
> > +$(OUTPUT)/io_uring_zerocopy_tx $(OUTPUT)/tun: CFLAGS += -I../../../include/
> >
> >   include bpf.mk
> > diff --git a/tools/testing/selftests/net/tun.c b/tools/testing/selftests/net/tun.c
> > index 463dd98f2b80b1bdcb398cee43c834e7dc5cf784..acadeea7194eaea9416a605b47f99f7a5f1f80cd 100644
> > --- a/tools/testing/selftests/net/tun.c
> > +++ b/tools/testing/selftests/net/tun.c
> > @@ -2,21 +2,38 @@
> >
> >   #define _GNU_SOURCE
> >
> > +#include <endian.h>
> >   #include <errno.h>
> >   #include <fcntl.h>
> > +#include <sched.h>
>
> Is this needed?

Yes, it is for unshare().

> > +#include <stddef.h>
> >   #include <stdio.h>
> >   #include <stdlib.h>
> >   #include <string.h>
> >   #include <unistd.h>
> > -#include <linux/if.h>
> > +#include <net/if.h>
> > +#include <netinet/ip.h>
> > +#include <sys/ioctl.h>
> > +#include <sys/socket.h>
> > +#include <linux/compiler.h>
> > +#include <linux/icmp.h>
> > +#include <linux/if_arp.h>
> >   #include <linux/if_tun.h>
> > +#include <linux/ipv6.h>
> >   #include <linux/netlink.h>
> >   #include <linux/rtnetlink.h>
> > -#include <sys/ioctl.h>
> > -#include <sys/socket.h>
> > +#include <linux/sockios.h>
> > +#include <linux/tcp.h>
> > +#include <linux/udp.h>
> > +#include <linux/virtio_net.h>
> >
> >   #include "../kselftest_harness.h"
> >
> > +#define TUN_HWADDR_SOURCE { 0x02, 0x00, 0x00, 0x00, 0x00, 0x00 }
> > +#define TUN_HWADDR_DEST { 0x02, 0x00, 0x00, 0x00, 0x00, 0x01 }
> > +#define TUN_IPADDR_SOURCE htonl((172 << 24) | (17 << 16) | 0)
> > +#define TUN_IPADDR_DEST htonl((172 << 24) | (17 << 16) | 1)
> > +
> >   static int tun_attach(int fd, char *dev)
> >   {
> >          struct ifreq ifr;
> > @@ -39,7 +56,7 @@ static int tun_detach(int fd, char *dev)
> >          return ioctl(fd, TUNSETQUEUE, (void *) &ifr);
> >   }
> >
> > -static int tun_alloc(char *dev)
> > +static int tun_alloc(char *dev, short flags)
> >   {
> >          struct ifreq ifr;
> >          int fd, err;
> > @@ -52,7 +69,8 @@ static int tun_alloc(char *dev)
> >
> >          memset(&ifr, 0, sizeof(ifr));
> >          strcpy(ifr.ifr_name, dev);
> > -       ifr.ifr_flags = IFF_TAP | IFF_NAPI | IFF_MULTI_QUEUE;
> > +       ifr.ifr_flags = flags | IFF_TAP | IFF_NAPI | IFF_NO_PI |
> > +                       IFF_MULTI_QUEUE;
> >
> >          err = ioctl(fd, TUNSETIFF, (void *) &ifr);
> >          if (err < 0) {
> > @@ -64,6 +82,40 @@ static int tun_alloc(char *dev)
> >          return fd;
> >   }
> >
> > +static bool tun_add_to_bridge(int local_fd, const char *name)
> > +{
>
> I wonder if a packet socket is more convenient here.

I'll try it. Thanks for suggestion.

Regards,
Akihiko Odaki

> Thanks