[PATCH bpf-next 0/6] skip verifier/map tests if kernel support is missing

[PATCH bpf-next 0/6] skip verifier/map tests if kernel support is missing

[Stanislav Fomichev]

If test_maps/test_verifier is running against the kernel which doesn't
have _all_ BPF features enabled, it fails with an error. This patch
series tries to probe kernel support for each failed test and skip
it instead. This lets users run BPF selftests in the not-all-bpf-yes
environments and received correct PASS/NON-PASS result.

See https://www.spinics.net/lists/netdev/msg539331.html for more
context.

The series goes like this:

* patch #1 adds bpf_prog_type_supported() and
  bpf_map_type_supported() which query the kernel (insert 'return 0'
  program or try to create empty map with correct key/value sizes) and
  return supported/unsupported.
  Note: this functionality can later be reimplemented on top of Quentin's
  recent 'bpftool feature' patchset if he decides to move the probes
  into libbpf.
* patch #2 skips sockmap tests in test_maps.c if BPF_MAP_TYPE_SOCKMAP
  map is not supported (if bpf_create_map fails, we probe the kernel
  for support)
* patch #3 skips verifier tests if test->prog_type is not supported (if
  bpf_verify_program fails, we probe the kernel for support)
* patch #4 skips verifier tests if test fixup map is not supported (if
  create_map fails, we probe the kernel for support)
  Note: we can probably move this probe into create_map helper and
  return some argument instead of adding skip_unsupported_map()
  to each fixup; but I'm not sure it's better.
  Also note: in current implementation we still print 'Failed to
  create hash map' from the create_map, but still skip the test.
* next patches fix various small issues that arise from the first four:
  * patch #5 sets "unknown func bpf_trace_printk#6" prog_type to
    BPF_PROG_TYPE_TRACEPOINT so it is correctly skipped in
    CONFIG_BPF_EVENTS=n case
  * patch #6 exposes BPF_PROG_TYPE_CGROUP_{SKB,SOCK,SOCK_ADDR} only when
    CONFIG_CGROUP_BPF=y, this makes verifier correctly skip appropriate
    tests
    Note: I'm not sure whether it's worth sprinkling CONFIG_CGROUP_BPF
    ifdefs all around net/core/filter.c, maybe it's enough to do only
    in bpf_types.h?

Stanislav Fomichev (6):
  selftests/bpf: add map/prog type probe helpers
  selftests/bpf: skip sockmap in test_maps if kernel doesn't have
    support
  selftests/bpf: skip verifier tests for unsupported program types
  selftests/bpf: skip verifier tests for unsupported map types
  selftests/bpf: mark verifier test that uses bpf_trace_printk as
    BPF_PROG_TYPE_TRACEPOINT
  bpf: BPF_PROG_TYPE_CGROUP_{SKB,SOCK,SOCK_ADDR} require cgroups enabled

 include/linux/bpf_types.h                   |  2 +
 net/core/filter.c                           | 18 ++++++
 tools/testing/selftests/bpf/Makefile        |  2 +
 tools/testing/selftests/bpf/probe_helpers.c | 69 +++++++++++++++++++++
 tools/testing/selftests/bpf/probe_helpers.h | 10 +++
 tools/testing/selftests/bpf/test_maps.c     | 14 ++++-
 tools/testing/selftests/bpf/test_verifier.c | 45 ++++++++++++--
 7 files changed, 155 insertions(+), 5 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/probe_helpers.c
 create mode 100644 tools/testing/selftests/bpf/probe_helpers.h

-- 
2.20.0.rc2.403.gdbc3b29805-goog

[PATCH bpf-next 1/6] selftests/bpf: add map/prog type probe helpers

[Stanislav Fomichev]

Export bpf_map_type_supported() and bpf_prog_type_supported() which
return true/false to indicate kernel support for the appropriate
program or map type. These helpers will be used in the next commits
to selectively skip test_verifier/test_maps tests.

bpf_map_type_supported() supports only limited set of maps for which we
do fixups in the test_verifier, for unknown maps it falls back to
'supported'.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 tools/testing/selftests/bpf/probe_helpers.c | 68 +++++++++++++++++++++
 tools/testing/selftests/bpf/probe_helpers.h | 10 +++
 2 files changed, 78 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/probe_helpers.c
 create mode 100644 tools/testing/selftests/bpf/probe_helpers.h

diff --git a/tools/testing/selftests/bpf/probe_helpers.c b/tools/testing/selftests/bpf/probe_helpers.c
new file mode 100644
index 000000000000..00467fdda813
--- /dev/null
+++ b/tools/testing/selftests/bpf/probe_helpers.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+#include <unistd.h>
+#include <bpf/bpf.h>
+
+#include "cgroup_helpers.h"
+#include "bpf_util.h"
+#include "../../../include/linux/filter.h"
+
+bool bpf_prog_type_supported(enum bpf_prog_type prog_type)
+{
+	struct bpf_load_program_attr attr;
+	struct bpf_insn insns[] = {
+		BPF_MOV64_IMM(BPF_REG_0, 0),
+		BPF_EXIT_INSN(),
+	};
+	int ret;
+
+	if (prog_type == BPF_PROG_TYPE_UNSPEC)
+		return true;
+
+	memset(&attr, 0, sizeof(attr));
+	attr.prog_type = prog_type;
+	attr.insns = insns;
+	attr.insns_cnt = ARRAY_SIZE(insns);
+	attr.license = "GPL";
+
+	ret = bpf_load_program_xattr(&attr, NULL, 0);
+	if (ret < 0)
+		return false;
+	close(ret);
+
+	return true;
+}
+
+bool bpf_map_type_supported(enum bpf_map_type map_type)
+{
+	int key_size, value_size, max_entries;
+	int fd;
+
+	key_size = sizeof(__u32);
+	value_size = sizeof(__u32);
+	max_entries = 1;
+
+	/* limited set of maps for test_verifier.c and test_maps.c */
+	switch (map_type) {
+	case BPF_MAP_TYPE_SOCKMAP:
+	case BPF_MAP_TYPE_SOCKHASH:
+	case BPF_MAP_TYPE_XSKMAP:
+		break;
+	case BPF_MAP_TYPE_STACK_TRACE:
+		value_size = sizeof(__u64);
+	case BPF_MAP_TYPE_CGROUP_STORAGE:
+	case BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE:
+		key_size = sizeof(struct bpf_cgroup_storage_key);
+		value_size = sizeof(__u64);
+		max_entries = 0;
+		break;
+	default:
+		return true;
+	}
+
+	fd = bpf_create_map(map_type, key_size, value_size, max_entries, 0);
+	if (fd < 0)
+		return false;
+	close(fd);
+
+	return true;
+}
diff --git a/tools/testing/selftests/bpf/probe_helpers.h b/tools/testing/selftests/bpf/probe_helpers.h
new file mode 100644
index 000000000000..9a107d6fe936
--- /dev/null
+++ b/tools/testing/selftests/bpf/probe_helpers.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+#ifndef __PROBE_HELPERS_H
+#define __PROBE_HELPERS_H
+
+#include <linux/bpf.h>
+
+bool bpf_prog_type_supported(enum bpf_prog_type prog_type);
+bool bpf_map_type_supported(enum bpf_map_type map_type);
+
+#endif
-- 
2.20.0.rc2.403.gdbc3b29805-goog

