Date:   Mon, 4 Jun 2018 13:04:35 +0200
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     iovisor-dev <iovisor-dev@...ts.iovisor.org>,
        Daniel Borkmann <borkmann@...earbox.net>
Cc:     Sebastiano Miano <sebastiano.miano@...ito.it>, brouer@...hat.com,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [iovisor-dev] bpf_redirect_map not working after tail call


On Fri, 1 Jun 2018 14:15:58 +0200
Sebastiano Miano via iovisor-dev <iovisor-dev@...ts.iovisor.org> wrote:

> Dear all,
> 
> We have noticed that the bpf_redirect_map returns an error when it is
> called after a tail call.
> The xdp_redirect_map program (under sample/bpf) works fine, but if we
> modify it as shown in the following diff, it doesn't work anymore.
> I have debugged it with the xdp_monitor application and the error
> returned is EFAULT.
> Is this a known issue? Am I doing something wrong?

Argh, this is likely an issue/bug due to the check xdp_map_invalid(),
that was introduced in commit 7c3001313396 ("bpf: fix ri->map_owner
pointer on bpf_prog_realloc").

To Daniel, I don't know how to solve this, could you give some advice?



 static inline bool xdp_map_invalid(const struct bpf_prog *xdp_prog,
				   unsigned long aux)
 {
	return (unsigned long)xdp_prog->aux != aux;
 }

 static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
			       struct bpf_prog *xdp_prog)
 {
	struct redirect_info *ri = this_cpu_ptr(&redirect_info);
	unsigned long map_owner = ri->map_owner;
	struct bpf_map *map = ri->map;
	u32 index = ri->ifindex;
	void *fwd = NULL;
	int err;

	[...]
	if (unlikely(xdp_map_invalid(xdp_prog, map_owner))) {
		err = -EFAULT;
		map = NULL;
		goto err;
	}
	[...]


> P.S. I have tested the program with the latest bpf-next kernel.
> 
> ------------
> 
> diff --git a/samples/bpf/xdp_redirect_map_kern.c
> b/samples/bpf/xdp_redirect_map_kern.c
> index 740a529..bf1275a 100644
> --- a/samples/bpf/xdp_redirect_map_kern.c
> +++ b/samples/bpf/xdp_redirect_map_kern.c
> @@ -36,6 +36,13 @@ struct bpf_map_def SEC("maps") rxcnt = {
>  	.max_entries = 1,
>  };
> 
> +struct bpf_map_def SEC("maps") prog_table = {
> +	.type 		= BPF_MAP_TYPE_PROG_ARRAY,
> +	.key_size = sizeof(int),
> +	.value_size = sizeof(int),
> +	.max_entries = 32,
> +};
> +
>  static void swap_src_dst_mac(void *data)
>  {
>  	unsigned short *p = data;
> @@ -89,4 +96,15 @@ int xdp_redirect_dummy_prog(struct xdp_md *ctx)
>  	return XDP_PASS;
>  }
> 
> +/* Entry point */
> +SEC("xdp_redirect_entry_point")
> +int xdp_redirect_entry_point_prog(struct xdp_md *ctx)
> +{
> +	//char fmt[] = "xdp_redirect_entry_point\n";
> +	//bpf_trace_printk(fmt, sizeof(fmt));
> +	bpf_tail_call(ctx, &prog_table, 0);
> +	// Tail call failed
> +	return XDP_DROP;
> +}
> +
>  char _license[] SEC("license") = "GPL";
> diff --git a/samples/bpf/xdp_redirect_map_user.c
> b/samples/bpf/xdp_redirect_map_user.c
> index 4445e76..b2d2059 100644
> --- a/samples/bpf/xdp_redirect_map_user.c
> +++ b/samples/bpf/xdp_redirect_map_user.c
> @@ -120,7 +120,13 @@ int main(int argc, char **argv)
>  		return 1;
>  	}
> 
> -	if (bpf_set_link_xdp_fd(ifindex_in, prog_fd[0], xdp_flags) < 0) {
> +	ret = bpf_map_update_elem(map_fd[2], &key, &prog_fd[0], 0);
> +	if (ret) {
> +		perror("bpf_update_elem");
> +		goto out;
> +	}
> +
> +	if (bpf_set_link_xdp_fd(ifindex_in, prog_fd[2], xdp_flags) < 0) {
>  		printf("ERROR: link set xdp fd failed on %d\n", ifindex_in);
>  		return 1;
>  	}
> _______________________________________________
> iovisor-dev mailing list
> iovisor-dev@...ts.iovisor.org
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Powered by blists - more mailing lists