On 31/07/2024 11:38, Alexis Lothoré (eBPF Foundation) wrote:
> test_skb_cgroup_id_kern.c is currently involved in a manual test. In its
> current form, it can not be used with the auto-generated skeleton APIs,
> because the section name is not valid to allow libbpf to deduce the program
> type.
> 
> Update section name to allow skeleton APIs usage. Also rename the program
> name to make it shorter and more straighforward regarding the API it is
> testing. While doing so, make sure that test_skb_cgroup_id.sh passes to get
> a working reference before converting it to test_progs
> - update the obj name
> - fix loading issue (verifier rejecting the program when loaded through tc,
>   because of map not found), by preloading the whole obj with bpftool
> 
> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.loth...@bootlin.com>

Reviewed-by: Alan Maguire <alan.magu...@oracle.com>

> ---
>  .../progs/{test_skb_cgroup_id_kern.c => cgroup_ancestor.c}   |  2 +-
>  tools/testing/selftests/bpf/test_skb_cgroup_id.sh            | 12 
> ++++++++----
>  2 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/test_skb_cgroup_id_kern.c 
> b/tools/testing/selftests/bpf/progs/cgroup_ancestor.c
> similarity index 97%
> rename from tools/testing/selftests/bpf/progs/test_skb_cgroup_id_kern.c
> rename to tools/testing/selftests/bpf/progs/cgroup_ancestor.c
> index 37aacc66cd68..4879645f5827 100644
> --- a/tools/testing/selftests/bpf/progs/test_skb_cgroup_id_kern.c
> +++ b/tools/testing/selftests/bpf/progs/cgroup_ancestor.c
> @@ -28,7 +28,7 @@ static __always_inline void log_nth_level(struct __sk_buff 
> *skb, __u32 level)
>       bpf_map_update_elem(&cgroup_ids, &level, &id, 0);
>  }
>  
> -SEC("cgroup_id_logger")
> +SEC("tc")
>  int log_cgroup_id(struct __sk_buff *skb)
>  {
>       /* Loop unroll can't be used here due to [1]. Unrolling manually.
> diff --git a/tools/testing/selftests/bpf/test_skb_cgroup_id.sh 
> b/tools/testing/selftests/bpf/test_skb_cgroup_id.sh
> index 515c2eafc97f..d7dad49175c2 100755
> --- a/tools/testing/selftests/bpf/test_skb_cgroup_id.sh
> +++ b/tools/testing/selftests/bpf/test_skb_cgroup_id.sh
> @@ -30,8 +30,10 @@ setup()
>       wait_for_ip
>  
>       tc qdisc add dev ${TEST_IF} clsact
> -     tc filter add dev ${TEST_IF} egress bpf obj ${BPF_PROG_OBJ} \
> -             sec ${BPF_PROG_SECTION} da
> +     mkdir -p /sys/fs/bpf/${BPF_PROG_PIN}
> +     bpftool prog loadall ${BPF_PROG_OBJ} /sys/fs/bpf/${BPF_PROG_PIN} type tc
> +     tc filter add dev ${TEST_IF} egress bpf da object-pinned \
> +             /sys/fs/bpf/${BPF_PROG_PIN}/${BPF_PROG_NAME}
>

Just out of curiosity; did you find that the pin is needed? I would have
thought tc attach would be enough to keep the program aroud.

>       BPF_PROG_ID=$(tc filter show dev ${TEST_IF} egress | \
>                       awk '/ id / {sub(/.* id /, "", $0); print($1)}')
> @@ -41,6 +43,7 @@ cleanup()
>  {
>       ip link del ${TEST_IF} 2>/dev/null || :
>       ip link del ${TEST_IF_PEER} 2>/dev/null || :
> +     rm -rf /sys/fs/bpf/${BPF_PROG_PIN}
>  }
>  
>  main()
> @@ -54,8 +57,9 @@ DIR=$(dirname $0)
>  TEST_IF="test_cgid_1"
>  TEST_IF_PEER="test_cgid_2"
>  MAX_PING_TRIES=5
> -BPF_PROG_OBJ="${DIR}/test_skb_cgroup_id_kern.bpf.o"
> -BPF_PROG_SECTION="cgroup_id_logger"
> +BPF_PROG_PIN="cgroup_ancestor"
> +BPF_PROG_OBJ="${DIR}/${BPF_PROG_PIN}.bpf.o"
> +BPF_PROG_NAME="log_cgroup_id"
>  BPF_PROG_ID=0
>  PROG="${DIR}/test_skb_cgroup_id_user"
>  type ping6 >/dev/null 2>&1 && PING6="ping6" || PING6="ping -6"
> 

Reply via email to