[PATCH bpf-next 2/6] selftests/bpf: skip sockmap in test_maps if kernel doesn't have support

[Stanislav Fomichev]

Use recently introduced bpf_map_type_supported() to skip test_sockmap()
if map creation fails. The skipped test is indicated in the output.

Example:

test_sockmap SKIP (unsupported map type BPF_MAP_TYPE_SOCKMAP)
Fork 1024 tasks to 'test_update_delete'
...
test_sockmap SKIP (unsupported map type BPF_MAP_TYPE_SOCKMAP)
Fork 1024 tasks to 'test_update_delete'
...
test_maps: OK, 2 SKIPPED

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 tools/testing/selftests/bpf/Makefile    |  1 +
 tools/testing/selftests/bpf/test_maps.c | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 73aa6d8f4a2f..52a0654145ad 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -81,6 +81,7 @@ $(OUTPUT)/test_progs: trace_helpers.c
 $(OUTPUT)/get_cgroup_id_user: cgroup_helpers.c
 $(OUTPUT)/test_cgroup_storage: cgroup_helpers.c
 $(OUTPUT)/test_netcnt: cgroup_helpers.c
+$(OUTPUT)/test_maps: probe_helpers.c
 
 .PHONY: force
 
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 9c79ee017df3..bdf9bdaea73e 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -27,11 +27,14 @@
 
 #include "bpf_util.h"
 #include "bpf_rlimit.h"
+#include "probe_helpers.h"
 
 #ifndef ENOTSUPP
 #define ENOTSUPP 524
 #endif
 
+static int skips;
+
 static int map_flags;
 
 #define CHECK(condition, tag, format...) ({				\
@@ -725,6 +728,15 @@ static void test_sockmap(int tasks, void *data)
 			    sizeof(key), sizeof(value),
 			    6, 0);
 	if (fd < 0) {
+		if (!bpf_map_type_supported(BPF_MAP_TYPE_SOCKMAP)) {
+			printf("%s SKIP (unsupported map type BPF_MAP_TYPE_SOCKMAP)\n",
+			       __func__);
+			skips++;
+			for (i = 0; i < 6; i++)
+				close(sfd[i]);
+			return;
+		}
+
 		printf("Failed to create sockmap %i\n", fd);
 		goto out_sockmap;
 	}
@@ -1702,6 +1714,6 @@ int main(void)
 	map_flags = BPF_F_NO_PREALLOC;
 	run_all_tests();
 
-	printf("test_maps: OK\n");
+	printf("test_maps: OK, %d SKIPPED\n", skips);
 	return 0;
 }
-- 
2.20.0.rc2.403.gdbc3b29805-goog

[PATCH bpf-next 3/6] selftests/bpf: skip verifier tests for unsupported program types

[Stanislav Fomichev]

Use recently introduced bpf_prog_type_supported() to skip tests in the
test_verifier() if bpf_verify_program() fails. The skipped test is
indicated in the output.

Example:

...
679/p bpf_get_stack return R0 within range SKIP (unsupported program
type 5)
680/p ld_abs: invalid op 1 OK
...
Summary: 863 PASSED, 165 SKIPPED, 3 FAILED

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 tools/testing/selftests/bpf/Makefile        | 1 +
 tools/testing/selftests/bpf/test_verifier.c | 9 ++++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 52a0654145ad..9e9c5ada3e9b 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -82,6 +82,7 @@ $(OUTPUT)/get_cgroup_id_user: cgroup_helpers.c
 $(OUTPUT)/test_cgroup_storage: cgroup_helpers.c
 $(OUTPUT)/test_netcnt: cgroup_helpers.c
 $(OUTPUT)/test_maps: probe_helpers.c
+$(OUTPUT)/test_verifier: probe_helpers.c
 
 .PHONY: force
 
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index a08c67c8767e..124d21306c27 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -45,6 +45,7 @@
 #include "bpf_rand.h"
 #include "bpf_util.h"
 #include "../../../include/linux/filter.h"
+#include "probe_helpers.h"
 
 #define MAX_INSNS	BPF_MAXINSNS
 #define MAX_FIXUPS	8
@@ -57,6 +58,7 @@
 
 #define UNPRIV_SYSCTL "kernel/unprivileged_bpf_disabled"
 static bool unpriv_disabled = false;
