All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/2] bpf: net: Detach BPF prog from reuseport sk
@ 2019-06-12 19:05 Martin KaFai Lau
  2019-06-12 19:05 ` [PATCH bpf-next 1/2] bpf: net: Add SO_DETACH_REUSEPORT_BPF Martin KaFai Lau
  2019-06-12 19:05 ` [PATCH bpf-next 2/2] bpf: Add test for SO_REUSEPORT_DETACH_BPF Martin KaFai Lau
  0 siblings, 2 replies; 15+ messages in thread
From: Martin KaFai Lau @ 2019-06-12 19:05 UTC (permalink / raw)
  To: bpf, netdev
  Cc: Alexei Starovoitov, Daniel Borkmann, David Miller, kernel-team

This patch adds SO_DETACH_REUSEPORT_BPF to detach BPF prog from
reuseport sk.

Martin KaFai Lau (2):
  bpf: net: Add SO_DETACH_REUSEPORT_BPF
  bpf: Add test for SO_REUSEPORT_DETACH_BPF

 arch/alpha/include/uapi/asm/socket.h          |  2 +
 arch/mips/include/uapi/asm/socket.h           |  2 +
 arch/parisc/include/uapi/asm/socket.h         |  2 +
 arch/sparc/include/uapi/asm/socket.h          |  2 +
 include/net/sock_reuseport.h                  |  2 +
 include/uapi/asm-generic/socket.h             |  2 +
 net/core/sock.c                               |  4 ++
 net/core/sock_reuseport.c                     | 24 +++++++++
 tools/testing/selftests/bpf/Makefile          |  1 +
 .../selftests/bpf/test_select_reuseport.c     | 50 +++++++++++++++++++
 10 files changed, 91 insertions(+)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH bpf-next 1/2] bpf: net: Add SO_DETACH_REUSEPORT_BPF
  2019-06-12 19:05 [PATCH bpf-next 0/2] bpf: net: Detach BPF prog from reuseport sk Martin KaFai Lau
@ 2019-06-12 19:05 ` Martin KaFai Lau
  2019-06-13  4:39   ` Andrii Nakryiko
                     ` (2 more replies)
  2019-06-12 19:05 ` [PATCH bpf-next 2/2] bpf: Add test for SO_REUSEPORT_DETACH_BPF Martin KaFai Lau
  1 sibling, 3 replies; 15+ messages in thread
From: Martin KaFai Lau @ 2019-06-12 19:05 UTC (permalink / raw)
  To: bpf, netdev
  Cc: Alexei Starovoitov, Daniel Borkmann, David Miller, kernel-team,
	Craig Gallek

There is SO_ATTACH_REUSEPORT_[CE]BPF but there is no DETACH.
This patch adds SO_DETACH_REUSEPORT_BPF sockopt.  The same
sockopt can be used to undo both SO_ATTACH_REUSEPORT_[CE]BPF.

reseport_detach_prog() is added and it is mostly a mirror
of the existing reuseport_attach_prog().  The differences are,
it does not call reuseport_alloc() and returns -ENOENT when
there is no old prog.

Cc: Craig Gallek <kraig@google.com>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 arch/alpha/include/uapi/asm/socket.h  |  2 ++
 arch/mips/include/uapi/asm/socket.h   |  2 ++
 arch/parisc/include/uapi/asm/socket.h |  2 ++
 arch/sparc/include/uapi/asm/socket.h  |  2 ++
 include/net/sock_reuseport.h          |  2 ++
 include/uapi/asm-generic/socket.h     |  2 ++
 net/core/sock.c                       |  4 ++++
 net/core/sock_reuseport.c             | 24 ++++++++++++++++++++++++
 8 files changed, 40 insertions(+)

diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
index 976e89b116e5..de6c4df61082 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -122,6 +122,8 @@
 #define SO_RCVTIMEO_NEW         66
 #define SO_SNDTIMEO_NEW         67
 
+#define SO_DETACH_REUSEPORT_BPF 68
+
 #if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64
diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
index d41765cfbc6e..d0a9ed2ca2d6 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -133,6 +133,8 @@
 #define SO_RCVTIMEO_NEW         66
 #define SO_SNDTIMEO_NEW         67
 
+#define SO_DETACH_REUSEPORT_BPF 68
+
 #if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64
diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
index 66c5dd245ac7..10173c32195e 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -114,6 +114,8 @@
 #define SO_RCVTIMEO_NEW         0x4040
 #define SO_SNDTIMEO_NEW         0x4041
 
+#define SO_DETACH_REUSEPORT_BPF 0x4042
+
 #if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64
diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
index 9265a9eece15..1895ac112a24 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -115,6 +115,8 @@
 #define SO_RCVTIMEO_NEW          0x0044
 #define SO_SNDTIMEO_NEW          0x0045
 
+#define SO_DETACH_REUSEPORT_BPF  0x0046
+
 #if !defined(__KERNEL__)
 
 
diff --git a/include/net/sock_reuseport.h b/include/net/sock_reuseport.h
index 8a5f70c7cdf2..d9112de85261 100644
--- a/include/net/sock_reuseport.h
+++ b/include/net/sock_reuseport.h
@@ -35,6 +35,8 @@ extern struct sock *reuseport_select_sock(struct sock *sk,
 					  struct sk_buff *skb,
 					  int hdr_len);
 extern int reuseport_attach_prog(struct sock *sk, struct bpf_prog *prog);
+extern int reuseport_detach_prog(struct sock *sk);
+
 int reuseport_get_id(struct sock_reuseport *reuse);
 
 #endif  /* _SOCK_REUSEPORT_H */
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index 8c1391c89171..77f7c1638eb1 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -117,6 +117,8 @@
 #define SO_RCVTIMEO_NEW         66
 #define SO_SNDTIMEO_NEW         67
 
+#define SO_DETACH_REUSEPORT_BPF 68
+
 #if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
diff --git a/net/core/sock.c b/net/core/sock.c
index 75b1c950b49f..06be30737b69 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1045,6 +1045,10 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 		}
 		break;
 
+	case SO_DETACH_REUSEPORT_BPF:
+		ret = reuseport_detach_prog(sk);
+		break;
+
 	case SO_DETACH_FILTER:
 		ret = sk_detach_filter(sk);
 		break;
diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c
index dc4aefdf2a08..e0cb29469fa7 100644
--- a/net/core/sock_reuseport.c
+++ b/net/core/sock_reuseport.c
@@ -332,3 +332,27 @@ int reuseport_attach_prog(struct sock *sk, struct bpf_prog *prog)
 	return 0;
 }
 EXPORT_SYMBOL(reuseport_attach_prog);
+
+int reuseport_detach_prog(struct sock *sk)
+{
+	struct sock_reuseport *reuse;
+	struct bpf_prog *old_prog;
+
+	if (!rcu_access_pointer(sk->sk_reuseport_cb))
+		return sk->sk_reuseport ? -ENOENT : -EINVAL;
+
+	spin_lock_bh(&reuseport_lock);
+	reuse = rcu_dereference_protected(sk->sk_reuseport_cb,
+					  lockdep_is_held(&reuseport_lock));
+	old_prog = rcu_dereference_protected(reuse->prog,
+					     lockdep_is_held(&reuseport_lock));
+	RCU_INIT_POINTER(reuse->prog, NULL);
+	spin_unlock_bh(&reuseport_lock);
+
+	if (!old_prog)
+		return -ENOENT;
+
+	sk_reuseport_prog_free(old_prog);
+	return 0;
+}
+EXPORT_SYMBOL(reuseport_detach_prog);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH bpf-next 2/2] bpf: Add test for SO_REUSEPORT_DETACH_BPF
  2019-06-12 19:05 [PATCH bpf-next 0/2] bpf: net: Detach BPF prog from reuseport sk Martin KaFai Lau
  2019-06-12 19:05 ` [PATCH bpf-next 1/2] bpf: net: Add SO_DETACH_REUSEPORT_BPF Martin KaFai Lau
@ 2019-06-12 19:05 ` Martin KaFai Lau
  2019-06-12 19:59   ` Stanislav Fomichev
  1 sibling, 1 reply; 15+ messages in thread
From: Martin KaFai Lau @ 2019-06-12 19:05 UTC (permalink / raw)
  To: bpf, netdev
  Cc: Alexei Starovoitov, Daniel Borkmann, David Miller, kernel-team

This patch adds a test for the new sockopt SO_REUSEPORT_DETACH_BPF.

'-I../../../../usr/include/' is added to the Makefile to get
the newly added SO_REUSEPORT_DETACH_BPF.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/testing/selftests/bpf/Makefile          |  1 +
 .../selftests/bpf/test_select_reuseport.c     | 50 +++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 44fb61f4d502..c7370361fa81 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -16,6 +16,7 @@ LLVM_OBJCOPY	?= llvm-objcopy
 LLVM_READELF	?= llvm-readelf
 BTF_PAHOLE	?= pahole
 CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include \
+	  -I../../../../usr/include/ \
 	  -Dbpf_prog_load=bpf_prog_test_load \
 	  -Dbpf_load_program=bpf_test_load_program
 LDLIBS += -lcap -lelf -lrt -lpthread
diff --git a/tools/testing/selftests/bpf/test_select_reuseport.c b/tools/testing/selftests/bpf/test_select_reuseport.c
index 75646d9b34aa..5aa00b4a4702 100644
--- a/tools/testing/selftests/bpf/test_select_reuseport.c
+++ b/tools/testing/selftests/bpf/test_select_reuseport.c
@@ -523,6 +523,54 @@ static void test_pass_on_err(int type, sa_family_t family)
 	printf("OK\n");
 }
 
+static void test_detach_bpf(int type, sa_family_t family)
+{
+	__u32 nr_run_before = 0, nr_run_after = 0, tmp, i;
+	struct epoll_event ev;
+	int cli_fd, err, nev;
+	struct cmd cmd = {};
+	int optvalue = 0;
+
+	printf("%s: ", __func__);
+	err = setsockopt(sk_fds[0], SOL_SOCKET, SO_DETACH_REUSEPORT_BPF,
+			 &optvalue, sizeof(optvalue));
+	CHECK(err == -1, "setsockopt(SO_DETACH_REUSEPORT_BPF)",
+	      "err:%d errno:%d\n", err, errno);
+
+	err = setsockopt(sk_fds[1], SOL_SOCKET, SO_DETACH_REUSEPORT_BPF,
+			 &optvalue, sizeof(optvalue));
+	CHECK(err == 0 || errno != ENOENT, "setsockopt(SO_DETACH_REUSEPORT_BPF)",
+	      "err:%d errno:%d\n", err, errno);
+
+	for (i = 0; i < NR_RESULTS; i++) {
+		err = bpf_map_lookup_elem(result_map, &i, &tmp);
+		CHECK(err == -1, "lookup_elem(result_map)",
+		      "i:%u err:%d errno:%d\n", i, err, errno);
+		nr_run_before += tmp;
+	}
+
+	cli_fd = send_data(type, family, &cmd, sizeof(cmd), PASS);
+	nev = epoll_wait(epfd, &ev, 1, 5);
+	CHECK(nev <= 0, "nev <= 0",
+	      "nev:%d expected:1 type:%d family:%d data:(0, 0)\n",
+	      nev,  type, family);
+
+	for (i = 0; i < NR_RESULTS; i++) {
+		err = bpf_map_lookup_elem(result_map, &i, &tmp);
+		CHECK(err == -1, "lookup_elem(result_map)",
+		      "i:%u err:%d errno:%d\n", i, err, errno);
+		nr_run_after += tmp;
+	}
+
+	CHECK(nr_run_before != nr_run_after,
+	      "nr_run_before != nr_run_after",
+	      "nr_run_before:%u nr_run_after:%u\n",
+	      nr_run_before, nr_run_after);
+
+	printf("OK\n");
+	close(cli_fd);
+}
+
 static void prepare_sk_fds(int type, sa_family_t family, bool inany)
 {
 	const int first = REUSEPORT_ARRAY_SIZE - 1;
@@ -664,6 +712,8 @@ static void test_all(void)
 			test_pass(type, family);
 			test_syncookie(type, family);
 			test_pass_on_err(type, family);
+			/* Must be the last test */
+			test_detach_bpf(type, family);
 
 			cleanup_per_test();
 			printf("\n");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH bpf-next 2/2] bpf: Add test for SO_REUSEPORT_DETACH_BPF
  2019-06-12 19:05 ` [PATCH bpf-next 2/2] bpf: Add test for SO_REUSEPORT_DETACH_BPF Martin KaFai Lau
