[PATCH v2 bpf-next 0/4] Support bpf_get_socket_cookie in more prog types

[PATCH v2 bpf-next 0/4] Support bpf_get_socket_cookie in more prog types

[Andrey Ignatov]

v1->v2:
- simplify attach_type guessing logic in selftest.

This patch set adds support to call bpf_get_socket_cookie() helper from two
more program types:
* BPF_PROG_TYPE_CGROUP_SOCK_ADDR;
* BPF_PROG_TYPE_SOCK_OPS.

Patch 1 adds kernel support and provides more details.
Patch 2 syncs UAPI changes to tools/.
Patch 3 adds missing helper to bpf_helpers.h.
Patch 4 adds selftest for new functionality.


Andrey Ignatov (4):
  bpf: Support bpf_get_socket_cookie in more prog types
  bpf: Sync bpf.h to tools/
  selftests/bpf: Add bpf_get_socket_cookie to bpf_helpers.h
  selftests/bpf: Test for get_socket_cookie

 include/uapi/linux/bpf.h                      |  14 ++
 net/core/filter.c                             |  28 +++
 tools/include/uapi/linux/bpf.h                |  14 ++
 tools/testing/selftests/bpf/Makefile          |   6 +-
 tools/testing/selftests/bpf/bpf_helpers.h     |   2 +
 .../selftests/bpf/socket_cookie_prog.c        |  60 +++++
 .../selftests/bpf/test_socket_cookie.c        | 225 ++++++++++++++++++
 7 files changed, 347 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/socket_cookie_prog.c
 create mode 100644 tools/testing/selftests/bpf/test_socket_cookie.c

-- 
2.17.1

[PATCH v2 bpf-next 1/4] bpf: Support bpf_get_socket_cookie in more prog types

[Andrey Ignatov]

bpf_get_socket_cookie() helper can be used to identify skb that
correspond to the same socket.

Though socket cookie can be useful in many other use-cases where socket is
available in program context. Specifically BPF_PROG_TYPE_CGROUP_SOCK_ADDR
and BPF_PROG_TYPE_SOCK_OPS programs can benefit from it so that one of
them can augment a value in a map prepared earlier by other program for
the same socket.

The patch adds support to call bpf_get_socket_cookie() from
BPF_PROG_TYPE_CGROUP_SOCK_ADDR and BPF_PROG_TYPE_SOCK_OPS.

It doesn't introduce new helpers. Instead it reuses same helper name
bpf_get_socket_cookie() but adds support to this helper to accept
`struct bpf_sock_addr` and `struct bpf_sock_ops`.

Documentation in bpf.h is changed in a way that should not break
automatic generation of markdown.

Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
 include/uapi/linux/bpf.h | 14 ++++++++++++++
 net/core/filter.c        | 28 ++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 870113916cac..0ebaaf7f3568 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1371,6 +1371,20 @@ union bpf_attr {
  * 		A 8-byte long non-decreasing number on success, or 0 if the
  * 		socket field is missing inside *skb*.
  *
+ * u64 bpf_get_socket_cookie(struct bpf_sock_addr *ctx)
+ * 	Description
+ * 		Equivalent to bpf_get_socket_cookie() helper that accepts
+ * 		*skb*, but gets socket from **struct bpf_sock_addr** contex.
+ * 	Return
+ * 		A 8-byte long non-decreasing number.
+ *
+ * u64 bpf_get_socket_cookie(struct bpf_sock_ops *ctx)
+ * 	Description
+ * 		Equivalent to bpf_get_socket_cookie() helper that accepts
+ * 		*skb*, but gets socket from **struct bpf_sock_ops** contex.
+ * 	Return
+ * 		A 8-byte long non-decreasing number.
+ *
  * u32 bpf_get_socket_uid(struct sk_buff *skb)
  * 	Return
  * 		The owner UID of the socket associated to *skb*. If the socket
diff --git a/net/core/filter.c b/net/core/filter.c
index 104d560946da..0b3c59468282 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3812,6 +3812,30 @@ static const struct bpf_func_proto bpf_get_socket_cookie_proto = {
 	.arg1_type      = ARG_PTR_TO_CTX,
 };
 
+BPF_CALL_1(bpf_get_socket_cookie_sock_addr, struct bpf_sock_addr_kern *, ctx)
+{
+	return sock_gen_cookie(ctx->sk);
+}
+
+static const struct bpf_func_proto bpf_get_socket_cookie_sock_addr_proto = {
+	.func		= bpf_get_socket_cookie_sock_addr,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+};
+
+BPF_CALL_1(bpf_get_socket_cookie_sock_ops, struct bpf_sock_ops_kern *, ctx)
+{
+	return sock_gen_cookie(ctx->sk);
+}
+
+static const struct bpf_func_proto bpf_get_socket_cookie_sock_ops_proto = {
+	.func		= bpf_get_socket_cookie_sock_ops,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+};
+
 BPF_CALL_1(bpf_get_socket_uid, struct sk_buff *, skb)
 {
 	struct sock *sk = sk_to_full_sk(skb->sk);
@@ -4788,6 +4812,8 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		default:
 			return NULL;
 		}
+	case BPF_FUNC_get_socket_cookie:
+		return &bpf_get_socket_cookie_sock_addr_proto;
 	default:
 		return bpf_base_func_proto(func_id);
 	}
