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

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

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.

>  			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);

(...)