diff mbox

[net-next,4/6] tools/lib/bpf: expose bpf_program__set_type()

Message ID 20170331013157.3298003-5-ast@fb.com
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Alexei Starovoitov March 31, 2017, 1:31 a.m. UTC
expose bpf_program__set_type() to set program type

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/lib/bpf/libbpf.c | 3 +--
 tools/lib/bpf/libbpf.h | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Wangnan (F) March 31, 2017, 2:33 a.m. UTC | #1
On 2017/3/31 9:31, Alexei Starovoitov wrote:
> expose bpf_program__set_type() to set program type
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Martin KaFai Lau <kafai@fb.com>
> ---
>   tools/lib/bpf/libbpf.c | 3 +--
>   tools/lib/bpf/libbpf.h | 1 +
>   2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index ac6eb863b2a4..1a2c07eb7795 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1618,8 +1618,7 @@ int bpf_program__nth_fd(struct bpf_program *prog, int n)
>   	return fd;
>   }
>   
> -static void bpf_program__set_type(struct bpf_program *prog,
> -				  enum bpf_prog_type type)
> +void bpf_program__set_type(struct bpf_program *prog, enum bpf_prog_type type)
>   {
>   	prog->type = type;
>   }
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index b30394f9947a..82adde30b696 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -185,6 +185,7 @@ int bpf_program__set_sched_cls(struct bpf_program *prog);
>   int bpf_program__set_sched_act(struct bpf_program *prog);
>   int bpf_program__set_xdp(struct bpf_program *prog);
>   int bpf_program__set_perf_event(struct bpf_program *prog);
> +void bpf_program__set_type(struct bpf_program *prog, enum bpf_prog_type type);
>   

This makes libbpf.h depend on uapi/linux/bpf.h (because of enum
bpf_prog_type), which is not always available.

What about defining another enum inside libbpf.h?

Thank you.
Alexei Starovoitov March 31, 2017, 2:37 a.m. UTC | #2
On 3/30/17 7:33 PM, Wangnan (F) wrote:
>> +void bpf_program__set_type(struct bpf_program *prog, enum
>> bpf_prog_type type);
>>
>
> This makes libbpf.h depend on uapi/linux/bpf.h (because of enum
> bpf_prog_type), which is not always available.
>
> What about defining another enum inside libbpf.h?

how about just including bpf.h? or making it 'int' instead of enum?
Wangnan (F) March 31, 2017, 2:48 a.m. UTC | #3
On 2017/3/31 10:37, Alexei Starovoitov wrote:
> On 3/30/17 7:33 PM, Wangnan (F) wrote:
>>> +void bpf_program__set_type(struct bpf_program *prog, enum
>>> bpf_prog_type type);
>>>
>>
>> This makes libbpf.h depend on uapi/linux/bpf.h (because of enum
>> bpf_prog_type), which is not always available.
>>
>> What about defining another enum inside libbpf.h?
>
> how about just including bpf.h? or making it 'int' instead of enum?
>

Including either kernel header into libbpf.h makes a lot of trouble,
because kernel header and uapi have many other things we don't need
and may conflict with existing code.

Making it 'int' looks like a backdoor. We still need macro to define
each program type.

Thank you.
Alexei Starovoitov March 31, 2017, 2:56 a.m. UTC | #4
On 3/30/17 7:48 PM, Wangnan (F) wrote:
>
>
> On 2017/3/31 10:37, Alexei Starovoitov wrote:
>> On 3/30/17 7:33 PM, Wangnan (F) wrote:
>>>> +void bpf_program__set_type(struct bpf_program *prog, enum
>>>> bpf_prog_type type);
>>>>
>>>
>>> This makes libbpf.h depend on uapi/linux/bpf.h (because of enum
>>> bpf_prog_type), which is not always available.
>>>
>>> What about defining another enum inside libbpf.h?
>>
>> how about just including bpf.h? or making it 'int' instead of enum?
>>
>
> Including either kernel header into libbpf.h makes a lot of trouble,
> because kernel header and uapi have many other things we don't need
> and may conflict with existing code.

I'm not proposing to include kernel headers. Regular 
/usr/include/linux/bpf.h is enough. This library isn't going to be 
compiled on distros
that don't have bpf support anyway.

> Making it 'int' looks like a backdoor. We still need macro to define
> each program type.

macro for each program wasn't the greatest idea. It always
behind new program types and not usable for this use case.
See patches 5 and 6.
diff mbox

Patch

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ac6eb863b2a4..1a2c07eb7795 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1618,8 +1618,7 @@  int bpf_program__nth_fd(struct bpf_program *prog, int n)
 	return fd;
 }
 
-static void bpf_program__set_type(struct bpf_program *prog,
-				  enum bpf_prog_type type)
+void bpf_program__set_type(struct bpf_program *prog, enum bpf_prog_type type)
 {
 	prog->type = type;
 }
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index b30394f9947a..82adde30b696 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -185,6 +185,7 @@  int bpf_program__set_sched_cls(struct bpf_program *prog);
 int bpf_program__set_sched_act(struct bpf_program *prog);
 int bpf_program__set_xdp(struct bpf_program *prog);
 int bpf_program__set_perf_event(struct bpf_program *prog);
+void bpf_program__set_type(struct bpf_program *prog, enum bpf_prog_type type);
 
 bool bpf_program__is_socket_filter(struct bpf_program *prog);
 bool bpf_program__is_tracepoint(struct bpf_program *prog);