Re: [PATCH bpf-next v7 2/5] libbpf: add function to setup XDP

[Eric Leblond <eric@regit.org>]

Hi,

On Sat, 2018-01-27 at 02:23 +0100, Daniel Borkmann wrote:
> On 01/25/2018 01:05 AM, Eric Leblond wrote:
> > Most of the code is taken from set_link_xdp_fd() in bpf_load.c and
> > slightly modified to be library compliant.
> > 
> > Signed-off-by: Eric Leblond <eric@regit.org>
> > Acked-by: Alexei Starovoitov <ast@kernel.org>
> > ---
> >  tools/lib/bpf/bpf.c    | 127
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> >  tools/lib/bpf/libbpf.c |   2 +
> >  tools/lib/bpf/libbpf.h |   4 ++
> >  3 files changed, 133 insertions(+)
> > 
> > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> > index 5128677e4117..749a447ec9ed 100644
> > --- a/tools/lib/bpf/bpf.c
> > +++ b/tools/lib/bpf/bpf.c
> > @@ -25,6 +25,17 @@
> >  #include <asm/unistd.h>
> >  #include <linux/bpf.h>
> >  #include "bpf.h"
> > +#include "libbpf.h"
> > +#include "nlattr.h"
> > +#include <uapi/linux/rtnetlink.h>
> 
> Doesn't libbpf pull in already -I$(srctree)/tools/include/uapi? Seems
> the
> other headers don't need 'uapi/' path prefix.

Right, it works without the uapi.
> 
> > +#include <sys/socket.h>
> > +#include <errno.h>
> > +
> > +#ifndef IFLA_XDP_MAX
> > +#define IFLA_XDP	43
> > +#define IFLA_XDP_FD	1
> > +#define IFLA_XDP_FLAGS	3
> > +#endif
> 
> Hm, given we pull in tools/include/uapi/linux/netlink.h, shouldn't we
> also
> get include/uapi/linux/if_link.h dependency in here, so above ifdef
> workaround
> can be avoided?

This values are fixed so we risk nothing by keeping a definition if
ever it is not available in system headers. But it is fine with me if
you want me to add if_link.h to include/uapi/. 

BR,
-- 
Eric Leblond <eric@regit.org>
Blog: https://home.regit.org/