diff mbox series

[bpf-next,4/4] bpf: try to use existing cgroup storage in bpf_prog_test_run_skb

Message ID 20200713182520.97606-5-zeil@yandex-team.ru
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series bpf: cgroup skb improvements for bpf_prog_test_run | expand

Commit Message

Dmitry Yakunin July 13, 2020, 6:25 p.m. UTC 
Now we cannot check results in cgroup storage after running
BPF_PROG_TEST_RUN command because it allocates dummy cgroup storage
during test. This patch implements simple logic for searching already
allocated cgroup storage through iterating effective programs of current
cgroup and finding the first match. If match is not found fallback to
temporary storage is happened.

Signed-off-by: Dmitry Yakunin <zeil@yandex-team.ru>
---
 net/bpf/test_run.c                                 | 53 ++++++++++++++-
 .../selftests/bpf/prog_tests/cgroup_skb_prog_run.c | 78 ++++++++++++++++++++++
 2 files changed, 128 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_skb_prog_run.c

Comments

kernel test robot July 13, 2020, 10:36 p.m. UTC | #1 
Hi Dmitry,

Thank you for the patch! Yet something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Dmitry-Yakunin/bpf-cgroup-skb-improvements-for-bpf_prog_test_run/20200714-022728
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: m68k-sun3_defconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        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
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k 

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

All error/warnings (new ones prefixed by >>):

   net/bpf/test_run.c: In function 'bpf_prog_find_active_storage':
>> net/bpf/test_run.c:47:9: error: implicit declaration of function 'task_dfl_cgroup' [-Werror=implicit-function-declaration]
      47 |  cgrp = task_dfl_cgroup(current);
         |         ^~~~~~~~~~~~~~~
>> net/bpf/test_run.c:47:7: warning: assignment to 'struct cgroup *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
      47 |  cgrp = task_dfl_cgroup(current);
         |       ^
>> net/bpf/test_run.c:50:13: error: dereferencing pointer to incomplete type 'struct cgroup'
      50 |         cgrp->bpf.effective[BPF_CGROUP_INET_INGRESS]);
         |             ^~
   net/bpf/test_run.c: In function 'bpf_test_run':
   net/bpf/test_run.c:67:8: error: implicit declaration of function 'bpf_cgroup_storages_alloc'; did you mean 'bpf_cgroup_storage_alloc'? [-Werror=implicit-function-declaration]
      67 |  ret = bpf_cgroup_storages_alloc(dummy_storage, prog);
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~
         |        bpf_cgroup_storage_alloc
   net/bpf/test_run.c:115:2: error: implicit declaration of function 'bpf_cgroup_storages_free'; did you mean 'bpf_cgroup_storage_free'? [-Werror=implicit-function-declaration]
     115 |  bpf_cgroup_storages_free(dummy_storage);
         |  ^~~~~~~~~~~~~~~~~~~~~~~~
         |  bpf_cgroup_storage_free
   cc1: some warnings being treated as errors

vim +/task_dfl_cgroup +47 net/bpf/test_run.c

    38	
    39	static struct bpf_cgroup_storage **bpf_prog_find_active_storage(struct bpf_prog *prog)
    40	{
    41		struct bpf_prog_array_item *item;
    42		struct cgroup *cgrp;
    43	
    44		if (prog->type != BPF_PROG_TYPE_CGROUP_SKB)
    45			return NULL;
    46	
  > 47		cgrp = task_dfl_cgroup(current);
    48	
    49		item = bpf_prog_find_active(prog,
  > 50					    cgrp->bpf.effective[BPF_CGROUP_INET_INGRESS]);
    51		if (!item)
    52			item = bpf_prog_find_active(prog,
    53						    cgrp->bpf.effective[BPF_CGROUP_INET_EGRESS]);
    54	
    55		return item ? item->cgroup_storage : NULL;
    56	}
    57	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot July 14, 2020, 1:14 a.m. UTC | #2 
Hi Dmitry,

Thank you for the patch! Yet something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Dmitry-Yakunin/bpf-cgroup-skb-improvements-for-bpf_prog_test_run/20200714-022728
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: x86_64-rhel-7.6 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

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

All errors (new ones prefixed by >>):

   net/bpf/test_run.c: In function 'bpf_prog_find_active_storage':
