Message ID | 20110506175629.BC59D1389B@rere.qmqm.pl |
---|---|
State | Superseded, archived |
Delegated to: | David Miller |
Headers | show |
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote: >This should also fix updating of vlan_features and propagating changes to >VLAN devices on the bond. > >Side effect: it allows user to force-disable some offloads on the bond >interface. > >Note: NETIF_F_VLAN_CHALLENGED is managed by bond_fix_features() now. > >BTW, What are the problems in creating VLAN devices on an empty bond >(as stated in one of bond_setup() comments)? If there are no slaves, then the bond does not have a MAC address assigned (because it gets its initial MAC from the first slave). It's therefore impossible to pass a MAC address up to the VLAN interface. So the limitation is that the bond must have at least one slave before a VLAN may be configured above it. -J >Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> >--- > >Note: This is only compile tested, yet. > > drivers/net/bonding/bond_main.c | 133 +++++++++++++++------------------------ > 1 files changed, 50 insertions(+), 83 deletions(-) > >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >index 9a5feaf..04a2205 100644 >--- a/drivers/net/bonding/bond_main.c >+++ b/drivers/net/bonding/bond_main.c >@@ -344,32 +344,6 @@ out: > } > > /** >- * bond_has_challenged_slaves >- * @bond: the bond we're working on >- * >- * Searches the slave list. Returns 1 if a vlan challenged slave >- * was found, 0 otherwise. >- * >- * Assumes bond->lock is held. >- */ >-static int bond_has_challenged_slaves(struct bonding *bond) >-{ >- struct slave *slave; >- int i; >- >- bond_for_each_slave(bond, slave, i) { >- if (slave->dev->features & NETIF_F_VLAN_CHALLENGED) { >- pr_debug("found VLAN challenged slave - %s\n", >- slave->dev->name); >- return 1; >- } >- } >- >- pr_debug("no VLAN challenged slaves found\n"); >- return 0; >-} >- >-/** > * bond_next_vlan - safely skip to the next item in the vlans list. > * @bond: the bond we're working on > * @curr: item we're advancing from >@@ -1406,52 +1380,61 @@ static int bond_sethwaddr(struct net_device *bond_dev, > return 0; > } > >-#define BOND_VLAN_FEATURES \ >- (NETIF_F_VLAN_CHALLENGED | NETIF_F_HW_VLAN_RX | NETIF_F_HW_VLAN_TX | \ >- NETIF_F_HW_VLAN_FILTER) >- >-/* >- * Compute the common dev->feature set available to all slaves. Some >- * feature bits are managed elsewhere, so preserve those feature bits >- * on the master device. >- */ >-static int bond_compute_features(struct bonding *bond) >+static u32 bond_fix_features(struct net_device *dev, u32 features) > { > struct slave *slave; >- struct net_device *bond_dev = bond->dev; >- u32 features = bond_dev->features; >- u32 vlan_features = 0; >- unsigned short max_hard_header_len = max((u16)ETH_HLEN, >- bond_dev->hard_header_len); >+ struct bonding *bond = netdev_priv(dev); >+ u32 mask; > int i; > >- features &= ~(NETIF_F_ALL_CSUM | BOND_VLAN_FEATURES); >- features |= NETIF_F_GSO_MASK | NETIF_F_NO_CSUM | NETIF_F_NOCACHE_COPY; >- > if (!bond->first_slave) >- goto done; >+ /* Disable adding VLANs to empty bond. But why? --mq */ >+ return features | NETIF_F_VLAN_CHALLENGED; > >+ mask = features; > features &= ~NETIF_F_ONE_FOR_ALL; >+ features |= NETIF_F_ALL_FOR_ALL; > >- vlan_features = bond->first_slave->dev->vlan_features; > bond_for_each_slave(bond, slave, i) { > features = netdev_increment_features(features, > slave->dev->features, >- NETIF_F_ONE_FOR_ALL); >+ mask); >+ } >+ >+ return features; >+} >+ >+#define BOND_VLAN_FEATURES (NETIF_F_ALL_TX_OFFLOADS | \ >+ NETIF_F_SOFT_FEATURES | \ >+ NETIF_F_LRO) >+ >+static void bond_compute_features(struct bonding *bond) >+{ >+ struct slave *slave; >+ struct net_device *bond_dev = bond->dev; >+ u32 old_features, vlan_features = BOND_VLAN_FEATURES; >+ unsigned short max_hard_header_len = ETH_HLEN; >+ int i; >+ >+ if (!bond->first_slave) >+ goto done; >+ >+ bond_for_each_slave(bond, slave, i) { > vlan_features = netdev_increment_features(vlan_features, >- slave->dev->vlan_features, >- NETIF_F_ONE_FOR_ALL); >+ slave->dev->vlan_features, BOND_VLAN_FEATURES); >+ > if (slave->dev->hard_header_len > max_hard_header_len) > max_hard_header_len = slave->dev->hard_header_len; > } > > done: >- features |= (bond_dev->features & BOND_VLAN_FEATURES); >- bond_dev->features = netdev_fix_features(bond_dev, features); >- bond_dev->vlan_features = netdev_fix_features(bond_dev, vlan_features); >+ bond_dev->vlan_features = vlan_features; > bond_dev->hard_header_len = max_hard_header_len; > >- return 0; >+ old_features = bond_dev->features; >+ netdev_update_features(bond_dev); >+ if (old_features == bond_dev->features) >+ netdev_features_change(bond_dev); > } > > static void bond_setup_by_slave(struct net_device *bond_dev, >@@ -1544,7 +1527,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) > struct netdev_hw_addr *ha; > struct sockaddr addr; > int link_reporting; >- int old_features = bond_dev->features; > int res = 0; > > if (!bond->params.use_carrier && slave_dev->ethtool_ops == NULL && >@@ -1577,16 +1559,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) > pr_warning("%s: Warning: enslaved VLAN challenged slave %s. Adding VLANs will be blocked as long as %s is part of bond %s\n", > bond_dev->name, slave_dev->name, > slave_dev->name, bond_dev->name); >- bond_dev->features |= NETIF_F_VLAN_CHALLENGED; > } > } else { > pr_debug("%s: ! NETIF_F_VLAN_CHALLENGED\n", slave_dev->name); >- if (bond->slave_cnt == 0) { >- /* First slave, and it is not VLAN challenged, >- * so remove the block of adding VLANs over the bond. >- */ >- bond_dev->features &= ~NETIF_F_VLAN_CHALLENGED; >- } > } > > /* >@@ -1958,7 +1933,7 @@ err_free: > kfree(new_slave); > > err_undo_flags: >- bond_dev->features = old_features; >+ bond_compute_features(bond); > > return res; > } >@@ -1979,6 +1954,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev) > struct bonding *bond = netdev_priv(bond_dev); > struct slave *slave, *oldcurrent; > struct sockaddr addr; >+ u32 old_features = bond_dev->features; > > /* slave is not a slave or master is not master of this slave */ > if (!(slave_dev->flags & IFF_SLAVE) || >@@ -2084,19 +2060,16 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev) > */ > memset(bond_dev->dev_addr, 0, bond_dev->addr_len); > >- if (!bond->vlgrp) { >- bond_dev->features |= NETIF_F_VLAN_CHALLENGED; >- } else { >+ if (bond->vlgrp) { > pr_warning("%s: Warning: clearing HW address of %s while it still has VLANs.\n", > bond_dev->name, bond_dev->name); > pr_warning("%s: When re-adding slaves, make sure the bond's HW address matches its VLANs'.\n", > bond_dev->name); > } >- } else if ((bond_dev->features & NETIF_F_VLAN_CHALLENGED) && >- !bond_has_challenged_slaves(bond)) { >+ } else if (!(bond_dev->features & NETIF_F_VLAN_CHALLENGED) && >+ old_features & NETIF_F_VLAN_CHALLENGED) { > pr_info("%s: last VLAN challenged slave %s left bond %s. VLAN blocking is removed\n", > bond_dev->name, slave_dev->name, bond_dev->name); >- bond_dev->features &= ~NETIF_F_VLAN_CHALLENGED; > } > > write_unlock_bh(&bond->lock); >@@ -2269,9 +2242,7 @@ static int bond_release_all(struct net_device *bond_dev) > */ > memset(bond_dev->dev_addr, 0, bond_dev->addr_len); > >- if (!bond->vlgrp) { >- bond_dev->features |= NETIF_F_VLAN_CHALLENGED; >- } else { >+ if (bond->vlgrp) { > pr_warning("%s: Warning: clearing HW address of %s while it still has VLANs.\n", > bond_dev->name, bond_dev->name); > pr_warning("%s: When re-adding slaves, make sure the bond's HW address matches its VLANs'.\n", >@@ -4347,11 +4318,6 @@ static void bond_ethtool_get_drvinfo(struct net_device *bond_dev, > static const struct ethtool_ops bond_ethtool_ops = { > .get_drvinfo = bond_ethtool_get_drvinfo, > .get_link = ethtool_op_get_link, >- .get_tx_csum = ethtool_op_get_tx_csum, >- .get_sg = ethtool_op_get_sg, >- .get_tso = ethtool_op_get_tso, >- .get_ufo = ethtool_op_get_ufo, >- .get_flags = ethtool_op_get_flags, > }; > > static const struct net_device_ops bond_netdev_ops = { >@@ -4377,6 +4343,7 @@ static const struct net_device_ops bond_netdev_ops = { > #endif > .ndo_add_slave = bond_enslave, > .ndo_del_slave = bond_release, >+ .ndo_fix_features = bond_fix_features, > }; > > static void bond_destructor(struct net_device *bond_dev) >@@ -4432,14 +4399,14 @@ static void bond_setup(struct net_device *bond_dev) > * when there are slaves that are not hw accel > * capable > */ >- bond_dev->features |= (NETIF_F_HW_VLAN_TX | >- NETIF_F_HW_VLAN_RX | >- NETIF_F_HW_VLAN_FILTER); > >- /* By default, we enable GRO on bonding devices. >- * Actual support requires lowlevel drivers are GRO ready. >- */ >- bond_dev->features |= NETIF_F_GRO; >+ bond_dev->hw_features = BOND_VLAN_FEATURES | >+ NETIF_F_HW_VLAN_TX | >+ NETIF_F_HW_VLAN_RX | >+ NETIF_F_HW_VLAN_FILTER; >+ >+ bond_dev->hw_features &= ~(NETIF_F_ALL_CSUM & ~NETIF_F_NO_CSUM); >+ bond_dev->features |= bond_dev->hw_features; > } > > static void bond_work_cancel_all(struct bonding *bond) >-- >1.7.2.5 > --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com -- 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 Fri, May 06, 2011 at 11:18:04AM -0700, Jay Vosburgh wrote: > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote: > >This should also fix updating of vlan_features and propagating changes to > >VLAN devices on the bond. > > > >Side effect: it allows user to force-disable some offloads on the bond > >interface. > > > >Note: NETIF_F_VLAN_CHALLENGED is managed by bond_fix_features() now. > > > >BTW, What are the problems in creating VLAN devices on an empty bond > >(as stated in one of bond_setup() comments)? > If there are no slaves, then the bond does not have a MAC > address assigned (because it gets its initial MAC from the first slave). > It's therefore impossible to pass a MAC address up to the VLAN > interface. > > So the limitation is that the bond must have at least one slave > before a VLAN may be configured above it. Hmm. That might be worked aroud by generating random MAC then. This would allow the user to first set a new MAC, create VLANs and then add slaves when they show up. Best Regards, Michał Mirosław -- 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 9a5feaf..04a2205 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -344,32 +344,6 @@ out: } /** - * bond_has_challenged_slaves - * @bond: the bond we're working on - * - * Searches the slave list. Returns 1 if a vlan challenged slave - * was found, 0 otherwise. - * - * Assumes bond->lock is held. - */ -static int bond_has_challenged_slaves(struct bonding *bond) -{ - struct slave *slave; - int i; - - bond_for_each_slave(bond, slave, i) { - if (slave->dev->features & NETIF_F_VLAN_CHALLENGED) { - pr_debug("found VLAN challenged slave - %s\n", - slave->dev->name); - return 1; - } - } - - pr_debug("no VLAN challenged slaves found\n"); - return 0; -} - -/** * bond_next_vlan - safely skip to the next item in the vlans list. * @bond: the bond we're working on * @curr: item we're advancing from @@ -1406,52 +1380,61 @@ static int bond_sethwaddr(struct net_device *bond_dev, return 0; } -#define BOND_VLAN_FEATURES \ - (NETIF_F_VLAN_CHALLENGED | NETIF_F_HW_VLAN_RX | NETIF_F_HW_VLAN_TX | \ - NETIF_F_HW_VLAN_FILTER) - -/* - * Compute the common dev->feature set available to all slaves. Some - * feature bits are managed elsewhere, so preserve those feature bits - * on the master device. - */ -static int bond_compute_features(struct bonding *bond) +static u32 bond_fix_features(struct net_device *dev, u32 features) { struct slave *slave; - struct net_device *bond_dev = bond->dev; - u32 features = bond_dev->features; - u32 vlan_features = 0; - unsigned short max_hard_header_len = max((u16)ETH_HLEN, - bond_dev->hard_header_len); + struct bonding *bond = netdev_priv(dev); + u32 mask; int i; - features &= ~(NETIF_F_ALL_CSUM | BOND_VLAN_FEATURES); - features |= NETIF_F_GSO_MASK | NETIF_F_NO_CSUM | NETIF_F_NOCACHE_COPY; - if (!bond->first_slave) - goto done; + /* Disable adding VLANs to empty bond. But why? --mq */ + return features | NETIF_F_VLAN_CHALLENGED; + mask = features; features &= ~NETIF_F_ONE_FOR_ALL; + features |= NETIF_F_ALL_FOR_ALL; - vlan_features = bond->first_slave->dev->vlan_features; bond_for_each_slave(bond, slave, i) { features = netdev_increment_features(features, slave->dev->features, - NETIF_F_ONE_FOR_ALL); + mask); + } + + return features; +} + +#define BOND_VLAN_FEATURES (NETIF_F_ALL_TX_OFFLOADS | \ + NETIF_F_SOFT_FEATURES | \ + NETIF_F_LRO) + +static void bond_compute_features(struct bonding *bond) +{ + struct slave *slave; + struct net_device *bond_dev = bond->dev; + u32 old_features, vlan_features = BOND_VLAN_FEATURES; + unsigned short max_hard_header_len = ETH_HLEN; + int i; + + if (!bond->first_slave) + goto done; + + bond_for_each_slave(bond, slave, i) { vlan_features = netdev_increment_features(vlan_features, - slave->dev->vlan_features, - NETIF_F_ONE_FOR_ALL); + slave->dev->vlan_features, BOND_VLAN_FEATURES); + if (slave->dev->hard_header_len > max_hard_header_len) max_hard_header_len = slave->dev->hard_header_len; } done: - features |= (bond_dev->features & BOND_VLAN_FEATURES); - bond_dev->features = netdev_fix_features(bond_dev, features); - bond_dev->vlan_features = netdev_fix_features(bond_dev, vlan_features); + bond_dev->vlan_features = vlan_features; bond_dev->hard_header_len = max_hard_header_len; - return 0; + old_features = bond_dev->features; + netdev_update_features(bond_dev); + if (old_features == bond_dev->features) + netdev_features_change(bond_dev); } static void bond_setup_by_slave(struct net_device *bond_dev, @@ -1544,7 +1527,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) struct netdev_hw_addr *ha; struct sockaddr addr; int link_reporting; - int old_features = bond_dev->features; int res = 0; if (!bond->params.use_carrier && slave_dev->ethtool_ops == NULL && @@ -1577,16 +1559,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) pr_warning("%s: Warning: enslaved VLAN challenged slave %s. Adding VLANs will be blocked as long as %s is part of bond %s\n", bond_dev->name, slave_dev->name, slave_dev->name, bond_dev->name); - bond_dev->features |= NETIF_F_VLAN_CHALLENGED; } } else { pr_debug("%s: ! NETIF_F_VLAN_CHALLENGED\n", slave_dev->name); - if (bond->slave_cnt == 0) { - /* First slave, and it is not VLAN challenged, - * so remove the block of adding VLANs over the bond. - */ - bond_dev->features &= ~NETIF_F_VLAN_CHALLENGED; - } } /* @@ -1958,7 +1933,7 @@ err_free: kfree(new_slave); err_undo_flags: - bond_dev->features = old_features; + bond_compute_features(bond); return res; } @@ -1979,6 +1954,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev) struct bonding *bond = netdev_priv(bond_dev); struct slave *slave, *oldcurrent; struct sockaddr addr; + u32 old_features = bond_dev->features; /* slave is not a slave or master is not master of this slave */ if (!(slave_dev->flags & IFF_SLAVE) || @@ -2084,19 +2060,16 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev) */ memset(bond_dev->dev_addr, 0, bond_dev->addr_len); - if (!bond->vlgrp) { - bond_dev->features |= NETIF_F_VLAN_CHALLENGED; - } else { + if (bond->vlgrp) { pr_warning("%s: Warning: clearing HW address of %s while it still has VLANs.\n", bond_dev->name, bond_dev->name); pr_warning("%s: When re-adding slaves, make sure the bond's HW address matches its VLANs'.\n", bond_dev->name); } - } else if ((bond_dev->features & NETIF_F_VLAN_CHALLENGED) && - !bond_has_challenged_slaves(bond)) { + } else if (!(bond_dev->features & NETIF_F_VLAN_CHALLENGED) && + old_features & NETIF_F_VLAN_CHALLENGED) { pr_info("%s: last VLAN challenged slave %s left bond %s. VLAN blocking is removed\n", bond_dev->name, slave_dev->name, bond_dev->name); - bond_dev->features &= ~NETIF_F_VLAN_CHALLENGED; } write_unlock_bh(&bond->lock); @@ -2269,9 +2242,7 @@ static int bond_release_all(struct net_device *bond_dev) */ memset(bond_dev->dev_addr, 0, bond_dev->addr_len); - if (!bond->vlgrp) { - bond_dev->features |= NETIF_F_VLAN_CHALLENGED; - } else { + if (bond->vlgrp) { pr_warning("%s: Warning: clearing HW address of %s while it still has VLANs.\n", bond_dev->name, bond_dev->name); pr_warning("%s: When re-adding slaves, make sure the bond's HW address matches its VLANs'.\n", @@ -4347,11 +4318,6 @@ static void bond_ethtool_get_drvinfo(struct net_device *bond_dev, static const struct ethtool_ops bond_ethtool_ops = { .get_drvinfo = bond_ethtool_get_drvinfo, .get_link = ethtool_op_get_link, - .get_tx_csum = ethtool_op_get_tx_csum, - .get_sg = ethtool_op_get_sg, - .get_tso = ethtool_op_get_tso, - .get_ufo = ethtool_op_get_ufo, - .get_flags = ethtool_op_get_flags, }; static const struct net_device_ops bond_netdev_ops = { @@ -4377,6 +4343,7 @@ static const struct net_device_ops bond_netdev_ops = { #endif .ndo_add_slave = bond_enslave, .ndo_del_slave = bond_release, + .ndo_fix_features = bond_fix_features, }; static void bond_destructor(struct net_device *bond_dev) @@ -4432,14 +4399,14 @@ static void bond_setup(struct net_device *bond_dev) * when there are slaves that are not hw accel * capable */ - bond_dev->features |= (NETIF_F_HW_VLAN_TX | - NETIF_F_HW_VLAN_RX | - NETIF_F_HW_VLAN_FILTER); - /* By default, we enable GRO on bonding devices. - * Actual support requires lowlevel drivers are GRO ready. - */ - bond_dev->features |= NETIF_F_GRO; + bond_dev->hw_features = BOND_VLAN_FEATURES | + NETIF_F_HW_VLAN_TX | + NETIF_F_HW_VLAN_RX | + NETIF_F_HW_VLAN_FILTER; + + bond_dev->hw_features &= ~(NETIF_F_ALL_CSUM & ~NETIF_F_NO_CSUM); + bond_dev->features |= bond_dev->hw_features; } static void bond_work_cancel_all(struct bonding *bond)
This should also fix updating of vlan_features and propagating changes to VLAN devices on the bond. Side effect: it allows user to force-disable some offloads on the bond interface. Note: NETIF_F_VLAN_CHALLENGED is managed by bond_fix_features() now. BTW, What are the problems in creating VLAN devices on an empty bond (as stated in one of bond_setup() comments)? Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> --- Note: This is only compile tested, yet. drivers/net/bonding/bond_main.c | 133 +++++++++++++++------------------------ 1 files changed, 50 insertions(+), 83 deletions(-)