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 |
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
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
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
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 --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(); +}
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