[PATCH net-next] samples/bpf: Add support for SKB_MODE to xdp1 and xdp_tx_iptunnel

[PATCH net-next] samples/bpf: Add support for SKB_MODE to xdp1 and xdp_tx_iptunnel

[David Ahern]

Add option to xdp1 and xdp_tx_iptunnel to insert xdp program in
SKB_MODE:
 - update set_link_xdp_fd to take a flags argument that is added to the
   RTM_SETLINK message

 - Add -S option to xdp1 and xdp_tx_iptunnel user code. When passed in
   XDP_FLAGS_SKB_MODE is set in the flags arg passed to set_link_xdp_fd

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 samples/bpf/bpf_load.c             | 19 +++++++++++++++---
 samples/bpf/bpf_load.h             |  2 +-
 samples/bpf/xdp1_user.c            | 40 ++++++++++++++++++++++++++++++--------
 samples/bpf/xdp_tx_iptunnel_user.c | 13 +++++++++----
 4 files changed, 58 insertions(+), 16 deletions(-)

diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
index 0d449d8032d1..d4433a47e6c3 100644
--- a/samples/bpf/bpf_load.c
+++ b/samples/bpf/bpf_load.c
@@ -563,7 +563,7 @@ struct ksym *ksym_search(long key)
 	return &syms[0];
 }
 
-int set_link_xdp_fd(int ifindex, int fd)
+int set_link_xdp_fd(int ifindex, int fd, int flags)
 {
 	struct sockaddr_nl sa;
 	int sock, seq = 0, len, ret = -1;
@@ -599,15 +599,28 @@ int set_link_xdp_fd(int ifindex, int fd)
 	req.nh.nlmsg_seq = ++seq;
 	req.ifinfo.ifi_family = AF_UNSPEC;
 	req.ifinfo.ifi_index = ifindex;
+
+	/* started nested attribute for XDP */
 	nla = (struct nlattr *)(((char *)&req)
 				+ NLMSG_ALIGN(req.nh.nlmsg_len));
 	nla->nla_type = NLA_F_NESTED | 43/*IFLA_XDP*/;
+	nla->nla_len = NLA_HDRLEN;
 
-	nla_xdp = (struct nlattr *)((char *)nla + NLA_HDRLEN);
+	/* add XDP fd */
+	nla_xdp = (struct nlattr *)((char *)nla + nla->nla_len);
 	nla_xdp->nla_type = 1/*IFLA_XDP_FD*/;
 	nla_xdp->nla_len = NLA_HDRLEN + sizeof(int);
 	memcpy((char *)nla_xdp + NLA_HDRLEN, &fd, sizeof(fd));
-	nla->nla_len = NLA_HDRLEN + nla_xdp->nla_len;
+	nla->nla_len += nla_xdp->nla_len;
+
+	/* if user passed in any flags, add those too */
+	if (flags) {
+		nla_xdp = (struct nlattr *)((char *)nla + nla->nla_len);
+		nla_xdp->nla_type = 3/*IFLA_XDP_FLAGS*/;
+		nla_xdp->nla_len = NLA_HDRLEN + sizeof(flags);
+		memcpy((char *)nla_xdp + NLA_HDRLEN, &flags, sizeof(flags));
+		nla->nla_len += nla_xdp->nla_len;
+	}
 
 	req.nh.nlmsg_len += NLA_ALIGN(nla->nla_len);
 
diff --git a/samples/bpf/bpf_load.h b/samples/bpf/bpf_load.h
index 68f6b2d22507..6bfd75ec6a16 100644
--- a/samples/bpf/bpf_load.h
+++ b/samples/bpf/bpf_load.h
@@ -47,5 +47,5 @@ struct ksym {
 
 int load_kallsyms(void);
 struct ksym *ksym_search(long key);
-int set_link_xdp_fd(int ifindex, int fd);
+int set_link_xdp_fd(int ifindex, int fd, int flags);
 #endif
diff --git a/samples/bpf/xdp1_user.c b/samples/bpf/xdp1_user.c
index d2be65d1fd86..deb05e630d84 100644
--- a/samples/bpf/xdp1_user.c
+++ b/samples/bpf/xdp1_user.c
@@ -5,6 +5,7 @@
  * License as published by the Free Software Foundation.
  */
 #include <linux/bpf.h>
+#include <linux/if_link.h>
 #include <assert.h>
 #include <errno.h>
 #include <signal.h>
@@ -12,16 +13,18 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <libgen.h>
 
 #include "bpf_load.h"
 #include "bpf_util.h"
 #include "libbpf.h"
 
 static int ifindex;
+static int flags;
 
 static void int_exit(int sig)
 {
-	set_link_xdp_fd(ifindex, -1);
+	set_link_xdp_fd(ifindex, -1, flags);
 	exit(0);
 }
 
@@ -54,18 +57,39 @@ static void poll_stats(int interval)
 	}
 }
 