@ 2019-06-12 19:59   ` Stanislav Fomichev
  2019-06-12 21:30     ` Martin Lau
  2019-06-12 21:47     ` Martin Lau
  0 siblings, 2 replies; 15+ messages in thread
From: Stanislav Fomichev @ 2019-06-12 19:59 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: bpf, netdev, Alexei Starovoitov, Daniel Borkmann, David Miller,
	kernel-team

On 06/12, Martin KaFai Lau wrote:
> This patch adds a test for the new sockopt SO_REUSEPORT_DETACH_BPF.
> 
> '-I../../../../usr/include/' is added to the Makefile to get
> the newly added SO_REUSEPORT_DETACH_BPF.
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
>  tools/testing/selftests/bpf/Makefile          |  1 +
>  .../selftests/bpf/test_select_reuseport.c     | 50 +++++++++++++++++++
>  2 files changed, 51 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 44fb61f4d502..c7370361fa81 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -16,6 +16,7 @@ LLVM_OBJCOPY	?= llvm-objcopy
>  LLVM_READELF	?= llvm-readelf
>  BTF_PAHOLE	?= pahole
>  CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include \
> +	  -I../../../../usr/include/ \
Why not copy inlude/uapi/asm-generic/socket.h into tools/include
instead? Will that work?

>  	  -Dbpf_prog_load=bpf_prog_test_load \
>  	  -Dbpf_load_program=bpf_test_load_program
>  LDLIBS += -lcap -lelf -lrt -lpthread
> diff --git a/tools/testing/selftests/bpf/test_select_reuseport.c b/tools/testing/selftests/bpf/test_select_reuseport.c
> index 75646d9b34aa..5aa00b4a4702 100644
> --- a/tools/testing/selftests/bpf/test_select_reuseport.c
> +++ b/tools/testing/selftests/bpf/test_select_reuseport.c
> @@ -523,6 +523,54 @@ static void test_pass_on_err(int type, sa_family_t family)
>  	printf("OK\n");
>  }
>  
> +static void test_detach_bpf(int type, sa_family_t family)
> +{
> +	__u32 nr_run_before = 0, nr_run_after = 0, tmp, i;
> +	struct epoll_event ev;
> +	int cli_fd, err, nev;
> +	struct cmd cmd = {};
> +	int optvalue = 0;
> +
> +	printf("%s: ", __func__);
> +	err = setsockopt(sk_fds[0], SOL_SOCKET, SO_DETACH_REUSEPORT_BPF,
> +			 &optvalue, sizeof(optvalue));
> +	CHECK(err == -1, "setsockopt(SO_DETACH_REUSEPORT_BPF)",
> +	      "err:%d errno:%d\n", err, errno);
> +
> +	err = setsockopt(sk_fds[1], SOL_SOCKET, SO_DETACH_REUSEPORT_BPF,
> +			 &optvalue, sizeof(optvalue));
> +	CHECK(err == 0 || errno != ENOENT, "setsockopt(SO_DETACH_REUSEPORT_BPF)",
> +	      "err:%d errno:%d\n", err, errno);
> +
> +	for (i = 0; i < NR_RESULTS; i++) {
> +		err = bpf_map_lookup_elem(result_map, &i, &tmp);
> +		CHECK(err == -1, "lookup_elem(result_map)",
> +		      "i:%u err:%d errno:%d\n", i, err, errno);
> +		nr_run_before += tmp;
> +	}
> +
> +	cli_fd = send_data(type, family, &cmd, sizeof(cmd), PASS);
> +	nev = epoll_wait(epfd, &ev, 1, 5);
> +	CHECK(nev <= 0, "nev <= 0",
> +	      "nev:%d expected:1 type:%d family:%d data:(0, 0)\n",
> +	      nev,  type, family);
> +
> +	for (i = 0; i < NR_RESULTS; i++) {
> +		err = bpf_map_lookup_elem(result_map, &i, &tmp);
> +		CHECK(err == -1, "lookup_elem(result_map)",
> +		      "i:%u err:%d errno:%d\n", i, err, errno);
> +		nr_run_after += tmp;
> +	}
> +
> +	CHECK(nr_run_before != nr_run_after,
> +	      "nr_run_before != nr_run_after",
> +	      "nr_run_before:%u nr_run_after:%u\n",
> +	      nr_run_before, nr_run_after);
> +
> +	printf("OK\n");
> +	close(cli_fd);
> +}
> +
>  static void prepare_sk_fds(int type, sa_family_t family, bool inany)
>  {
>  	const int first = REUSEPORT_ARRAY_SIZE - 1;
> @@ -664,6 +712,8 @@ static void test_all(void)
>  			test_pass(type, family);
>  			test_syncookie(type, family);
>  			test_pass_on_err(type, family);
> +			/* Must be the last test */
> +			test_detach_bpf(type, family);
>  
>  			cleanup_per_test();
>  			printf("\n");
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bpf-next 2/2] bpf: Add test for SO_REUSEPORT_DETACH_BPF
  2019-06-12 19:59   ` Stanislav Fomichev
@ 2019-06-12 21:30     ` Martin Lau
  2019-06-12 21:47       ` Stanislav Fomichev
  2019-06-12 21:47     ` Martin Lau
  1 sibling, 1 reply; 15+ messages in thread
From: Martin Lau @ 2019-06-12 21:30 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: bpf, netdev, Alexei Starovoitov, Daniel Borkmann, David Miller,
	Kernel Team

On Wed, Jun 12, 2019 at 12:59:17PM -0700, Stanislav Fomichev wrote:
> On 06/12, Martin KaFai Lau wrote:
> > This patch adds a test for the new sockopt SO_REUSEPORT_DETACH_BPF.
> > 
> > '-I../../../../usr/include/' is added to the Makefile to get
> > the newly added SO_REUSEPORT_DETACH_BPF.
> > 
> > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> > ---
> >  tools/testing/selftests/bpf/Makefile          |  1 +
> >  .../selftests/bpf/test_select_reuseport.c     | 50 +++++++++++++++++++
> >  2 files changed, 51 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 44fb61f4d502..c7370361fa81 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -16,6 +16,7 @@ LLVM_OBJCOPY	?= llvm-objcopy
> >  LLVM_READELF	?= llvm-readelf
> >  BTF_PAHOLE	?= pahole
> >  CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include \
> > +	  -I../../../../usr/include/ \
> Why not copy inlude/uapi/asm-generic/socket.h into tools/include
> instead? Will that work?
Sure. I am ok with copy.  I don't think we need to sync very often.
Do you know how to do that considering multiple arch's socket.h
have been changed in Patch 1?

Is copy better?

> 
> >  	  -Dbpf_prog_load=bpf_prog_test_load \
> >  	  -Dbpf_load_program=bpf_test_load_program
> >  LDLIBS += -lcap -lelf -lrt -lpthread
> > diff --git a/tools/testing/selftests/bpf/test_select_reuseport.c b/tools/testing/selftests/bpf/test_select_reuseport.c
> > index 75646d9b34aa..5aa00b4a4702 100644
> > --- a/tools/testing/selftests/bpf/test_select_reuseport.c
> > +++ b/tools/testing/selftests/bpf/test_select_reuseport.c
> > @@ -523,6 +523,54 @@ static void test_pass_on_err(int type, sa_family_t family)
> >  	printf("OK\n");
> >  }
> >  
> > +static void test_detach_bpf(int type, sa_family_t family)
> > +{
> > +	__u32 nr_run_before = 0, nr_run_after = 0, tmp, i;
> > +	struct epoll_event ev;
> > +	int cli_fd, err, nev;
> > +	struct cmd cmd = {};
> > +	int optvalue = 0;
> > +
> > +	printf("%s: ", __func__);
> > +	err = setsockopt(sk_fds[0], SOL_SOCKET, SO_DETACH_REUSEPORT_BPF,
> > +			 &optvalue, sizeof(optvalue));
> > +	CHECK(err == -1, "setsockopt(SO_DETACH_REUSEPORT_BPF)",
> > +	      "err:%d errno:%d\n", err, errno);
> > +
> > +	err = setsockopt(sk_fds[1], SOL_SOCKET, SO_DETACH_REUSEPORT_BPF,
> > +			 &optvalue, sizeof(optvalue));
> > +	CHECK(err == 0 || errno != ENOENT, "setsockopt(SO_DETACH_REUSEPORT_BPF)",
> > +	      "err:%d errno:%d\n", err, errno);
> > +
> > +	for (i = 0; i < NR_RESULTS; i++) {
> > +		err = bpf_map_lookup_elem(result_map, &i, &tmp);
> > +		CHECK(err == -1, "lookup_elem(result_map)",
> > +		      "i:%u err:%d errno:%d\n", i, err, errno);
> > +		nr_run_before += tmp;
> > +	}
> > +
> > +	cli_fd = send_data(type, family, &cmd, sizeof(cmd), PASS);
> > +	nev = epoll_wait(epfd, &ev, 1, 5);
> > +	CHECK(nev <= 0, "nev <= 0",
> > +	      "nev:%d expected:1 type:%d family:%d data:(0, 0)\n",
> > +	      nev,  type, family);
> > +
> > +	for (i = 0; i < NR_RESULTS; i++) {
> > +		err = bpf_map_lookup_elem(result_map, &i, &tmp);
> > +		CHECK(err == -1, "lookup_elem(result_map)",
> > +		      "i:%u err:%d errno:%d\n", i, err, errno);
> > +		nr_run_after += tmp;
> > +	}
> > +
> > +	CHECK(nr_run_before != nr_run_after,
> > +	      "nr_run_before != nr_run_after",
> > +	      "nr_run_before:%u nr_run_after:%u\n",
> > +	      nr_run_before, nr_run_after);
> > +
> > +	printf("OK\n");
> > +	close(cli_fd);
> > +}
> > +
> >  static void prepare_sk_fds(int type, sa_family_t family, bool inany)
> >  {
> >  	const int first = REUSEPORT_ARRAY_SIZE - 1;
> > @@ -664,6 +712,8 @@ static void test_all(void)
> >  			test_pass(type, family);
> >  			test_syncookie(type, family);
> >  			test_pass_on_err(type, family);
> > +			/* Must be the last test */
> > +			test_detach_bpf(type, family);
> >  
> >  			cleanup_per_test();
> >  			printf("\n");
> > -- 
> > 2.17.1
> > 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bpf-next 2/2] bpf: Add test for SO_REUSEPORT_DETACH_BPF
  2019-06-12 19:59   ` Stanislav Fomichev
  2019-06-12 21:30     ` Martin Lau
@ 2019-06-12 21:47     ` Martin Lau
  1 sibling, 0 replies; 15+ messages in thread
From: Martin Lau @ 2019-06-12 21:47 UTC (permalink / raw)
  To: Stanislav Fomichev; +Cc: bpf, netdev, Kernel Team

On Wed, Jun 12, 2019 at 12:59:17PM -0700, Stanislav Fomichev wrote:
> On 06/12, Martin KaFai Lau wrote:
> > This patch adds a test for the new sockopt SO_REUSEPORT_DETACH_BPF.
> > 
> > '-I../../../../usr/include/' is added to the Makefile to get
> > the newly added SO_REUSEPORT_DETACH_BPF.
> > 
> > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> > ---
> >  tools/testing/selftests/bpf/Makefile          |  1 +
> >  .../selftests/bpf/test_select_reuseport.c     | 50 +++++++++++++++++++
> >  2 files changed, 51 insertions(+)
> > 
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 44fb61f4d502..c7370361fa81 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -16,6 +16,7 @@ LLVM_OBJCOPY	?= llvm-objcopy
> >  LLVM_READELF	?= llvm-readelf
> >  BTF_PAHOLE	?= pahole
> >  CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include \
> > +	  -I../../../../usr/include/ \
> Why not copy inlude/uapi/asm-generic/socket.h into tools/include
> instead? Will that work?
Sure. I am ok with copy.  I don't think we need to sync very often.
Do you know how to do that considering multiple arch's socket.h
have been changed in Patch 1?

Is copy better?

> 
> >  	  -Dbpf_prog_load=bpf_prog_test_load \
> >  	  -Dbpf_load_program=bpf_test_load_program
> >  LDLIBS += -lcap -lelf -lrt -lpthread
> > diff --git a/tools/testing/selftests/bpf/test_select_reuseport.c b/tools/testing/selftests/bpf/test_select_reuseport.c
> > index 75646d9b34aa..5aa00b4a4702 100644
> > --- a/tools/testing/selftests/bpf/test_select_reuseport.c
> > +++ b/tools/testing/selftests/bpf/test_select_reuseport.c
> > @@ -523,6 +523,54 @@ static void test_pass_on_err(int type, sa_family_t family)
> >  	printf("OK\n");
> >  }
> >  
> > +static void test_detach_bpf(int type, sa_family_t family)
> > +{
> > +	__u32 nr_run_before = 0, nr_run_after = 0, tmp, i;
> > +	struct epoll_event ev;
> > +	int cli_fd, err, nev;
> > +	struct cmd cmd = {};
> > +	int optvalue = 0;
> > +
> > +	printf("%s: ", __func__);
> > +	err = setsockopt(sk_fds[0], SOL_SOCKET, SO_DETACH_REUSEPORT_BPF,
> > +			 &optvalue, sizeof(optvalue));
> > +	CHECK(err == -1, "setsockopt(SO_DETACH_REUSEPORT_BPF)",
> > +	      "err:%d errno:%d\n", err, errno);
> > +
> > +	err = setsockopt(sk_fds[1], SOL_SOCKET, SO_DETACH_REUSEPORT_BPF,
> > +			 &optvalue, sizeof(optvalue));
> > +	CHECK(err == 0 || errno != ENOENT, "setsockopt(SO_DETACH_REUSEPORT_BPF)",
> > +	      "err:%d errno:%d\n", err, errno);
> > +
> > +	for (i = 0; i < NR_RESULTS; i++) {
> > +		err = bpf_map_lookup_elem(result_map, &i, &tmp);
> > +		CHECK(err == -1, "lookup_elem(result_map)",
> > +		      "i:%u err:%d errno:%d\n", i, err, errno);
> > +		nr_run_before += tmp;
> > +	}
> > +
> > +	cli_fd = send_data(type, family, &cmd, sizeof(cmd), PASS);
> > +	nev = epoll_wait(epfd, &ev, 1, 5);
> > +	CHECK(nev <= 0, "nev <= 0",
> > +	      "nev:%d expected:1 type:%d family:%d data:(0, 0)\n",
> > +	      nev,  type, family);
> > +
> > +	for (i = 0; i < NR_RESULTS; i++) {
> > +		err = bpf_map_lookup_elem(result_map, &i, &tmp);
> > +		CHECK(err == -1, "lookup_elem(result_map)",
> > +		      "i:%u err:%d errno:%d\n", i, err, errno);
> > +		nr_run_after += tmp;
> > +	}
> > +
> > +	CHECK(nr_run_before != nr_run_after,
> > +	      "nr_run_before != nr_run_after",
> > +	      "nr_run_before:%u nr_run_after:%u\n",
> > +	      nr_run_before, nr_run_after);
> > +
> > +	printf("OK\n");
> > +	close(cli_fd);
> > +}
> > +
> >  static void prepare_sk_fds(int type, sa_family_t family, bool inany)
> >  {
> >  	const int first = REUSEPORT_ARRAY_SIZE - 1;
> > @@ -664,6 +712,8 @@ static void test_all(void)
> >  			test_pass(type, family);
> >  			test_syncookie(type, family);
> >  			test_pass_on_err(type, family);
> > +			/* Must be the last test */
> > +			test_detach_bpf(type, family);
> >  
> >  			cleanup_per_test();
> >  			printf("\n");
> > -- 
> > 2.17.1
> > 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bpf-next 2/2] bpf: Add test for SO_REUSEPORT_DETACH_BPF
  2019-06-12 21:30     ` Martin Lau
@ 2019-06-12 21:47       ` Stanislav Fomichev
  2019-06-12 21:53         ` Alexei Starovoitov
  0 siblings, 1 reply; 15+ messages in thread
From: Stanislav Fomichev @ 2019-06-12 21:47 UTC (permalink / raw)
  To: Martin Lau
  Cc: bpf, netdev, Alexei Starovoitov, Daniel Borkmann, David Miller,
	Kernel Team

On 06/12, Martin Lau wrote:
> On Wed, Jun 12, 2019 at 12:59:17PM -0700, Stanislav Fomichev wrote:
> > On 06/12, Martin KaFai Lau wrote:
> > > This patch adds a test for the new sockopt SO_REUSEPORT_DETACH_BPF.
> > > 
> > > '-I../../../../usr/include/' is added to the Makefile to get
> > > the newly added SO_REUSEPORT_DETACH_BPF.
> > > 
> > > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> > > ---
> > >  tools/testing/selftests/bpf/Makefile          |  1 +
> > >  .../selftests/bpf/test_select_reuseport.c     | 50 +++++++++++++++++++
> > >  2 files changed, 51 insertions(+)
> > > 
> > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > > index 44fb61f4d502..c7370361fa81 100644
> > > --- a/tools/testing/selftests/bpf/Makefile
> > > +++ b/tools/testing/selftests/bpf/Makefile
> > > @@ -16,6 +16,7 @@ LLVM_OBJCOPY	?= llvm-objcopy
> > >  LLVM_READELF	?= llvm-readelf
> > >  BTF_PAHOLE	?= pahole
> > >  CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include \
> > > +	  -I../../../../usr/include/ \
> > Why not copy inlude/uapi/asm-generic/socket.h into tools/include
> > instead? Will that work?
> Sure. I am ok with copy.  I don't think we need to sync very often.
> Do you know how to do that considering multiple arch's socket.h
> have been changed in Patch 1?
No, I don't know how to handle arch specific stuff. I suggest to copy
asm-generic and have ifdefs in the tests if someone complains :-)

> Is copy better?
Doesn't ../../../../usr/include provide the same headers we have in
tools/include/uapi? If you add -I../../../../usr/include, then is there
a point of having copies under tools/include/uapi? I don't really
know why we keep the copies under tools/include/uapi rather than including
../../../usr/include directly.

> > >  	  -Dbpf_prog_load=bpf_prog_test_load \
> > >  	  -Dbpf_load_program=bpf_test_load_program
> > >  LDLIBS += -lcap -lelf -lrt -lpthread
> > > diff --git a/tools/testing/selftests/bpf/test_select_reuseport.c b/tools/testing/selftests/bpf/test_select_reuseport.c
> > > index 75646d9b34aa..5aa00b4a4702 100644
> > > --- a/tools/testing/selftests/bpf/test_select_reuseport.c
> > > +++ b/tools/testing/selftests/bpf/test_select_reuseport.c
> > > @@ -523,6 +523,54 @@ static void test_pass_on_err(int type, sa_family_t family)
> > >  	printf("OK\n");
> > >  }
> > >  
> > > +static void test_detach_bpf(int type, sa_family_t family)
> > > +{
> > > +	__u32 nr_run_before = 0, nr_run_after = 0, tmp, i;
> > > +	struct epoll_event ev;
> > > +	int cli_fd, err, nev;
> > > +	struct cmd cmd = {};
> > > +	int optvalue = 0;
> > > +
> > > +	printf("%s: ", __func__);
> > > +	err = setsockopt(sk_fds[0], SOL_SOCKET, SO_DETACH_REUSEPORT_BPF,
> > > +			 &optvalue, sizeof(optvalue));
> > > +	CHECK(err == -1, "setsockopt(SO_DETACH_REUSEPORT_BPF)",
> > > +	      "err:%d errno:%d\n", err, errno);
> > > +
> > > +	err = setsockopt(sk_fds[1], SOL_SOCKET, SO_DETACH_REUSEPORT_BPF,
> > > +			 &optvalue, sizeof(optvalue));
> > > +	CHECK(err == 0 || errno != ENOENT, "setsockopt(SO_DETACH_REUSEPORT_BPF)",
> > > +	      "err:%d errno:%d\n", err, errno);
> > > +
> > > +	for (i = 0; i < NR_RESULTS; i++) {
> > > +		err = bpf_map_lookup_elem(result_map, &i, &tmp);
> > > +		CHECK(err == -1, "lookup_elem(result_map)",
> > > +		      "i:%u err:%d errno:%d\n", i, err, errno);
> > > +		nr_run_before += tmp;
> > > +	}
> > > +
> > > +	cli_fd = send_data(type, family, &cmd, sizeof(cmd), PASS);
> > > +	nev = epoll_wait(epfd, &ev, 1, 5);
> > > +	CHECK(nev <= 0, "nev <= 0",
> > > +	      "nev:%d expected:1 type:%d family:%d data:(0, 0)\n",
> > > +	      nev,  type, family);
> > > +
> > > +	for (i = 0; i < NR_RESULTS; i++) {
> > > +		err = bpf_map_lookup_elem(result_map, &i, &tmp);
> > > +		CHECK(err == -1, "lookup_elem(result_map)",
> > > +		      "i:%u err:%d errno:%d\n", i, err, errno);
> > > +		nr_run_after += tmp;
> > > +	}
> > > +
> > > +	CHECK(nr_run_before != nr_run_after,
> > > +	      "nr_run_before != nr_run_after",
> > > +	      "nr_run_before:%u nr_run_after:%u\n",
> > > +	      nr_run_before, nr_run_after);
> > > +
> > > +	printf("OK\n");
> > > +	close(cli_fd);
> > > +}
> > > +
> > >  static void prepare_sk_fds(int type, sa_family_t family, bool inany)
> > >  {
> > >  	const int first = REUSEPORT_ARRAY_SIZE - 1;
> > > @@ -664,6 +712,8 @@ static void test_all(void)
> > >  			test_pass(type, family);
> > >  			test_syncookie(type, family);
> > >  			test_pass_on_err(type, family);
> > > +			/* Must be the last test */
> > > +			test_detach_bpf(type, family);
> > >  
> > >  			cleanup_per_test();
> > >  			printf("\n");
> > > -- 
> > > 2.17.1
> > > 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bpf-next 2/2] bpf: Add test for SO_REUSEPORT_DETACH_BPF
  2019-06-12 21:47       ` Stanislav Fomichev
