[PATCH bpf-next v2] bpf testing: permit ingress_ifindex in bpf_prog_test_run_xattr

[PATCH bpf-next v2] bpf testing: permit ingress_ifindex in bpf_prog_test_run_xattr

[Neil Spring]

bpf_prog_test_run_xattr takes a struct __sk_buff, but did not permit
that __skbuff to include an nonzero ingress_ifindex.

This patch updates to allow ingress_ifindex, convert the __sk_buff field to
sk_buff (skb_iif) and back, and test that the value is present from
tested bpf.  The test sets an unlikely distinct value for ingress_ifindex
(11) from ifindex (1), but that seems in keeping with the rest of the
synthetic fields.

Adding this support allows testing BPF that operates differently on
incoming and outgoing skbs by discriminating on this field.

Signed-off-by: Neil Spring <ntspring@fb.com>
---
v2 - correct mistaken removal of a prior test of ifindex

 net/bpf/test_run.c                               | 8 +++-----
 tools/testing/selftests/bpf/prog_tests/skb_ctx.c | 5 +++++
 tools/testing/selftests/bpf/progs/test_skb_ctx.c | 4 ++++
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 4b855af267b1..a322578efa5f 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -483,11 +483,7 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
 		return -EINVAL;
 
 	/* priority is allowed */
-
-	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, priority),
-			   offsetof(struct __sk_buff, ifindex)))
-		return -EINVAL;
-
+	/* ingress_ifindex is allowed */
 	/* ifindex is allowed */
 
 	if (!range_is_zero(__skb, offsetofend(struct __sk_buff, ifindex),
@@ -516,6 +512,7 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
 
 	skb->mark = __skb->mark;
 	skb->priority = __skb->priority;
+	skb->skb_iif = __skb->ingress_ifindex;
 	skb->tstamp = __skb->tstamp;
 	memcpy(&cb->data, __skb->cb, QDISC_CB_PRIV_LEN);
 
@@ -545,6 +542,7 @@ static void convert_skb_to___skb(struct sk_buff *skb, struct __sk_buff *__skb)
 
 	__skb->mark = skb->mark;
 	__skb->priority = skb->priority;
+	__skb->ingress_ifindex = skb->skb_iif;
 	__skb->ifindex = skb->dev->ifindex;
 	__skb->tstamp = skb->tstamp;
 	memcpy(__skb->cb, &cb->data, QDISC_CB_PRIV_LEN);
diff --git a/tools/testing/selftests/bpf/prog_tests/skb_ctx.c b/tools/testing/selftests/bpf/prog_tests/skb_ctx.c
index fafeddaad6a9..878401709d8a 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,
+		.ingress_ifindex = 11,
 		.ifindex = 1,
 		.tstamp = 7,
 		.wire_len = 100,
@@ -97,6 +98,10 @@ void test_skb_ctx(void)
 		   "ctx_out_ifindex",
 		   "skb->ifindex == %d, expected %d\n",
 		   skb.ifindex, 1);
+	CHECK_ATTR(skb.ifindex != 11,
+		   "ctx_out_ifindex",
+		   "skb->ifindex == %d, expected %d\n",
+		   skb.ifindex, 11);
 	CHECK_ATTR(skb.tstamp != 8,
 		   "ctx_out_tstamp",
 		   "skb->tstamp == %lld, expected %d\n",
diff --git a/tools/testing/selftests/bpf/progs/test_skb_ctx.c b/tools/testing/selftests/bpf/progs/test_skb_ctx.c
index b02ea589ce7e..bbd5a9c1c4df 100644
--- a/tools/testing/selftests/bpf/progs/test_skb_ctx.c
+++ b/tools/testing/selftests/bpf/progs/test_skb_ctx.c
@@ -25,6 +25,10 @@ int process(struct __sk_buff *skb)
 		return 1;
 	if (skb->gso_size != 10)
 		return 1;
+	if (skb->ingress_ifindex != 11)
+		return 1;
+	if (skb->ifindex != 1)
+		return 1;
 
 	return 0;
 }
-- 
2.30.2

Re: [PATCH bpf-next v2] bpf testing: permit ingress_ifindex in bpf_prog_test_run_xattr

[Daniel Borkmann]

On 8/28/21 3:14 AM, Neil Spring wrote:
> bpf_prog_test_run_xattr takes a struct __sk_buff, but did not permit
> that __skbuff to include an nonzero ingress_ifindex.
> 
> This patch updates to allow ingress_ifindex, convert the __sk_buff field to
> sk_buff (skb_iif) and back, and test that the value is present from
> tested bpf.  The test sets an unlikely distinct value for ingress_ifindex
> (11) from ifindex (1), but that seems in keeping with the rest of the
> synthetic fields.
> 
> Adding this support allows testing BPF that operates differently on
> incoming and outgoing skbs by discriminating on this field.
> 
> Signed-off-by: Neil Spring <ntspring@fb.com>

This triggers CI test suite failure, pls double check and fix:

   [...]
   test_skb_ctx:PASS:load 0 nsec
   test_skb_ctx:PASS:ctx_size_in 0 nsec
   test_skb_ctx:PASS:ctx_size_out 0 nsec
   test_skb_ctx:PASS:len 0 nsec
   test_skb_ctx:PASS:tc_index 0 nsec
   test_skb_ctx:PASS:hash 0 nsec
   test_skb_ctx:PASS:sk 0 nsec
   test_skb_ctx:PASS:run 146200 nsec
   test_skb_ctx:PASS:ctx_size_out 146200 nsec
   test_skb_ctx:PASS:ctx_out_cb 146200 nsec
   test_skb_ctx:PASS:ctx_out_cb 146200 nsec
   test_skb_ctx:PASS:ctx_out_cb 146200 nsec
   test_skb_ctx:PASS:ctx_out_cb 146200 nsec
   test_skb_ctx:PASS:ctx_out_cb 146200 nsec
   test_skb_ctx:PASS:ctx_out_priority 146200 nsec
   test_skb_ctx:PASS:ctx_out_ifindex 146200 nsec
   test_skb_ctx:FAIL:ctx_out_ifindex skb->ifindex == 1, expected 11
   test_skb_ctx:PASS:ctx_out_tstamp 146200 nsec
   test_skb_ctx:PASS:ctx_out_mark 146200 nsec
   #111 skb_ctx:FAIL
   [...]

https://github.com/kernel-patches/bpf/runs/3466106681?check_suite_focus=true

Thanks,
Daniel