On Wed, 2012-10-03 at 02:24 +0300, Julian Anastasov wrote:

	Hello,

On Tue, 2 Oct 2012, Eric Dumazet wrote:

quoted

quoted

David, shouldnt we use a nh_rth_forward instead of a nh_rth_input in
__mkroute_input() ?

(And change rt_cache_route() as well ?)

I am testing a patch right now.

Yeah, this patch seems to fix the bug for me.

[PATCH] ipv4: properly cache forward routes

commit d2d68ba9fe8 (ipv4: Cache input routes in fib_info nexthops.)
introduced a regression for forwarding.

This was hard to reproduce but the symptom was that packets were
delivered to local host instead of being forwarded.

Add a separate cache (nh_rth_forward) to solve the problem.

	Can it be a problem related to fib_info reuse
from different routes. For example, when local IP address
is created for subnet we have:

broadcast 192.168.0.255 dev DEV  proto kernel  scope link  src 192.168.0.1
192.168.0.0/24 dev DEV  proto kernel  scope link  src 192.168.0.1
local 192.168.0.1 dev DEV  proto kernel  scope host  src 192.168.0.1

	The "dev DEV  proto kernel  scope link  src 192.168.0.1" is
a reused fib_info structure where we put cached routes.
The result can be same fib_info for 192.168.0.255 and
192.168.0.0/24. RTN_BROADCAST is cached only for input
routes. Incoming broadcast to 192.168.0.255 can be cached
and can cause problems for traffic forwarded to 192.168.0.0/24.
So, this patch should solve the problem because it
separates the broadcast from unicast traffic.

	And the ip_route_input_slow caching will work for
local and broadcast input routes (above routes 1 and 3) just
because they differ in scope and use different fib_info.

	Another possible failure is for output routes:

multicast 224.0.0.0/4 fib_info
with unicast
192.168.0.0/24 fib_info

	The multicast sets RTCF_MULTICAST | RTCF_LOCAL
and can cause problems for generated unicast traffic on
fib_info reuse. Depends on the scope, for multicast it is
usually scope global, so may be it is difficult to happen
in practice.

	__mkroute_output works for local/unicast routes
because they differ in scope.

Thanks Julian for these informations.

BTW, it seems we dont properly increase UDP MIB counters when a
multicast message is not delivered to at least one socket.

Lets fix this to ease future bug hunting.

I hate when "netstat -s" is useless and we have to use dropwatch to
figure out where we drop a frame.

[PATCH] udp: increment UDP_MIB_NOPORTS in multicast receive

We should increment UDP_MIB_NOPORTS in the case we found
no socket to deliver a copy of one incoming UDP message.

(RFC 4113 udpNoPorts)

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/udp.c |    1 +
 net/ipv6/udp.c |    1 +
 2 files changed, 2 insertions(+)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 79c8dbe..dfa73c5 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c

@@ -1591,6 +1591,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
 			sock_put(stack[i]);
 	} else {
 		kfree_skb(skb);
+		UDP_INC_STATS_BH(net, UDP_MIB_NOPORTS, udptable != &udp_table);
 	}
 	return 0;
 }

diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index fc99972..0be9ac2 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c

@@ -748,6 +748,7 @@ static int __udp6_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
 			sock_put(stack[i]);
 	} else {
 		kfree_skb(skb);
+		UDP6_INC_STATS_BH(net, UDP_MIB_NOPORTS, udptable != &udp_table);
 	}
 	return 0;
 }