netdev - Re: [PATCH 2/6] net: pad skb data and shinfo as a whole rather than individually

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1325785607.4759.12.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Date:	Thu, 05 Jan 2012 18:46:47 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Ian Campbell <ian.campbell@...rix.com>
Cc:	netdev@...r.kernel.org, David Miller <davem@...emloft.net>
Subject: Re: [PATCH 2/6] net: pad skb data and shinfo as a whole rather than
 individually

Le jeudi 05 janvier 2012 à 17:13 +0000, Ian Campbell a écrit :
> We already push the shinfo to the very end of the actual allocation which is
> already cache-line aligned.
> 
> This reduces the minimum overhead required for this allocation such that the
> shinfo can be grown in the following patch without overflowing 2048 bytes for a
> 1500 byte frame. Reducing this overhead means that sometimes the tail end of
> the data can end up in the same cache line as the beginning of the shinfo but
> in many cases the allocation slop means that there is no overlap.
> 
> Signed-off-by: Ian Campbell <ian.campbell@...rix.com>
> ---
>  include/linux/skbuff.h |    4 ++--
>  net/core/skbuff.c      |    3 +--
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 50db9b0..f04333b 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -41,7 +41,7 @@
>  #define SKB_DATA_ALIGN(X)	(((X) + (SMP_CACHE_BYTES - 1)) & \
>  				 ~(SMP_CACHE_BYTES - 1))
>  #define SKB_WITH_OVERHEAD(X)	\
> -	((X) - SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
> +	((X) - sizeof(struct skb_shared_info))
>  #define SKB_MAX_ORDER(X, ORDER) \
>  	SKB_WITH_OVERHEAD((PAGE_SIZE << (ORDER)) - (X))
>  #define SKB_MAX_HEAD(X)		(SKB_MAX_ORDER((X), 0))
> @@ -50,7 +50,7 @@
>  /* return minimum truesize of one skb containing X bytes of data */
>  #define SKB_TRUESIZE(X) ((X) +						\
>  			 SKB_DATA_ALIGN(sizeof(struct sk_buff)) +	\
> -			 SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
> +			 sizeof(struct skb_shared_info))
>  
>  /* A. Checksumming of received packets by device.
>   *
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index da0c97f..b6de604 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -189,8 +189,7 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
>  	 * aligned memory blocks, unless SLUB/SLAB debug is enabled.
>  	 * Both skb->head and skb_shared_info are cache line aligned.
>  	 */
> -	size = SKB_DATA_ALIGN(size);
> -	size += SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> +	size = SKB_DATA_ALIGN(size + sizeof(struct skb_shared_info));
>  	data = kmalloc_node_track_caller(size, gfp_mask, node);
>  	if (!data)
>  		goto nodata;

Unfortunately this makes the frequently use part of skb_shared_info not
any more in a single cache line.

This will slow down many operations like kfree_skb() of cold skbs (TX
completion for example)




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