@ 2019-06-12 21:53         ` Alexei Starovoitov
  2019-06-12 22:15           ` Martin Lau
  0 siblings, 1 reply; 15+ messages in thread
From: Alexei Starovoitov @ 2019-06-12 21:53 UTC (permalink / raw)
  To: Stanislav Fomichev, Martin Lau
  Cc: bpf, netdev, Daniel Borkmann, David Miller, Kernel Team

On 6/12/19 2:47 PM, Stanislav Fomichev wrote:
>>>> CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include \
>>>> +	  -I../../../../usr/include/  \
>>> Why not copy inlude/uapi/asm-generic/socket.h into tools/include
>>> instead? Will that work?
>> Sure. I am ok with copy.  I don't think we need to sync very often.
>> Do you know how to do that considering multiple arch's socket.h
>> have been changed in Patch 1?
> No, I don't know how to handle arch specific stuff. I suggest to copy
> asm-generic and have ifdefs in the tests if someone complains:-)
> 
>> Is copy better?
> Doesn't ../../../../usr/include provide the same headers we have in
> tools/include/uapi? If you add -I../../../../usr/include, then is there
> a point of having copies under tools/include/uapi? I don't really
> know why we keep the copies under tools/include/uapi rather than including
> ../../../usr/include directly.

for out-of-src builds ../../../../usr/include/ directory doesn't exist.



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bpf-next 2/2] bpf: Add test for SO_REUSEPORT_DETACH_BPF
  2019-06-12 21:53         ` Alexei Starovoitov
@ 2019-06-12 22:15           ` Martin Lau
  2019-06-12 22:25             ` Alexei Starovoitov
  0 siblings, 1 reply; 15+ messages in thread
From: Martin Lau @ 2019-06-12 22:15 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Stanislav Fomichev, bpf, netdev, Daniel Borkmann, David Miller,
	Kernel Team

On Wed, Jun 12, 2019 at 02:53:35PM -0700, Alexei Starovoitov wrote:
> On 6/12/19 2:47 PM, Stanislav Fomichev wrote:
> >>>> CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include \
> >>>> +	  -I../../../../usr/include/  \
> >>> Why not copy inlude/uapi/asm-generic/socket.h into tools/include
> >>> instead? Will that work?
> >> Sure. I am ok with copy.  I don't think we need to sync very often.
> >> Do you know how to do that considering multiple arch's socket.h
> >> have been changed in Patch 1?
> > No, I don't know how to handle arch specific stuff. I suggest to copy
> > asm-generic and have ifdefs in the tests if someone complains:-)
It is not very nice but I am ok with that also.  It is the only
arch I can test ;)

> > 
> >> Is copy better?
> > Doesn't ../../../../usr/include provide the same headers we have in
> > tools/include/uapi? If you add -I../../../../usr/include, then is there
> > a point of having copies under tools/include/uapi? I don't really
> > know why we keep the copies under tools/include/uapi rather than including
> > ../../../usr/include directly.
> 
> for out-of-src builds ../../../../usr/include/ directory doesn't exist.
Is out-of-src build mostly for libbpf?
or selftests/bpf also requires out-of-src build?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bpf-next 2/2] bpf: Add test for SO_REUSEPORT_DETACH_BPF
  2019-06-12 22:15           ` Martin Lau
@ 2019-06-12 22:25             ` Alexei Starovoitov
  2019-06-12 22:39               ` Martin Lau
  0 siblings, 1 reply; 15+ messages in thread
From: Alexei Starovoitov @ 2019-06-12 22:25 UTC (permalink / raw)
  To: Martin Lau
  Cc: Stanislav Fomichev, bpf, netdev, Daniel Borkmann, David Miller,
	Kernel Team

On 6/12/19 3:15 PM, Martin Lau wrote:
> On Wed, Jun 12, 2019 at 02:53:35PM -0700, Alexei Starovoitov wrote:
>> On 6/12/19 2:47 PM, Stanislav Fomichev wrote:
>>>>>> CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include \
>>>>>> +	  -I../../../../usr/include/  \
>>>>> Why not copy inlude/uapi/asm-generic/socket.h into tools/include
>>>>> instead? Will that work?
>>>> Sure. I am ok with copy.  I don't think we need to sync very often.
>>>> Do you know how to do that considering multiple arch's socket.h
>>>> have been changed in Patch 1?
>>> No, I don't know how to handle arch specific stuff. I suggest to copy
>>> asm-generic and have ifdefs in the tests if someone complains:-)
> It is not very nice but I am ok with that also.  It is the only
> arch I can test ;)
> 
>>>
>>>> Is copy better?
>>> Doesn't ../../../../usr/include provide the same headers we have in
>>> tools/include/uapi? If you add -I../../../../usr/include, then is there
>>> a point of having copies under tools/include/uapi? I don't really
>>> know why we keep the copies under tools/include/uapi rather than including
>>> ../../../usr/include directly.
>>
>> for out-of-src builds ../../../../usr/include/ directory doesn't exist.
> Is out-of-src build mostly for libbpf?
> or selftests/bpf also requires out-of-src build?

out-of-src for kernel.
In my setup:
$ pwd -P
..../bpf-next/tools/testing/selftests/bpf
$ ls ../../../../usr/include/
ls: cannot access ../../../../usr/include/: No such file or directory
$ ls ../../../../bld_x64/usr/include/
asm  asm-generic  drm  linux  misc  mtd  rdma  scsi  sound  video  xen



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bpf-next 2/2] bpf: Add test for SO_REUSEPORT_DETACH_BPF
  2019-06-12 22:25             ` Alexei Starovoitov
@ 2019-06-12 22:39               ` Martin Lau
  0 siblings, 0 replies; 15+ messages in thread
From: Martin Lau @ 2019-06-12 22:39 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Stanislav Fomichev, bpf, netdev, Daniel Borkmann, David Miller,
	Kernel Team

On Wed, Jun 12, 2019 at 03:25:47PM -0700, Alexei Starovoitov wrote:
> On 6/12/19 3:15 PM, Martin Lau wrote:
> > On Wed, Jun 12, 2019 at 02:53:35PM -0700, Alexei Starovoitov wrote:
> >> On 6/12/19 2:47 PM, Stanislav Fomichev wrote:
> >>>>>> CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(BPFDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include \
> >>>>>> +	  -I../../../../usr/include/  \
> >>>>> Why not copy inlude/uapi/asm-generic/socket.h into tools/include
> >>>>> instead? Will that work?
> >>>> Sure. I am ok with copy.  I don't think we need to sync very often.
> >>>> Do you know how to do that considering multiple arch's socket.h
> >>>> have been changed in Patch 1?
> >>> No, I don't know how to handle arch specific stuff. I suggest to copy
> >>> asm-generic and have ifdefs in the tests if someone complains:-)
> > It is not very nice but I am ok with that also.  It is the only
> > arch I can test ;)
> > 
> >>>
> >>>> Is copy better?
> >>> Doesn't ../../../../usr/include provide the same headers we have in
> >>> tools/include/uapi? If you add -I../../../../usr/include, then is there
> >>> a point of having copies under tools/include/uapi? I don't really
> >>> know why we keep the copies under tools/include/uapi rather than including
> >>> ../../../usr/include directly.
> >>
> >> for out-of-src builds ../../../../usr/include/ directory doesn't exist.
> > Is out-of-src build mostly for libbpf?
> > or selftests/bpf also requires out-of-src build?
> 
> out-of-src for kernel.
> In my setup:
> $ pwd -P
> ..../bpf-next/tools/testing/selftests/bpf
> $ ls ../../../../usr/include/
> ls: cannot access ../../../../usr/include/: No such file or directory
> $ ls ../../../../bld_x64/usr/include/
> asm  asm-generic  drm  linux  misc  mtd  rdma  scsi  sound  video  xen
Got it.  Agreed.
I will copy asm-generic/socket.h and do the ifdef.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bpf-next 1/2] bpf: net: Add SO_DETACH_REUSEPORT_BPF
  2019-06-12 19:05 ` [PATCH bpf-next 1/2] bpf: net: Add SO_DETACH_REUSEPORT_BPF Martin KaFai Lau
@ 2019-06-13  4:39   ` Andrii Nakryiko
  2019-06-13 19:18   ` kbuild test robot
  2019-06-13 20:14   ` Andrii Nakryiko
  2 siblings, 0 replies; 15+ messages in thread
From: Andrii Nakryiko @ 2019-06-13  4:39 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
	David Miller, Kernel Team, Craig Gallek

On Wed, Jun 12, 2019 at 12:08 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> There is SO_ATTACH_REUSEPORT_[CE]BPF but there is no DETACH.
> This patch adds SO_DETACH_REUSEPORT_BPF sockopt.  The same
> sockopt can be used to undo both SO_ATTACH_REUSEPORT_[CE]BPF.
>
> reseport_detach_prog() is added and it is mostly a mirror
> of the existing reuseport_attach_prog().  The differences are,
> it does not call reuseport_alloc() and returns -ENOENT when
> there is no old prog.
>
> Cc: Craig Gallek <kraig@google.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---

Acked-by: Andrii Nakryiko <andriin@fb.com>

>  arch/alpha/include/uapi/asm/socket.h  |  2 ++
>  arch/mips/include/uapi/asm/socket.h   |  2 ++
>  arch/parisc/include/uapi/asm/socket.h |  2 ++
>  arch/sparc/include/uapi/asm/socket.h  |  2 ++
>  include/net/sock_reuseport.h          |  2 ++
>  include/uapi/asm-generic/socket.h     |  2 ++
>  net/core/sock.c                       |  4 ++++
>  net/core/sock_reuseport.c             | 24 ++++++++++++++++++++++++
>  8 files changed, 40 insertions(+)
>
> diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
> index 976e89b116e5..de6c4df61082 100644
> --- a/arch/alpha/include/uapi/asm/socket.h
> +++ b/arch/alpha/include/uapi/asm/socket.h
> @@ -122,6 +122,8 @@
>  #define SO_RCVTIMEO_NEW         66
>  #define SO_SNDTIMEO_NEW         67
>
> +#define SO_DETACH_REUSEPORT_BPF 68
> +
>  #if !defined(__KERNEL__)
>
>  #if __BITS_PER_LONG == 64
> diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
> index d41765cfbc6e..d0a9ed2ca2d6 100644
> --- a/arch/mips/include/uapi/asm/socket.h
> +++ b/arch/mips/include/uapi/asm/socket.h
> @@ -133,6 +133,8 @@
>  #define SO_RCVTIMEO_NEW         66
>  #define SO_SNDTIMEO_NEW         67
>
> +#define SO_DETACH_REUSEPORT_BPF 68
> +
>  #if !defined(__KERNEL__)
>
>  #if __BITS_PER_LONG == 64
> diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
> index 66c5dd245ac7..10173c32195e 100644
> --- a/arch/parisc/include/uapi/asm/socket.h
> +++ b/arch/parisc/include/uapi/asm/socket.h
> @@ -114,6 +114,8 @@
>  #define SO_RCVTIMEO_NEW         0x4040
>  #define SO_SNDTIMEO_NEW         0x4041
>
> +#define SO_DETACH_REUSEPORT_BPF 0x4042
> +
>  #if !defined(__KERNEL__)
>
>  #if __BITS_PER_LONG == 64
> diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
> index 9265a9eece15..1895ac112a24 100644
> --- a/arch/sparc/include/uapi/asm/socket.h
> +++ b/arch/sparc/include/uapi/asm/socket.h
> @@ -115,6 +115,8 @@
>  #define SO_RCVTIMEO_NEW          0x0044
>  #define SO_SNDTIMEO_NEW          0x0045
>
> +#define SO_DETACH_REUSEPORT_BPF  0x0046
> +
>  #if !defined(__KERNEL__)
>
>
> diff --git a/include/net/sock_reuseport.h b/include/net/sock_reuseport.h
> index 8a5f70c7cdf2..d9112de85261 100644
> --- a/include/net/sock_reuseport.h
> +++ b/include/net/sock_reuseport.h
> @@ -35,6 +35,8 @@ extern struct sock *reuseport_select_sock(struct sock *sk,
>                                           struct sk_buff *skb,
>                                           int hdr_len);
>  extern int reuseport_attach_prog(struct sock *sk, struct bpf_prog *prog);
> +extern int reuseport_detach_prog(struct sock *sk);
> +
>  int reuseport_get_id(struct sock_reuseport *reuse);
>
>  #endif  /* _SOCK_REUSEPORT_H */
> diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
> index 8c1391c89171..77f7c1638eb1 100644
> --- a/include/uapi/asm-generic/socket.h
> +++ b/include/uapi/asm-generic/socket.h
> @@ -117,6 +117,8 @@
>  #define SO_RCVTIMEO_NEW         66
>  #define SO_SNDTIMEO_NEW         67
>
> +#define SO_DETACH_REUSEPORT_BPF 68
> +
>  #if !defined(__KERNEL__)
>
>  #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 75b1c950b49f..06be30737b69 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1045,6 +1045,10 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
>                 }
>                 break;
>
> +       case SO_DETACH_REUSEPORT_BPF:
> +               ret = reuseport_detach_prog(sk);
> +               break;
> +
>         case SO_DETACH_FILTER:
>                 ret = sk_detach_filter(sk);
>                 break;
> diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c
> index dc4aefdf2a08..e0cb29469fa7 100644
> --- a/net/core/sock_reuseport.c
> +++ b/net/core/sock_reuseport.c
> @@ -332,3 +332,27 @@ int reuseport_attach_prog(struct sock *sk, struct bpf_prog *prog)
>         return 0;
>  }
>  EXPORT_SYMBOL(reuseport_attach_prog);
> +
> +int reuseport_detach_prog(struct sock *sk)
> +{
> +       struct sock_reuseport *reuse;
> +       struct bpf_prog *old_prog;
> +
> +       if (!rcu_access_pointer(sk->sk_reuseport_cb))
> +               return sk->sk_reuseport ? -ENOENT : -EINVAL;
> +
> +       spin_lock_bh(&reuseport_lock);
> +       reuse = rcu_dereference_protected(sk->sk_reuseport_cb,
> +                                         lockdep_is_held(&reuseport_lock));
> +       old_prog = rcu_dereference_protected(reuse->prog,
> +                                            lockdep_is_held(&reuseport_lock));
> +       RCU_INIT_POINTER(reuse->prog, NULL);
> +       spin_unlock_bh(&reuseport_lock);
> +
> +       if (!old_prog)
> +               return -ENOENT;
> +
> +       sk_reuseport_prog_free(old_prog);
> +       return 0;
> +}
> +EXPORT_SYMBOL(reuseport_detach_prog);
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bpf-next 1/2] bpf: net: Add SO_DETACH_REUSEPORT_BPF
  2019-06-12 19:05 ` [PATCH bpf-next 1/2] bpf: net: Add SO_DETACH_REUSEPORT_BPF Martin KaFai Lau
  2019-06-13  4:39   ` Andrii Nakryiko
@ 2019-06-13 19:18   ` kbuild test robot
  2019-06-13 20:14   ` Andrii Nakryiko
  2 siblings, 0 replies; 15+ messages in thread
From: kbuild test robot @ 2019-06-13 19:18 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: kbuild-all, bpf, netdev, Alexei Starovoitov, Daniel Borkmann,
	David Miller, kernel-team, Craig Gallek

