Re: [PATCH] net: use hardware buffer pool to allocate skb

[Alexander Duyck <alexander.h.duyck@redhat.com>]

On 10/16/2014 09:57 AM, Eric Dumazet wrote:
> On Thu, 2014-10-16 at 08:28 -0700, Alexander Duyck wrote:
>
>> I think the part you are not getting is that is how buffers are
>> essentially handled now.  So for example in the case if igb the only
>> part we have copied out is usually the header, or the entire frame in
>> the case of small packets.  This has to happen in order to allow for
>> changes to the header for routing and such.  Beyond that the frags that
>> are passed are the buffers that igb is still holding onto.  So
>> effectively what the other device transmits in a bridging/routing
>> scenario is my own net card specified buffer plus the copied/modified
>> header.
>>
>> For a brief period igb used build_skb but that isn't valid on most
>> systems as memory mapped for a device can be overwritten if the page is
>> unmapped resulting in any changes to the header for routing/bridging
>> purposes being invalidated.  Thus we cannot use the buffers for both the
>> skb->data header which may be changed and Rx DMA simultaneously.
> This reminds me that igb still has skb->truesize underestimation by 100%
>
> If a fragment is held in some socket receive buffer, a full page is
> consumed, not 2048 bytes.
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index a21b14495ebd..56ca6c78985e 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -6586,9 +6586,11 @@ static bool igb_add_rx_frag(struct igb_ring *rx_ring,
>   	struct page *page = rx_buffer->page;
>   	unsigned int size = le16_to_cpu(rx_desc->wb.upper.length);
>   #if (PAGE_SIZE < 8192)
> -	unsigned int truesize = IGB_RX_BUFSZ;
> +	unsigned int segsize = IGB_RX_BUFSZ;
> +	unsigned int truesize = PAGE_SIZE;
>   #else
> -	unsigned int truesize = ALIGN(size, L1_CACHE_BYTES);
> +	unsigned int segsize = ALIGN(size, L1_CACHE_BYTES);
> +	unsigned int truesize = segsize;
>   #endif

So if a page is used twice we are double counting the page size for the 
socket then, is that correct?  I just want to make sure because prior to 
this patch both flows did the same thing and counted the portion of the 
page used in this pass, now with this change for PAGE_SIZE of 4K we 
count the entire page, and for all other cases we count the portion of 
the page used.

Thanks,

Alex