Re: [PATCH bpf v3 2/2] selftests/bpf: Add test for l3 use of bpf_redirect_peer

From: Daniel Borkmann <daniel@iogearbox.net>
To: Jussi Maki <joamaki@gmail.com>, bpf@vger.kernel.org
Cc: andrii.nakryiko@gmail.com
Subject: Re: [PATCH bpf v3 2/2] selftests/bpf: Add test for l3 use of bpf_redirect_peer
Date: Wed, 19 May 2021 17:33:59 +0200	[thread overview]
Message-ID: <2dc37982-9889-c2e8-9fb4-17ba26c28da9@iogearbox.net> (raw)
In-Reply-To: <20210518142356.1852779-3-joamaki@gmail.com>

On 5/18/21 4:23 PM, Jussi Maki wrote:
> Add a test case for using bpf_skb_change_head in combination with
> bpf_redirect_peer to redirect a packet from a L3 device to veth and back.
> 
> The test uses a BPF program that adds L2 headers to the packet coming
> from a L3 device and then calls bpf_redirect_peer to redirect the packet
> to a veth device. The test fails as skb->mac_len is not set properly and
> thus the ethernet headers are not properly skb_pull'd in cls_bpf_classify,
> causing tcp_v4_rcv to point the TCP header into middle of the IP header.
> 
> Signed-off-by: Jussi Maki <joamaki@gmail.com>
[...]
>   
>   /**
> - * setns_by_name() - Set networks namespace by name
> + * open_netns() - Switch to specified network namespace by name.
> + *
> + * Returns token with which to restore the original namespace
> + * using close_netns().
>    */
> -static int setns_by_name(const char *name)
> +static struct nstoken *open_netns(const char *name)
>   {
>   	int nsfd;
>   	char nspath[PATH_MAX];
>   	int err;
> +	struct nstoken *token;
> +
> +	token = malloc(sizeof(struct nstoken));
> +	if (!ASSERT_OK_PTR(token, "malloc token"))
> +		return NULL;
> +
> +	token->orig_netns_fd = open("/proc/self/ns/net", O_RDONLY);
> +	if (!ASSERT_GE(token->orig_netns_fd, 0, "open /proc/self/ns/net"))
> +		goto fail;
>   
>   	snprintf(nspath, sizeof(nspath), "%s/%s", "/var/run/netns", name);
>   	nsfd = open(nspath, O_RDONLY | O_CLOEXEC);
> -	if (nsfd < 0)
> -		return nsfd;
> +	if (!ASSERT_GE(nsfd, 0, "open netns fd"))
> +		goto fail;
>   
> -	err = setns(nsfd, CLONE_NEWNET);
> -	close(nsfd);
> +	err = setns_by_fd(nsfd);
> +	if (!ASSERT_OK(err, "setns_by_fd"))
> +		goto fail;
>   
> -	return err;
> +	return token;
> +fail:
> +	free(token);
> +	return NULL;
>   }

As discussed earlier, the selftest seems to be causing issues in the bpf CI [0] likely
due to the setns() interaction/cleanup. Pls investigate and resubmit once fixed. Thanks
a lot, Jussi!

Cheers,
Daniel

   [0] https://travis-ci.com/github/kernel-patches/bpf/builds/226213040