[PATCH net-next] netxen: write IP address to firmware when using bonding

[PATCH net-next] netxen: write IP address to firmware when using bonding

[Andy Gospodarek]

The following patch was added to enable NX3031 devices to properly
aggregate frames for LRO:

	commit 6598b169b856793f8f9b80a3f3c5a48f5eaf40e3
	Author: Dhananjay Phadke <dhananjay@netxen.com>
	Date:   Sun Jul 26 20:07:37 2009 +0000

	    netxen: enable ip addr hashing

This patch is a followup to that fix as it allows LRO aggregation on
bonded devices that contain an NX3031 device.  This was tested on an
older distro and modified slightly to the latest upstream.

Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
---
 .../net/ethernet/qlogic/netxen/netxen_nic_main.c   |  119 +++++++++++++++-----
 1 files changed, 92 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
index 7dd9a4b..64eb618 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
@@ -3038,18 +3038,45 @@ recheck:
 		goto recheck;
 	}
 
-	if (!is_netxen_netdev(dev))
-		goto done;
+	/* If this is a bonding device, look for netxen-based slaves*/
+	if (dev->priv_flags & IFF_BONDING) {
+		struct net_device *slave;
 
-	adapter = netdev_priv(dev);
+		rcu_read_lock();
+		for_each_netdev_rcu(&init_net, slave) {
+			/* check to see if the device is in the bond */
+			if (slave->master == dev) {
 
-	if (!adapter)
-		goto done;
+				if (!is_netxen_netdev(slave))
+					continue;
 
-	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
-		goto done;
+				adapter = netdev_priv(slave);
+
+				if (!adapter)
+					continue;
+
+				if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
+					continue;
+
+				netxen_config_indev_addr(adapter, orig_dev, event);
+			}
+		}
+		rcu_read_unlock();
+
+	} else {
+		if (!is_netxen_netdev(dev))
+			goto done;
+
+		adapter = netdev_priv(dev);
 
-	netxen_config_indev_addr(adapter, orig_dev, event);
+		if (!adapter)
+			goto done;
+
+		if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
+			goto done;
+
+		netxen_config_indev_addr(adapter, orig_dev, event);
+	}
 done:
 	return NOTIFY_DONE;
 }
@@ -3074,30 +3101,68 @@ recheck:
 		goto recheck;
 	}
 
-	if (!is_netxen_netdev(dev))
-		goto done;
+	/* If this is a bonding device, look for netxen-based slaves*/
+	if (dev->priv_flags & IFF_BONDING) {
+		struct net_device *slave;
 
-	adapter = netdev_priv(dev);
+		rcu_read_lock();
+		for_each_netdev_rcu(&init_net, slave) {
+			/* check to see if the device is in the bond */
+			if (slave->master == dev) {
 
-	if (!adapter || !netxen_destip_supported(adapter))
-		goto done;
+				if (!is_netxen_netdev(slave))
+					continue;
 
-	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
-		goto done;
+				adapter = netdev_priv(slave);
 
-	switch (event) {
-	case NETDEV_UP:
-		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP);
-		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
-		break;
-	case NETDEV_DOWN:
-		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN);
-		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
-		break;
-	default:
-		break;
-	}
+				if (!adapter || !netxen_destip_supported(adapter))
+					continue;
+
+				if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
+					continue;
+
+				switch (event) {
+				case NETDEV_UP:
+					netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP);
+					netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
+					break;
+				case NETDEV_DOWN:
+					netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN);
+					netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
+					break;
+				default:
+					break;
+				}
+			}
+		}
+		rcu_read_unlock();
+
+	} else {
+
+		if (!is_netxen_netdev(dev))
+			goto done;
+
+		adapter = netdev_priv(dev);
+
+		if (!adapter || !netxen_destip_supported(adapter))
+			goto done;
 
+		if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
+			goto done;
+
+		switch (event) {
+		case NETDEV_UP:
+			netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP);
+			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
+			break;
+		case NETDEV_DOWN:
+			netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN);
+			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
+			break;
+		default:
+			break;
+		}
+	}
 done:
 	return NOTIFY_DONE;
 }
-- 
1.7.4.4

Re: [PATCH net-next] netxen: write IP address to firmware when using bonding

[David Miller]

From: Andy Gospodarek <andy@greyhouse.net>
Date: Wed, 23 Nov 2011 13:43:11 -0500

> The following patch was added to enable NX3031 devices to properly
> aggregate frames for LRO:
> 
> 	commit 6598b169b856793f8f9b80a3f3c5a48f5eaf40e3
> 	Author: Dhananjay Phadke <dhananjay@netxen.com>
> 	Date:   Sun Jul 26 20:07:37 2009 +0000
> 
> 	    netxen: enable ip addr hashing
> 
> This patch is a followup to that fix as it allows LRO aggregation on
> bonded devices that contain an NX3031 device.  This was tested on an
> older distro and modified slightly to the latest upstream.
> 
> Signed-off-by: Andy Gospodarek <andy@greyhouse.net>

Let's put a helper function somewhere instead of expanding this sequence
in every driver that might want to determine if it is a bonding slave.

Re: [PATCH net-next] netxen: write IP address to firmware when using bonding

[Andy Gospodarek]

On Wed, Nov 23, 2011 at 04:09:22PM -0500, David Miller wrote:
> From: Andy Gospodarek <andy@greyhouse.net>
> Date: Wed, 23 Nov 2011 13:43:11 -0500
> 
> > The following patch was added to enable NX3031 devices to properly
> > aggregate frames for LRO:
> > 
> > 	commit 6598b169b856793f8f9b80a3f3c5a48f5eaf40e3
> > 	Author: Dhananjay Phadke <dhananjay@netxen.com>
> > 	Date:   Sun Jul 26 20:07:37 2009 +0000
> > 
> > 	    netxen: enable ip addr hashing
> > 
> > This patch is a followup to that fix as it allows LRO aggregation on
> > bonded devices that contain an NX3031 device.  This was tested on an
> > older distro and modified slightly to the latest upstream.
> > 
> > Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
> 
> Let's put a helper function somewhere instead of expanding this sequence
> in every driver that might want to determine if it is a bonding slave.

Dave,

Are you talking about adding a macro like this:

	for_each_dev_in_bond(bond,slave) {
		[...]
	}

to replace the statements I added that were like this:

	for_each_netdev_rcu(&init_net, slave) {
		if (slave->master == dev) {
			[...]
		}
	}

If so, that totally seems reasonable.  If were requesting something
else, please let me know.

Thanks,

-andy

Re: [PATCH net-next] netxen: write IP address to firmware when using bonding

[David Miller]

From: Andy Gospodarek <andy@greyhouse.net>
Date: Wed, 23 Nov 2011 22:24:27 -0500

> Are you talking about adding a macro like this:
> 
> 	for_each_dev_in_bond(bond,slave) {
> 		[...]
> 	}
> 
> to replace the statements I added that were like this:
> 
> 	for_each_netdev_rcu(&init_net, slave) {
> 		if (slave->master == dev) {
> 			[...]
> 		}
> 	}
> 
> If so, that totally seems reasonable.  If were requesting something
> else, please let me know.

Yes, some helper that walks the device list and tries to find
a device whose ->master matches 'dev'.

RE: [PATCH net-next] netxen: write IP address to firmware when using bonding

[Rajesh Borundia]

Hi Andy,

We need to restore the ip address after the adapter reset.
netxen_restore_indev_addr is the function that restores normal ip addresses
and vlan ip addresses. If we could find bond device directly from slave then 
we can use netxen_config_indev_addr to add the ip address of the bond device.
Otherwise we may need to cache the bond ip address in function netxen_list_config_vlan_ip
and change the condition from

if (!is_vlan_dev(dev))
                return;

to

if (!is_vlan_dev(dev) && !is_bond_dev(dev))
                return;


Some of the code in if and else part is repeated.
If possible can we have small functions for that ?
eg:
if (!is_netxen_netdev(dev))
+                       goto done;
+
+               adapter = netdev_priv(dev);
+               if (!adapter)
+                       goto done;
+
+               if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
+                       goto done;
+
+               netxen_config_indev_addr(adapter, orig_dev, event);


Rajesh
________________________________________
From: David Miller [davem@davemloft.net]
Sent: Thursday, November 24, 2011 11:08 AM
To: andy@greyhouse.net
Cc: netdev; Sony Chacko; Rajesh Borundia
Subject: Re: [PATCH net-next] netxen: write IP address to firmware when using bonding

From: Andy Gospodarek <andy@greyhouse.net>
Date: Wed, 23 Nov 2011 22:24:27 -0500

> Are you talking about adding a macro like this:
>
>       for_each_dev_in_bond(bond,slave) {
>               [...]
>       }
>
> to replace the statements I added that were like this:
>
>       for_each_netdev_rcu(&init_net, slave) {
>               if (slave->master == dev) {
>                       [...]
>               }
>       }
>
> If so, that totally seems reasonable.  If were requesting something
> else, please let me know.

Yes, some helper that walks the device list and tries to find
a device whose ->master matches 'dev'.

Re: [PATCH net-next] netxen: write IP address to firmware when using bonding

[Andy Gospodarek]

On Mon, Nov 28, 2011 at 12:02:28AM -0600, Rajesh Borundia wrote:
> Hi Andy,
> 
> We need to restore the ip address after the adapter reset.
> netxen_restore_indev_addr is the function that restores normal ip addresses
> and vlan ip addresses. If we could find bond device directly from slave then 
> we can use netxen_config_indev_addr to add the ip address of the bond device.
> Otherwise we may need to cache the bond ip address in function netxen_list_config_vlan_ip
> and change the condition from
> 
> if (!is_vlan_dev(dev))
>                 return;
> 
> to
> 
> if (!is_vlan_dev(dev) && !is_bond_dev(dev))
>                 return;
> 

I don't think this was what you were originally intending, but what
about changing the name of the vlan_ip_list (and the vlan functions that
modify/read it) to something different and use it for all virtual or
stacked devices?

Something like 'virtual' or 'virt' might work instead of 'vlan.'
Something else that denotes that a device at a higher layer than just
the physical device has an IP address that we need to know about, but
that the device is not specific to VLANs.

> Some of the code in if and else part is repeated.
> If possible can we have small functions for that ?
> eg:
> if (!is_netxen_netdev(dev))
> +                       goto done;
> +
> +               adapter = netdev_priv(dev);
> +               if (!adapter)
> +                       goto done;
> +
> +               if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
> +                       goto done;
> +
> +               netxen_config_indev_addr(adapter, orig_dev, event);
> 
> 

I could change that.  I started to look at sharing some code, but
decided against too much initial code churn.  Since the fundamental idea
behind this chance seems acceptable, I can take a look at it more
closely.

I'll take a look at this and send a v2 of this patch.


> Rajesh
> ________________________________________
> From: David Miller [davem@davemloft.net]
> Sent: Thursday, November 24, 2011 11:08 AM
> To: andy@greyhouse.net
> Cc: netdev; Sony Chacko; Rajesh Borundia
> Subject: Re: [PATCH net-next] netxen: write IP address to firmware when using bonding
> 
> From: Andy Gospodarek <andy@greyhouse.net>
> Date: Wed, 23 Nov 2011 22:24:27 -0500
> 
> > Are you talking about adding a macro like this:
> >
> >       for_each_dev_in_bond(bond,slave) {
> >               [...]
> >       }
> >
> > to replace the statements I added that were like this:
> >
> >       for_each_netdev_rcu(&init_net, slave) {
> >               if (slave->master == dev) {
> >                       [...]
> >               }
> >       }
> >
> > If so, that totally seems reasonable.  If were requesting something
> > else, please let me know.
> 
> Yes, some helper that walks the device list and tries to find
> a device whose ->master matches 'dev'.
> 
> 
> --
> 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

RE: [PATCH net-next] netxen: write IP address to firmware when using bonding

[Rajesh Borundia]

Yes virt would be fine.

Thanks,
Rajesh
________________________________________
From: Andy Gospodarek [andy@greyhouse.net]
Sent: Tuesday, November 29, 2011 2:13 AM
To: Rajesh Borundia
Cc: David Miller; andy@greyhouse.net; netdev; Sony Chacko
Subject: Re: [PATCH net-next] netxen: write IP address to firmware when using bonding

On Mon, Nov 28, 2011 at 12:02:28AM -0600, Rajesh Borundia wrote:
> Hi Andy,
>
> We need to restore the ip address after the adapter reset.
> netxen_restore_indev_addr is the function that restores normal ip addresses
> and vlan ip addresses. If we could find bond device directly from slave then
> we can use netxen_config_indev_addr to add the ip address of the bond device.
> Otherwise we may need to cache the bond ip address in function netxen_list_config_vlan_ip
> and change the condition from
>
> if (!is_vlan_dev(dev))
>                 return;
>
> to
>
> if (!is_vlan_dev(dev) && !is_bond_dev(dev))
>                 return;
>

I don't think this was what you were originally intending, but what
about changing the name of the vlan_ip_list (and the vlan functions that
modify/read it) to something different and use it for all virtual or
stacked devices?

Something like 'virtual' or 'virt' might work instead of 'vlan.'
Something else that denotes that a device at a higher layer than just
the physical device has an IP address that we need to know about, but
that the device is not specific to VLANs.

> Some of the code in if and else part is repeated.
> If possible can we have small functions for that ?
> eg:
> if (!is_netxen_netdev(dev))
> +                       goto done;
> +
> +               adapter = netdev_priv(dev);
> +               if (!adapter)
> +                       goto done;
> +
> +               if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
> +                       goto done;
> +
> +               netxen_config_indev_addr(adapter, orig_dev, event);
>
>

I could change that.  I started to look at sharing some code, but
decided against too much initial code churn.  Since the fundamental idea
behind this chance seems acceptable, I can take a look at it more
closely.

I'll take a look at this and send a v2 of this patch.


> Rajesh
> ________________________________________
> From: David Miller [davem@davemloft.net]
> Sent: Thursday, November 24, 2011 11:08 AM
> To: andy@greyhouse.net
> Cc: netdev; Sony Chacko; Rajesh Borundia
> Subject: Re: [PATCH net-next] netxen: write IP address to firmware when using bonding
>
> From: Andy Gospodarek <andy@greyhouse.net>
> Date: Wed, 23 Nov 2011 22:24:27 -0500
>
> > Are you talking about adding a macro like this:
> >
> >       for_each_dev_in_bond(bond,slave) {
> >               [...]
> >       }
> >
> > to replace the statements I added that were like this:
> >
> >       for_each_netdev_rcu(&init_net, slave) {
> >               if (slave->master == dev) {
> >                       [...]
> >               }
> >       }
> >
> > If so, that totally seems reasonable.  If were requesting something
> > else, please let me know.
>
> Yes, some helper that walks the device list and tries to find
> a device whose ->master matches 'dev'.
>
>
> --
> 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

[PATCH net-next] netxen: write IP address to firmware when using bonding

[Nikolay Aleksandrov]

From: Nikolay Aleksandrov <naleksan@redhat.com>

This patch allows LRO aggregation on bonded devices that contain an NX3031
device. It also adds a for_each_netdev_in_bond_rcu(bond, slave) macro
which executes for each slave that has bond as master.

Signed-off-by: Andy Gospodarek <agospoda@redhat.com>
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 .../net/ethernet/qlogic/netxen/netxen_nic_main.c   | 113 +++++++++++++++------
 include/linux/netdevice.h                          |   3 +
 2 files changed, 87 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
index e2a4858..aaf6cf7 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
@@ -3244,6 +3244,25 @@ netxen_restore_indev_addr(struct net_device *netdev, unsigned long event)
 	}
 }
 
+static inline int 
+netxen_config_checkdev(struct net_device *dev)
+{
+	struct netxen_adapter *adapter;
+
+	if (!is_netxen_netdev(dev))
+		return -ENODEV;
+	
+	adapter = netdev_priv(dev);
+
+	if(!adapter)
+		return -ENODEV;
+
+	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
+		return -ENODEV;
+
+	return 0;
+}
+
 static int netxen_netdev_event(struct notifier_block *this,
 				 unsigned long event, void *ptr)
 {
@@ -3260,18 +3279,27 @@ recheck:
 		goto recheck;
 	}
 
-	if (!is_netxen_netdev(dev))
-		goto done;
-
-	adapter = netdev_priv(dev);
-
-	if (!adapter)
-		goto done;
+	/* If this is a bonding device, look for netxen-based slaves*/
+	if (dev->priv_flags & IFF_BONDING) {
+		struct net_device *slave;
 
-	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
-		goto done;
+		rcu_read_lock();
+		for_each_netdev_in_bond_rcu(dev, slave) {
+			if (netxen_config_checkdev(slave) < 0)
+				continue;
+			
+			adapter = netdev_priv(slave);
+			netxen_config_indev_addr(adapter, orig_dev, event);
+		}
+		rcu_read_unlock();
 
-	netxen_config_indev_addr(adapter, orig_dev, event);
+	} else {
+		if (netxen_config_checkdev(dev) < 0)
+			goto done;
+		
+		adapter = netdev_priv(dev);
+		netxen_config_indev_addr(adapter, orig_dev, event);
+	}
 done:
 	return NOTIFY_DONE;
 }
@@ -3296,30 +3324,57 @@ recheck:
 		goto recheck;
 	}
 
-	if (!is_netxen_netdev(dev))
-		goto done;
+	/* If this is a bonding device, look for netxen-based slaves*/
+	if (dev->priv_flags & IFF_BONDING) {
+		struct net_device *slave;
 
-	adapter = netdev_priv(dev);
+		rcu_read_lock();
+		for_each_netdev_in_bond_rcu(dev, slave) {
+			if (netxen_config_checkdev(slave) < 0)
+				continue;
 
-	if (!adapter || !netxen_destip_supported(adapter))
-		goto done;
+			adapter = netdev_priv(slave);
 
-	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
-		goto done;
+			if (!netxen_destip_supported(adapter))
+				continue;
 
-	switch (event) {
-	case NETDEV_UP:
-		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP);
-		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
-		break;
-	case NETDEV_DOWN:
-		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN);
-		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
-		break;
-	default:
-		break;
-	}
+			switch (event) {
+			case NETDEV_UP:
+				netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP);
+				netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
+				break;
+			case NETDEV_DOWN:
+				netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN);
+				netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
+				break;
+			default:
+				break;
+			}
+		}
+		rcu_read_unlock();
 
+	} else {
+		if (netxen_config_checkdev(dev) < 0)
+			goto done;
+
+		adapter = netdev_priv(dev);
+
+		if (!netxen_destip_supported(adapter))
+			goto done;
+
+		switch (event) {
+		case NETDEV_UP:
+			netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP);
+			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
+			break;
+		case NETDEV_DOWN:
+			netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN);
+			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
+			break;
+		default:
+			break;
+		}
+	}
 done:
 	return NOTIFY_DONE;
 }
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 59dc05f3..463bb40 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1578,6 +1578,9 @@ extern rwlock_t				dev_base_lock;		/* Device list lock */
 		list_for_each_entry_continue(d, &(net)->dev_base_head, dev_list)
 #define for_each_netdev_continue_rcu(net, d)		\
 	list_for_each_entry_continue_rcu(d, &(net)->dev_base_head, dev_list)
+#define for_each_netdev_in_bond_rcu(bond, slave)	\
+	for_each_netdev_rcu(&init_net, slave)		\
+		if (slave->master == bond)
 #define net_device_entry(lh)	list_entry(lh, struct net_device, dev_list)
 
 static inline struct net_device *next_net_device(struct net_device *dev)
-- 
1.7.11.4

RE: [PATCH net-next] netxen: write IP address to firmware when using bonding

[Rajesh Borundia]

>-----Original Message-----
>From: Nikolay Aleksandrov [mailto:nikolay@redhat.com]
>Sent: Tuesday, September 25, 2012 2:18 PM
>To: Sony Chacko
>Cc: netdev; agospoda@redhat.com; Rajesh Borundia; David Miller
>Subject: [PATCH net-next] netxen: write IP address to firmware when
>using bonding
>
>From: Nikolay Aleksandrov <naleksan@redhat.com>
>
>This patch allows LRO aggregation on bonded devices that contain an
>NX3031
>device. It also adds a for_each_netdev_in_bond_rcu(bond, slave) macro
>which executes for each slave that has bond as master.
>
>Signed-off-by: Andy Gospodarek <agospoda@redhat.com>
>Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
>---
> .../net/ethernet/qlogic/netxen/netxen_nic_main.c   | 113
>+++++++++++++++------
> include/linux/netdevice.h                          |   3 +
> 2 files changed, 87 insertions(+), 29 deletions(-)
>
>diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>index e2a4858..aaf6cf7 100644
>--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>@@ -3244,6 +3244,25 @@ netxen_restore_indev_addr(struct net_device
>*netdev, unsigned long event)
> 	}
> }
>
>+static inline int
>+netxen_config_checkdev(struct net_device *dev)
>+{
>+	struct netxen_adapter *adapter;
>+
>+	if (!is_netxen_netdev(dev))
>+		return -ENODEV;
>+
>+	adapter = netdev_priv(dev);
>+
>+	if(!adapter)
>+		return -ENODEV;

It Seems space is needed after if.
 
>+
>+	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
>+		return -ENODEV;
>+
>+	return 0;
>+}
>+
> static int netxen_netdev_event(struct notifier_block *this,
> 				 unsigned long event, void *ptr)
> {
>@@ -3260,18 +3279,27 @@ recheck:
> 		goto recheck;
> 	}
>
>-	if (!is_netxen_netdev(dev))
>-		goto done;
>-
>-	adapter = netdev_priv(dev);
>-
>-	if (!adapter)
>-		goto done;
>+	/* If this is a bonding device, look for netxen-based slaves*/
>+	if (dev->priv_flags & IFF_BONDING) {
>+		struct net_device *slave;
>
>-	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
>-		goto done;
>+		rcu_read_lock();
>+		for_each_netdev_in_bond_rcu(dev, slave) {
>+			if (netxen_config_checkdev(slave) < 0)
>+				continue;
>+
>+			adapter = netdev_priv(slave);
>+			netxen_config_indev_addr(adapter, orig_dev, event);
>+		}
>+		rcu_read_unlock();
>
>-	netxen_config_indev_addr(adapter, orig_dev, event);
>+	} else {
>+		if (netxen_config_checkdev(dev) < 0)
>+			goto done;
>+
>+		adapter = netdev_priv(dev);
>+		netxen_config_indev_addr(adapter, orig_dev, event);
>+	}
> done:
> 	return NOTIFY_DONE;
> }
>@@ -3296,30 +3324,57 @@ recheck:
> 		goto recheck;
> 	}
>
>-	if (!is_netxen_netdev(dev))
>-		goto done;
>+	/* If this is a bonding device, look for netxen-based slaves*/
>+	if (dev->priv_flags & IFF_BONDING) {
>+		struct net_device *slave;
>
>-	adapter = netdev_priv(dev);
>+		rcu_read_lock();
>+		for_each_netdev_in_bond_rcu(dev, slave) {
>+			if (netxen_config_checkdev(slave) < 0)
>+				continue;
>
>-	if (!adapter || !netxen_destip_supported(adapter))
>-		goto done;
>+			adapter = netdev_priv(slave);
>
>-	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
>-		goto done;
>+			if (!netxen_destip_supported(adapter))
>+				continue;
>
>-	switch (event) {
>-	case NETDEV_UP:
>-		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP);
>-		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
>-		break;
>-	case NETDEV_DOWN:
>-		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN);
>-		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
>-		break;
>-	default:
>-		break;
>-	}
>+			switch (event) {
>+			case NETDEV_UP:
>+				netxen_config_ipaddr(adapter, ifa->ifa_address,
>NX_IP_UP);
>+				netxen_list_config_vlan_ip(adapter, ifa,
>NX_IP_UP);
>+				break;
>+			case NETDEV_DOWN:
>+				netxen_config_ipaddr(adapter, ifa->ifa_address,
>NX_IP_DOWN);
>+				netxen_list_config_vlan_ip(adapter, ifa,
>NX_IP_DOWN);
>+				break;
>+			default:
>+				break;
>+			}
>+		}
>+		rcu_read_unlock();
>
>+	} else {
>+		if (netxen_config_checkdev(dev) < 0)
>+			goto done;
>+
>+		adapter = netdev_priv(dev);
>+
>+		if (!netxen_destip_supported(adapter))
>+			goto done;
>+
>+		switch (event) {
>+		case NETDEV_UP:
>+			netxen_config_ipaddr(adapter, ifa->ifa_address,
>NX_IP_UP);
>+			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
>+			break;
>+		case NETDEV_DOWN:
>+			netxen_config_ipaddr(adapter, ifa->ifa_address,
>NX_IP_DOWN);
>+			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
>+			break;
>+		default:
>+			break;
>+		}
>+	}
> done:
> 	return NOTIFY_DONE;
> }
>diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>index 59dc05f3..463bb40 100644
>--- a/include/linux/netdevice.h
>+++ b/include/linux/netdevice.h
>@@ -1578,6 +1578,9 @@ extern rwlock_t
>	dev_base_lock;		/* Device list lock */
> 		list_for_each_entry_continue(d, &(net)->dev_base_head,
>dev_list)
> #define for_each_netdev_continue_rcu(net, d)		\
> 	list_for_each_entry_continue_rcu(d, &(net)->dev_base_head,
>dev_list)
>+#define for_each_netdev_in_bond_rcu(bond, slave)	\
>+	for_each_netdev_rcu(&init_net, slave)		\
>+		if (slave->master == bond)
> #define net_device_entry(lh)	list_entry(lh, struct net_device,
>dev_list)
>
> static inline struct net_device *next_net_device(struct net_device
>*dev)
>--
>1.7.11.4
>

It seems at some places line is over 80 characters.

Re: [PATCH net-next] netxen: write IP address to firmware when using bonding

[Nikolay Aleksandrov]

On 25/09/12 10:48, Nikolay Aleksandrov wrote:
> From: Nikolay Aleksandrov<naleksan@redhat.com>
>
> This patch allows LRO aggregation on bonded devices that contain an NX3031
> device. It also adds a for_each_netdev_in_bond_rcu(bond, slave) macro
> which executes for each slave that has bond as master.
>
> Signed-off-by: Andy Gospodarek<agospoda@redhat.com>
> Signed-off-by: Nikolay Aleksandrov<nikolay@redhat.com>
> ---
>   .../net/ethernet/qlogic/netxen/netxen_nic_main.c   | 113 +++++++++++++++------
>   include/linux/netdevice.h                          |   3 +
>   2 files changed, 87 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
> index e2a4858..aaf6cf7 100644
> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
> @@ -3244,6 +3244,25 @@ netxen_restore_indev_addr(struct net_device *netdev, unsigned long event)
>   	}
>   }
>
> +static inline int
> +netxen_config_checkdev(struct net_device *dev)
> +{
> +	struct netxen_adapter *adapter;
> +
> +	if (!is_netxen_netdev(dev))
> +		return -ENODEV;
> +	
> +	adapter = netdev_priv(dev);
> +
> +	if(!adapter)
> +		return -ENODEV;
> +
> +	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
>   static int netxen_netdev_event(struct notifier_block *this,
>   				 unsigned long event, void *ptr)
>   {
> @@ -3260,18 +3279,27 @@ recheck:
>   		goto recheck;
>   	}
>
> -	if (!is_netxen_netdev(dev))
> -		goto done;
> -
> -	adapter = netdev_priv(dev);
> -
> -	if (!adapter)
> -		goto done;
> +	/* If this is a bonding device, look for netxen-based slaves*/
> +	if (dev->priv_flags&  IFF_BONDING) {
> +		struct net_device *slave;
>
> -	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
> -		goto done;
> +		rcu_read_lock();
> +		for_each_netdev_in_bond_rcu(dev, slave) {
> +			if (netxen_config_checkdev(slave)<  0)
> +				continue;
> +			
> +			adapter = netdev_priv(slave);
> +			netxen_config_indev_addr(adapter, orig_dev, event);
> +		}
> +		rcu_read_unlock();
>
> -	netxen_config_indev_addr(adapter, orig_dev, event);
> +	} else {
> +		if (netxen_config_checkdev(dev)<  0)
> +			goto done;
> +		
> +		adapter = netdev_priv(dev);
> +		netxen_config_indev_addr(adapter, orig_dev, event);
> +	}
>   done:
>   	return NOTIFY_DONE;
>   }
> @@ -3296,30 +3324,57 @@ recheck:
>   		goto recheck;
>   	}
>
> -	if (!is_netxen_netdev(dev))
> -		goto done;
> +	/* If this is a bonding device, look for netxen-based slaves*/
> +	if (dev->priv_flags&  IFF_BONDING) {
> +		struct net_device *slave;
>
> -	adapter = netdev_priv(dev);
> +		rcu_read_lock();
> +		for_each_netdev_in_bond_rcu(dev, slave) {
> +			if (netxen_config_checkdev(slave)<  0)
> +				continue;
>
> -	if (!adapter || !netxen_destip_supported(adapter))
> -		goto done;
> +			adapter = netdev_priv(slave);
>
> -	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
> -		goto done;
> +			if (!netxen_destip_supported(adapter))
> +				continue;
>
> -	switch (event) {
> -	case NETDEV_UP:
> -		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP);
> -		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
> -		break;
> -	case NETDEV_DOWN:
> -		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN);
> -		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
> -		break;
> -	default:
> -		break;
> -	}
> +			switch (event) {
> +			case NETDEV_UP:
> +				netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP);
> +				netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
> +				break;
> +			case NETDEV_DOWN:
> +				netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN);
> +				netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
> +				break;
> +			default:
> +				break;
> +			}
> +		}
> +		rcu_read_unlock();
>
> +	} else {
> +		if (netxen_config_checkdev(dev)<  0)
> +			goto done;
> +
> +		adapter = netdev_priv(dev);
> +
> +		if (!netxen_destip_supported(adapter))
> +			goto done;
> +
> +		switch (event) {
> +		case NETDEV_UP:
> +			netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP);
> +			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
> +			break;
> +		case NETDEV_DOWN:
> +			netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN);
> +			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
>   done:
>   	return NOTIFY_DONE;
>   }
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 59dc05f3..463bb40 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1578,6 +1578,9 @@ extern rwlock_t				dev_base_lock;		/* Device list lock */
>   		list_for_each_entry_continue(d,&(net)->dev_base_head, dev_list)
>   #define for_each_netdev_continue_rcu(net, d)		\
>   	list_for_each_entry_continue_rcu(d,&(net)->dev_base_head, dev_list)
> +#define for_each_netdev_in_bond_rcu(bond, slave)	\
> +	for_each_netdev_rcu(&init_net, slave)		\
> +		if (slave->master == bond)
>   #define net_device_entry(lh)	list_entry(lh, struct net_device, dev_list)
>
>   static inline struct net_device *next_net_device(struct net_device *dev)
Ah yes, you're correct. I'll fix the cosmetic issues and re-post it.
I would like to know if the patch is acceptable otherwise, and if
there are any comments about the implementation so I will
wait a little bit to see if anything else comes up.
Thank you for the review.

Best regards,
  Nikolay Aleksandrov

RE: [PATCH net-next] netxen: write IP address to firmware when using bonding

[Rajesh Borundia]

>-----Original Message-----
>From: Nikolay Aleksandrov [mailto:nikolay@redhat.com]
>Sent: Tuesday, September 25, 2012 10:14 PM
>To: Sony Chacko
>Cc: netdev; agospoda@redhat.com; Rajesh Borundia; David Miller
>Subject: Re: [PATCH net-next] netxen: write IP address to firmware when
>using bonding
>
>On 25/09/12 10:48, Nikolay Aleksandrov wrote:
>> From: Nikolay Aleksandrov<naleksan@redhat.com>
>>
>> This patch allows LRO aggregation on bonded devices that contain an
>NX3031
>> device. It also adds a for_each_netdev_in_bond_rcu(bond, slave) macro
>> which executes for each slave that has bond as master.
>>
>> Signed-off-by: Andy Gospodarek<agospoda@redhat.com>
>> Signed-off-by: Nikolay Aleksandrov<nikolay@redhat.com>
>> ---
>>   .../net/ethernet/qlogic/netxen/netxen_nic_main.c   | 113
>+++++++++++++++------
>>   include/linux/netdevice.h                          |   3 +
>>   2 files changed, 87 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>> index e2a4858..aaf6cf7 100644
>> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>> @@ -3244,6 +3244,25 @@ netxen_restore_indev_addr(struct net_device
>*netdev, unsigned long event)
>>   	}
>>   }
>>
>> +static inline int
>> +netxen_config_checkdev(struct net_device *dev)
>> +{
>> +	struct netxen_adapter *adapter;
>> +
>> +	if (!is_netxen_netdev(dev))
>> +		return -ENODEV;
>> +
>> +	adapter = netdev_priv(dev);
>> +
>> +	if(!adapter)
>> +		return -ENODEV;
>> +
>> +	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
>> +		return -ENODEV;
>> +
>> +	return 0;
>> +}
>> +
>>   static int netxen_netdev_event(struct notifier_block *this,
>>   				 unsigned long event, void *ptr)
>>   {
>> @@ -3260,18 +3279,27 @@ recheck:
>>   		goto recheck;
>>   	}
>>
>> -	if (!is_netxen_netdev(dev))
>> -		goto done;
>> -
>> -	adapter = netdev_priv(dev);
>> -
>> -	if (!adapter)
>> -		goto done;
>> +	/* If this is a bonding device, look for netxen-based slaves*/
>> +	if (dev->priv_flags&  IFF_BONDING) {
>> +		struct net_device *slave;
>>
>> -	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
>> -		goto done;
>> +		rcu_read_lock();
>> +		for_each_netdev_in_bond_rcu(dev, slave) {
>> +			if (netxen_config_checkdev(slave)<  0)
>> +				continue;
>> +
>> +			adapter = netdev_priv(slave);
>> +			netxen_config_indev_addr(adapter, orig_dev, event);
>> +		}
>> +		rcu_read_unlock();
>>
>> -	netxen_config_indev_addr(adapter, orig_dev, event);
>> +	} else {
>> +		if (netxen_config_checkdev(dev)<  0)
>> +			goto done;
>> +
>> +		adapter = netdev_priv(dev);
>> +		netxen_config_indev_addr(adapter, orig_dev, event);
>> +	}
>>   done:
>>   	return NOTIFY_DONE;
>>   }
>> @@ -3296,30 +3324,57 @@ recheck:
>>   		goto recheck;
>>   	}
>>
>> -	if (!is_netxen_netdev(dev))
>> -		goto done;
>> +	/* If this is a bonding device, look for netxen-based slaves*/
>> +	if (dev->priv_flags&  IFF_BONDING) {
>> +		struct net_device *slave;
>>
>> -	adapter = netdev_priv(dev);
>> +		rcu_read_lock();
>> +		for_each_netdev_in_bond_rcu(dev, slave) {
>> +			if (netxen_config_checkdev(slave)<  0)
>> +				continue;
>>
>> -	if (!adapter || !netxen_destip_supported(adapter))
>> -		goto done;
>> +			adapter = netdev_priv(slave);
>>
>> -	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
>> -		goto done;
>> +			if (!netxen_destip_supported(adapter))
>> +				continue;
>>
>> -	switch (event) {
>> -	case NETDEV_UP:
>> -		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP);
>> -		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
>> -		break;
>> -	case NETDEV_DOWN:
>> -		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN);
>> -		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
>> -		break;
>> -	default:
>> -		break;
>> -	}
>> +			switch (event) {
>> +			case NETDEV_UP:
>> +				netxen_config_ipaddr(adapter, ifa->ifa_address,
>NX_IP_UP);
>> +				netxen_list_config_vlan_ip(adapter, ifa,
>NX_IP_UP);
>> +				break;
>> +			case NETDEV_DOWN:
>> +				netxen_config_ipaddr(adapter, ifa->ifa_address,
>NX_IP_DOWN);
>> +				netxen_list_config_vlan_ip(adapter, ifa,
>NX_IP_DOWN);
>> +				break;
>> +			default:
>> +				break;
>> +			}
>> +		}
>> +		rcu_read_unlock();
>>
>> +	} else {
>> +		if (netxen_config_checkdev(dev)<  0)
>> +			goto done;
>> +
>> +		adapter = netdev_priv(dev);
>> +
>> +		if (!netxen_destip_supported(adapter))
>> +			goto done;
>> +
>> +		switch (event) {
>> +		case NETDEV_UP:
>> +			netxen_config_ipaddr(adapter, ifa->ifa_address,
>NX_IP_UP);
>> +			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
>> +			break;
>> +		case NETDEV_DOWN:
>> +			netxen_config_ipaddr(adapter, ifa->ifa_address,
>NX_IP_DOWN);
>> +			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
>> +			break;
>> +		default:
>> +			break;
>> +		}
>> +	}
>>   done:
>>   	return NOTIFY_DONE;
>>   }
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 59dc05f3..463bb40 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1578,6 +1578,9 @@ extern rwlock_t
>	dev_base_lock;		/* Device list lock */
>>   		list_for_each_entry_continue(d,&(net)->dev_base_head,
>dev_list)
>>   #define for_each_netdev_continue_rcu(net, d)		\
>>   	list_for_each_entry_continue_rcu(d,&(net)->dev_base_head,
>dev_list)
>> +#define for_each_netdev_in_bond_rcu(bond, slave)	\
>> +	for_each_netdev_rcu(&init_net, slave)		\
>> +		if (slave->master == bond)
>>   #define net_device_entry(lh)	list_entry(lh, struct net_device,
>dev_list)
>>
>>   static inline struct net_device *next_net_device(struct net_device
>*dev)
>Ah yes, you're correct. I'll fix the cosmetic issues and re-post it.
>I would like to know if the patch is acceptable otherwise, and if
>there are any comments about the implementation so I will
>wait a little bit to see if anything else comes up.

You need to add bond interface ip again after adapter is reset.
netxen_restore_indev_addr() function does this for normal as well as vlan interface.
Is there an API where we could find that interface has a master bond interface and
we could program the ip of that bond interface ?
Otherwise we may have to cache the ip address like we do for vlan interfaces in this function
netxen_list_config_vlan_ip() and program that in netxen_restore_indev_addr().

>Thank you for the review.
>
>Best regards,
>  Nikolay Aleksandrov
>

[PATCH net-next v2] netxen: write IP address to firmware when using bonding

[Nikolay Aleksandrov]

This patch allows LRO aggregation on bonded devices that contain an NX3031
device. It also adds a for_each_netdev_in_bond_rcu(bond, slave) macro
which executes for each slave that has bond as master.

V2: Remove local ip caching, retrieve addresses dynamically and
    restore them if necessary.

Note: Tested with NX3031 adapter.

Signed-off-by: Andy Gospodarek <agospoda@redhat.com>
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/ethernet/qlogic/netxen/netxen_nic.h    |   6 -
 .../net/ethernet/qlogic/netxen/netxen_nic_main.c   | 192 +++++++++++----------
 include/linux/netdevice.h                          |   3 +
 3 files changed, 100 insertions(+), 101 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
index eb3dfdb..cb4f2ce 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
@@ -955,11 +955,6 @@ typedef struct nx_mac_list_s {
 	uint8_t mac_addr[ETH_ALEN+2];
 } nx_mac_list_t;
 
-struct nx_vlan_ip_list {
-	struct list_head list;
-	__be32 ip_addr;
-};
-
 /*
  * Interrupt coalescing defaults. The defaults are for 1500 MTU. It is
  * adjusted based on configured MTU.
@@ -1605,7 +1600,6 @@ struct netxen_adapter {
 	struct net_device *netdev;
 	struct pci_dev *pdev;
 	struct list_head mac_list;
-	struct list_head vlan_ip_list;
 
 	spinlock_t tx_clean_lock;
 
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
index e2a4858..8e3eb61 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
@@ -90,7 +90,6 @@ static irqreturn_t netxen_intr(int irq, void *data);
 static irqreturn_t netxen_msi_intr(int irq, void *data);
 static irqreturn_t netxen_msix_intr(int irq, void *data);
 
-static void netxen_free_vlan_ip_list(struct netxen_adapter *);
 static void netxen_restore_indev_addr(struct net_device *dev, unsigned long);
 static struct rtnl_link_stats64 *netxen_nic_get_stats(struct net_device *dev,
 						      struct rtnl_link_stats64 *stats);
@@ -1451,7 +1450,6 @@ netxen_nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	spin_lock_init(&adapter->tx_clean_lock);
 	INIT_LIST_HEAD(&adapter->mac_list);
-	INIT_LIST_HEAD(&adapter->vlan_ip_list);
 
 	err = netxen_setup_pci_map(adapter);
 	if (err)
@@ -1586,7 +1584,7 @@ static void __devexit netxen_nic_remove(struct pci_dev *pdev)
 
 	cancel_work_sync(&adapter->tx_timeout_task);
 
-	netxen_free_vlan_ip_list(adapter);
+	netxen_restore_indev_addr(netdev, NETDEV_DOWN);
 	netxen_nic_detach(adapter);
 
 	nx_decr_dev_ref_cnt(adapter);
@@ -3135,66 +3133,22 @@ netxen_destip_supported(struct netxen_adapter *adapter)
 	return 1;
 }
 
-static void
-netxen_free_vlan_ip_list(struct netxen_adapter *adapter)
+static inline __be32
+netxen_get_addr(struct net_device *dev)
 {
-	struct nx_vlan_ip_list  *cur;
-	struct list_head *head = &adapter->vlan_ip_list;
-
-	while (!list_empty(head)) {
-		cur = list_entry(head->next, struct nx_vlan_ip_list, list);
-		netxen_config_ipaddr(adapter, cur->ip_addr, NX_IP_DOWN);
-		list_del(&cur->list);
-		kfree(cur);
-	}
-
-}
-static void
-netxen_list_config_vlan_ip(struct netxen_adapter *adapter,
-		struct in_ifaddr *ifa, unsigned long event)
-{
-	struct net_device *dev;
-	struct nx_vlan_ip_list *cur, *tmp_cur;
-	struct list_head *head;
-
-	dev = ifa->ifa_dev ? ifa->ifa_dev->dev : NULL;
-
-	if (dev == NULL)
-		return;
-
-	if (!is_vlan_dev(dev))
-		return;
+	struct in_device *in_dev;
+	__be32 addr = 0;
 
-	switch (event) {
-	case NX_IP_UP:
-		list_for_each(head, &adapter->vlan_ip_list) {
-			cur = list_entry(head, struct nx_vlan_ip_list, list);
+	rcu_read_lock();
+	in_dev = __in_dev_get_rcu(dev);
 
-			if (cur->ip_addr == ifa->ifa_address)
-				return;
-		}
+	if (in_dev)
+		addr = inet_confirm_addr(in_dev, 0, 0, RT_SCOPE_HOST);
 
-		cur = kzalloc(sizeof(struct nx_vlan_ip_list), GFP_ATOMIC);
-		if (cur == NULL) {
-			printk(KERN_ERR "%s: failed to add vlan ip to list\n",
-					adapter->netdev->name);
-			return;
-		}
-
-		cur->ip_addr = ifa->ifa_address;
-		list_add_tail(&cur->list, &adapter->vlan_ip_list);
-		break;
-	case NX_IP_DOWN:
-		list_for_each_entry_safe(cur, tmp_cur,
-					&adapter->vlan_ip_list, list) {
-			if (cur->ip_addr == ifa->ifa_address) {
-				list_del(&cur->list);
-				kfree(cur);
-				break;
-			}
-		}
-	}
+	rcu_read_unlock();
+	return addr;
 }
+
 static void
 netxen_config_indev_addr(struct netxen_adapter *adapter,
 		struct net_device *dev, unsigned long event)
@@ -3213,12 +3167,10 @@ netxen_config_indev_addr(struct netxen_adapter *adapter,
 		case NETDEV_UP:
 			netxen_config_ipaddr(adapter,
 					ifa->ifa_address, NX_IP_UP);
-			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
 			break;
 		case NETDEV_DOWN:
 			netxen_config_ipaddr(adapter,
 					ifa->ifa_address, NX_IP_DOWN);
-			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
 			break;
 		default:
 			break;
@@ -3233,15 +3185,54 @@ netxen_restore_indev_addr(struct net_device *netdev, unsigned long event)
 
 {
 	struct netxen_adapter *adapter = netdev_priv(netdev);
-	struct nx_vlan_ip_list *pos, *tmp_pos;
+	struct net_device *vlan_dev, *real_dev;
 	unsigned long ip_event;
+	__be32 addr = 0;
 
 	ip_event = (event == NETDEV_UP) ? NX_IP_UP : NX_IP_DOWN;
 	netxen_config_indev_addr(adapter, netdev, event);
 
-	list_for_each_entry_safe(pos, tmp_pos, &adapter->vlan_ip_list, list) {
-		netxen_config_ipaddr(adapter, pos->ip_addr, ip_event);
+	rcu_read_lock();
+	for_each_netdev_rcu(&init_net, vlan_dev) {
+		if (vlan_dev->priv_flags & IFF_802_1Q_VLAN) {
+			real_dev = vlan_dev_real_dev(vlan_dev);
+
+			if (real_dev == netdev) {
+				addr = netxen_get_addr(vlan_dev);
+
+				if (addr) {
+					netxen_config_ipaddr(adapter, addr,
+							     ip_event);
+				}
+			}
+		}
 	}
+
+	if (netdev->master) {
+		addr = netxen_get_addr(netdev->master);
+		if (addr)
+			netxen_config_ipaddr(adapter, addr, ip_event);
+	}
+	rcu_read_unlock();
+}
+
+static inline int
+netxen_config_checkdev(struct net_device *dev)
+{
+	struct netxen_adapter *adapter;
+
+	if (!is_netxen_netdev(dev))
+		return -ENODEV;
+
+	adapter = netdev_priv(dev);
+
+	if (!adapter)
+		return -ENODEV;
+
+	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
+		return -ENODEV;
+
+	return 0;
 }
 
 static int netxen_netdev_event(struct notifier_block *this,
@@ -3260,18 +3251,26 @@ recheck:
 		goto recheck;
 	}
 
-	if (!is_netxen_netdev(dev))
-		goto done;
+	/* If this is a bonding device, look for netxen-based slaves*/
+	if (dev->priv_flags & IFF_BONDING) {
+		struct net_device *slave;
 
-	adapter = netdev_priv(dev);
+		rcu_read_lock();
+		for_each_netdev_in_bond_rcu(dev, slave) {
+			if (netxen_config_checkdev(slave) < 0)
+				continue;
 
-	if (!adapter)
-		goto done;
-
-	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
-		goto done;
+			adapter = netdev_priv(slave);
+			netxen_config_indev_addr(adapter, orig_dev, event);
+		}
+		rcu_read_unlock();
+	} else {
+		if (netxen_config_checkdev(dev) < 0)
+			goto done;
 
-	netxen_config_indev_addr(adapter, orig_dev, event);
+		adapter = netdev_priv(dev);
+		netxen_config_indev_addr(adapter, orig_dev, event);
+	}
 done:
 	return NOTIFY_DONE;
 }
@@ -3282,10 +3281,11 @@ netxen_inetaddr_event(struct notifier_block *this,
 {
 	struct netxen_adapter *adapter;
 	struct net_device *dev;
-
+	unsigned long ip_event;
 	struct in_ifaddr *ifa = (struct in_ifaddr *)ptr;
 
 	dev = ifa->ifa_dev ? ifa->ifa_dev->dev : NULL;
+	ip_event = (event == NETDEV_UP) ? NX_IP_UP : NX_IP_DOWN;
 
 recheck:
 	if (dev == NULL)
@@ -3296,30 +3296,35 @@ recheck:
 		goto recheck;
 	}
 
-	if (!is_netxen_netdev(dev))
-		goto done;
+	/* If this is a bonding device, look for netxen-based slaves*/
+	if (dev->priv_flags & IFF_BONDING) {
+		struct net_device *slave;
 
-	adapter = netdev_priv(dev);
+		rcu_read_lock();
+		for_each_netdev_in_bond_rcu(dev, slave) {
+			if (netxen_config_checkdev(slave) < 0)
+				continue;
 
-	if (!adapter || !netxen_destip_supported(adapter))
-		goto done;
+			adapter = netdev_priv(slave);
 
-	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
-		goto done;
+			if (!netxen_destip_supported(adapter))
+				continue;
 
-	switch (event) {
-	case NETDEV_UP:
-		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP);
-		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
-		break;
-	case NETDEV_DOWN:
-		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN);
-		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
-		break;
-	default:
-		break;
-	}
+			netxen_config_ipaddr(adapter, ifa->ifa_address,
+					     ip_event);
+		}
+		rcu_read_unlock();
+	} else {
+		if (netxen_config_checkdev(dev) < 0)
+			goto done;
+
+		adapter = netdev_priv(dev);
 
+		if (!netxen_destip_supported(adapter))
+			goto done;
+
+		netxen_config_ipaddr(adapter, ifa->ifa_address, ip_event);
+	}
 done:
 	return NOTIFY_DONE;
 }
