[PATCH bpf-next 0/4] bpf: cgroup skb improvements for bpf_prog_test_run

[PATCH bpf-next 0/4] bpf: cgroup skb improvements for bpf_prog_test_run

[Dmitry Yakunin]

This patchset contains some improvements for testing cgroup/skb programs
through BPF_PROG_TEST_RUN command.

Dmitry Yakunin (4):
  bpf: setup socket family and addresses in bpf_prog_test_run_skb
  bpf: allow to specify ifindex for skb in bpf_prog_test_run_skb
  bpf: export some cgroup storages allocation helpers for reusing
  bpf: try to use existing cgroup storage in bpf_prog_test_run_skb

 include/linux/bpf-cgroup.h                         |  27 ++++++
 kernel/bpf/cgroup.c                                |  25 -----
 net/bpf/test_run.c                                 | 102 ++++++++++++++++++---
 .../selftests/bpf/prog_tests/cgroup_skb_prog_run.c |  78 ++++++++++++++++
 tools/testing/selftests/bpf/prog_tests/skb_ctx.c   |   5 +
 5 files changed, 197 insertions(+), 40 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_skb_prog_run.c

-- 
2.7.4

[PATCH bpf-next 1/4] bpf: setup socket family and addresses in bpf_prog_test_run_skb

[Dmitry Yakunin]

Now it's impossible to test all branches of cgroup_skb bpf program which
accesses skb->family and skb->{local,remote}_ip{4,6} fields because they
are zeroed during socket allocation. This commit fills socket family and
addresses from related fields in constructed skb.

Signed-off-by: Dmitry Yakunin <zeil@yandex-team.ru>
---
 net/bpf/test_run.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index bfd4ccd..a58b399 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -432,6 +432,21 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 	skb->protocol = eth_type_trans(skb, current->nsproxy->net_ns->loopback_dev);
 	skb_reset_network_header(skb);
 
+	switch (skb->protocol) {
+	case htons(ETH_P_IP):
+		sk->sk_family = AF_INET;
+		sk->sk_rcv_saddr = ip_hdr(skb)->saddr;
+		sk->sk_daddr = ip_hdr(skb)->daddr;
+		break;
+	case htons(ETH_P_IPV6):
+		sk->sk_family = AF_INET6;
+		sk->sk_v6_rcv_saddr = ipv6_hdr(skb)->saddr;
+		sk->sk_v6_daddr = ipv6_hdr(skb)->daddr;
+		break;
+	default:
+		break;
+	}
+
 	if (is_l2)
 		__skb_push(skb, hh_len);
 	if (is_direct_pkt_access)
-- 
2.7.4

[PATCH bpf-next 2/4] bpf: allow to specify ifindex for skb in bpf_prog_test_run_skb

[Dmitry Yakunin]

Now skb->dev is unconditionally set to the loopback device in current net
namespace. But if we want to test bpf program which contains code branch
based on ifindex condition (eg filters out localhost packets) it is useful
to allow specifying of ifindex from userspace. This patch adds such option
through ctx_in (__sk_buff) parameter.

Signed-off-by: Dmitry Yakunin <zeil@yandex-team.ru>
---
 net/bpf/test_run.c                               | 22 ++++++++++++++++++++--
 tools/testing/selftests/bpf/prog_tests/skb_ctx.c |  5 +++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index a58b399..1e10a7e 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -310,6 +310,12 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
 	/* priority is allowed */
 
 	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, priority),
+			   offsetof(struct __sk_buff, ifindex)))
+		return -EINVAL;
+
+	/* ifindex is allowed */
+
+	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, ifindex),
 			   offsetof(struct __sk_buff, cb)))
 		return -EINVAL;
 
@@ -364,6 +370,7 @@ static void convert_skb_to___skb(struct sk_buff *skb, struct __sk_buff *__skb)
 
 	__skb->mark = skb->mark;
 	__skb->priority = skb->priority;
