Re: [PATCH] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB

On 1/4/22 4:27 PM, Tyler Wear (QUIC) wrote:

quoted

-----Original Message-----
From: Martin KaFai Lau <redacted>
Sent: Wednesday, December 29, 2021 1:06 PM
To: Tyler Wear <redacted>
Cc: Yonghong Song <redacted>; Tyler Wear (QUIC) <redacted>; 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:

quoted

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?