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

[Alexei Starovoitov]

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.

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

[Wangnan (F)]

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.

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

[Alexei Starovoitov]

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?

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

[Wangnan (F)]

On 2017/3/31 9:31, Alexei Starovoitov wrote:

expose bpf_program__set_type() to set program type

Signed-off-by: Alexei Starovoitov 
Acked-by: Daniel Borkmann 
Acked-by: Martin KaFai Lau 
---
  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.