bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next v2 1/4] bpf: add BPF_CGROUP_INET_SOCK_RELEASE hook
@ 2020-06-26 16:52 Stanislav Fomichev
  2020-06-26 16:52 ` [PATCH bpf-next v2 2/4] libbpf: add support for BPF_CGROUP_INET_SOCK_RELEASE Stanislav Fomichev
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Stanislav Fomichev @ 2020-06-26 16:52 UTC (permalink / raw)
  To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev

Sometimes it's handy to know when the socket gets freed.
In particular, we'd like to try to use a smarter allocation
of ports for bpf_bind and explore the possibility of
limiting the number of SOCK_DGRAM sockets the process can have.
Adding a release pair to existing BPF_CGROUP_INET_SOCK_CREATE
can unlock both of the mentioned features.

The only questionable part here is the sock->sk check
in the inet_release. Looking at the places where we
do 'sock->sk = NULL', I don't understand how it can race
with inet_release and why the check is there (it's been
there since the initial git import). Otherwise, the
change itself is pretty simple, we add a BPF hook
to the inet_release and avoid calling it for kernel
sockets.

v2:
* fix compile issue with CONFIG_CGROUP_BPF=n (kernel test robot)

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 include/linux/bpf-cgroup.h | 4 ++++
 include/uapi/linux/bpf.h   | 1 +
 kernel/bpf/syscall.c       | 3 +++
 net/core/filter.c          | 1 +
 net/ipv4/af_inet.c         | 3 +++
 5 files changed, 12 insertions(+)

diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index c66c545e161a..2c6f26670acc 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -210,6 +210,9 @@ int bpf_percpu_cgroup_storage_update(struct bpf_map *map, void *key,
 #define BPF_CGROUP_RUN_PROG_INET_SOCK(sk)				       \
 	BPF_CGROUP_RUN_SK_PROG(sk, BPF_CGROUP_INET_SOCK_CREATE)
 
+#define BPF_CGROUP_RUN_PROG_INET_SOCK_RELEASE(sk)			       \
+	BPF_CGROUP_RUN_SK_PROG(sk, BPF_CGROUP_INET_SOCK_RELEASE)
+
 #define BPF_CGROUP_RUN_PROG_INET4_POST_BIND(sk)				       \
 	BPF_CGROUP_RUN_SK_PROG(sk, BPF_CGROUP_INET4_POST_BIND)
 
@@ -401,6 +404,7 @@ static inline int bpf_percpu_cgroup_storage_update(struct bpf_map *map,
 #define BPF_CGROUP_RUN_PROG_INET_INGRESS(sk,skb) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_INET_EGRESS(sk,skb) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_INET_SOCK(sk) ({ 0; })
+#define BPF_CGROUP_RUN_PROG_INET_SOCK_RELEASE(sk) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_INET4_BIND(sk, uaddr) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_INET6_BIND(sk, uaddr) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_INET4_POST_BIND(sk) ({ 0; })
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index c65b374a5090..d7aea1d0167a 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -226,6 +226,7 @@ enum bpf_attach_type {
 	BPF_CGROUP_INET4_GETSOCKNAME,
 	BPF_CGROUP_INET6_GETSOCKNAME,
 	BPF_XDP_DEVMAP,
+	BPF_CGROUP_INET_SOCK_RELEASE,
 	__MAX_BPF_ATTACH_TYPE
 };
 
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 4d530b1d5683..2a3d4b8f95c7 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1994,6 +1994,7 @@ bpf_prog_load_check_attach(enum bpf_prog_type prog_type,
 	case BPF_PROG_TYPE_CGROUP_SOCK:
 		switch (expected_attach_type) {
 		case BPF_CGROUP_INET_SOCK_CREATE:
+		case BPF_CGROUP_INET_SOCK_RELEASE:
 		case BPF_CGROUP_INET4_POST_BIND:
 		case BPF_CGROUP_INET6_POST_BIND:
 			return 0;
@@ -2792,6 +2793,7 @@ attach_type_to_prog_type(enum bpf_attach_type attach_type)
 		return BPF_PROG_TYPE_CGROUP_SKB;
 		break;
 	case BPF_CGROUP_INET_SOCK_CREATE:
+	case BPF_CGROUP_INET_SOCK_RELEASE:
 	case BPF_CGROUP_INET4_POST_BIND:
 	case BPF_CGROUP_INET6_POST_BIND:
 		return BPF_PROG_TYPE_CGROUP_SOCK;
@@ -2942,6 +2944,7 @@ static int bpf_prog_query(const union bpf_attr *attr,
 	case BPF_CGROUP_INET_INGRESS:
 	case BPF_CGROUP_INET_EGRESS:
 	case BPF_CGROUP_INET_SOCK_CREATE:
+	case BPF_CGROUP_INET_SOCK_RELEASE:
 	case BPF_CGROUP_INET4_BIND:
 	case BPF_CGROUP_INET6_BIND:
 	case BPF_CGROUP_INET4_POST_BIND:
diff --git a/net/core/filter.c b/net/core/filter.c
index 209482a4eaa2..7bcac182383c 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -6855,6 +6855,7 @@ static bool __sock_filter_check_attach_type(int off,
 	case offsetof(struct bpf_sock, priority):
 		switch (attach_type) {
 		case BPF_CGROUP_INET_SOCK_CREATE:
+		case BPF_CGROUP_INET_SOCK_RELEASE:
 			goto full_access;
 		default:
 			return false;
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 02aa5cb3a4fd..965a96ea1168 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -411,6 +411,9 @@ int inet_release(struct socket *sock)
 	if (sk) {
 		long timeout;
 
+		if (!sk->sk_kern_sock)
+			BPF_CGROUP_RUN_PROG_INET_SOCK_RELEASE(sk);
+
 		/* Applications forget to leave groups before exiting */
 		ip_mc_drop_socket(sk);
 
-- 
2.27.0.212.ge8ba1cc988-goog


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

* [PATCH bpf-next v2 2/4] libbpf: add support for BPF_CGROUP_INET_SOCK_RELEASE
  2020-06-26 16:52 [PATCH bpf-next v2 1/4] bpf: add BPF_CGROUP_INET_SOCK_RELEASE hook Stanislav Fomichev
@ 2020-06-26 16:52 ` Stanislav Fomichev
  2020-06-26 22:08   ` Andrii Nakryiko
  2020-06-26 16:52 ` [PATCH bpf-next v2 3/4] bpftool: support BPF_CGROUP_INET_SOCK_RELEASE Stanislav Fomichev
  2020-06-26 16:52 ` [PATCH bpf-next v2 4/4] selftests/bpf: test BPF_CGROUP_INET_SOCK_RELEASE Stanislav Fomichev
  2 siblings, 1 reply; 10+ messages in thread
From: Stanislav Fomichev @ 2020-06-26 16:52 UTC (permalink / raw)
  To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev

Add auto-detection for the cgroup/sock_release programs.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 tools/include/uapi/linux/bpf.h | 1 +
 tools/lib/bpf/libbpf.c         | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index c65b374a5090..d7aea1d0167a 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -226,6 +226,7 @@ enum bpf_attach_type {
 	BPF_CGROUP_INET4_GETSOCKNAME,
 	BPF_CGROUP_INET6_GETSOCKNAME,
 	BPF_XDP_DEVMAP,
+	BPF_CGROUP_INET_SOCK_RELEASE,
 	__MAX_BPF_ATTACH_TYPE
 };
 
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 7f01be2b88b8..acbab6d0672d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6670,6 +6670,8 @@ static const struct bpf_sec_def section_defs[] = {
 	BPF_APROG_SEC("cgroup_skb/egress",	BPF_PROG_TYPE_CGROUP_SKB,
 						BPF_CGROUP_INET_EGRESS),
 	BPF_APROG_COMPAT("cgroup/skb",		BPF_PROG_TYPE_CGROUP_SKB),
+	BPF_EAPROG_SEC("cgroup/sock_release",	BPF_PROG_TYPE_CGROUP_SOCK,
+						BPF_CGROUP_INET_SOCK_RELEASE),
 	BPF_APROG_SEC("cgroup/sock",		BPF_PROG_TYPE_CGROUP_SOCK,
 						BPF_CGROUP_INET_SOCK_CREATE),
 	BPF_EAPROG_SEC("cgroup/post_bind4",	BPF_PROG_TYPE_CGROUP_SOCK,
-- 
2.27.0.212.ge8ba1cc988-goog


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

* [PATCH bpf-next v2 3/4] bpftool: support BPF_CGROUP_INET_SOCK_RELEASE
  2020-06-26 16:52 [PATCH bpf-next v2 1/4] bpf: add BPF_CGROUP_INET_SOCK_RELEASE hook Stanislav Fomichev
  2020-06-26 16:52 ` [PATCH bpf-next v2 2/4] libbpf: add support for BPF_CGROUP_INET_SOCK_RELEASE Stanislav Fomichev
@ 2020-06-26 16:52 ` Stanislav Fomichev
  2020-06-26 23:07   ` Daniel Borkmann
  2020-06-26 16:52 ` [PATCH bpf-next v2 4/4] selftests/bpf: test BPF_CGROUP_INET_SOCK_RELEASE Stanislav Fomichev
  2 siblings, 1 reply; 10+ messages in thread
From: Stanislav Fomichev @ 2020-06-26 16:52 UTC (permalink / raw)
  To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev

Support attaching to sock_release from the bpftool.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 tools/bpf/bpftool/main.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 5cdf0bc049bd..0a281d3cceb8 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -92,6 +92,7 @@ static const char * const attach_type_name[__MAX_BPF_ATTACH_TYPE] = {
 	[BPF_CGROUP_INET_INGRESS] = "ingress",
 	[BPF_CGROUP_INET_EGRESS] = "egress",
 	[BPF_CGROUP_INET_SOCK_CREATE] = "sock_create",
+	[BPF_CGROUP_INET_SOCK_RELEASE] = "sock_release",
 	[BPF_CGROUP_SOCK_OPS] = "sock_ops",
 	[BPF_CGROUP_DEVICE] = "device",
 	[BPF_CGROUP_INET4_BIND] = "bind4",
-- 
2.27.0.212.ge8ba1cc988-goog


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

* [PATCH bpf-next v2 4/4] selftests/bpf: test BPF_CGROUP_INET_SOCK_RELEASE
  2020-06-26 16:52 [PATCH bpf-next v2 1/4] bpf: add BPF_CGROUP_INET_SOCK_RELEASE hook Stanislav Fomichev
  2020-06-26 16:52 ` [PATCH bpf-next v2 2/4] libbpf: add support for BPF_CGROUP_INET_SOCK_RELEASE Stanislav Fomichev
  2020-06-26 16:52 ` [PATCH bpf-next v2 3/4] bpftool: support BPF_CGROUP_INET_SOCK_RELEASE Stanislav Fomichev
@ 2020-06-26 16:52 ` Stanislav Fomichev
  2 siblings, 0 replies; 10+ messages in thread
