Re: [PATCH bpf-next 08/15] selftests/xsk: remove namespaces

[Magnus Karlsson <magnus.karlsson@gmail.com>]

On Mon, Dec 12, 2022 at 3:20 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Tue, Dec 06, 2022 at 10:08:19AM +0100, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Remove the namespaces used as they fill no function. This will
> > simplify the code for speeding up the tests in the following commits.
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> >  tools/testing/selftests/bpf/test_xsk.sh    | 33 +++++++----------
> >  tools/testing/selftests/bpf/xsk_prereqs.sh | 12 ++-----
> >  tools/testing/selftests/bpf/xskxceiver.c   | 42 +++-------------------
> >  tools/testing/selftests/bpf/xskxceiver.h   |  3 --
> >  4 files changed, 19 insertions(+), 71 deletions(-)
> >
>
> (...)
>
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> > index 72578cebfbf7..0aaf2f0a9d75 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -55,12 +55,11 @@
> >   * Flow:
> >   * -----
> >   * - Single process spawns two threads: Tx and Rx
> > - * - Each of these two threads attach to a veth interface within their assigned
> > - *   namespaces
> > - * - Each thread Creates one AF_XDP socket connected to a unique umem for each
> > + * - Each of these two threads attach to a veth interface
> > + * - Each thread creates one AF_XDP socket connected to a unique umem for each
> >   *   veth interface
> > - * - Tx thread Transmits 10k packets from veth<xxxx> to veth<yyyy>
> > - * - Rx thread verifies if all 10k packets were received and delivered in-order,
> > + * - Tx thread Transmits a number of packets from veth<xxxx> to veth<yyyy>
> > + * - Rx thread verifies if all packets were received and delivered in-order,
> >   *   and have the right content
> >   *
> >   * Enable/disable packet dump mode:
> > @@ -399,28 +398,6 @@ static void usage(const char *prog)
> >       ksft_print_msg(str, prog);
> >  }
> >
> > -static int switch_namespace(const char *nsname)
> > -{
> > -     char fqns[26] = "/var/run/netns/";
> > -     int nsfd;
> > -
> > -     if (!nsname || strlen(nsname) == 0)
> > -             return -1;
> > -
> > -     strncat(fqns, nsname, sizeof(fqns) - strlen(fqns) - 1);
> > -     nsfd = open(fqns, O_RDONLY);
> > -
> > -     if (nsfd == -1)
> > -             exit_with_error(errno);
> > -
> > -     if (setns(nsfd, 0) == -1)
> > -             exit_with_error(errno);
> > -
> > -     print_verbose("NS switched: %s\n", nsname);
> > -
> > -     return nsfd;
> > -}
> > -
> >  static bool validate_interface(struct ifobject *ifobj)
> >  {
> >       if (!strcmp(ifobj->ifname, ""))
> > @@ -438,7 +415,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> >       opterr = 0;
> >
> >       for (;;) {
> > -             char *sptr, *token;
> > +             char *sptr;
> >
> >               c = getopt_long(argc, argv, "i:Dvb", long_options, &option_index);
> >               if (c == -1)
> > @@ -455,9 +432,6 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> >
> >                       sptr = strndupa(optarg, strlen(optarg));
>
> Wasn't this strndupa needed only because of strsep usage?
> I feel that now you can just memcpy directly from optarg to ifobj->nsname.

Will fix.

> >                       memcpy(ifobj->ifname, strsep(&sptr, ","), MAX_INTERFACE_NAME_CHARS);
> > -                     token = strsep(&sptr, ",");
> > -                     if (token)
> > -                             memcpy(ifobj->nsname, token, MAX_INTERFACES_NAMESPACE_CHARS);
> >                       interface_nb++;
> >                       break;
> >               case 'D':
> > @@ -1283,8 +1257,6 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
> >       int ret, ifindex;
> >       void *bufs;
> >
> > -     ifobject->ns_fd = switch_namespace(ifobject->nsname);
> > -
> >       if (ifobject->umem->unaligned_mode)
> >               mmap_flags |= MAP_HUGETLB;
> >
> > @@ -1843,8 +1815,6 @@ static struct ifobject *ifobject_create(void)
> >       if (!ifobj->umem)
> >               goto out_umem;
> >
> > -     ifobj->ns_fd = -1;
> > -
> >       return ifobj;
> >
> >  out_umem:
> > @@ -1856,8 +1826,6 @@ static struct ifobject *ifobject_create(void)
> >
> >  static void ifobject_delete(struct ifobject *ifobj)
> >  {
> > -     if (ifobj->ns_fd != -1)
> > -             close(ifobj->ns_fd);
> >       free(ifobj->umem);
> >       free(ifobj->xsk_arr);
> >       free(ifobj);
>
> (...)