* Re: [PATCH bpf-next 0/4] libbpf: add XDP binding support
[not found] <57ad1a20-4e41-474c-8829-d6a48c6b1883@email.android.com>
@ 2018-01-22 14:00 ` Daniel Borkmann
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Borkmann @ 2018-01-22 14:00 UTC (permalink / raw)
To: Éric Leblond; +Cc: Alexei Starovoitov, netdev, linux-kernel
On 01/20/2018 09:28 AM, Éric Leblond wrote:
> Hi,
>
> Le 20 janv. 2018 9:21 AM, Daniel Borkmann <daniel@iogearbox.net> a écrit :
>
> On 01/20/2018 03:27 AM, Alexei Starovoitov wrote:
> > On Sat, Jan 20, 2018 at 03:00:37AM +0100, Daniel Borkmann wrote:
> >> On 01/19/2018 12:43 AM, Eric Leblond wrote:
> >>> Hello,
> >>>
> >>> This patchset rebases the libbpf code on latest bpf-next code and
> addresses
> >>> remarks by Daniel.
> >>
> >> Ok, I think it's a good start. We should later on clean up the
> >> netlink handling code a bit, but that's all internal and can be
> >> done in a second step. Applied to bpf-next, thanks Eric.
> >
> > Sorry, Eric, Daniel.
> > I had to revert this patch set. It breaks build on systems
> > where headers are not the most recent.
>
> Oops, sorry.
>
> > Since libbpf is used by perf it has to be built cleanly on centos7 at least.
> >
> > The errors I got:
> > bpf.c: In function ‘bpf_set_link_xdp_fd’:
> > bpf.c:456:23: error: ‘SOL_NETLINK’ undeclared (first use in this function)
> > if (setsockopt(sock, SOL_NETLINK, NETLINK_EXT_ACK,
> > ^~~~~~~~~~~
> > bpf.c:456:23: note: each undeclared identifier is reported only once for
> each function it appears in
> > bpf.c:456:36: error: ‘NETLINK_EXT_ACK’ undeclared (first use in this
> function)
> > if (setsockopt(sock, SOL_NETLINK, NETLINK_EXT_ACK,
> > ^~~~~~~~~~~~~~~
> > nlattr.c: In function ‘nla_dump_errormsg’:
> > nlattr.c:152:34: error: ‘NLMSGERR_ATTR_MAX’ undeclared (first use in this
> function)
> > struct nla_policy extack_policy[NLMSGERR_ATTR_MAX + 1] = {
>
> Yeah, fully agree, thanks for catching this, Alexei!
>
> What's the recommended solution here ? Include some kernel tree files or define
> constant if not defined ?
I think typical way is to pull such headers into tools/include/.
Cheers,
Daniel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next v5 2/4] libbpf: add error reporting in XDP
@ 2018-01-18 23:35 Eric Leblond
2018-01-18 23:43 ` [PATCH bpf-next 0/4] libbpf: add XDP binding support Eric Leblond
0 siblings, 1 reply; 5+ messages in thread
From: Eric Leblond @ 2018-01-18 23:35 UTC (permalink / raw)
To: Daniel Borkmann, Toshiaki Makita, Philippe Ombredanne
Cc: Alexei Starovoitov, netdev, linux-kernel, Thomas Graf
Hi,
Sorry for the delay, missed the mail.
On Sat, 2018-01-06 at 22:16 +0100, Daniel Borkmann wrote:
> On 01/04/2018 09:21 AM, Eric Leblond wrote:
> > Parse netlink ext attribute to get the error message returned by
> > the card. Code is partially take from libnl.
> >
> > Signed-off-by: Eric Leblond <eric@regit.org>
> > Acked-by: Alexei Starovoitov <ast@kernel.org>
> > ---
> > samples/bpf/Makefile | 2 +-
> > tools/lib/bpf/Build | 2 +-
> > tools/lib/bpf/bpf.c | 10 ++-
> > tools/lib/bpf/nlattr.c | 187
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> > tools/lib/bpf/nlattr.h | 70 ++++++++++++++++++
> > 5 files changed, 268 insertions(+), 3 deletions(-)
> > create mode 100644 tools/lib/bpf/nlattr.c
> > create mode 100644 tools/lib/bpf/nlattr.h
> >
> > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> > index 4fb944a7ecf8..c889ebcba9b3 100644
> > --- a/samples/bpf/Makefile
> > +++ b/samples/bpf/Makefile
> > @@ -44,7 +44,7 @@ hostprogs-y += xdp_monitor
> > hostprogs-y += syscall_tp
> >
> > # Libbpf dependencies
> > -LIBBPF := ../../tools/lib/bpf/bpf.o
> > +LIBBPF := ../../tools/lib/bpf/bpf.o ../../tools/lib/bpf/nlattr.o
> > CGROUP_HELPERS :=
> > ../../tools/testing/selftests/bpf/cgroup_helpers.o
> >
> > test_lru_dist-objs := test_lru_dist.o $(LIBBPF)
> > diff --git a/tools/lib/bpf/Build b/tools/lib/bpf/Build
> > index d8749756352d..64c679d67109 100644
> > --- a/tools/lib/bpf/Build
> > +++ b/tools/lib/bpf/Build
> > @@ -1 +1 @@
> > -libbpf-y := libbpf.o bpf.o
> > +libbpf-y := libbpf.o bpf.o nlattr.o
> > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> > index e6c61035b64c..10d71b9fdbd0 100644
> > --- a/tools/lib/bpf/bpf.c
> > +++ b/tools/lib/bpf/bpf.c
> > @@ -26,6 +26,7 @@
> > #include <linux/bpf.h>
> > #include "bpf.h"
> > #include "libbpf.h"
> > +#include "nlattr.h"
> > #include <linux/rtnetlink.h>
> > #include <sys/socket.h>
> > #include <errno.h>
> > @@ -440,6 +441,7 @@ int bpf_set_link_xdp_fd(int ifindex, int fd,
> > __u32 flags)
> > struct nlmsghdr *nh;
> > struct nlmsgerr *err;
> > socklen_t addrlen;
> > + int one;
>
> Hmm, it's not initialized here to 1 ...
>
> > memset(&sa, 0, sizeof(sa));
> > sa.nl_family = AF_NETLINK;
> > @@ -449,6 +451,11 @@ int bpf_set_link_xdp_fd(int ifindex, int fd,
> > __u32 flags)
> > return -errno;
> > }
> >
> > + if (setsockopt(sock, SOL_NETLINK, NETLINK_EXT_ACK,
> > + &one, sizeof(one)) < 0) {
>
> ... so we turn it on by chance here.
Indeed, fixing that.
> > + fprintf(stderr, "Netlink error reporting not
> > supported\n");
> > + }
> > +
> > if (bind(sock, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
> > ret = -errno;
> > goto cleanup;
> > @@ -524,7 +531,8 @@ int bpf_set_link_xdp_fd(int ifindex, int fd,
> > __u32 flags)
> > err = (struct nlmsgerr *)NLMSG_DATA(nh);
> > if (!err->error)
> > continue;
> > - ret = err->error;
> > + ret = -err->error;
>
> This one looks strange. Your prior patch added the 'ret = err->error'
> and this one negates it. Which variant is the correct version? From
> digging into the kernel code, my take is that 'ret = err->error' was
> the correct variant since it already holds the negative error code.
> Could you double check?
Yes all netlink_ack usage I have seen are using the negative value of
the error. Fixing that too.
BR,
--
Eric Leblond <eric@regit.org>
Blog: https://home.regit.org/
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH bpf-next 0/4] libbpf: add XDP binding support
2018-01-18 23:35 [PATCH bpf-next v5 2/4] libbpf: add error reporting in XDP Eric Leblond
@ 2018-01-18 23:43 ` Eric Leblond
2018-01-20 2:00 ` Daniel Borkmann
0 siblings, 1 reply; 5+ messages in thread
From: Eric Leblond @ 2018-01-18 23:43 UTC (permalink / raw)
To: daniel; +Cc: alexei.starovoitov, netdev, linux-kernel
Hello,
This patchset rebases the libbpf code on latest bpf-next code and addresses
remarks by Daniel.
Best regards,
--
Eric Leblond
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next 0/4] libbpf: add XDP binding support
2018-01-18 23:43 ` [PATCH bpf-next 0/4] libbpf: add XDP binding support Eric Leblond
@ 2018-01-20 2:00 ` Daniel Borkmann
2018-01-20 2:27 ` Alexei Starovoitov
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Borkmann @ 2018-01-20 2:00 UTC (permalink / raw)
To: Eric Leblond; +Cc: alexei.starovoitov, netdev, linux-kernel
On 01/19/2018 12:43 AM, Eric Leblond wrote:
> Hello,
>
> This patchset rebases the libbpf code on latest bpf-next code and addresses
> remarks by Daniel.
Ok, I think it's a good start. We should later on clean up the
netlink handling code a bit, but that's all internal and can be
done in a second step. Applied to bpf-next, thanks Eric.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next 0/4] libbpf: add XDP binding support
2018-01-20 2:00 ` Daniel Borkmann
@ 2018-01-20 2:27 ` Alexei Starovoitov
2018-01-20 8:21 ` Daniel Borkmann
0 siblings, 1 reply; 5+ messages in thread
From: Alexei Starovoitov @ 2018-01-20 2:27 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Eric Leblond, netdev, linux-kernel
On Sat, Jan 20, 2018 at 03:00:37AM +0100, Daniel Borkmann wrote:
> On 01/19/2018 12:43 AM, Eric Leblond wrote:
> > Hello,
> >
> > This patchset rebases the libbpf code on latest bpf-next code and addresses
> > remarks by Daniel.
>
> Ok, I think it's a good start. We should later on clean up the
> netlink handling code a bit, but that's all internal and can be
> done in a second step. Applied to bpf-next, thanks Eric.
Sorry, Eric, Daniel.
I had to revert this patch set. It breaks build on systems
where headers are not the most recent.
Since libbpf is used by perf it has to be built cleanly on centos7 at least.
The errors I got:
bpf.c: In function ‘bpf_set_link_xdp_fd’:
bpf.c:456:23: error: ‘SOL_NETLINK’ undeclared (first use in this function)
if (setsockopt(sock, SOL_NETLINK, NETLINK_EXT_ACK,
^~~~~~~~~~~
bpf.c:456:23: note: each undeclared identifier is reported only once for each function it appears in
bpf.c:456:36: error: ‘NETLINK_EXT_ACK’ undeclared (first use in this function)
if (setsockopt(sock, SOL_NETLINK, NETLINK_EXT_ACK,
^~~~~~~~~~~~~~~
nlattr.c: In function ‘nla_dump_errormsg’:
nlattr.c:152:34: error: ‘NLMSGERR_ATTR_MAX’ undeclared (first use in this function)
struct nla_policy extack_policy[NLMSGERR_ATTR_MAX + 1] = {
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next 0/4] libbpf: add XDP binding support
2018-01-20 2:27 ` Alexei Starovoitov
@ 2018-01-20 8:21 ` Daniel Borkmann
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Borkmann @ 2018-01-20 8:21 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Eric Leblond, netdev, linux-kernel
On 01/20/2018 03:27 AM, Alexei Starovoitov wrote:
> On Sat, Jan 20, 2018 at 03:00:37AM +0100, Daniel Borkmann wrote:
>> On 01/19/2018 12:43 AM, Eric Leblond wrote:
>>> Hello,
>>>
>>> This patchset rebases the libbpf code on latest bpf-next code and addresses
>>> remarks by Daniel.
>>
>> Ok, I think it's a good start. We should later on clean up the
>> netlink handling code a bit, but that's all internal and can be
>> done in a second step. Applied to bpf-next, thanks Eric.
>
> Sorry, Eric, Daniel.
> I had to revert this patch set. It breaks build on systems
> where headers are not the most recent.
>
> Since libbpf is used by perf it has to be built cleanly on centos7 at least.
>
> The errors I got:
> bpf.c: In function ‘bpf_set_link_xdp_fd’:
> bpf.c:456:23: error: ‘SOL_NETLINK’ undeclared (first use in this function)
> if (setsockopt(sock, SOL_NETLINK, NETLINK_EXT_ACK,
> ^~~~~~~~~~~
> bpf.c:456:23: note: each undeclared identifier is reported only once for each function it appears in
> bpf.c:456:36: error: ‘NETLINK_EXT_ACK’ undeclared (first use in this function)
> if (setsockopt(sock, SOL_NETLINK, NETLINK_EXT_ACK,
> ^~~~~~~~~~~~~~~
> nlattr.c: In function ‘nla_dump_errormsg’:
> nlattr.c:152:34: error: ‘NLMSGERR_ATTR_MAX’ undeclared (first use in this function)
> struct nla_policy extack_policy[NLMSGERR_ATTR_MAX + 1] = {
Yeah, fully agree, thanks for catching this, Alexei!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-01-22 14:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <57ad1a20-4e41-474c-8829-d6a48c6b1883@email.android.com>
2018-01-22 14:00 ` [PATCH bpf-next 0/4] libbpf: add XDP binding support Daniel Borkmann
2018-01-18 23:35 [PATCH bpf-next v5 2/4] libbpf: add error reporting in XDP Eric Leblond
2018-01-18 23:43 ` [PATCH bpf-next 0/4] libbpf: add XDP binding support Eric Leblond
2018-01-20 2:00 ` Daniel Borkmann
2018-01-20 2:27 ` Alexei Starovoitov
2018-01-20 8:21 ` Daniel Borkmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).