lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <56020F74.8000605@gmail.com>
Date:	Tue, 22 Sep 2015 19:33:24 -0700
From:	Alexander Duyck <alexander.duyck@...il.com>
To:	David Ahern <dsa@...ulusnetworks.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 4/9] net: Move rth handling from
 ip_route_input_slow to helper

On 09/22/2015 03:55 PM, David Ahern wrote:
> Move the rth lookup and allocation from ip_route_input_slow into a
> helper function. Code move only; no operational change intended.
>
> Signed-off-by: David Ahern <dsa@...ulusnetworks.com>
> ---
>   net/ipv4/route.c | 104 ++++++++++++++++++++++++++++++++-----------------------
>   1 file changed, 60 insertions(+), 44 deletions(-)
>
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index e3b18cc1952f..79c4cecdb7a1 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1667,6 +1667,63 @@ static int ip_mkroute_input(struct sk_buff *skb,
>   	return __mkroute_input(skb, res, in_dev, daddr, saddr, tos);
>   }
>
> +static int ip_route_local_input(struct sk_buff *skb,
> +				struct fib_result *res,
> +				struct net_device *loopback_dev,
> +				unsigned int flags,
> +				u32 itag,
> +				int rth_err,
> +				bool nopolicy)

Why pass loopback_dev instead of just passing the net pointer?  Seems 
like in the path below there are cases where you end up not needing to 
dereference it and it might be worth the effort to avoid dereferencing 
it if you don't need to.

Same thing for nopolicy.  Why not just pass the in_dev pointer and 
handle getting the policy if you need it.

> +{
> +	bool do_cache = false;
> +	struct rtable *rth;
> +	int err = 0;
> +
> +	if (res->fi) {
> +		if (!itag) {
> +			rth = rcu_dereference(FIB_RES_NH(*res).nh_rth_input);
> +			if (rt_cache_valid(rth)) {
> +				skb_dst_set_noref(skb, &rth->dst);
> +				goto out;
> +			}
> +			do_cache = true;
> +		}
> +	}
> +
> +	rth = rt_dst_alloc(loopback_dev, flags | RTCF_LOCAL, res->type,
> +			   nopolicy, false, do_cache);
> +	if (!rth) {
> +		err = -ENOBUFS;
> +		goto out;
> +	}
> +
> +	rth->dst.output = ip_rt_bug;
> +#ifdef CONFIG_IP_ROUTE_CLASSID
> +	rth->dst.tclassid = itag;
> +#endif
> +	rth->rt_is_input = 1;
> +	if (res->table)
> +		rth->rt_table_id = res->table->tb_id;
> +
> +	RT_CACHE_STAT_INC(in_slow_tot);
> +	if (res->type == RTN_UNREACHABLE) {
> +		rth->dst.input = ip_error;
> +		rth->dst.error = -rth_err;
> +		rth->rt_flags &= ~RTCF_LOCAL;
> +	}
> +
> +	if (do_cache) {
> +		if (unlikely(!rt_cache_route(&FIB_RES_NH(*res), rth))) {
> +			rth->dst.flags |= DST_NOCACHE;
> +			rt_add_uncached_list(rth);
> +		}
> +	}
> +	skb_dst_set(skb, &rth->dst);
> +
> +out:
> +	return err;
> +}
> +
>   /*
>    *	NOTE. We drop all the packets that has local source
>    *	addresses, because every properly looped back packet
> @@ -1687,10 +1744,8 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
>   	struct flowi4	fl4;
>   	unsigned int	flags = 0;
>   	u32		itag = 0;
> -	struct rtable	*rth;
>   	int		err = -EINVAL;
>   	struct net    *net = dev_net(dev);
> -	bool do_cache;
>
>   	/* IP on this device is disabled. */
>
> @@ -1790,48 +1845,9 @@ out:	return err;
>   	RT_CACHE_STAT_INC(in_brd);
>
>   local_input:
> -	do_cache = false;
> -	if (res.fi) {
> -		if (!itag) {
> -			rth = rcu_dereference(FIB_RES_NH(res).nh_rth_input);
> -			if (rt_cache_valid(rth)) {
> -				skb_dst_set_noref(skb, &rth->dst);
> -				err = 0;
> -				goto out;
> -			}
> -			do_cache = true;
> -		}
> -	}
> -
> -	rth = rt_dst_alloc(net->loopback_dev, flags | RTCF_LOCAL, res.type,
> -			   IN_DEV_CONF_GET(in_dev, NOPOLICY), false, do_cache);
> -	if (!rth) {
> -		err = -ENOBUFS;
> -		goto out;
> -	}
> -
> -	rth->dst.output= ip_rt_bug;
> -#ifdef CONFIG_IP_ROUTE_CLASSID
> -	rth->dst.tclassid = itag;
> -#endif
> -	rth->rt_is_input = 1;
> -	if (res.table)
> -		rth->rt_table_id = res.table->tb_id;
> -
> -	RT_CACHE_STAT_INC(in_slow_tot);
> -	if (res.type == RTN_UNREACHABLE) {
> -		rth->dst.input= ip_error;
> -		rth->dst.error= -err;
> -		rth->rt_flags 	&= ~RTCF_LOCAL;
> -	}
> -	if (do_cache) {
> -		if (unlikely(!rt_cache_route(&FIB_RES_NH(res), rth))) {
> -			rth->dst.flags |= DST_NOCACHE;
> -			rt_add_uncached_list(rth);
> -		}
> -	}
> -	skb_dst_set(skb, &rth->dst);
> -	err = 0;
> +	err = ip_route_local_input(skb, &res, net->loopback_dev,
> +				   flags, itag, err,
> +				   IN_DEV_CONF_GET(in_dev, NOPOLICY));
>   	goto out;
>
>   no_route:
>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