Re: [PATCH] ipv4: do not cache local route when using SO_BINDTODEVICE

[Kouya Shimura]

Thanks for reviewing.

David Miller  writes:
> From: Kouya Shimura 
> Date: Tue, 23 Feb 2016 14:27:32 +0900
>
>> After v3.6, output routes are cached, however, the 'rt_iif' field of
>> struct rtable can not be shared by various traffics using SO_BINDTODEVICE.
>> It causes that traffic can not reach the local receiver which also uses 
>> SO_BINDTODEVICE after another traffic creates a local route cache. 
>> 
>>   /* sender and receiver are running on same host. */
>>   sender:
>> - socket(SOCK_DGRAM)
>> - setsockopt(SO_BINDTODEVICE, "eth0")
>> - connect(INADDR_ANY)
>> - send("message") -->[loopback]--+
>>  |
>>   receiver:  |
>> - socket(SOCK_DGRAM) |
>> - setsockopt(SO_BINDTODEVICE, "eth0")|
>> - bind(INADDR_ANY)   |
>> - recv()  <--+
>>   /* sometimes reach, sometimes not reach!!! */
>> 
>> Before v3.6, above traffics always reached. It seems to be a
>> regression. Actually the 'dhcp_release' command relies on it.
>> 
>> Commit 7bd86cc282a458b66c41e3f6 ("ipv4: Cache local output routes")
>> originated this issue. Revert it conditionally.
>> 
>> Signed-off-by: Kouya Shimura 
>
> I strongly fear that this will cause a performance regression for some 
> important
> cases.  If you elide the cache, it means that every route lookup has
> to allocate,
> create, and destroy the cache object.  This is really expensive.

The performace degradation was on my mind. Thus, I intended that this
patch affects only special (abnormal?) case that destination is local
and using SO_BINDTODEVICE. Isn't it acceptable?

This patch deesn't elide the existing cache. Normal (not using
SO_BINDTODEVICE) routes are still cached (not destoyed). Outer
routes, too. I confirmed it.

If you worry about non-zeroed 'orig_oif' case, this patch certainly
implies a perfomance problem. 'orig_oif' seems to be non-zero in
cases of ICMP, GRE, MULTICAST besides SO_BINDTODEVICE. 

>
> There has to be a better way to do so, so I'm not applying this patch,
> sorry.

Yes, it has to be. The root cause is sharing the 'rt_iif' field.
I'm looking forward to the right fix.

Thanks,
Kouya

Re: [PATCH] ipv4: do not cache local route when using SO_BINDTODEVICE

[David Miller]

From: Kouya Shimura 
Date: Tue, 23 Feb 2016 14:27:32 +0900

> After v3.6, output routes are cached, however, the 'rt_iif' field of
> struct rtable can not be shared by various traffics using SO_BINDTODEVICE.
> It causes that traffic can not reach the local receiver which also uses 
> SO_BINDTODEVICE after another traffic creates a local route cache. 
> 
>   /* sender and receiver are running on same host. */
>   sender:
> - socket(SOCK_DGRAM)
> - setsockopt(SO_BINDTODEVICE, "eth0")
> - connect(INADDR_ANY)
> - send("message") -->[loopback]--+
>  |
>   receiver:  |
> - socket(SOCK_DGRAM) |
> - setsockopt(SO_BINDTODEVICE, "eth0")|
> - bind(INADDR_ANY)   |
> - recv()  <--+
>   /* sometimes reach, sometimes not reach!!! */
> 
> Before v3.6, above traffics always reached. It seems to be a
> regression. Actually the 'dhcp_release' command relies on it.
> 
> Commit 7bd86cc282a458b66c41e3f6 ("ipv4: Cache local output routes")
> originated this issue. Revert it conditionally.
> 
> Signed-off-by: Kouya Shimura 

I strongly fear that this will cause a performance regression for some important
cases.  If you elide the cache, it means that every route lookup has to 
allocate,
create, and destroy the cache object.  This is really expensive.

There has to be a better way to do so, so I'm not applying this patch, sorry.

[PATCH] ipv4: do not cache local route when using SO_BINDTODEVICE

[Kouya Shimura]

After v3.6, output routes are cached, however, the 'rt_iif' field of
struct rtable can not be shared by various traffics using SO_BINDTODEVICE.
It causes that traffic can not reach the local receiver which also uses 
SO_BINDTODEVICE after another traffic creates a local route cache. 

  /* sender and receiver are running on same host. */
  sender:
- socket(SOCK_DGRAM)
- setsockopt(SO_BINDTODEVICE, "eth0")
- connect(INADDR_ANY)
- send("message") -->[loopback]--+
 |
  receiver:  |
- socket(SOCK_DGRAM) |
- setsockopt(SO_BINDTODEVICE, "eth0")|
- bind(INADDR_ANY)   |
- recv()  <--+
  /* sometimes reach, sometimes not reach!!! */

Before v3.6, above traffics always reached. It seems to be a
regression. Actually the 'dhcp_release' command relies on it.

Commit 7bd86cc282a458b66c41e3f6 ("ipv4: Cache local output routes")
originated this issue. Revert it conditionally.

Signed-off-by: Kouya Shimura 
---
 net/ipv4/route.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 02c6229..4b5e024 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2285,6 +2285,8 @@ struct rtable *__ip_route_output_key_hash(struct net 
*net, struct flowi4 *fl4,
}
dev_out = net->loopback_dev;
fl4->flowi4_oif = dev_out->ifindex;
+   if (orig_oif)
+   res.fi = NULL;
flags |= RTCF_LOCAL;
goto make_route;
}
-- 
1.9.1