netdev - Re: [PATCH net-next 2/3] ipv4: rate limit updating of next hop exceptions with same pmtu

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: <alpine.LFD.2.00.1305281127340.1693@ja.ssi.bg>
Date:	Tue, 28 May 2013 11:45:51 +0300 (EEST)
From:	Julian Anastasov <ja@....bg>
To:	Timo Teräs <timo.teras@....fi>
cc:	netdev@...r.kernel.org
Subject: Re: [PATCH net-next 2/3] ipv4: rate limit updating of next hop
 exceptions with same pmtu


	Hello,

On Tue, 28 May 2013, Timo Teräs wrote:

> The tunnel devices call update_pmtu for each packet sent, this causes
> contention on the fnhe_lock. Ignore the pmtu update if pmtu is not
> actually changed, and there is still plenty of time before the entry
> expires.
> 
> Signed-off-by: Timo Teräs <timo.teras@....fi>
> ---
>  net/ipv4/route.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 561a378..a4082be 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -947,6 +947,10 @@ static void __ip_rt_update_pmtu(struct rtable *rt, struct flowi4 *fl4, u32 mtu)
>  	if (mtu < ip_rt_min_pmtu)
>  		mtu = ip_rt_min_pmtu;
>  
> +	if (rt->rt_pmtu == mtu &&
> +	    time_before(jiffies, dst->expires - ip_rt_mtu_expires / 2))
> +		return;
> +

	Can we also add logic in this patch in
update_or_create_fnhe, so that we avoid invalidation for cached
routes when only pmtu expiration is updated (same pmtu), i.e.:

+	if (gw || pmtu != fnhe->fnhe_pmtu) {
+		/* Exception created; mark the cached routes for the nexthop
+		...
+	}

	BTW, I now see that previous patch should
call for_each_possible_cpu for the both cases, not
only when fnhe is created but also on update:

	bool invalidate;

	if (fnhe) {
		invalidate = gw || pmtu != fnhe->fnhe_pmtu;
		...
	} else {
		...
		invalidate = true;
	}

	if (invalidate) {
		/* Exception created; mark the cached routes for the nexthop
		...
	}

Regards

--
Julian Anastasov <ja@....bg>