[-- Attachment #1: Type: text/plain, Size: 12521 bytes --]

Hi Martin,

I love your patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Martin-KaFai-Lau/bpf-net-Add-SO_DETACH_REUSEPORT_BPF/20190614-015829
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=sparc64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   net//core/sock.c: In function 'sock_setsockopt':
>> net//core/sock.c:1048:2: error: duplicate case value
     case SO_DETACH_REUSEPORT_BPF:
     ^~~~
   net//core/sock.c:912:2: note: previously used here
     case SO_TIMESTAMP_NEW:
     ^~~~

vim +1048 net//core/sock.c

   722	
   723	/*
   724	 *	This is meant for all protocols to use and covers goings on
   725	 *	at the socket level. Everything here is generic.
   726	 */
   727	
   728	int sock_setsockopt(struct socket *sock, int level, int optname,
   729			    char __user *optval, unsigned int optlen)
   730	{
   731		struct sock_txtime sk_txtime;
   732		struct sock *sk = sock->sk;
   733		int val;
   734		int valbool;
   735		struct linger ling;
   736		int ret = 0;
   737	
   738		/*
   739		 *	Options without arguments
   740		 */
   741	
   742		if (optname == SO_BINDTODEVICE)
   743			return sock_setbindtodevice(sk, optval, optlen);
   744	
   745		if (optlen < sizeof(int))
   746			return -EINVAL;
   747	
   748		if (get_user(val, (int __user *)optval))
   749			return -EFAULT;
   750	
   751		valbool = val ? 1 : 0;
   752	
   753		lock_sock(sk);
   754	
   755		switch (optname) {
   756		case SO_DEBUG:
   757			if (val && !capable(CAP_NET_ADMIN))
   758				ret = -EACCES;
   759			else
   760				sock_valbool_flag(sk, SOCK_DBG, valbool);
   761			break;
   762		case SO_REUSEADDR:
   763			sk->sk_reuse = (valbool ? SK_CAN_REUSE : SK_NO_REUSE);
   764			break;
   765		case SO_REUSEPORT:
   766			sk->sk_reuseport = valbool;
   767			break;
   768		case SO_TYPE:
   769		case SO_PROTOCOL:
   770		case SO_DOMAIN:
   771		case SO_ERROR:
   772			ret = -ENOPROTOOPT;
   773			break;
   774		case SO_DONTROUTE:
   775			sock_valbool_flag(sk, SOCK_LOCALROUTE, valbool);
   776			sk_dst_reset(sk);
   777			break;
   778		case SO_BROADCAST:
   779			sock_valbool_flag(sk, SOCK_BROADCAST, valbool);
   780			break;
   781		case SO_SNDBUF:
   782			/* Don't error on this BSD doesn't and if you think
   783			 * about it this is right. Otherwise apps have to
   784			 * play 'guess the biggest size' games. RCVBUF/SNDBUF
   785			 * are treated in BSD as hints
   786			 */
   787			val = min_t(u32, val, sysctl_wmem_max);
   788	set_sndbuf:
   789			/* Ensure val * 2 fits into an int, to prevent max_t()
   790			 * from treating it as a negative value.
   791			 */
   792			val = min_t(int, val, INT_MAX / 2);
   793			sk->sk_userlocks |= SOCK_SNDBUF_LOCK;
   794			sk->sk_sndbuf = max_t(int, val * 2, SOCK_MIN_SNDBUF);
   795			/* Wake up sending tasks if we upped the value. */
   796			sk->sk_write_space(sk);
   797			break;
   798	
   799		case SO_SNDBUFFORCE:
   800			if (!capable(CAP_NET_ADMIN)) {
   801				ret = -EPERM;
   802				break;
   803			}
   804	
   805			/* No negative values (to prevent underflow, as val will be
   806			 * multiplied by 2).
   807			 */
   808			if (val < 0)
   809				val = 0;
   810			goto set_sndbuf;
   811	
   812		case SO_RCVBUF:
   813			/* Don't error on this BSD doesn't and if you think
   814			 * about it this is right. Otherwise apps have to
   815			 * play 'guess the biggest size' games. RCVBUF/SNDBUF
   816			 * are treated in BSD as hints
   817			 */
   818			val = min_t(u32, val, sysctl_rmem_max);
   819	set_rcvbuf:
   820			/* Ensure val * 2 fits into an int, to prevent max_t()
   821			 * from treating it as a negative value.
   822			 */
   823			val = min_t(int, val, INT_MAX / 2);
   824			sk->sk_userlocks |= SOCK_RCVBUF_LOCK;
   825			/*
   826			 * We double it on the way in to account for
   827			 * "struct sk_buff" etc. overhead.   Applications
   828			 * assume that the SO_RCVBUF setting they make will
   829			 * allow that much actual data to be received on that
   830			 * socket.
   831			 *
   832			 * Applications are unaware that "struct sk_buff" and
   833			 * other overheads allocate from the receive buffer
   834			 * during socket buffer allocation.
   835			 *
   836			 * And after considering the possible alternatives,
   837			 * returning the value we actually used in getsockopt
   838			 * is the most desirable behavior.
   839			 */
   840			sk->sk_rcvbuf = max_t(int, val * 2, SOCK_MIN_RCVBUF);
   841			break;
   842	
   843		case SO_RCVBUFFORCE:
   844			if (!capable(CAP_NET_ADMIN)) {
   845				ret = -EPERM;
   846				break;
   847			}
   848	
   849			/* No negative values (to prevent underflow, as val will be
   850			 * multiplied by 2).
   851			 */
   852			if (val < 0)
   853				val = 0;
   854			goto set_rcvbuf;
   855	
   856		case SO_KEEPALIVE:
   857			if (sk->sk_prot->keepalive)
   858				sk->sk_prot->keepalive(sk, valbool);
   859			sock_valbool_flag(sk, SOCK_KEEPOPEN, valbool);
   860			break;
   861	
   862		case SO_OOBINLINE:
   863			sock_valbool_flag(sk, SOCK_URGINLINE, valbool);
   864			break;
   865	
   866		case SO_NO_CHECK:
   867			sk->sk_no_check_tx = valbool;
   868			break;
   869	
   870		case SO_PRIORITY:
   871			if ((val >= 0 && val <= 6) ||
   872			    ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))
   873				sk->sk_priority = val;
   874			else
   875				ret = -EPERM;
   876			break;
   877	
   878		case SO_LINGER:
   879			if (optlen < sizeof(ling)) {
   880				ret = -EINVAL;	/* 1003.1g */
   881				break;
   882			}
   883			if (copy_from_user(&ling, optval, sizeof(ling))) {
   884				ret = -EFAULT;
   885				break;
   886			}
   887			if (!ling.l_onoff)
   888				sock_reset_flag(sk, SOCK_LINGER);
   889			else {
   890	#if (BITS_PER_LONG == 32)
   891				if ((unsigned int)ling.l_linger >= MAX_SCHEDULE_TIMEOUT/HZ)
   892					sk->sk_lingertime = MAX_SCHEDULE_TIMEOUT;
   893				else
   894	#endif
   895					sk->sk_lingertime = (unsigned int)ling.l_linger * HZ;
   896				sock_set_flag(sk, SOCK_LINGER);
   897			}
   898			break;
   899	
   900		case SO_BSDCOMPAT:
   901			sock_warn_obsolete_bsdism("setsockopt");
   902			break;
   903	
   904		case SO_PASSCRED:
   905			if (valbool)
   906				set_bit(SOCK_PASSCRED, &sock->flags);
   907			else
   908				clear_bit(SOCK_PASSCRED, &sock->flags);
   909			break;
   910	
   911		case SO_TIMESTAMP_OLD:
   912		case SO_TIMESTAMP_NEW:
   913		case SO_TIMESTAMPNS_OLD:
   914		case SO_TIMESTAMPNS_NEW:
   915			if (valbool)  {
   916				if (optname == SO_TIMESTAMP_NEW || optname == SO_TIMESTAMPNS_NEW)
   917					sock_set_flag(sk, SOCK_TSTAMP_NEW);
   918				else
   919					sock_reset_flag(sk, SOCK_TSTAMP_NEW);
   920	
   921				if (optname == SO_TIMESTAMP_OLD || optname == SO_TIMESTAMP_NEW)
   922					sock_reset_flag(sk, SOCK_RCVTSTAMPNS);
   923				else
   924					sock_set_flag(sk, SOCK_RCVTSTAMPNS);
   925				sock_set_flag(sk, SOCK_RCVTSTAMP);
   926				sock_enable_timestamp(sk, SOCK_TIMESTAMP);
   927			} else {
   928				sock_reset_flag(sk, SOCK_RCVTSTAMP);
   929				sock_reset_flag(sk, SOCK_RCVTSTAMPNS);
   930				sock_reset_flag(sk, SOCK_TSTAMP_NEW);
   931			}
   932			break;
   933	
   934		case SO_TIMESTAMPING_NEW:
   935			sock_set_flag(sk, SOCK_TSTAMP_NEW);
   936			/* fall through */
   937		case SO_TIMESTAMPING_OLD:
   938			if (val & ~SOF_TIMESTAMPING_MASK) {
   939				ret = -EINVAL;
   940				break;
   941			}
   942	
   943			if (val & SOF_TIMESTAMPING_OPT_ID &&
   944			    !(sk->sk_tsflags & SOF_TIMESTAMPING_OPT_ID)) {
   945				if (sk->sk_protocol == IPPROTO_TCP &&
   946				    sk->sk_type == SOCK_STREAM) {
   947					if ((1 << sk->sk_state) &
   948					    (TCPF_CLOSE | TCPF_LISTEN)) {
   949						ret = -EINVAL;
   950						break;
   951					}
   952					sk->sk_tskey = tcp_sk(sk)->snd_una;
   953				} else {
   954					sk->sk_tskey = 0;
   955				}
   956			}
   957	
   958			if (val & SOF_TIMESTAMPING_OPT_STATS &&
   959			    !(val & SOF_TIMESTAMPING_OPT_TSONLY)) {
   960				ret = -EINVAL;
   961				break;
   962			}
   963	
   964			sk->sk_tsflags = val;
   965			if (val & SOF_TIMESTAMPING_RX_SOFTWARE)
   966				sock_enable_timestamp(sk,
   967						      SOCK_TIMESTAMPING_RX_SOFTWARE);
   968			else {
   969				if (optname == SO_TIMESTAMPING_NEW)
   970					sock_reset_flag(sk, SOCK_TSTAMP_NEW);
   971	
   972				sock_disable_timestamp(sk,
   973						       (1UL << SOCK_TIMESTAMPING_RX_SOFTWARE));
   974			}
   975			break;
   976	
   977		case SO_RCVLOWAT:
   978			if (val < 0)
   979				val = INT_MAX;
   980			if (sock->ops->set_rcvlowat)
   981				ret = sock->ops->set_rcvlowat(sk, val);
   982			else
   983				sk->sk_rcvlowat = val ? : 1;
   984			break;
   985	
   986		case SO_RCVTIMEO_OLD:
   987		case SO_RCVTIMEO_NEW:
   988			ret = sock_set_timeout(&sk->sk_rcvtimeo, optval, optlen, optname == SO_RCVTIMEO_OLD);
   989			break;
   990	
   991		case SO_SNDTIMEO_OLD:
   992		case SO_SNDTIMEO_NEW:
   993			ret = sock_set_timeout(&sk->sk_sndtimeo, optval, optlen, optname == SO_SNDTIMEO_OLD);
   994			break;
   995	
   996		case SO_ATTACH_FILTER:
   997			ret = -EINVAL;
   998			if (optlen == sizeof(struct sock_fprog)) {
   999				struct sock_fprog fprog;
  1000	
  1001				ret = -EFAULT;
  1002				if (copy_from_user(&fprog, optval, sizeof(fprog)))
  1003					break;
  1004	
  1005				ret = sk_attach_filter(&fprog, sk);
  1006			}
  1007			break;
  1008	
  1009		case SO_ATTACH_BPF:
  1010			ret = -EINVAL;
  1011			if (optlen == sizeof(u32)) {
  1012				u32 ufd;
  1013	
  1014				ret = -EFAULT;
  1015				if (copy_from_user(&ufd, optval, sizeof(ufd)))
  1016					break;
  1017	
  1018				ret = sk_attach_bpf(ufd, sk);
  1019			}
  1020			break;
  1021	
  1022		case SO_ATTACH_REUSEPORT_CBPF:
  1023			ret = -EINVAL;
  1024			if (optlen == sizeof(struct sock_fprog)) {
  1025				struct sock_fprog fprog;
  1026	
  1027				ret = -EFAULT;
  1028				if (copy_from_user(&fprog, optval, sizeof(fprog)))
  1029					break;
  1030	
  1031				ret = sk_reuseport_attach_filter(&fprog, sk);
  1032			}
  1033			break;
  1034	
  1035		case SO_ATTACH_REUSEPORT_EBPF:
  1036			ret = -EINVAL;
  1037			if (optlen == sizeof(u32)) {
  1038				u32 ufd;
  1039	
  1040				ret = -EFAULT;
  1041				if (copy_from_user(&ufd, optval, sizeof(ufd)))
  1042					break;
  1043	
  1044				ret = sk_reuseport_attach_bpf(ufd, sk);
  1045			}
  1046			break;
  1047	
> 1048		case SO_DETACH_REUSEPORT_BPF:
  1049			ret = reuseport_detach_prog(sk);
  1050			break;
  1051	
  1052		case SO_DETACH_FILTER:
  1053			ret = sk_detach_filter(sk);
  1054			break;
  1055	
  1056		case SO_LOCK_FILTER:
  1057			if (sock_flag(sk, SOCK_FILTER_LOCKED) && !valbool)
  1058				ret = -EPERM;
  1059			else
  1060				sock_valbool_flag(sk, SOCK_FILTER_LOCKED, valbool);
  1061			break;
  1062	
  1063		case SO_PASSSEC:
  1064			if (valbool)
  1065				set_bit(SOCK_PASSSEC, &sock->flags);
  1066			else
  1067				clear_bit(SOCK_PASSSEC, &sock->flags);
  1068			break;
  1069		case SO_MARK:
  1070			if (!ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN)) {
  1071				ret = -EPERM;
  1072			} else if (val != sk->sk_mark) {
  1073				sk->sk_mark = val;
  1074				sk_dst_reset(sk);
  1075			}
  1076			break;
  1077	
  1078		case SO_RXQ_OVFL:
  1079			sock_valbool_flag(sk, SOCK_RXQ_OVFL, valbool);
  1080			break;
  1081	
  1082		case SO_WIFI_STATUS:
  1083			sock_valbool_flag(sk, SOCK_WIFI_STATUS, valbool);
  1084			break;
  1085	
  1086		case SO_PEEK_OFF:
  1087			if (sock->ops->set_peek_off)
  1088				ret = sock->ops->set_peek_off(sk, val);
  1089			else
  1090				ret = -EOPNOTSUPP;
  1091			break;
  1092	
  1093		case SO_NOFCS:
  1094			sock_valbool_flag(sk, SOCK_NOFCS, valbool);
  1095			break;
  1096	
  1097		case SO_SELECT_ERR_QUEUE:
  1098			sock_valbool_flag(sk, SOCK_SELECT_ERR_QUEUE, valbool);
  1099			break;
  1100	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 58090 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bpf-next 1/2] bpf: net: Add SO_DETACH_REUSEPORT_BPF
  2019-06-12 19:05 ` [PATCH bpf-next 1/2] bpf: net: Add SO_DETACH_REUSEPORT_BPF Martin KaFai Lau
  2019-06-13  4:39   ` Andrii Nakryiko
  2019-06-13 19:18   ` kbuild test robot
@ 2019-06-13 20:14   ` Andrii Nakryiko
  2019-06-13 21:18     ` Martin Lau
  2 siblings, 1 reply; 15+ messages in thread
From: Andrii Nakryiko @ 2019-06-13 20:14 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
	David Miller, Kernel Team, Craig Gallek

On Wed, Jun 12, 2019 at 12:08 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> There is SO_ATTACH_REUSEPORT_[CE]BPF but there is no DETACH.
> This patch adds SO_DETACH_REUSEPORT_BPF sockopt.  The same
> sockopt can be used to undo both SO_ATTACH_REUSEPORT_[CE]BPF.
>
> reseport_detach_prog() is added and it is mostly a mirror
> of the existing reuseport_attach_prog().  The differences are,
> it does not call reuseport_alloc() and returns -ENOENT when
> there is no old prog.
>
> Cc: Craig Gallek <kraig@google.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
>  arch/alpha/include/uapi/asm/socket.h  |  2 ++
>  arch/mips/include/uapi/asm/socket.h   |  2 ++
>  arch/parisc/include/uapi/asm/socket.h |  2 ++
>  arch/sparc/include/uapi/asm/socket.h  |  2 ++
>  include/net/sock_reuseport.h          |  2 ++
>  include/uapi/asm-generic/socket.h     |  2 ++
>  net/core/sock.c                       |  4 ++++
>  net/core/sock_reuseport.c             | 24 ++++++++++++++++++++++++
>  8 files changed, 40 insertions(+)
>
> diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
> index 976e89b116e5..de6c4df61082 100644
> --- a/arch/alpha/include/uapi/asm/socket.h
> +++ b/arch/alpha/include/uapi/asm/socket.h
> @@ -122,6 +122,8 @@
>  #define SO_RCVTIMEO_NEW         66
>  #define SO_SNDTIMEO_NEW         67
>
> +#define SO_DETACH_REUSEPORT_BPF 68
> +
>  #if !defined(__KERNEL__)
>
>  #if __BITS_PER_LONG == 64
> diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
> index d41765cfbc6e..d0a9ed2ca2d6 100644
> --- a/arch/mips/include/uapi/asm/socket.h
> +++ b/arch/mips/include/uapi/asm/socket.h
> @@ -133,6 +133,8 @@
>  #define SO_RCVTIMEO_NEW         66
>  #define SO_SNDTIMEO_NEW         67
>
> +#define SO_DETACH_REUSEPORT_BPF 68
> +
>  #if !defined(__KERNEL__)
>
>  #if __BITS_PER_LONG == 64
> diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
> index 66c5dd245ac7..10173c32195e 100644
> --- a/arch/parisc/include/uapi/asm/socket.h
> +++ b/arch/parisc/include/uapi/asm/socket.h
> @@ -114,6 +114,8 @@
>  #define SO_RCVTIMEO_NEW         0x4040
>  #define SO_SNDTIMEO_NEW         0x4041
>
> +#define SO_DETACH_REUSEPORT_BPF 0x4042
> +
>  #if !defined(__KERNEL__)
>
>  #if __BITS_PER_LONG == 64
> diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
> index 9265a9eece15..1895ac112a24 100644
> --- a/arch/sparc/include/uapi/asm/socket.h
> +++ b/arch/sparc/include/uapi/asm/socket.h
> @@ -115,6 +115,8 @@
>  #define SO_RCVTIMEO_NEW          0x0044
>  #define SO_SNDTIMEO_NEW          0x0045
>
> +#define SO_DETACH_REUSEPORT_BPF  0x0046


Oh, it's so nasty, there is

#define SO_TIMESTAMP_NEW         0x0046

few lines above this, completely out of order. Let's reorder it to
avoid issues like this one.

> +
>  #if !defined(__KERNEL__)
>
>
> diff --git a/include/net/sock_reuseport.h b/include/net/sock_reuseport.h
> index 8a5f70c7cdf2..d9112de85261 100644
> --- a/include/net/sock_reuseport.h
> +++ b/include/net/sock_reuseport.h
> @@ -35,6 +35,8 @@ extern struct sock *reuseport_select_sock(struct sock *sk,
>                                           struct sk_buff *skb,
>                                           int hdr_len);
>  extern int reuseport_attach_prog(struct sock *sk, struct bpf_prog *prog);
> +extern int reuseport_detach_prog(struct sock *sk);
> +
>  int reuseport_get_id(struct sock_reuseport *reuse);
>
>  #endif  /* _SOCK_REUSEPORT_H */
> diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
> index 8c1391c89171..77f7c1638eb1 100644
> --- a/include/uapi/asm-generic/socket.h
> +++ b/include/uapi/asm-generic/socket.h
> @@ -117,6 +117,8 @@
>  #define SO_RCVTIMEO_NEW         66
>  #define SO_SNDTIMEO_NEW         67
>
> +#define SO_DETACH_REUSEPORT_BPF 68
> +
>  #if !defined(__KERNEL__)
>
>  #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 75b1c950b49f..06be30737b69 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1045,6 +1045,10 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
>                 }
>                 break;
>
> +       case SO_DETACH_REUSEPORT_BPF:
> +               ret = reuseport_detach_prog(sk);
> +               break;
> +
>         case SO_DETACH_FILTER:
>                 ret = sk_detach_filter(sk);
>                 break;
> diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c
> index dc4aefdf2a08..e0cb29469fa7 100644
> --- a/net/core/sock_reuseport.c
> +++ b/net/core/sock_reuseport.c
> @@ -332,3 +332,27 @@ int reuseport_attach_prog(struct sock *sk, struct bpf_prog *prog)
>         return 0;
>  }
>  EXPORT_SYMBOL(reuseport_attach_prog);
> +
> +int reuseport_detach_prog(struct sock *sk)
> +{
> +       struct sock_reuseport *reuse;
> +       struct bpf_prog *old_prog;
> +
> +       if (!rcu_access_pointer(sk->sk_reuseport_cb))
> +               return sk->sk_reuseport ? -ENOENT : -EINVAL;
> +
> +       spin_lock_bh(&reuseport_lock);
> +       reuse = rcu_dereference_protected(sk->sk_reuseport_cb,
> +                                         lockdep_is_held(&reuseport_lock));
> +       old_prog = rcu_dereference_protected(reuse->prog,
> +                                            lockdep_is_held(&reuseport_lock));
> +       RCU_INIT_POINTER(reuse->prog, NULL);
> +       spin_unlock_bh(&reuseport_lock);
> +
> +       if (!old_prog)
> +               return -ENOENT;
> +
> +       sk_reuseport_prog_free(old_prog);
> +       return 0;
> +}
> +EXPORT_SYMBOL(reuseport_detach_prog);
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH bpf-next 1/2] bpf: net: Add SO_DETACH_REUSEPORT_BPF
  2019-06-13 20:14   ` Andrii Nakryiko
