From patchwork Tue Oct 27 04:52:04 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andy Gospodarek X-Patchwork-Id: 536441 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 5B374141319 for ; Tue, 27 Oct 2015 15:52:16 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=cumulusnetworks.com header.i=@cumulusnetworks.com header.b=Y43bFirW; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751867AbbJ0EwK (ORCPT ); Tue, 27 Oct 2015 00:52:10 -0400 Received: from mail-qg0-f43.google.com ([209.85.192.43]:34394 "EHLO mail-qg0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750920AbbJ0EwI (ORCPT ); Tue, 27 Oct 2015 00:52:08 -0400 Received: by qgem9 with SMTP id m9so137710030qge.1 for ; Mon, 26 Oct 2015 21:52:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cumulusnetworks.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=/YIjkoo7sYHcTsD81ecW70x0eo6IV3kofym9oxv8kCs=; b=Y43bFirWdmqN1k7lj9qnuDklEwR4lh1oUhSaub2c7MBKAMtMlTPIBC+SB3kuqtQaIZ aEPEWJSO+3LdJyAJ6Gq4ExftXC7mOQ1KRRszQypZrgJUiuByqR+OPshjJfwYmgT1Lz/m DaQ6J8Dd0R1Txuc48cP0WIrFNPAozjPaqahp8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=/YIjkoo7sYHcTsD81ecW70x0eo6IV3kofym9oxv8kCs=; b=PnyBMag0wkEV+D5nCYfRstxYzz27BNG0r6yFoB6XqaZv1lo9KrQ0YU2iXZKNLW4rAl AAeA+jyxP4M98WIMaYYNqyJXJynh/WsSy/Wtapf5X47QJorePsi+quT0gL1Pi1XL91GT vlTkB559yjhZdRRbFd05vdvMSQ41mKiAMjKAsoJp/NapVMqgMNBblhSZR6D1+WUuYP2n 7GeJooP/ICkIvvfdnKY1Z7XuzQMnFnbzT7iCohgo1K1TSnitz/5pp/n7EiyGlHZLefkp hMExjKWmW4CHaCIncTwR8uq2u793eec4YjpmjPGEawjW6ZqH7l+GB1E4rljg8zLomH/u OiZQ== X-Gm-Message-State: ALoCoQn2864oPOYYiRwr1D8Ks4h8Lu5R3RsZtwehC6LxST4G+nQy/Xe0GZoSkxXGWlUkpDX8dvF6 X-Received: by 10.140.25.139 with SMTP id 11mr46804005qgt.73.1445921527700; Mon, 26 Oct 2015 21:52:07 -0700 (PDT) Received: from gospo.home.greyhouse.net (cpe-65-190-8-248.nc.res.rr.com. [65.190.8.248]) by smtp.gmail.com with ESMTPSA id f66sm14488219qkf.33.2015.10.26.21.52.06 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 26 Oct 2015 21:52:07 -0700 (PDT) Date: Tue, 27 Oct 2015 00:52:04 -0400 From: Andy Gospodarek To: Julian Anastasov Cc: David Miller , netdev@vger.kernel.org Subject: Re: [PATCHv2 net 2/2] ipv4: update RTNH_F_LINKDOWN flag on UP event Message-ID: <20151027045203.GG3567@gospo.home.greyhouse.net> References: <1445896753-14464-1-git-send-email-ja@ssi.bg> <1445896753-14464-3-git-send-email-ja@ssi.bg> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1445896753-14464-3-git-send-email-ja@ssi.bg> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Mon, Oct 26, 2015 at 11:59:13PM +0200, Julian Anastasov wrote: > When nexthop is part of multipath route we should clear the > LINKDOWN flag when link goes UP or when first address is added. > This is needed because we always set LINKDOWN flag when DEAD flag > was set but now on UP the nexthop is not dead anymore. Examples when > LINKDOWN bit can be forgotten when no NETDEV_CHANGE is delivered: > > - link goes down (LINKDOWN is set), then link goes UP and device > shows carrier OK but LINKDOWN remains set > > - last address is deleted (LINKDOWN is set), then address is > added and device shows carrier OK but LINKDOWN remains set > > Steps to reproduce: > modprobe dummy > ifconfig dummy0 192.168.168.1 up > > here add a multipath route where one nexthop is for dummy0: > > ip route add 1.2.3.4 nexthop dummy0 nexthop SOME_OTHER_DEVICE > ifconfig dummy0 down > ifconfig dummy0 up > > now ip route shows nexthop that is not dead. Now set the sysctl var: > > echo 1 > /proc/sys/net/ipv4/conf/dummy0/ignore_routes_with_linkdown > > now ip route will show a dead nexthop because the forgotten > RTNH_F_LINKDOWN is propagated as RTNH_F_DEAD. I tested this patch and I now see that your reported problem is a result of dummy never taking carrier down. There was a presumption that carrier notification would go down when hardware went down (or when the logical device backing the hardware went down, but this is clearly not always the case. > Fixes: 8a3d03166f19 ("net: track link-status of ipv4 nexthops") > Signed-off-by: Julian Anastasov > --- > net/ipv4/fib_semantics.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c > index f493eff..f657418 100644 > --- a/net/ipv4/fib_semantics.c > +++ b/net/ipv4/fib_semantics.c > @@ -1445,6 +1445,13 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags) > if (!(dev->flags & IFF_UP)) > return 0; > > + if (nh_flags & RTNH_F_DEAD) { > + unsigned int flags = dev_get_flags(dev); > + > + if (flags & (IFF_RUNNING | IFF_LOWER_UP)) > + nh_flags |= RTNH_F_LINKDOWN; > + } > + > prev_fi = NULL; > hash = fib_devindex_hashfn(dev->ifindex); > head = &fib_info_devhash[hash]; Logically this patch makes sense, but I feel as though there may be a slightly better option. Possibly this: --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c index 42778d9..7eb7c40 100644 --- a/net/ipv4/fib_semantics.c +++ b/net/ipv4/fib_semantics.c @@ -1376,7 +1376,8 @@ int fib_sync_down_dev(struct net_device *dev, unsigned long event) nexthop_nh->nh_flags |= RTNH_F_DEAD; /* fall through */ case NETDEV_CHANGE: - nexthop_nh->nh_flags |= RTNH_F_LINKDOWN; + if (!netif_carrier_ok(dev)) + nexthop_nh->nh_flags |= RTNH_F_LINKDOWN; break; } dead++; @@ -1396,7 +1397,8 @@ int fib_sync_down_dev(struct net_device *dev, unsigned long event) fi->fib_flags |= RTNH_F_DEAD; /* fall through */ case NETDEV_CHANGE: - fi->fib_flags |= RTNH_F_LINKDOWN; + if (!netif_carrier_ok(dev)) + fi->fib_flags |= RTNH_F_LINKDOWN; break; } ret++;