Re: [RFC][net-next-2.6 PATCH 3/4] ethtool: set hard_header_len using ETH_FLAG_{TX|RX}VLAN

[John Fastabend <john.r.fastabend@intel.com>]

On 10/25/2010 3:45 PM, Jesse Gross wrote:
> On Fri, Oct 22, 2010 at 6:00 AM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
>> On Thu, 2010-10-21 at 15:10 -0700, John Fastabend wrote:
>>> Toggling the vlan tx|rx hw offloads needs to set the hard_header_len
>>> as well otherwise we end up using LL_RESERVED_SPACE incorrectly.
>>> This results in pskb_expand_head() being used unnecessarily.
>>>
>>> This add a check in ethtool_op_set_flags to catch the ETH_FLAG_TXVLAN
>>> flag and set the header length.
>> [...]
>>
>> Note that not every driver that implements the set_flags operation calls
>> back to ethtool_op_set_flags().
> 
> Currently all of the drivers that support toggling this using ethtool
> call into ethtool_op_set_flags.  Even if they don't, things will
> continue to work correctly, albeit with a performance hit, so it's not
> a catastrophe.
> 
> This does assume that drivers which support offloading will start with
> it enabled.  If they don't and just use the non-vlan header length
> then this will drop the header length down even further when
> offloading is enabled.  All current drivers that support toggling do
> start with offloading enabled, so maybe it's not that big a deal.
> 
> Another issue is that cards that don't support vlan offloading at all
> probably won't take the header into account, so they'll get hit every
> time.
> 

The lower layer driver should not include the vlan tag in
hard_header_len because pkts pushed to the real net device will not add
the vlan tag. The vlan device however should increment/dec the len value
depending on if the underlying net device is offloading the vlan tagging.

> When we are using vlan devices we also manually add the vlan header
> length but it doesn't update if we change the underlying device.  It
> seems a little redundant to have to do it in both places.

Right, I think doing this in vlan_transfer_features() should work.
	
> 
> I like that this is generic and independent of vlan devices.
> Hopefully we can figure out these corner cases (or maybe decide that
> they're not important or this is strictly an improvement).

I'll post an update. Thanks for the comments.

-- John

> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html