* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.