@@ -4930,6 +4956,8 @@ sock_ops_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_sock_map_update_proto;
 	case BPF_FUNC_sock_hash_update:
 		return &bpf_sock_hash_update_proto;
+	case BPF_FUNC_get_socket_cookie:
+		return &bpf_get_socket_cookie_sock_ops_proto;
 	default:
 		return bpf_base_func_proto(func_id);
 	}
-- 
2.17.1

[PATCH v2 bpf-next 2/4] bpf: Sync bpf.h to tools/

[Andrey Ignatov]

Sync bpf_get_socket_cookie() related bpf UAPI changes to tools/.

Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
 tools/include/uapi/linux/bpf.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 870113916cac..0ebaaf7f3568 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1371,6 +1371,20 @@ union bpf_attr {
  * 		A 8-byte long non-decreasing number on success, or 0 if the
  * 		socket field is missing inside *skb*.
  *
+ * u64 bpf_get_socket_cookie(struct bpf_sock_addr *ctx)
+ * 	Description
+ * 		Equivalent to bpf_get_socket_cookie() helper that accepts
+ * 		*skb*, but gets socket from **struct bpf_sock_addr** contex.
+ * 	Return
+ * 		A 8-byte long non-decreasing number.
+ *
+ * u64 bpf_get_socket_cookie(struct bpf_sock_ops *ctx)
+ * 	Description
+ * 		Equivalent to bpf_get_socket_cookie() helper that accepts
+ * 		*skb*, but gets socket from **struct bpf_sock_ops** contex.
+ * 	Return
+ * 		A 8-byte long non-decreasing number.
+ *
  * u32 bpf_get_socket_uid(struct sk_buff *skb)
  * 	Return
  * 		The owner UID of the socket associated to *skb*. If the socket
-- 
2.17.1

[PATCH v2 bpf-next 3/4] selftests/bpf: Add bpf_get_socket_cookie to bpf_helpers.h

[Andrey Ignatov]

Add missing helper to bpf_helpers.h that is used in tests and samples.

Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
 tools/testing/selftests/bpf/bpf_helpers.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index f2f28b6c8915..19a424483f6e 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -65,6 +65,8 @@ static int (*bpf_xdp_adjust_head)(void *ctx, int offset) =
 	(void *) BPF_FUNC_xdp_adjust_head;
 static int (*bpf_xdp_adjust_meta)(void *ctx, int offset) =
 	(void *) BPF_FUNC_xdp_adjust_meta;
+static int (*bpf_get_socket_cookie)(void *ctx) =
+	(void *) BPF_FUNC_get_socket_cookie;
 static int (*bpf_setsockopt)(void *ctx, int level, int optname, void *optval,
 			     int optlen) =
 	(void *) BPF_FUNC_setsockopt;
-- 
2.17.1

[PATCH v2 bpf-next 4/4] selftests/bpf: Test for get_socket_cookie

[Andrey Ignatov]

Add test to use get_socket_cookie() from BPF programs of types
BPF_PROG_TYPE_SOCK_OPS and BPF_PROG_TYPE_CGROUP_SOCK_ADDR.

The test attaches two programs to cgroup, runs TCP server and client in
the cgroup and checks that two operations are done properly on client
socket when user calls connect(2):

1. In BPF_CGROUP_INET6_CONNECT socket cookie is used as the key to write
   new value in a map for client socket.

2. In BPF_CGROUP_SOCK_OPS (BPF_SOCK_OPS_TCP_CONNECT_CB callback) the
   value written in "1." is found by socket cookie, since it's the same
   socket, and updated.

Finally the test verifies the value in the map.

Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
 tools/testing/selftests/bpf/Makefile          |   6 +-
 .../selftests/bpf/socket_cookie_prog.c        |  60 +++++
 .../selftests/bpf/test_socket_cookie.c        | 225 ++++++++++++++++++
 3 files changed, 289 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/socket_cookie_prog.c
 create mode 100644 tools/testing/selftests/bpf/test_socket_cookie.c

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 478bf1bcbbf5..1b28277998e2 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -22,7 +22,8 @@ $(TEST_CUSTOM_PROGS): $(OUTPUT)/%: %.c
 # Order correspond to 'make run_tests' order
 TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \
 	test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \
-	test_sock test_btf test_sockmap test_lirc_mode2_user get_cgroup_id_user
+	test_sock test_btf test_sockmap test_lirc_mode2_user get_cgroup_id_user \
+	test_socket_cookie
 
 TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test_obj_id.o \
 	test_pkt_md_access.o test_xdp_redirect.o test_xdp_meta.o sockmap_parse_prog.o     \
@@ -33,7 +34,7 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
 	test_btf_haskv.o test_btf_nokv.o test_sockmap_kern.o test_tunnel_kern.o \
 	test_get_stack_rawtp.o test_sockmap_kern.o test_sockhash_kern.o \
 	test_lwt_seg6local.o sendmsg4_prog.o sendmsg6_prog.o test_lirc_mode2_kern.o \
-	get_cgroup_id_kern.o
+	get_cgroup_id_kern.o socket_cookie_prog.o
 
 # Order correspond to 'make run_tests' order
 TEST_PROGS := test_kmod.sh \
@@ -60,6 +61,7 @@ $(TEST_GEN_PROGS_EXTENDED): $(OUTPUT)/libbpf.a
 $(OUTPUT)/test_dev_cgroup: cgroup_helpers.c
 $(OUTPUT)/test_sock: cgroup_helpers.c
 $(OUTPUT)/test_sock_addr: cgroup_helpers.c
+$(OUTPUT)/test_socket_cookie: cgroup_helpers.c
 $(OUTPUT)/test_sockmap: cgroup_helpers.c
 $(OUTPUT)/test_tcpbpf_user: cgroup_helpers.c
 $(OUTPUT)/test_progs: trace_helpers.c
diff --git a/tools/testing/selftests/bpf/socket_cookie_prog.c b/tools/testing/selftests/bpf/socket_cookie_prog.c
new file mode 100644
index 000000000000..9ff8ac4b0bf6
--- /dev/null
+++ b/tools/testing/selftests/bpf/socket_cookie_prog.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Facebook
+
+#include <linux/bpf.h>
+#include <sys/socket.h>
+
+#include "bpf_helpers.h"
+#include "bpf_endian.h"
+
+struct bpf_map_def SEC("maps") socket_cookies = {
+	.type = BPF_MAP_TYPE_HASH,
+	.key_size = sizeof(__u64),
+	.value_size = sizeof(__u32),
+	.max_entries = 1 << 8,
+};
+
+SEC("cgroup/connect6")
+int set_cookie(struct bpf_sock_addr *ctx)
+{
+	__u32 cookie_value = 0xFF;
+	__u64 cookie_key;
+
+	if (ctx->family != AF_INET6 || ctx->user_family != AF_INET6)
+		return 1;
+
+	cookie_key = bpf_get_socket_cookie(ctx);
+	if (bpf_map_update_elem(&socket_cookies, &cookie_key, &cookie_value, 0))
+		return 0;
+
+	return 1;
+}
+
+SEC("sockops")
+int update_cookie(struct bpf_sock_ops *ctx)
+{
+	__u32 new_cookie_value;
+	__u32 *cookie_value;
+	__u64 cookie_key;
+
+	if (ctx->family != AF_INET6)
+		return 1;
+
+	if (ctx->op != BPF_SOCK_OPS_TCP_CONNECT_CB)
+		return 1;
+
+	cookie_key = bpf_get_socket_cookie(ctx);
+
+	cookie_value = bpf_map_lookup_elem(&socket_cookies, &cookie_key);
+	if (!cookie_value)
+		return 1;
+
+	new_cookie_value = (ctx->local_port << 8) | *cookie_value;
+	bpf_map_update_elem(&socket_cookies, &cookie_key, &new_cookie_value, 0);
+
+	return 1;
+}
+
+int _version SEC("version") = 1;
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_socket_cookie.c b/tools/testing/selftests/bpf/test_socket_cookie.c
new file mode 100644
index 000000000000..68e108e4687a
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_socket_cookie.c
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Facebook
+
+#include <string.h>
+#include <unistd.h>
+
+#include <arpa/inet.h>
+#include <netinet/in.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#include "bpf_rlimit.h"
+#include "cgroup_helpers.h"
+
+#define CG_PATH			"/foo"
+#define SOCKET_COOKIE_PROG	"./socket_cookie_prog.o"
+
+static int start_server(void)
+{
+	struct sockaddr_in6 addr;
+	int fd;
+
+	fd = socket(AF_INET6, SOCK_STREAM, 0);
+	if (fd == -1) {
+		log_err("Failed to create server socket");
+		goto out;
+	}
+
+	memset(&addr, 0, sizeof(addr));
+	addr.sin6_family = AF_INET6;
+	addr.sin6_addr = in6addr_loopback;
+	addr.sin6_port = 0;
+
+	if (bind(fd, (const struct sockaddr *)&addr, sizeof(addr)) == -1) {
+		log_err("Failed to bind server socket");
+		goto close_out;
+	}
+
+	if (listen(fd, 128) == -1) {
+		log_err("Failed to listen on server socket");
+		goto close_out;
+	}
+
+	goto out;
+
+close_out:
+	close(fd);
+	fd = -1;
+out:
+	return fd;
+}
+
+static int connect_to_server(int server_fd)
+{
+	struct sockaddr_storage addr;
+	socklen_t len = sizeof(addr);
+	int fd;
+
+	fd = socket(AF_INET6, SOCK_STREAM, 0);
+	if (fd == -1) {
+		log_err("Failed to create client socket");
+		goto out;
+	}
+
+	if (getsockname(server_fd, (struct sockaddr *)&addr, &len)) {
+		log_err("Failed to get server addr");
+		goto close_out;
+	}
+
+	if (connect(fd, (const struct sockaddr *)&addr, len) == -1) {
+		log_err("Fail to connect to server");
+		goto close_out;
+	}
+
+	goto out;
+
+close_out:
+	close(fd);
+	fd = -1;
+out:
+	return fd;
+}
+
+static int validate_map(struct bpf_map *map, int client_fd)
+{
+	__u32 cookie_expected_value;
+	struct sockaddr_in6 addr;
+	socklen_t len = sizeof(addr);
+	__u32 cookie_value;
+	__u64 cookie_key;
+	int err = 0;
+	int map_fd;
+
+	if (!map) {
+		log_err("Map not found in BPF object");
+		goto err;
+	}
+
+	map_fd = bpf_map__fd(map);
+
+	err = bpf_map_get_next_key(map_fd, NULL, &cookie_key);
+	if (err) {
+		log_err("Can't get cookie key from map");
+		goto out;
+	}
+
+	err = bpf_map_lookup_elem(map_fd, &cookie_key, &cookie_value);
+	if (err) {
+		log_err("Can't get cookie value from map");
+		goto out;
+	}
+
+	err = getsockname(client_fd, (struct sockaddr *)&addr, &len);
+	if (err) {
+		log_err("Can't get client local addr");
+		goto out;
+	}
+
+	cookie_expected_value = (ntohs(addr.sin6_port) << 8) | 0xFF;
+	if (cookie_value != cookie_expected_value) {
+		log_err("Unexpected value in map: %x != %x", cookie_value,
+			cookie_expected_value);
+		goto err;
+	}
+
+	goto out;
+err:
+	err = -1;
+out:
+	return err;
+}
+
+static int run_test(int cgfd)
+{
+	enum bpf_attach_type attach_type;
+	struct bpf_prog_load_attr attr;
+	struct bpf_program *prog;
+	struct bpf_object *pobj;
+	const char *prog_name;
+	int server_fd = -1;
+	int client_fd = -1;
+	int prog_fd = -1;
+	int err = 0;
+
+	memset(&attr, 0, sizeof(attr));
+	attr.file = SOCKET_COOKIE_PROG;
+	attr.prog_type = BPF_PROG_TYPE_UNSPEC;
+
+	err = bpf_prog_load_xattr(&attr, &pobj, &prog_fd);
+	if (err) {
+		log_err("Failed to load %s", attr.file);
+		goto out;
+	}
+
+	bpf_object__for_each_program(prog, pobj) {
+		prog_name = bpf_program__title(prog, /*needs_copy*/ false);
+
+		if (strcmp(prog_name, "cgroup/connect6") == 0) {
+			attach_type = BPF_CGROUP_INET6_CONNECT;
+		} else if (strcmp(prog_name, "sockops") == 0) {
+			attach_type = BPF_CGROUP_SOCK_OPS;
+		} else {
+			log_err("Unexpected prog: %s", prog_name);
+			goto err;
+		}
+
+		err = bpf_prog_attach(bpf_program__fd(prog), cgfd, attach_type,
+				      BPF_F_ALLOW_OVERRIDE);
+		if (err) {
+			log_err("Failed to attach prog %s", prog_name);
+			goto out;
+		}
+	}
+
+	server_fd = start_server();
+	if (server_fd == -1)
+		goto err;
+
+	client_fd = connect_to_server(server_fd);
+	if (client_fd == -1)
+		goto err;
+
+	if (validate_map(bpf_map__next(NULL, pobj), client_fd))
+		goto err;
+
+	goto out;
+err:
+	err = -1;
+out:
+	close(client_fd);
+	close(server_fd);
+	bpf_object__close(pobj);
+	printf("%s\n", err ? "FAILED" : "PASSED");
+	return err;
+}
+
+int main(int argc, char **argv)
+{
+	int cgfd = -1;
+	int err = 0;
+
+	if (setup_cgroup_environment())
+		goto err;
+
+	cgfd = create_and_get_cgroup(CG_PATH);
+	if (!cgfd)
+		goto err;
+
+	if (join_cgroup(CG_PATH))
+		goto err;
+
+	if (run_test(cgfd))
+		goto err;
+
+	goto out;
+err:
+	err = -1;
+out:
+	close(cgfd);
+	cleanup_cgroup_environment();
+	return err;
+}
-- 
2.17.1

Re: [PATCH v2 bpf-next 4/4] selftests/bpf: Test for get_socket_cookie

[Yonghong Song]

On 7/30/18 5:42 PM, Andrey Ignatov wrote:
> Add test to use get_socket_cookie() from BPF programs of types
> BPF_PROG_TYPE_SOCK_OPS and BPF_PROG_TYPE_CGROUP_SOCK_ADDR.
> 
> The test attaches two programs to cgroup, runs TCP server and client in
> the cgroup and checks that two operations are done properly on client
> socket when user calls connect(2):
> 
> 1. In BPF_CGROUP_INET6_CONNECT socket cookie is used as the key to write
>     new value in a map for client socket.
> 
> 2. In BPF_CGROUP_SOCK_OPS (BPF_SOCK_OPS_TCP_CONNECT_CB callback) the
>     value written in "1." is found by socket cookie, since it's the same
>     socket, and updated.
> 
> Finally the test verifies the value in the map.
> 
> Signed-off-by: Andrey Ignatov <rdna@fb.com>

Acked-by: Yonghong Song <yhs@fb.com>

Re: [PATCH v2 bpf-next 0/4] Support bpf_get_socket_cookie in more prog types

[Daniel Borkmann]

On 07/31/2018 02:42 AM, Andrey Ignatov wrote:
> v1->v2:
> - simplify attach_type guessing logic in selftest.
> 
> This patch set adds support to call bpf_get_socket_cookie() helper from two
> more program types:
> * BPF_PROG_TYPE_CGROUP_SOCK_ADDR;
> * BPF_PROG_TYPE_SOCK_OPS.
> 
> Patch 1 adds kernel support and provides more details.
> Patch 2 syncs UAPI changes to tools/.
> Patch 3 adds missing helper to bpf_helpers.h.
> Patch 4 adds selftest for new functionality.
> 
> 
> Andrey Ignatov (4):
>   bpf: Support bpf_get_socket_cookie in more prog types
>   bpf: Sync bpf.h to tools/
>   selftests/bpf: Add bpf_get_socket_cookie to bpf_helpers.h
>   selftests/bpf: Test for get_socket_cookie

Applied to bpf-next, thanks Andrey!