* [PATCH bpf-next v2 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs
@ 2024-05-24 11:06 Vadim Fedorenko
2024-05-24 11:06 ` [PATCH bpf-next v2 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option Vadim Fedorenko
2024-05-24 16:15 ` [PATCH bpf-next v2 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Daniel Borkmann
0 siblings, 2 replies; 5+ messages in thread
From: Vadim Fedorenko @ 2024-05-24 11:06 UTC (permalink / raw)
To: Vadim Fedorenko, Martin KaFai Lau, Andrii Nakryiko,
Alexei Starovoitov, Mykola Lysenko, Jakub Kicinski
Cc: Vadim Fedorenko, bpf, netdev
Add special flag to validate that TC BPF program properly updates
checksum information in skb.
Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
---
include/uapi/linux/bpf.h | 2 ++
net/bpf/test_run.c | 17 ++++++++++++++++-
tools/include/uapi/linux/bpf.h | 2 ++
3 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 90706a47f6ff..f7d458d88111 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1425,6 +1425,8 @@ enum {
#define BPF_F_TEST_RUN_ON_CPU (1U << 0)
/* If set, XDP frames will be transmitted after processing */
#define BPF_F_TEST_XDP_LIVE_FRAMES (1U << 1)
+/* If set, apply CHECKSUM_COMPLETE to skb and validate the checksum */
+#define BPF_F_TEST_SKB_CHECKSUM_COMPLETE (1U << 2)
/* type for BPF_ENABLE_STATS */
enum bpf_stats_type {
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index f6aad4ed2ab2..c6189bb9bf67 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -977,7 +977,8 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
void *data;
int ret;
- if (kattr->test.flags || kattr->test.cpu || kattr->test.batch_size)
+ if ((kattr->test.flags & ~BPF_F_TEST_SKB_CHECKSUM_COMPLETE) ||
+ kattr->test.cpu || kattr->test.batch_size)
return -EINVAL;
data = bpf_test_init(kattr, kattr->test.data_size_in,
@@ -1025,6 +1026,12 @@ 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);
+
+ if (kattr->test.flags & BPF_F_TEST_SKB_CHECKSUM_COMPLETE) {
+ skb->csum = skb_checksum(skb, 0, skb->len, 0);
+ skb->ip_summed = CHECKSUM_COMPLETE;
+ }
+
if (ctx && ctx->ifindex > 1) {
dev = dev_get_by_index(net, ctx->ifindex);
if (!dev) {
@@ -1079,6 +1086,15 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
}
convert_skb_to___skb(skb, ctx);
+ if (kattr->test.flags & BPF_F_TEST_SKB_CHECKSUM_COMPLETE) {
+ __wsum csum = skb_checksum(skb, 0, skb->len, 0);
+
+ if (skb->csum != csum) {
+ ret = -EINVAL;
+ goto out;
+ }
+ }
+
size = skb->len;
/* bpf program can never convert linear skb to non-linear */
if (WARN_ON_ONCE(skb_is_nonlinear(skb)))
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 90706a47f6ff..f7d458d88111 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1425,6 +1425,8 @@ enum {
#define BPF_F_TEST_RUN_ON_CPU (1U << 0)
/* If set, XDP frames will be transmitted after processing */
#define BPF_F_TEST_XDP_LIVE_FRAMES (1U << 1)
+/* If set, apply CHECKSUM_COMPLETE to skb and validate the checksum */
+#define BPF_F_TEST_SKB_CHECKSUM_COMPLETE BIT(2)
/* type for BPF_ENABLE_STATS */
enum bpf_stats_type {
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH bpf-next v2 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option
2024-05-24 11:06 [PATCH bpf-next v2 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
@ 2024-05-24 11:06 ` Vadim Fedorenko
2024-05-24 16:18 ` Daniel Borkmann
2024-05-24 16:15 ` [PATCH bpf-next v2 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Daniel Borkmann
1 sibling, 1 reply; 5+ messages in thread
From: Vadim Fedorenko @ 2024-05-24 11:06 UTC (permalink / raw)
To: Vadim Fedorenko, Martin KaFai Lau, Andrii Nakryiko,
Alexei Starovoitov, Mykola Lysenko, Jakub Kicinski
Cc: Vadim Fedorenko, bpf, netdev
Adjust skb program test to run with checksum validation.
Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
---
.../selftests/bpf/prog_tests/test_skb_pkt_end.c | 1 +
tools/testing/selftests/bpf/progs/skb_pkt_end.c | 11 ++++++++++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/test_skb_pkt_end.c b/tools/testing/selftests/bpf/prog_tests/test_skb_pkt_end.c
index ae93411fd582..09ca13bdf6ca 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_skb_pkt_end.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_skb_pkt_end.c
@@ -11,6 +11,7 @@ static int sanity_run(struct bpf_program *prog)
.data_in = &pkt_v4,
.data_size_in = sizeof(pkt_v4),
.repeat = 1,
+ .flags = BPF_F_TEST_SKB_CHECKSUM_COMPLETE,
);
prog_fd = bpf_program__fd(prog);
diff --git a/tools/testing/selftests/bpf/progs/skb_pkt_end.c b/tools/testing/selftests/bpf/progs/skb_pkt_end.c
index db4abd2682fc..3bb4451524a1 100644
--- a/tools/testing/selftests/bpf/progs/skb_pkt_end.c
+++ b/tools/testing/selftests/bpf/progs/skb_pkt_end.c
@@ -33,6 +33,8 @@ int main_prog(struct __sk_buff *skb)
struct iphdr *ip = NULL;
struct tcphdr *tcp;
__u8 proto = 0;
+ int urg_ptr;
+ u32 offset;
if (!(ip = get_iphdr(skb)))
goto out;
@@ -48,7 +50,14 @@ int main_prog(struct __sk_buff *skb)
if (!tcp)
goto out;
- return tcp->urg_ptr;
+ urg_ptr = tcp->urg_ptr;
+
+ /* Checksum validation part */
+ proto++;
+ offset = sizeof(struct ethhdr) + offsetof(struct iphdr, protocol);
+ bpf_skb_store_bytes(skb, offset, &proto, sizeof(proto), BPF_F_RECOMPUTE_CSUM);
+
+ return urg_ptr;
out:
return -1;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next v2 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option
2024-05-24 11:06 ` [PATCH bpf-next v2 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option Vadim Fedorenko
@ 2024-05-24 16:18 ` Daniel Borkmann
2024-05-24 16:23 ` Vadim Fedorenko
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Borkmann @ 2024-05-24 16:18 UTC (permalink / raw)
To: Vadim Fedorenko, Vadim Fedorenko, Martin KaFai Lau,
Andrii Nakryiko, Alexei Starovoitov, Mykola Lysenko,
Jakub Kicinski
Cc: bpf, netdev
On 5/24/24 1:06 PM, Vadim Fedorenko wrote:
> Adjust skb program test to run with checksum validation.
>
> Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
BPF CI complains :
[...]
/tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/test_skb_pkt_end.c:14:12: error: call to undeclared function 'BIT'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
14 | .flags = BPF_F_TEST_SKB_CHECKSUM_COMPLETE,
| ^
/tmp/work/bpf/bpf/tools/include/uapi/linux/bpf.h:1429:42: note: expanded from macro 'BPF_F_TEST_SKB_CHECKSUM_COMPLETE'
1429 | #define BPF_F_TEST_SKB_CHECKSUM_COMPLETE BIT(2)
| ^
1 error generated.
make: *** [Makefile:654: /tmp/work/bpf/bpf/tools/testing/selftests/bpf/test_skb_pkt_end.test.o] Error 1
make: *** Waiting for unfinished jobs....
make: Leaving directory '/tmp/work/bpf/bpf/tools/testing/selftests/bpf'
Error: Process completed with exit code 2.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH bpf-next v2 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option
2024-05-24 16:18 ` Daniel Borkmann
@ 2024-05-24 16:23 ` Vadim Fedorenko
0 siblings, 0 replies; 5+ messages in thread
From: Vadim Fedorenko @ 2024-05-24 16:23 UTC (permalink / raw)
To: Daniel Borkmann, Vadim Fedorenko, Martin KaFai Lau,
Andrii Nakryiko, Alexei Starovoitov, Mykola Lysenko,
Jakub Kicinski
Cc: bpf, netdev
On 24/05/2024 17:18, Daniel Borkmann wrote:
> On 5/24/24 1:06 PM, Vadim Fedorenko wrote:
>> Adjust skb program test to run with checksum validation.
>>
>> Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
>
> BPF CI complains :
>
> [...]
>
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/prog_tests/test_skb_pkt_end.c:14:12: error: call to undeclared function 'BIT'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> 14 | .flags = BPF_F_TEST_SKB_CHECKSUM_COMPLETE,
> | ^
> /tmp/work/bpf/bpf/tools/include/uapi/linux/bpf.h:1429:42: note:
> expanded from macro 'BPF_F_TEST_SKB_CHECKSUM_COMPLETE'
> 1429 | #define BPF_F_TEST_SKB_CHECKSUM_COMPLETE BIT(2)
> | ^
> 1 error generated.
> make: *** [Makefile:654:
> /tmp/work/bpf/bpf/tools/testing/selftests/bpf/test_skb_pkt_end.test.o]
> Error 1
> make: *** Waiting for unfinished jobs....
> make: Leaving directory '/tmp/work/bpf/bpf/tools/testing/selftests/bpf'
> Error: Process completed with exit code 2.
Oops, looks like checkpatch.pl was too smart and replaced original
define with BIT() macro, but in one file only. I'll re-send it with
fixes.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next v2 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs
2024-05-24 11:06 [PATCH bpf-next v2 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
2024-05-24 11:06 ` [PATCH bpf-next v2 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option Vadim Fedorenko
@ 2024-05-24 16:15 ` Daniel Borkmann
1 sibling, 0 replies; 5+ messages in thread
From: Daniel Borkmann @ 2024-05-24 16:15 UTC (permalink / raw)
To: Vadim Fedorenko, Vadim Fedorenko, Martin KaFai Lau,
Andrii Nakryiko, Alexei Starovoitov, Mykola Lysenko,
Jakub Kicinski
Cc: bpf, netdev
On 5/24/24 1:06 PM, Vadim Fedorenko wrote:
> Add special flag to validate that TC BPF program properly updates
> checksum information in skb.
>
> Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
> ---
> include/uapi/linux/bpf.h | 2 ++
> net/bpf/test_run.c | 17 ++++++++++++++++-
> tools/include/uapi/linux/bpf.h | 2 ++
> 3 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 90706a47f6ff..f7d458d88111 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1425,6 +1425,8 @@ enum {
> #define BPF_F_TEST_RUN_ON_CPU (1U << 0)
> /* If set, XDP frames will be transmitted after processing */
> #define BPF_F_TEST_XDP_LIVE_FRAMES (1U << 1)
> +/* If set, apply CHECKSUM_COMPLETE to skb and validate the checksum */
> +#define BPF_F_TEST_SKB_CHECKSUM_COMPLETE (1U << 2)
>
> /* type for BPF_ENABLE_STATS */
> enum bpf_stats_type {
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index f6aad4ed2ab2..c6189bb9bf67 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -977,7 +977,8 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> void *data;
> int ret;
>
> - if (kattr->test.flags || kattr->test.cpu || kattr->test.batch_size)
> + if ((kattr->test.flags & ~BPF_F_TEST_SKB_CHECKSUM_COMPLETE) ||
> + kattr->test.cpu || kattr->test.batch_size)
> return -EINVAL;
>
> data = bpf_test_init(kattr, kattr->test.data_size_in,
> @@ -1025,6 +1026,12 @@ 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);
> +
> + if (kattr->test.flags & BPF_F_TEST_SKB_CHECKSUM_COMPLETE) {
> + skb->csum = skb_checksum(skb, 0, skb->len, 0);
> + skb->ip_summed = CHECKSUM_COMPLETE;
> + }
> +
> if (ctx && ctx->ifindex > 1) {
> dev = dev_get_by_index(net, ctx->ifindex);
> if (!dev) {
> @@ -1079,6 +1086,15 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> }
> convert_skb_to___skb(skb, ctx);
>
> + if (kattr->test.flags & BPF_F_TEST_SKB_CHECKSUM_COMPLETE) {
> + __wsum csum = skb_checksum(skb, 0, skb->len, 0);
> +
> + if (skb->csum != csum) {
> + ret = -EINVAL;
This is very useful. Maybe just a nit that EINVAL seems too generic and some more concrete
signal (e.g. EBADMSG) might be better. Do you also plan to add test infra for other types
like CHECKSUM_PARTIAL?
> + goto out;
> + }
> + }
> +
> size = skb->len;
> /* bpf program can never convert linear skb to non-linear */
> if (WARN_ON_ONCE(skb_is_nonlinear(skb)))
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 90706a47f6ff..f7d458d88111 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -1425,6 +1425,8 @@ enum {
> #define BPF_F_TEST_RUN_ON_CPU (1U << 0)
> /* If set, XDP frames will be transmitted after processing */
> #define BPF_F_TEST_XDP_LIVE_FRAMES (1U << 1)
> +/* If set, apply CHECKSUM_COMPLETE to skb and validate the checksum */
> +#define BPF_F_TEST_SKB_CHECKSUM_COMPLETE BIT(2)
>
> /* type for BPF_ENABLE_STATS */
> enum bpf_stats_type {
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-05-24 16:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-24 11:06 [PATCH bpf-next v2 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
2024-05-24 11:06 ` [PATCH bpf-next v2 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option Vadim Fedorenko
2024-05-24 16:18 ` Daniel Borkmann
2024-05-24 16:23 ` Vadim Fedorenko
2024-05-24 16:15 ` [PATCH bpf-next v2 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Daniel Borkmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox