Subject: Re: [PATCH bpf-next v3 2/5] selftests/bpf: xsk selftests - SKB POLL, NOPOLL

From: Yonghong Song <[email-masked]>
Date: Thu, 26 Nov 2020 20:31:49 -0800

On 11/25/20 10:37 AM, Weqaar Janjua wrote:
> Adds following tests:
> 
> 1. AF_XDP SKB mode
>     Generic mode XDP is driver independent, used when the driver does
>     not have support for XDP. Works on any netdevice using sockets and
>     generic XDP path. XDP hook from netif_receive_skb().
>     a. nopoll - soft-irq processing
>     b. poll - using poll() syscall
> 
> Signed-off-by: Weqaar Janjua <[email-masked]>
> ---
>   tools/testing/selftests/bpf/Makefile     |   2 +-
>   tools/testing/selftests/bpf/test_xsk.sh  |  36 +-
>   tools/testing/selftests/bpf/xdpxceiver.c | 961 +++++++++++++++++++++++
>   tools/testing/selftests/bpf/xdpxceiver.h | 151 ++++
>   tools/testing/selftests/bpf/xsk_env.sh   |  17 +
>   5 files changed, 1158 insertions(+), 9 deletions(-)
>   create mode 100644 tools/testing/selftests/bpf/xdpxceiver.c
>   create mode 100644 tools/testing/selftests/bpf/xdpxceiver.h
> 
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 596ee5c27906..a2be2725be11 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -83,7 +83,7 @@ TEST_PROGS_EXTENDED := with_addr.sh \
>   # Compile but not part of 'make run_tests'
>   TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
>   	flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
> -	test_lirc_mode2_user xdping test_cpp runqslower bench
> +	test_lirc_mode2_user xdping test_cpp runqslower bench xdpxceiver
>   
>   TEST_CUSTOM_PROGS = urandom_read
>   
[...]
> +
> +static void parse_command_line(int argc, char **argv)
> +{
> +	int option_index, interface_index = 0, c;
> +
> +	opterr = 0;
> +
> +	for (;;) {
> +		c = getopt_long(argc, argv, "i:q:pScDC:", long_options, &option_index);
> +
> +		if (c == -1)
> +			break;
> +
> +		switch (c) {
> +		case 'i':
> +			if (interface_index == MAX_INTERFACES)
> +				break;
> +			char *sptr, *token;
> +
> +			memcpy(ifdict[interface_index]->ifname,
> +			       strtok_r(optarg, ",", &sptr), MAX_INTERFACE_NAME_CHARS);

During compilation, I hit the following compiler warnings,

xdpxceiver.c: In function ‘main’:
xdpxceiver.c:461:4: warning: ‘__s’ may be used uninitialized in this 
function [-Wmaybe-uninitialized]
     memcpy(ifdict[interface_index]->ifname,
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
            strtok_r(optarg, ",", &sptr), MAX_INTERFACE_NAME_CHARS);
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
xdpxceiver.c:443:13: note: ‘__s’ was declared here
  static void parse_command_line(int argc, char **argv)
              ^~~~~~~~~~~~~~~~~~

I am using gcc8. I am not sure whether we should silence such
warning or not (-Wno-maybe-uninitialized). Did you see such a warning
during your compilation?

> +			token = strtok_r(NULL, ",", &sptr);
> +			if (token)
> +				memcpy(ifdict[interface_index]->nsname, token,
> +				       MAX_INTERFACES_NAMESPACE_CHARS);
> +			interface_index++;
> +			break;
> +		case 'q':
> +			opt_queue = atoi(optarg);
> +			break;
> +		case 'p':
> +			opt_poll = 1;
> +			break;
> +		case 'S':
> +			opt_xdp_flags |= XDP_FLAGS_SKB_MODE;
> +			opt_xdp_bind_flags |= XDP_COPY;
> +			uut = ORDER_CONTENT_VALIDATE_XDP_SKB;
> +			break;
> +		case 'c':
> +			opt_xdp_bind_flags |= XDP_COPY;
> +			break;
> +		case 'D':
> +			debug_pkt_dump = 1;
> +			break;
> +		case 'C':
> +			opt_pkt_count = atoi(optarg);
> +			break;
> +		default:
> +			usage(basename(argv[0]));
> +			ksft_exit_xfail();
> +		}
> +	}
> +
> +	if (!validate_interfaces()) {
> +		usage(basename(argv[0]));
> +		ksft_exit_xfail();
> +	}
> +}
> +
[...]