BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next v3 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs
@ 2024-05-27 18:59 Vadim Fedorenko
  2024-05-27 18:59 ` [PATCH bpf-next v3 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option Vadim Fedorenko
  2024-06-05  9:42 ` [PATCH bpf-next v3 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
  0 siblings, 2 replies; 5+ messages in thread
From: Vadim Fedorenko @ 2024-05-27 18:59 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             | 18 +++++++++++++++++-
 tools/include/uapi/linux/bpf.h |  2 ++
 3 files changed, 21 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..4c21562ad526 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 = -EBADMSG;
+			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	(1U << 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 v3 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option
  2024-05-27 18:59 [PATCH bpf-next v3 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
@ 2024-05-27 18:59 ` Vadim Fedorenko
  2024-06-05  9:42 ` [PATCH bpf-next v3 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
  1 sibling, 0 replies; 5+ messages in thread
From: Vadim Fedorenko @ 2024-05-27 18:59 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 v3 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs
  2024-05-27 18:59 [PATCH bpf-next v3 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
  2024-05-27 18:59 ` [PATCH bpf-next v3 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option Vadim Fedorenko
@ 2024-06-05  9:42 ` Vadim Fedorenko
  2024-06-05 14:43   ` Daniel Borkmann
  1 sibling, 1 reply; 5+ messages in thread
From: Vadim Fedorenko @ 2024-06-05  9:42 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: bpf, netdev, Jakub Kicinski, Mykola Lysenko, Andrii Nakryiko,
	Martin KaFai Lau, Alexei Starovoitov

On 27/05/2024 19:59, 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             | 18 +++++++++++++++++-
>   tools/include/uapi/linux/bpf.h |  2 ++
>   3 files changed, 21 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..4c21562ad526 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 = -EBADMSG;
> +			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	(1U << 2)
>   
>   /* type for BPF_ENABLE_STATS */
>   enum bpf_stats_type {

Hi Daniel!

Have you had a chance to look at v3 of this patch?
I think I addressed all your comments form v2.

Thanks,
Vadim


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next v3 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs
  2024-06-05  9:42 ` [PATCH bpf-next v3 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
@ 2024-06-05 14:43   ` Daniel Borkmann
  2024-06-06 14:42     ` Vadim Fedorenko
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Borkmann @ 2024-06-05 14:43 UTC (permalink / raw)
  To: Vadim Fedorenko
  Cc: bpf, netdev, Jakub Kicinski, Mykola Lysenko, Andrii Nakryiko,
	Martin KaFai Lau, Alexei Starovoitov

On 6/5/24 11:42 AM, Vadim Fedorenko wrote:
> On 27/05/2024 19:59, 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             | 18 +++++++++++++++++-
>>   tools/include/uapi/linux/bpf.h |  2 ++
>>   3 files changed, 21 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..4c21562ad526 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 = -EBADMSG;
>> +            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    (1U << 2)
>>   /* type for BPF_ENABLE_STATS */
>>   enum bpf_stats_type {
> 
> Hi Daniel!
> 
> Have you had a chance to look at v3 of this patch?
> I think I addressed all your comments form v2.

Looks good, but I think there is something off given the test on arm64 and s390x
fails in skb_pkt_end with EBADMSG. I wonder if we're missing a :

   skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);

right after the eth_type_trans()?

Thanks,
Daniel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH bpf-next v3 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs
  2024-06-05 14:43   ` Daniel Borkmann
@ 2024-06-06 14:42     ` Vadim Fedorenko
  0 siblings, 0 replies; 5+ messages in thread
From: Vadim Fedorenko @ 2024-06-06 14:42 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: bpf, netdev, Jakub Kicinski, Mykola Lysenko, Andrii Nakryiko,
	Martin KaFai Lau, Alexei Starovoitov

On 05/06/2024 15:43, Daniel Borkmann wrote:
> On 6/5/24 11:42 AM, Vadim Fedorenko wrote:
>> On 27/05/2024 19:59, 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             | 18 +++++++++++++++++-
>>>   tools/include/uapi/linux/bpf.h |  2 ++
>>>   3 files changed, 21 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..4c21562ad526 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 = -EBADMSG;
>>> +            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    (1U << 2)
>>>   /* type for BPF_ENABLE_STATS */
>>>   enum bpf_stats_type {
>>
>> Hi Daniel!
>>
>> Have you had a chance to look at v3 of this patch?
>> I think I addressed all your comments form v2.
> 
> Looks good, but I think there is something off given the test on arm64 
> and s390x
> fails in skb_pkt_end with EBADMSG. I wonder if we're missing a :
> 
>    skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
> 
> right after the eth_type_trans()?

Oh, thanks for bringing it up. Looks like it's a bit more complex.
Apparently, CHECKSUM_COMPLETE covers everything after the ethernet
header. That's why the code has to calculate checksum after network and
transport offsets are set. And for L2 case we have to skip ethernet
header.

Another issue is that correct check should use folded checksums instead
of raw values. I believe that was flagged by tests for other archs.

I'll do v4 soon and will be sure to have tests passing on all archs.

Thanks,
Vadim



> Thanks,
> Daniel


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-06-06 14:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-27 18:59 [PATCH bpf-next v3 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
2024-05-27 18:59 ` [PATCH bpf-next v3 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option Vadim Fedorenko
2024-06-05  9:42 ` [PATCH bpf-next v3 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
2024-06-05 14:43   ` Daniel Borkmann
2024-06-06 14:42     ` Vadim Fedorenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox