| Message ID | 52CCFE18.7040606@huawei.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
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 > > > -- 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
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 >> >> >> > > . > -- 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 --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; }
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(-)