@ 2019-06-13 21:18     ` Martin Lau
  0 siblings, 0 replies; 15+ messages in thread
From: Martin Lau @ 2019-06-13 21:18 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
	David Miller, Kernel Team, Craig Gallek

On Thu, Jun 13, 2019 at 01:14:46PM -0700, Andrii Nakryiko wrote:
> On Wed, Jun 12, 2019 at 12:08 PM Martin KaFai Lau <kafai@fb.com> wrote:
> >
> > There is SO_ATTACH_REUSEPORT_[CE]BPF but there is no DETACH.
> > This patch adds SO_DETACH_REUSEPORT_BPF sockopt.  The same
> > sockopt can be used to undo both SO_ATTACH_REUSEPORT_[CE]BPF.
> >
> > reseport_detach_prog() is added and it is mostly a mirror
> > of the existing reuseport_attach_prog().  The differences are,
> > it does not call reuseport_alloc() and returns -ENOENT when
> > there is no old prog.
> >
> > Cc: Craig Gallek <kraig@google.com>
> > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> > ---
> >  arch/alpha/include/uapi/asm/socket.h  |  2 ++
> >  arch/mips/include/uapi/asm/socket.h   |  2 ++
> >  arch/parisc/include/uapi/asm/socket.h |  2 ++
> >  arch/sparc/include/uapi/asm/socket.h  |  2 ++
> >  include/net/sock_reuseport.h          |  2 ++
> >  include/uapi/asm-generic/socket.h     |  2 ++
> >  net/core/sock.c                       |  4 ++++
> >  net/core/sock_reuseport.c             | 24 ++++++++++++++++++++++++
> >  8 files changed, 40 insertions(+)
> >
> > diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
> > index 976e89b116e5..de6c4df61082 100644
> > --- a/arch/alpha/include/uapi/asm/socket.h
> > +++ b/arch/alpha/include/uapi/asm/socket.h
> > @@ -122,6 +122,8 @@
> >  #define SO_RCVTIMEO_NEW         66
> >  #define SO_SNDTIMEO_NEW         67
> >
> > +#define SO_DETACH_REUSEPORT_BPF 68
> > +
> >  #if !defined(__KERNEL__)
> >
> >  #if __BITS_PER_LONG == 64
> > diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
> > index d41765cfbc6e..d0a9ed2ca2d6 100644
> > --- a/arch/mips/include/uapi/asm/socket.h
> > +++ b/arch/mips/include/uapi/asm/socket.h
> > @@ -133,6 +133,8 @@
> >  #define SO_RCVTIMEO_NEW         66
> >  #define SO_SNDTIMEO_NEW         67
> >
> > +#define SO_DETACH_REUSEPORT_BPF 68
> > +
> >  #if !defined(__KERNEL__)
> >
> >  #if __BITS_PER_LONG == 64
> > diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
> > index 66c5dd245ac7..10173c32195e 100644
> > --- a/arch/parisc/include/uapi/asm/socket.h
> > +++ b/arch/parisc/include/uapi/asm/socket.h
> > @@ -114,6 +114,8 @@
> >  #define SO_RCVTIMEO_NEW         0x4040
> >  #define SO_SNDTIMEO_NEW         0x4041
> >
> > +#define SO_DETACH_REUSEPORT_BPF 0x4042
> > +
> >  #if !defined(__KERNEL__)
> >
> >  #if __BITS_PER_LONG == 64
> > diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
> > index 9265a9eece15..1895ac112a24 100644
> > --- a/arch/sparc/include/uapi/asm/socket.h
> > +++ b/arch/sparc/include/uapi/asm/socket.h
> > @@ -115,6 +115,8 @@
> >  #define SO_RCVTIMEO_NEW          0x0044
> >  #define SO_SNDTIMEO_NEW          0x0045
> >
> > +#define SO_DETACH_REUSEPORT_BPF  0x0046
> 
> 
> Oh, it's so nasty, there is
> 
> #define SO_TIMESTAMP_NEW         0x0046
> 
> few lines above this, completely out of order. Let's reorder it to
> avoid issues like this one.
I probably will not reorder any existing one in v3.  SO_TIMESTAMP_NEW
seems not the only one.  It may worth a separate patch effort.

I will use 0x0047.

>
> > +
> >  #if !defined(__KERNEL__)
> >
> >
> > diff --git a/include/net/sock_reuseport.h b/include/net/sock_reuseport.h
> > index 8a5f70c7cdf2..d9112de85261 100644
> > --- a/include/net/sock_reuseport.h
> > +++ b/include/net/sock_reuseport.h
> > @@ -35,6 +35,8 @@ extern struct sock *reuseport_select_sock(struct sock *sk,
> >                                           struct sk_buff *skb,
> >                                           int hdr_len);
> >  extern int reuseport_attach_prog(struct sock *sk, struct bpf_prog *prog);
> > +extern int reuseport_detach_prog(struct sock *sk);
> > +
> >  int reuseport_get_id(struct sock_reuseport *reuse);
> >
> >  #endif  /* _SOCK_REUSEPORT_H */
> > diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
> > index 8c1391c89171..77f7c1638eb1 100644
> > --- a/include/uapi/asm-generic/socket.h
> > +++ b/include/uapi/asm-generic/socket.h
> > @@ -117,6 +117,8 @@
> >  #define SO_RCVTIMEO_NEW         66
> >  #define SO_SNDTIMEO_NEW         67
> >
> > +#define SO_DETACH_REUSEPORT_BPF 68
> > +
> >  #if !defined(__KERNEL__)
> >
> >  #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
> > diff --git a/net/core/sock.c b/net/core/sock.c
> > index 75b1c950b49f..06be30737b69 100644
> > --- a/net/core/sock.c
> > +++ b/net/core/sock.c
> > @@ -1045,6 +1045,10 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
> >                 }
> >                 break;
> >
> > +       case SO_DETACH_REUSEPORT_BPF:
> > +               ret = reuseport_detach_prog(sk);
> > +               break;
> > +
> >         case SO_DETACH_FILTER:
> >                 ret = sk_detach_filter(sk);
> >                 break;
> > diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c
> > index dc4aefdf2a08..e0cb29469fa7 100644
> > --- a/net/core/sock_reuseport.c
> > +++ b/net/core/sock_reuseport.c
> > @@ -332,3 +332,27 @@ int reuseport_attach_prog(struct sock *sk, struct bpf_prog *prog)
> >         return 0;
> >  }
> >  EXPORT_SYMBOL(reuseport_attach_prog);
> > +
> > +int reuseport_detach_prog(struct sock *sk)
> > +{
> > +       struct sock_reuseport *reuse;
> > +       struct bpf_prog *old_prog;
> > +
> > +       if (!rcu_access_pointer(sk->sk_reuseport_cb))
> > +               return sk->sk_reuseport ? -ENOENT : -EINVAL;
> > +
> > +       spin_lock_bh(&reuseport_lock);
> > +       reuse = rcu_dereference_protected(sk->sk_reuseport_cb,
> > +                                         lockdep_is_held(&reuseport_lock));
> > +       old_prog = rcu_dereference_protected(reuse->prog,
> > +                                            lockdep_is_held(&reuseport_lock));
> > +       RCU_INIT_POINTER(reuse->prog, NULL);
> > +       spin_unlock_bh(&reuseport_lock);
> > +
> > +       if (!old_prog)
> > +               return -ENOENT;
> > +
> > +       sk_reuseport_prog_free(old_prog);
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL(reuseport_detach_prog);
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-06-13 21:19 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-12 19:05 [PATCH bpf-next 0/2] bpf: net: Detach BPF prog from reuseport sk Martin KaFai Lau
2019-06-12 19:05 ` [PATCH bpf-next 1/2] bpf: net: Add SO_DETACH_REUSEPORT_BPF Martin KaFai Lau
2019-06-13  4:39   ` Andrii Nakryiko
2019-06-13 19:18   ` kbuild test robot
2019-06-13 20:14   ` Andrii Nakryiko
2019-06-13 21:18     ` Martin Lau
2019-06-12 19:05 ` [PATCH bpf-next 2/2] bpf: Add test for SO_REUSEPORT_DETACH_BPF Martin KaFai Lau
2019-06-12 19:59   ` Stanislav Fomichev
2019-06-12 21:30     ` Martin Lau
2019-06-12 21:47       ` Stanislav Fomichev
2019-06-12 21:53         ` Alexei Starovoitov
2019-06-12 22:15           ` Martin Lau
2019-06-12 22:25             ` Alexei Starovoitov
2019-06-12 22:39               ` Martin Lau
2019-06-12 21:47     ` Martin Lau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.