>> net/bpf/test_run.c:50:18: error: 'struct cgroup_bpf' has no member named 'effective'
      50 |         cgrp->bpf.effective[BPF_CGROUP_INET_INGRESS]);
         |                  ^
   net/bpf/test_run.c:53:19: error: 'struct cgroup_bpf' has no member named 'effective'
      53 |          cgrp->bpf.effective[BPF_CGROUP_INET_EGRESS]);
         |                   ^
   net/bpf/test_run.c: In function 'bpf_test_run':
   net/bpf/test_run.c:67:8: error: implicit declaration of function 'bpf_cgroup_storages_alloc'; did you mean 'bpf_cgroup_storage_alloc'? [-Werror=implicit-function-declaration]
      67 |  ret = bpf_cgroup_storages_alloc(dummy_storage, prog);
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~
         |        bpf_cgroup_storage_alloc
   net/bpf/test_run.c:115:2: error: implicit declaration of function 'bpf_cgroup_storages_free'; did you mean 'bpf_cgroup_storage_free'? [-Werror=implicit-function-declaration]
     115 |  bpf_cgroup_storages_free(dummy_storage);
         |  ^~~~~~~~~~~~~~~~~~~~~~~~
         |  bpf_cgroup_storage_free
   cc1: some warnings being treated as errors

vim +50 net/bpf/test_run.c

    38	
    39	static struct bpf_cgroup_storage **bpf_prog_find_active_storage(struct bpf_prog *prog)
    40	{
    41		struct bpf_prog_array_item *item;
    42		struct cgroup *cgrp;
    43	
    44		if (prog->type != BPF_PROG_TYPE_CGROUP_SKB)
    45			return NULL;
    46	
    47		cgrp = task_dfl_cgroup(current);
    48	
    49		item = bpf_prog_find_active(prog,
  > 50					    cgrp->bpf.effective[BPF_CGROUP_INET_INGRESS]);
    51		if (!item)
    52			item = bpf_prog_find_active(prog,
    53						    cgrp->bpf.effective[BPF_CGROUP_INET_EGRESS]);
    54	
    55		return item ? item->cgroup_storage : NULL;
    56	}
    57	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot July 14, 2020, 6:04 a.m. UTC | #3 
Hi Dmitry,

Thank you for the patch! Perhaps something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Dmitry-Yakunin/bpf-cgroup-skb-improvements-for-bpf_prog_test_run/20200714-022728
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: microblaze-randconfig-s031-20200714 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.2-41-g14e84ffc-dirty
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=microblaze 

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


sparse warnings: (new ones prefixed by >>)

>> net/bpf/test_run.c:25:17: sparse: sparse: incompatible types in comparison expression (different address spaces):
>> net/bpf/test_run.c:25:17: sparse:    struct bpf_prog_array [noderef] __rcu *
>> net/bpf/test_run.c:25:17: sparse:    struct bpf_prog_array *
   net/bpf/test_run.c:50:46: sparse: sparse: using member 'effective' in incomplete struct cgroup_bpf
   net/bpf/test_run.c:53:54: sparse: sparse: using member 'effective' in incomplete struct cgroup_bpf
   net/bpf/test_run.c:67:15: sparse: sparse: undefined identifier 'bpf_cgroup_storages_alloc'
   net/bpf/test_run.c:115:9: sparse: sparse: undefined identifier 'bpf_cgroup_storages_free'

vim +25 net/bpf/test_run.c

    17	
    18	static struct bpf_prog_array_item *bpf_prog_find_active(struct bpf_prog *prog,
    19								struct bpf_prog_array *effective)
    20	{
    21		struct bpf_prog_array_item *item;
    22		struct bpf_prog_array *array;
    23		struct bpf_prog *p;
    24	
  > 25		array = rcu_dereference(effective);
    26		if (!array)
    27			return NULL;
    28	
    29		item = &array->items[0];
    30		while ((p = READ_ONCE(item->prog))) {
    31			if (p == prog)
    32				return item;
    33			item++;
    34		}
    35	
    36		return NULL;
    37	}
    38	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot July 14, 2020, 7:09 a.m. UTC | #4 
Hi Dmitry,

Thank you for the patch! Perhaps something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Dmitry-Yakunin/bpf-cgroup-skb-improvements-for-bpf_prog_test_run/20200714-022728
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: x86_64-randconfig-s022-20200714 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.2-41-g14e84ffc-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

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


