diff mbox

[v2,net-next] bridge: Add ability to enable TSO

Message ID 1420780600-11313-1-git-send-email-makita.toshiaki@lab.ntt.co.jp
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Toshiaki Makita Jan. 9, 2015, 5:16 a.m. UTC
Currently a bridge device turns off TSO feature if no bridge ports
support it. We can always enable it, since packets can be segmented on
ports by software as well as on the bridge device.
This will reduce the number of packets processed in the bridge.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
v2: Use an existing helper function.

 net/bridge/br_if.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Pankaj Gupta Jan. 9, 2015, 5:44 a.m. UTC | #1
> 
> Currently a bridge device turns off TSO feature if no bridge ports
> support it. We can always enable it, since packets can be segmented on
> ports by software as well as on the bridge device.
> This will reduce the number of packets processed in the bridge.
> 
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
> v2: Use an existing helper function.
> 
>  net/bridge/br_if.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index ed307db..81e49fb 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -424,6 +424,7 @@ netdev_features_t br_features_recompute(struct net_bridge
> *br,
>  		features = netdev_increment_features(features,
>  						     p->dev->features, mask);
>  	}
> +	features = netdev_add_tso_features(features, mask);

Just a doubt. Are we inducing latency if source has traffic at very low rate.
I mean by default do we need it?

>  
>  	return features;
>  }
> --
> 1.8.1.2
> 
> 
> --
> 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
> 
--
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
Toshiaki Makita Jan. 9, 2015, 6:22 a.m. UTC | #2
On 2015/01/09 14:44, Pankaj Gupta wrote:
> 
>>
>> Currently a bridge device turns off TSO feature if no bridge ports
>> support it. We can always enable it, since packets can be segmented on
>> ports by software as well as on the bridge device.
>> This will reduce the number of packets processed in the bridge.
>>
>> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>> ---
>> v2: Use an existing helper function.
>>
>>  net/bridge/br_if.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
>> index ed307db..81e49fb 100644
>> --- a/net/bridge/br_if.c
>> +++ b/net/bridge/br_if.c
>> @@ -424,6 +424,7 @@ netdev_features_t br_features_recompute(struct net_bridge
>> *br,
>>  		features = netdev_increment_features(features,
>>  						     p->dev->features, mask);
>>  	}
>> +	features = netdev_add_tso_features(features, mask);
> 
> Just a doubt. Are we inducing latency if source has traffic at very low rate.
> I mean by default do we need it?

Is your concern tcp_tso_should_defer() in tcp_write_xmit()?
If so, since TSO packet is created by GSO even without this patch, this
should not increase latency there.
This patch just delays the point of software segmentation from the
bridge device to its port.

Thanks,
Toshiaki Makita

--
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
Pankaj Gupta Jan. 9, 2015, 7:10 p.m. UTC | #3
> > 
> >>
> >> Currently a bridge device turns off TSO feature if no bridge ports
> >> support it. We can always enable it, since packets can be segmented on
> >> ports by software as well as on the bridge device.
> >> This will reduce the number of packets processed in the bridge.
> >>
> >> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> >> ---
> >> v2: Use an existing helper function.
> >>
> >>  net/bridge/br_if.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> >> index ed307db..81e49fb 100644
> >> --- a/net/bridge/br_if.c
> >> +++ b/net/bridge/br_if.c
> >> @@ -424,6 +424,7 @@ netdev_features_t br_features_recompute(struct
> >> net_bridge
> >> *br,
> >>  		features = netdev_increment_features(features,
> >>  						     p->dev->features, mask);
> >>  	}
> >> +	features = netdev_add_tso_features(features, mask);
> > 
> > Just a doubt. Are we inducing latency if source has traffic at very low
> > rate.
> > I mean by default do we need it?
> 
> Is your concern tcp_tso_should_defer() in tcp_write_xmit()?
yes.

> If so, since TSO packet is created by GSO even without this patch, this
> should not increase latency there.
> This patch just delays the point of software segmentation from the
> bridge device to its port.

I think now I got your point. 

Thanks,
Pankaj
> 
> Thanks,
> Toshiaki Makita
> 
> --
> 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
> 
--
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
David Miller Jan. 12, 2015, 9:18 p.m. UTC | #4
From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Date: Fri,  9 Jan 2015 14:16:40 +0900

> Currently a bridge device turns off TSO feature if no bridge ports
> support it. We can always enable it, since packets can be segmented on
> ports by software as well as on the bridge device.
> This will reduce the number of packets processed in the bridge.
> 
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
> v2: Use an existing helper function.

Applied, thank you.
--
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
diff mbox

Patch

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index ed307db..81e49fb 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -424,6 +424,7 @@  netdev_features_t br_features_recompute(struct net_bridge *br,
 		features = netdev_increment_features(features,
 						     p->dev->features, mask);
 	}
+	features = netdev_add_tso_features(features, mask);
 
 	return features;
 }