From: Stanislav Fomichev @ 2020-06-26 16:52 UTC (permalink / raw)
  To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev

Simple test that enforces a single SOCK_DGRAM socker per cgroup.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 .../selftests/bpf/prog_tests/udp_limit.c      | 71 +++++++++++++++++++
 tools/testing/selftests/bpf/progs/udp_limit.c | 42 +++++++++++
 2 files changed, 113 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/udp_limit.c
 create mode 100644 tools/testing/selftests/bpf/progs/udp_limit.c

diff --git a/tools/testing/selftests/bpf/prog_tests/udp_limit.c b/tools/testing/selftests/bpf/prog_tests/udp_limit.c
new file mode 100644
index 000000000000..fe359a927d92
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/udp_limit.c
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include "udp_limit.skel.h"
+
+#include <sys/types.h>
+#include <sys/socket.h>
+
+void test_udp_limit(void)
+{
+	struct udp_limit *skel;
+	int cgroup_fd;
+	int fd1, fd2;
+	int err;
+
+	cgroup_fd = test__join_cgroup("/udp_limit");
+	if (CHECK_FAIL(cgroup_fd < 0))
+		return;
+
+	skel = udp_limit__open_and_load();
+	if (CHECK_FAIL(!skel))
+		goto close_cgroup_fd;
+
+	err = bpf_prog_attach(bpf_program__fd(skel->progs.sock),
+			      cgroup_fd, BPF_CGROUP_INET_SOCK_CREATE, 0);
+	if (CHECK_FAIL(err))
+		goto close_skeleton;
+
+	err = bpf_prog_attach(bpf_program__fd(skel->progs.sock_release),
+			      cgroup_fd, BPF_CGROUP_INET_SOCK_RELEASE, 0);
+	if (CHECK_FAIL(err))
+		goto close_skeleton;
+
+	/* BPF program enforces a single UDP socket per cgroup,
+	 * verify that.
+	 */
+	fd1 = socket(AF_INET, SOCK_DGRAM, 0);
+	if (CHECK_FAIL(fd1 < 0))
+		goto close_skeleton;
+
+	fd2 = socket(AF_INET, SOCK_DGRAM, 0);
+	if (CHECK_FAIL(fd2 != -1))
+		goto close_fd1;
+
+	/* We can reopen again after close. */
+	close(fd1);
+
+	fd1 = socket(AF_INET, SOCK_DGRAM, 0);
+	if (CHECK_FAIL(fd1 < 0))
+		goto close_skeleton;
+
+	/* Make sure the program was invoked the expected
+	 * number of times:
+	 * - open fd1           - BPF_CGROUP_INET_SOCK_CREATE
+	 * - attempt to openfd2 - BPF_CGROUP_INET_SOCK_CREATE
+	 * - close fd1          - BPF_CGROUP_INET_SOCK_RELEASE
+	 * - open fd1 again     - BPF_CGROUP_INET_SOCK_CREATE
+	 */
+	if (CHECK_FAIL(skel->bss->invocations != 4))
+		goto close_fd1;
+
+	/* We should still have a single socket in use */
+	if (CHECK_FAIL(skel->bss->in_use != 1))
+		goto close_fd1;
+
+close_fd1:
+	close(fd1);
+close_skeleton:
+	udp_limit__destroy(skel);
+close_cgroup_fd:
+	close(cgroup_fd);
+}
diff --git a/tools/testing/selftests/bpf/progs/udp_limit.c b/tools/testing/selftests/bpf/progs/udp_limit.c
new file mode 100644
index 000000000000..98fe294d9c21
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/udp_limit.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <sys/socket.h>
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+int invocations, in_use;
+
+SEC("cgroup/sock")
+int sock(struct bpf_sock *ctx)
+{
+	__u32 key;
+
+	if (ctx->type != SOCK_DGRAM)
+		return 1;
+
+	__sync_fetch_and_add(&invocations, 1);
+
+	if (&in_use > 0) {
+		/* BPF_CGROUP_INET_SOCK_RELEASE is _not_ called
+		 * when we return an error from the BPF
+		 * program!
+		 */
+		return 0;
+	}
+
+	__sync_fetch_and_add(&in_use, 1);
+	return 1;
+}
+
+SEC("cgroup/sock_release")
+int sock_release(struct bpf_sock *ctx)
+{
+	__u32 key;
+
+	if (ctx->type != SOCK_DGRAM)
+		return 1;
+
+	__sync_fetch_and_add(&invocations, 1);
+	__sync_fetch_and_add(&in_use, -1);
+	return 1;
+}
-- 
2.27.0.212.ge8ba1cc988-goog


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

* Re: [PATCH bpf-next v2 2/4] libbpf: add support for BPF_CGROUP_INET_SOCK_RELEASE
  2020-06-26 16:52 ` [PATCH bpf-next v2 2/4] libbpf: add support for BPF_CGROUP_INET_SOCK_RELEASE Stanislav Fomichev
@ 2020-06-26 22:08   ` Andrii Nakryiko
  2020-06-26 23:52     ` Stanislav Fomichev
  0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2020-06-26 22:08 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Networking, bpf, David S. Miller, Alexei Starovoitov, Daniel Borkmann

On Fri, Jun 26, 2020 at 10:22 AM Stanislav Fomichev <sdf@google.com> wrote:
>
> Add auto-detection for the cgroup/sock_release programs.
>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---

Left a bunch of comments on v1 of this series, oops.

But where is the cover letter? Did I miss it?

>  tools/include/uapi/linux/bpf.h | 1 +
>  tools/lib/bpf/libbpf.c         | 2 ++
>  2 files changed, 3 insertions(+)
>

[...]

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

* Re: [PATCH bpf-next v2 3/4] bpftool: support BPF_CGROUP_INET_SOCK_RELEASE
  2020-06-26 16:52 ` [PATCH bpf-next v2 3/4] bpftool: support BPF_CGROUP_INET_SOCK_RELEASE Stanislav Fomichev
