[PATCH] vlan: allow TSO setting on vlan interfaces

[PATCH] vlan: allow TSO setting on vlan interfaces

[Eric Dumazet]

When we need to shape traffic with low speeds, we need to disable tso on
network interface :

ethtool -K eth0.2240 tso off

It seems vlan interfaces miss the set_tso() ethtool method.
Propagating tso changes from lower device is not always wanted, some
vlan want TSO on, other want TSO off.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index c6456cb..d827c2e 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -844,6 +844,7 @@ static const struct ethtool_ops vlan_ethtool_ops = {
 	.get_link		= ethtool_op_get_link,
 	.get_rx_csum		= vlan_ethtool_get_rx_csum,
 	.get_flags		= vlan_ethtool_get_flags,
+	.set_tso		= ethtool_op_set_tso,
 };
 
 static const struct net_device_ops vlan_netdev_ops = {

Re: [PATCH] vlan: allow TSO setting on vlan interfaces

[Ben Hutchings]

On Wed, 2010-07-07 at 19:14 +0200, Eric Dumazet wrote:
> When we need to shape traffic with low speeds, we need to disable tso on
> network interface :
> 
> ethtool -K eth0.2240 tso off
> 
> It seems vlan interfaces miss the set_tso() ethtool method.
> Propagating tso changes from lower device is not always wanted, some
> vlan want TSO on, other want TSO off.

But it should not be possible to enable TSO if the underlying device
doesn't support it.

Ben.

> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> 
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index c6456cb..d827c2e 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -844,6 +844,7 @@ static const struct ethtool_ops vlan_ethtool_ops = {
>  	.get_link		= ethtool_op_get_link,
>  	.get_rx_csum		= vlan_ethtool_get_rx_csum,
>  	.get_flags		= vlan_ethtool_get_flags,
> +	.set_tso		= ethtool_op_set_tso,
>  };
>  
>  static const struct net_device_ops vlan_netdev_ops = {
> 

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH] vlan: allow TSO setting on vlan interfaces

[Eric Dumazet]

Le mercredi 07 juillet 2010 à 18:25 +0100, Ben Hutchings a écrit :
> On Wed, 2010-07-07 at 19:14 +0200, Eric Dumazet wrote:
> > When we need to shape traffic with low speeds, we need to disable tso on
> > network interface :
> > 
> > ethtool -K eth0.2240 tso off
> > 
> > It seems vlan interfaces miss the set_tso() ethtool method.
> > Propagating tso changes from lower device is not always wanted, some
> > vlan want TSO on, other want TSO off.
> 
> But it should not be possible to enable TSO if the underlying device
> doesn't support it.

Yes indeed, I'll provide a more generic patch, thanks !

[PATCH v2] vlan: allow TSO setting on vlan interfaces

[Eric Dumazet]

When we need to shape traffic with low speeds, we need to disable tso on
network interface :

ethtool -K eth0.2240 tso off

It seems vlan interfaces miss the set_tso() ethtool method.
Propagating tso changes from lower device is not always wanted, some
vlans want TSO on, others want TSO off.

Before enabling TSO, we must check real device supports it.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/8021q/vlan_dev.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index c6456cb..870bc53 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -838,12 +838,25 @@ static struct rtnl_link_stats64 *vlan_dev_get_stats64(struct net_device *dev)
 	return stats;
 }
 
