All of lore.kernel.org
 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 5.8 13/56] ipv4: Update exception handling for multipath routes via same device
Date: Fri, 25 Sep 2020 14:48:03 +0200	[thread overview]
Message-ID: <20200925124729.815379998@linuxfoundation.org> (raw)
In-Reply-To: <20200925124727.878494124@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 |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -786,8 +786,10 @@ static void __ip_do_redirect(struct rtab
 			neigh_event_send(n, NULL);
 		} else {
 			if (fib_lookup(net, fl4, &res, 0) == 0) {
-				struct fib_nh_common *nhc = FIB_RES_NHC(res);
+				struct fib_nh_common *nhc;
 
+				fib_select_path(net, &res, fl4, skb);
+				nhc = FIB_RES_NHC(res);
 				update_or_create_fnhe(nhc, fl4->daddr, new_gw,
 						0, false,
 						jiffies + ip_rt_gc_timeout);
@@ -1013,6 +1015,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;
@@ -1033,9 +1036,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_common *nhc = FIB_RES_NHC(res);
+	if (fib_lookup(net, fl4, &res, 0) == 0) {
+		struct fib_nh_common *nhc;
 
+		fib_select_path(net, &res, fl4, NULL);
+		nhc = FIB_RES_NHC(res);
 		update_or_create_fnhe(nhc, fl4->daddr, 0, mtu, lock,
 				      jiffies + ip_rt_mtu_expires);
 	}
@@ -2663,8 +2668,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:58 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-25 12:47 [PATCH 5.8 00/56] 5.8.12-rc1 review Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 01/56] ibmvnic fix NULL tx_pools and rx_tools issue at do_reset Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 02/56] ibmvnic: add missing parenthesis in do_reset() Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 03/56] act_ife: load meta modules before tcf_idr_check_alloc() Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 04/56] bnxt_en: Avoid sending firmware messages when AER error is detected Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 05/56] bnxt_en: Fix NULL ptr dereference crash in bnxt_fw_reset_task() Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 06/56] cxgb4: fix memory leak during module unload Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 07/56] cxgb4: Fix offset when clearing filter byte counters Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 08/56] geneve: add transport ports in route lookup for geneve Greg Kroah-Hartman
2020-09-25 12:47 ` [PATCH 5.8 09/56] hdlc_ppp: add range checks in ppp_cp_parse_cr() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 10/56] hinic: bump up the timeout of SET_FUNC_STATE cmd Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 11/56] ip: fix tos reflection in ack and reset packets Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 12/56] ipv4: Initialize flowi4_multipath_hash in data path Greg Kroah-Hartman
2020-09-25 12:48 ` Greg Kroah-Hartman [this message]
2020-09-25 12:48 ` [PATCH 5.8 14/56] ipv6: avoid lockdep issue in fib6_del() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 15/56] net: bridge: br_vlan_get_pvid_rcu() should dereference the VLAN group under RCU Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 16/56] net: DCB: Validate DCB_ATTR_DCB_BUFFER argument Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 17/56] net: dsa: rtl8366: Properly clear member config Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 18/56] net: Fix bridge enslavement failure Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 19/56] net: ipv6: fix kconfig dependency warning for IPV6_SEG6_HMAC Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 20/56] net/mlx5: Fix FTE cleanup Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 21/56] net: phy: call phy_disable_interrupts() in phy_attach_direct() instead Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 22/56] net: sched: initialize with 0 before setting erspan md->u Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 23/56] net: sch_generic: aviod concurrent reset and enqueue op for lockless qdisc Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 24/56] net: sctp: Fix IPv6 ancestor_size calc in sctp_copy_descendant Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 25/56] nfp: use correct define to return NONE fec Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 26/56] taprio: Fix allowing too small intervals Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 27/56] tipc: Fix memory leak in tipc_group_create_member() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 28/56] tipc: fix shutdown() of connection oriented socket Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 29/56] tipc: use skb_unshare() instead in tipc_buf_append() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 30/56] net/mlx5e: Enable adding peer miss rules only if merged eswitch is supported Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 31/56] net/mlx5e: TLS, Do not expose FPGA TLS counter if not supported Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 32/56] bnxt_en: Use memcpy to copy VPD field info Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 33/56] bnxt_en: return proper error codes in bnxt_show_temp Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 34/56] bnxt_en: Protect bnxt_set_eee() and bnxt_set_pauseparam() with mutex Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 35/56] net: lantiq: Wake TX queue again Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 36/56] net: lantiq: use netif_tx_napi_add() for TX NAPI Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 37/56] net: lantiq: Use napi_complete_done() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 38/56] net: lantiq: Disable IRQs only if NAPI gets scheduled Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 39/56] net: phy: Avoid NPD upon phy_detach() when driver is unbound Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 40/56] net: phy: Do not warn in phy_stop() on PHY_DOWN Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 41/56] net: qrtr: check skb_put_padto() return value Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 42/56] net: add __must_check to skb_put_padto() Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 43/56] net: ethernet: ti: cpsw_new: fix suspend/resume Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 44/56] wireguard: noise: take lock when removing handshake entry from table Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 45/56] wireguard: peerlookup: take lock before checking hash in replace operation Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 46/56] net: ipa: fix u32_replace_bits by u32p_xxx version Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 47/56] net/mlx5e: Fix memory leak of tunnel info when rule under multipath not ready Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 48/56] hinic: fix rewaking txq after netif_tx_disable Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 49/56] hv_netvsc: Fix hibernation for mlx5 VF driver Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 50/56] net: dsa: link interfaces with the DSA master to get rid of lockdep warnings Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 51/56] net: dsa: microchip: ksz8795: really set the correct number of ports Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 52/56] net: macb: fix for pause frame receive enable bit Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 53/56] Revert "netns: dont disable BHs when locking "nsid_lock"" Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 54/56] net/mlx5e: Use RCU to protect rq->xdp_prog Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 55/56] net/mlx5e: Use synchronize_rcu to sync with NAPI Greg Kroah-Hartman
2020-09-25 12:48 ` [PATCH 5.8 56/56] net/mlx5e: Fix endianness when calculating pedit mask first bit Greg Kroah-Hartman
2020-09-25 18:01 ` [PATCH 5.8 00/56] 5.8.12-rc1 review Jon Hunter
2020-09-26 16:09   ` Greg Kroah-Hartman
2020-09-25 18:32 ` Naresh Kamboju
2020-09-26 16:10   ` Greg Kroah-Hartman
2020-09-25 20:01 ` Shuah Khan
2020-09-26 16:10   ` Greg Kroah-Hartman
2020-09-25 21:22 ` Jeffrin Jose T
2020-09-26 16:10   ` Greg Kroah-Hartman
2020-09-26 15:44 ` Guenter Roeck
2020-09-26 16:10   ` Greg Kroah-Hartman

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=20200925124729.815379998@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.