Message-ID: <CAF=yD-+SQJzuw7c+TjP19m1EUcf812CDqjrQFbw5zMxdZ9baiw@mail.gmail.com>
Date:   Mon, 24 Sep 2018 17:14:28 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Petar Penkov <ppenkov@...gle.com>
Cc:     ys114321@...il.com,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Petar Penkov <peterpenkov96@...il.com>,
        Network Development <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        simon.horman@...ronome.com, ecree@...arflare.com,
        songliubraving@...com, Tom Herbert <tom@...bertland.com>
Subject: Re: [bpf-next, v4 0/5] Introduce eBPF flow dissector

On Fri, Sep 14, 2018 at 5:51 PM Petar Penkov <ppenkov@...gle.com> wrote:
>
> On Fri, Sep 14, 2018 at 2:47 PM, Y Song <ys114321@...il.com> wrote:
> > On Fri, Sep 14, 2018 at 12:24 PM Alexei Starovoitov
> > <alexei.starovoitov@...il.com> wrote:
> >>
> >> On Fri, Sep 14, 2018 at 07:46:17AM -0700, Petar Penkov wrote:
> >> > From: Petar Penkov <ppenkov@...gle.com>
> >> >
> >> > This patch series hardens the RX stack by allowing flow dissection in BPF,
> >> > as previously discussed [1]. Because of the rigorous checks of the BPF
> >> > verifier, this provides significant security guarantees. In particular, the
> >> > BPF flow dissector cannot get inside of an infinite loop, as with
> >> > CVE-2013-4348, because BPF programs are guaranteed to terminate. It cannot
> >> > read outside of packet bounds, because all memory accesses are checked.
> >> > Also, with BPF the administrator can decide which protocols to support,
> >> > reducing potential attack surface. Rarely encountered protocols can be
> >> > excluded from dissection and the program can be updated without kernel
> >> > recompile or reboot if a bug is discovered.
> >> >
> >> > Patch 1 adds infrastructure to execute a BPF program in __skb_flow_dissect.
> >> > This includes a new BPF program and attach type.
> >> >
> >> > Patch 2 adds the new BPF flow dissector definitions to tools/uapi.
> >> >
> >> > Patch 3 adds support for the new BPF program type to libbpf and bpftool.
> >> >
> >> > Patch 4 adds a flow dissector program in BPF. This parses most protocols in
> >> > __skb_flow_dissect in BPF for a subset of flow keys (basic, control, ports,
> >> > and address types).
> >> >
> >> > Patch 5 adds a selftest that attaches the BPF program to the flow dissector
> >> > and sends traffic with different levels of encapsulation.
> >> >
> >> > Performance Evaluation:
> >> > The in-kernel implementation was compared against the demo program from
> >> > patch 4 using the test in patch 5 with IPv4/UDP traffic over 10 seconds.
> >> >       $perf record -a -C 4 taskset -c 4 ./test_flow_dissector -i 4 -f 8 \
> >> >               -t 10
> >>
> >> Looks great. Applied to bpf-next with one extra patch:
> >>  SEC("dissect")
> >> -int dissect(struct __sk_buff *skb)
> >> +int _dissect(struct __sk_buff *skb)
> >>
> >> otherwise the test doesn't build.
> >> I'm not sure how it builds for you. Which llvm did you use?
> >
> > This is a known issue. IIRC, llvm <= 4 should be okay and llvm >= 5 would fail.
> >
> I was running a much older version of llvm so I imagine this was the
> issue. Thanks for the fix!
> >>
> >> Also above command works and ipv4 test in ./test_flow_dissector.sh
> >> is passing as well, but it still fails at the end for me:
> >> ./test_flow_dissector.sh
> >> bpffs not mounted. Mounting...
> >> 0: IP
> >> 1: IPV6
> >> 2: IPV6OP
> >> 3: IPV6FR
> >> 4: MPLS
> >> 5: VLAN
> >> Testing IPv4...
> >> inner.dest4: 127.0.0.1
> >> inner.source4: 127.0.0.3
> >> pkts: tx=10 rx=10
> >> inner.dest4: 127.0.0.1
> >> inner.source4: 127.0.0.3
> >> pkts: tx=10 rx=0
> >> inner.dest4: 127.0.0.1
> >> inner.source4: 127.0.0.3
> >> pkts: tx=10 rx=10
> >> Testing IPIP...
> >> tunnels before test:
> >> tunl0: any/ip remote any local any ttl inherit nopmtudisc
> >> sit_test_LV5N: any/ip remote 127.0.0.2 local 127.0.0.1 dev lo ttl inherit
> >> ipip_test_LV5N: any/ip remote 127.0.0.2 local 127.0.0.1 dev lo ttl inherit
> >> sit0: ipv6/ip remote any local any ttl 64 nopmtudisc
> >> gre_test_LV5N: gre/ip remote 127.0.0.2 local 127.0.0.1 dev lo ttl inherit
> >> gre0: gre/ip remote any local any ttl inherit nopmtudisc
> >> inner.dest4: 192.168.0.1
> >> inner.source4: 1.1.1.1
> >> encap proto:   4
> >> outer.dest4: 127.0.0.1
> >> outer.source4: 127.0.0.2
> >> pkts: tx=10 rx=0
> >> tunnels after test:
> >> tunl0: any/ip remote any local any ttl inherit nopmtudisc
> >> sit0: ipv6/ip remote any local any ttl 64 nopmtudisc
> >> gre0: gre/ip remote any local any ttl inherit nopmtudisc
> >> selftests: test_flow_dissector [FAILED]
> >>
> >> is it something in my setup or test is broken?
> >>
> I just reran the test and it is passing. We will investigate what
> could be causing the issue.

I've tried, but I am still not able to reproduce this exact issue.

This may be due to how we split the compilation and execution

With

  make defconfig &&
  make kvmconfig &&
  make kselftest-merge &&
  make -j $N bzImage

and starting the result in qemu -kernel I miss a few built-ins. The test
complains loudly if sch_ingress or ipip are missing. After updating to
=y, the tests all pass for me.

The confounding part is that in the above output the test shows no
error output and all tunnels are setup correctly.

For debugging purposes, I can update the script to run all tests instead
of existing on the first failure and to output more state, including the
address on the device and the state of the netns.

Powered by blists - more mailing lists