Re: [PATCH net-next 05/13] ipv6: Create init helper for fib6_nh

[Ido Schimmel <idosch@mellanox.com>]

On Tue, Mar 26, 2019 at 08:29:34PM -0700, David Ahern wrote:
> +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;

Same comment as previous patch. Earlier in the function you take a
reference on the LWT object, but don't drop it in error path and instead
rely on the error path of ip6_route_info_create() to take care of that.

> +			}
> +		}
> +		cfg->fc_flags = RTF_REJECT | RTF_NONEXTHOP;
> +		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 && dev)
> +		dev_put(dev);
> +
> +	return err;
> +}