netdev - Re: [PATCH 3/5] e1000e: Allow TSO to trickle down to VLAN device

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]

Date:	Mon, 21 Apr 2008 16:24:25 +0200
From:	Patrick McHardy <kaber@...sh.net>
To:	"Kok, Auke" <auke-jan.h.kok@...el.com>
CC:	jeff@...zik.org, netdev@...r.kernel.org,
	e1000-devel@...ts.sourceforge.net,
	"Waskiewicz Jr, Peter P" <peter.p.waskiewicz.jr@...el.com>
Subject: Re: [PATCH 3/5] e1000e: Allow TSO to trickle down to VLAN device

Kok, Auke wrote:
> Patrick McHardy wrote:
>> Auke Kok wrote:
>>> Fix TSO over VLAN's by propagating settings to our VLAN devices.
>>>   
>> What exactly is this supposed to fix? If this simply wants
>> to propagate feature changes, I think it should use
>> netdev_feat_change and handle that within the VLAN code.
> 
> I asked PJ and got this reply:
> 
> //
> VLAN devices didn't get the parent's feature flags on creation.  I went
> to fix this in the kernel, people pushed back that some devices couldn't
> support both VLAN tag insertion offload and TSO, so I didn't push the
> issue.  I worked around the issue by copying the flags in the driver.
> The downside is when we turn off TSO using ethtool, we need to remove
> TSO from all VLAN devices, since the hardware segmenter is no longer
> available if the parent device doesn't have it enabled as a feature.  We
> were seeing stack-based panics with gso_segment(), which was corrected
> by removing the TSO flag from all VLAN devices.
> 
> I can't seem to find netdev_feat_change anywhere in the kernel, or
> variants of that name, so I'm not sure what Patrick is pointing us at.
> 
> -PJ
> //

That was a typo, I meant netdev_features_change(), which is invoked
by dev_ethtool() and calls the netdev notifier with NETDEV_FEAT_CHANGE,
on which the VLAN code could react. I wasn't aware of the TSO + VLAN
acceleration problems you've mentioned, do you have a pointer to more
information about this?

In any case I would prefer to avoid having drivers mess with VLAN
device flags. How about adding a device flag indicating that the
driver supports TSO + VLAN acceleration and using the
NETDEV_FEAT_CHANGE notification inside the VLAN code do adjust
the device's flags properly?

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