* [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB
@ 2021-12-22 2:27 Tyler Wear
2021-12-22 3:43 ` Yonghong Song
0 siblings, 1 reply; 10+ messages in thread
From: Tyler Wear @ 2021-12-22 2:27 UTC (permalink / raw)
To: netdev, bpf; +Cc: kafai, maze, yhs, Tyler Wear
Need to modify the ds field to support upcoming
Wifi QoS Alliance spec. Instead of adding generic
function for just modifying the ds field, add
skb_store_bytes for BPF_PROG_TYPE_CGROUP_SKB. This
allows other fields in the network and transport header
to be modified in the future.
Signed-off-by: Tyler Wear <quic_twear@quicinc.com>
---
net/core/filter.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/core/filter.c b/net/core/filter.c
index 6102f093d59a..0c25aa2212a2 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7289,6 +7289,8 @@ static const struct bpf_func_proto *
cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
switch (func_id) {
+ case BPF_FUNC_skb_store_bytes:
+ return &bpf_skb_store_bytes_proto;
case BPF_FUNC_get_local_storage:
return &bpf_get_local_storage_proto;
case BPF_FUNC_sk_fullsock:
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB
2021-12-22 2:27 [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB Tyler Wear
@ 2021-12-22 3:43 ` Yonghong Song
2021-12-22 22:49 ` Tyler Wear
0 siblings, 1 reply; 10+ messages in thread
From: Yonghong Song @ 2021-12-22 3:43 UTC (permalink / raw)
To: Tyler Wear, netdev, bpf; +Cc: kafai, maze
On 12/21/21 6:27 PM, Tyler Wear wrote:
> Need to modify the ds field to support upcoming
> Wifi QoS Alliance spec. Instead of adding generic
> function for just modifying the ds field, add
> skb_store_bytes for BPF_PROG_TYPE_CGROUP_SKB. This
> allows other fields in the network and transport header
> to be modified in the future.
Could change tag from "[PATCH]" to "[PATCH bpf-next]"?
Please also indicate the version of the patch, so in
this case, it should be "[PATCH bpf-next v2]".
I think you can add more contents in the commit
message about why existing bpf_setsockopt() won't work
and why CGROUP_UDP[4|6]_SENDMSG is not preferred.
These have been discussed in v1 of this patch and they
are valuable for people to understand full context
and reasoning.
>
> Signed-off-by: Tyler Wear <quic_twear@quicinc.com>
> ---
> net/core/filter.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 6102f093d59a..0c25aa2212a2 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -7289,6 +7289,8 @@ static const struct bpf_func_proto *
> cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> {
> switch (func_id) {
> + case BPF_FUNC_skb_store_bytes:
> + return &bpf_skb_store_bytes_proto;
Typically different 'case's are added in chronological order to people
can guess what is added earlier and what is added later. Maybe add
the new helper after BPF_FUNC_perf_event_output?
> case BPF_FUNC_get_local_storage:
> return &bpf_get_local_storage_proto;
> case BPF_FUNC_sk_fullsock:
Please add a test case to exercise the new usage of
bpf_skb_store_bytes() helper. You may piggy back on
some existing cg_skb progs if it is easier to do.
^ permalink raw reply [flat|nested] 10+ messages in thread* RE: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB
2021-12-22 3:43 ` Yonghong Song
@ 2021-12-22 22:49 ` Tyler Wear
2021-12-22 23:50 ` Martin KaFai Lau
0 siblings, 1 reply; 10+ messages in thread
From: Tyler Wear @ 2021-12-22 22:49 UTC (permalink / raw)
To: Yonghong Song, Tyler Wear (QUIC), netdev, bpf; +Cc: kafai, maze
> On 12/21/21 6:27 PM, Tyler Wear wrote:
> > Need to modify the ds field to support upcoming Wifi QoS Alliance
> > spec. Instead of adding generic function for just modifying the ds
> > field, add skb_store_bytes for BPF_PROG_TYPE_CGROUP_SKB. This allows
> > other fields in the network and transport header to be modified in the
> > future.
>
> Could change tag from "[PATCH]" to "[PATCH bpf-next]"?
> Please also indicate the version of the patch, so in this case, it should be "[PATCH bpf-next v2]".
>
> I think you can add more contents in the commit message about why existing bpf_setsockopt() won't work and why
> CGROUP_UDP[4|6]_SENDMSG is not preferred.
> These have been discussed in v1 of this patch and they are valuable for people to understand full context and reasoning.
>
> >
> > Signed-off-by: Tyler Wear <quic_twear@quicinc.com>
> > ---
> > net/core/filter.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/net/core/filter.c b/net/core/filter.c index
> > 6102f093d59a..0c25aa2212a2 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -7289,6 +7289,8 @@ static const struct bpf_func_proto *
> > cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > {
> > switch (func_id) {
> > + case BPF_FUNC_skb_store_bytes:
> > + return &bpf_skb_store_bytes_proto;
>
> Typically different 'case's are added in chronological order to people can guess what is added earlier and what is added later. Maybe
> add the new helper after BPF_FUNC_perf_event_output?
>
> > case BPF_FUNC_get_local_storage:
> > return &bpf_get_local_storage_proto;
> > case BPF_FUNC_sk_fullsock:
>
> Please add a test case to exercise the new usage of
> bpf_skb_store_bytes() helper. You may piggy back on some existing cg_skb progs if it is easier to do.
Would it be sufficient to change the dscp value in tools/testing/selftests/bpf/progs/test_sock_fields.c via bpf_skb_store_bytes()
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB
2021-12-22 22:49 ` Tyler Wear
@ 2021-12-22 23:50 ` Martin KaFai Lau
2021-12-29 18:29 ` Tyler Wear
0 siblings, 1 reply; 10+ messages in thread
From: Martin KaFai Lau @ 2021-12-22 23:50 UTC (permalink / raw)
To: Tyler Wear; +Cc: Yonghong Song, Tyler Wear (QUIC), netdev, bpf, maze
On Wed, Dec 22, 2021 at 10:49:45PM +0000, Tyler Wear wrote:
> > On 12/21/21 6:27 PM, Tyler Wear wrote:
> > > Need to modify the ds field to support upcoming Wifi QoS Alliance
> > > spec. Instead of adding generic function for just modifying the ds
> > > field, add skb_store_bytes for BPF_PROG_TYPE_CGROUP_SKB. This allows
> > > other fields in the network and transport header to be modified in the
> > > future.
> >
> > Could change tag from "[PATCH]" to "[PATCH bpf-next]"?
> > Please also indicate the version of the patch, so in this case, it should be "[PATCH bpf-next v2]".
> >
> > I think you can add more contents in the commit message about why existing bpf_setsockopt() won't work and why
> > CGROUP_UDP[4|6]_SENDMSG is not preferred.
> > These have been discussed in v1 of this patch and they are valuable for people to understand full context and reasoning.
> >
> > >
> > > Signed-off-by: Tyler Wear <quic_twear@quicinc.com>
> > > ---
> > > net/core/filter.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/net/core/filter.c b/net/core/filter.c index
> > > 6102f093d59a..0c25aa2212a2 100644
> > > --- a/net/core/filter.c
> > > +++ b/net/core/filter.c
> > > @@ -7289,6 +7289,8 @@ static const struct bpf_func_proto *
> > > cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > > {
> > > switch (func_id) {
> > > + case BPF_FUNC_skb_store_bytes:
> > > + return &bpf_skb_store_bytes_proto;
> >
> > Typically different 'case's are added in chronological order to people can guess what is added earlier and what is added later. Maybe
> > add the new helper after BPF_FUNC_perf_event_output?
> >
> > > case BPF_FUNC_get_local_storage:
> > > return &bpf_get_local_storage_proto;
> > > case BPF_FUNC_sk_fullsock:
> >
> > Please add a test case to exercise the new usage of
> > bpf_skb_store_bytes() helper. You may piggy back on some existing cg_skb progs if it is easier to do.
>
> Would it be sufficient to change the dscp value in tools/testing/selftests/bpf/progs/test_sock_fields.c via bpf_skb_store_bytes()
test_sock_fields focus on sk instead of skb, so it will not be a good fit.
load_bytes_relative.c may be a better fit.
The minimal is to write the dscp value by bpf_skb_store_bytes()
and be able to read it back at the receiver side (e.g.
by making a TCP connection like load_bytes_relative).
^ permalink raw reply [flat|nested] 10+ messages in thread* RE: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB
2021-12-22 23:50 ` Martin KaFai Lau
@ 2021-12-29 18:29 ` Tyler Wear
2021-12-29 21:05 ` Martin KaFai Lau
0 siblings, 1 reply; 10+ messages in thread
From: Tyler Wear @ 2021-12-29 18:29 UTC (permalink / raw)
To: Martin KaFai Lau; +Cc: Yonghong Song, Tyler Wear (QUIC), netdev, bpf, maze
> -----Original Message-----
> From: Martin KaFai Lau <kafai@fb.com>
> Sent: Wednesday, December 22, 2021 3:51 PM
> To: Tyler Wear <twear@quicinc.com>
> Cc: Yonghong Song <yhs@fb.com>; Tyler Wear (QUIC) <quic_twear@quicinc.com>; netdev@vger.kernel.org; bpf@vger.kernel.org;
> maze@google.com
> Subject: Re: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB
>
> WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
>
> On Wed, Dec 22, 2021 at 10:49:45PM +0000, Tyler Wear wrote:
> > > On 12/21/21 6:27 PM, Tyler Wear wrote:
> > > > Need to modify the ds field to support upcoming Wifi QoS Alliance
> > > > spec. Instead of adding generic function for just modifying the ds
> > > > field, add skb_store_bytes for BPF_PROG_TYPE_CGROUP_SKB. This
> > > > allows other fields in the network and transport header to be
> > > > modified in the future.
> > >
> > > Could change tag from "[PATCH]" to "[PATCH bpf-next]"?
> > > Please also indicate the version of the patch, so in this case, it should be "[PATCH bpf-next v2]".
> > >
> > > I think you can add more contents in the commit message about why
> > > existing bpf_setsockopt() won't work and why CGROUP_UDP[4|6]_SENDMSG is not preferred.
> > > These have been discussed in v1 of this patch and they are valuable for people to understand full context and reasoning.
> > >
> > > >
> > > > Signed-off-by: Tyler Wear <quic_twear@quicinc.com>
> > > > ---
> > > > net/core/filter.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/net/core/filter.c b/net/core/filter.c index
> > > > 6102f093d59a..0c25aa2212a2 100644
> > > > --- a/net/core/filter.c
> > > > +++ b/net/core/filter.c
> > > > @@ -7289,6 +7289,8 @@ static const struct bpf_func_proto *
> > > > cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > > > {
> > > > switch (func_id) {
> > > > + case BPF_FUNC_skb_store_bytes:
> > > > + return &bpf_skb_store_bytes_proto;
> > >
> > > Typically different 'case's are added in chronological order to
> > > people can guess what is added earlier and what is added later. Maybe add the new helper after BPF_FUNC_perf_event_output?
> > >
> > > > case BPF_FUNC_get_local_storage:
> > > > return &bpf_get_local_storage_proto;
> > > > case BPF_FUNC_sk_fullsock:
> > >
> > > Please add a test case to exercise the new usage of
> > > bpf_skb_store_bytes() helper. You may piggy back on some existing cg_skb progs if it is easier to do.
> >
> > Would it be sufficient to change the dscp value in
> > tools/testing/selftests/bpf/progs/test_sock_fields.c via
> > bpf_skb_store_bytes()
> test_sock_fields focus on sk instead of skb, so it will not be a good fit.
>
> load_bytes_relative.c may be a better fit.
> The minimal is to write the dscp value by bpf_skb_store_bytes() and be able to read it back at the receiver side (e.g.
> by making a TCP connection like load_bytes_relative).
Unable to run any bpf tests do to errors below. These occur with and without the new patch. Is this a known issue?
Is the new test case required since bpf_skb_store_bytes() is already a tested function for other prog types?
libbpf: failed to find BTF for extern 'bpf_testmod_invalid_mod_kfunc' [18] section: -2
Error: failed to open BPF object file: No such file or directory
libbpf: failed to find BTF info for global/extern symbol 'my_tid'
Error: failed to link '/local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/linked_funcs1.o': Unknown error -2 (-2)
libbpf: failed to find BTF for extern 'bpf_kfunc_call_test1' [27] section: -2
Error: failed to open BPF object file: No such file or directory
make: *** [Makefile:484: /local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/test_ksyms_module.skel.h] Error 255
make: *** Deleting file '/local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/test_ksyms_module.skel.h'
make: *** Waiting for unfinished jobs....
make: *** [Makefile:484: /local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/kfunc_call_test_subprog.skel.h] Error 255
make: *** Deleting file '/local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/kfunc_call_test_subprog.skel.h'
make: *** [Makefile:482: /local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/linked_funcs.skel.h] Error 254
libbpf: failed to find BTF info for global/extern symbol 'input_rodata_weak'
Error: failed to link '/local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/linked_vars1.o': Unknown error -2 (-2)
make: *** [Makefile:482: /local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/linked_vars.skel.h] Error 254
libbpf: failed to find BTF for extern 'tcp_cong_avoid_ai' [27] section: -2
Error: failed to open BPF object file: No such file or directory
make: *** [Makefile:486: /local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/bpf_cubic.skel.h] Error 255
make: *** Deleting file '/local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/bpf_cubic.skel.h'
libbpf: failed to find BTF for extern 'bpf_kfunc_call_test1' [28] section: -2
Error: failed to open BPF object file: No such file or directory
make: *** [Makefile:486: /local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/kfunc_call_test.lskel.h] Error 255
make: *** Deleting file '/local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/kfunc_call_test.lskel.h'
libbpf: failed to find BTF for extern 'tcp_reno_cong_avoid' [38] section: -2
Error: failed to open BPF object file: No such file or directory
libbpf: failed to find BTF for extern 'bpf_testmod_invalid_mod_kfunc' [18] section: -2
Error: failed to open BPF object file: No such file or directory
make: *** [Makefile:486: /local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/bpf_dctcp.skel.h] Error 255
make: *** Deleting file '/local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/bpf_dctcp.skel.h'
make: *** [Makefile:486: /local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/test_ksyms_module.lskel.h] Error 255
make: *** Deleting file '/local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/test_ksyms_module.lskel.h'
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB
2021-12-29 18:29 ` Tyler Wear
@ 2021-12-29 21:05 ` Martin KaFai Lau
2022-01-05 0:27 ` Tyler Wear (QUIC)
0 siblings, 1 reply; 10+ messages in thread
From: Martin KaFai Lau @ 2021-12-29 21:05 UTC (permalink / raw)
To: Tyler Wear; +Cc: Yonghong Song, Tyler Wear (QUIC), netdev, bpf, maze
On Wed, Dec 29, 2021 at 06:29:05PM +0000, Tyler Wear wrote:
> Unable to run any bpf tests do to errors below. These occur with and without the new patch. Is this a known issue?
> Is the new test case required since bpf_skb_store_bytes() is already a tested function for other prog types?
>
> libbpf: failed to find BTF for extern 'bpf_testmod_invalid_mod_kfunc' [18] section: -2
> Error: failed to open BPF object file: No such file or directory
> libbpf: failed to find BTF info for global/extern symbol 'my_tid'
> Error: failed to link '/local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/linked_funcs1.o': Unknown error -2 (-2)
> libbpf: failed to find BTF for extern 'bpf_kfunc_call_test1' [27] section: -2
tools/testing/selftests/bpf/README.rst has details on these.
Ensure the llvm and pahole are up to date.
Also take a look at the "Testing patches" and "LLVM" section
in Documentation/bpf/bpf_devel_QA.rst.
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB
2021-12-29 21:05 ` Martin KaFai Lau
@ 2022-01-05 0:27 ` Tyler Wear (QUIC)
2022-01-06 7:50 ` Yonghong Song
0 siblings, 1 reply; 10+ messages in thread
From: Tyler Wear (QUIC) @ 2022-01-05 0:27 UTC (permalink / raw)
To: Martin KaFai Lau; +Cc: Yonghong Song, Tyler Wear (QUIC), netdev, bpf, maze
> -----Original Message-----
> From: Martin KaFai Lau <kafai@fb.com>
> Sent: Wednesday, December 29, 2021 1:06 PM
> To: Tyler Wear <twear@quicinc.com>
> Cc: Yonghong Song <yhs@fb.com>; Tyler Wear (QUIC) <quic_twear@quicinc.com>; netdev@vger.kernel.org; bpf@vger.kernel.org;
> maze@google.com
> Subject: Re: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB
>
> WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
>
> On Wed, Dec 29, 2021 at 06:29:05PM +0000, Tyler Wear wrote:
> > Unable to run any bpf tests do to errors below. These occur with and without the new patch. Is this a known issue?
> > Is the new test case required since bpf_skb_store_bytes() is already a tested function for other prog types?
> >
> > libbpf: failed to find BTF for extern 'bpf_testmod_invalid_mod_kfunc'
> > [18] section: -2
> > Error: failed to open BPF object file: No such file or directory
> > libbpf: failed to find BTF info for global/extern symbol 'my_tid'
> > Error: failed to link
> > '/local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/linked_
> > funcs1.o': Unknown error -2 (-2)
> > libbpf: failed to find BTF for extern 'bpf_kfunc_call_test1' [27]
> > section: -2
> tools/testing/selftests/bpf/README.rst has details on these.
>
> Ensure the llvm and pahole are up to date.
> Also take a look at the "Testing patches" and "LLVM" section in Documentation/bpf/bpf_devel_QA.rst.
This will also require adding the l3/l4_ csum_replace() api's then. Adding the csum_replace() to a cgroup test case results in the below error during bpf program validation:
"BPF_LD_[ABS|IND] instructions not allowed for this program type"
Is there something else that needs to be added? Or would it be better to create the function just for ds_field?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB
2022-01-05 0:27 ` Tyler Wear (QUIC)
@ 2022-01-06 7:50 ` Yonghong Song
2022-01-06 17:18 ` Tyler Wear (QUIC)
0 siblings, 1 reply; 10+ messages in thread
From: Yonghong Song @ 2022-01-06 7:50 UTC (permalink / raw)
To: Tyler Wear (QUIC), Martin KaFai Lau; +Cc: netdev, bpf, maze
On 1/4/22 4:27 PM, Tyler Wear (QUIC) wrote:
>
>
>> -----Original Message-----
>> From: Martin KaFai Lau <kafai@fb.com>
>> Sent: Wednesday, December 29, 2021 1:06 PM
>> To: Tyler Wear <twear@quicinc.com>
>> Cc: Yonghong Song <yhs@fb.com>; Tyler Wear (QUIC) <quic_twear@quicinc.com>; netdev@vger.kernel.org; bpf@vger.kernel.org;
>> maze@google.com
>> Subject: Re: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB
>>
>> WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
>>
>> On Wed, Dec 29, 2021 at 06:29:05PM +0000, Tyler Wear wrote:
>>> Unable to run any bpf tests do to errors below. These occur with and without the new patch. Is this a known issue?
>>> Is the new test case required since bpf_skb_store_bytes() is already a tested function for other prog types?
>>>
>>> libbpf: failed to find BTF for extern 'bpf_testmod_invalid_mod_kfunc'
>>> [18] section: -2
>>> Error: failed to open BPF object file: No such file or directory
>>> libbpf: failed to find BTF info for global/extern symbol 'my_tid'
>>> Error: failed to link
>>> '/local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/linked_
>>> funcs1.o': Unknown error -2 (-2)
>>> libbpf: failed to find BTF for extern 'bpf_kfunc_call_test1' [27]
>>> section: -2
>> tools/testing/selftests/bpf/README.rst has details on these.
>>
>> Ensure the llvm and pahole are up to date.
>> Also take a look at the "Testing patches" and "LLVM" section in Documentation/bpf/bpf_devel_QA.rst.
>
> This will also require adding the l3/l4_ csum_replace() api's then. Adding the csum_replace() to a cgroup test case results in the below error during bpf program validation:
> "BPF_LD_[ABS|IND] instructions not allowed for this program type"
I saw you posted a new patch, so it seems you have resolved this
BPF_LD_[ABS|IND] issue. Do you know what is the reason for this
verification error? Here, the program type is cgroup_skb which should
not mess up with BPF_LD_[ABS|IND] which is mostly for classic bpf to
extended bpf conversion. Did I miss anything here?
>
> Is there something else that needs to be added? Or would it be better to create the function just for ds_field?
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB
2022-01-06 7:50 ` Yonghong Song
@ 2022-01-06 17:18 ` Tyler Wear (QUIC)
2022-01-06 22:35 ` Yonghong Song
0 siblings, 1 reply; 10+ messages in thread
From: Tyler Wear (QUIC) @ 2022-01-06 17:18 UTC (permalink / raw)
To: Yonghong Song, Tyler Wear (QUIC), Martin KaFai Lau; +Cc: netdev, bpf, maze
> -----Original Message-----
> From: Yonghong Song <yhs@fb.com>
> Sent: Wednesday, January 5, 2022 11:51 PM
> To: Tyler Wear (QUIC) <quic_twear@quicinc.com>; Martin KaFai Lau <kafai@fb.com>
> Cc: netdev@vger.kernel.org; bpf@vger.kernel.org; maze@google.com
> Subject: Re: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB
>
> WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
>
> On 1/4/22 4:27 PM, Tyler Wear (QUIC) wrote:
> >
> >
> >> -----Original Message-----
> >> From: Martin KaFai Lau <kafai@fb.com>
> >> Sent: Wednesday, December 29, 2021 1:06 PM
> >> To: Tyler Wear <twear@quicinc.com>
> >> Cc: Yonghong Song <yhs@fb.com>; Tyler Wear (QUIC)
> >> <quic_twear@quicinc.com>; netdev@vger.kernel.org;
> >> bpf@vger.kernel.org; maze@google.com
> >> Subject: Re: [PATCH] Add skb_store_bytes() for
> >> BPF_PROG_TYPE_CGROUP_SKB
> >>
> >> WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable
> macros.
> >>
> >> On Wed, Dec 29, 2021 at 06:29:05PM +0000, Tyler Wear wrote:
> >>> Unable to run any bpf tests do to errors below. These occur with and without the new patch. Is this a known issue?
> >>> Is the new test case required since bpf_skb_store_bytes() is already a tested function for other prog types?
> >>>
> >>> libbpf: failed to find BTF for extern 'bpf_testmod_invalid_mod_kfunc'
> >>> [18] section: -2
> >>> Error: failed to open BPF object file: No such file or directory
> >>> libbpf: failed to find BTF info for global/extern symbol 'my_tid'
> >>> Error: failed to link
> >>> '/local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/linke
> >>> d_
> >>> funcs1.o': Unknown error -2 (-2)
> >>> libbpf: failed to find BTF for extern 'bpf_kfunc_call_test1' [27]
> >>> section: -2
> >> tools/testing/selftests/bpf/README.rst has details on these.
> >>
> >> Ensure the llvm and pahole are up to date.
> >> Also take a look at the "Testing patches" and "LLVM" section in Documentation/bpf/bpf_devel_QA.rst.
> >
> > This will also require adding the l3/l4_ csum_replace() api's then. Adding the csum_replace() to a cgroup test case results in the
> below error during bpf program validation:
> > "BPF_LD_[ABS|IND] instructions not allowed for this program type"
>
> I saw you posted a new patch, so it seems you have resolved this BPF_LD_[ABS|IND] issue. Do you know what is the reason for this
> verification error? Here, the program type is cgroup_skb which should not mess up with BPF_LD_[ABS|IND] which is mostly for classic
> bpf to extended bpf conversion. Did I miss anything here?
>
Was an issue with using bpf_legacy.h to load bytes.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB
2022-01-06 17:18 ` Tyler Wear (QUIC)
@ 2022-01-06 22:35 ` Yonghong Song
0 siblings, 0 replies; 10+ messages in thread
From: Yonghong Song @ 2022-01-06 22:35 UTC (permalink / raw)
To: Tyler Wear (QUIC), Martin KaFai Lau; +Cc: netdev, bpf, maze
On 1/6/22 9:18 AM, Tyler Wear (QUIC) wrote:
>
>
>> -----Original Message-----
>> From: Yonghong Song <yhs@fb.com>
>> Sent: Wednesday, January 5, 2022 11:51 PM
>> To: Tyler Wear (QUIC) <quic_twear@quicinc.com>; Martin KaFai Lau <kafai@fb.com>
>> Cc: netdev@vger.kernel.org; bpf@vger.kernel.org; maze@google.com
>> Subject: Re: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB
>>
>> WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
>>
>> On 1/4/22 4:27 PM, Tyler Wear (QUIC) wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Martin KaFai Lau <kafai@fb.com>
>>>> Sent: Wednesday, December 29, 2021 1:06 PM
>>>> To: Tyler Wear <twear@quicinc.com>
>>>> Cc: Yonghong Song <yhs@fb.com>; Tyler Wear (QUIC)
>>>> <quic_twear@quicinc.com>; netdev@vger.kernel.org;
>>>> bpf@vger.kernel.org; maze@google.com
>>>> Subject: Re: [PATCH] Add skb_store_bytes() for
>>>> BPF_PROG_TYPE_CGROUP_SKB
>>>>
>>>> WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable
>> macros.
>>>>
>>>> On Wed, Dec 29, 2021 at 06:29:05PM +0000, Tyler Wear wrote:
>>>>> Unable to run any bpf tests do to errors below. These occur with and without the new patch. Is this a known issue?
>>>>> Is the new test case required since bpf_skb_store_bytes() is already a tested function for other prog types?
>>>>>
>>>>> libbpf: failed to find BTF for extern 'bpf_testmod_invalid_mod_kfunc'
>>>>> [18] section: -2
>>>>> Error: failed to open BPF object file: No such file or directory
>>>>> libbpf: failed to find BTF info for global/extern symbol 'my_tid'
>>>>> Error: failed to link
>>>>> '/local/mnt/workspace/linux-stable/tools/testing/selftests/bpf/linke
>>>>> d_
>>>>> funcs1.o': Unknown error -2 (-2)
>>>>> libbpf: failed to find BTF for extern 'bpf_kfunc_call_test1' [27]
>>>>> section: -2
>>>> tools/testing/selftests/bpf/README.rst has details on these.
>>>>
>>>> Ensure the llvm and pahole are up to date.
>>>> Also take a look at the "Testing patches" and "LLVM" section in Documentation/bpf/bpf_devel_QA.rst.
>>>
>>> This will also require adding the l3/l4_ csum_replace() api's then. Adding the csum_replace() to a cgroup test case results in the
>> below error during bpf program validation:
>>> "BPF_LD_[ABS|IND] instructions not allowed for this program type"
>>
>> I saw you posted a new patch, so it seems you have resolved this BPF_LD_[ABS|IND] issue. Do you know what is the reason for this
>> verification error? Here, the program type is cgroup_skb which should not mess up with BPF_LD_[ABS|IND] which is mostly for classic
>> bpf to extended bpf conversion. Did I miss anything here?
>>
>
> Was an issue with using bpf_legacy.h to load bytes.
Okay, I see. that makes sense. The helper bpf_skb_load_bytes() is the
way to go.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-01-06 22:36 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-22 2:27 [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB Tyler Wear
2021-12-22 3:43 ` Yonghong Song
2021-12-22 22:49 ` Tyler Wear
2021-12-22 23:50 ` Martin KaFai Lau
2021-12-29 18:29 ` Tyler Wear
2021-12-29 21:05 ` Martin KaFai Lau
2022-01-05 0:27 ` Tyler Wear (QUIC)
2022-01-06 7:50 ` Yonghong Song
2022-01-06 17:18 ` Tyler Wear (QUIC)
2022-01-06 22:35 ` Yonghong Song
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.