ipv4: add a fib_type to fib_info
authorEric Dumazet <edumazet@google.com>
Thu, 4 Oct 2012 01:25:26 +0000 (01:25 +0000)
committerDavid S. Miller <davem@davemloft.net>
Thu, 4 Oct 2012 17:58:26 +0000 (13:58 -0400)
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.

David suggested to add fib_type to fib_info so that we dont
inadvertently share same fib_info for different purposes.

With help from Julian Anastasov who provided very helpful
hints, reproduced here :

<quote>
        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.

</quote>

Many thanks to Chris Clayton for his patience and help.

Reported-by: Chris Clayton <chris2553@googlemail.com>
Bisected-by: Chris Clayton <chris2553@googlemail.com>
Reported-by: Dave Jones <davej@redhat.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Julian Anastasov <ja@ssi.bg>
Tested-by: Chris Clayton <chris2553@googlemail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
include/net/ip_fib.h
net/ipv4/fib_semantics.c

index 926142ed8d7aa67a55e560242c94c903611a8372..9497be1ad4c0c5cdf4fe2df49b79fa5f66a255f8 100644 (file)
@@ -102,6 +102,7 @@ struct fib_info {
        unsigned char           fib_dead;
        unsigned char           fib_protocol;
        unsigned char           fib_scope;
+       unsigned char           fib_type;
        __be32                  fib_prefsrc;
        u32                     fib_priority;
        u32                     *fib_metrics;
index 3509065e409ab2782fe23cc8a174e369f0da501d..267753060ffc5c7efe6a3e183113d8dc3fbb49e6 100644 (file)
@@ -314,6 +314,7 @@ static struct fib_info *fib_find_info(const struct fib_info *nfi)
                    nfi->fib_scope == fi->fib_scope &&
                    nfi->fib_prefsrc == fi->fib_prefsrc &&
                    nfi->fib_priority == fi->fib_priority &&
+                   nfi->fib_type == fi->fib_type &&
                    memcmp(nfi->fib_metrics, fi->fib_metrics,
                           sizeof(u32) * RTAX_MAX) == 0 &&
                    ((nfi->fib_flags ^ fi->fib_flags) & ~RTNH_F_DEAD) == 0 &&
@@ -833,6 +834,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
        fi->fib_flags = cfg->fc_flags;
        fi->fib_priority = cfg->fc_priority;
        fi->fib_prefsrc = cfg->fc_prefsrc;
+       fi->fib_type = cfg->fc_type;
 
        fi->fib_nhs = nhs;
        change_nexthops(fi) {