+	__skb->ifindex = skb->dev->ifindex;
 	__skb->tstamp = skb->tstamp;
 	memcpy(__skb->cb, &cb->data, QDISC_CB_PRIV_LEN);
 	__skb->wire_len = cb->pkt_len;
@@ -374,6 +381,8 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 			  union bpf_attr __user *uattr)
 {
 	bool is_l2 = false, is_direct_pkt_access = false;
+	struct net *net = current->nsproxy->net_ns;
+	struct net_device *dev = net->loopback_dev;
 	u32 size = kattr->test.data_size_in;
 	u32 repeat = kattr->test.repeat;
 	struct __sk_buff *ctx = NULL;
@@ -415,7 +424,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 		kfree(ctx);
 		return -ENOMEM;
 	}
-	sock_net_set(sk, current->nsproxy->net_ns);
+	sock_net_set(sk, net);
 	sock_init_data(NULL, sk);
 
 	skb = build_skb(data, 0);
@@ -429,7 +438,14 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 
 	skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
 	__skb_put(skb, size);
-	skb->protocol = eth_type_trans(skb, current->nsproxy->net_ns->loopback_dev);
+	if (ctx && ctx->ifindex > 1) {
+		dev = dev_get_by_index(net, ctx->ifindex);
+		if (!dev) {
+			ret = -ENODEV;
+			goto out;
+		}
+	}
+	skb->protocol = eth_type_trans(skb, dev);
 	skb_reset_network_header(skb);
 
 	switch (skb->protocol) {
@@ -479,6 +495,8 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
 		ret = bpf_ctx_finish(kattr, uattr, ctx,
 				     sizeof(struct __sk_buff));
 out:
+	if (dev && dev != net->loopback_dev)
+		dev_put(dev);
 	kfree_skb(skb);
 	bpf_sk_storage_free(sk);
 	kfree(sk);
diff --git a/tools/testing/selftests/bpf/prog_tests/skb_ctx.c b/tools/testing/selftests/bpf/prog_tests/skb_ctx.c
index 7021b92..25de86a 100644
--- a/tools/testing/selftests/bpf/prog_tests/skb_ctx.c
+++ b/tools/testing/selftests/bpf/prog_tests/skb_ctx.c
@@ -11,6 +11,7 @@ void test_skb_ctx(void)
 		.cb[3] = 4,
 		.cb[4] = 5,
 		.priority = 6,
+		.ifindex = 1,
 		.tstamp = 7,
 		.wire_len = 100,
 		.gso_segs = 8,
@@ -92,6 +93,10 @@ void test_skb_ctx(void)
 		   "ctx_out_priority",
 		   "skb->priority == %d, expected %d\n",
 		   skb.priority, 7);
