* [PATCH net 1/2] bonding: ensure that the TSO being set on bond master
@ 2014-01-08 7:28 Ding Tianhong
2014-01-08 8:33 ` Veaceslav Falico
0 siblings, 1 reply; 3+ messages in thread
From: Ding Tianhong @ 2014-01-08 7:28 UTC (permalink / raw)
To: Jay Vosburgh, Veaceslav Falico, Eric Dumazet, David S. Miller,
Netdev
The commit b0ce3508(bonding: allow TSO being set on bonding master)
has make the TSO being set for bond dev, but in some situation, if
the slave did not have the NETIF_F_SG features, the bond master will
miss the TSO features in netdev_fix_features because the TSO is
depended on SG. So I have to add SG and TSO features on bond master
together.
The netdev_add_tso_features() was only used for bonding, so I think no
need to export it to netdevice.h.
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
---
drivers/net/bonding/bond_main.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e06c445..8ce67ed 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1045,6 +1045,20 @@ static void bond_netpoll_cleanup(struct net_device *bond_dev)
/*---------------------------------- IOCTL ----------------------------------*/
+/* Allow TSO being used on stacked device:
+ * Performing the GSO segmentation before last device
+ * is a performance improvement.
+ * The TSO is depended on SG, so add SG and TSO together,
+ * otherwise the netdev_fix_features() may clean the TSO.
+ */
+static netdev_features_t bond_add_tso_features(netdev_features_t features,
+ netdev_features_t mask)
+{
+ return netdev_increment_features(features,
+ NETIF_F_ALL_TSO | NETIF_F_SG,
+ mask);
+}
+
static netdev_features_t bond_fix_features(struct net_device *dev,
netdev_features_t features)
{
@@ -1068,7 +1082,7 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
slave->dev->features,
mask);
}
- features = netdev_add_tso_features(features, mask);
+ features = bond_add_tso_features(features, mask);
return features;
}
--
1.8.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net 1/2] bonding: ensure that the TSO being set on bond master
2014-01-08 7:28 [PATCH net 1/2] bonding: ensure that the TSO being set on bond master Ding Tianhong
@ 2014-01-08 8:33 ` Veaceslav Falico
2014-01-08 9:25 ` Ding Tianhong
0 siblings, 1 reply; 3+ messages in thread
From: Veaceslav Falico @ 2014-01-08 8:33 UTC (permalink / raw)
To: Ding Tianhong; +Cc: Jay Vosburgh, Eric Dumazet, David S. Miller, Netdev
On Wed, Jan 08, 2014 at 03:28:24PM +0800, Ding Tianhong wrote:
>The commit b0ce3508(bonding: allow TSO being set on bonding master)
>has make the TSO being set for bond dev, but in some situation, if
>the slave did not have the NETIF_F_SG features, the bond master will
>miss the TSO features in netdev_fix_features because the TSO is
>depended on SG. So I have to add SG and TSO features on bond master
>together.
Do you know why TSO depends on SG? And what will happen if bonding supports
SG, but one of its slaves doesn't?
>
>The netdev_add_tso_features() was only used for bonding, so I think no
>need to export it to netdevice.h.
>
>Cc: Eric Dumazet <edumazet@google.com>
>Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>---
> drivers/net/bonding/bond_main.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index e06c445..8ce67ed 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -1045,6 +1045,20 @@ static void bond_netpoll_cleanup(struct net_device *bond_dev)
>
> /*---------------------------------- IOCTL ----------------------------------*/
>
>+/* Allow TSO being used on stacked device:
>+ * Performing the GSO segmentation before last device
>+ * is a performance improvement.
>+ * The TSO is depended on SG, so add SG and TSO together,
>+ * otherwise the netdev_fix_features() may clean the TSO.
>+ */
>+static netdev_features_t bond_add_tso_features(netdev_features_t features,
>+ netdev_features_t mask)
>+{
>+ return netdev_increment_features(features,
>+ NETIF_F_ALL_TSO | NETIF_F_SG,
>+ mask);
>+}
>+
> static netdev_features_t bond_fix_features(struct net_device *dev,
> netdev_features_t features)
> {
>@@ -1068,7 +1082,7 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
> slave->dev->features,
> mask);
> }
>- features = netdev_add_tso_features(features, mask);
>+ features = bond_add_tso_features(features, mask);
>
> return features;
> }
>--
>1.8.0
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net 1/2] bonding: ensure that the TSO being set on bond master
2014-01-08 8:33 ` Veaceslav Falico
@ 2014-01-08 9:25 ` Ding Tianhong
0 siblings, 0 replies; 3+ messages in thread
From: Ding Tianhong @ 2014-01-08 9:25 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: Jay Vosburgh, Eric Dumazet, David S. Miller, Netdev
On 2014/1/8 16:33, Veaceslav Falico wrote:
> On Wed, Jan 08, 2014 at 03:28:24PM +0800, Ding Tianhong wrote:
>> The commit b0ce3508(bonding: allow TSO being set on bonding master)
>> has make the TSO being set for bond dev, but in some situation, if
>> the slave did not have the NETIF_F_SG features, the bond master will
>> miss the TSO features in netdev_fix_features because the TSO is
>> depended on SG. So I have to add SG and TSO features on bond master
>> together.
>
> Do you know why TSO depends on SG? And what will happen if bonding supports
> SG, but one of its slaves doesn't?
Yes, I miss it, something terrible will happen, thanks.
I think if the slave's hw_features support the SG and TSO, but not set, the bond
maybe could set them and then performance will be better, otherwise, the bond should
not do anything for it.
Regards
Ding
>
>>
>> The netdev_add_tso_features() was only used for bonding, so I think no
>> need to export it to netdevice.h.
>>
>> Cc: Eric Dumazet <edumazet@google.com>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bond_main.c | 16 +++++++++++++++-
>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index e06c445..8ce67ed 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -1045,6 +1045,20 @@ static void bond_netpoll_cleanup(struct net_device *bond_dev)
>>
>> /*---------------------------------- IOCTL ----------------------------------*/
>>
>> +/* Allow TSO being used on stacked device:
>> + * Performing the GSO segmentation before last device
>> + * is a performance improvement.
>> + * The TSO is depended on SG, so add SG and TSO together,
>> + * otherwise the netdev_fix_features() may clean the TSO.
>> + */
>> +static netdev_features_t bond_add_tso_features(netdev_features_t features,
>> + netdev_features_t mask)
>> +{
>> + return netdev_increment_features(features,
>> + NETIF_F_ALL_TSO | NETIF_F_SG,
>> + mask);
>> +}
>> +
>> static netdev_features_t bond_fix_features(struct net_device *dev,
>> netdev_features_t features)
>> {
>> @@ -1068,7 +1082,7 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
>> slave->dev->features,
>> mask);
>> }
>> - features = netdev_add_tso_features(features, mask);
>> + features = bond_add_tso_features(features, mask);
>>
>> return features;
>> }
>> --
>> 1.8.0
>>
>>
>>
>
> .
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-01-08 9:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-08 7:28 [PATCH net 1/2] bonding: ensure that the TSO being set on bond master Ding Tianhong
2014-01-08 8:33 ` Veaceslav Falico
2014-01-08 9:25 ` Ding Tianhong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).