+static int vlan_ethtool_set_tso(struct net_device *dev, u32 data)
+{
+	if (data) {
+		struct net_device *real_dev = vlan_dev_info(dev)->real_dev;
+		unsigned long rdev_vfeatures = real_dev->features & real_dev->vlan_features;
+
+		dev->features |= (NETIF_F_TSO & rdev_vfeatures);
+	} else
+		dev->features &= ~NETIF_F_TSO;
+	return 0;
+}
+
 static const struct ethtool_ops vlan_ethtool_ops = {
 	.get_settings	        = vlan_ethtool_get_settings,
 	.get_drvinfo	        = vlan_ethtool_get_drvinfo,
 	.get_link		= ethtool_op_get_link,
 	.get_rx_csum		= vlan_ethtool_get_rx_csum,
 	.get_flags		= vlan_ethtool_get_flags,
+	.set_tso		= vlan_ethtool_set_tso,
 };
 
 static const struct net_device_ops vlan_netdev_ops = {

Re: [PATCH v2] vlan: allow TSO setting on vlan interfaces

[Ben Hutchings]

On Thu, 2010-07-08 at 11:39 +0200, Eric Dumazet wrote:
> When we need to shape traffic with low speeds, we need to disable tso on
> network interface :
> 
> ethtool -K eth0.2240 tso off
> 
> It seems vlan interfaces miss the set_tso() ethtool method.
> Propagating tso changes from lower device is not always wanted, some
> vlans want TSO on, others want TSO off.
> 
> Before enabling TSO, we must check real device supports it.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  net/8021q/vlan_dev.c |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index c6456cb..870bc53 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -838,12 +838,25 @@ static struct rtnl_link_stats64 *vlan_dev_get_stats64(struct net_device *dev)
>  	return stats;
>  }
>  
> +static int vlan_ethtool_set_tso(struct net_device *dev, u32 data)
> +{
> +	if (data) {
> +		struct net_device *real_dev = vlan_dev_info(dev)->real_dev;
> +		unsigned long rdev_vfeatures = real_dev->features & real_dev->vlan_features;
> +
> +		dev->features |= (NETIF_F_TSO & rdev_vfeatures);
> +	} else
> +		dev->features &= ~NETIF_F_TSO;
> +	return 0;
> +}
[...]

This should not silently ignore attempts to enable TSO.  I think it
should be something like:

static int vlan_ethtool_set_tso(struct net_device *dev, u32 data)
{
	if (data) {
		struct net_device *real_dev = vlan_dev_info(dev)->real_dev;

		/* Underlying device must support TSO for VLAN-tagged packets
		 * and must have TSO enabled now. */
		if (!(real_dev->vlan_features & NETIF_F_TSO))
			return -EOPNOTSUPP;
		if (!(real_dev->features & NETIF_F_TSO))
			return -EINVAL;

		dev->features |= NETIF_F_TSO;
	} else {
		dev->features &= ~NETIF_F_TSO;
	}

	return 0;
}

I think the vlan driver should also have a netdev notifier to handle
feature changes on the underlying device.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH v2] vlan: allow TSO setting on vlan interfaces

[Ben Hutchings]

On Thu, 2010-07-08 at 16:43 +0100, Ben Hutchings wrote:
> On Thu, 2010-07-08 at 11:39 +0200, Eric Dumazet wrote:
> > When we need to shape traffic with low speeds, we need to disable tso on
> > network interface :
> > 
> > ethtool -K eth0.2240 tso off
> > 
> > It seems vlan interfaces miss the set_tso() ethtool method.
> > Propagating tso changes from lower device is not always wanted, some
> > vlans want TSO on, others want TSO off.
[...]
> I think the vlan driver should also have a netdev notifier to handle
> feature changes on the underlying device.

To clarify, I think offload features should be disabled on a vlan device
if they are later disabled on the underlying device.  Propagating
changes to enable features, as you say, might not be wanted.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH v2] vlan: allow TSO setting on vlan interfaces

[Patrick McHardy]

Ben Hutchings wrote:
> On Thu, 2010-07-08 at 16:43 +0100, Ben Hutchings wrote:
>   
>> On Thu, 2010-07-08 at 11:39 +0200, Eric Dumazet wrote:
>>     
>>> When we need to shape traffic with low speeds, we need to disable tso on
>>> network interface :
>>>
>>> ethtool -K eth0.2240 tso off
>>>
>>> It seems vlan interfaces miss the set_tso() ethtool method.
>>> Propagating tso changes from lower device is not always wanted, some
>>> vlans want TSO on, others want TSO off.
>>>       
> [...]
>   
>> I think the vlan driver should also have a netdev notifier to handle
>> feature changes on the underlying device.
>>     
>
> To clarify, I think offload features should be disabled on a vlan device
> if they are later disabled on the underlying device.  Propagating
> changes to enable features, as you say, might not be wanted.

I agree. The VLAN driver should also propagate feature changes
upwards itself.

Re: [PATCH v2] vlan: allow TSO setting on vlan interfaces

[Eric Dumazet]

Le jeudi 08 juillet 2010 à 16:49 +0100, Ben Hutchings a écrit :
> On Thu, 2010-07-08 at 16:43 +0100, Ben Hutchings wrote:
> > On Thu, 2010-07-08 at 11:39 +0200, Eric Dumazet wrote:
> > > When we need to shape traffic with low speeds, we need to disable tso on
> > > network interface :
> > > 
> > > ethtool -K eth0.2240 tso off
> > > 
> > > It seems vlan interfaces miss the set_tso() ethtool method.
> > > Propagating tso changes from lower device is not always wanted, some
> > > vlans want TSO on, others want TSO off.
> [...]
> > I think the vlan driver should also have a netdev notifier to handle
> > feature changes on the underlying device.
> 
> To clarify, I think offload features should be disabled on a vlan device
> if they are later disabled on the underlying device.  Propagating
> changes to enable features, as you say, might not be wanted.

OK, but isnt it already done ?

Check vlan_transfer_features() in net/8021q/vlan.c


# ethtool -k eth3|grep tcp-segmentation
tcp-segmentation-offload: on
# ethtool -k eth3.103|grep tcp-segmentation
tcp-segmentation-offload: on

# ethtool -K eth3 tso off

# ethtool -k eth3|grep tcp-segmentation
tcp-segmentation-offload: off
# ethtool -k eth3.103|grep tcp-segmentation
tcp-segmentation-offload: off

# ethtool -K eth3 tso on
# ethtool -k eth3.103|grep tcp-segmentation
tcp-segmentation-offload: on


We should not change it to avoid ORing TSO, it might break some setups.

Re: [PATCH v2] vlan: allow TSO setting on vlan interfaces

[Eric Dumazet]

Le jeudi 08 juillet 2010 à 16:43 +0100, Ben Hutchings a écrit :
> On Thu, 2010-07-08 at 11:39 +0200, Eric Dumazet wrote:
> > When we need to shape traffic with low speeds, we need to disable tso on
> > network interface :
> > 
> > ethtool -K eth0.2240 tso off
> > 
> > It seems vlan interfaces miss the set_tso() ethtool method.
> > Propagating tso changes from lower device is not always wanted, some
> > vlans want TSO on, others want TSO off.
> > 
> > Before enabling TSO, we must check real device supports it.
> > 
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> > ---
> >  net/8021q/vlan_dev.c |   13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> > index c6456cb..870bc53 100644
> > --- a/net/8021q/vlan_dev.c
> > +++ b/net/8021q/vlan_dev.c
> > @@ -838,12 +838,25 @@ static struct rtnl_link_stats64 *vlan_dev_get_stats64(struct net_device *dev)
> >  	return stats;
> >  }
> >  
> > +static int vlan_ethtool_set_tso(struct net_device *dev, u32 data)
> > +{
> > +	if (data) {
> > +		struct net_device *real_dev = vlan_dev_info(dev)->real_dev;
> > +		unsigned long rdev_vfeatures = real_dev->features & real_dev->vlan_features;
> > +
> > +		dev->features |= (NETIF_F_TSO & rdev_vfeatures);
> > +	} else
> > +		dev->features &= ~NETIF_F_TSO;
> > +	return 0;
> > +}
> [...]
> 
> This should not silently ignore attempts to enable TSO.  I think it
> should be something like:
> 
> static int vlan_ethtool_set_tso(struct net_device *dev, u32 data)
> {
> 	if (data) {
> 		struct net_device *real_dev = vlan_dev_info(dev)->real_dev;
> 
> 		/* Underlying device must support TSO for VLAN-tagged packets
> 		 * and must have TSO enabled now. */
> 		if (!(real_dev->vlan_features & NETIF_F_TSO))
> 			return -EOPNOTSUPP;
> 		if (!(real_dev->features & NETIF_F_TSO))
> 			return -EINVAL;
> 
> 		dev->features |= NETIF_F_TSO;
> 	} else {
> 		dev->features &= ~NETIF_F_TSO;
> 	}
> 
> 	return 0;
> }