+static int skips;
 
 struct bpf_test {
 	const char *descr;
@@ -14409,6 +14411,11 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
 		pflags |= BPF_F_ANY_ALIGNMENT;
 	fd_prog = bpf_verify_program(prog_type, prog, prog_len, pflags,
 				     "GPL", 0, bpf_vlog, sizeof(bpf_vlog), 1);
+	if (fd_prog < 0 && !bpf_prog_type_supported(prog_type)) {
+		printf("SKIP (unsupported program type %d)\n", prog_type);
+		skips++;
+		goto close_fds;
+	}
 
 	expected_ret = unpriv && test->result_unpriv != UNDEF ?
 		       test->result_unpriv : test->result;
@@ -14532,7 +14539,7 @@ static bool test_as_unpriv(struct bpf_test *test)
 
 static int do_test(bool unpriv, unsigned int from, unsigned int to)
 {
-	int i, passes = 0, errors = 0, skips = 0;
+	int i, passes = 0, errors = 0;
 
 	for (i = from; i < to; i++) {
 		struct bpf_test *test = &tests[i];
-- 
2.20.0.rc2.403.gdbc3b29805-goog

[PATCH bpf-next 4/6] selftests/bpf: skip verifier tests for unsupported map types

[Stanislav Fomichev]

Use recently introduced bpf_map_type_supported() to skip tests in the
test_verifier if map creation (create_map) fails. It's handled
explicitly for each fixup, i.e. if bpf_create_map returns negative fd,
we probe the kernel for the appropriate map support and skip the
test is map type is not supported.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 tools/testing/selftests/bpf/test_verifier.c | 34 +++++++++++++++++++--
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 124d21306c27..d267f5248b5d 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -14221,10 +14221,20 @@ static int create_cgroup_storage(bool percpu)
 	return fd;
 }
 
+static bool skip_unsupported_map(int ret, enum bpf_map_type map_type)
+{
+	if (ret < 0 && !bpf_map_type_supported(map_type)) {
+		printf("SKIP (unsupported map type %d)\n", map_type);
+		skips++;
+		return true;
+	}
+	return false;
+}
+
 static char bpf_vlog[UINT_MAX >> 8];
 
-static void do_test_fixup(struct bpf_test *test, enum bpf_map_type prog_type,
-			  struct bpf_insn *prog, int *map_fds)
+static int do_test_fixup(struct bpf_test *test, enum bpf_map_type prog_type,
+			 struct bpf_insn *prog, int *map_fds)
 {
 	int *fixup_map_hash_8b = test->fixup_map_hash_8b;
 	int *fixup_map_hash_48b = test->fixup_map_hash_48b;
@@ -14309,6 +14319,9 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_map_type prog_type,
 
 	if (*fixup_cgroup_storage) {
 		map_fds[7] = create_cgroup_storage(false);
+		if (skip_unsupported_map(map_fds[7],
+					 BPF_MAP_TYPE_CGROUP_STORAGE))
+			return -1;
 		do {
 			prog[*fixup_cgroup_storage].imm = map_fds[7];
 			fixup_cgroup_storage++;
@@ -14317,6 +14330,9 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_map_type prog_type,
 
 	if (*fixup_percpu_cgroup_storage) {
 		map_fds[8] = create_cgroup_storage(true);
+		if (skip_unsupported_map(map_fds[8],
+					 BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE))
+			return -1;
 		do {
 			prog[*fixup_percpu_cgroup_storage].imm = map_fds[8];
 			fixup_percpu_cgroup_storage++;
@@ -14325,6 +14341,8 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_map_type prog_type,
 	if (*fixup_map_sockmap) {
 		map_fds[9] = create_map(BPF_MAP_TYPE_SOCKMAP, sizeof(int),
 					sizeof(int), 1);
+		if (skip_unsupported_map(map_fds[9], BPF_MAP_TYPE_SOCKMAP))
+			return -1;
 		do {
 			prog[*fixup_map_sockmap].imm = map_fds[9];
 			fixup_map_sockmap++;
@@ -14333,6 +14351,8 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_map_type prog_type,
 	if (*fixup_map_sockhash) {
 		map_fds[10] = create_map(BPF_MAP_TYPE_SOCKHASH, sizeof(int),
 					sizeof(int), 1);
+		if (skip_unsupported_map(map_fds[10], BPF_MAP_TYPE_SOCKHASH))
+			return -1;
 		do {
 			prog[*fixup_map_sockhash].imm = map_fds[10];
 			fixup_map_sockhash++;
@@ -14341,6 +14361,8 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_map_type prog_type,
 	if (*fixup_map_xskmap) {
 		map_fds[11] = create_map(BPF_MAP_TYPE_XSKMAP, sizeof(int),
 					sizeof(int), 1);
+		if (skip_unsupported_map(map_fds[11], BPF_MAP_TYPE_XSKMAP))
+			return -1;
 		do {
 			prog[*fixup_map_xskmap].imm = map_fds[11];
 			fixup_map_xskmap++;
@@ -14349,11 +14371,16 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_map_type prog_type,
 	if (*fixup_map_stacktrace) {
 		map_fds[12] = create_map(BPF_MAP_TYPE_STACK_TRACE, sizeof(u32),
 					 sizeof(u64), 1);
+		if (skip_unsupported_map(map_fds[12],
+					 BPF_MAP_TYPE_STACK_TRACE))
+			return -1;
 		do {
 			prog[*fixup_map_stacktrace].imm = map_fds[12];
 			fixup_map_stacktrace++;
 		} while (fixup_map_stacktrace);
 	}
+
+	return 0;
 }
 
 static int set_admin(bool admin)
@@ -14401,7 +14428,8 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
 
 	if (!prog_type)
 		prog_type = BPF_PROG_TYPE_SOCKET_FILTER;
-	do_test_fixup(test, prog_type, prog, map_fds);
+	if (do_test_fixup(test, prog_type, prog, map_fds) < 0)
+		return;
 	prog_len = probe_filter_length(prog);
 
 	pflags = 0;
-- 
2.20.0.rc2.403.gdbc3b29805-goog

[PATCH bpf-next 5/6] selftests/bpf: mark verifier test that uses bpf_trace_printk as BPF_PROG_TYPE_TRACEPOINT

[Stanislav Fomichev]

We don't have this helper if the kernel was compiled without
CONFIG_BPF_EVENTS. Setting prog_type to BPF_PROG_TYPE_TRACEPOINT
let's verifier correctly skip this test based on the missing
prog_type support in the kernel.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 tools/testing/selftests/bpf/test_verifier.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index d267f5248b5d..590bf9a63ad9 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -2622,6 +2622,7 @@ static struct bpf_test tests[] = {
 		.errstr_unpriv = "unknown func bpf_trace_printk#6",
 		.result_unpriv = REJECT,
 		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_TRACEPOINT,
 	},
 	{
 		"unpriv: pass pointer to helper function",
-- 
2.20.0.rc2.403.gdbc3b29805-goog

[PATCH bpf-next 6/6] bpf: BPF_PROG_TYPE_CGROUP_{SKB,SOCK,SOCK_ADDR} require cgroups enabled

[Stanislav Fomichev]

There is no way to exercise appropriate attach points without cgroups
enabled. This lets test_verifier correctly skip tests for these
prog_types if kernel was compiled without BPF cgroup support.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 include/linux/bpf_types.h |  2 ++
 net/core/filter.c         | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index 44d9ab4809bd..08bf2f1fe553 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -6,9 +6,11 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_SOCKET_FILTER, sk_filter)
 BPF_PROG_TYPE(BPF_PROG_TYPE_SCHED_CLS, tc_cls_act)
 BPF_PROG_TYPE(BPF_PROG_TYPE_SCHED_ACT, tc_cls_act)
 BPF_PROG_TYPE(BPF_PROG_TYPE_XDP, xdp)
+#ifdef CONFIG_CGROUP_BPF
 BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SKB, cg_skb)
 BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCK, cg_sock)
 BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCK_ADDR, cg_sock_addr)
+#endif
 BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_IN, lwt_in)
 BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_OUT, lwt_out)
 BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_XMIT, lwt_xmit)
diff --git a/net/core/filter.c b/net/core/filter.c
index f9348806e843..6a390e519431 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5315,6 +5315,7 @@ bpf_base_func_proto(enum bpf_func_id func_id)
 	}
 }
 
+#ifdef CONFIG_CGROUP_BPF
 static const struct bpf_func_proto *
 sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
@@ -5364,6 +5365,7 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return bpf_base_func_proto(func_id);
 	}
 }
+#endif
 
 static const struct bpf_func_proto *
 sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
@@ -5382,6 +5384,7 @@ sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	}
 }
 
