netdev - Re: [RFC][net-next-2.6 PATCH 3/4] ethtool: set hard_header_len using ETH_FLAG

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: <4CC74F04.6080007@intel.com>
Date:	Tue, 26 Oct 2010 14:58:28 -0700
From:	John Fastabend <john.r.fastabend@...el.com>
To:	Jesse Gross <jesse@...ira.com>
CC:	Ben Hutchings <bhutchings@...arflare.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [RFC][net-next-2.6 PATCH 3/4] ethtool: set hard_header_len using
 ETH_FLAG_{TX|RX}VLAN

On 10/25/2010 3:45 PM, Jesse Gross wrote:
> On Fri, Oct 22, 2010 at 6:00 AM, Ben Hutchings
> <bhutchings@...arflare.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@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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