On 10/30/16 at 01:34pm, Tom Herbert wrote: > On Sun, Oct 30, 2016 at 4:58 AM, Thomas Graf <tg...@suug.ch> wrote: > > + if (unlikely(!dst->lwtstate->orig_output)) { > > + WARN_ONCE(1, "orig_output not set on dst for prog %s\n", > > + bpf->out.name); > > + kfree_skb(skb); > > + return -EINVAL; > > + } > > + > > + return dst->lwtstate->orig_output(net, sk, skb); > > The BPF program may have changed the destination address so continuing > with original route in skb may not be appropriate here. This was fixed > in ila_lwt by calling ip6_route_output and we were able to dst cache > facility to cache the route to avoid cost of looking it up on every > packet. Since the kernel has no insight into what the BPF program > does to the packet I'd suggest 1) checking if destination address > changed by BPF and if it did then call route_output to get new route > 2) If the LWT destination is a host route then try to keep a dst > cache. This would entail checking destination address on return that > it is the same one as kept in the dst cache.
Instead of building complex logic, we can allow the program to return a code to indicate when to perform another route lookup just as we do for the redirect case. Just because the destination address has changed may not require another lookup in all cases. A typical example would be a program rewriting addresses for the default route to other address which are always handled by the default route as well. An unconditional lookup would hurt performance in many cases.