+#ifdef CONFIG_CGROUP_BPF
 static const struct bpf_func_proto *
 cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
@@ -5392,6 +5395,7 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return sk_filter_func_proto(func_id, prog);
 	}
 }
+#endif
 
 static const struct bpf_func_proto *
 tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
@@ -5790,6 +5794,7 @@ static bool sk_filter_is_valid_access(int off, int size,
 	return bpf_skb_is_valid_access(off, size, type, prog, info);
 }
 
+#ifdef CONFIG_CGROUP_BPF
 static bool cg_skb_is_valid_access(int off, int size,
 				   enum bpf_access_type type,
 				   const struct bpf_prog *prog,
@@ -5834,6 +5839,7 @@ static bool cg_skb_is_valid_access(int off, int size,
 
 	return bpf_skb_is_valid_access(off, size, type, prog, info);
 }
+#endif
 
 static bool lwt_is_valid_access(int off, int size,
 				enum bpf_access_type type,
@@ -5873,6 +5879,7 @@ static bool lwt_is_valid_access(int off, int size,
 	return bpf_skb_is_valid_access(off, size, type, prog, info);
 }
 
+#ifdef CONFIG_CGROUP_BPF
 /* Attach type specific accesses */
 static bool __sock_filter_check_attach_type(int off,
 					    enum bpf_access_type access_type,
@@ -5916,6 +5923,7 @@ static bool __sock_filter_check_attach_type(int off,
 full_access:
 	return true;
 }
+#endif
 
 static bool __sock_filter_check_size(int off, int size,
 				     struct bpf_insn_access_aux *info)
@@ -5944,6 +5952,7 @@ bool bpf_sock_is_valid_access(int off, int size, enum bpf_access_type type,
 	return true;
 }
 
+#ifdef CONFIG_CGROUP_BPF
 static bool sock_filter_is_valid_access(int off, int size,
 					enum bpf_access_type type,
 					const struct bpf_prog *prog,
@@ -5954,6 +5963,7 @@ static bool sock_filter_is_valid_access(int off, int size,
 	return __sock_filter_check_attach_type(off, type,
 					       prog->expected_attach_type);
 }
+#endif
 
 static int bpf_noop_prologue(struct bpf_insn *insn_buf, bool direct_write,
 			     const struct bpf_prog *prog)
@@ -6133,6 +6143,7 @@ void bpf_warn_invalid_xdp_action(u32 act)
 }
 EXPORT_SYMBOL_GPL(bpf_warn_invalid_xdp_action);
 
+#ifdef CONFIG_CGROUP_BPF
 static bool sock_addr_is_valid_access(int off, int size,
 				      enum bpf_access_type type,
 				      const struct bpf_prog *prog,
@@ -6219,6 +6230,7 @@ static bool sock_addr_is_valid_access(int off, int size,
 
 	return true;
 }
+#endif
 
 static bool sock_ops_is_valid_access(int off, int size,
 				     enum bpf_access_type type,
@@ -6955,6 +6967,7 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
 	SOCK_ADDR_LOAD_OR_STORE_NESTED_FIELD_SIZE_OFF(			       \
 		S, NS, F, NF, BPF_FIELD_SIZEOF(NS, NF), 0, TF)
 
+#ifdef CONFIG_CGROUP_BPF
 static u32 sock_addr_convert_ctx_access(enum bpf_access_type type,
 					const struct bpf_insn *si,
 					struct bpf_insn *insn_buf,
@@ -7043,6 +7056,7 @@ static u32 sock_addr_convert_ctx_access(enum bpf_access_type type,
 
 	return insn - insn_buf;
 }
+#endif
 
 static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
 				       const struct bpf_insn *si,
@@ -7569,6 +7583,7 @@ const struct bpf_prog_ops xdp_prog_ops = {
 	.test_run		= bpf_prog_test_run_xdp,
 };
 
+#ifdef CONFIG_CGROUP_BPF
 const struct bpf_verifier_ops cg_skb_verifier_ops = {
 	.get_func_proto		= cg_skb_func_proto,
 	.is_valid_access	= cg_skb_is_valid_access,
@@ -7578,6 +7593,7 @@ const struct bpf_verifier_ops cg_skb_verifier_ops = {
 const struct bpf_prog_ops cg_skb_prog_ops = {
 	.test_run		= bpf_prog_test_run_skb,
 };
+#endif
 
 const struct bpf_verifier_ops lwt_in_verifier_ops = {
 	.get_func_proto		= lwt_in_func_proto,
@@ -7620,6 +7636,7 @@ const struct bpf_prog_ops lwt_seg6local_prog_ops = {
 	.test_run		= bpf_prog_test_run_skb,
 };
 
+#ifdef CONFIG_CGROUP_BPF
 const struct bpf_verifier_ops cg_sock_verifier_ops = {
 	.get_func_proto		= sock_filter_func_proto,
 	.is_valid_access	= sock_filter_is_valid_access,
@@ -7637,6 +7654,7 @@ const struct bpf_verifier_ops cg_sock_addr_verifier_ops = {
 
 const struct bpf_prog_ops cg_sock_addr_prog_ops = {
 };
+#endif
 
 const struct bpf_verifier_ops sock_ops_verifier_ops = {
 	.get_func_proto		= sock_ops_func_proto,
-- 
2.20.0.rc2.403.gdbc3b29805-goog

Re: [PATCH bpf-next 1/6] selftests/bpf: add map/prog type probe helpers

[Quentin Monnet]

Hi Stanislav,

2018-12-13 11:02 UTC-0800 ~ Stanislav Fomichev <sdf@google.com>
> Export bpf_map_type_supported() and bpf_prog_type_supported() which
> return true/false to indicate kernel support for the appropriate
> program or map type. These helpers will be used in the next commits
> to selectively skip test_verifier/test_maps tests.
> 
> bpf_map_type_supported() supports only limited set of maps for which we
> do fixups in the test_verifier, for unknown maps it falls back to
> 'supported'.

Why would you fall back on “supported” if it does not know about them?
Would that be worth having an enum as a return type (..._SUPPORTED,
..._UNSUPPORTED, ..._UNKNOWN) maybe? Or default to not supported?

Not related - We would need to put a warning somewhere, maybe a comment
in the header, that using probes repeatedly in a short amount of time
needs to update resources limits (setrlimit()), otherwise probes won't
work correctly.

> 
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
>  tools/testing/selftests/bpf/probe_helpers.c | 68 +++++++++++++++++++++
>  tools/testing/selftests/bpf/probe_helpers.h | 10 +++
>  2 files changed, 78 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/probe_helpers.c
>  create mode 100644 tools/testing/selftests/bpf/probe_helpers.h
> 
> diff --git a/tools/testing/selftests/bpf/probe_helpers.c b/tools/testing/selftests/bpf/probe_helpers.c
> new file mode 100644
> index 000000000000..00467fdda813
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/probe_helpers.c
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +#include <unistd.h>
> +#include <bpf/bpf.h>
> +
> +#include "cgroup_helpers.h"
> +#include "bpf_util.h"
> +#include "../../../include/linux/filter.h"
> +
> +bool bpf_prog_type_supported(enum bpf_prog_type prog_type)

Can we please make it possible to add an ifindex for testing offload
support?

> +{
> +	struct bpf_load_program_attr attr;
> +	struct bpf_insn insns[] = {
> +		BPF_MOV64_IMM(BPF_REG_0, 0),
> +		BPF_EXIT_INSN(),
> +	};
> +	int ret;
> +
> +	if (prog_type == BPF_PROG_TYPE_UNSPEC)
> +		return true;
> +
> +	memset(&attr, 0, sizeof(attr));
> +	attr.prog_type = prog_type;
> +	attr.insns = insns;
> +	attr.insns_cnt = ARRAY_SIZE(insns);
> +	attr.license = "GPL";
> +
> +	ret = bpf_load_program_xattr(&attr, NULL, 0);
> +	if (ret < 0)
> +		return false;
> +	close(ret);
> +
> +	return true;
> +}
> +
> +bool bpf_map_type_supported(enum bpf_map_type map_type)

Could we take an ifindex here as well?

> +{
> +	int key_size, value_size, max_entries;
> +	int fd;
> +
> +	key_size = sizeof(__u32);
> +	value_size = sizeof(__u32);
> +	max_entries = 1;
> +
> +	/* limited set of maps for test_verifier.c and test_maps.c */
> +	switch (map_type) {
> +	case BPF_MAP_TYPE_SOCKMAP:
> +	case BPF_MAP_TYPE_SOCKHASH:
> +	case BPF_MAP_TYPE_XSKMAP:
> +		break;
> +	case BPF_MAP_TYPE_STACK_TRACE:
> +		value_size = sizeof(__u64);
> +	case BPF_MAP_TYPE_CGROUP_STORAGE:
> +	case BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE:
> +		key_size = sizeof(struct bpf_cgroup_storage_key);
> +		value_size = sizeof(__u64);
> +		max_entries = 0;
> +		break;
> +	default:

Why not probing the other types of maps and blindly assume everything
else is supported?

> +		return true;
> +	}

For the record if you were to probe all existing map types at this date
you have would have issues here for LPM_TRIE (key_size, value_size,
map_flags), QUEUE and STACK (key_size). Also, maps in maps.

> +
> +	fd = bpf_create_map(map_type, key_size, value_size, max_entries, 0);
> +	if (fd < 0)
> +		return false;
> +	close(fd);
> +
> +	return true;
> +}
> diff --git a/tools/testing/selftests/bpf/probe_helpers.h b/tools/testing/selftests/bpf/probe_helpers.h
> new file mode 100644
> index 000000000000..9a107d6fe936
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/probe_helpers.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +#ifndef __PROBE_HELPERS_H
> +#define __PROBE_HELPERS_H
> +
> +#include <linux/bpf.h>
> +
> +bool bpf_prog_type_supported(enum bpf_prog_type prog_type);
> +bool bpf_map_type_supported(enum bpf_map_type map_type);

Should these get a visibility attribute with "LIBBPF_API" in front of
the declarations?

> +
> +#endif

Re: [PATCH bpf-next 1/6] selftests/bpf: add map/prog type probe helpers

[Stanislav Fomichev]

On 12/14, Quentin Monnet wrote:
> Hi Stanislav,
> 
> 2018-12-13 11:02 UTC-0800 ~ Stanislav Fomichev <sdf@google.com>
> > Export bpf_map_type_supported() and bpf_prog_type_supported() which
> > return true/false to indicate kernel support for the appropriate
> > program or map type. These helpers will be used in the next commits
> > to selectively skip test_verifier/test_maps tests.
> > 
> > bpf_map_type_supported() supports only limited set of maps for which we
> > do fixups in the test_verifier, for unknown maps it falls back to
> > 'supported'.
> 
> Why would you fall back on “supported” if it does not know about them?
> Would that be worth having an enum as a return type (..._SUPPORTED,
> ..._UNSUPPORTED, ..._UNKNOWN) maybe? Or default to not supported?
I thought that it's safer for verifier to FAIL in case we forgot to add
a specific map support to bpf_map_type_supported(). This is not the case
if we were to use your version where you try to support every map type.

> Not related - We would need to put a warning somewhere, maybe a comment
> in the header, that using probes repeatedly in a short amount of time
> needs to update resources limits (setrlimit()), otherwise probes won't
> work correctly.
If we were to move this to libbpf, yes. For tests, I think we include
bpr_rlimit.h everywhere and things just work :-)
> 
> > 
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> >  tools/testing/selftests/bpf/probe_helpers.c | 68 +++++++++++++++++++++
> >  tools/testing/selftests/bpf/probe_helpers.h | 10 +++
> >  2 files changed, 78 insertions(+)
> >  create mode 100644 tools/testing/selftests/bpf/probe_helpers.c
> >  create mode 100644 tools/testing/selftests/bpf/probe_helpers.h
> > 
> > diff --git a/tools/testing/selftests/bpf/probe_helpers.c b/tools/testing/selftests/bpf/probe_helpers.c
> > new file mode 100644
> > index 000000000000..00467fdda813
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/probe_helpers.c
> > @@ -0,0 +1,68 @@
> > +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +#include <unistd.h>
> > +#include <bpf/bpf.h>
> > +
> > +#include "cgroup_helpers.h"
> > +#include "bpf_util.h"
> > +#include "../../../include/linux/filter.h"
> > +
> > +bool bpf_prog_type_supported(enum bpf_prog_type prog_type)
> 
> Can we please make it possible to add an ifindex for testing offload
> support?
Can we extend it later as we go? This is just a test helper with a
limited support.
If you want to start with putting this to libbpf, then yes, we need
to add ifindex and properly support all map types.
> 
> > +{
> > +	struct bpf_load_program_attr attr;
> > +	struct bpf_insn insns[] = {
> > +		BPF_MOV64_IMM(BPF_REG_0, 0),
> > +		BPF_EXIT_INSN(),
> > +	};
> > +	int ret;
> > +
> > +	if (prog_type == BPF_PROG_TYPE_UNSPEC)
> > +		return true;
> > +
> > +	memset(&attr, 0, sizeof(attr));
> > +	attr.prog_type = prog_type;
> > +	attr.insns = insns;
> > +	attr.insns_cnt = ARRAY_SIZE(insns);
> > +	attr.license = "GPL";
> > +
> > +	ret = bpf_load_program_xattr(&attr, NULL, 0);
> > +	if (ret < 0)
> > +		return false;
> > +	close(ret);
> > +
> > +	return true;
> > +}
> > +
> > +bool bpf_map_type_supported(enum bpf_map_type map_type)
> 
> Could we take an ifindex here as well?
ditto, see above
> 
> > +{
> > +	int key_size, value_size, max_entries;
> > +	int fd;
> > +
> > +	key_size = sizeof(__u32);
> > +	value_size = sizeof(__u32);
> > +	max_entries = 1;
> > +
> > +	/* limited set of maps for test_verifier.c and test_maps.c */
> > +	switch (map_type) {
> > +	case BPF_MAP_TYPE_SOCKMAP:
> > +	case BPF_MAP_TYPE_SOCKHASH:
> > +	case BPF_MAP_TYPE_XSKMAP:
> > +		break;
> > +	case BPF_MAP_TYPE_STACK_TRACE:
> > +		value_size = sizeof(__u64);
> > +	case BPF_MAP_TYPE_CGROUP_STORAGE:
> > +	case BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE:
> > +		key_size = sizeof(struct bpf_cgroup_storage_key);
> > +		value_size = sizeof(__u64);
> > +		max_entries = 0;
> > +		break;
> > +	default:
> 
> Why not probing the other types of maps and blindly assume everything
> else is supported?
Again, for a verifier, I'd rather fail for a case where we didn't
explicitly allow it to skip.
> 
> > +		return true;
> > +	}
> 
> For the record if you were to probe all existing map types at this date
> you have would have issues here for LPM_TRIE (key_size, value_size,
> map_flags), QUEUE and STACK (key_size). Also, maps in maps.
Ack, again, this just for a limited set of maps where we do fixups in
verifier.
> 
> > +
> > +	fd = bpf_create_map(map_type, key_size, value_size, max_entries, 0);
> > +	if (fd < 0)
> > +		return false;
> > +	close(fd);
> > +
> > +	return true;
> > +}
> > diff --git a/tools/testing/selftests/bpf/probe_helpers.h b/tools/testing/selftests/bpf/probe_helpers.h
> > new file mode 100644
> > index 000000000000..9a107d6fe936
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/probe_helpers.h
> > @@ -0,0 +1,10 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > +#ifndef __PROBE_HELPERS_H
> > +#define __PROBE_HELPERS_H
> > +
> > +#include <linux/bpf.h>
> > +
> > +bool bpf_prog_type_supported(enum bpf_prog_type prog_type);
> > +bool bpf_map_type_supported(enum bpf_map_type map_type);
> 
> Should these get a visibility attribute with "LIBBPF_API" in front of
> the declarations?
If we were to move them to the libbpf, yes. So far, it's only a test
helper.
> 
> > +
> > +#endif

Re: [PATCH bpf-next 1/6] selftests/bpf: add map/prog type probe helpers

[Quentin Monnet]

2018-12-14 10:16 UTC-0800 ~ Stanislav Fomichev <sdf@fomichev.me>
> On 12/14, Quentin Monnet wrote:
>> Hi Stanislav,
>>
>> 2018-12-13 11:02 UTC-0800 ~ Stanislav Fomichev <sdf@google.com>
>>> Export bpf_map_type_supported() and bpf_prog_type_supported() which
>>> return true/false to indicate kernel support for the appropriate
>>> program or map type. These helpers will be used in the next commits
>>> to selectively skip test_verifier/test_maps tests.
>>>
>>> bpf_map_type_supported() supports only limited set of maps for which we
>>> do fixups in the test_verifier, for unknown maps it falls back to
>>> 'supported'.
>>
>> Why would you fall back on “supported” if it does not know about them?
>> Would that be worth having an enum as a return type (..._SUPPORTED,
>> ..._UNSUPPORTED, ..._UNKNOWN) maybe? Or default to not supported?
> I thought that it's safer for verifier to FAIL in case we forgot to add
> a specific map support to bpf_map_type_supported(). This is not the case
> if we were to use your version where you try to support every map type.
> 
>> Not related - We would need to put a warning somewhere, maybe a comment
>> in the header, that using probes repeatedly in a short amount of time
>> needs to update resources limits (setrlimit()), otherwise probes won't
>> work correctly.
> If we were to move this to libbpf, yes. For tests, I think we include
> bpr_rlimit.h everywhere and things just work :-)

Hmm. I was so focused on bpftool and libbpf that somehow I read you
patch as a proposal to include these probes directly into libbpf. Which,
as you explain (and as I should have read), is not the case. So please
accept my apologies, in this case your decisions (here and in the rest
of the patch) make sense to me :).

Re: [PATCH bpf-next 1/6] selftests/bpf: add map/prog type probe helpers

[Stanislav Fomichev]

On 12/14, Quentin Monnet wrote:
> 2018-12-14 10:16 UTC-0800 ~ Stanislav Fomichev <sdf@fomichev.me>
> > On 12/14, Quentin Monnet wrote:
> >> Hi Stanislav,
> >>
> >> 2018-12-13 11:02 UTC-0800 ~ Stanislav Fomichev <sdf@google.com>
> >>> Export bpf_map_type_supported() and bpf_prog_type_supported() which
> >>> return true/false to indicate kernel support for the appropriate
> >>> program or map type. These helpers will be used in the next commits
> >>> to selectively skip test_verifier/test_maps tests.
> >>>
> >>> bpf_map_type_supported() supports only limited set of maps for which we
> >>> do fixups in the test_verifier, for unknown maps it falls back to
> >>> 'supported'.
> >>
> >> Why would you fall back on “supported” if it does not know about them?
> >> Would that be worth having an enum as a return type (..._SUPPORTED,
> >> ..._UNSUPPORTED, ..._UNKNOWN) maybe? Or default to not supported?
> > I thought that it's safer for verifier to FAIL in case we forgot to add
> > a specific map support to bpf_map_type_supported(). This is not the case
> > if we were to use your version where you try to support every map type.
> > 
> >> Not related - We would need to put a warning somewhere, maybe a comment
> >> in the header, that using probes repeatedly in a short amount of time
> >> needs to update resources limits (setrlimit()), otherwise probes won't
> >> work correctly.
> > If we were to move this to libbpf, yes. For tests, I think we include
> > bpr_rlimit.h everywhere and things just work :-)
> 
> Hmm. I was so focused on bpftool and libbpf that somehow I read you
> patch as a proposal to include these probes directly into libbpf. Which,
> as you explain (and as I should have read), is not the case. So please
> accept my apologies, in this case your decisions (here and in the rest
> of the patch) make sense to me :).
No worries, I was just scratching my own itch with these (wanted to have
a simple non-controversial probers for the test cases, I can migrate
to your libbpf helpers whenever they are available).

Re: [PATCH bpf-next 6/6] bpf: BPF_PROG_TYPE_CGROUP_{SKB,SOCK,SOCK_ADDR} require cgroups enabled

[Alexei Starovoitov]

On Thu, Dec 13, 2018 at 11:03 AM Stanislav Fomichev <sdf@google.com> wrote:
>
> There is no way to exercise appropriate attach points without cgroups
> enabled. This lets test_verifier correctly skip tests for these
> prog_types if kernel was compiled without BPF cgroup support.
>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
>  include/linux/bpf_types.h |  2 ++
>  net/core/filter.c         | 18 ++++++++++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> index 44d9ab4809bd..08bf2f1fe553 100644
> --- a/include/linux/bpf_types.h
> +++ b/include/linux/bpf_types.h
> @@ -6,9 +6,11 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_SOCKET_FILTER, sk_filter)
>  BPF_PROG_TYPE(BPF_PROG_TYPE_SCHED_CLS, tc_cls_act)
>  BPF_PROG_TYPE(BPF_PROG_TYPE_SCHED_ACT, tc_cls_act)
>  BPF_PROG_TYPE(BPF_PROG_TYPE_XDP, xdp)
> +#ifdef CONFIG_CGROUP_BPF
>  BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SKB, cg_skb)
>  BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCK, cg_sock)
>  BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCK_ADDR, cg_sock_addr)
> +#endif
>  BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_IN, lwt_in)
>  BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_OUT, lwt_out)
>  BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_XMIT, lwt_xmit)
> diff --git a/net/core/filter.c b/net/core/filter.c
> index f9348806e843..6a390e519431 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5315,6 +5315,7 @@ bpf_base_func_proto(enum bpf_func_id func_id)
>         }
>  }
>
> +#ifdef CONFIG_CGROUP_BPF
>  static const struct bpf_func_proto *
>  sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  {
> @@ -5364,6 +5365,7 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>                 return bpf_base_func_proto(func_id);
>         }
>  }
> +#endif
>
>  static const struct bpf_func_proto *
>  sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> @@ -5382,6 +5384,7 @@ sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>         }
>  }
>
> +#ifdef CONFIG_CGROUP_BPF
>  static const struct bpf_func_proto *
>  cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)

