mbox series

[bpf-next,v2,0/2] sample: xdp1 improvements

Message ID 20181201002306.25834-1-mcroce@redhat.com
Headers show
Series sample: xdp1 improvements | expand

Message

Matteo Croce Dec. 1, 2018, 12:23 a.m. UTC
Small improvements to improve the readability and easiness
to use of the xdp1 sample.

Matteo Croce (2):
  samples: bpf: improve xdp1 example
  samples: bpf: get ifindex from ifname

 samples/bpf/xdp1_user.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

Comments

Alexei Starovoitov Dec. 1, 2018, 6:11 a.m. UTC | #1
On Sat, Dec 01, 2018 at 01:23:04AM +0100, Matteo Croce wrote:
> Small improvements to improve the readability and easiness
> to use of the xdp1 sample.

Applied to bpf-next.

I think that sample code could be more useful if it's wrapped with bash
script like selftests/test_xdp* tests do.
At that point it can move to selftests to get 0bot coverage.
Would you be interested in doing that?
Matteo Croce Dec. 2, 2018, 2:23 a.m. UTC | #2
On Sat, Dec 1, 2018 at 6:11 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Sat, Dec 01, 2018 at 01:23:04AM +0100, Matteo Croce wrote:
> > Small improvements to improve the readability and easiness
> > to use of the xdp1 sample.
>
> Applied to bpf-next.
>
> I think that sample code could be more useful if it's wrapped with bash
> script like selftests/test_xdp* tests do.
> At that point it can move to selftests to get 0bot coverage.
> Would you be interested in doing that?
>

It would be nice, but I think that the samples have more urgent issues
right now.
Many examples doesn't compile on my system (Fedora 29, GCC 8.2.1, Clang 7.0.0),
these are the errors that I encounter:

  HOSTCC  /home/matteo/src/kernel/linux/samples/bpf/test_lru_dist
