netdev - RE: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 29 Dec 2021 18:29:05 +0000
From:   Tyler Wear <twear@...cinc.com>
To:     Martin KaFai Lau <kafai@...com>
CC:     Yonghong Song <yhs@...com>,
        "Tyler Wear (QUIC)" <quic_twear@...cinc.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "maze@...gle.com" <maze@...gle.com>
Subject: RE: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB



> -----Original Message-----
> From: Martin KaFai Lau <kafai@...com>
> Sent: Wednesday, December 22, 2021 3:51 PM
> To: Tyler Wear <twear@...cinc.com>
> Cc: Yonghong Song <yhs@...com>; Tyler Wear (QUIC) <quic_twear@...cinc.com>; netdev@...r.kernel.org; bpf@...r.kernel.org;
> maze@...gle.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@...cinc.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'