* [PATCH bpf-next v4 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs
@ 2024-06-06 14:58 Vadim Fedorenko
2024-06-06 14:58 ` [PATCH bpf-next v4 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option Vadim Fedorenko
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Vadim Fedorenko @ 2024-06-06 14:58 UTC (permalink / raw)
To: Vadim Fedorenko, Daniel Borkmann, Andrii Nakryiko,
Alexei Starovoitov, Mykola Lysenko, Jakub Kicinski
Cc: Vadim Fedorenko, Martin KaFai Lau, 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>
---
v3 -> v4:
- use network header offset as starting point for checksum
- use folded checksum values to compare results
v2 -> v3:
- remove BIT() macro from uapi bpf.h
- change error code to EBADMSG
v1 -> v2:
- clean unused variable
---
include/uapi/linux/bpf.h | 2 ++
net/bpf/test_run.c | 28 +++++++++++++++++++++++++++-
tools/include/uapi/linux/bpf.h | 2 ++
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 25ea393cf084..35bcf52dbc65 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..c87df2c4cd57 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,7 @@ 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 (ctx && ctx->ifindex > 1) {
dev = dev_get_by_index(net, ctx->ifindex);
if (!dev) {
@@ -1060,9 +1062,19 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
__skb_push(skb, hh_len);
if (is_direct_pkt_access)
bpf_compute_data_pointers(skb);
+
ret = convert___skb_to_skb(skb, ctx);
if (ret)
goto out;
+
+ if (kattr->test.flags & BPF_F_TEST_SKB_CHECKSUM_COMPLETE) {
+ const int off = skb_network_offset(skb);
+ int len = skb->len - off;
+
+ skb->csum = skb_checksum(skb, off, len, 0);
+ skb->ip_summed = CHECKSUM_COMPLETE;
+ }
+
ret = bpf_test_run(prog, skb, repeat, &retval, &duration, false);
if (ret)
goto out;
@@ -1077,6 +1089,20 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
}
memset(__skb_push(skb, hh_len), 0, hh_len);
}
+
+ if (kattr->test.flags & BPF_F_TEST_SKB_CHECKSUM_COMPLETE) {
+ const int off = skb_network_offset(skb);
+ int len = skb->len - off;
+ __wsum csum;
+
+ csum = skb_checksum(skb, off, len, 0);
+
+ if (csum_fold(skb->csum) != csum_fold(csum)) {
+ ret = -EBADMSG;
+ goto out;
+ }
+ }
+
convert_skb_to___skb(skb, ctx);
size = skb->len;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 25ea393cf084..35bcf52dbc65 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 (1U << 2)
/* type for BPF_ENABLE_STATS */
enum bpf_stats_type {
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next v4 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option
2024-06-06 14:58 [PATCH bpf-next v4 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
@ 2024-06-06 14:58 ` Vadim Fedorenko
2024-06-12 14:49 ` [PATCH bpf-next v4 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Jakub Kicinski
2024-06-13 12:40 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 6+ messages in thread
From: Vadim Fedorenko @ 2024-06-06 14:58 UTC (permalink / raw)
To: Vadim Fedorenko, Daniel Borkmann, Andrii Nakryiko,
Alexei Starovoitov, Mykola Lysenko, Jakub Kicinski
Cc: Vadim Fedorenko, Martin KaFai Lau, 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] 6+ messages in thread
* Re: [PATCH bpf-next v4 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs
2024-06-06 14:58 [PATCH bpf-next v4 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
2024-06-06 14:58 ` [PATCH bpf-next v4 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option Vadim Fedorenko
@ 2024-06-12 14:49 ` Jakub Kicinski
2024-06-13 11:18 ` Vadim Fedorenko
2024-06-13 12:40 ` patchwork-bot+netdevbpf
2 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2024-06-12 14:49 UTC (permalink / raw)
To: Vadim Fedorenko
Cc: Vadim Fedorenko, Daniel Borkmann, Andrii Nakryiko,
Alexei Starovoitov, Mykola Lysenko, Martin KaFai Lau, bpf, netdev
On Thu, 6 Jun 2024 07:58:50 -0700 Vadim Fedorenko wrote:
> @@ -1060,9 +1062,19 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> __skb_push(skb, hh_len);
> if (is_direct_pkt_access)
> bpf_compute_data_pointers(skb);
> +
> ret = convert___skb_to_skb(skb, ctx);
> if (ret)
> goto out;
> +
> + if (kattr->test.flags & BPF_F_TEST_SKB_CHECKSUM_COMPLETE) {
> + const int off = skb_network_offset(skb);
> + int len = skb->len - off;
> +
> + skb->csum = skb_checksum(skb, off, len, 0);
> + skb->ip_summed = CHECKSUM_COMPLETE;
> + }
Looks good, overall, although I'd be tempted to place this before
the L2 is pushed, a few lines up, so that we don't need to worry
about network offset. Then again, with you approach there is a nice
symmetry between the pre- and post- if blocks so either way is fine:
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v4 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs
2024-06-12 14:49 ` [PATCH bpf-next v4 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Jakub Kicinski
@ 2024-06-13 11:18 ` Vadim Fedorenko
2024-06-13 12:32 ` Daniel Borkmann
0 siblings, 1 reply; 6+ messages in thread
From: Vadim Fedorenko @ 2024-06-13 11:18 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Andrii Nakryiko, Alexei Starovoitov, Mykola Lysenko,
Jakub Kicinski, Martin KaFai Lau, bpf, netdev
On 12/06/2024 15:49, Jakub Kicinski wrote:
> On Thu, 6 Jun 2024 07:58:50 -0700 Vadim Fedorenko wrote:
>> @@ -1060,9 +1062,19 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>> __skb_push(skb, hh_len);
>> if (is_direct_pkt_access)
>> bpf_compute_data_pointers(skb);
>> +
>> ret = convert___skb_to_skb(skb, ctx);
>> if (ret)
>> goto out;
>> +
>> + if (kattr->test.flags & BPF_F_TEST_SKB_CHECKSUM_COMPLETE) {
>> + const int off = skb_network_offset(skb);
>> + int len = skb->len - off;
>> +
>> + skb->csum = skb_checksum(skb, off, len, 0);
>> + skb->ip_summed = CHECKSUM_COMPLETE;
>> + }
>
> Looks good, overall, although I'd be tempted to place this before
> the L2 is pushed, a few lines up, so that we don't need to worry
> about network offset. Then again, with you approach there is a nice
> symmetry between the pre- and post- if blocks so either way is fine:
>
> Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Hi Daniel!
Could you please take a look and merge the series?
Thanks,
Vadim
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v4 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs
2024-06-13 11:18 ` Vadim Fedorenko
@ 2024-06-13 12:32 ` Daniel Borkmann
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Borkmann @ 2024-06-13 12:32 UTC (permalink / raw)
To: Vadim Fedorenko
Cc: Andrii Nakryiko, Alexei Starovoitov, Mykola Lysenko,
Jakub Kicinski, Martin KaFai Lau, bpf, netdev
On 6/13/24 1:18 PM, Vadim Fedorenko wrote:
> On 12/06/2024 15:49, Jakub Kicinski wrote:
>> On Thu, 6 Jun 2024 07:58:50 -0700 Vadim Fedorenko wrote:
>>> @@ -1060,9 +1062,19 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>>> __skb_push(skb, hh_len);
>>> if (is_direct_pkt_access)
>>> bpf_compute_data_pointers(skb);
>>> +
>>> ret = convert___skb_to_skb(skb, ctx);
>>> if (ret)
>>> goto out;
>>> +
>>> + if (kattr->test.flags & BPF_F_TEST_SKB_CHECKSUM_COMPLETE) {
>>> + const int off = skb_network_offset(skb);
>>> + int len = skb->len - off;
>>> +
>>> + skb->csum = skb_checksum(skb, off, len, 0);
>>> + skb->ip_summed = CHECKSUM_COMPLETE;
>>> + }
>>
>> Looks good, overall, although I'd be tempted to place this before
>> the L2 is pushed, a few lines up, so that we don't need to worry
>> about network offset. Then again, with you approach there is a nice
>> symmetry between the pre- and post- if blocks so either way is fine:
>>
>> Reviewed-by: Jakub Kicinski <kuba@kernel.org>
>
> Could you please take a look and merge the series?
Looks good, done now, thanks Vadim!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v4 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs
2024-06-06 14:58 [PATCH bpf-next v4 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
2024-06-06 14:58 ` [PATCH bpf-next v4 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option Vadim Fedorenko
2024-06-12 14:49 ` [PATCH bpf-next v4 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Jakub Kicinski
@ 2024-06-13 12:40 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-06-13 12:40 UTC (permalink / raw)
To: Vadim Fedorenko
Cc: vadim.fedorenko, daniel, andrii, ast, mykolal, kuba, martin.lau,
bpf, netdev
Hello:
This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:
On Thu, 6 Jun 2024 07:58:50 -0700 you wrote:
> Add special flag to validate that TC BPF program properly updates
> checksum information in skb.
>
> Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
> ---
> v3 -> v4:
> - use network header offset as starting point for checksum
> - use folded checksum values to compare results
> v2 -> v3:
> - remove BIT() macro from uapi bpf.h
> - change error code to EBADMSG
> v1 -> v2:
> - clean unused variable
>
> [...]
Here is the summary with links:
- [bpf-next,v4,1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs
https://git.kernel.org/bpf/bpf-next/c/a3cfe84cca28
- [bpf-next,v4,2/2] selftests: bpf: validate CHECKSUM_COMPLETE option
https://git.kernel.org/bpf/bpf-next/c/041c1dc988fd
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-06-13 12:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-06 14:58 [PATCH bpf-next v4 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
2024-06-06 14:58 ` [PATCH bpf-next v4 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option Vadim Fedorenko
2024-06-12 14:49 ` [PATCH bpf-next v4 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Jakub Kicinski
2024-06-13 11:18 ` Vadim Fedorenko
2024-06-13 12:32 ` Daniel Borkmann
2024-06-13 12:40 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).