+	CHECK_ATTR(skb.ifindex != 1,
+		   "ctx_out_ifindex",
+		   "skb->ifindex == %d, expected %d\n",
+		   skb.ifindex, 1);
 	CHECK_ATTR(skb.tstamp != 8,
 		   "ctx_out_tstamp",
 		   "skb->tstamp == %lld, expected %d\n",
-- 
2.7.4

[PATCH bpf-next 3/4] bpf: export some cgroup storages allocation helpers for reusing

[Dmitry Yakunin]

This patch exports bpf_cgroup_storages_alloc and bpf_cgroup_storages_free
helpers to the header file and reuses them in bpf_test_run.

Signed-off-by: Dmitry Yakunin <zeil@yandex-team.ru>
---
 include/linux/bpf-cgroup.h | 27 +++++++++++++++++++++++++++
 kernel/bpf/cgroup.c        | 25 -------------------------
 net/bpf/test_run.c         | 16 ++++------------
 3 files changed, 31 insertions(+), 37 deletions(-)

diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index 2c6f266..8bde9bb 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -175,6 +175,33 @@ int bpf_percpu_cgroup_storage_copy(struct bpf_map *map, void *key, void *value);
 int bpf_percpu_cgroup_storage_update(struct bpf_map *map, void *key,
 				     void *value, u64 flags);
 
+static inline void bpf_cgroup_storages_free(struct bpf_cgroup_storage
+					    *storage[MAX_BPF_CGROUP_STORAGE_TYPE])
+{
+	enum bpf_cgroup_storage_type stype;
+
+	for_each_cgroup_storage_type(stype)
+		bpf_cgroup_storage_free(storage[stype]);
+}
+
+static inline int bpf_cgroup_storages_alloc(struct bpf_cgroup_storage
+					    *storage[MAX_BPF_CGROUP_STORAGE_TYPE],
+					    struct bpf_prog *prog)
+{
+	enum bpf_cgroup_storage_type stype;
+
+	for_each_cgroup_storage_type(stype) {
+		storage[stype] = bpf_cgroup_storage_alloc(prog, stype);
+		if (IS_ERR(storage[stype])) {
+			storage[stype] = NULL;
+			bpf_cgroup_storages_free(storage);
+			return -ENOMEM;
+		}
+	}
+
+	return 0;
+}
+
 /* Wrappers for __cgroup_bpf_run_filter_skb() guarded by cgroup_bpf_enabled. */
 #define BPF_CGROUP_RUN_PROG_INET_INGRESS(sk, skb)			      \
 ({									      \
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index ac53102..e4c2792 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -28,31 +28,6 @@ void cgroup_bpf_offline(struct cgroup *cgrp)
 	percpu_ref_kill(&cgrp->bpf.refcnt);
 }
 
-static void bpf_cgroup_storages_free(struct bpf_cgroup_storage *storages[])
-{
-	enum bpf_cgroup_storage_type stype;
-
-	for_each_cgroup_storage_type(stype)
-		bpf_cgroup_storage_free(storages[stype]);
-}
-
-static int bpf_cgroup_storages_alloc(struct bpf_cgroup_storage *storages[],
-				     struct bpf_prog *prog)
-{
-	enum bpf_cgroup_storage_type stype;
-
-	for_each_cgroup_storage_type(stype) {
-		storages[stype] = bpf_cgroup_storage_alloc(prog, stype);
-		if (IS_ERR(storages[stype])) {
-			storages[stype] = NULL;
-			bpf_cgroup_storages_free(storages);
-			return -ENOMEM;
-		}
-	}
-
-	return 0;
-}
-
 static void bpf_cgroup_storages_assign(struct bpf_cgroup_storage *dst[],
 				       struct bpf_cgroup_storage *src[])
 {
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 1e10a7e..5c4835c 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -19,20 +19,13 @@ 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 };
-	enum bpf_cgroup_storage_type stype;
 	u64 time_start, time_spent = 0;
 	int ret = 0;
 	u32 i;
 
-	for_each_cgroup_storage_type(stype) {
-		storage[stype] = bpf_cgroup_storage_alloc(prog, stype);
-		if (IS_ERR(storage[stype])) {
-			storage[stype] = NULL;
-			for_each_cgroup_storage_type(stype)
-				bpf_cgroup_storage_free(storage[stype]);
-			return -ENOMEM;
-		}
-	}
+	ret = bpf_cgroup_storages_alloc(storage, prog);
+	if (ret)
+		return ret;
 
 	if (!repeat)
 		repeat = 1;
@@ -72,8 +65,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;
 
-	for_each_cgroup_storage_type(stype)
-		bpf_cgroup_storage_free(storage[stype]);
+	bpf_cgroup_storages_free(storage);
 
 	return ret;
 }
-- 
2.7.4

Re: [PATCH bpf-next 3/4] bpf: export some cgroup storages allocation helpers for reusing

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3013 bytes --]

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_test_run':
>> net/bpf/test_run.c:26:8: error: implicit declaration of function 'bpf_cgroup_storages_alloc'; did you mean 'bpf_cgroup_storage_alloc'? [-Werror=implicit-function-declaration]
      26 |  ret = bpf_cgroup_storages_alloc(storage, prog);
         |        ^~~~~~~~~~~~~~~~~~~~~~~~~
         |        bpf_cgroup_storage_alloc
>> net/bpf/test_run.c:68:2: error: implicit declaration of function 'bpf_cgroup_storages_free'; did you mean 'bpf_cgroup_storage_free'? [-Werror=implicit-function-declaration]
      68 |  bpf_cgroup_storages_free(storage);
         |  ^~~~~~~~~~~~~~~~~~~~~~~~
         |  bpf_cgroup_storage_free
   cc1: some warnings being treated as errors

vim +26 net/bpf/test_run.c

    17	
    18	static int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat,
    19				u32 *retval, u32 *time, bool xdp)
    20	{
    21		struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = { NULL };
    22		u64 time_start, time_spent = 0;
    23		int ret = 0;
    24		u32 i;
    25	
  > 26		ret = bpf_cgroup_storages_alloc(storage, prog);
    27		if (ret)
    28			return ret;
    29	
    30		if (!repeat)
    31			repeat = 1;
    32	
    33		rcu_read_lock();
    34		migrate_disable();
    35		time_start = ktime_get_ns();
    36		for (i = 0; i < repeat; i++) {
    37			bpf_cgroup_storage_set(storage);
    38	
    39			if (xdp)
    40				*retval = bpf_prog_run_xdp(prog, ctx);
    41			else
    42				*retval = BPF_PROG_RUN(prog, ctx);
    43	
    44			if (signal_pending(current)) {
    45				ret = -EINTR;
    46				break;
    47			}
    48	
    49			if (need_resched()) {
    50				time_spent += ktime_get_ns() - time_start;
    51				migrate_enable();
    52				rcu_read_unlock();
    53	
    54				cond_resched();
    55	
    56				rcu_read_lock();
    57				migrate_disable();
    58				time_start = ktime_get_ns();
    59			}
    60		}
    61		time_spent += ktime_get_ns() - time_start;
    62		migrate_enable();
    63		rcu_read_unlock();
    64	
    65		do_div(time_spent, repeat);
    66		*time = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
    67	
  > 68		bpf_cgroup_storages_free(storage);
    69	
    70		return ret;
    71	}
    72	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 49241 bytes --]