I agree.

What about other TSO flags like NETIF_F_TSO6 & NETIF_F_TSO_ECN ?

Not sure if we should manipulate them in set_tso()

Re: [PATCH v2] vlan: allow TSO setting on vlan interfaces

[Eric Dumazet]

Le jeudi 08 juillet 2010 à 17:55 +0200, Patrick McHardy a écrit :

> I agree. The VLAN driver should also propagate feature changes
> upwards itself.

I noticed bonding is also at fault in this area.

bond0.xxx are not noticed of TSO changes on bond0

Re: [PATCH v2] vlan: allow TSO setting on vlan interfaces

[Ben Hutchings]

On Thu, 2010-07-08 at 18:00 +0200, Eric Dumazet wrote:
> Le jeudi 08 juillet 2010 à 16:49 +0100, Ben Hutchings a écrit :
> > On Thu, 2010-07-08 at 16:43 +0100, Ben Hutchings wrote:
> > > On Thu, 2010-07-08 at 11:39 +0200, Eric Dumazet wrote:
> > > > When we need to shape traffic with low speeds, we need to disable tso on
> > > > network interface :
> > > > 
> > > > ethtool -K eth0.2240 tso off
> > > > 
> > > > It seems vlan interfaces miss the set_tso() ethtool method.
> > > > Propagating tso changes from lower device is not always wanted, some
> > > > vlans want TSO on, others want TSO off.
> > [...]
> > > I think the vlan driver should also have a netdev notifier to handle
> > > feature changes on the underlying device.
> > 
> > To clarify, I think offload features should be disabled on a vlan device
> > if they are later disabled on the underlying device.  Propagating
> > changes to enable features, as you say, might not be wanted.
> 
> OK, but isnt it already done ?
[...]

Yes, sorry for the noise.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH v2] vlan: allow TSO setting on vlan interfaces

[Eric Dumazet]

Le jeudi 08 juillet 2010 à 17:05 +0100, Ben Hutchings a écrit :

> 
> Yes, sorry for the noise.

No problem, it was a good point to recheck anyway.

[PATCH v3] vlan: allow TSO setting on vlan interfaces

[Eric Dumazet]

When we need to shape traffic using low speeds, we need to
disable tso on network interface :

ethtool -K eth0.2240 tso off

It seems vlan interfaces miss the set_tso() ethtool method.

Before enabling TSO, we must check real device supports 
TSO for VLAN-tagged packets and enables TSO.

Note that a TSO change on real device propagates TSO setting
on all vlans, even if admin selected a different TSO setting.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/8021q/vlan_dev.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 7865a4c..a1b8171 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -836,12 +836,32 @@ static struct rtnl_link_stats64 *vlan_dev_get_stats64(struct net_device *dev, st
 	return stats;
 }
 
