netdev - Re: [RFC PATCH] net: Implement read-only protection and COW'ing of metrics.

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: <1292529359.2655.2.camel@edumazet-laptop>
Date:	Thu, 16 Dec 2010 20:55:59 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	David Miller <davem@...emloft.net>
Cc:	netdev@...r.kernel.org
Subject: Re: [RFC PATCH] net: Implement read-only protection and COW'ing of
 metrics.

Le mercredi 15 décembre 2010 à 13:21 -0800, David Miller a écrit :
> Routing metrics are now copy-on-write.
> 
> Initially a route entry points it's metrics at a read-only location.
> If a routing table entry exists, it will point there.  Else it will
> point at the all zero metric place- holder 'dst_default_metrics'.
> 
> The writeability state of the metrics is stored in the low bits of the
> metrics pointer, we have two bits left to spare if we want to store
> more states.
> 
> For the initial implementation, COW is implemented simply via kmalloc.
> However future enhancements will change this to place the writable
> metrics somewhere else, in order to increase sharing.  Very likely
> this "somewhere else" will be the inetpeer cache.
> 
> Note also that this means that metrics updates may transiently fail
> if we cannot COW the metrics successfully.
> 
> But even by itself, this patch should decrease memory usage and
> increase cache locality especially for routing workloads.  In those
> cases the read-only metric copies stay in place and never get written
> to.
> 
> TCP workloads where metrics get updated, and those rare cases where
> PMTU triggers occur, will take a very slight performance hit.  But
> that hit will be alleviated when the long-term writable metrics
> move to a more sharable location.
> 
> Since the metrics storage went from a u32 array of RTAX_MAX entries to
> what is essentially a pointer, some retooling of the dst_entry layout
> was necessary.
> 
> Most importantly, we need to preserve the alignment of the reference
> count so that it doesn't share cache lines with the read-mostly state,
> as per Eric Dumazet's alignment assertion checks.
> 
> The only non-trivial bit here is the move of the 'flags' member into
> the writeable cacheline.  This is OK since we are always accessing the
> flags around the same moment when we made a modification to the
> reference count.
> 
> Signed-off-by: David S. Miller <davem@...emloft.net>
> ---

Hi David

>  
> 
> @@ -1840,7 +1843,7 @@ static void rt_set_nexthop(struct rtable *rt, struct fib_result *res, u32 itag)
>  		if (FIB_RES_GW(*res) &&
>  		    FIB_RES_NH(*res).nh_scope == RT_SCOPE_LINK)
>  			rt->rt_gateway = FIB_RES_GW(*res);
> -		dst_import_metrics(dst, fi->fib_metrics);
> +		dst_attach_metrics(dst, fi->fib_metrics, true);

I am a bit concerned about this part.

What prevents fi->fib_metrics to disappear,  if fib is destroyed, since
we dont take a reference ?



--
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