-int main(int ac, char **argv)
+static void usage(const char *prog)
 {
-	char filename[256];
+	fprintf(stderr,
+		"usage: %s [OPTS] IFINDEX\n\n"
+		"OPTS:\n"
+		"    -S    use skb-mode\n",
+		prog);
+}
 
-	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+int main(int argc, char **argv)
+{
+	const char *optstr = "S";
+	char filename[256];
+	int opt;
+
+	while ((opt = getopt(argc, argv, optstr)) != -1) {
+		switch (opt) {
+		case 'S':
+			flags |= XDP_FLAGS_SKB_MODE;
+			break;
+		default:
+			usage(basename(argv[0]));
+			return 1;
+		}
+	}
 
-	if (ac != 2) {
-		printf("usage: %s IFINDEX\n", argv[0]);
+	if (optind == argc) {
+		usage(basename(argv[0]));
 		return 1;
 	}
+	ifindex = strtoul(argv[optind], NULL, 0);
 
-	ifindex = strtoul(argv[1], NULL, 0);
+	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
 
 	if (load_bpf_file(filename)) {
 		printf("%s", bpf_log_buf);
@@ -79,7 +103,7 @@ int main(int ac, char **argv)
 
 	signal(SIGINT, int_exit);
 
-	if (set_link_xdp_fd(ifindex, prog_fd[0]) < 0) {
+	if (set_link_xdp_fd(ifindex, prog_fd[0], flags) < 0) {
 		printf("link set xdp fd failed\n");
 		return 1;
 	}
diff --git a/samples/bpf/xdp_tx_iptunnel_user.c b/samples/bpf/xdp_tx_iptunnel_user.c
index 70e192fc61aa..cb2bda7b5346 100644
--- a/samples/bpf/xdp_tx_iptunnel_user.c
+++ b/samples/bpf/xdp_tx_iptunnel_user.c
@@ -5,6 +5,7 @@
  * License as published by the Free Software Foundation.
  */
 #include <linux/bpf.h>
+#include <linux/if_link.h>
 #include <assert.h>
 #include <errno.h>
 #include <signal.h>
@@ -28,7 +29,7 @@ static int ifindex = -1;
 static void int_exit(int sig)
 {
 	if (ifindex > -1)
-		set_link_xdp_fd(ifindex, -1);
+		set_link_xdp_fd(ifindex, -1, 0);
 	exit(0);
 }
 
@@ -136,12 +137,13 @@ int main(int argc, char **argv)
 {
 	unsigned char opt_flags[256] = {};
 	unsigned int kill_after_s = 0;
-	const char *optstr = "i:a:p:s:d:m:T:P:h";
+	const char *optstr = "i:a:p:s:d:m:T:P:Sh";
 	int min_port = 0, max_port = 0;
 	struct iptnl_info tnl = {};
 	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	struct vip vip = {};
 	char filename[256];
+	int flags = 0;
 	int opt;
 	int i;
 
@@ -201,6 +203,9 @@ int main(int argc, char **argv)
 		case 'T':
 			kill_after_s = atoi(optarg);
 			break;
+		case 'S':
+			flags |= XDP_FLAGS_SKB_MODE;
+			break;
 		default:
 			usage(argv[0]);
 			return 1;
@@ -243,14 +248,14 @@ int main(int argc, char **argv)
 		}
 	}
 
-	if (set_link_xdp_fd(ifindex, prog_fd[0]) < 0) {
+	if (set_link_xdp_fd(ifindex, prog_fd[0], flags) < 0) {
 		printf("link set xdp fd failed\n");
 		return 1;
 	}
 
 	poll_stats(kill_after_s);
 
-	set_link_xdp_fd(ifindex, -1);
+	set_link_xdp_fd(ifindex, -1, flags);
 
 	return 0;
 }
-- 
2.1.4

Re: [PATCH net-next] samples/bpf: Add support for SKB_MODE to xdp1 and xdp_tx_iptunnel

[Alexei Starovoitov]

On 4/27/17 9:11 AM, David Ahern wrote:
> Add option to xdp1 and xdp_tx_iptunnel to insert xdp program in
> SKB_MODE:
>  - update set_link_xdp_fd to take a flags argument that is added to the
>    RTM_SETLINK message
>
>  - Add -S option to xdp1 and xdp_tx_iptunnel user code. When passed in
>    XDP_FLAGS_SKB_MODE is set in the flags arg passed to set_link_xdp_fd
>
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>

awesome. thanks!
Acked-by: Alexei Starovoitov <ast@kernel.org>

Re: [PATCH net-next] samples/bpf: Add support for SKB_MODE to xdp1 and xdp_tx_iptunnel

[David Miller]

From: Alexei Starovoitov <ast@fb.com>
Date: Thu, 27 Apr 2017 09:38:37 -0700

> On 4/27/17 9:11 AM, David Ahern wrote:
>> Add option to xdp1 and xdp_tx_iptunnel to insert xdp program in
>> SKB_MODE:
>>  - update set_link_xdp_fd to take a flags argument that is added to the
>>    RTM_SETLINK message
>>
>>  - Add -S option to xdp1 and xdp_tx_iptunnel user code. When passed in
>>    XDP_FLAGS_SKB_MODE is set in the flags arg passed to set_link_xdp_fd
>>
>> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> 
> awesome. thanks!
> Acked-by: Alexei Starovoitov <ast@kernel.org>

Indeed, very awesome.

Applied!

Re: [PATCH net-next] samples/bpf: Add support for SKB_MODE to xdp1 and xdp_tx_iptunnel

[Jesper Dangaard Brouer]

On Thu, 27 Apr 2017 09:11:13 -0700
David Ahern <dsa@cumulusnetworks.com> wrote:

> Add option to xdp1 and xdp_tx_iptunnel to insert xdp program in
> SKB_MODE:
>  - update set_link_xdp_fd to take a flags argument that is added to the
>    RTM_SETLINK message
> 
>  - Add -S option to xdp1 and xdp_tx_iptunnel user code. When passed in
>    XDP_FLAGS_SKB_MODE is set in the flags arg passed to set_link_xdp_fd
> 
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>

Thanks, this will make is a lot easier to measure the overhead of the
network stack compared to XDP, like I did here:
 http://prototype-kernel.readthedocs.io/en/latest/blogposts/xdp25_eval_generic_xdp_tx.html

[...]
> diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
> index 0d449d8032d1..d4433a47e6c3 100644
> --- a/samples/bpf/bpf_load.c
> +++ b/samples/bpf/bpf_load.c
> @@ -563,7 +563,7 @@ struct ksym *ksym_search(long key)
>  	return &syms[0];
>  }
>  
> -int set_link_xdp_fd(int ifindex, int fd)
> +int set_link_xdp_fd(int ifindex, int fd, int flags)

Shouldn't the flags be a unsigned int, actually a __u32 ?

>  {
>  	struct sockaddr_nl sa;
>  	int sock, seq = 0, len, ret = -1;
> @@ -599,15 +599,28 @@ int set_link_xdp_fd(int ifindex, int fd)
>  	req.nh.nlmsg_seq = ++seq;
>  	req.ifinfo.ifi_family = AF_UNSPEC;
>  	req.ifinfo.ifi_index = ifindex;
> +
> +	/* started nested attribute for XDP */
>  	nla = (struct nlattr *)(((char *)&req)
>  				+ NLMSG_ALIGN(req.nh.nlmsg_len));
>  	nla->nla_type = NLA_F_NESTED | 43/*IFLA_XDP*/;
> +	nla->nla_len = NLA_HDRLEN;
>  
> -	nla_xdp = (struct nlattr *)((char *)nla + NLA_HDRLEN);
> +	/* add XDP fd */
> +	nla_xdp = (struct nlattr *)((char *)nla + nla->nla_len);
>  	nla_xdp->nla_type = 1/*IFLA_XDP_FD*/;
>  	nla_xdp->nla_len = NLA_HDRLEN + sizeof(int);
>  	memcpy((char *)nla_xdp + NLA_HDRLEN, &fd, sizeof(fd));
> -	nla->nla_len = NLA_HDRLEN + nla_xdp->nla_len;
> +	nla->nla_len += nla_xdp->nla_len;
> +
> +	/* if user passed in any flags, add those too */
> +	if (flags) {
> +		nla_xdp = (struct nlattr *)((char *)nla + nla->nla_len);
> +		nla_xdp->nla_type = 3/*IFLA_XDP_FLAGS*/;
> +		nla_xdp->nla_len = NLA_HDRLEN + sizeof(flags);
> +		memcpy((char *)nla_xdp + NLA_HDRLEN, &flags, sizeof(flags));
> +		nla->nla_len += nla_xdp->nla_len;
> +	}
>  

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

Re: [PATCH net-next] samples/bpf: Add support for SKB_MODE to xdp1 and xdp_tx_iptunnel

[David Ahern]

On 4/28/17 3:40 PM, Jesper Dangaard Brouer wrote:
> [...]
>> diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
>> index 0d449d8032d1..d4433a47e6c3 100644
>> --- a/samples/bpf/bpf_load.c
>> +++ b/samples/bpf/bpf_load.c
>> @@ -563,7 +563,7 @@ struct ksym *ksym_search(long key)
>>  	return &syms[0];
>>  }
>>  
>> -int set_link_xdp_fd(int ifindex, int fd)
>> +int set_link_xdp_fd(int ifindex, int fd, int flags)
> Shouldn't the flags be a unsigned int, actually a __u32 ?
> 

sure. I'll send a patch

Re: [PATCH net-next] samples/bpf: Add support for SKB_MODE to xdp1 and xdp_tx_iptunnel

[Jesper Dangaard Brouer]

On Sun, 30 Apr 2017 17:46:13 -0600
David Ahern <dsa@cumulusnetworks.com> wrote:

> On 4/28/17 3:40 PM, Jesper Dangaard Brouer wrote:
> > [...]  
> >> diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
> >> index 0d449d8032d1..d4433a47e6c3 100644
> >> --- a/samples/bpf/bpf_load.c
> >> +++ b/samples/bpf/bpf_load.c
> >> @@ -563,7 +563,7 @@ struct ksym *ksym_search(long key)
> >>  	return &syms[0];
> >>  }
> >>  
> >> -int set_link_xdp_fd(int ifindex, int fd)
> >> +int set_link_xdp_fd(int ifindex, int fd, int flags)  
> > Shouldn't the flags be a unsigned int, actually a __u32 ?
> >   
> 
> sure. I'll send a patch

I found another bug in xdp_tx_iptunnel ... I'll send a patch for both issues.

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

[net-next PATCH 0/2] samples/bpf: two bug fixes to XDP_FLAGS_SKB_MODE attaching

[Jesper Dangaard Brouer]

Two small bugfixes for:
 commit 3993f2cb983b ("samples/bpf: Add support for SKB_MODE to xdp1 and xdp_tx_iptunnel")

---

Jesper Dangaard Brouer (2):
      samples/bpf: fix SKB_MODE flag to be a 32-bit unsigned int
      samples/bpf: fix XDP_FLAGS_SKB_MODE detach for xdp_tx_iptunnel


 samples/bpf/bpf_load.c             |    3 ++-
 samples/bpf/bpf_load.h             |    2 +-
 samples/bpf/xdp1_user.c            |    8 ++++----
 samples/bpf/xdp_tx_iptunnel_user.c |   10 +++++-----
 4 files changed, 12 insertions(+), 11 deletions(-)

[net-next PATCH 1/2] samples/bpf: fix SKB_MODE flag to be a 32-bit unsigned int

[Jesper Dangaard Brouer]

The kernel side of XDP_FLAGS_SKB_MODE is unsigned, and the rtnetlink
IFLA_XDP_FLAGS is defined as NLA_U32. Thus, userspace programs under
samples/bpf/ should use the correct type.

Fixes: 3993f2cb983b ("samples/bpf: Add support for SKB_MODE to xdp1 and xdp_tx_iptunnel")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 samples/bpf/bpf_load.c             |    3 ++-
 samples/bpf/bpf_load.h             |    2 +-
 samples/bpf/xdp1_user.c            |    8 ++++----
 samples/bpf/xdp_tx_iptunnel_user.c |    8 ++++----
 4 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
index 0ec0dea3c41e..4221dc359453 100644
--- a/samples/bpf/bpf_load.c
+++ b/samples/bpf/bpf_load.c
@@ -14,6 +14,7 @@
 #include <linux/perf_event.h>
 #include <linux/netlink.h>
 #include <linux/rtnetlink.h>
+#include <linux/types.h>
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/syscall.h>
@@ -585,7 +586,7 @@ struct ksym *ksym_search(long key)
 	return &syms[0];
 }
 