@ 2020-06-26 23:07   ` Daniel Borkmann
  2020-06-26 23:52     ` Stanislav Fomichev
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Borkmann @ 2020-06-26 23:07 UTC (permalink / raw)
  To: Stanislav Fomichev, netdev, bpf; +Cc: davem, ast

On 6/26/20 6:52 PM, Stanislav Fomichev wrote:
> Support attaching to sock_release from the bpftool.
> 
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
>   tools/bpf/bpftool/main.h | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> index 5cdf0bc049bd..0a281d3cceb8 100644
> --- a/tools/bpf/bpftool/main.h
> +++ b/tools/bpf/bpftool/main.h
> @@ -92,6 +92,7 @@ static const char * const attach_type_name[__MAX_BPF_ATTACH_TYPE] = {
>   	[BPF_CGROUP_INET_INGRESS] = "ingress",
>   	[BPF_CGROUP_INET_EGRESS] = "egress",
>   	[BPF_CGROUP_INET_SOCK_CREATE] = "sock_create",
> +	[BPF_CGROUP_INET_SOCK_RELEASE] = "sock_release",
>   	[BPF_CGROUP_SOCK_OPS] = "sock_ops",
>   	[BPF_CGROUP_DEVICE] = "device",
>   	[BPF_CGROUP_INET4_BIND] = "bind4",

This one is not on latest bpf-next, needs rebase due to 16d37ee3d2b1 ("tools, bpftool: Define
attach_type_name array only once").

Thanks,
Daniel

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

* Re: [PATCH bpf-next v2 2/4] libbpf: add support for BPF_CGROUP_INET_SOCK_RELEASE
  2020-06-26 22:08   ` Andrii Nakryiko
@ 2020-06-26 23:52     ` Stanislav Fomichev
  2020-06-27  0:59       ` Andrii Nakryiko
  0 siblings, 1 reply; 10+ messages in thread
From: Stanislav Fomichev @ 2020-06-26 23:52 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Networking, bpf, David S. Miller, Alexei Starovoitov, Daniel Borkmann

On Fri, Jun 26, 2020 at 3:08 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Jun 26, 2020 at 10:22 AM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > Add auto-detection for the cgroup/sock_release programs.
> >
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
>
> Left a bunch of comments on v1 of this series, oops.
No worries, there is not much changed since then, thanks for the review!

> But where is the cover letter? Did I miss it?
I didn't feel like it deserves a cover letter. The main patch is
small, most of the changes are in libbpf/selftests and I mostly split
them to make it easier for you to push to github (is it still a
requirement or I shouldn't worry about it anymore?).

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

* Re: [PATCH bpf-next v2 3/4] bpftool: support BPF_CGROUP_INET_SOCK_RELEASE
  2020-06-26 23:07   ` Daniel Borkmann
@ 2020-06-26 23:52     ` Stanislav Fomichev
  0 siblings, 0 replies; 10+ messages in thread
From: Stanislav Fomichev @ 2020-06-26 23:52 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: Netdev, bpf, David Miller, Alexei Starovoitov

On Fri, Jun 26, 2020 at 4:08 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 6/26/20 6:52 PM, Stanislav Fomichev wrote:
> > Support attaching to sock_release from the bpftool.
> >
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> >   tools/bpf/bpftool/main.h | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> > index 5cdf0bc049bd..0a281d3cceb8 100644
> > --- a/tools/bpf/bpftool/main.h
> > +++ b/tools/bpf/bpftool/main.h
> > @@ -92,6 +92,7 @@ static const char * const attach_type_name[__MAX_BPF_ATTACH_TYPE] = {
> >       [BPF_CGROUP_INET_INGRESS] = "ingress",
> >       [BPF_CGROUP_INET_EGRESS] = "egress",
> >       [BPF_CGROUP_INET_SOCK_CREATE] = "sock_create",
> > +     [BPF_CGROUP_INET_SOCK_RELEASE] = "sock_release",
> >       [BPF_CGROUP_SOCK_OPS] = "sock_ops",
> >       [BPF_CGROUP_DEVICE] = "device",
> >       [BPF_CGROUP_INET4_BIND] = "bind4",
>
> This one is not on latest bpf-next, needs rebase due to 16d37ee3d2b1 ("tools, bpftool: Define
> attach_type_name array only once").
Sure, will follow up with a v3 to address Andrii's suggestions + will
rebase on top of the latest bpf-next!

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

* Re: [PATCH bpf-next v2 2/4] libbpf: add support for BPF_CGROUP_INET_SOCK_RELEASE
  2020-06-26 23:52     ` Stanislav Fomichev
@ 2020-06-27  0:59       ` Andrii Nakryiko
  2020-06-28 17:35         ` Alexei Starovoitov
  0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2020-06-27  0:59 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Networking, bpf, David S. Miller, Alexei Starovoitov, Daniel Borkmann

On Fri, Jun 26, 2020 at 4:52 PM Stanislav Fomichev <sdf@google.com> wrote:
>
> On Fri, Jun 26, 2020 at 3:08 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Fri, Jun 26, 2020 at 10:22 AM Stanislav Fomichev <sdf@google.com> wrote:
> > >
> > > Add auto-detection for the cgroup/sock_release programs.
> > >
> > > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > > ---
> >
> > Left a bunch of comments on v1 of this series, oops.
> No worries, there is not much changed since then, thanks for the review!
>
> > But where is the cover letter? Did I miss it?
> I didn't feel like it deserves a cover letter. The main patch is
> small, most of the changes are in libbpf/selftests and I mostly split
> them to make it easier for you to push to github (is it still a
> requirement or I shouldn't worry about it anymore?).

It's not a requirement, sync script is pretty smart about all that, so
no need to do it specifically for the sync purposes. Still a good idea
to have selftests separately, IMO.

As for the cover letter, unless it's some trivial independent fixes,
cover letter ties everything together and is a good place to track
"what changed between versions". So I'd recommend adding it anyways.

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

* Re: [PATCH bpf-next v2 2/4] libbpf: add support for BPF_CGROUP_INET_SOCK_RELEASE
  2020-06-27  0:59       ` Andrii Nakryiko
@ 2020-06-28 17:35         ` Alexei Starovoitov
  0 siblings, 0 replies; 10+ messages in thread
From: Alexei Starovoitov @ 2020-06-28 17:35 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Stanislav Fomichev, Networking, bpf, David S. Miller,
	Alexei Starovoitov, Daniel Borkmann

On Fri, Jun 26, 2020 at 05:59:41PM -0700, Andrii Nakryiko wrote:
> 
> As for the cover letter, unless it's some trivial independent fixes,
> cover letter ties everything together and is a good place to track
> "what changed between versions". So I'd recommend adding it anyways.

+1
For independent fixes it's ok to skip cover, but it's often nice to have one.
New release hook like this one certainly deserves a cover.

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

end of thread, other threads:[~2020-06-28 17:35 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-26 16:52 [PATCH bpf-next v2 1/4] bpf: add BPF_CGROUP_INET_SOCK_RELEASE hook Stanislav Fomichev
2020-06-26 16:52 ` [PATCH bpf-next v2 2/4] libbpf: add support for BPF_CGROUP_INET_SOCK_RELEASE Stanislav Fomichev
2020-06-26 22:08   ` Andrii Nakryiko
2020-06-26 23:52     ` Stanislav Fomichev
2020-06-27  0:59       ` Andrii Nakryiko
2020-06-28 17:35         ` Alexei Starovoitov
2020-06-26 16:52 ` [PATCH bpf-next v2 3/4] bpftool: support BPF_CGROUP_INET_SOCK_RELEASE Stanislav Fomichev
2020-06-26 23:07   ` Daniel Borkmann
2020-06-26 23:52     ` Stanislav Fomichev
2020-06-26 16:52 ` [PATCH bpf-next v2 4/4] selftests/bpf: test BPF_CGROUP_INET_SOCK_RELEASE Stanislav Fomichev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).