Attachment 253302 Details for Bug 438610

[patch] Patch for this bug.

bug#438610 (text/plain), 10.11 KB, created by Wei Kong on 2008-11-19 09:54:37 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Wei Kong

Created: 2008-11-19 09:54:37 UTC

Size: 10.11 KB

patch

obsolete

>http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=b63365a2d60268a3988285d6c3c6003d7066f93a
>
>net: Enable TSO if supported by at least one device
>
>didn't do what was intended because the netdev_compute_features
>function was designed for conjunctions.  So what happened was that
>it would simply take the TSO status of the last constituent device.
>
>This patch extends it to support both conjunctions and disjunctions
>under the new name of netdev_increment_features.
>
>It also adds a new function netdev_fix_features which does the
>sanity checking that usually occurs upon registration.  This ensures
>that the computation doesn't result in an illegal combination
>since this checking is absent when the change is initiated via
>ethtool.
>
>The two users of netdev_compute_features have been converted.
>
>Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>Signed-off-by: David S. Miller <davem@davemloft.net>
>
>drivers/net/bonding/bond_main.c
>include/linux/netdevice.h 
>net/bridge/br_device.c 	
>net/bridge/br_if.p
>net/core/dev.c 
>
>diff -uNrp linux-2.6.27.5-2.bak/drivers/net/bonding/bond_main.c linux-2.6.27.5-2/drivers/net/bonding/bond_main.c
>--- linux-2.6.27.5-2.bak/drivers/net/bonding/bond_main.c	2008-11-19 17:12:19.000000000 +0800
>+++ linux-2.6.27.5-2/drivers/net/bonding/bond_main.c	2008-11-19 17:16:15.000000000 +0800
>@@ -1341,18 +1341,24 @@ static int bond_compute_features(struct
> 	int i;
> 
> 	features &= ~(NETIF_F_ALL_CSUM | BOND_VLAN_FEATURES);
>-	features |= NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HIGHDMA |
>-		    NETIF_F_GSO_MASK | NETIF_F_NO_CSUM;
>+	features |=  NETIF_F_GSO_MASK | NETIF_F_NO_CSUM;
>+
>+	if (!bond->first_slave)
>+		goto done;
>+
>+	features &= ~NETIF_F_ONE_FOR_ALL;
> 
> 	bond_for_each_slave(bond, slave, i) {
>-		features = netdev_compute_features(features,
>-						   slave->dev->features);
>+		features = netdev_increment_features(features,
>+						     slave->dev->features,
>+						     NETIF_F_ONE_FOR_ALL);
> 		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 = features;
>+	bond_dev->features = netdev_fix_features(features, NULL);
> 	bond_dev->hard_header_len = max_hard_header_len;
> 
> 	return 0;
>diff -uNrp linux-2.6.27.5-2.bak/include/linux/netdevice.h linux-2.6.27.5-2/include/linux/netdevice.h
>--- linux-2.6.27.5-2.bak/include/linux/netdevice.h	2008-11-19 17:12:15.000000000 +0800
>+++ linux-2.6.27.5-2/include/linux/netdevice.h	2008-11-19 17:16:15.000000000 +0800
>@@ -541,6 +541,14 @@ struct net_device
> #define NETIF_F_V6_CSUM		(NETIF_F_GEN_CSUM | NETIF_F_IPV6_CSUM)
> #define NETIF_F_ALL_CSUM	(NETIF_F_V4_CSUM | NETIF_F_V6_CSUM)
> 
>+	/*
>+	 * If one device supports one of these features, then enable them
>+	 * for all in netdev_increment_features.
>+	 */
>+#define NETIF_F_ONE_FOR_ALL	(NETIF_F_GSO_SOFTWARE | NETIF_F_GSO_ROBUST | \
>+				 NETIF_F_SG | NETIF_F_HIGHDMA | \
>+				 NETIF_F_FRAGLIST)
>+
> 	/* Interface index. Unique device identifier	*/
> 	int			ifindex;
> 	int			iflink;
>@@ -1679,7 +1687,9 @@ extern char *netdev_drivername(struct ne
> 
> extern void linkwatch_run_queue(void);
> 
>-extern int netdev_compute_features(unsigned long all, unsigned long one);
>+unsigned long netdev_increment_features(unsigned long all, unsigned long one,
>+					unsigned long mask);
>+unsigned long netdev_fix_features(unsigned long features, const char *name);
> 
> static inline int net_gso_ok(int features, int gso_type)
> {
>diff -uNrp linux-2.6.27.5-2.bak/net/bridge/br_device.c linux-2.6.27.5-2/net/bridge/br_device.c
>--- linux-2.6.27.5-2.bak/net/bridge/br_device.c	2008-11-19 17:12:15.000000000 +0800
>+++ linux-2.6.27.5-2/net/bridge/br_device.c	2008-11-19 17:16:15.000000000 +0800
>@@ -178,5 +178,6 @@ void br_dev_setup(struct net_device *dev
> 	dev->priv_flags = IFF_EBRIDGE;
> 
> 	dev->features = NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HIGHDMA |
>-			NETIF_F_GSO_MASK | NETIF_F_NO_CSUM | NETIF_F_LLTX;
>+			NETIF_F_GSO_MASK | NETIF_F_NO_CSUM | NETIF_F_LLTX |
>+			NETIF_F_NETNS_LOCAL | NETIF_F_GSO;
> }
>diff -uNrp linux-2.6.27.5-2.bak/net/bridge/br_if.c linux-2.6.27.5-2/net/bridge/br_if.c
>--- linux-2.6.27.5-2.bak/net/bridge/br_if.c	2008-11-19 17:12:15.000000000 +0800
>+++ linux-2.6.27.5-2/net/bridge/br_if.c	2008-11-19 17:16:15.000000000 +0800
>@@ -355,15 +355,21 @@ int br_min_mtu(const struct net_bridge *
> void br_features_recompute(struct net_bridge *br)
> {
> 	struct net_bridge_port *p;
>-	unsigned long features;
>+	unsigned long features, mask;
> 
>-	features = br->feature_mask;
>+	features = mask = br->feature_mask;
>+	if (list_empty(&br->port_list))
>+		goto done;
>+
>+	features &= ~NETIF_F_ONE_FOR_ALL;
> 
> 	list_for_each_entry(p, &br->port_list, list) {
>-		features = netdev_compute_features(features, p->dev->features);
>+		features = netdev_increment_features(features,
>+						     p->dev->features, mask);
> 	}
> 
>-	br->dev->features = features;
>+done:
>+	br->dev->features = netdev_fix_features(features, NULL);
> }
> 
> /* called with RTNL */
>diff -uNrp linux-2.6.27.5-2.bak/net/core/dev.c linux-2.6.27.5-2/net/core/dev.c
>--- linux-2.6.27.5-2.bak/net/core/dev.c	2008-11-19 17:12:15.000000000 +0800
>+++ linux-2.6.27.5-2/net/core/dev.c	2008-11-19 17:16:15.000000000 +0800
>@@ -4019,6 +4019,46 @@ static void netdev_init_queue_locks(stru
> 	__netdev_init_queue_locks_one(dev, &dev->rx_queue, NULL);
> }
> 
>+unsigned long netdev_fix_features(unsigned long features, const char *name)
>+{
>+	/* Fix illegal SG+CSUM combinations. */
>+	if ((features & NETIF_F_SG) &&
>+	    !(features & NETIF_F_ALL_CSUM)) {
>+		if (name)
>+			printk(KERN_NOTICE "%s: Dropping NETIF_F_SG since no "
>+			       "checksum feature.\n", name);
>+		features &= ~NETIF_F_SG;
>+	}
>+
>+	/* TSO requires that SG is present as well. */
>+	if ((features & NETIF_F_TSO) && !(features & NETIF_F_SG)) {
>+		if (name)
>+			printk(KERN_NOTICE "%s: Dropping NETIF_F_TSO since no "
>+			       "SG feature.\n", name);
>+		features &= ~NETIF_F_TSO;
>+	}
>+
>+	if (features & NETIF_F_UFO) {
>+		if (!(features & NETIF_F_GEN_CSUM)) {
>+			if (name)
>+				printk(KERN_ERR "%s: Dropping NETIF_F_UFO "
>+				       "since no NETIF_F_HW_CSUM feature.\n",
>+				       name);
>+			features &= ~NETIF_F_UFO;
>+		}
>+
>+		if (!(features & NETIF_F_SG)) {
>+			if (name)
>+				printk(KERN_ERR "%s: Dropping NETIF_F_UFO "
>+				       "since no NETIF_F_SG feature.\n", name);
>+			features &= ~NETIF_F_UFO;
>+		}
>+	}
>+
>+	return features;
>+}
>+EXPORT_SYMBOL(netdev_fix_features);
>+
> /**
>  *	register_netdevice	- register a network device
>  *	@dev: device to register
>@@ -4104,36 +4144,7 @@ int register_netdevice(struct net_device
> 		dev->features &= ~(NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM|NETIF_F_HW_CSUM);
> 	}
> 
>-
>-	/* Fix illegal SG+CSUM combinations. */
>-	if ((dev->features & NETIF_F_SG) &&
>-	    !(dev->features & NETIF_F_ALL_CSUM)) {
>-		printk(KERN_NOTICE "%s: Dropping NETIF_F_SG since no checksum feature.\n",
>-		       dev->name);
>-		dev->features &= ~NETIF_F_SG;
>-	}
>-
>-	/* TSO requires that SG is present as well. */
>-	if ((dev->features & NETIF_F_TSO) &&
>-	    !(dev->features & NETIF_F_SG)) {
>-		printk(KERN_NOTICE "%s: Dropping NETIF_F_TSO since no SG feature.\n",
>-		       dev->name);
>-		dev->features &= ~NETIF_F_TSO;
>-	}
>-	if (dev->features & NETIF_F_UFO) {
>-		if (!(dev->features & NETIF_F_HW_CSUM)) {
>-			printk(KERN_ERR "%s: Dropping NETIF_F_UFO since no "
>-					"NETIF_F_HW_CSUM feature.\n",
>-							dev->name);
>-			dev->features &= ~NETIF_F_UFO;
>-		}
>-		if (!(dev->features & NETIF_F_SG)) {
>-			printk(KERN_ERR "%s: Dropping NETIF_F_UFO since no "
>-					"NETIF_F_SG feature.\n",
>-					dev->name);
>-			dev->features &= ~NETIF_F_UFO;
>-		}
>-	}
>+	dev->features = netdev_fix_features(dev->features, dev->name);
> 
> 	/* Enable software GSO if SG is supported. */
> 	if (dev->features & NETIF_F_SG)
>@@ -4767,43 +4778,45 @@ static int __init netdev_dma_register(vo
> #endif /* CONFIG_NET_DMA */
> 
> /**
>- *	netdev_compute_feature - compute conjunction of two feature sets
>- *	@all: first feature set
>- *	@one: second feature set
>+ *	netdev_increment_features - increment feature set by one
>+ *	@all: current feature set
>+ *	@one: new feature set
>+ *	@mask: mask feature set
>  *
>  *	Computes a new feature set after adding a device with feature set
>- *	@one to the master device with current feature set @all.  Returns
>- *	the new feature set.
>+ *	@one to the master device with current feature set @all.  Will not
>+ *	enable anything that is off in @mask. Returns the new feature set.
>  */
>-int netdev_compute_features(unsigned long all, unsigned long one)
>-{
>-	/* if device needs checksumming, downgrade to hw checksumming */
>-	if (all & NETIF_F_NO_CSUM && !(one & NETIF_F_NO_CSUM))
>-		all ^= NETIF_F_NO_CSUM | NETIF_F_HW_CSUM;
>-
>-	/* if device can't do all checksum, downgrade to ipv4/ipv6 */
>-	if (all & NETIF_F_HW_CSUM && !(one & NETIF_F_HW_CSUM))
>-		all ^= NETIF_F_HW_CSUM
>-			| NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
>-
>-	if (one & NETIF_F_GSO)
>-		one |= NETIF_F_GSO_SOFTWARE;
>-	one |= NETIF_F_GSO;
>+unsigned long netdev_increment_features(unsigned long all, unsigned long one,
>+					unsigned long mask)
>+{
>+	/* If device needs checksumming, downgrade to it. */
>+        if (all & NETIF_F_NO_CSUM && !(one & NETIF_F_NO_CSUM))
>+		all ^= NETIF_F_NO_CSUM | (one & NETIF_F_ALL_CSUM);
>+	else if (mask & NETIF_F_ALL_CSUM) {
>+		/* If one device supports v4/v6 checksumming, set for all. */
>+		if (one & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM) &&
>+		    !(all & NETIF_F_GEN_CSUM)) {
>+			all &= ~NETIF_F_ALL_CSUM;
>+			all |= one & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM);
>+		}
> 
>-	/* If even one device supports robust GSO, enable it for all. */
>-	if (one & NETIF_F_GSO_ROBUST)
>-		all |= NETIF_F_GSO_ROBUST;
>+		/* If one device supports hw checksumming, set for all. */
>+		if (one & NETIF_F_GEN_CSUM && !(all & NETIF_F_GEN_CSUM)) {
>+			all &= ~NETIF_F_ALL_CSUM;
>+			all |= NETIF_F_HW_CSUM;
>+		}
>+	}
> 
>-	all &= one | NETIF_F_LLTX;
>+	one |= NETIF_F_ALL_CSUM;
> 
>-	if (!(all & NETIF_F_ALL_CSUM))
>-		all &= ~NETIF_F_SG;
>-	if (!(all & NETIF_F_SG))
>-		all &= ~NETIF_F_GSO_MASK;
>+	one |= all & NETIF_F_ONE_FOR_ALL;
>+	all &= one | NETIF_F_LLTX | NETIF_F_GSO;
>+	all |= one & mask & NETIF_F_ONE_FOR_ALL;
> 
> 	return all;
> }
>-EXPORT_SYMBOL(netdev_compute_features);
>+EXPORT_SYMBOL(netdev_increment_features);
> 
> static struct hlist_head *netdev_create_hash(void)
> {

Actions: View | Diff

Attachments on bug 438610: 247766 | 253302 | 256043 | 256044