[PATCH] bonding: convert to ndo_fix_features

[PATCH] bonding: convert to ndo_fix_features

[Michał Mirosław]

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(-)

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

Re: [PATCH] bonding: convert to ndo_fix_features

[Jay Vosburgh]

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

Re: [PATCH] bonding: convert to ndo_fix_features

[Michał Mirosław]

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

[PATCH v2] bonding: convert to ndo_fix_features

[Michał Mirosław]

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.                                      

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---

Note: Depends on netdev_change_features() patch.

v2:	- use netdev_change_features()
	- fix locking around bond_compute_features()

 drivers/net/bonding/bond_main.c |  161 ++++++++++++++++----------------------
 1 files changed, 68 insertions(+), 93 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 9a5feaf..36ff316 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,68 @@ 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;
+	read_lock(&bond->lock);
 
-	if (!bond->first_slave)
-		goto done;
+	if (!bond->first_slave) {
+		/* Disable adding VLANs to empty bond. But why? --mq */
+		features |= NETIF_F_VLAN_CHALLENGED;
+		goto out;
+	}
 
+	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);
+	}
+
+out:
+	read_unlock(&bond->lock);
+	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 vlan_features = BOND_VLAN_FEATURES;
+	unsigned short max_hard_header_len = ETH_HLEN;
+	int i;
+
+	read_lock(&bond->lock);
+
+	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;
+	read_unlock(&bond->lock);
+
+	netdev_change_features(bond_dev);
 }
 
 static void bond_setup_by_slave(struct net_device *bond_dev,
@@ -1544,7 +1534,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 +1566,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;
-		}
 	}
 
 	/*
@@ -1775,10 +1757,10 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 	new_slave->delay = 0;
 	new_slave->link_failure_count = 0;
 
-	bond_compute_features(bond);
-
 	write_unlock_bh(&bond->lock);
 
+	bond_compute_features(bond);
+
 	read_lock(&bond->lock);
 
 	new_slave->last_arp_rx = jiffies;
@@ -1958,7 +1940,7 @@ err_free:
 	kfree(new_slave);
 
 err_undo_flags:
-	bond_dev->features = old_features;
+	bond_compute_features(bond);
 
 	return res;
 }
@@ -1979,6 +1961,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) ||
@@ -2039,8 +2022,6 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
 	/* release the slave from its bond */
 	bond_detach_slave(bond, slave);
 
-	bond_compute_features(bond);
-
 	if (bond->primary_slave == slave)
 		bond->primary_slave = NULL;
 
@@ -2084,23 +2065,22 @@ 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)) {
+	}
+
+	write_unlock_bh(&bond->lock);
+	unblock_netpoll_tx();
+
+	bond_compute_features(bond);
+	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);
-	unblock_netpoll_tx();
 
 	/* must do this from outside any spinlocks */
 	bond_destroy_slave_symlinks(bond_dev, slave_dev);
@@ -2219,8 +2199,6 @@ static int bond_release_all(struct net_device *bond_dev)
 			bond_alb_deinit_slave(bond, slave);
 		}
 
-		bond_compute_features(bond);
-
 		bond_destroy_slave_symlinks(bond_dev, slave_dev);
 		bond_del_vlans_from_slave(bond, slave_dev);
 
@@ -2269,9 +2247,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",
@@ -2282,6 +2258,9 @@ static int bond_release_all(struct net_device *bond_dev)
 
 out:
 	write_unlock_bh(&bond->lock);
+
+	bond_compute_features(bond);
+
 	return 0;
 }
 
@@ -4347,11 +4326,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 +4351,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 +4407,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

Re: [PATCH v2] bonding: convert to ndo_fix_features

[David Miller]

From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Sat,  7 May 2011 15:22:17 +0200 (CEST)

> 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.                                      
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

Applied.