sparse warnings: (new ones prefixed by >>)

   net/bpf/test_run.c:25:17: sparse: sparse: incompatible types in comparison expression (different address spaces):
   net/bpf/test_run.c:25:17: sparse:    struct bpf_prog_array [noderef] __rcu *
   net/bpf/test_run.c:25:17: sparse:    struct bpf_prog_array *
>> net/bpf/test_run.c:50:56: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected struct bpf_prog_array *[addressable] effective @@     got struct bpf_prog_array [noderef] __rcu * @@
>> net/bpf/test_run.c:50:56: sparse:     expected struct bpf_prog_array *[addressable] effective
>> net/bpf/test_run.c:50:56: sparse:     got struct bpf_prog_array [noderef] __rcu *
   net/bpf/test_run.c:53:64: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected struct bpf_prog_array *[addressable] effective @@     got struct bpf_prog_array [noderef] __rcu * @@
   net/bpf/test_run.c:53:64: sparse:     expected struct bpf_prog_array *[addressable] effective
   net/bpf/test_run.c:53:64: sparse:     got struct bpf_prog_array [noderef] __rcu *

vim +50 net/bpf/test_run.c

    38	
    39	static struct bpf_cgroup_storage **bpf_prog_find_active_storage(struct bpf_prog *prog)
    40	{
    41		struct bpf_prog_array_item *item;
    42		struct cgroup *cgrp;
    43	
    44		if (prog->type != BPF_PROG_TYPE_CGROUP_SKB)
    45			return NULL;
    46	
    47		cgrp = task_dfl_cgroup(current);
    48	
    49		item = bpf_prog_find_active(prog,
  > 50					    cgrp->bpf.effective[BPF_CGROUP_INET_INGRESS]);
    51		if (!item)
    52			item = bpf_prog_find_active(prog,
    53						    cgrp->bpf.effective[BPF_CGROUP_INET_EGRESS]);
    54	
    55		return item ? item->cgroup_storage : NULL;
    56	}
    57	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 5c4835c..16808cb 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -15,15 +15,56 @@ 
 #define CREATE_TRACE_POINTS
 #include <trace/events/bpf_test_run.h>
 
