linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Kfir Itzhak <mastertheknife@gmail.com>,
	David Ahern <dsahern@kernel.org>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 4.19 23/37] ipv4: Update exception handling for multipath routes via same device
Date: Fri, 25 Sep 2020 14:48:51 +0200	[thread overview]
Message-ID: <20200925124724.448531559@linuxfoundation.org> (raw)
In-Reply-To: <20200925124720.972208530@linuxfoundation.org>

From: David Ahern <dsahern@kernel.org>

[ Upstream commit 2fbc6e89b2f1403189e624cabaf73e189c5e50c6 ]

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: 4895c771c7f0 ("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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/ipv4/route.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -779,6 +779,8 @@ static void __ip_do_redirect(struct rtab
 			if (fib_lookup(net, fl4, &res, 0) == 0) {
 				struct fib_nh *nh = &FIB_RES_NH(res);
 
+				fib_select_path(net, &res, fl4, skb);
+				nh = &FIB_RES_NH(res);
 				update_or_create_fnhe(nh, fl4->daddr, new_gw,
 						0, false,
 						jiffies + ip_rt_gc_timeout);
@@ -1004,6 +1006,7 @@ out:	kfree_skb(skb);
 static void __ip_rt_update_pmtu(struct rtable *rt, struct flowi4 *fl4, u32 mtu)
 {
 	struct dst_entry *dst = &rt->dst;
+	struct net *net = dev_net(dst->dev);
 	u32 old_mtu = ipv4_mtu(dst);
 	struct fib_result res;
 	bool lock = false;
@@ -1024,9 +1027,11 @@ static void __ip_rt_update_pmtu(struct r
 		return;
 
 	rcu_read_lock();
-	if (fib_lookup(dev_net(dst->dev), fl4, &res, 0) == 0) {
-		struct fib_nh *nh = &FIB_RES_NH(res);
+	if (fib_lookup(net, fl4, &res, 0) == 0) {
+		struct fib_nh *nh;
 
+		fib_select_path(net, &res, fl4, NULL);
+		nh = &FIB_RES_NH(res);
 		update_or_create_fnhe(nh, fl4->daddr, 0, mtu, lock,
 				      jiffies + ip_rt_mtu_expires);
 	}
@@ -2536,8 +2541,6 @@ struct rtable *ip_route_output_key_hash_
 	fib_select_path(net, res, fl4, skb);
 
 	dev_out = FIB_RES_DEV(*res);
-	fl4->flowi4_oif = dev_out->ifindex;
-
 
 make_route:
 	rth = __mkroute_output(res, fl4, orig_oif, dev_out, flags);



  parent reply	other threads:[~2020-09-25 12:54 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-25 12:48 [PATCH 4.19 00/37] 4.19.148-rc1 review Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 01/37] af_key: pfkey_dump needs parameter validation Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 02/37] KVM: fix memory leak in kvm_io_bus_unregister_dev() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 03/37] kprobes: fix kill kprobe which has been marked as gone Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 04/37] mm/thp: fix __split_huge_pmd_locked() for migration PMD Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 05/37] cxgb4: Fix offset when clearing filter byte counters Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 06/37] geneve: add transport ports in route lookup for geneve Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 07/37] hdlc_ppp: add range checks in ppp_cp_parse_cr() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 08/37] ip: fix tos reflection in ack and reset packets Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 09/37] ipv6: avoid lockdep issue in fib6_del() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 10/37] net: DCB: Validate DCB_ATTR_DCB_BUFFER argument Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 11/37] net: dsa: rtl8366: Properly clear member config Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 12/37] net: ipv6: fix kconfig dependency warning for IPV6_SEG6_HMAC Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 13/37] net: sch_generic: aviod concurrent reset and enqueue op for lockless qdisc Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 14/37] nfp: use correct define to return NONE fec Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 15/37] tipc: Fix memory leak in tipc_group_create_member() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 16/37] tipc: fix shutdown() of connection oriented socket Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 17/37] tipc: use skb_unshare() instead in tipc_buf_append() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 18/37] bnxt_en: return proper error codes in bnxt_show_temp Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 19/37] bnxt_en: Protect bnxt_set_eee() and bnxt_set_pauseparam() with mutex Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 20/37] net: phy: Avoid NPD upon phy_detach() when driver is unbound Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 21/37] net: qrtr: check skb_put_padto() return value Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 22/37] net: add __must_check to skb_put_padto() Greg Kroah-Hartman
2020-09-25 12:48 ` Greg Kroah-Hartman [this message]
2020-09-25 16:51   ` [PATCH 4.19 23/37] ipv4: Update exception handling for multipath routes via same device Pavel Machek
2020-09-26 15:46     ` Greg Kroah-Hartman
2020-09-29 12:26       ` Pavel Machek
2020-09-25 12:48 ` [PATCH 4.19 24/37] MAINTAINERS: add CLANG/LLVM BUILD SUPPORT info Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 25/37] kbuild: add OBJSIZE variable for the size tool Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 26/37] Documentation/llvm: add documentation on building w/ Clang/LLVM Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 27/37] Documentation/llvm: fix the name of llvm-size Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 28/37] net: wan: wanxl: use allow to pass CROSS_COMPILE_M68k for rebuilding firmware Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 29/37] net: wan: wanxl: use $(M68KCC) instead of $(M68KAS) " Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 30/37] x86/boot: kbuild: allow readelf executable to be specified Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 4.19 31/37] kbuild: remove AS variable Greg Kroah-Hartman
2020-09-25 12:49 ` [PATCH 4.19 32/37] kbuild: replace AS=clang with LLVM_IAS=1 Greg Kroah-Hartman
2020-09-25 12:49 ` [PATCH 4.19 33/37] kbuild: support LLVM=1 to switch the default tools to Clang/LLVM Greg Kroah-Hartman
2020-09-25 12:49 ` [PATCH 4.19 34/37] mm: memcg: fix memcg reclaim soft lockup Greg Kroah-Hartman
2020-09-25 12:49 ` [PATCH 4.19 35/37] tcp_bbr: refactor bbr_target_cwnd() for general inflight provisioning Greg Kroah-Hartman
2020-09-25 12:49 ` [PATCH 4.19 36/37] tcp_bbr: adapt cwnd based on ack aggregation estimation Greg Kroah-Hartman
2020-09-25 12:49 ` [PATCH 4.19 37/37] serial: 8250: Avoid error message on reprobe Greg Kroah-Hartman
2020-09-25 17:39 ` [PATCH 4.19 00/37] 4.19.148-rc1 review Pavel Machek
2020-09-26 15:50   ` Greg Kroah-Hartman
2020-09-25 20:02 ` Shuah Khan
2020-09-26 12:35 ` Naresh Kamboju
2020-09-26 15:42 ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200925124724.448531559@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mastertheknife@gmail.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).