On Sat, Jul 13, 2019 at 3:42 PM David Ahern <dsah...@gmail.com> wrote: > > On 7/12/19 2:17 PM, Cong Wang wrote: > > diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c > > index 317339cd7f03..8662a44a28f9 100644 > > --- a/net/ipv4/fib_frontend.c > > +++ b/net/ipv4/fib_frontend.c > > @@ -388,6 +388,12 @@ static int __fib_validate_source(struct sk_buff *skb, > > __be32 src, __be32 dst, > > fib_combine_itag(itag, &res); > > > > dev_match = fib_info_nh_uses_dev(res.fi, dev); > > + /* This is rare, loopback packets retain skb_dst so normally they > > + * would not even hit this slow path. > > + */ > > + dev_match = dev_match || (res.type == RTN_LOCAL && > > + dev == net->loopback_dev && > > The dev should not be needed. res.type == RTN_LOCAL should be enough, no? > > > + IN_DEV_ACCEPT_LOCAL(idev)); > > Why is this check needed? Can you give an example use that is fixed -
I am not sure if I should have this check either, my initial version didn't have it either, later I add it because I find out it is checked for rp_filter=0 case too. On the other hand, loopback always accepts local traffic, so it may be redundant to check it. So, I am not sure. What do you think? > and add one to selftests/net/fib_tests.sh? It's complicated, Mesos network isolation uses this case: https://cgit.twitter.biz/mesos/tree/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp Even if I use a simplified case, it still has to use TC filters and mirred action to redirect the packet, which I am not sure they fit in fib_tests.sh. Thanks.