+static int vlan_ethtool_set_tso(struct net_device *dev, u32 data)
+{
+       if (data) {
+		struct net_device *real_dev = vlan_dev_info(dev)->real_dev;
+
+		/* Underlying device must support TSO for VLAN-tagged packets
+		 * and must have TSO enabled now.
+		 */
+		if (!(real_dev->vlan_features & NETIF_F_TSO))
+			return -EOPNOTSUPP;
+		if (!(real_dev->features & NETIF_F_TSO))
+			return -EINVAL;
+		dev->features |= NETIF_F_TSO;
+	} else {
+		dev->features &= ~NETIF_F_TSO;
+	}
+	return 0;
+}
+
 static const struct ethtool_ops vlan_ethtool_ops = {
 	.get_settings	        = vlan_ethtool_get_settings,
 	.get_drvinfo	        = vlan_ethtool_get_drvinfo,
 	.get_link		= ethtool_op_get_link,
 	.get_rx_csum		= vlan_ethtool_get_rx_csum,
 	.get_flags		= vlan_ethtool_get_flags,
+	.set_tso                = vlan_ethtool_set_tso,
 };
 
 static const struct net_device_ops vlan_netdev_ops = {

Re: [PATCH v3] vlan: allow TSO setting on vlan interfaces

[Ben Hutchings]

On Thu, 2010-07-08 at 18:37 +0200, Eric Dumazet wrote:
> When we need to shape traffic using low speeds, we need to
> disable tso on network interface :
> 
> ethtool -K eth0.2240 tso off
> 
> It seems vlan interfaces miss the set_tso() ethtool method.
> 
> Before enabling TSO, we must check real device supports 
> TSO for VLAN-tagged packets and enables TSO.
> 
> Note that a TSO change on real device propagates TSO setting
> on all vlans, even if admin selected a different TSO setting.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>

> ---
>  net/8021q/vlan_dev.c |   20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index 7865a4c..a1b8171 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -836,12 +836,32 @@ static struct rtnl_link_stats64 *vlan_dev_get_stats64(struct net_device *dev, st
>  	return stats;
>  }
>  
> +static int vlan_ethtool_set_tso(struct net_device *dev, u32 data)
> +{
> +       if (data) {
> +		struct net_device *real_dev = vlan_dev_info(dev)->real_dev;
> +
> +		/* Underlying device must support TSO for VLAN-tagged packets
> +		 * and must have TSO enabled now.
> +		 */
> +		if (!(real_dev->vlan_features & NETIF_F_TSO))
> +			return -EOPNOTSUPP;
> +		if (!(real_dev->features & NETIF_F_TSO))
> +			return -EINVAL;
> +		dev->features |= NETIF_F_TSO;
> +	} else {
> +		dev->features &= ~NETIF_F_TSO;
> +	}
> +	return 0;
> +}
> +
>  static const struct ethtool_ops vlan_ethtool_ops = {
>  	.get_settings	        = vlan_ethtool_get_settings,
>  	.get_drvinfo	        = vlan_ethtool_get_drvinfo,
>  	.get_link		= ethtool_op_get_link,
>  	.get_rx_csum		= vlan_ethtool_get_rx_csum,
>  	.get_flags		= vlan_ethtool_get_flags,
> +	.set_tso                = vlan_ethtool_set_tso,
>  };
>  
>  static const struct net_device_ops vlan_netdev_ops = {
> 
> 
-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH v2] vlan: allow TSO setting on vlan interfaces

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 08 Jul 2010 11:39:31 +0200

> When we need to shape traffic with low speeds, we need to disable tso on
> network interface :
> 
> ethtool -K eth0.2240 tso off
> 
> It seems vlan interfaces miss the set_tso() ethtool method.
> Propagating tso changes from lower device is not always wanted, some
> vlans want TSO on, others want TSO off.
> 
> Before enabling TSO, we must check real device supports it.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied to net-next-2.6, thanks Eric.

Re: [PATCH v3] vlan: allow TSO setting on vlan interfaces

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Thu, 08 Jul 2010 19:25:48 +0100

> On Thu, 2010-07-08 at 18:37 +0200, Eric Dumazet wrote:
>> When we need to shape traffic using low speeds, we need to
>> disable tso on network interface :
>> 
>> ethtool -K eth0.2240 tso off
>> 
>> It seems vlan interfaces miss the set_tso() ethtool method.
>> 
>> Before enabling TSO, we must check real device supports 
>> TSO for VLAN-tagged packets and enables TSO.
>> 
>> Note that a TSO change on real device propagates TSO setting
>> on all vlans, even if admin selected a different TSO setting.
>> 
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>

Ok, just to clarify, I'll make sure I get this version instead of "v2"
which I just replied to :)