Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <1517048544.24241.16.camel@regit.org>
Subject: Re: [PATCH bpf-next v7 2/5] libbpf: add function to setup XDP
From:   Eric Leblond <eric@regit.org>
To:     Daniel Borkmann <daniel@iogearbox.net>
Cc:     alexei.starovoitov@gmail.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Date:   Sat, 27 Jan 2018 11:22:24 +0100
In-Reply-To: <f3b716a4-2fb8-86c9-bf0c-d87511d85446@iogearbox.net>
References: <4dc597a1-5046-3805-eb81-03a23bf0acbb@iogearbox.net>
         <20180125000548.12320-1-eric@regit.org>
         <20180125000548.12320-3-eric@regit.org>
         <f3b716a4-2fb8-86c9-bf0c-d87511d85446@iogearbox.net>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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/