On Sat, 19 Sep 2020 at 00:06, Maciej Żenczykowski <maze@xxxxxxxxxx> wrote: > > > This is a good point. As bpf_skb_adjust_room() can just be run after > > bpf_redirect() call, then a MTU check in bpf_redirect() actually > > doesn't make much sense. As clever/bad BPF program can then avoid the > > MTU check anyhow. This basically means that we have to do the MTU > > check (again) on kernel side anyhow to catch such clever/bad BPF > > programs. (And I don't like wasting cycles on doing the same check two > > times). > > If you get rid of the check in bpf_redirect() you might as well get > rid of *all* the checks for excessive mtu in all the helpers that > adjust packet size one way or another way. They *all* then become > useless overhead. > > I don't like that. There may be something the bpf program could do to > react to the error condition (for example in my case, not modify > things and just let the core stack deal with things - which will > probably just generate packet too big icmp error). > > btw. right now our forwarding programs first adjust the packet size > then call bpf_redirect() and almost immediately return what it > returned. > > but this could I think easily be changed to reverse the ordering, so > we wouldn't increase packet size before the core stack was informed we > would be forwarding via a different interface. We do the same, except that we also use XDP_TX when appropriate. This complicates the matter, because there is no helper call we could return an error from. My preference would be to have three helpers: get MTU for a device, redirect ctx to a device (with MTU check), resize ctx (without MTU check) but that doesn't work with XDP_TX. Your idea of doing checks in redirect and adjust_room is pragmatic and seems easier to implement. -- Lorenz Bauer | Systems Engineer 6th Floor, County Hall/The Riverside Building, SE1 7PB, UK www.cloudflare.com
