Re: [PATCH bpf-next] selftests/bpf: test_sockmap, use section names understood by libbpf

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jakub,

On Wed, 2024-05-22 at 10:09 +0200, Jakub Sitnicki wrote:
> libbpf can deduce program type and attach type from the ELF section
> name.
> We don't need to pass it out-of-band if we switch to libbpf
> convention [1].
> 
> [1] https://docs.kernel.org/bpf/libbpf/program_types.html
> 
> Signed-off-by: Jakub Sitnicki <jakub@xxxxxxxxxxxxxx>

Thanks for this patch, it works well. But ...

> ---
>  .../selftests/bpf/progs/test_sockmap_kern.h   | 17 +++++-----
>  tools/testing/selftests/bpf/test_sockmap.c    | 31 -----------------
> --
>  2 files changed, 9 insertions(+), 39 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_kern.h
> b/tools/testing/selftests/bpf/progs/test_sockmap_kern.h
> index 99d2ea9fb658..3dff0813730b 100644
> --- a/tools/testing/selftests/bpf/progs/test_sockmap_kern.h
> +++ b/tools/testing/selftests/bpf/progs/test_sockmap_kern.h
> @@ -92,7 +92,7 @@ struct {
>  	__uint(value_size, sizeof(int));
>  } tls_sock_map SEC(".maps");
>  
> -SEC("sk_skb1")
> +SEC("sk_skb/stream_parser")
>  int bpf_prog1(struct __sk_buff *skb)
>  {
>  	int *f, two = 2;
> @@ -104,7 +104,7 @@ int bpf_prog1(struct __sk_buff *skb)
>  	return skb->len;
>  }
>  
> -SEC("sk_skb2")
> +SEC("sk_skb/stream_verdict")
>  int bpf_prog2(struct __sk_buff *skb)
>  {
>  	__u32 lport = skb->local_port;
> @@ -151,7 +151,7 @@ static inline void bpf_write_pass(struct
> __sk_buff *skb, int offset)
>  		memcpy(c + offset, "PASS", 4);
>  }
>  
> -SEC("sk_skb3")
> +SEC("sk_skb/stream_verdict")
>  int bpf_prog3(struct __sk_buff *skb)
>  {
>  	int err, *f, ret = SK_PASS;
> @@ -233,7 +233,7 @@ int bpf_sockmap(struct bpf_sock_ops *skops)
>  	return 0;
>  }
>  
> -SEC("sk_msg1")
> +SEC("sk_msg")
>  int bpf_prog4(struct sk_msg_md *msg)
>  {
>  	int *bytes, zero = 0, one = 1, two = 2, three = 3, four = 4,
> five = 5;
> @@ -263,7 +263,7 @@ int bpf_prog4(struct sk_msg_md *msg)
>  	return SK_PASS;
>  }
>  
> -SEC("sk_msg2")
> +SEC("sk_msg")
>  int bpf_prog6(struct sk_msg_md *msg)
>  {
>  	int zero = 0, one = 1, two = 2, three = 3, four = 4, five =
> 5, key = 0;
> @@ -308,7 +308,7 @@ int bpf_prog6(struct sk_msg_md *msg)
>  #endif
>  }
>  
> -SEC("sk_msg3")
> +SEC("sk_msg")
>  int bpf_prog8(struct sk_msg_md *msg)
>  {
>  	void *data_end = (void *)(long) msg->data_end;
> @@ -329,7 +329,8 @@ int bpf_prog8(struct sk_msg_md *msg)
>  
>  	return SK_PASS;
>  }
> -SEC("sk_msg4")
> +
> +SEC("sk_msg")
>  int bpf_prog9(struct sk_msg_md *msg)
>  {
>  	void *data_end = (void *)(long) msg->data_end;
> @@ -347,7 +348,7 @@ int bpf_prog9(struct sk_msg_md *msg)
>  	return SK_PASS;
>  }
>  
> -SEC("sk_msg5")
> +SEC("sk_msg")
>  int bpf_prog10(struct sk_msg_md *msg)
>  {
>  	int *bytes, *start, *end, *start_push, *end_push,
> *start_pop, *pop;
> diff --git a/tools/testing/selftests/bpf/test_sockmap.c
> b/tools/testing/selftests/bpf/test_sockmap.c
> index 4499b3cfc3a6..ddc6a9cef36f 100644
> --- a/tools/testing/selftests/bpf/test_sockmap.c
> +++ b/tools/testing/selftests/bpf/test_sockmap.c
> @@ -1783,30 +1783,6 @@ char *map_names[] = {
>  	"tls_sock_map",
>  };
>  
> -int prog_attach_type[] = {
> -	BPF_SK_SKB_STREAM_PARSER,
> -	BPF_SK_SKB_STREAM_VERDICT,
> -	BPF_SK_SKB_STREAM_VERDICT,
> -	BPF_CGROUP_SOCK_OPS,
> -	BPF_SK_MSG_VERDICT,
> -	BPF_SK_MSG_VERDICT,
> -	BPF_SK_MSG_VERDICT,
> -	BPF_SK_MSG_VERDICT,
> -	BPF_SK_MSG_VERDICT,
> -};

prog_attach_type is still used by my commit:

https://lore.kernel.org/bpf/e27d7d0c1e0e79b0acd22ac6ad5d8f9f00225303.1716372485.git.tanggeliang@xxxxxxxxxx/T/#u

Please review it for me.

If my commit is acceptable, this patch will conflict with it. It's a
bit strange to delete this prog_attach_type in your patch and then add
it back in my commit. So could you please rebase this patch on my
commit in that case. Sorry for the trouble.

Anyway please add my tag:

Acked-and-tested-by: Geliang Tang <geliang@xxxxxxxxxx> 

Thanks,
-Geliang

> -
> -int prog_type[] = {
> -	BPF_PROG_TYPE_SK_SKB,
> -	BPF_PROG_TYPE_SK_SKB,
> -	BPF_PROG_TYPE_SK_SKB,
> -	BPF_PROG_TYPE_SOCK_OPS,
> -	BPF_PROG_TYPE_SK_MSG,
> -	BPF_PROG_TYPE_SK_MSG,
> -	BPF_PROG_TYPE_SK_MSG,
> -	BPF_PROG_TYPE_SK_MSG,
> -	BPF_PROG_TYPE_SK_MSG,
> -};
> -
>  static int populate_progs(char *bpf_file)
>  {
>  	struct bpf_program *prog;
> @@ -1825,13 +1801,6 @@ static int populate_progs(char *bpf_file)
>  		return -1;
>  	}
>  
> -	bpf_object__for_each_program(prog, obj) {
> -		bpf_program__set_type(prog, prog_type[i]);
> -		bpf_program__set_expected_attach_type(prog,
> -						     
> prog_attach_type[i]);
> -		i++;
> -	}
> -
>  	i = bpf_object__load(obj);
>  	i = 0;
>  	bpf_object__for_each_program(prog, obj) {





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux