[PATCH] [BRIDGE]

[PATCH] [BRIDGE]

[Catalin(ux aka Dino) BOIE]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 384 bytes --]

Hello!

This patch set bridge device features using the common features of all 
slaves. Also, it reacts to features change using ethtool on the slaves.

This patch depends on NETDEV_FEAT_CHANGE.patch.

David, please apply if it looks good.

Thank you.

Signed-off-by: Catalin BOIE <catab at umrella.ro>
---
Catalin(ux aka Dino) BOIE
catab at deuroconsult.ro
http://kernel.umbrella.ro/

[-- Attachment #2: Type: TEXT/PLAIN, Size: 2886 bytes --]

--- bridge1/net/bridge/br_private.h	2005-03-02 09:37:50.000000000 +0200
+++ linux/net/bridge/br_private.h	2005-05-23 11:11:22.000000000 +0300
@@ -27,6 +27,10 @@
 #define BR_PORT_BITS	10
 #define BR_MAX_PORTS	(1<<BR_PORT_BITS)
 
+#define BR_FEAT_MASK	(NETIF_F_SG \
+			| NETIF_F_FRAGLIST \
+			| NETIF_F_HIGHDMA | NETIF_F_TSO)
+
 typedef struct bridge_id bridge_id;
 typedef struct mac_addr mac_addr;
 typedef __u16 port_id;
@@ -174,6 +178,7 @@ extern int br_add_if(struct net_bridge *
 extern int br_del_if(struct net_bridge *br,
 	      struct net_device *dev);
 extern int br_min_mtu(const struct net_bridge *br);
+extern void br_features_recompute(struct net_bridge *br);
 
 /* br_input.c */
 extern int br_handle_frame_finish(struct sk_buff *skb);
--- bridge1/net/bridge/br_notify.c	2005-03-26 05:28:14.000000000 +0200
+++ linux/net/bridge/br_notify.c	2005-05-23 11:20:14.000000000 +0300
@@ -65,6 +65,10 @@ static int br_device_event(struct notifi
 		}
 		break;
 
+	case NETDEV_FEAT_CHANGE:
+		br_features_recompute(br);
+		break;
+
 	case NETDEV_DOWN:
 		if (br->dev->flags & IFF_UP)
 			br_stp_disable_port(p);
--- bridge1/net/bridge/br_device.c	2005-03-02 09:37:30.000000000 +0200
+++ linux/net/bridge/br_device.c	2005-05-20 08:53:37.000000000 +0300
@@ -107,4 +107,5 @@ void br_dev_setup(struct net_device *dev
 	dev->tx_queue_len = 0;
 	dev->set_mac_address = NULL;
 	dev->priv_flags = IFF_EBRIDGE;
+	dev->features = BR_FEAT_MASK | NETIF_F_NO_CSUM | NETIF_F_LLTX;
 }
--- bridge1/net/bridge/br_if.c	2005-03-02 09:38:33.000000000 +0200
+++ linux/net/bridge/br_if.c	2005-05-20 09:18:29.000000000 +0300
@@ -314,6 +314,27 @@ int br_min_mtu(const struct net_bridge *
 	return mtu;
 }
 
+/*
+ * If slave device (@dev) doesn't support special features,
+ * turn them off globally.
+ */
+void br_features_change(struct net_bridge *br, struct net_device *dev)
+{
+	br->dev->features &= dev->features | ~BR_FEAT_MASK;
+}
+
+/*
+ * Recomputes features using slave's features
+ */
+void br_features_recompute(struct net_bridge *br)
+{
+	struct net_bridge_port *p;
+
+	br->dev->features |= BR_FEAT_MASK;
+	list_for_each_entry(p, &br->port_list, list)
+		br_features_change(br, p->dev);
+}
+
 /* called with RTNL */
 int br_add_if(struct net_bridge *br, struct net_device *dev)
 {
@@ -332,9 +353,10 @@ int br_add_if(struct net_bridge *br, str
 	if (IS_ERR(p = new_nbp(br, dev, br_initial_port_cost(dev))))
 		return PTR_ERR(p);
 
+	br_features_change(br, dev);
+
  	if ((err = br_fdb_insert(br, p, dev->dev_addr, 1)))
 		destroy_nbp(p);
- 
 	else if ((err = br_sysfs_addif(p)))
 		del_nbp(p);
 	else {
@@ -368,6 +390,7 @@ int br_del_if(struct net_bridge *br, str
 
 	spin_lock_bh(&br->lock);
 	br_stp_recalculate_bridge_id(br);
+	br_features_recompute(br);
 	spin_unlock_bh(&br->lock);
 
 	return 0;

Re: [PATCH] [BRIDGE]

[Stephen Hemminger]

On Tue, 24 May 2005 10:29:25 +0300 (EEST)
"Catalin(ux aka Dino) BOIE" <util@deuroconsult.ro> wrote:

> Hello!
> 
> This patch set bridge device features using the common features of all 
> slaves. Also, it reacts to features change using ethtool on the slaves.
> 
> This patch depends on NETDEV_FEAT_CHANGE.patch.
> 

I will add it to the next set of patches for bridging. Hold off until
2.6.13

Re: [PATCH] [BRIDGE]

[Stephen Hemminger]

Here is the revised version, it deals with checksum mixtures better and
ignores stuff until bridge is up.

Index: bridge/net/bridge/br_private.h
===================================================================
--- bridge.orig/net/bridge/br_private.h
+++ bridge/net/bridge/br_private.h
@@ -174,6 +174,7 @@ extern int br_add_if(struct net_bridge *
 extern int br_del_if(struct net_bridge *br,
 	      struct net_device *dev);
 extern int br_min_mtu(const struct net_bridge *br);
+extern void br_features_recompute(struct net_bridge *br);
 
 /* br_input.c */
 extern int br_handle_frame_finish(struct sk_buff *skb);
Index: bridge/net/bridge/br_notify.c
===================================================================
--- bridge.orig/net/bridge/br_notify.c
+++ bridge/net/bridge/br_notify.c
@@ -65,6 +65,15 @@ static int br_device_event(struct notifi
 		}
 		break;
 
+	case NETDEV_FEAT_CHANGE:
+		if (br->dev->flags & IFF_UP) 
+			br_features_recompute(br);
+
+		/* could do recursive feature change notification
+		 * but who would care?? 
+		 */
+		break;
+
 	case NETDEV_DOWN:
 		if (br->dev->flags & IFF_UP)
 			br_stp_disable_port(p);
Index: bridge/net/bridge/br_device.c
===================================================================
--- bridge.orig/net/bridge/br_device.c
+++ bridge/net/bridge/br_device.c
@@ -21,10 +21,7 @@
 
 static struct net_device_stats *br_dev_get_stats(struct net_device *dev)
 {
-	struct net_bridge *br;
-
-	br = dev->priv;
-
+	struct net_bridge *br = netdev_priv(dev);
 	return &br->statistics;
 }
 
@@ -54,9 +51,11 @@ int br_dev_xmit(struct sk_buff *skb, str
 
 static int br_dev_open(struct net_device *dev)
 {
-	netif_start_queue(dev);
+	struct net_bridge *br = netdev_priv(dev);
 
-	br_stp_enable_bridge(dev->priv);
+	br_features_recompute(br);
+	netif_start_queue(dev);
+	br_stp_enable_bridge(br);
 
 	return 0;
 }
@@ -67,7 +66,7 @@ static void br_dev_set_multicast_list(st
 
 static int br_dev_stop(struct net_device *dev)
 {
-	br_stp_disable_bridge(dev->priv);
+	br_stp_disable_bridge(netdev_priv(dev));
 
 	netif_stop_queue(dev);
 
@@ -76,7 +75,7 @@ static int br_dev_stop(struct net_device
 
 static int br_change_mtu(struct net_device *dev, int new_mtu)
 {
-	if ((new_mtu < 68) || new_mtu > br_min_mtu(dev->priv))
+	if (new_mtu < 68 || new_mtu > br_min_mtu(netdev_priv(dev)))
 		return -EINVAL;
 
 	dev->mtu = new_mtu;
Index: bridge/net/bridge/br_if.c
===================================================================
--- bridge.orig/net/bridge/br_if.c
+++ bridge/net/bridge/br_if.c
@@ -314,6 +314,28 @@ int br_min_mtu(const struct net_bridge *
 	return mtu;
 }
 
+/*
+ * Recomputes features using slave's features
+ */
+void br_features_recompute(struct net_bridge *br)
+{
+	struct net_bridge_port *p;
+	unsigned long features, checksum;
+
+	features = NETIF_F_SG | NETIF_F_FRAGLIST 
+		| NETIF_F_HIGHDMA | NETIF_F_TSO;
+	checksum = NETIF_F_IP_CSUM;	/* least commmon subset */
+
+	list_for_each_entry(p, &br->port_list, list) {
+		if (!(p->dev->features 
+		      & (NETIF_F_IP_CSUM|NETIF_F_NO_CSUM|NETIF_F_HW_CSUM)))
+			checksum = 0;
+		features &= p->dev->features;
+	}
+
+	br->dev->features = features | checksum | NETIF_F_LLTX;
+}
+
 /* called with RTNL */
 int br_add_if(struct net_bridge *br, struct net_device *dev)
 {
@@ -368,6 +390,7 @@ int br_del_if(struct net_bridge *br, str
 
 	spin_lock_bh(&br->lock);
 	br_stp_recalculate_bridge_id(br);
+	br_features_recompute(br);
 	spin_unlock_bh(&br->lock);
 
 	return 0;

Re: [PATCH] [BRIDGE]

[Catalin(ux aka Dino) BOIE]

On Wed, 25 May 2005, Stephen Hemminger wrote:

> Here is the revised version, it deals with checksum mixtures better and
> ignores stuff until bridge is up.

This patch missing the case when we have HW_CSUM (for example) on the 
bridge device, and a new device that has no HW_CSUM is added.
The fix is to call br_features_recompute(br) also in br_add_if().

Thanks.

> Index: bridge/net/bridge/br_private.h
> ===================================================================
> --- bridge.orig/net/bridge/br_private.h
> +++ bridge/net/bridge/br_private.h
> @@ -174,6 +174,7 @@ extern int br_add_if(struct net_bridge *
> extern int br_del_if(struct net_bridge *br,
> 	      struct net_device *dev);
> extern int br_min_mtu(const struct net_bridge *br);
> +extern void br_features_recompute(struct net_bridge *br);
>
> /* br_input.c */
> extern int br_handle_frame_finish(struct sk_buff *skb);
> Index: bridge/net/bridge/br_notify.c
> ===================================================================
> --- bridge.orig/net/bridge/br_notify.c
> +++ bridge/net/bridge/br_notify.c
> @@ -65,6 +65,15 @@ static int br_device_event(struct notifi
> 		}
> 		break;
>
> +	case NETDEV_FEAT_CHANGE:
> +		if (br->dev->flags & IFF_UP)
> +			br_features_recompute(br);
> +
> +		/* could do recursive feature change notification
> +		 * but who would care??
> +		 */
> +		break;
> +
> 	case NETDEV_DOWN:
> 		if (br->dev->flags & IFF_UP)
> 			br_stp_disable_port(p);
> Index: bridge/net/bridge/br_device.c
> ===================================================================
> --- bridge.orig/net/bridge/br_device.c
> +++ bridge/net/bridge/br_device.c
> @@ -21,10 +21,7 @@
>
> static struct net_device_stats *br_dev_get_stats(struct net_device *dev)
> {
> -	struct net_bridge *br;
> -
> -	br = dev->priv;
> -
> +	struct net_bridge *br = netdev_priv(dev);
> 	return &br->statistics;
> }
>
> @@ -54,9 +51,11 @@ int br_dev_xmit(struct sk_buff *skb, str
>
> static int br_dev_open(struct net_device *dev)
> {
> -	netif_start_queue(dev);
> +	struct net_bridge *br = netdev_priv(dev);
>
> -	br_stp_enable_bridge(dev->priv);
> +	br_features_recompute(br);
> +	netif_start_queue(dev);
> +	br_stp_enable_bridge(br);
>
> 	return 0;
> }
> @@ -67,7 +66,7 @@ static void br_dev_set_multicast_list(st
>
> static int br_dev_stop(struct net_device *dev)
> {
> -	br_stp_disable_bridge(dev->priv);
> +	br_stp_disable_bridge(netdev_priv(dev));
>
> 	netif_stop_queue(dev);
>
> @@ -76,7 +75,7 @@ static int br_dev_stop(struct net_device
>
> static int br_change_mtu(struct net_device *dev, int new_mtu)
> {
> -	if ((new_mtu < 68) || new_mtu > br_min_mtu(dev->priv))
> +	if (new_mtu < 68 || new_mtu > br_min_mtu(netdev_priv(dev)))
> 		return -EINVAL;
>
> 	dev->mtu = new_mtu;
> Index: bridge/net/bridge/br_if.c
> ===================================================================
> --- bridge.orig/net/bridge/br_if.c
> +++ bridge/net/bridge/br_if.c
> @@ -314,6 +314,28 @@ int br_min_mtu(const struct net_bridge *
> 	return mtu;
> }
>
> +/*
> + * Recomputes features using slave's features
> + */
> +void br_features_recompute(struct net_bridge *br)
> +{
> +	struct net_bridge_port *p;
> +	unsigned long features, checksum;
> +
> +	features = NETIF_F_SG | NETIF_F_FRAGLIST
> +		| NETIF_F_HIGHDMA | NETIF_F_TSO;
> +	checksum = NETIF_F_IP_CSUM;	/* least commmon subset */
> +
> +	list_for_each_entry(p, &br->port_list, list) {
> +		if (!(p->dev->features
> +		      & (NETIF_F_IP_CSUM|NETIF_F_NO_CSUM|NETIF_F_HW_CSUM)))
> +			checksum = 0;
> +		features &= p->dev->features;
> +	}
> +
> +	br->dev->features = features | checksum | NETIF_F_LLTX;
> +}
> +
> /* called with RTNL */
> int br_add_if(struct net_bridge *br, struct net_device *dev)
> {
> @@ -368,6 +390,7 @@ int br_del_if(struct net_bridge *br, str
>
> 	spin_lock_bh(&br->lock);
> 	br_stp_recalculate_bridge_id(br);
> +	br_features_recompute(br);
> 	spin_unlock_bh(&br->lock);
>
> 	return 0;
>

---
Catalin(ux aka Dino) BOIE
catab at deuroconsult.ro
http://kernel.umbrella.ro/

Re: [PATCH] [BRIDGE]

[Catalin(ux aka Dino) BOIE]

On Wed, 25 May 2005, Stephen Hemminger wrote:

> On Tue, 24 May 2005 10:29:25 +0300 (EEST)
> "Catalin(ux aka Dino) BOIE" <util@deuroconsult.ro> wrote:
>
>> Hello!
>>
>> This patch set bridge device features using the common features of all
>> slaves. Also, it reacts to features change using ethtool on the slaves.
>>
>> This patch depends on NETDEV_FEAT_CHANGE.patch.
>>
>
> I will add it to the next set of patches for bridging. Hold off until
> 2.6.13

Thanks, Stephen!

P.S. This patch needs NETDEV_FEAT_CHANGE.patch. Do you include both?

---
Catalin(ux aka Dino) BOIE
catab at deuroconsult.ro
http://kernel.umbrella.ro/