Re: [PATCH bpf-next 3/4] bpf: export some cgroup storages allocation helpers for reusing

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 6439 bytes --]

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: mips-randconfig-r034-20200713 (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 02946de3802d3bc65bc9f2eb9b8d4969b5a7add8)
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
        # install mips cross compiling tool for clang build
        # apt-get install binutils-mips-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips 

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:26:8: error: implicit declaration of function 'bpf_cgroup_storages_alloc' [-Werror,-Wimplicit-function-declaration]
           ret = bpf_cgroup_storages_alloc(storage, prog);
                 ^
   net/bpf/test_run.c:26:8: note: did you mean 'bpf_cgroup_storage_alloc'?
   include/linux/bpf-cgroup.h:415:42: note: 'bpf_cgroup_storage_alloc' declared here
   static inline struct bpf_cgroup_storage *bpf_cgroup_storage_alloc(
                                            ^
>> net/bpf/test_run.c:68:2: error: implicit declaration of function 'bpf_cgroup_storages_free' [-Werror,-Wimplicit-function-declaration]
           bpf_cgroup_storages_free(storage);
           ^
   net/bpf/test_run.c:68:2: note: did you mean 'bpf_cgroup_storage_free'?
   include/linux/bpf-cgroup.h:417:20: note: 'bpf_cgroup_storage_free' declared here
   static inline void bpf_cgroup_storage_free(
                      ^
   net/bpf/test_run.c:112:14: warning: no previous prototype for function 'bpf_fentry_test1' [-Wmissing-prototypes]
   int noinline bpf_fentry_test1(int a)
                ^
   net/bpf/test_run.c:112:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int noinline bpf_fentry_test1(int a)
   ^
   static 
   net/bpf/test_run.c:117:14: warning: no previous prototype for function 'bpf_fentry_test2' [-Wmissing-prototypes]
   int noinline bpf_fentry_test2(int a, u64 b)
                ^
   net/bpf/test_run.c:117:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int noinline bpf_fentry_test2(int a, u64 b)
   ^
   static 
   net/bpf/test_run.c:122:14: warning: no previous prototype for function 'bpf_fentry_test3' [-Wmissing-prototypes]
   int noinline bpf_fentry_test3(char a, int b, u64 c)
                ^
   net/bpf/test_run.c:122:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int noinline bpf_fentry_test3(char a, int b, u64 c)
   ^
   static 
   net/bpf/test_run.c:127:14: warning: no previous prototype for function 'bpf_fentry_test4' [-Wmissing-prototypes]
   int noinline bpf_fentry_test4(void *a, char b, int c, u64 d)
                ^
   net/bpf/test_run.c:127:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int noinline bpf_fentry_test4(void *a, char b, int c, u64 d)
   ^
   static 
   net/bpf/test_run.c:132:14: warning: no previous prototype for function 'bpf_fentry_test5' [-Wmissing-prototypes]
   int noinline bpf_fentry_test5(u64 a, void *b, short c, int d, u64 e)
                ^
   net/bpf/test_run.c:132:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int noinline bpf_fentry_test5(u64 a, void *b, short c, int d, u64 e)
   ^
   static 
   net/bpf/test_run.c:137:14: warning: no previous prototype for function 'bpf_fentry_test6' [-Wmissing-prototypes]
   int noinline bpf_fentry_test6(u64 a, void *b, short c, int d, void *e, u64 f)
                ^
   net/bpf/test_run.c:137:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int noinline bpf_fentry_test6(u64 a, void *b, short c, int d, void *e, u64 f)
   ^
   static 
   net/bpf/test_run.c:142:14: warning: no previous prototype for function 'bpf_modify_return_test' [-Wmissing-prototypes]
   int noinline bpf_modify_return_test(int a, int *b)
                ^
   net/bpf/test_run.c:142:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int noinline bpf_modify_return_test(int a, int *b)
   ^
   static 
   7 warnings and 2 errors generated.

vim +/bpf_cgroup_storages_alloc +26 net/bpf/test_run.c

    17	
    18	static int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat,
    19				u32 *retval, u32 *time, bool xdp)
    20	{
    21		struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = { NULL };
    22		u64 time_start, time_spent = 0;
    23		int ret = 0;
    24		u32 i;
    25	
  > 26		ret = bpf_cgroup_storages_alloc(storage, prog);
    27		if (ret)
    28			return ret;
    29	
    30		if (!repeat)
    31			repeat = 1;
    32	
    33		rcu_read_lock();
    34		migrate_disable();
    35		time_start = ktime_get_ns();
    36		for (i = 0; i < repeat; i++) {
    37			bpf_cgroup_storage_set(storage);
    38	
    39			if (xdp)
    40				*retval = bpf_prog_run_xdp(prog, ctx);
    41			else
    42				*retval = BPF_PROG_RUN(prog, ctx);
    43	
    44			if (signal_pending(current)) {
    45				ret = -EINTR;
    46				break;
    47			}
    48	
    49			if (need_resched()) {
    50				time_spent += ktime_get_ns() - time_start;
    51				migrate_enable();
    52				rcu_read_unlock();
    53	
    54				cond_resched();
    55	
    56				rcu_read_lock();
    57				migrate_disable();
    58				time_start = ktime_get_ns();
    59			}
    60		}
    61		time_spent += ktime_get_ns() - time_start;
    62		migrate_enable();
    63		rcu_read_unlock();
    64	
    65		do_div(time_spent, repeat);
    66		*time = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
    67	
  > 68		bpf_cgroup_storages_free(storage);
    69	
    70		return ret;
    71	}
    72	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26790 bytes --]

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

[Dmitry Yakunin]

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

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

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

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3054 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 13230 bytes --]

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

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2608 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 49241 bytes --]

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

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2372 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24972 bytes --]

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

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2641 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31714 bytes --]