Skip to content

Commit 2fbc6e8

Browse files
dsaherndavem330
authored andcommitted
ipv4: Update exception handling for multipath routes via same device
Kfir reported that pmtu exceptions are not created properly for deployments where multipath routes use the same device. After some digging I see 2 compounding problems: 1. ip_route_output_key_hash_rcu is updating the flowi4_oif *after* the route lookup. This is the second use case where this has been a problem (the first is related to use of vti devices with VRF). I can not find any reason for the oif to be changed after the lookup; the code goes back to the start of git. It does not seem logical so remove it. 2. fib_lookups for exceptions do not call fib_select_path to handle multipath route selection based on the hash. The end result is that the fib_lookup used to add the exception always creates it based using the first leg of the route. An example topology showing the problem: | host1 +------+ | eth0 | .209 +------+ | +------+ switch | br0 | +------+ | +---------+---------+ | host2 | host3 +------+ +------+ | eth0 | .250 | eth0 | 192.168.252.252 +------+ +------+ +-----+ +-----+ | vti | .2 | vti | 192.168.247.3 +-----+ +-----+ \ / ================================= tunnels 192.168.247.1/24 for h in host1 host2 host3; do ip netns add ${h} ip -netns ${h} link set lo up ip netns exec ${h} sysctl -wq net.ipv4.ip_forward=1 done ip netns add switch ip -netns switch li set lo up ip -netns switch link add br0 type bridge stp 0 ip -netns switch link set br0 up for n in 1 2 3; do ip -netns switch link add eth-sw type veth peer name eth-h${n} ip -netns switch li set eth-h${n} master br0 up ip -netns switch li set eth-sw netns host${n} name eth0 done ip -netns host1 addr add 192.168.252.209/24 dev eth0 ip -netns host1 link set dev eth0 up ip -netns host1 route add 192.168.247.0/24 \ nexthop via 192.168.252.250 dev eth0 nexthop via 192.168.252.252 dev eth0 ip -netns host2 addr add 192.168.252.250/24 dev eth0 ip -netns host2 link set dev eth0 up ip -netns host2 addr add 192.168.252.252/24 dev eth0 ip -netns host3 link set dev eth0 up ip netns add tunnel ip -netns tunnel li set lo up ip -netns tunnel li add br0 type bridge ip -netns tunnel li set br0 up for n in $(seq 11 20); do ip -netns tunnel addr add dev br0 192.168.247.${n}/24 done for n in 2 3 do ip -netns tunnel link add vti${n} type veth peer name eth${n} ip -netns tunnel link set eth${n} mtu 1360 master br0 up ip -netns tunnel link set vti${n} netns host${n} mtu 1360 up ip -netns host${n} addr add dev vti${n} 192.168.247.${n}/24 done ip -netns tunnel ro add default nexthop via 192.168.247.2 nexthop via 192.168.247.3 ip netns exec host1 ping -M do -s 1400 -c3 -I 192.168.252.209 192.168.247.11 ip netns exec host1 ping -M do -s 1400 -c3 -I 192.168.252.209 192.168.247.15 ip -netns host1 ro ls cache Before this patch the cache always shows exceptions against the first leg in the multipath route; 192.168.252.250 per this example. Since the hash has an initial random seed, you may need to vary the final octet more than what is listed. In my tests, using addresses between 11 and 19 usually found 1 that used both legs. With this patch, the cache will have exceptions for both legs. Fixes: 4895c77 ("ipv4: Add FIB nexthop exceptions") Reported-by: Kfir Itzhak <mastertheknife@gmail.com> Signed-off-by: David Ahern <dsahern@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 2e5117b commit 2fbc6e8

File tree

1 file changed

+8
-5
lines changed

1 file changed

+8
-5
lines changed

net/ipv4/route.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -786,8 +786,10 @@ static void __ip_do_redirect(struct rtable *rt, struct sk_buff *skb, struct flow
786786
neigh_event_send(n, NULL);
787787
} else {
788788
if (fib_lookup(net, fl4, &res, 0) == 0) {
789-
struct fib_nh_common *nhc = FIB_RES_NHC(res);
789+
struct fib_nh_common *nhc;
790790

791+
fib_select_path(net, &res, fl4, skb);
792+
nhc = FIB_RES_NHC(res);
791793
update_or_create_fnhe(nhc, fl4->daddr, new_gw,
792794
0, false,
793795
jiffies + ip_rt_gc_timeout);
@@ -1013,6 +1015,7 @@ out: kfree_skb(skb);
10131015
static void __ip_rt_update_pmtu(struct rtable *rt, struct flowi4 *fl4, u32 mtu)
10141016
{
10151017
struct dst_entry *dst = &rt->dst;
1018+
struct net *net = dev_net(dst->dev);
10161019
u32 old_mtu = ipv4_mtu(dst);
10171020
struct fib_result res;
10181021
bool lock = false;
@@ -1033,9 +1036,11 @@ static void __ip_rt_update_pmtu(struct rtable *rt, struct flowi4 *fl4, u32 mtu)
10331036
return;
10341037

10351038
rcu_read_lock();
1036-
if (fib_lookup(dev_net(dst->dev), fl4, &res, 0) == 0) {
1037-
struct fib_nh_common *nhc = FIB_RES_NHC(res);
1039+
if (fib_lookup(net, fl4, &res, 0) == 0) {
1040+
struct fib_nh_common *nhc;
10381041

1042+
fib_select_path(net, &res, fl4, NULL);
1043+
nhc = FIB_RES_NHC(res);
10391044
update_or_create_fnhe(nhc, fl4->daddr, 0, mtu, lock,
10401045
jiffies + ip_rt_mtu_expires);
10411046
}
@@ -2668,8 +2673,6 @@ struct rtable *ip_route_output_key_hash_rcu(struct net *net, struct flowi4 *fl4,
26682673
fib_select_path(net, res, fl4, skb);
26692674

26702675
dev_out = FIB_RES_DEV(*res);
2671-
fl4->flowi4_oif = dev_out->ifindex;
2672-
26732676

26742677
make_route:
26752678
rth = __mkroute_output(res, fl4, orig_oif, dev_out, flags);

0 commit comments

Comments
 (0)