/home/matteo/src/kernel/linux/samples/bpf/test_lru_dist.c:39:8: error:
redefinition of ‘struct list_head’
 struct list_head {
        ^~~~~~~~~

  HOSTCC  /home/matteo/src/kernel/linux/samples/bpf/sock_example
In file included from
/home/matteo/src/kernel/linux/samples/bpf/sock_example.c:27:
/usr/include/linux/ip.h:102:2: error: unknown type name ‘__sum16’
  __sum16 check;
  ^~~~~~~

  HOSTCC  /home/matteo/src/kernel/linux/samples/bpf/tracex5_user.o
/home/matteo/src/kernel/linux/samples/bpf/tracex5_user.c: In function
‘install_accept_all_seccomp’:
/home/matteo/src/kernel/linux/samples/bpf/tracex5_user.c:17:21: error:
array type has incomplete element type ‘struct sock_filter’
  struct sock_filter filter[] = {
                     ^~~~~~

  HOSTCC  /home/matteo/src/kernel/linux/samples/bpf/test_cgrp2_attach2.o
/home/matteo/src/kernel/linux/samples/bpf/test_cgrp2_attach2.c: In
function ‘prog_load_cnt’:
/home/matteo/src/kernel/linux/samples/bpf/test_cgrp2_attach2.c:229:3:
error: ‘BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE’ undeclared (first use in
this function); did you mean ‘BPF_MAP_TYPE_CGROUP_STORAGE’?
   BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  HOSTCC  /home/matteo/src/kernel/linux/samples/bpf/xdpsock_user.o
/home/matteo/src/kernel/linux/samples/bpf/xdpsock_user.c:59:15: error:
conflicting types for ‘u64’
 typedef __u64 u64;
               ^~~

To be able to compile the samples, I temporarily removed them from the
compilation this way:

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index be0a961450bc..33d7161f2231 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -4,8 +4,6 @@ BPF_SAMPLES_PATH ?= $(abspath $(srctree)/$(src))
 TOOLS_PATH := $(BPF_SAMPLES_PATH)/../../tools

 # List of programs to build
-hostprogs-y := test_lru_dist
-hostprogs-y += sock_example
 hostprogs-y += fds_example
 hostprogs-y += sockex1
 hostprogs-y += sockex2
@@ -14,7 +12,6 @@ hostprogs-y += tracex1
 hostprogs-y += tracex2
 hostprogs-y += tracex3
 hostprogs-y += tracex4
-hostprogs-y += tracex5
 hostprogs-y += tracex6
 hostprogs-y += tracex7
 hostprogs-y += test_probe_write_user
@@ -26,7 +23,6 @@ hostprogs-y += map_perf_test
 hostprogs-y += test_overhead
 hostprogs-y += test_cgrp2_array_pin
 hostprogs-y += test_cgrp2_attach
-hostprogs-y += test_cgrp2_attach2
 hostprogs-y += test_cgrp2_sock
 hostprogs-y += test_cgrp2_sock2
 hostprogs-y += xdp1
@@ -49,7 +45,6 @@ hostprogs-y += xdp_rxq_info
 hostprogs-y += syscall_tp
 hostprogs-y += cpustat
 hostprogs-y += xdp_adjust_tail
-hostprogs-y += xdpsock
 hostprogs-y += xdp_fwd
 hostprogs-y += task_fd_query
 hostprogs-y += xdp_sample_pkts

Regards,
Jakub Kicinski Dec. 3, 2018, 11 p.m. UTC | #3
On Sun, 2 Dec 2018 02:23:41 +0000, Matteo Croce wrote:
> On Sat, Dec 1, 2018 at 6:11 AM Alexei Starovoitov wrote:
> >
> > On Sat, Dec 01, 2018 at 01:23:04AM +0100, Matteo Croce wrote:  
> > > Small improvements to improve the readability and easiness
> > > to use of the xdp1 sample.  
> >
> > Applied to bpf-next.
> >
> > I think that sample code could be more useful if it's wrapped with bash
> > script like selftests/test_xdp* tests do.
> > At that point it can move to selftests to get 0bot coverage.
> > Would you be interested in doing that?
> >  
> 
> It would be nice, but I think that the samples have more urgent issues
> right now.
> Many examples doesn't compile on my system (Fedora 29, GCC 8.2.1, Clang 7.0.0),
> these are the errors that I encounter:
> 
>   HOSTCC  /home/matteo/src/kernel/linux/samples/bpf/test_lru_dist
> /home/matteo/src/kernel/linux/samples/bpf/test_lru_dist.c:39:8: error:
> redefinition of ‘struct list_head’
>  struct list_head {
>         ^~~~~~~~~

I wonder if there is a way to catch that, and print "run make
headers_install, please" rather than confuse newcomers.  Hm..

Like this?

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 35444f4a846b..b1fdea806d4d 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -205,6 +205,15 @@ HOSTCC = $(CROSS_COMPILE)gcc
 CLANG_ARCH_ARGS = -target $(ARCH)
 endif
 
+HDR_PROBE := $(shell echo "\#include <linux/types.h>\n struct list_head { int a; }; int main() { return 0; }" | \
+       gcc $(KBUILD_HOSTCFLAGS) -x c - -o /dev/null 2>/dev/null && \
+       echo okay)
+
+ifeq ($(HDR_PROBE),)
+$(warning Detected possible issues with include path.)
+$(warning Please install kernel headers locally (make headers_install))
+endif
+
 BTF_LLC_PROBE := $(shell $(LLC) -march=bpf -mattr=help 2>&1 | grep dwarfris)
 BTF_PAHOLE_PROBE := $(shell $(BTF_PAHOLE) --help 2>&1 | grep BTF)
 BTF_OBJCOPY_PROBE := $(shell $(LLVM_OBJCOPY) --help 2>&1 | grep -i 'usage.*llvm')