I don't think it's worth uglifying the code like this.
I prefer to leave it as-is.

Re: [PATCH bpf-next 6/6] bpf: BPF_PROG_TYPE_CGROUP_{SKB,SOCK,SOCK_ADDR} require cgroups enabled

[Stanislav Fomichev]

On 12/14, Alexei Starovoitov wrote:
> On Thu, Dec 13, 2018 at 11:03 AM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > There is no way to exercise appropriate attach points without cgroups
> > enabled. This lets test_verifier correctly skip tests for these
> > prog_types if kernel was compiled without BPF cgroup support.
> >
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> >  include/linux/bpf_types.h |  2 ++
> >  net/core/filter.c         | 18 ++++++++++++++++++
> >  2 files changed, 20 insertions(+)
> >
> > diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> > index 44d9ab4809bd..08bf2f1fe553 100644
> > --- a/include/linux/bpf_types.h
> > +++ b/include/linux/bpf_types.h
> > @@ -6,9 +6,11 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_SOCKET_FILTER, sk_filter)
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_SCHED_CLS, tc_cls_act)
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_SCHED_ACT, tc_cls_act)
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_XDP, xdp)
> > +#ifdef CONFIG_CGROUP_BPF
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SKB, cg_skb)
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCK, cg_sock)
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCK_ADDR, cg_sock_addr)
> > +#endif
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_IN, lwt_in)
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_OUT, lwt_out)
> >  BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_XMIT, lwt_xmit)
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index f9348806e843..6a390e519431 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -5315,6 +5315,7 @@ bpf_base_func_proto(enum bpf_func_id func_id)
> >         }
> >  }
> >
> > +#ifdef CONFIG_CGROUP_BPF
> >  static const struct bpf_func_proto *
> >  sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> >  {
> > @@ -5364,6 +5365,7 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> >                 return bpf_base_func_proto(func_id);
> >         }
> >  }
> > +#endif
> >
> >  static const struct bpf_func_proto *
> >  sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > @@ -5382,6 +5384,7 @@ sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> >         }
> >  }
> >
> > +#ifdef CONFIG_CGROUP_BPF
> >  static const struct bpf_func_proto *
> >  cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> 
> I don't think it's worth uglifying the code like this.
> I prefer to leave it as-is.
Sure, up to you. I mostly included it for completeness sake. I tested on
two configs: the first one is allyesbpf, the second one is minimal set
of bpf features and no cgroups.

