| Message ID | 160200013701.719143.12665708317930272219.stgit@firesoul |
|---|---|
| Headers | show |
| Series | bpf: New approach for BPF MTU handling and enforcement | expand |
On Tue, Oct 6, 2020 at 6:19 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Tue, 6 Oct 2020 18:33:02 +0200 Jesper Dangaard Brouer wrote: > > > +static const struct bpf_func_proto bpf_xdp_mtu_lookup_proto = { > > > + .func = bpf_xdp_mtu_lookup, > > > + .gpl_only = true, > > > + .ret_type = RET_INTEGER, > > > + .arg1_type = ARG_PTR_TO_CTX, > > > + .arg2_type = ARG_ANYTHING, > > > + .arg3_type = ARG_ANYTHING, > > > +}; > > > + > > > + > > FWIW > > CHECK: Please don't use multiple blank lines > #112: FILE: net/core/filter.c:5566: FYI: It would be nice to have a similar function to return a device's L2 header size (ie. 14 for ethernet) and/or hwtype. Also, should this be restricted to gpl only? [I'm not actually sure, I'm actually fed up with non-gpl code atm, and wouldn't be against all bpf code needing to be gpl'ed...]
On Tue, 6 Oct 2020 18:24:28 -0700 Maciej Żenczykowski <maze@google.com> wrote: > On Tue, Oct 6, 2020 at 6:19 PM Jakub Kicinski <kuba@kernel.org> wrote: > > > > On Tue, 6 Oct 2020 18:33:02 +0200 Jesper Dangaard Brouer wrote: > > > > +static const struct bpf_func_proto bpf_xdp_mtu_lookup_proto = { > > > > + .func = bpf_xdp_mtu_lookup, > > > > + .gpl_only = true, > > > > + .ret_type = RET_INTEGER, > > > > + .arg1_type = ARG_PTR_TO_CTX, > > > > + .arg2_type = ARG_ANYTHING, > > > > + .arg3_type = ARG_ANYTHING, > > > > +}; > > > > + > > > > + > > > > FWIW > > > > CHECK: Please don't use multiple blank lines > > #112: FILE: net/core/filter.c:5566: > > FYI: It would be nice to have a similar function to return a device's > L2 header size (ie. 14 for ethernet) and/or hwtype. > > Also, should this be restricted to gpl only? That is mostly because I copy-pasted it from the fib lookup helper, which with good reason is GPL. I would prefer it to be GPL, but given how simple it is I shouldn't. Maybe I could argue that my new mtu_check could be GPL as it does more work. > [I'm not actually sure, I'm actually fed up with non-gpl code atm, and > wouldn't be against all bpf code needing to be gpl'ed...] -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
On 10/6/20 6:24 PM, Maciej Żenczykowski wrote: > > FYI: It would be nice to have a similar function to return a device's > L2 header size (ie. 14 for ethernet) and/or hwtype. Why does that need to be looked up via a helper? It's a static number for a device and can plumbed to a program in a number of ways.
> > FYI: It would be nice to have a similar function to return a device's > > L2 header size (ie. 14 for ethernet) and/or hwtype. > > Why does that need to be looked up via a helper? It's a static number > for a device and can plumbed to a program in a number of ways. Fair enough.
This patchset drops all the MTU checks in TC BPF-helpers that limits growing the packet size. This is done because these BPF-helpers doesn't take redirect into account, which can result in their MTU check being done against the wrong netdev. The new approach is to give BPF-programs knowledge about the MTU on a netdev (via ifindex) and fib route lookup level. Meaning some BPF-helpers are added and extended to make it possible to do MTU checks in the BPF-code. If BPF-prog doesn't comply with the MTU this is enforced on the kernel side. Realizing MTU should only apply to transmitted packets, the MTU enforcement is now done after the TC egress hook. This gives TC-BPF programs most flexibility and allows to shrink packet size again in egress hook prior to transmit. This patchset is primarily focused on TC-BPF, but I've made sure that the MTU BPF-helpers also works for XDP BPF-programs. --- Jesper Dangaard Brouer (6): bpf: Remove MTU check in __bpf_skb_max_len bpf: bpf_fib_lookup return MTU value as output when looked up bpf: add BPF-helper for reading MTU from net_device via ifindex bpf: make it possible to identify BPF redirected SKBs bpf: Add MTU check for TC-BPF packets after egress hook bpf: drop MTU check when doing TC-BPF redirect to ingress include/linux/netdevice.h | 5 ++- include/uapi/linux/bpf.h | 24 +++++++++++- net/core/dev.c | 24 +++++++++++- net/core/filter.c | 88 ++++++++++++++++++++++++++++++++++++++++----- net/sched/Kconfig | 1 + 5 files changed, 126 insertions(+), 16 deletions(-) --