* [PATCH bpf-next 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option
2024-05-22 14:57 [PATCH bpf-next 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
@ 2024-05-22 14:57 ` Vadim Fedorenko
2024-05-22 18:32 ` [PATCH bpf-next 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Simon Horman
2024-05-23 14:22 ` kernel test robot
2 siblings, 0 replies; 5+ messages in thread
From: Vadim Fedorenko @ 2024-05-22 14:57 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 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs
2024-05-22 14:57 [PATCH bpf-next 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
2024-05-22 14:57 ` [PATCH bpf-next 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option Vadim Fedorenko
@ 2024-05-22 18:32 ` Simon Horman
2024-05-22 21:21 ` Vadim Fedorenko
2024-05-23 14:22 ` kernel test robot
2 siblings, 1 reply; 5+ messages in thread
From: Simon Horman @ 2024-05-22 18:32 UTC (permalink / raw)
To: Vadim Fedorenko
Cc: Vadim Fedorenko, Martin KaFai Lau, Andrii Nakryiko,
Alexei Starovoitov, Mykola Lysenko, Jakub Kicinski, bpf, netdev
On Wed, May 22, 2024 at 07:57:10AM -0700, 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>
...
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index f6aad4ed2ab2..841552785c65 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -974,10 +974,13 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> int hh_len = ETH_HLEN;
> struct sk_buff *skb;
> struct sock *sk;
> + __wsum csum;
> + __sum16 sum;
Hi Vadim,
sum seems to be is unused in this function.
And, fwiiw, the scope of csum looks like it could be reduced.
> 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 +1028,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 +1088,14 @@ 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) {
> + 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)))
...
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH bpf-next 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs
2024-05-22 18:32 ` [PATCH bpf-next 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Simon Horman
@ 2024-05-22 21:21 ` Vadim Fedorenko
0 siblings, 0 replies; 5+ messages in thread
From: Vadim Fedorenko @ 2024-05-22 21:21 UTC (permalink / raw)
To: Simon Horman
Cc: Martin KaFai Lau, Andrii Nakryiko, Alexei Starovoitov,
Mykola Lysenko, Jakub Kicinski, bpf, netdev
On 22/05/2024 19:32, Simon Horman wrote:
> On Wed, May 22, 2024 at 07:57:10AM -0700, 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>
>
> ...
>
>> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
>> index f6aad4ed2ab2..841552785c65 100644
>> --- a/net/bpf/test_run.c
>> +++ b/net/bpf/test_run.c
>> @@ -974,10 +974,13 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>> int hh_len = ETH_HLEN;
>> struct sk_buff *skb;
>> struct sock *sk;
>> + __wsum csum;
>> + __sum16 sum;
>
> Hi Vadim,
>
> sum seems to be is unused in this function.
> And, fwiiw, the scope of csum looks like it could be reduced.
Ah, leftover from previous iteration, thanks for catching it!
Ok, I'll move csum to the "if" block in v2.
>> 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 +1028,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 +1088,14 @@ 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) {
>> + 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)))
>
> ...
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs
2024-05-22 14:57 [PATCH bpf-next 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Vadim Fedorenko
2024-05-22 14:57 ` [PATCH bpf-next 2/2] selftests: bpf: validate CHECKSUM_COMPLETE option Vadim Fedorenko
2024-05-22 18:32 ` [PATCH bpf-next 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs Simon Horman
@ 2024-05-23 14:22 ` kernel test robot
2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2024-05-23 14:22 UTC (permalink / raw)
To: Vadim Fedorenko, Vadim Fedorenko, Martin KaFai Lau,
Andrii Nakryiko, Alexei Starovoitov, Mykola Lysenko,
Jakub Kicinski
Cc: oe-kbuild-all, bpf, netdev
Hi Vadim,
kernel test robot noticed the following build warnings:
[auto build test WARNING on bpf-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Vadim-Fedorenko/selftests-bpf-validate-CHECKSUM_COMPLETE-option/20240522-225856
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20240522145712.3523593-1-vadfed%40meta.com
patch subject: [PATCH bpf-next 1/2] bpf: add CHECKSUM_COMPLETE to bpf test progs
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20240523/202405232220.e9PuO2yW-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240523/202405232220.e9PuO2yW-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405232220.e9PuO2yW-lkp@intel.com/
All warnings (new ones prefixed by >>):
net/bpf/test_run.c: In function 'bpf_prog_test_run_skb':
>> net/bpf/test_run.c:978:17: warning: unused variable 'sum' [-Wunused-variable]
978 | __sum16 sum;
| ^~~
vim +/sum +978 net/bpf/test_run.c
963
964 int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
965 union bpf_attr __user *uattr)
966 {
967 bool is_l2 = false, is_direct_pkt_access = false;
968 struct net *net = current->nsproxy->net_ns;
969 struct net_device *dev = net->loopback_dev;
970 u32 size = kattr->test.data_size_in;
971 u32 repeat = kattr->test.repeat;
972 struct __sk_buff *ctx = NULL;
973 u32 retval, duration;
974 int hh_len = ETH_HLEN;
975 struct sk_buff *skb;
976 struct sock *sk;
977 __wsum csum;
> 978 __sum16 sum;
979 void *data;
980 int ret;
981
982 if ((kattr->test.flags & ~BPF_F_TEST_SKB_CHECKSUM_COMPLETE) ||
983 kattr->test.cpu || kattr->test.batch_size)
984 return -EINVAL;
985
986 data = bpf_test_init(kattr, kattr->test.data_size_in,
987 size, NET_SKB_PAD + NET_IP_ALIGN,
988 SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
989 if (IS_ERR(data))
990 return PTR_ERR(data);
991
992 ctx = bpf_ctx_init(kattr, sizeof(struct __sk_buff));
993 if (IS_ERR(ctx)) {
994 kfree(data);
995 return PTR_ERR(ctx);
996 }
997
998 switch (prog->type) {
999 case BPF_PROG_TYPE_SCHED_CLS:
1000 case BPF_PROG_TYPE_SCHED_ACT:
1001 is_l2 = true;
1002 fallthrough;
1003 case BPF_PROG_TYPE_LWT_IN:
1004 case BPF_PROG_TYPE_LWT_OUT:
1005 case BPF_PROG_TYPE_LWT_XMIT:
1006 is_direct_pkt_access = true;
1007 break;
1008 default:
1009 break;
1010 }
1011
1012 sk = sk_alloc(net, AF_UNSPEC, GFP_USER, &bpf_dummy_proto, 1);
1013 if (!sk) {
1014 kfree(data);
1015 kfree(ctx);
1016 return -ENOMEM;
1017 }
1018 sock_init_data(NULL, sk);
1019
1020 skb = slab_build_skb(data);
1021 if (!skb) {
1022 kfree(data);
1023 kfree(ctx);
1024 sk_free(sk);
1025 return -ENOMEM;
1026 }
1027 skb->sk = sk;
1028
1029 skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
1030 __skb_put(skb, size);
1031
1032 if (kattr->test.flags & BPF_F_TEST_SKB_CHECKSUM_COMPLETE) {
1033 skb->csum = skb_checksum(skb, 0, skb->len, 0);
1034 skb->ip_summed = CHECKSUM_COMPLETE;
1035 }
1036
1037 if (ctx && ctx->ifindex > 1) {
1038 dev = dev_get_by_index(net, ctx->ifindex);
1039 if (!dev) {
1040 ret = -ENODEV;
1041 goto out;
1042 }
1043 }
1044 skb->protocol = eth_type_trans(skb, dev);
1045 skb_reset_network_header(skb);
1046
1047 switch (skb->protocol) {
1048 case htons(ETH_P_IP):
1049 sk->sk_family = AF_INET;
1050 if (sizeof(struct iphdr) <= skb_headlen(skb)) {
1051 sk->sk_rcv_saddr = ip_hdr(skb)->saddr;
1052 sk->sk_daddr = ip_hdr(skb)->daddr;
1053 }
1054 break;
1055 #if IS_ENABLED(CONFIG_IPV6)
1056 case htons(ETH_P_IPV6):
1057 sk->sk_family = AF_INET6;
1058 if (sizeof(struct ipv6hdr) <= skb_headlen(skb)) {
1059 sk->sk_v6_rcv_saddr = ipv6_hdr(skb)->saddr;
1060 sk->sk_v6_daddr = ipv6_hdr(skb)->daddr;
1061 }
1062 break;
1063 #endif
1064 default:
1065 break;
1066 }
1067
1068 if (is_l2)
1069 __skb_push(skb, hh_len);
1070 if (is_direct_pkt_access)
1071 bpf_compute_data_pointers(skb);
1072 ret = convert___skb_to_skb(skb, ctx);
1073 if (ret)
1074 goto out;
1075 ret = bpf_test_run(prog, skb, repeat, &retval, &duration, false);
1076 if (ret)
1077 goto out;
1078 if (!is_l2) {
1079 if (skb_headroom(skb) < hh_len) {
1080 int nhead = HH_DATA_ALIGN(hh_len - skb_headroom(skb));
1081
1082 if (pskb_expand_head(skb, nhead, 0, GFP_USER)) {
1083 ret = -ENOMEM;
1084 goto out;
1085 }
1086 }
1087 memset(__skb_push(skb, hh_len), 0, hh_len);
1088 }
1089 convert_skb_to___skb(skb, ctx);
1090
1091 if (kattr->test.flags & BPF_F_TEST_SKB_CHECKSUM_COMPLETE) {
1092 csum = skb_checksum(skb, 0, skb->len, 0);
1093 if (skb->csum != csum) {
1094 ret = -EINVAL;
1095 goto out;
1096 }
1097 }
1098
1099 size = skb->len;
1100 /* bpf program can never convert linear skb to non-linear */
1101 if (WARN_ON_ONCE(skb_is_nonlinear(skb)))
1102 size = skb_headlen(skb);
1103 ret = bpf_test_finish(kattr, uattr, skb->data, NULL, size, retval,
1104 duration);
1105 if (!ret)
1106 ret = bpf_ctx_finish(kattr, uattr, ctx,
1107 sizeof(struct __sk_buff));
1108 out:
1109 if (dev && dev != net->loopback_dev)
1110 dev_put(dev);
1111 kfree_skb(skb);
1112 sk_free(sk);
1113 kfree(ctx);
1114 return ret;
1115 }
1116
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 5+ messages in thread