From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS,USER_AGENT_NEOMUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DA4E5C43381 for ; Wed, 27 Mar 2019 22:52:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 90F7F2070B for ; Wed, 27 Mar 2019 22:52:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="JK99JS5v" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728121AbfC0WwR (ORCPT ); Wed, 27 Mar 2019 18:52:17 -0400 Received: from mail-pl1-f193.google.com ([209.85.214.193]:40361 "EHLO mail-pl1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726275AbfC0WwQ (ORCPT ); Wed, 27 Mar 2019 18:52:16 -0400 Received: by mail-pl1-f193.google.com with SMTP id b11so3989916plr.7 for ; Wed, 27 Mar 2019 15:52:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=zuHW8RGC2RmlRoNUtirOEdWnCy3wD7+TemkQX8QH8rw=; b=JK99JS5vzMmNhdszY3F1uKa/i7AeyoouHC+oEHPEhdwHO50C8T7paW1Z9LEigm9c0I lM5DXyM9g7SPd8Hk5M/bDCIudg2tIfjNAvi6+Sz9d+TXUA46ZaeDZ5gJxYH/JW9C/r8z rX3W2SfRpl5aQBgXJ2tsuGJIhjV2AHIhOlut7x2yfhgrr/uww+34EbR4Ff1LGJFgEu9a jUWjkHrN8Q6fwwjxduIUQ78C5FyPOluTMLQrKaTznOt9bX/RgK6QgLWTLn2zLOOMrUze sYFmFzn2/XyYCFDj2XmFGGIBK8UiIUY9WMx2f1oC2sp1+pn1HVpU0oIs8h/nndRRDG/W HpbQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=zuHW8RGC2RmlRoNUtirOEdWnCy3wD7+TemkQX8QH8rw=; b=QTh4wFektOg89+2w9K8p0jjswyBPz/X1EyBPNWyEOJWjgUWOgrCL9pq2d3Q03sfvgP hJGe6oW7I+C4YmwJz6HqcHcivrbshEmu0ytyGxV2XBRkrNfbXHmJMRF072G4LMcKoFZR W7UUdxC9rfqWlnGWOjGGOTs+PKSaKcOTeRBWAe2wtTif1KJvkxOMnxv1FiCXCFbZyCOJ DAZDyWRSFFVxRAWyYAk9LamHFsvilruWv0VdDAZjnXwChRZW8EEzbc40N/RvlDfgzWK2 oq/TIX857DqHSF8dsG6dO1VrYkHiw1aHfYYlf4imHO8lpLizIPihnlk75KXLYm6IeDwB SKQw== X-Gm-Message-State: APjAAAWK4OmOtGtKlVZPyclWOqUb66gQ2W71/2o/szCMoHguFwg9KWPI fK3NoXsdRuCIiNvJgd55aFY= X-Google-Smtp-Source: APXvYqyyZRnTH422NaJL2FoXtSWBdmDk/0AR9bZ3BKJAzKKYb1UZg/K98J0FwCQ6qiR22v2hRj4adg== X-Received: by 2002:a17:902:9a83:: with SMTP id w3mr39507659plp.137.1553727135685; Wed, 27 Mar 2019 15:52:15 -0700 (PDT) Received: from ast-mbp ([2620:10d:c090:180::e17b]) by smtp.gmail.com with ESMTPSA id p34sm36767455pgb.18.2019.03.27.15.52.13 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 27 Mar 2019 15:52:14 -0700 (PDT) Date: Wed, 27 Mar 2019 15:52:12 -0700 From: Alexei Starovoitov To: David Ahern Cc: davem@davemloft.net, netdev@vger.kernel.org, idosch@mellanox.com, jiri@mellanox.com, saeedm@mellanox.com, David Ahern Subject: Re: [PATCH v2 net-next 05/13] ipv6: Create init helper for fib6_nh Message-ID: <20190327225210.kbpivej33a72adwq@ast-mbp> References: <20190327182329.18149-1-dsahern@kernel.org> <20190327182329.18149-6-dsahern@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190327182329.18149-6-dsahern@kernel.org> User-Agent: NeoMutt/20180223 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed, Mar 27, 2019 at 11:23:21AM -0700, David Ahern wrote: > From: David Ahern > > Similar to IPv4, consolidate the fib6_nh initialization into a helper. > > Signed-off-by: David Ahern > --- > include/net/ip6_fib.h | 4 + > net/ipv6/route.c | 235 +++++++++++++++++++++++++++----------------------- > 2 files changed, 131 insertions(+), 108 deletions(-) > > diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h > index 2acb78a762ee..ce1f81345c8e 100644 > --- a/include/net/ip6_fib.h > +++ b/include/net/ip6_fib.h > @@ -444,6 +444,10 @@ static inline struct net_device *fib6_info_nh_dev(const struct fib6_info *f6i) > return f6i->fib6_nh.nh_dev; > } > > +int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh, > + struct fib6_config *cfg, gfp_t gfp_flags, > + struct netlink_ext_ack *extack); > + > static inline > struct lwtunnel_state *fib6_info_nh_lwt(const struct fib6_info *f6i) > { > diff --git a/net/ipv6/route.c b/net/ipv6/route.c > index b804be3cbf05..e577d2d51b5f 100644 > --- a/net/ipv6/route.c > +++ b/net/ipv6/route.c > @@ -2896,16 +2896,133 @@ static int ip6_validate_gw(struct net *net, struct fib6_config *cfg, > return err; > } > > +int fib6_nh_init(struct net *net, struct fib6_nh *fib6_nh, > + struct fib6_config *cfg, gfp_t gfp_flags, > + struct netlink_ext_ack *extack) > +{ > + struct net_device *dev = NULL; > + struct inet6_dev *idev = NULL; > + int addr_type; > + int err; > + > + err = -ENODEV; > + if (cfg->fc_ifindex) { > + dev = dev_get_by_index(net, cfg->fc_ifindex); > + if (!dev) > + goto out; > + idev = in6_dev_get(dev); > + if (!idev) > + goto out; > + } > + > + if (cfg->fc_flags & RTNH_F_ONLINK) { > + if (!dev) { > + NL_SET_ERR_MSG(extack, > + "Nexthop device required for onlink"); > + goto out; > + } > + > + if (!(dev->flags & IFF_UP)) { > + NL_SET_ERR_MSG(extack, "Nexthop device is not up"); > + err = -ENETDOWN; > + goto out; > + } > + > + fib6_nh->nh_flags |= RTNH_F_ONLINK; > + } > + > + if (cfg->fc_encap) { > + struct lwtunnel_state *lwtstate; > + > + err = lwtunnel_build_state(cfg->fc_encap_type, > + cfg->fc_encap, AF_INET6, cfg, > + &lwtstate, extack); > + if (err) > + goto out; > + > + fib6_nh->nh_lwtstate = lwtstate_get(lwtstate); > + } > + > + fib6_nh->nh_weight = 1; > + > + /* We cannot add true routes via loopback here, > + * they would result in kernel looping; promote them to reject routes > + */ > + addr_type = ipv6_addr_type(&cfg->fc_dst); > + if ((cfg->fc_flags & RTF_REJECT) || > + (dev && (dev->flags & IFF_LOOPBACK) && > + !(addr_type & IPV6_ADDR_LOOPBACK) && > + !(cfg->fc_flags & RTF_LOCAL))) { > + /* hold loopback dev/idev if we haven't done so. */ > + if (dev != net->loopback_dev) { > + if (dev) { > + dev_put(dev); > + in6_dev_put(idev); > + } > + dev = net->loopback_dev; > + dev_hold(dev); > + idev = in6_dev_get(dev); > + if (!idev) { > + err = -ENODEV; > + goto out; > + } > + } > + cfg->fc_flags = RTF_REJECT | RTF_NONEXTHOP; imo it would be cleaner not to mess with cfg. Ideally it should be marked 'const'. > + goto set_dev; > + } > + > + if (cfg->fc_flags & RTF_GATEWAY) { > + err = ip6_validate_gw(net, cfg, &dev, &idev, extack); > + if (err) > + goto out; > + > + fib6_nh->nh_gw = cfg->fc_gateway; > + } > + > + err = -ENODEV; > + if (!dev) > + goto out; > + > + if (idev->cnf.disable_ipv6) { > + NL_SET_ERR_MSG(extack, "IPv6 is disabled on nexthop device"); > + err = -EACCES; > + goto out; > + } > + > + if (!(dev->flags & IFF_UP) && !cfg->fc_ignore_dev_down) { > + NL_SET_ERR_MSG(extack, "Nexthop device is not up"); > + err = -ENETDOWN; > + goto out; > + } > + > + if (!(cfg->fc_flags & (RTF_LOCAL | RTF_ANYCAST)) && > + !netif_carrier_ok(dev)) > + fib6_nh->nh_flags |= RTNH_F_LINKDOWN; > + > +set_dev: > + fib6_nh->nh_dev = dev; > + err = 0; > +out: > + if (idev) > + in6_dev_put(idev); > + > + if (err) { > + lwtstate_put(fib6_nh->nh_lwtstate); lwtstate_put() is missing in the error path of existing code. Is this a bug fix? Why there is nothing about this in commit log? > + fib6_nh->nh_lwtstate = NULL; > + if (dev) > + dev_put(dev); > + } > + > + return err; > +} > + > static struct fib6_info *ip6_route_info_create(struct fib6_config *cfg, > gfp_t gfp_flags, > struct netlink_ext_ack *extack) > { > struct net *net = cfg->fc_nlinfo.nl_net; > struct fib6_info *rt = NULL; > - struct net_device *dev = NULL; > - struct inet6_dev *idev = NULL; > struct fib6_table *table; > - int addr_type; > int err = -EINVAL; > > /* RTF_PCPU is an internal flag; can not be set by userspace */ > @@ -2940,30 +3057,6 @@ static struct fib6_info *ip6_route_info_create(struct fib6_config *cfg, > goto out; > } > #endif > - if (cfg->fc_ifindex) { > - err = -ENODEV; > - dev = dev_get_by_index(net, cfg->fc_ifindex); > - if (!dev) > - goto out; > - idev = in6_dev_get(dev); > - if (!idev) > - goto out; > - } > - > - if (cfg->fc_flags & RTNH_F_ONLINK) { > - if (!dev) { > - NL_SET_ERR_MSG(extack, > - "Nexthop device required for onlink"); > - err = -ENODEV; > - goto out; > - } > - > - if (!(dev->flags & IFF_UP)) { > - NL_SET_ERR_MSG(extack, "Nexthop device is not up"); > - err = -ENETDOWN; > - goto out; > - } > - } > > err = -ENOBUFS; > if (cfg->fc_nlinfo.nlh && > @@ -3007,18 +3100,9 @@ static struct fib6_info *ip6_route_info_create(struct fib6_config *cfg, > cfg->fc_protocol = RTPROT_BOOT; > rt->fib6_protocol = cfg->fc_protocol; > > - addr_type = ipv6_addr_type(&cfg->fc_dst); > - > - if (cfg->fc_encap) { > - struct lwtunnel_state *lwtstate; > - > - err = lwtunnel_build_state(cfg->fc_encap_type, > - cfg->fc_encap, AF_INET6, cfg, > - &lwtstate, extack); > - if (err) > - goto out; > - rt->fib6_nh.nh_lwtstate = lwtstate_get(lwtstate); > - } > + rt->fib6_table = table; > + rt->fib6_metric = cfg->fc_metric; > + rt->fib6_type = cfg->fc_type; > > ipv6_addr_prefix(&rt->fib6_dst.addr, &cfg->fc_dst, cfg->fc_dst_len); > rt->fib6_dst.plen = cfg->fc_dst_len; > @@ -3029,62 +3113,13 @@ static struct fib6_info *ip6_route_info_create(struct fib6_config *cfg, > ipv6_addr_prefix(&rt->fib6_src.addr, &cfg->fc_src, cfg->fc_src_len); > rt->fib6_src.plen = cfg->fc_src_len; > #endif > - > - rt->fib6_metric = cfg->fc_metric; > - rt->fib6_nh.nh_weight = 1; > - > - rt->fib6_type = cfg->fc_type; > - > - /* We cannot add true routes via loopback here, > - they would result in kernel looping; promote them to reject routes > - */ > - if ((cfg->fc_flags & RTF_REJECT) || > - (dev && (dev->flags & IFF_LOOPBACK) && > - !(addr_type & IPV6_ADDR_LOOPBACK) && > - !(cfg->fc_flags & RTF_LOCAL))) { > - /* hold loopback dev/idev if we haven't done so. */ > - if (dev != net->loopback_dev) { > - if (dev) { > - dev_put(dev); > - in6_dev_put(idev); > - } > - dev = net->loopback_dev; > - dev_hold(dev); > - idev = in6_dev_get(dev); > - if (!idev) { > - err = -ENODEV; > - goto out; > - } > - } > - rt->fib6_flags = RTF_REJECT|RTF_NONEXTHOP; > - goto install_route; > - } > - > - if (cfg->fc_flags & RTF_GATEWAY) { > - err = ip6_validate_gw(net, cfg, &dev, &idev, extack); > - if (err) > - goto out; > - > - rt->fib6_nh.nh_gw = cfg->fc_gateway; > - } > - > - err = -ENODEV; > - if (!dev) > - goto out; > - > - if (idev->cnf.disable_ipv6) { > - NL_SET_ERR_MSG(extack, "IPv6 is disabled on nexthop device"); > - err = -EACCES; > - goto out; > - } > - > - if (!(dev->flags & IFF_UP) && !cfg->fc_ignore_dev_down) { > - NL_SET_ERR_MSG(extack, "Nexthop device is not up"); > - err = -ENETDOWN; > + err = fib6_nh_init(net, &rt->fib6_nh, cfg, gfp_flags, extack); > + if (err) > goto out; > - } > > if (!ipv6_addr_any(&cfg->fc_prefsrc)) { > + struct net_device *dev = fib6_info_nh_dev(rt); > + > if (!ipv6_chk_addr(net, &cfg->fc_prefsrc, dev, 0)) { > NL_SET_ERR_MSG(extack, "Invalid source address"); > err = -EINVAL; > @@ -3097,24 +3132,8 @@ static struct fib6_info *ip6_route_info_create(struct fib6_config *cfg, > > rt->fib6_flags = cfg->fc_flags; > > -install_route: > - if (!(rt->fib6_flags & (RTF_LOCAL | RTF_ANYCAST)) && > - !netif_carrier_ok(dev)) > - rt->fib6_nh.nh_flags |= RTNH_F_LINKDOWN; > - rt->fib6_nh.nh_flags |= (cfg->fc_flags & RTNH_F_ONLINK); > - rt->fib6_nh.nh_dev = dev; > - rt->fib6_table = table; > - > - if (idev) > - in6_dev_put(idev); > - > return rt; > out: > - if (dev) > - dev_put(dev); > - if (idev) > - in6_dev_put(idev); > - > fib6_info_release(rt); > return ERR_PTR(err); > } > -- > 2.11.0 >