netdev - Re: [RFC PATCH 07/12] vlan: convert VLAN devices to use ndo_fix

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: <20101219010151.GD12005@rere.qmqm.pl>
Date:	Sun, 19 Dec 2010 02:01:51 +0100
From:	Michał Mirosław <mirq-linux@...e.qmqm.pl>
To:	Ben Hutchings <bhutchings@...arflare.com>
Cc:	netdev@...r.kernel.org
Subject: Re: [RFC PATCH 07/12] vlan: convert VLAN devices to use
	ndo_fix_features()

On Thu, Dec 16, 2010 at 11:36:58PM +0000, Ben Hutchings wrote:
> On Wed, 2010-12-15 at 23:24 +0100, Michał Mirosław wrote:
> > Note: get_flags was actually broken, because it should return the
> > flags capped with vlan_features. This is now done implicitly by
> > limiting netdev->hw_features.
> > 
> > RX checksumming offload control is (and was) broken, as there was no way
> > before to say whether it's done for tagged packets.
> > 
> > Signed-off-by: Michał Mirosław <mirq-linux@...e.qmqm.pl>
> > ---
> >  net/8021q/vlan.c     |    3 +-
> >  net/8021q/vlan_dev.c |   51 ++++++++++++++-----------------------------------
> >  2 files changed, 16 insertions(+), 38 deletions(-)
> > 
> > diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
> > index 6e64f7c..583d47b 100644
> > --- a/net/8021q/vlan.c
> > +++ b/net/8021q/vlan.c
> > @@ -329,8 +329,7 @@ static void vlan_transfer_features(struct net_device *dev,
> >  {
> >  	unsigned long old_features = vlandev->features;
> >  
> > -	vlandev->features &= ~dev->vlan_features;
> > -	vlandev->features |= dev->features & dev->vlan_features;
> > +	netdev_update_features(vlandev);
> >  	vlandev->gso_max_size = dev->gso_max_size;
> >  
> >  	if (dev->features & NETIF_F_HW_VLAN_TX)
> > diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> > index be73753..468c899 100644
> > --- a/net/8021q/vlan_dev.c
> > +++ b/net/8021q/vlan_dev.c
> > @@ -691,8 +691,8 @@ static int vlan_dev_init(struct net_device *dev)
> >  					  (1<<__LINK_STATE_DORMANT))) |
> >  		      (1<<__LINK_STATE_PRESENT);
> >  
> > -	dev->features |= real_dev->features & real_dev->vlan_features;
> > -	dev->features |= NETIF_F_LLTX;
> > +	dev->hw_features = real_dev->vlan_features;
> [...]
> net_device::hw_features is supposed to represent features that can be
> toggled, but the inclusion of a flag in net_device::vlan_features does
> not mean the feature can be toggled.

All TX offloads are by definition togglable since it just a matter of not
issuing skb's that need them. RX offloads on the other hand are controlled
by underlying device, so probably should be removed from this set and
forced on or left alone in ndo_fix_features().

> If this is to be a straight conversion, that line should be:
> 	dev->hw_features = real_dev->vlan_features & NETIF_F_TSO;

I'll mask this with ALL_CSUM+SG+ALL_TSO if you don't mind extending the
possibilities anyway.

Best Regards,
Michał Mirosław
--
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