Re: [net-next 01/13] ixgbe: Add support for IPv6 and UDP to ixgbe_get_headlen

[Alexander Duyck <alexander.duyck@gmail.com>]

On 10/20/2012 1:01 AM, Eric Dumazet wrote:
> On Fri, 2012-10-19 at 23:25 -0700, Jeff Kirsher wrote:
>> From: Alexander Duyck <alexander.h.duyck@intel.com>
>>
>> This change adds support for IPv6 and UDP to ixgbe_get_headlen. The
>> advantage to this is that we can now handle ipv4/UDP, ipv6/TCP, and
>> ipv6/UDP with a single memcpy instead of having to do them in multiple
>> pskb_may_pull calls.
>>
>> A quick bit of testing shows that we increase throughput for a single
>> session of netperf from 8800Mpbs to about 9300Mpbs in the case of ipv6/TCP.
>> As such overall ipv6 performance should improve with this change.
>>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> Tested-by: Stephen Ko  <stephen.s.ko@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> ---
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)
> Thats pretty interesting.
>
> Why this work is not done generically, so that all drivers can have
> benefit of it ?
>
> Even a driver not using fragments could have benefit, because we could
> prefetch() so that GRO stack is faster (no stall to get the last 2 bytes
> of a TCP header when timestamps are used)
>
> Thanks

The ixgbe_get_headlen function originally was written to handle the case 
where we were doing the hardware LRO/RSC feature but we weren't setting 
gso_size.  We were using it to get the size of the header so we could 
determine the average payload size of the frame.  When we switched over 
to page based receive the function came along with it and had actually 
prevented a performance regression for IPv4/TCP which this patch is 
fixing for the other supported protocols.  The main idea of this code is 
now to emulate what our hardware was doing in determining the length of 
the header when we had header-data split enabled.  I hadn't really 
thought of any way it could be used in a generic sense.  The function is 
essentially is built around several assumptions that only apply to the 
igb/ixgbe case such as the fact that the entire block being linear and 
the limitation of the protocols so that they match up with what we used 
to do for header-data split.

That being said, if there is a way for you to use it in the generic case 
feel free to copy the ixgbe version, rename it, and export it for use.  
The key bit for me is that I just need to be able to get the header 
length before calling eth_type_trans as I only want to do one memcpy for 
the entire header.

Thanks,

Alex