+static struct bpf_prog_array_item *bpf_prog_find_active(struct bpf_prog *prog,
+							struct bpf_prog_array *effective)
+{
+	struct bpf_prog_array_item *item;
+	struct bpf_prog_array *array;
+	struct bpf_prog *p;
+
+	array = rcu_dereference(effective);
+	if (!array)
+		return NULL;
+
+	item = &array->items[0];
+	while ((p = READ_ONCE(item->prog))) {
+		if (p == prog)
+			return item;
+		item++;
+	}
+
+	return NULL;
+}
+
+static struct bpf_cgroup_storage **bpf_prog_find_active_storage(struct bpf_prog *prog)
+{
+	struct bpf_prog_array_item *item;
+	struct cgroup *cgrp;
+
+	if (prog->type != BPF_PROG_TYPE_CGROUP_SKB)
+		return NULL;
+
+	cgrp = task_dfl_cgroup(current);
+
+	item = bpf_prog_find_active(prog,
+				    cgrp->bpf.effective[BPF_CGROUP_INET_INGRESS]);
+	if (!item)
+		item = bpf_prog_find_active(prog,
+					    cgrp->bpf.effective[BPF_CGROUP_INET_EGRESS]);
+
+	return item ? item->cgroup_storage : NULL;
+}
+
 static int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat,
 			u32 *retval, u32 *time, bool xdp)
 {
-	struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = { NULL };
+	struct bpf_cgroup_storage *dummy_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = { NULL };
+	struct bpf_cgroup_storage **storage = dummy_storage;
 	u64 time_start, time_spent = 0;
 	int ret = 0;
 	u32 i;
 
-	ret = bpf_cgroup_storages_alloc(storage, prog);
+	ret = bpf_cgroup_storages_alloc(dummy_storage, prog);
 	if (ret)
 		return ret;
 
@@ -31,6 +72,9 @@  static int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat,
 		repeat = 1;
 
 	rcu_read_lock();
+	storage = bpf_prog_find_active_storage(prog);
+	if (!storage)
+		storage = dummy_storage;
 	migrate_disable();
 	time_start = ktime_get_ns();
 	for (i = 0; i < repeat; i++) {
@@ -54,6 +98,9 @@  static int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat,
 			cond_resched();
 
 			rcu_read_lock();
+			storage = bpf_prog_find_active_storage(prog);
+			if (!storage)
+				storage = dummy_storage;
 			migrate_disable();
 			time_start = ktime_get_ns();
 		}
@@ -65,7 +112,7 @@  static int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat,
 	do_div(time_spent, repeat);
 	*time = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
 
-	bpf_cgroup_storages_free(storage);
+	bpf_cgroup_storages_free(dummy_storage);
 
 	return ret;
 }
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_skb_prog_run.c b/tools/testing/selftests/bpf/prog_tests/cgroup_skb_prog_run.c
new file mode 100644
index 0000000..12ca881
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_skb_prog_run.c
@@ -0,0 +1,78 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+
+#include "cgroup_helpers.h"
+#include "network_helpers.h"
+
+static char bpf_log_buf[BPF_LOG_BUF_SIZE];
+
+void test_cgroup_skb_prog_run(void)
+{
+	struct bpf_insn prog[] = {
+		BPF_LD_MAP_FD(BPF_REG_1, 0), /* map fd */
+		BPF_MOV64_IMM(BPF_REG_2, 0), /* flags, not used */
+		BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_get_local_storage),
+		BPF_MOV64_IMM(BPF_REG_1, 1),
+		BPF_RAW_INSN(BPF_STX | BPF_XADD | BPF_W, BPF_REG_0, BPF_REG_1, 0, 0),
+
+		BPF_MOV64_IMM(BPF_REG_0, 1), /* r0 = 1 */
+		BPF_EXIT_INSN(),
+	};
+	size_t insns_cnt = sizeof(prog) / sizeof(struct bpf_insn);
+	int storage_fd = -1, prog_fd = -1, cg_fd = -1;
+	struct bpf_cgroup_storage_key key;
+	__u32 duration, retval, size;
+	char buf[128];
+	__u64 value;
+	int err;
+
+	storage_fd = bpf_create_map(BPF_MAP_TYPE_CGROUP_STORAGE,
+				    sizeof(struct bpf_cgroup_storage_key),
+				    8, 0, 0);
+	if (CHECK(storage_fd < 0, "create_map", "%s\n", strerror(errno)))
+		goto out;
+
+	prog[0].imm = storage_fd;
+
+	prog_fd = bpf_load_program(BPF_PROG_TYPE_CGROUP_SKB,
+				   prog, insns_cnt, "GPL", 0,
+				   bpf_log_buf, BPF_LOG_BUF_SIZE);
+	if (CHECK(prog_fd < 0, "prog_load",
+		  "verifier output:\n%s\n-------\n", bpf_log_buf))
+		goto out;
+
+	if (CHECK_FAIL(setup_cgroup_environment()))
+		goto out;
+
+	cg_fd = create_and_get_cgroup("/cg");
+	if (CHECK_FAIL(cg_fd < 0))
+		goto out;
+
+	if (CHECK_FAIL(join_cgroup("/cg")))
+		goto out;
+
+	if (CHECK(bpf_prog_attach(prog_fd, cg_fd, BPF_CGROUP_INET_EGRESS, 0),
+		  "prog_attach", "%s\n", strerror(errno)))
+		goto out;
+
+	err = bpf_prog_test_run(prog_fd, NUM_ITER, &pkt_v4, sizeof(pkt_v4),
+				buf, &size, &retval, &duration);
+	CHECK(err || retval != 1, "prog_test_run",
+	      "err %d errno %d retval %d\n", err, errno, retval);
+
+	/* check that cgroup storage results are available after test run */
+
+	err = bpf_map_get_next_key(storage_fd, NULL, &key);
+	CHECK(err, "map_get_next_key", "%s\n", strerror(errno));
+
+	err = bpf_map_lookup_elem(storage_fd, &key, &value);
+	CHECK(err || value != NUM_ITER,
+	      "map_lookup_elem",
+	      "err %d errno %d cnt %lld(%d)\n", err, errno, value, NUM_ITER);
+out:
+	close(storage_fd);
+	close(prog_fd);
+	close(cg_fd);
+	cleanup_cgroup_environment();
+}