@@ -3335,9 +3340,6 @@ static struct notifier_block netxen_inetaddr_cb = {
 static void
 netxen_restore_indev_addr(struct net_device *dev, unsigned long event)
 { }
-static void
-netxen_free_vlan_ip_list(struct netxen_adapter *adapter)
-{ }
 #endif
 
 static struct pci_error_handlers netxen_err_handler = {
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 59dc05f3..463bb40 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1578,6 +1578,9 @@ extern rwlock_t				dev_base_lock;		/* Device list lock */
 		list_for_each_entry_continue(d, &(net)->dev_base_head, dev_list)
 #define for_each_netdev_continue_rcu(net, d)		\
 	list_for_each_entry_continue_rcu(d, &(net)->dev_base_head, dev_list)
+#define for_each_netdev_in_bond_rcu(bond, slave)	\
+	for_each_netdev_rcu(&init_net, slave)		\
+		if (slave->master == bond)
 #define net_device_entry(lh)	list_entry(lh, struct net_device, dev_list)
 
 static inline struct net_device *next_net_device(struct net_device *dev)
-- 
1.7.11.4

Re: [PATCH net-next v2] netxen: write IP address to firmware when using bonding

[David Miller]

From: Nikolay Aleksandrov <nikolay@redhat.com>
Date: Tue,  2 Oct 2012 11:16:26 +0200

> This patch allows LRO aggregation on bonded devices that contain an NX3031
> device. It also adds a for_each_netdev_in_bond_rcu(bond, slave) macro
> which executes for each slave that has bond as master.
> 
> V2: Remove local ip caching, retrieve addresses dynamically and
>     restore them if necessary.
> 
> Note: Tested with NX3031 adapter.
> 
> Signed-off-by: Andy Gospodarek <agospoda@redhat.com>
> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>

This doesn't apply cleanly to the current tree.

It is also poorly styled, for example:

> +				if (addr) {
> +					netxen_config_ipaddr(adapter, addr,
> +							     ip_event);
> +				}

No curley braces for a single statement.

There are also a lot of extransoue empty lines added to the
code which do not enhance readability in any way and just take
up valuable vertical screen realestate.

You've also missed the net-next merge window cutoff, so you'll
need to wait until after the merge window to send this in again.

Thanks.

Re: [PATCH net-next v2] netxen: write IP address to firmware when using bonding

[Nikolay Aleksandrov]

On 10/02/2012 11:16 AM, Nikolay Aleksandrov wrote:
> This patch allows LRO aggregation on bonded devices that contain an NX3031
> device. It also adds a for_each_netdev_in_bond_rcu(bond, slave) macro
> which executes for each slave that has bond as master.
>
> V2: Remove local ip caching, retrieve addresses dynamically and
>      restore them if necessary.
>
> Note: Tested with NX3031 adapter.
>
> Signed-off-by: Andy Gospodarek <agospoda@redhat.com>
> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
> ---
>   drivers/net/ethernet/qlogic/netxen/netxen_nic.h    |   6 -
>   .../net/ethernet/qlogic/netxen/netxen_nic_main.c   | 192 +++++++++++----------
>   include/linux/netdevice.h                          |   3 +
>   3 files changed, 100 insertions(+), 101 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
> index eb3dfdb..cb4f2ce 100644
> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
> @@ -955,11 +955,6 @@ typedef struct nx_mac_list_s {
>   	uint8_t mac_addr[ETH_ALEN+2];
>   } nx_mac_list_t;
>   
> -struct nx_vlan_ip_list {
> -	struct list_head list;
> -	__be32 ip_addr;
> -};
> -
>   /*
>    * Interrupt coalescing defaults. The defaults are for 1500 MTU. It is
>    * adjusted based on configured MTU.
> @@ -1605,7 +1600,6 @@ struct netxen_adapter {
>   	struct net_device *netdev;
>   	struct pci_dev *pdev;
>   	struct list_head mac_list;
> -	struct list_head vlan_ip_list;
>   
>   	spinlock_t tx_clean_lock;
>   
> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
> index e2a4858..8e3eb61 100644
> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
> @@ -90,7 +90,6 @@ static irqreturn_t netxen_intr(int irq, void *data);
>   static irqreturn_t netxen_msi_intr(int irq, void *data);
>   static irqreturn_t netxen_msix_intr(int irq, void *data);
>   
> -static void netxen_free_vlan_ip_list(struct netxen_adapter *);
>   static void netxen_restore_indev_addr(struct net_device *dev, unsigned long);
>   static struct rtnl_link_stats64 *netxen_nic_get_stats(struct net_device *dev,
>   						      struct rtnl_link_stats64 *stats);
> @@ -1451,7 +1450,6 @@ netxen_nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   
>   	spin_lock_init(&adapter->tx_clean_lock);
>   	INIT_LIST_HEAD(&adapter->mac_list);
> -	INIT_LIST_HEAD(&adapter->vlan_ip_list);
>   
>   	err = netxen_setup_pci_map(adapter);
>   	if (err)
> @@ -1586,7 +1584,7 @@ static void __devexit netxen_nic_remove(struct pci_dev *pdev)
>   
>   	cancel_work_sync(&adapter->tx_timeout_task);
>   
> -	netxen_free_vlan_ip_list(adapter);
> +	netxen_restore_indev_addr(netdev, NETDEV_DOWN);
>   	netxen_nic_detach(adapter);
>   
>   	nx_decr_dev_ref_cnt(adapter);
> @@ -3135,66 +3133,22 @@ netxen_destip_supported(struct netxen_adapter *adapter)
>   	return 1;
>   }
>   
> -static void
> -netxen_free_vlan_ip_list(struct netxen_adapter *adapter)
> +static inline __be32
> +netxen_get_addr(struct net_device *dev)
>   {
> -	struct nx_vlan_ip_list  *cur;
> -	struct list_head *head = &adapter->vlan_ip_list;
> -
> -	while (!list_empty(head)) {
> -		cur = list_entry(head->next, struct nx_vlan_ip_list, list);
> -		netxen_config_ipaddr(adapter, cur->ip_addr, NX_IP_DOWN);
> -		list_del(&cur->list);
> -		kfree(cur);
> -	}
> -
> -}
> -static void
> -netxen_list_config_vlan_ip(struct netxen_adapter *adapter,
> -		struct in_ifaddr *ifa, unsigned long event)
> -{
> -	struct net_device *dev;
> -	struct nx_vlan_ip_list *cur, *tmp_cur;
> -	struct list_head *head;
> -
> -	dev = ifa->ifa_dev ? ifa->ifa_dev->dev : NULL;
> -
> -	if (dev == NULL)
> -		return;
> -
> -	if (!is_vlan_dev(dev))
> -		return;
> +	struct in_device *in_dev;
> +	__be32 addr = 0;
>   
> -	switch (event) {
> -	case NX_IP_UP:
> -		list_for_each(head, &adapter->vlan_ip_list) {
> -			cur = list_entry(head, struct nx_vlan_ip_list, list);
> +	rcu_read_lock();
> +	in_dev = __in_dev_get_rcu(dev);
>   
> -			if (cur->ip_addr == ifa->ifa_address)
> -				return;
> -		}
> +	if (in_dev)
> +		addr = inet_confirm_addr(in_dev, 0, 0, RT_SCOPE_HOST);
>   
> -		cur = kzalloc(sizeof(struct nx_vlan_ip_list), GFP_ATOMIC);
> -		if (cur == NULL) {
> -			printk(KERN_ERR "%s: failed to add vlan ip to list\n",
> -					adapter->netdev->name);
> -			return;
> -		}
> -
> -		cur->ip_addr = ifa->ifa_address;
> -		list_add_tail(&cur->list, &adapter->vlan_ip_list);
> -		break;
> -	case NX_IP_DOWN:
> -		list_for_each_entry_safe(cur, tmp_cur,
> -					&adapter->vlan_ip_list, list) {
> -			if (cur->ip_addr == ifa->ifa_address) {
> -				list_del(&cur->list);
> -				kfree(cur);
> -				break;
> -			}
> -		}
> -	}
> +	rcu_read_unlock();
> +	return addr;
>   }
> +
>   static void
>   netxen_config_indev_addr(struct netxen_adapter *adapter,
>   		struct net_device *dev, unsigned long event)
> @@ -3213,12 +3167,10 @@ netxen_config_indev_addr(struct netxen_adapter *adapter,
>   		case NETDEV_UP:
>   			netxen_config_ipaddr(adapter,
>   					ifa->ifa_address, NX_IP_UP);
> -			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
>   			break;
>   		case NETDEV_DOWN:
>   			netxen_config_ipaddr(adapter,
>   					ifa->ifa_address, NX_IP_DOWN);
> -			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
>   			break;
>   		default:
>   			break;
> @@ -3233,15 +3185,54 @@ netxen_restore_indev_addr(struct net_device *netdev, unsigned long event)
>   
>   {
>   	struct netxen_adapter *adapter = netdev_priv(netdev);
> -	struct nx_vlan_ip_list *pos, *tmp_pos;
> +	struct net_device *vlan_dev, *real_dev;
>   	unsigned long ip_event;
> +	__be32 addr = 0;
>   
>   	ip_event = (event == NETDEV_UP) ? NX_IP_UP : NX_IP_DOWN;
>   	netxen_config_indev_addr(adapter, netdev, event);
>   
> -	list_for_each_entry_safe(pos, tmp_pos, &adapter->vlan_ip_list, list) {
> -		netxen_config_ipaddr(adapter, pos->ip_addr, ip_event);
> +	rcu_read_lock();
> +	for_each_netdev_rcu(&init_net, vlan_dev) {
> +		if (vlan_dev->priv_flags & IFF_802_1Q_VLAN) {
> +			real_dev = vlan_dev_real_dev(vlan_dev);
> +
> +			if (real_dev == netdev) {
> +				addr = netxen_get_addr(vlan_dev);
> +
> +				if (addr) {
> +					netxen_config_ipaddr(adapter, addr,
> +							     ip_event);
> +				}
> +			}
> +		}
>   	}
> +
> +	if (netdev->master) {
> +		addr = netxen_get_addr(netdev->master);
> +		if (addr)
> +			netxen_config_ipaddr(adapter, addr, ip_event);
> +	}
> +	rcu_read_unlock();
> +}
> +
> +static inline int
> +netxen_config_checkdev(struct net_device *dev)
> +{
> +	struct netxen_adapter *adapter;
> +
> +	if (!is_netxen_netdev(dev))
> +		return -ENODEV;
> +
> +	adapter = netdev_priv(dev);
> +
> +	if (!adapter)
> +		return -ENODEV;
> +
> +	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
> +		return -ENODEV;
> +
> +	return 0;
>   }
>   
>   static int netxen_netdev_event(struct notifier_block *this,
> @@ -3260,18 +3251,26 @@ recheck:
>   		goto recheck;
>   	}
>   
> -	if (!is_netxen_netdev(dev))
> -		goto done;
> +	/* If this is a bonding device, look for netxen-based slaves*/
> +	if (dev->priv_flags & IFF_BONDING) {
> +		struct net_device *slave;
>   
> -	adapter = netdev_priv(dev);
> +		rcu_read_lock();
> +		for_each_netdev_in_bond_rcu(dev, slave) {
> +			if (netxen_config_checkdev(slave) < 0)
> +				continue;
>   
> -	if (!adapter)
> -		goto done;
> -
> -	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
> -		goto done;
> +			adapter = netdev_priv(slave);
> +			netxen_config_indev_addr(adapter, orig_dev, event);
> +		}
> +		rcu_read_unlock();
> +	} else {
> +		if (netxen_config_checkdev(dev) < 0)
> +			goto done;
>   
> -	netxen_config_indev_addr(adapter, orig_dev, event);
> +		adapter = netdev_priv(dev);
> +		netxen_config_indev_addr(adapter, orig_dev, event);
> +	}
>   done:
>   	return NOTIFY_DONE;
>   }
> @@ -3282,10 +3281,11 @@ netxen_inetaddr_event(struct notifier_block *this,
>   {
>   	struct netxen_adapter *adapter;
>   	struct net_device *dev;
> -
> +	unsigned long ip_event;
>   	struct in_ifaddr *ifa = (struct in_ifaddr *)ptr;
>   
>   	dev = ifa->ifa_dev ? ifa->ifa_dev->dev : NULL;
> +	ip_event = (event == NETDEV_UP) ? NX_IP_UP : NX_IP_DOWN;
>   
>   recheck:
>   	if (dev == NULL)
> @@ -3296,30 +3296,35 @@ recheck:
>   		goto recheck;
>   	}
>   
> -	if (!is_netxen_netdev(dev))
> -		goto done;
> +	/* If this is a bonding device, look for netxen-based slaves*/
> +	if (dev->priv_flags & IFF_BONDING) {
> +		struct net_device *slave;
>   
> -	adapter = netdev_priv(dev);
> +		rcu_read_lock();
> +		for_each_netdev_in_bond_rcu(dev, slave) {
> +			if (netxen_config_checkdev(slave) < 0)
> +				continue;
>   
> -	if (!adapter || !netxen_destip_supported(adapter))
> -		goto done;
> +			adapter = netdev_priv(slave);
>   
> -	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
> -		goto done;
> +			if (!netxen_destip_supported(adapter))
> +				continue;
>   
> -	switch (event) {
> -	case NETDEV_UP:
> -		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP);
> -		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
> -		break;
> -	case NETDEV_DOWN:
> -		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN);
> -		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
> -		break;
> -	default:
> -		break;
> -	}
> +			netxen_config_ipaddr(adapter, ifa->ifa_address,
> +					     ip_event);
> +		}
> +		rcu_read_unlock();
> +	} else {
> +		if (netxen_config_checkdev(dev) < 0)
> +			goto done;
> +
> +		adapter = netdev_priv(dev);
>   
> +		if (!netxen_destip_supported(adapter))
> +			goto done;
> +
> +		netxen_config_ipaddr(adapter, ifa->ifa_address, ip_event);
> +	}
>   done:
>   	return NOTIFY_DONE;
>   }
> @@ -3335,9 +3340,6 @@ static struct notifier_block netxen_inetaddr_cb = {
>   static void
>   netxen_restore_indev_addr(struct net_device *dev, unsigned long event)
>   { }
> -static void
> -netxen_free_vlan_ip_list(struct netxen_adapter *adapter)
> -{ }
>   #endif
>   
>   static struct pci_error_handlers netxen_err_handler = {
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 59dc05f3..463bb40 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1578,6 +1578,9 @@ extern rwlock_t				dev_base_lock;		/* Device list lock */
>   		list_for_each_entry_continue(d, &(net)->dev_base_head, dev_list)
>   #define for_each_netdev_continue_rcu(net, d)		\
>   	list_for_each_entry_continue_rcu(d, &(net)->dev_base_head, dev_list)
> +#define for_each_netdev_in_bond_rcu(bond, slave)	\
> +	for_each_netdev_rcu(&init_net, slave)		\
> +		if (slave->master == bond)
>   #define net_device_entry(lh)	list_entry(lh, struct net_device, dev_list)
>   
>   static inline struct net_device *next_net_device(struct net_device *dev)
Hello Dave,
I just synced with upstream, I've missed a few patches and it seems
that it doesn't apply cleanly because my previous patch was
changed before it was applied. There is one character missing from
a comment - "/* root bus? */", in upstream it was changed to
/* root bus */.
("netxen: check for root bus in netxen_mask_aer_correctable")

About the rest, after QLogic test the functionality I'll clean-up the
empty lines and re-send it.

Thank you,
  Nik

Re: [PATCH net-next v2] netxen: write IP address to firmware when using bonding

[David Miller]

From: Nikolay Aleksandrov <nikolay@redhat.com>
Date: Wed, 03 Oct 2012 11:20:06 +0200

> I just synced with upstream, I've missed a few patches and it seems
> that it doesn't apply cleanly because my previous patch was
> changed before it was applied. There is one character missing from
> a comment - "/* root bus? */", in upstream it was changed to
> /* root bus */.
> ("netxen: check for root bus in netxen_mask_aer_correctable")
> 
> About the rest, after QLogic test the functionality I'll clean-up the
> empty lines and re-send it.

Can you please not quote an entire patch just to make a small series
of comments?

Just quote the exact relevant portions of the patch if you want to
specifically make comments about something.

Quoting the entire patch is extremely bad netiquette, wastes
bandwidth, and everyone on the list has to receive a whole new copy of
the entire patch again for no good reason.

RE: [PATCH net-next v2] netxen: write IP address to firmware when using bonding

[Rajesh Borundia]

>-----Original Message-----
>From: Nikolay Aleksandrov [mailto:nikolay@redhat.com]
>Sent: Wednesday, October 03, 2012 2:50 PM
>To: Sony Chacko
>Cc: agospoda@redhat.com; Rajesh Borundia; David Miller; netdev
>Subject: Re: [PATCH net-next v2] netxen: write IP address to firmware
>when using bonding
>
>On 10/02/2012 11:16 AM, Nikolay Aleksandrov wrote:
>> This patch allows LRO aggregation on bonded devices that contain an
>NX3031
>> device. It also adds a for_each_netdev_in_bond_rcu(bond, slave) macro
>> which executes for each slave that has bond as master.
>>
>> V2: Remove local ip caching, retrieve addresses dynamically and
>>      restore them if necessary.
>>
>> Note: Tested with NX3031 adapter.
>>
>> Signed-off-by: Andy Gospodarek <agospoda@redhat.com>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
>> ---
>>   drivers/net/ethernet/qlogic/netxen/netxen_nic.h    |   6 -
>>   .../net/ethernet/qlogic/netxen/netxen_nic_main.c   | 192
>+++++++++++----------
>>   include/linux/netdevice.h                          |   3 +
>>   3 files changed, 100 insertions(+), 101 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
>b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
>> index eb3dfdb..cb4f2ce 100644
>> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
>> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic.h
>> @@ -955,11 +955,6 @@ typedef struct nx_mac_list_s {
>>   	uint8_t mac_addr[ETH_ALEN+2];
>>   } nx_mac_list_t;
>>
>> -struct nx_vlan_ip_list {
>> -	struct list_head list;
>> -	__be32 ip_addr;
>> -};
>> -
>>   /*
>>    * Interrupt coalescing defaults. The defaults are for 1500 MTU. It
>is
>>    * adjusted based on configured MTU.
>> @@ -1605,7 +1600,6 @@ struct netxen_adapter {
>>   	struct net_device *netdev;
>>   	struct pci_dev *pdev;
>>   	struct list_head mac_list;
>> -	struct list_head vlan_ip_list;
>>
>>   	spinlock_t tx_clean_lock;
>>
>> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>> index e2a4858..8e3eb61 100644
>> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>> @@ -90,7 +90,6 @@ static irqreturn_t netxen_intr(int irq, void *data);
>>   static irqreturn_t netxen_msi_intr(int irq, void *data);
>>   static irqreturn_t netxen_msix_intr(int irq, void *data);
>>
>> -static void netxen_free_vlan_ip_list(struct netxen_adapter *);
>>   static void netxen_restore_indev_addr(struct net_device *dev,
>unsigned long);
>>   static struct rtnl_link_stats64 *netxen_nic_get_stats(struct
>net_device *dev,
>>   						      struct rtnl_link_stats64
>*stats);
>> @@ -1451,7 +1450,6 @@ netxen_nic_probe(struct pci_dev *pdev, const
>struct pci_device_id *ent)
>>
>>   	spin_lock_init(&adapter->tx_clean_lock);
>>   	INIT_LIST_HEAD(&adapter->mac_list);
>> -	INIT_LIST_HEAD(&adapter->vlan_ip_list);
>>
>>   	err = netxen_setup_pci_map(adapter);
>>   	if (err)
>> @@ -1586,7 +1584,7 @@ static void __devexit netxen_nic_remove(struct
>pci_dev *pdev)
>>
>>   	cancel_work_sync(&adapter->tx_timeout_task);
>>
>> -	netxen_free_vlan_ip_list(adapter);
>> +	netxen_restore_indev_addr(netdev, NETDEV_DOWN);
>>   	netxen_nic_detach(adapter);
>>
>>   	nx_decr_dev_ref_cnt(adapter);
>> @@ -3135,66 +3133,22 @@ netxen_destip_supported(struct netxen_adapter
>*adapter)
>>   	return 1;
>>   }
>>
>> -static void
>> -netxen_free_vlan_ip_list(struct netxen_adapter *adapter)
>> +static inline __be32
>> +netxen_get_addr(struct net_device *dev)
>>   {
>> -	struct nx_vlan_ip_list  *cur;
>> -	struct list_head *head = &adapter->vlan_ip_list;
>> -
>> -	while (!list_empty(head)) {
>> -		cur = list_entry(head->next, struct nx_vlan_ip_list, list);
>> -		netxen_config_ipaddr(adapter, cur->ip_addr, NX_IP_DOWN);
>> -		list_del(&cur->list);
>> -		kfree(cur);
>> -	}
>> -
>> -}
>> -static void
>> -netxen_list_config_vlan_ip(struct netxen_adapter *adapter,
>> -		struct in_ifaddr *ifa, unsigned long event)
>> -{
>> -	struct net_device *dev;
>> -	struct nx_vlan_ip_list *cur, *tmp_cur;
>> -	struct list_head *head;
>> -
>> -	dev = ifa->ifa_dev ? ifa->ifa_dev->dev : NULL;
>> -
>> -	if (dev == NULL)
>> -		return;
>> -
>> -	if (!is_vlan_dev(dev))
>> -		return;
>> +	struct in_device *in_dev;
>> +	__be32 addr = 0;
>>
>> -	switch (event) {
>> -	case NX_IP_UP:
>> -		list_for_each(head, &adapter->vlan_ip_list) {
>> -			cur = list_entry(head, struct nx_vlan_ip_list, list);
>> +	rcu_read_lock();
>> +	in_dev = __in_dev_get_rcu(dev);
>>
>> -			if (cur->ip_addr == ifa->ifa_address)
>> -				return;
>> -		}
>> +	if (in_dev)
>> +		addr = inet_confirm_addr(in_dev, 0, 0, RT_SCOPE_HOST);
>>
>> -		cur = kzalloc(sizeof(struct nx_vlan_ip_list), GFP_ATOMIC);
>> -		if (cur == NULL) {
>> -			printk(KERN_ERR "%s: failed to add vlan ip to list\n",
>> -					adapter->netdev->name);
>> -			return;
>> -		}
>> -
>> -		cur->ip_addr = ifa->ifa_address;
>> -		list_add_tail(&cur->list, &adapter->vlan_ip_list);
>> -		break;
>> -	case NX_IP_DOWN:
>> -		list_for_each_entry_safe(cur, tmp_cur,
>> -					&adapter->vlan_ip_list, list) {
>> -			if (cur->ip_addr == ifa->ifa_address) {
>> -				list_del(&cur->list);
>> -				kfree(cur);
>> -				break;
>> -			}
>> -		}
>> -	}
>> +	rcu_read_unlock();
>> +	return addr;
>>   }
>> +
>>   static void
>>   netxen_config_indev_addr(struct netxen_adapter *adapter,
>>   		struct net_device *dev, unsigned long event)
>> @@ -3213,12 +3167,10 @@ netxen_config_indev_addr(struct netxen_adapter
>*adapter,
>>   		case NETDEV_UP:
>>   			netxen_config_ipaddr(adapter,
>>   					ifa->ifa_address, NX_IP_UP);
>> -			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
>>   			break;
>>   		case NETDEV_DOWN:
>>   			netxen_config_ipaddr(adapter,
>>   					ifa->ifa_address, NX_IP_DOWN);
>> -			netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
>>   			break;
>>   		default:
>>   			break;
>> @@ -3233,15 +3185,54 @@ netxen_restore_indev_addr(struct net_device
>*netdev, unsigned long event)
>>
>>   {
>>   	struct netxen_adapter *adapter = netdev_priv(netdev);
>> -	struct nx_vlan_ip_list *pos, *tmp_pos;
>> +	struct net_device *vlan_dev, *real_dev;
>>   	unsigned long ip_event;
>> +	__be32 addr = 0;
>>
>>   	ip_event = (event == NETDEV_UP) ? NX_IP_UP : NX_IP_DOWN;
>>   	netxen_config_indev_addr(adapter, netdev, event);
>>
>> -	list_for_each_entry_safe(pos, tmp_pos, &adapter->vlan_ip_list,
>list) {
>> -		netxen_config_ipaddr(adapter, pos->ip_addr, ip_event);
>> +	rcu_read_lock();
>> +	for_each_netdev_rcu(&init_net, vlan_dev) {
>> +		if (vlan_dev->priv_flags & IFF_802_1Q_VLAN) {
>> +			real_dev = vlan_dev_real_dev(vlan_dev);
>> +
>> +			if (real_dev == netdev) {
>> +				addr = netxen_get_addr(vlan_dev);
>> +
>> +				if (addr) {
>> +					netxen_config_ipaddr(adapter, addr,
>> +							     ip_event);
>> +				}
>> +			}
>> +		}
>>   	}
>> +
>> +	if (netdev->master) {
>> +		addr = netxen_get_addr(netdev->master);
>> +		if (addr)
>> +			netxen_config_ipaddr(adapter, addr, ip_event);
>> +	}
>> +	rcu_read_unlock();
>> +}
>> +
>> +static inline int
>> +netxen_config_checkdev(struct net_device *dev)
>> +{
>> +	struct netxen_adapter *adapter;
>> +
>> +	if (!is_netxen_netdev(dev))
>> +		return -ENODEV;
>> +
>> +	adapter = netdev_priv(dev);
>> +
>> +	if (!adapter)
>> +		return -ENODEV;
>> +
>> +	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
>> +		return -ENODEV;
>> +
>> +	return 0;
>>   }
>>
>>   static int netxen_netdev_event(struct notifier_block *this,
>> @@ -3260,18 +3251,26 @@ recheck:
>>   		goto recheck;
>>   	}
>>
>> -	if (!is_netxen_netdev(dev))
>> -		goto done;
>> +	/* If this is a bonding device, look for netxen-based slaves*/
>> +	if (dev->priv_flags & IFF_BONDING) {
>> +		struct net_device *slave;
>>
>> -	adapter = netdev_priv(dev);
>> +		rcu_read_lock();
>> +		for_each_netdev_in_bond_rcu(dev, slave) {
>> +			if (netxen_config_checkdev(slave) < 0)
>> +				continue;
>>
>> -	if (!adapter)
>> -		goto done;
>> -
>> -	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
>> -		goto done;
>> +			adapter = netdev_priv(slave);
>> +			netxen_config_indev_addr(adapter, orig_dev, event);
>> +		}
>> +		rcu_read_unlock();
>> +	} else {
>> +		if (netxen_config_checkdev(dev) < 0)
>> +			goto done;
>>
>> -	netxen_config_indev_addr(adapter, orig_dev, event);
>> +		adapter = netdev_priv(dev);
>> +		netxen_config_indev_addr(adapter, orig_dev, event);
>> +	}
>>   done:
>>   	return NOTIFY_DONE;
>>   }
>> @@ -3282,10 +3281,11 @@ netxen_inetaddr_event(struct notifier_block
>*this,
>>   {
>>   	struct netxen_adapter *adapter;
>>   	struct net_device *dev;
>> -
>> +	unsigned long ip_event;
>>   	struct in_ifaddr *ifa = (struct in_ifaddr *)ptr;
>>
>>   	dev = ifa->ifa_dev ? ifa->ifa_dev->dev : NULL;
>> +	ip_event = (event == NETDEV_UP) ? NX_IP_UP : NX_IP_DOWN;
>>
>>   recheck:
>>   	if (dev == NULL)
>> @@ -3296,30 +3296,35 @@ recheck:
>>   		goto recheck;
>>   	}
>>
>> -	if (!is_netxen_netdev(dev))
>> -		goto done;
>> +	/* If this is a bonding device, look for netxen-based slaves*/
>> +	if (dev->priv_flags & IFF_BONDING) {
>> +		struct net_device *slave;
>>
>> -	adapter = netdev_priv(dev);
>> +		rcu_read_lock();
>> +		for_each_netdev_in_bond_rcu(dev, slave) {
>> +			if (netxen_config_checkdev(slave) < 0)
>> +				continue;
>>
>> -	if (!adapter || !netxen_destip_supported(adapter))
>> -		goto done;
>> +			adapter = netdev_priv(slave);
>>
>> -	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
>> -		goto done;
>> +			if (!netxen_destip_supported(adapter))
>> +				continue;
>>
>> -	switch (event) {
>> -	case NETDEV_UP:
>> -		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_UP);
>> -		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_UP);
>> -		break;
>> -	case NETDEV_DOWN:
>> -		netxen_config_ipaddr(adapter, ifa->ifa_address, NX_IP_DOWN);
>> -		netxen_list_config_vlan_ip(adapter, ifa, NX_IP_DOWN);
>> -		break;
>> -	default:
>> -		break;
>> -	}
>> +			netxen_config_ipaddr(adapter, ifa->ifa_address,
>> +					     ip_event);
>> +		}
>> +		rcu_read_unlock();
>> +	} else {
>> +		if (netxen_config_checkdev(dev) < 0)
>> +			goto done;
>> +
>> +		adapter = netdev_priv(dev);
>>
>> +		if (!netxen_destip_supported(adapter))
>> +			goto done;
>> +
>> +		netxen_config_ipaddr(adapter, ifa->ifa_address, ip_event);
>> +	}
>>   done:
>>   	return NOTIFY_DONE;
>>   }
>> @@ -3335,9 +3340,6 @@ static struct notifier_block netxen_inetaddr_cb
>= {
>>   static void
>>   netxen_restore_indev_addr(struct net_device *dev, unsigned long
>event)
>>   { }
>> -static void
>> -netxen_free_vlan_ip_list(struct netxen_adapter *adapter)
>> -{ }
>>   #endif
>>
>>   static struct pci_error_handlers netxen_err_handler = {
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 59dc05f3..463bb40 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1578,6 +1578,9 @@ extern rwlock_t
>	dev_base_lock;		/* Device list lock */
>>   		list_for_each_entry_continue(d, &(net)->dev_base_head,
>dev_list)
>>   #define for_each_netdev_continue_rcu(net, d)		\
>>   	list_for_each_entry_continue_rcu(d, &(net)->dev_base_head,
>dev_list)
>> +#define for_each_netdev_in_bond_rcu(bond, slave)	\
>> +	for_each_netdev_rcu(&init_net, slave)		\
>> +		if (slave->master == bond)
>>   #define net_device_entry(lh)	list_entry(lh, struct net_device,
>dev_list)
>>
>>   static inline struct net_device *next_net_device(struct net_device
>*dev)
>Hello Dave,
>I just synced with upstream, I've missed a few patches and it seems
>that it doesn't apply cleanly because my previous patch was
>changed before it was applied. There is one character missing from
>a comment - "/* root bus? */", in upstream it was changed to
>/* root bus */.
>("netxen: check for root bus in netxen_mask_aer_correctable")
>
>About the rest, after QLogic test the functionality I'll clean-up the
>empty lines and re-send it.
>
>Thank you,
>  Nik
>
Hello Nikolay,

While testing the patch I found that mac address does not get added when
We perform following steps.
a. bond is created and assigned the ip address.
b. then the slave device is enslaved.

We add ip address in case of NETDEV_UP event. But in above case after enslave is done
we get NETDEV_FEAT_CHANGE event so ip address does not get added. 

Rajesh