(For my usecase cgroups and hence these prog types are always enabled,
so it doesn't matter for me).

Re: [PATCH bpf-next 6/6] bpf: BPF_PROG_TYPE_CGROUP_{SKB,SOCK,SOCK_ADDR} require cgroups enabled

[Stanislav Fomichev]

On 12/15, Stanislav Fomichev wrote:
> On 12/14, Alexei Starovoitov wrote:
> > On Thu, Dec 13, 2018 at 11:03 AM Stanislav Fomichev <sdf@google.com> wrote:
> > >
> > > There is no way to exercise appropriate attach points without cgroups
> > > enabled. This lets test_verifier correctly skip tests for these
> > > prog_types if kernel was compiled without BPF cgroup support.
> > >
> > > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > > ---
> > >  include/linux/bpf_types.h |  2 ++
> > >  net/core/filter.c         | 18 ++++++++++++++++++
> > >  2 files changed, 20 insertions(+)
> > >
> > > diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> > > index 44d9ab4809bd..08bf2f1fe553 100644
> > > --- a/include/linux/bpf_types.h
> > > +++ b/include/linux/bpf_types.h
> > > @@ -6,9 +6,11 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_SOCKET_FILTER, sk_filter)
> > >  BPF_PROG_TYPE(BPF_PROG_TYPE_SCHED_CLS, tc_cls_act)
> > >  BPF_PROG_TYPE(BPF_PROG_TYPE_SCHED_ACT, tc_cls_act)
> > >  BPF_PROG_TYPE(BPF_PROG_TYPE_XDP, xdp)
> > > +#ifdef CONFIG_CGROUP_BPF
> > >  BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SKB, cg_skb)
> > >  BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCK, cg_sock)
> > >  BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCK_ADDR, cg_sock_addr)
> > > +#endif
> > >  BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_IN, lwt_in)
> > >  BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_OUT, lwt_out)
> > >  BPF_PROG_TYPE(BPF_PROG_TYPE_LWT_XMIT, lwt_xmit)
> > > diff --git a/net/core/filter.c b/net/core/filter.c
> > > index f9348806e843..6a390e519431 100644
> > > --- a/net/core/filter.c
> > > +++ b/net/core/filter.c
> > > @@ -5315,6 +5315,7 @@ bpf_base_func_proto(enum bpf_func_id func_id)
> > >         }
> > >  }
> > >
> > > +#ifdef CONFIG_CGROUP_BPF
> > >  static const struct bpf_func_proto *
> > >  sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > >  {
> > > @@ -5364,6 +5365,7 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > >                 return bpf_base_func_proto(func_id);
> > >         }
> > >  }
> > > +#endif
> > >
> > >  static const struct bpf_func_proto *
> > >  sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > > @@ -5382,6 +5384,7 @@ sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > >         }
> > >  }
> > >
> > > +#ifdef CONFIG_CGROUP_BPF
> > >  static const struct bpf_func_proto *
> > >  cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > 
> > I don't think it's worth uglifying the code like this.
> > I prefer to leave it as-is.
> Sure, up to you. I mostly included it for completeness sake. I tested on
> two configs: the first one is allyesbpf, the second one is minimal set
> of bpf features and no cgroups.
> 
> (For my usecase cgroups and hence these prog types are always enabled,
> so it doesn't matter for me).
On a second thought, I think I can just have a single ifdef in the
bpf_types.h and don't touch the C file. That should have the same effect
without too much ugliness in the C file. I'll send a v2 shortly.