netdev - Re: [PATCH net-next] bnx2x: reduce skb truesize by 50%

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: <1320942423.307.10.camel@lb-tlvb-eilong.il.broadcom.com>
Date:	Thu, 10 Nov 2011 18:27:03 +0200
From:	"Eilon Greenstein" <eilong@...adcom.com>
To:	"Eric Dumazet" <eric.dumazet@...il.com>
cc:	"David Miller" <davem@...emloft.net>,
	"bhutchings@...arflare.com" <bhutchings@...arflare.com>,
	"pstaszewski@...are.pl" <pstaszewski@...are.pl>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] bnx2x: reduce skb truesize by 50%

On Thu, 2011-11-10 at 07:27 -0800, Eric Dumazet wrote:
> Le jeudi 10 novembre 2011 à 17:05 +0200, Eilon Greenstein a écrit :
> > > --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
> > > +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x.h
> > > @@ -1185,9 +1185,14 @@ struct bnx2x {
> > >  #define ETH_MAX_PACKET_SIZE		1500
> > >  #define ETH_MAX_JUMBO_PACKET_SIZE	9600
> > >  
> > > -	/* Max supported alignment is 256 (8 shift) */
> > > -#define BNX2X_RX_ALIGN_SHIFT		((L1_CACHE_SHIFT < 8) ? \
> > > -					 L1_CACHE_SHIFT : 8)
> > > +/* Max supported alignment is 256 (8 shift)
> > > + * It should ideally be min(L1_CACHE_SHIFT, 8)
> > > + * Choosing 5 (32 bytes) permits to get skb heads of 2048 bytes
> > > + * instead of 4096 bytes.
> > > + * With SLUB/SLAB allocators, data will be cache line aligned anyway.
> > > + */
> > > +#define BNX2X_RX_ALIGN_SHIFT		5
> > > +
> > 
> > Hi Eric,
> > 
> > This can seriously hurt the PCI utilization. So in scenarios in which
> > the PCI is the bottle neck, you will see performance degradation. We are
> > looking at alternatives to reduce the allocation, but it is taking a
> > while. Please hold off with this patch.
> 
> What do you mean exactly ?
> 
> This patch doesnt change skb->data alignment, its still 64 bytes
> aligned. (cqe_fp->placement_offset == 2). PCI utilization is the same.
> 
> Only SLOB could get a misalignement, but who uses SLOB for performance ?

Obviously you are right... But the FW is configured to the wrong
alignment and that will affect the end alignment (padding) which is
significant in small packets scenarios where the PCI is the bottle neck.

> Alternative would be to check why hardware need 2*L1_CACHE_BYTES extra
> room for alignment... Normaly it could be 1*L1_CACHE_BYTES ?

Again - you are a mind reader :) This is what we are looking into right
now. The problem is that `if` the buffer is not aligned (SLOB) we can
overstep the allocated boundaries by configuring the FW to align.

>  	/* FW use 2 Cache lines Alignment for start packet and size  */
> -#define BNX2X_FW_RX_ALIGN              (2 << BNX2X_RX_ALIGN_SHIFT)
> +#define BNX2X_FW_RX_ALIGN              (1 << BNX2X_RX_ALIGN_SHIFT)
> 
> 
> 
> 




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