-int set_link_xdp_fd(int ifindex, int fd, int flags)
+int set_link_xdp_fd(int ifindex, int fd, __u32 flags)
 {
 	struct sockaddr_nl sa;
 	int sock, seq = 0, len, ret = -1;
diff --git a/samples/bpf/bpf_load.h b/samples/bpf/bpf_load.h
index 6bfd75ec6a16..05822f83173a 100644
--- a/samples/bpf/bpf_load.h
+++ b/samples/bpf/bpf_load.h
@@ -47,5 +47,5 @@ struct ksym {
 
 int load_kallsyms(void);
 struct ksym *ksym_search(long key);
-int set_link_xdp_fd(int ifindex, int fd, int flags);
+int set_link_xdp_fd(int ifindex, int fd, __u32 flags);
 #endif
diff --git a/samples/bpf/xdp1_user.c b/samples/bpf/xdp1_user.c
index deb05e630d84..378850c70eb8 100644
--- a/samples/bpf/xdp1_user.c
+++ b/samples/bpf/xdp1_user.c
@@ -20,11 +20,11 @@
 #include "libbpf.h"
 
 static int ifindex;
-static int flags;
+static __u32 xdp_flags;
 
 static void int_exit(int sig)
 {
-	set_link_xdp_fd(ifindex, -1, flags);
+	set_link_xdp_fd(ifindex, -1, xdp_flags);
 	exit(0);
 }
 
@@ -75,7 +75,7 @@ int main(int argc, char **argv)
 	while ((opt = getopt(argc, argv, optstr)) != -1) {
 		switch (opt) {
 		case 'S':
-			flags |= XDP_FLAGS_SKB_MODE;
+			xdp_flags |= XDP_FLAGS_SKB_MODE;
 			break;
 		default:
 			usage(basename(argv[0]));
@@ -103,7 +103,7 @@ int main(int argc, char **argv)
 
 	signal(SIGINT, int_exit);
 
-	if (set_link_xdp_fd(ifindex, prog_fd[0], flags) < 0) {
+	if (set_link_xdp_fd(ifindex, prog_fd[0], xdp_flags) < 0) {
 		printf("link set xdp fd failed\n");
 		return 1;
 	}
diff --git a/samples/bpf/xdp_tx_iptunnel_user.c b/samples/bpf/xdp_tx_iptunnel_user.c
index cb2bda7b5346..880dd4aebfa4 100644
--- a/samples/bpf/xdp_tx_iptunnel_user.c
+++ b/samples/bpf/xdp_tx_iptunnel_user.c
@@ -142,8 +142,8 @@ int main(int argc, char **argv)
 	struct iptnl_info tnl = {};
 	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	struct vip vip = {};
+	__u32 xdp_flags = 0;
 	char filename[256];
-	int flags = 0;
 	int opt;
 	int i;
 
@@ -204,7 +204,7 @@ int main(int argc, char **argv)
 			kill_after_s = atoi(optarg);
 			break;
 		case 'S':
-			flags |= XDP_FLAGS_SKB_MODE;
+			xdp_flags |= XDP_FLAGS_SKB_MODE;
 			break;
 		default:
 			usage(argv[0]);
@@ -248,14 +248,14 @@ int main(int argc, char **argv)
 		}
 	}
 
-	if (set_link_xdp_fd(ifindex, prog_fd[0], flags) < 0) {
+	if (set_link_xdp_fd(ifindex, prog_fd[0], xdp_flags) < 0) {
 		printf("link set xdp fd failed\n");
 		return 1;
 	}
 
 	poll_stats(kill_after_s);
 
-	set_link_xdp_fd(ifindex, -1, flags);
+	set_link_xdp_fd(ifindex, -1, xdp_flags);
 
 	return 0;
 }

[net-next PATCH 2/2] samples/bpf: fix XDP_FLAGS_SKB_MODE detach for xdp_tx_iptunnel

[Jesper Dangaard Brouer]

The xdp_tx_iptunnel program can be terminated in two ways, after
N-seconds or via Ctrl-C SIGINT.  The SIGINT code path does not
handle detatching the correct XDP program, in-case the program
was attached with XDP_FLAGS_SKB_MODE.

Fix this by storing the XDP flags as a global variable, which is
available for the SIGINT handler function.

Fixes: 3993f2cb983b ("samples/bpf: Add support for SKB_MODE to xdp1 and xdp_tx_iptunnel")
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 samples/bpf/xdp_tx_iptunnel_user.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/samples/bpf/xdp_tx_iptunnel_user.c b/samples/bpf/xdp_tx_iptunnel_user.c
index 880dd4aebfa4..92b8bde9337c 100644
--- a/samples/bpf/xdp_tx_iptunnel_user.c
+++ b/samples/bpf/xdp_tx_iptunnel_user.c
@@ -25,11 +25,12 @@
 #define STATS_INTERVAL_S 2U
 
 static int ifindex = -1;
+static __u32 xdp_flags = 0;
 
 static void int_exit(int sig)
 {
 	if (ifindex > -1)
-		set_link_xdp_fd(ifindex, -1, 0);
+		set_link_xdp_fd(ifindex, -1, xdp_flags);
 	exit(0);
 }
 
@@ -142,7 +143,6 @@ int main(int argc, char **argv)
 	struct iptnl_info tnl = {};
 	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	struct vip vip = {};
-	__u32 xdp_flags = 0;
 	char filename[256];
 	int opt;
 	int i;

Re: [net-next PATCH 1/2] samples/bpf: fix SKB_MODE flag to be a 32-bit unsigned int

[Daniel Borkmann]

On 05/01/2017 11:26 AM, Jesper Dangaard Brouer wrote:
> The kernel side of XDP_FLAGS_SKB_MODE is unsigned, and the rtnetlink
> IFLA_XDP_FLAGS is defined as NLA_U32. Thus, userspace programs under
> samples/bpf/ should use the correct type.
>
> Fixes: 3993f2cb983b ("samples/bpf: Add support for SKB_MODE to xdp1 and xdp_tx_iptunnel")
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

Re: [net-next PATCH 2/2] samples/bpf: fix XDP_FLAGS_SKB_MODE detach for xdp_tx_iptunnel

[Daniel Borkmann]

On 05/01/2017 11:26 AM, Jesper Dangaard Brouer wrote:
> The xdp_tx_iptunnel program can be terminated in two ways, after
> N-seconds or via Ctrl-C SIGINT.  The SIGINT code path does not
> handle detatching the correct XDP program, in-case the program
> was attached with XDP_FLAGS_SKB_MODE.
>
> Fix this by storing the XDP flags as a global variable, which is
> available for the SIGINT handler function.
>
> Fixes: 3993f2cb983b ("samples/bpf: Add support for SKB_MODE to xdp1 and xdp_tx_iptunnel")
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

Re: [net-next PATCH 1/2] samples/bpf: fix SKB_MODE flag to be a 32-bit unsigned int

[Andy Gospodarek]

On Mon, May 01, 2017 at 11:26:15AM +0200, Jesper Dangaard Brouer wrote:
> The kernel side of XDP_FLAGS_SKB_MODE is unsigned, and the rtnetlink
> IFLA_XDP_FLAGS is defined as NLA_U32. Thus, userspace programs under
> samples/bpf/ should use the correct type.
> 
> Fixes: 3993f2cb983b ("samples/bpf: Add support for SKB_MODE to xdp1 and xdp_tx_iptunnel")
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>

Reviewed-by: Andy Gospodarek <andy@greyhouse.net>

> ---
>  samples/bpf/bpf_load.c             |    3 ++-
>  samples/bpf/bpf_load.h             |    2 +-
>  samples/bpf/xdp1_user.c            |    8 ++++----
>  samples/bpf/xdp_tx_iptunnel_user.c |    8 ++++----
>  4 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
> index 0ec0dea3c41e..4221dc359453 100644
> --- a/samples/bpf/bpf_load.c
> +++ b/samples/bpf/bpf_load.c
> @@ -14,6 +14,7 @@
>  #include <linux/perf_event.h>
>  #include <linux/netlink.h>
>  #include <linux/rtnetlink.h>
> +#include <linux/types.h>
>  #include <sys/types.h>
>  #include <sys/socket.h>
>  #include <sys/syscall.h>
> @@ -585,7 +586,7 @@ struct ksym *ksym_search(long key)
>  	return &syms[0];
>  }
>  
> -int set_link_xdp_fd(int ifindex, int fd, int flags)
> +int set_link_xdp_fd(int ifindex, int fd, __u32 flags)
>  {
>  	struct sockaddr_nl sa;
>  	int sock, seq = 0, len, ret = -1;
> diff --git a/samples/bpf/bpf_load.h b/samples/bpf/bpf_load.h
> index 6bfd75ec6a16..05822f83173a 100644
> --- a/samples/bpf/bpf_load.h
> +++ b/samples/bpf/bpf_load.h
> @@ -47,5 +47,5 @@ struct ksym {
>  
>  int load_kallsyms(void);
>  struct ksym *ksym_search(long key);
> -int set_link_xdp_fd(int ifindex, int fd, int flags);
> +int set_link_xdp_fd(int ifindex, int fd, __u32 flags);
>  #endif
> diff --git a/samples/bpf/xdp1_user.c b/samples/bpf/xdp1_user.c
> index deb05e630d84..378850c70eb8 100644
> --- a/samples/bpf/xdp1_user.c
> +++ b/samples/bpf/xdp1_user.c
> @@ -20,11 +20,11 @@
>  #include "libbpf.h"
>  
>  static int ifindex;
> -static int flags;
> +static __u32 xdp_flags;
>  
>  static void int_exit(int sig)
>  {
> -	set_link_xdp_fd(ifindex, -1, flags);
> +	set_link_xdp_fd(ifindex, -1, xdp_flags);
>  	exit(0);
>  }
>  
> @@ -75,7 +75,7 @@ int main(int argc, char **argv)
>  	while ((opt = getopt(argc, argv, optstr)) != -1) {
>  		switch (opt) {
>  		case 'S':
> -			flags |= XDP_FLAGS_SKB_MODE;
> +			xdp_flags |= XDP_FLAGS_SKB_MODE;
>  			break;
>  		default:
>  			usage(basename(argv[0]));
> @@ -103,7 +103,7 @@ int main(int argc, char **argv)
>  
>  	signal(SIGINT, int_exit);
>  
> -	if (set_link_xdp_fd(ifindex, prog_fd[0], flags) < 0) {
> +	if (set_link_xdp_fd(ifindex, prog_fd[0], xdp_flags) < 0) {
>  		printf("link set xdp fd failed\n");
>  		return 1;
>  	}
> diff --git a/samples/bpf/xdp_tx_iptunnel_user.c b/samples/bpf/xdp_tx_iptunnel_user.c
> index cb2bda7b5346..880dd4aebfa4 100644
> --- a/samples/bpf/xdp_tx_iptunnel_user.c
> +++ b/samples/bpf/xdp_tx_iptunnel_user.c
> @@ -142,8 +142,8 @@ int main(int argc, char **argv)
>  	struct iptnl_info tnl = {};
>  	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
>  	struct vip vip = {};
> +	__u32 xdp_flags = 0;
>  	char filename[256];
> -	int flags = 0;
>  	int opt;
>  	int i;
>  
> @@ -204,7 +204,7 @@ int main(int argc, char **argv)
>  			kill_after_s = atoi(optarg);
>  			break;
>  		case 'S':
> -			flags |= XDP_FLAGS_SKB_MODE;
> +			xdp_flags |= XDP_FLAGS_SKB_MODE;
>  			break;
>  		default:
>  			usage(argv[0]);
> @@ -248,14 +248,14 @@ int main(int argc, char **argv)
>  		}
>  	}
>  
> -	if (set_link_xdp_fd(ifindex, prog_fd[0], flags) < 0) {
> +	if (set_link_xdp_fd(ifindex, prog_fd[0], xdp_flags) < 0) {
>  		printf("link set xdp fd failed\n");
>  		return 1;
>  	}
>  
>  	poll_stats(kill_after_s);
>  
> -	set_link_xdp_fd(ifindex, -1, flags);
> +	set_link_xdp_fd(ifindex, -1, xdp_flags);
>  
>  	return 0;
>  }

Re: [net-next PATCH 2/2] samples/bpf: fix XDP_FLAGS_SKB_MODE detach for xdp_tx_iptunnel

[Andy Gospodarek]

On Mon, May 01, 2017 at 11:26:20AM +0200, Jesper Dangaard Brouer wrote:
> The xdp_tx_iptunnel program can be terminated in two ways, after
> N-seconds or via Ctrl-C SIGINT.  The SIGINT code path does not
> handle detatching the correct XDP program, in-case the program
> was attached with XDP_FLAGS_SKB_MODE.
> 
> Fix this by storing the XDP flags as a global variable, which is
> available for the SIGINT handler function.
> 
> Fixes: 3993f2cb983b ("samples/bpf: Add support for SKB_MODE to xdp1 and xdp_tx_iptunnel")
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>

Reviewed-by: Andy Gospodarek <andy@greyhouse.net>

> ---
>  samples/bpf/xdp_tx_iptunnel_user.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/samples/bpf/xdp_tx_iptunnel_user.c b/samples/bpf/xdp_tx_iptunnel_user.c
> index 880dd4aebfa4..92b8bde9337c 100644
> --- a/samples/bpf/xdp_tx_iptunnel_user.c
> +++ b/samples/bpf/xdp_tx_iptunnel_user.c
> @@ -25,11 +25,12 @@
>  #define STATS_INTERVAL_S 2U
>  
>  static int ifindex = -1;
> +static __u32 xdp_flags = 0;
>  
>  static void int_exit(int sig)
>  {
>  	if (ifindex > -1)
> -		set_link_xdp_fd(ifindex, -1, 0);
> +		set_link_xdp_fd(ifindex, -1, xdp_flags);
>  	exit(0);
>  }
>  
> @@ -142,7 +143,6 @@ int main(int argc, char **argv)
>  	struct iptnl_info tnl = {};
>  	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
>  	struct vip vip = {};
> -	__u32 xdp_flags = 0;
>  	char filename[256];
>  	int opt;
>  	int i;

Re: [net-next PATCH 0/2] samples/bpf: two bug fixes to XDP_FLAGS_SKB_MODE attaching

[David Miller]

From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Mon, 01 May 2017 11:26:10 +0200

> Two small bugfixes for:
>  commit 3993f2cb983b ("samples/bpf: Add support for SKB_MODE to xdp1 and xdp_tx_iptunnel")

Series applied.