From patchwork Fri Mar 31 14:13:59 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Ahern X-Patchwork-Id: 745660 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3vvk5Q20Zfz9ryr for ; Sat, 1 Apr 2017 01:14:18 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=cumulusnetworks.com header.i=@cumulusnetworks.com header.b="RXA8Al1/"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933197AbdCaOOQ (ORCPT ); Fri, 31 Mar 2017 10:14:16 -0400 Received: from mail-pg0-f42.google.com ([74.125.83.42]:35029 "EHLO mail-pg0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754754AbdCaOON (ORCPT ); Fri, 31 Mar 2017 10:14:13 -0400 Received: by mail-pg0-f42.google.com with SMTP id 81so72528495pgh.2 for ; Fri, 31 Mar 2017 07:14:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cumulusnetworks.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=031onfKQM/v/G6rir2oMbmLBGZfC7lAF1+OQjbPdt9Y=; b=RXA8Al1/tGcP/ieoKee+c6ql04DuFa0TkDKm+3JC859SuzlV1Bgrdqb0M3b50T+7Ok s91xCkdkVBxPcZyDT2DuQB7cjvO4IR6hf5YJBd6Xu8oaziwH+iW5WKUtl0qEpoR3PXAF HE9yrnLH1afv4PrtqWh1cW7vsOEwW0Veye9h4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=031onfKQM/v/G6rir2oMbmLBGZfC7lAF1+OQjbPdt9Y=; b=ZZ4G7zugzijFwKMpHlsofwpnVKoc64teopKdFhYvmxaA+xJPPrdOBl3+Se0IK2eRI/ TGRKpYF4Bg7A2+Go4l9EzcWitMXKCppZhSpCcA+jwODVHe0KJtJCzZrBM6LM1d3jPZWC qtfsRqHTeRYLy2xdEU9leS/21GtQUbXbzGXgS69/nJ3Iv6U+XPVG7q6TDsbUVODQcKQ7 xJdaSgTrmvAehRLPg0UbeWcG6vjU7rTOP71UW1KXBNxXS1ctW+lFZtzjjhllsLYyDixr wi+Bs2AeUDwssQFQlHl78Rjnikq7o6JEbv8ufjW/wQdy95MqVtdjs+4r531lfSkExOre Szmw== X-Gm-Message-State: AFeK/H23cRo5NkUENGB/lUlWfscvO5cdkbPt8fGnusmg6SWEadcKvkE9NEhO0nGhBKER7q9p X-Received: by 10.84.211.130 with SMTP id c2mr3923169pli.82.1490969651716; Fri, 31 Mar 2017 07:14:11 -0700 (PDT) Received: from kenny.it.cumulusnetworks.com. ([216.129.126.126]) by smtp.googlemail.com with ESMTPSA id h14sm3985756pgn.64.2017.03.31.07.14.10 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 31 Mar 2017 07:14:11 -0700 (PDT) From: David Ahern To: netdev@vger.kernel.org Cc: roopa@cumulusnetworks.com, rshearma@brocade.com, ebiederm@xmission.com, David Ahern Subject: [PATCH net-next v3 1/6] net: mpls: rt_nhn_alive and nh_flags should be accessed using READ_ONCE Date: Fri, 31 Mar 2017 07:13:59 -0700 Message-Id: <1490969644-15499-2-git-send-email-dsa@cumulusnetworks.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1490969644-15499-1-git-send-email-dsa@cumulusnetworks.com> References: <1490969644-15499-1-git-send-email-dsa@cumulusnetworks.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org The number of alive nexthops for a route (rt->rt_nhn_alive) and the flags for a next hop (nh->nh_flags) are modified by netdev event handlers. The event handlers run with rtnl_lock held so updates are always done with the lock held. The packet path accesses the fields under the rcu lock. Since those fields can change at any moment in the packet path, both fields should be accessed using READ_ONCE. Updates to both fields should use WRITE_ONCE. Update mpls_select_multipath (packet path) and mpls_ifdown and mpls_ifup (event handlers) accordingly. Signed-off-by: David Ahern --- v3 - no change v2 - new patch in v2 of set net/mpls/af_mpls.c | 36 +++++++++++++++++++++++++----------- net/mpls/internal.h | 8 ++++++++ 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c index 06ffafde70da..6bdd2f95b576 100644 --- a/net/mpls/af_mpls.c +++ b/net/mpls/af_mpls.c @@ -189,10 +189,15 @@ static u32 mpls_multipath_hash(struct mpls_route *rt, struct sk_buff *skb) return hash; } +/* number of alive nexthops (rt->rt_nhn_alive) and the flags for + * a next hop (nh->nh_flags) are modified by netdev event handlers. + * Since those fields can change at any moment, use READ_ONCE to + * access both. + */ static struct mpls_nh *mpls_select_multipath(struct mpls_route *rt, struct sk_buff *skb) { - int alive = ACCESS_ONCE(rt->rt_nhn_alive); + unsigned int alive; u32 hash = 0; int nh_index = 0; int n = 0; @@ -203,7 +208,8 @@ static struct mpls_nh *mpls_select_multipath(struct mpls_route *rt, if (rt->rt_nhn == 1) goto out; - if (alive <= 0) + alive = READ_ONCE(rt->rt_nhn_alive); + if (alive == 0) return NULL; hash = mpls_multipath_hash(rt, skb); @@ -211,7 +217,9 @@ static struct mpls_nh *mpls_select_multipath(struct mpls_route *rt, if (alive == rt->rt_nhn) goto out; for_nexthops(rt) { - if (nh->nh_flags & (RTNH_F_DEAD | RTNH_F_LINKDOWN)) + unsigned int nh_flags = READ_ONCE(nh->nh_flags); + + if (nh_flags & (RTNH_F_DEAD | RTNH_F_LINKDOWN)) continue; if (n == nh_index) return nh; @@ -1302,7 +1310,6 @@ static void mpls_ifdown(struct net_device *dev, int event) { struct mpls_route __rcu **platform_label; struct net *net = dev_net(dev); - unsigned int nh_flags = RTNH_F_DEAD | RTNH_F_LINKDOWN; unsigned int alive, deleted; unsigned index; @@ -1316,22 +1323,27 @@ static void mpls_ifdown(struct net_device *dev, int event) alive = 0; deleted = 0; change_nexthops(rt) { + unsigned int nh_flags = nh->nh_flags; + if (rtnl_dereference(nh->nh_dev) != dev) goto next; switch (event) { case NETDEV_DOWN: case NETDEV_UNREGISTER: - nh->nh_flags |= RTNH_F_DEAD; + nh_flags |= RTNH_F_DEAD; /* fall through */ case NETDEV_CHANGE: - nh->nh_flags |= RTNH_F_LINKDOWN; + nh_flags |= RTNH_F_LINKDOWN; break; } if (event == NETDEV_UNREGISTER) RCU_INIT_POINTER(nh->nh_dev, NULL); + + if (nh->nh_flags != nh_flags) + WRITE_ONCE(nh->nh_flags, nh_flags); next: - if (!(nh->nh_flags & nh_flags)) + if (!(nh_flags & (RTNH_F_DEAD | RTNH_F_LINKDOWN))) alive++; if (!rtnl_dereference(nh->nh_dev)) deleted++; @@ -1345,7 +1357,7 @@ static void mpls_ifdown(struct net_device *dev, int event) } } -static void mpls_ifup(struct net_device *dev, unsigned int nh_flags) +static void mpls_ifup(struct net_device *dev, unsigned int flags) { struct mpls_route __rcu **platform_label; struct net *net = dev_net(dev); @@ -1361,20 +1373,22 @@ static void mpls_ifup(struct net_device *dev, unsigned int nh_flags) alive = 0; change_nexthops(rt) { + unsigned int nh_flags = nh->nh_flags; struct net_device *nh_dev = rtnl_dereference(nh->nh_dev); - if (!(nh->nh_flags & nh_flags)) { + if (!(nh_flags & flags)) { alive++; continue; } if (nh_dev != dev) continue; alive++; - nh->nh_flags &= ~nh_flags; + nh_flags &= ~flags; + WRITE_ONCE(nh->nh_flags, flags); } endfor_nexthops(rt); - ACCESS_ONCE(rt->rt_nhn_alive) = alive; + WRITE_ONCE(rt->rt_nhn_alive, alive); } } diff --git a/net/mpls/internal.h b/net/mpls/internal.h index 62928d8fabd1..91419fe63464 100644 --- a/net/mpls/internal.h +++ b/net/mpls/internal.h @@ -83,6 +83,10 @@ enum mpls_payload_type { struct mpls_nh { /* next hop label forwarding entry */ struct net_device __rcu *nh_dev; + + /* nh_flags is accessed under RCU in the packet path; it is + * modified handling netdev events with rtnl lock held + */ unsigned int nh_flags; u32 nh_label[MAX_NEW_LABELS]; u8 nh_labels; @@ -124,6 +128,10 @@ struct mpls_route { /* next hop label forwarding entry */ u8 rt_max_alen; u8 rt_ttl_propagate; unsigned int rt_nhn; + + /* rt_nhn_alive is accessed under RCU in the packet path; it + * is modified handling netdev events with rtnl lock held + */ unsigned int rt_nhn_alive; struct mpls_nh rt_nh[0]; };