[patch next 0/4] netxen fixes

[patch next 0/4] netxen fixes

[Dhananjay Phadke]

Dave,

Series of 4 fixes for net-next-2.6, please apply.

Thanks.

[patch next 1/4] netxen: fix race in tx ring acccess

[Dhananjay Phadke]

Fix the distance check between tx ring producer and consumer that
could lead to tx ring wrap around.

Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
---
 drivers/net/netxen/netxen_nic_hw.c   |    4 ++--
 drivers/net/netxen/netxen_nic_init.c |    8 ++++----
 drivers/net/netxen/netxen_nic_main.c |    3 ++-
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic_hw.c b/drivers/net/netxen/netxen_nic_hw.c
index 9f5ced3..23c974a 100644
--- a/drivers/net/netxen/netxen_nic_hw.c
+++ b/drivers/net/netxen/netxen_nic_hw.c
@@ -526,7 +526,7 @@ netxen_send_cmd_descs(struct netxen_adapter *adapter,
 	producer = tx_ring->producer;
 	consumer = tx_ring->sw_consumer;
 
-	if (nr_desc > find_diff_among(producer, consumer, tx_ring->num_desc)) {
+	if (nr_desc >= find_diff_among(producer, consumer, tx_ring->num_desc)) {
 		netif_tx_unlock_bh(adapter->netdev);
 		return -EBUSY;
 	}
@@ -752,7 +752,7 @@ int netxen_linkevent_request(struct netxen_adapter *adapter, int enable)
 
 	word = NX_NIC_H2C_OPCODE_GET_LINKEVENT | ((u64)adapter->portnum << 16);
 	req.req_hdr = cpu_to_le64(word);
-	req.words[0] = cpu_to_le64(enable);
+	req.words[0] = cpu_to_le64(enable | (enable << 8));
 
 	rv = netxen_send_cmd_descs(adapter, (struct cmd_desc_type0 *)&req, 1);
 	if (rv != 0) {
diff --git a/drivers/net/netxen/netxen_nic_init.c b/drivers/net/netxen/netxen_nic_init.c
index d182167..de9ebbd 100644
--- a/drivers/net/netxen/netxen_nic_init.c
+++ b/drivers/net/netxen/netxen_nic_init.c
@@ -1319,10 +1319,11 @@ int netxen_process_cmd_ring(struct netxen_adapter *adapter)
 			break;
 	}
 
-	if (count) {
-		tx_ring->sw_consumer = sw_consumer;
+	tx_ring->sw_consumer = sw_consumer;
+
+	if (count && netif_running(netdev)) {
 		smp_mb();
-		if (netif_queue_stopped(netdev) && netif_running(netdev)) {
+		if (netif_queue_stopped(netdev) && netif_carrier_ok(netdev)) {
 			netif_tx_lock(netdev);
 			netif_wake_queue(netdev);
 			smp_mb();
@@ -1450,7 +1451,6 @@ netxen_post_rx_buffers_nodb(struct netxen_adapter *adapter,
 		rds_ring->producer = producer;
 		NXWR32(adapter, rds_ring->crb_rcv_producer,
 				(producer - 1) & (rds_ring->num_desc - 1));
-			wmb();
 	}
 	spin_unlock(&rds_ring->lock);
 }
diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index 5d79c19..6a496b1 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -809,6 +809,7 @@ netxen_nic_down(struct netxen_adapter *adapter, struct net_device *netdev)
 {
 	netif_carrier_off(netdev);
 	netif_stop_queue(netdev);
+	smp_mb();
 	netxen_napi_disable(adapter);
 
 	if (adapter->stop_port)
@@ -1340,7 +1341,7 @@ netxen_nic_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 	producer = tx_ring->producer;
 	smp_mb();
 	consumer = tx_ring->sw_consumer;
-	if ((no_of_desc+2) > find_diff_among(producer, consumer, num_txd)) {
+	if ((no_of_desc+2) >= find_diff_among(producer, consumer, num_txd)) {
 		netif_stop_queue(netdev);
 		smp_mb();
 		return NETDEV_TX_BUSY;
-- 
1.6.0.2

Re: [patch next 1/4] netxen: fix race in tx ring acccess

[David Miller]

From: Dhananjay Phadke <dhananjay@netxen.com>
Date: Tue,  5 May 2009 22:05:06 -0700

> Fix the distance check between tx ring producer and consumer that
> could lead to tx ring wrap around.
> 
> Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>

Applied.

[patch next 2/4] netxen: fix mac list management

[Dhananjay Phadke]

o use standard linked list api for mac addr list management
  in NX3031.
o release mac addresses in firmware in dev close().

Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
---
 drivers/net/netxen/netxen_nic.h      |    6 +-
 drivers/net/netxen/netxen_nic_hw.c   |  124 ++++++++++++++--------------------
 drivers/net/netxen/netxen_nic_main.c |    7 +-
 3 files changed, 58 insertions(+), 79 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
index 8dacfbb..d368e24 100644
--- a/drivers/net/netxen/netxen_nic.h
+++ b/drivers/net/netxen/netxen_nic.h
@@ -1019,8 +1019,8 @@ typedef struct {
 #define NETXEN_MAC_DEL	2
 
 typedef struct nx_mac_list_s {
-	struct nx_mac_list_s *next;
-	uint8_t mac_addr[MAX_ADDR_LEN];
+	struct list_head list;
+	uint8_t mac_addr[ETH_ALEN+2];
 } nx_mac_list_t;
 
 /*
@@ -1213,7 +1213,7 @@ struct netxen_adapter {
 
 	struct net_device *netdev;
 	struct pci_dev *pdev;
-	nx_mac_list_t	*mac_list;
+	struct list_head mac_list;
 
 	u32 curr_window;
 	u32 crb_win;
diff --git a/drivers/net/netxen/netxen_nic_hw.c b/drivers/net/netxen/netxen_nic_hw.c
index 23c974a..be643ea 100644
--- a/drivers/net/netxen/netxen_nic_hw.c
+++ b/drivers/net/netxen/netxen_nic_hw.c
@@ -470,45 +470,6 @@ void netxen_p2_nic_set_multi(struct net_device *netdev)
 		netxen_nic_set_mcast_addr(adapter, index, null_addr);
 }
 
-static int nx_p3_nic_add_mac(struct netxen_adapter *adapter,
-		u8 *addr, nx_mac_list_t **add_list, nx_mac_list_t **del_list)
-{
-	nx_mac_list_t *cur, *prev;
-
-	/* if in del_list, move it to adapter->mac_list */
-	for (cur = *del_list, prev = NULL; cur;) {
-		if (memcmp(addr, cur->mac_addr, ETH_ALEN) == 0) {
-			if (prev == NULL)
-				*del_list = cur->next;
-			else
-				prev->next = cur->next;
-			cur->next = adapter->mac_list;
-			adapter->mac_list = cur;
-			return 0;
-		}
-		prev = cur;
-		cur = cur->next;
-	}
-
-	/* make sure to add each mac address only once */
-	for (cur = adapter->mac_list; cur; cur = cur->next) {
-		if (memcmp(addr, cur->mac_addr, ETH_ALEN) == 0)
-			return 0;
-	}
-	/* not in del_list, create new entry and add to add_list */
-	cur = kmalloc(sizeof(*cur), in_atomic()? GFP_ATOMIC : GFP_KERNEL);
-	if (cur == NULL) {
-		printk(KERN_ERR "%s: cannot allocate memory. MAC filtering may"
-				"not work properly from now.\n", __func__);
-		return -1;
-	}
-
-	memcpy(cur->mac_addr, addr, ETH_ALEN);
-	cur->next = *add_list;
-	*add_list = cur;
-	return 0;
-}
-
 static int
 netxen_send_cmd_descs(struct netxen_adapter *adapter,
 		struct cmd_desc_type0 *cmd_desc_arr, int nr_desc)
@@ -555,14 +516,12 @@ netxen_send_cmd_descs(struct netxen_adapter *adapter,
 	return 0;
 }
 
-static int nx_p3_sre_macaddr_change(struct net_device *dev,
-		u8 *addr, unsigned op)
+static int
+nx_p3_sre_macaddr_change(struct netxen_adapter *adapter, u8 *addr, unsigned op)
 {
-	struct netxen_adapter *adapter = netdev_priv(dev);
 	nx_nic_req_t req;
 	nx_mac_req_t *mac_req;
 	u64 word;
-	int rv;
 
 	memset(&req, 0, sizeof(nx_nic_req_t));
 	req.qhdr = cpu_to_le64(NX_NIC_REQUEST << 23);
@@ -574,28 +533,51 @@ static int nx_p3_sre_macaddr_change(struct net_device *dev,
 	mac_req->op = op;
 	memcpy(mac_req->mac_addr, addr, 6);
 
-	rv = netxen_send_cmd_descs(adapter, (struct cmd_desc_type0 *)&req, 1);
-	if (rv != 0) {
-		printk(KERN_ERR "ERROR. Could not send mac update\n");
-		return rv;
+	return netxen_send_cmd_descs(adapter, (struct cmd_desc_type0 *)&req, 1);
+}
+
+static int nx_p3_nic_add_mac(struct netxen_adapter *adapter,
+		u8 *addr, struct list_head *del_list)
+{
+	struct list_head *head;
+	nx_mac_list_t *cur;
+
+	/* look up if already exists */
+	list_for_each(head, del_list) {
+		cur = list_entry(head, nx_mac_list_t, list);
+
+		if (memcmp(addr, cur->mac_addr, ETH_ALEN) == 0) {
+			list_move_tail(head, &adapter->mac_list);
+			return 0;
+		}
 	}
 
-	return 0;
+	cur = kzalloc(sizeof(nx_mac_list_t), GFP_ATOMIC);
+	if (cur == NULL) {
+		printk(KERN_ERR "%s: failed to add mac address filter\n",
+				adapter->netdev->name);
+		return -ENOMEM;
+	}
+	memcpy(cur->mac_addr, addr, ETH_ALEN);
+	list_add_tail(&cur->list, &adapter->mac_list);
+	return nx_p3_sre_macaddr_change(adapter,
+				cur->mac_addr, NETXEN_MAC_ADD);
 }
 
 void netxen_p3_nic_set_multi(struct net_device *netdev)
 {
 	struct netxen_adapter *adapter = netdev_priv(netdev);
-	nx_mac_list_t *cur, *next, *del_list, *add_list = NULL;
 	struct dev_mc_list *mc_ptr;
 	u8 bcast_addr[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 	u32 mode = VPORT_MISS_MODE_DROP;
+	LIST_HEAD(del_list);
+	struct list_head *head;
+	nx_mac_list_t *cur;
 
-	del_list = adapter->mac_list;
-	adapter->mac_list = NULL;
+	list_splice_tail_init(&adapter->mac_list, &del_list);
 
-	nx_p3_nic_add_mac(adapter, netdev->dev_addr, &add_list, &del_list);
-	nx_p3_nic_add_mac(adapter, bcast_addr, &add_list, &del_list);
+	nx_p3_nic_add_mac(adapter, netdev->dev_addr, &del_list);
+	nx_p3_nic_add_mac(adapter, bcast_addr, &del_list);
 
 	if (netdev->flags & IFF_PROMISC) {
 		mode = VPORT_MISS_MODE_ACCEPT_ALL;
@@ -611,25 +593,20 @@ void netxen_p3_nic_set_multi(struct net_device *netdev)
 	if (netdev->mc_count > 0) {
 		for (mc_ptr = netdev->mc_list; mc_ptr;
 		     mc_ptr = mc_ptr->next) {
-			nx_p3_nic_add_mac(adapter, mc_ptr->dmi_addr,
-					  &add_list, &del_list);
+			nx_p3_nic_add_mac(adapter, mc_ptr->dmi_addr, &del_list);
 		}
 	}
 
 send_fw_cmd:
 	adapter->set_promisc(adapter, mode);
-	for (cur = del_list; cur;) {
-		nx_p3_sre_macaddr_change(netdev, cur->mac_addr, NETXEN_MAC_DEL);
-		next = cur->next;
+	head = &del_list;
+	while (!list_empty(head)) {
+		cur = list_entry(head->next, nx_mac_list_t, list);
+
+		nx_p3_sre_macaddr_change(adapter,
+				cur->mac_addr, NETXEN_MAC_DEL);
+		list_del(&cur->list);
 		kfree(cur);
-		cur = next;
-	}
-	for (cur = add_list; cur;) {
-		nx_p3_sre_macaddr_change(netdev, cur->mac_addr, NETXEN_MAC_ADD);
-		next = cur->next;
-		cur->next = adapter->mac_list;
-		adapter->mac_list = cur;
-		cur = next;
 	}
 }
 
@@ -654,14 +631,15 @@ int netxen_p3_nic_set_promisc(struct netxen_adapter *adapter, u32 mode)
 
 void netxen_p3_free_mac_list(struct netxen_adapter *adapter)
 {
-	nx_mac_list_t *cur, *next;
-
-	cur = adapter->mac_list;
-
-	while (cur) {
-		next = cur->next;
+	nx_mac_list_t *cur;
+	struct list_head *head = &adapter->mac_list;
+
+	while (!list_empty(head)) {
+		cur = list_entry(head->next, nx_mac_list_t, list);
+		nx_p3_sre_macaddr_change(adapter,
+				cur->mac_addr, NETXEN_MAC_DEL);
+		list_del(&cur->list);
 		kfree(cur);
-		cur = next;
 	}
 }
 
diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index 6a496b1..a5577f8 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -815,6 +815,9 @@ netxen_nic_down(struct netxen_adapter *adapter, struct net_device *netdev)
 	if (adapter->stop_port)
 		adapter->stop_port(adapter);
 
+	if (NX_IS_REVISION_P3(adapter->ahw.revision_id))
+		netxen_p3_free_mac_list(adapter);
+
 	netxen_release_tx_buffers(adapter);
 
 	FLUSH_SCHEDULED_WORK();
@@ -961,6 +964,7 @@ netxen_nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	rwlock_init(&adapter->adapter_lock);
 	spin_lock_init(&adapter->tx_clean_lock);
+	INIT_LIST_HEAD(&adapter->mac_list);
 
 	err = netxen_setup_pci_map(adapter);
 	if (err)
@@ -1114,9 +1118,6 @@ static void __devexit netxen_nic_remove(struct pci_dev *pdev)
 
 	if (adapter->is_up == NETXEN_ADAPTER_UP_MAGIC) {
 		netxen_nic_detach(adapter);
-
-		if (NX_IS_REVISION_P3(adapter->ahw.revision_id))
-			netxen_p3_free_mac_list(adapter);
 	}
 
 	if (adapter->portnum == 0)
-- 
1.6.0.2

Re: [patch next 2/4] netxen: fix mac list management

[David Miller]

From: Dhananjay Phadke <dhananjay@netxen.com>
Date: Tue,  5 May 2009 22:05:07 -0700

> o use standard linked list api for mac addr list management
>   in NX3031.
> o release mac addresses in firmware in dev close().
> 
> Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>

Applied.

[patch next 3/4] netxen: fix bonding support

[Dhananjay Phadke]

o Pause traffic during mac addr change.
o Enable setting mac address for NX3031.

Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
---
 drivers/net/netxen/netxen_nic.h      |    6 +-
 drivers/net/netxen/netxen_nic_hw.c   |   28 ++-----
 drivers/net/netxen/netxen_nic_hw.h   |   10 --
 drivers/net/netxen/netxen_nic_init.c |    7 +-
 drivers/net/netxen/netxen_nic_main.c |   45 +++++++---
 drivers/net/netxen/netxen_nic_niu.c  |  154 +++++++--------------------------
 6 files changed, 82 insertions(+), 168 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
index d368e24..9350c86 100644
--- a/drivers/net/netxen/netxen_nic.h
+++ b/drivers/net/netxen/netxen_nic.h
@@ -1264,9 +1264,10 @@ struct netxen_adapter {
 
 	int (*enable_phy_interrupts) (struct netxen_adapter *);
 	int (*disable_phy_interrupts) (struct netxen_adapter *);
-	int (*macaddr_set) (struct netxen_adapter *, netxen_ethernet_macaddr_t);
+	int (*macaddr_set) (struct netxen_adapter *, u8 *);
 	int (*set_mtu) (struct netxen_adapter *, int);
 	int (*set_promisc) (struct netxen_adapter *, u32);
+	void (*set_multi) (struct net_device *);
 	int (*phy_read) (struct netxen_adapter *, long reg, u32 *);
 	int (*phy_write) (struct netxen_adapter *, long reg, u32 val);
 	int (*init_port) (struct netxen_adapter *, int);
@@ -1331,6 +1332,9 @@ int netxen_niu_gbe_phy_write(struct netxen_adapter *adapter,
 int netxen_nic_set_mtu_xgb(struct netxen_adapter *adapter, int new_mtu);
 int netxen_nic_set_mtu_gb(struct netxen_adapter *adapter, int new_mtu);
 
+int netxen_p2_nic_set_mac_addr(struct netxen_adapter *adapter, u8 *addr);
+int netxen_p3_nic_set_mac_addr(struct netxen_adapter *adapter, u8 *addr);
+
 #define NXRD32(adapter, off) \
 	(adapter->hw_read_wx(adapter, off))
 #define NXWR32(adapter, off, val) \
diff --git a/drivers/net/netxen/netxen_nic_hw.c b/drivers/net/netxen/netxen_nic_hw.c
index be643ea..86c9e78 100644
--- a/drivers/net/netxen/netxen_nic_hw.c
+++ b/drivers/net/netxen/netxen_nic_hw.c
@@ -321,27 +321,6 @@ static unsigned crb_hub_agt[64] =
 
 #define NETXEN_WINDOW_ONE 	0x2000000 /*CRB Window: bit 25 of CRB address */
 
-int netxen_nic_set_mac(struct net_device *netdev, void *p)
-{
-	struct netxen_adapter *adapter = netdev_priv(netdev);
-	struct sockaddr *addr = p;
-
-	if (netif_running(netdev))
-		return -EBUSY;
-
-	if (!is_valid_ether_addr(addr->sa_data))
-		return -EADDRNOTAVAIL;
-
-	memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len);
-
-	/* For P3, MAC addr is not set in NIU */
-	if (NX_IS_REVISION_P2(adapter->ahw.revision_id))
-		if (adapter->macaddr_set)
-			adapter->macaddr_set(adapter, addr->sa_data);
-
-	return 0;
-}
-
 #define NETXEN_UNICAST_ADDR(port, index) \
 	(NETXEN_UNICAST_ADDR_BASE+(port*32)+(index*8))
 #define NETXEN_MCAST_ADDR(port, index) \
@@ -643,6 +622,13 @@ void netxen_p3_free_mac_list(struct netxen_adapter *adapter)
 	}
 }
 
+int netxen_p3_nic_set_mac_addr(struct netxen_adapter *adapter, u8 *addr)
+{
+	/* assuming caller has already copied new addr to netdev */
+	netxen_p3_nic_set_multi(adapter->netdev);
+	return 0;
+}
+
 #define	NETXEN_CONFIG_INTR_COALESCE	3
 
 /*
diff --git a/drivers/net/netxen/netxen_nic_hw.h b/drivers/net/netxen/netxen_nic_hw.h
index f20c965..d4e8333 100644
--- a/drivers/net/netxen/netxen_nic_hw.h
+++ b/drivers/net/netxen/netxen_nic_hw.h
@@ -42,8 +42,6 @@ struct netxen_adapter;
 
 void netxen_nic_set_link_parameters(struct netxen_adapter *adapter);
 
-typedef u8 netxen_ethernet_macaddr_t[6];
-
 /* Nibble or Byte mode for phy interface (GbE mode only) */
 
 #define _netxen_crb_get_bit(var, bit)  ((var >> bit) & 0x1)
@@ -395,14 +393,6 @@ int netxen_niu_set_promiscuous_mode(struct netxen_adapter *adapter,
 int netxen_niu_xg_set_promiscuous_mode(struct netxen_adapter *adapter,
 				       u32 mode);
 
-/* set the MAC address for a given MAC */
-int netxen_niu_macaddr_set(struct netxen_adapter *adapter,
-			   netxen_ethernet_macaddr_t addr);
-
-/* XG version */
-int netxen_niu_xg_macaddr_set(struct netxen_adapter *adapter,
-			      netxen_ethernet_macaddr_t addr);
-
 /* Generic enable for GbE ports. Will detect the speed of the link. */
 int netxen_niu_gbe_init_port(struct netxen_adapter *adapter, int port);
 
diff --git a/drivers/net/netxen/netxen_nic_init.c b/drivers/net/netxen/netxen_nic_init.c
index de9ebbd..4a51c31 100644
--- a/drivers/net/netxen/netxen_nic_init.c
+++ b/drivers/net/netxen/netxen_nic_init.c
@@ -319,13 +319,15 @@ err_out:
 
 void netxen_initialize_adapter_ops(struct netxen_adapter *adapter)
 {
+	adapter->macaddr_set = netxen_p2_nic_set_mac_addr;
+	adapter->set_multi = netxen_p2_nic_set_multi;
+
 	switch (adapter->ahw.port_type) {
 	case NETXEN_NIC_GBE:
 		adapter->enable_phy_interrupts =
 		    netxen_niu_gbe_enable_phy_interrupts;
 		adapter->disable_phy_interrupts =
 		    netxen_niu_gbe_disable_phy_interrupts;
-		adapter->macaddr_set = netxen_niu_macaddr_set;
 		adapter->set_mtu = netxen_nic_set_mtu_gb;
 		adapter->set_promisc = netxen_niu_set_promiscuous_mode;
 		adapter->phy_read = netxen_niu_gbe_phy_read;
@@ -339,7 +341,6 @@ void netxen_initialize_adapter_ops(struct netxen_adapter *adapter)
 		    netxen_niu_xgbe_enable_phy_interrupts;
 		adapter->disable_phy_interrupts =
 		    netxen_niu_xgbe_disable_phy_interrupts;
-		adapter->macaddr_set = netxen_niu_xg_macaddr_set;
 		adapter->set_mtu = netxen_nic_set_mtu_xgb;
 		adapter->init_port = netxen_niu_xg_init_port;
 		adapter->set_promisc = netxen_niu_xg_set_promiscuous_mode;
@@ -353,6 +354,8 @@ void netxen_initialize_adapter_ops(struct netxen_adapter *adapter)
 	if (NX_IS_REVISION_P3(adapter->ahw.revision_id)) {
 		adapter->set_mtu = nx_fw_cmd_set_mtu;
 		adapter->set_promisc = netxen_p3_nic_set_promisc;
+		adapter->macaddr_set = netxen_p3_nic_set_mac_addr;
+		adapter->set_multi = netxen_p3_nic_set_multi;
 	}
 }
 
diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index a5577f8..83dadfd 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -442,20 +442,38 @@ netxen_read_mac_addr(struct netxen_adapter *adapter)
 
 	if (!is_valid_ether_addr(netdev->perm_addr))
 		dev_warn(&pdev->dev, "Bad MAC address %pM.\n", netdev->dev_addr);
-	else
-		adapter->macaddr_set(adapter, netdev->dev_addr);
 
 	return 0;
 }
 
+int netxen_nic_set_mac(struct net_device *netdev, void *p)
+{
+	struct netxen_adapter *adapter = netdev_priv(netdev);
+	struct sockaddr *addr = p;
+
+	if (!is_valid_ether_addr(addr->sa_data))
+		return -EINVAL;
+
+	if (netif_running(netdev)) {
+		netif_device_detach(netdev);
+		netxen_napi_disable(adapter);
+	}
+
+	memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len);
+	adapter->macaddr_set(adapter, addr->sa_data);
+
+	if (netif_running(netdev)) {
+		netif_device_attach(netdev);
+		netxen_napi_enable(adapter);
+	}
+	return 0;
+}
+
 static void netxen_set_multicast_list(struct net_device *dev)
 {
 	struct netxen_adapter *adapter = netdev_priv(dev);
 
-	if (NX_IS_REVISION_P3(adapter->ahw.revision_id))
-		netxen_p3_nic_set_multi(dev);
-	else
-		netxen_p2_nic_set_multi(dev);
+	adapter->set_multi(dev);
 }
 
 static const struct net_device_ops netxen_netdev_ops = {
@@ -782,16 +800,13 @@ netxen_nic_up(struct netxen_adapter *adapter, struct net_device *netdev)
 				netxen_nic_driver_name, adapter->portnum);
 		return err;
 	}
-	adapter->macaddr_set(adapter, netdev->dev_addr);
-
-	netxen_nic_set_link_parameters(adapter);
+	if (NX_IS_REVISION_P2(adapter->ahw.revision_id))
+		adapter->macaddr_set(adapter, netdev->dev_addr);
 
-	netxen_set_multicast_list(netdev);
-	if (adapter->set_mtu)
-		adapter->set_mtu(adapter, netdev->mtu);
+	adapter->set_multi(netdev);
+	adapter->set_mtu(adapter, netdev->mtu);
 
 	adapter->ahw.linkup = 0;
-	mod_timer(&adapter->watchdog_timer, jiffies);
 
 	netxen_napi_enable(adapter);
 
@@ -800,6 +815,10 @@ netxen_nic_up(struct netxen_adapter *adapter, struct net_device *netdev)
 
 	if (adapter->capabilities & NX_FW_CAPABILITY_LINK_NOTIFICATION)
 		netxen_linkevent_request(adapter, 1);
+	else
+		netxen_nic_set_link_parameters(adapter);
+
+	mod_timer(&adapter->watchdog_timer, jiffies);
 
 	return 0;
 }
diff --git a/drivers/net/netxen/netxen_nic_niu.c b/drivers/net/netxen/netxen_nic_niu.c
index 5e2698b..5941c79 100644
--- a/drivers/net/netxen/netxen_nic_niu.c
+++ b/drivers/net/netxen/netxen_nic_niu.c
@@ -402,76 +402,6 @@ int netxen_niu_xg_init_port(struct netxen_adapter *adapter, int port)
 	return 0;
 }
 
-/*
- * Return the current station MAC address.
- * Note that the passed-in value must already be in network byte order.
- */
-static int netxen_niu_macaddr_get(struct netxen_adapter *adapter,
-				  netxen_ethernet_macaddr_t * addr)
-{
-	u32 stationhigh;
-	u32 stationlow;
-	int phy = adapter->physical_port;
-	u8 val[8];
-
-	if (addr == NULL)
-		return -EINVAL;
-	if ((phy < 0) || (phy > 3))
-		return -EINVAL;
-
-	stationhigh = NXRD32(adapter, NETXEN_NIU_GB_STATION_ADDR_0(phy));
-	stationlow = NXRD32(adapter, NETXEN_NIU_GB_STATION_ADDR_1(phy));
-	((__le32 *)val)[1] = cpu_to_le32(stationhigh);
-	((__le32 *)val)[0] = cpu_to_le32(stationlow);
-
-	memcpy(addr, val + 2, 6);
-
-	return 0;
-}
-
-/*
- * Set the station MAC address.
- * Note that the passed-in value must already be in network byte order.
- */
-int netxen_niu_macaddr_set(struct netxen_adapter *adapter,
-			   netxen_ethernet_macaddr_t addr)
-{
-	u8 temp[4];
-	u32 val;
-	int phy = adapter->physical_port;
-	unsigned char mac_addr[6];
-	int i;
-
-	if (NX_IS_REVISION_P3(adapter->ahw.revision_id))
-		return 0;
-
-	for (i = 0; i < 10; i++) {
-		temp[0] = temp[1] = 0;
-		memcpy(temp + 2, addr, 2);
-		val = le32_to_cpu(*(__le32 *)temp);
-		if (NXWR32(adapter, NETXEN_NIU_GB_STATION_ADDR_1(phy), val))
-			return -EIO;
-
-		memcpy(temp, ((u8 *) addr) + 2, sizeof(__le32));
-		val = le32_to_cpu(*(__le32 *)temp);
-		if (NXWR32(adapter, NETXEN_NIU_GB_STATION_ADDR_0(phy), val))
-			return -2;
-
-		netxen_niu_macaddr_get(adapter,
-				       (netxen_ethernet_macaddr_t *) mac_addr);
-		if (memcmp(mac_addr, addr, 6) == 0)
-			break;
-	}
-
-	if (i == 10) {
-		printk(KERN_ERR "%s: cannot set Mac addr for %s\n",
-		       netxen_nic_driver_name, adapter->netdev->name);
-		printk(KERN_ERR "MAC address set: %pM.\n", addr);
-		printk(KERN_ERR "MAC address get: %pM.\n", mac_addr);
-	}
-	return 0;
-}
-
 /* Disable a GbE interface */
 int netxen_niu_disable_gbe_port(struct netxen_adapter *adapter)
 {
@@ -561,57 +491,6 @@ int netxen_niu_set_promiscuous_mode(struct netxen_adapter *adapter,
 	return 0;
 }
 
-/*
- * Set the MAC address for an XG port
- * Note that the passed-in value must already be in network byte order.
- */
-int netxen_niu_xg_macaddr_set(struct netxen_adapter *adapter,
-			      netxen_ethernet_macaddr_t addr)
-{
-	int phy = adapter->physical_port;
-	u8 temp[4];
-	u32 val;
-
-	if (NX_IS_REVISION_P3(adapter->ahw.revision_id))
-		return 0;
-
-	if ((phy < 0) || (phy > NETXEN_NIU_MAX_XG_PORTS))
-		return -EIO;
-
-	temp[0] = temp[1] = 0;
-	switch (phy) {
-	case 0:
-	    memcpy(temp + 2, addr, 2);
-	    val = le32_to_cpu(*(__le32 *)temp);
-	    if (NXWR32(adapter, NETXEN_NIU_XGE_STATION_ADDR_0_1, val))
-		return -EIO;
-
-	    memcpy(&temp, ((u8 *) addr) + 2, sizeof(__le32));
-	    val = le32_to_cpu(*(__le32 *)temp);
-	    if (NXWR32(adapter, NETXEN_NIU_XGE_STATION_ADDR_0_HI, val))
-		return -EIO;
-	    break;
-
-	case 1:
-	    memcpy(temp + 2, addr, 2);
-	    val = le32_to_cpu(*(__le32 *)temp);
-	    if (NXWR32(adapter, NETXEN_NIU_XG1_STATION_ADDR_0_1, val))
-		return -EIO;
-
-	    memcpy(&temp, ((u8 *) addr) + 2, sizeof(__le32));
-	    val = le32_to_cpu(*(__le32 *)temp);
-	    if (NXWR32(adapter, NETXEN_NIU_XG1_STATION_ADDR_0_HI, val))
-		return -EIO;
-	    break;
-
-	default:
-	    printk(KERN_ERR "Unknown port %d\n", phy);
-	    break;
-	}
-
-	return 0;
-}
-
 int netxen_niu_xg_set_promiscuous_mode(struct netxen_adapter *adapter,
 		u32 mode)
 {
@@ -636,3 +515,36 @@ int netxen_niu_xg_set_promiscuous_mode(struct netxen_adapter *adapter,
 
 	return 0;
 }
+
+int netxen_p2_nic_set_mac_addr(struct netxen_adapter *adapter, u8 *addr)
+{
+	u32 mac_hi, mac_lo;
+	u32 reg_hi, reg_lo;
+
+	u8 phy = adapter->physical_port;
+	u8 phy_count = (adapter->ahw.port_type == NETXEN_NIC_XGBE) ?
+		NETXEN_NIU_MAX_XG_PORTS : NETXEN_NIU_MAX_GBE_PORTS;
+
+	if (phy >= phy_count)
+		return -EINVAL;
+
+	mac_lo = ((u32)addr[0] << 16) | ((u32)addr[1] << 24);
+	mac_hi = addr[2] | ((u32)addr[3] << 8) |
+		((u32)addr[4] << 16) | ((u32)addr[5] << 24);
+
+	if (adapter->ahw.port_type == NETXEN_NIC_XGBE) {
+		reg_lo = NETXEN_NIU_XGE_STATION_ADDR_0_1 + (0x10000 * phy);
+		reg_hi = NETXEN_NIU_XGE_STATION_ADDR_0_HI + (0x10000 * phy);
+	} else {
+		reg_lo = NETXEN_NIU_GB_STATION_ADDR_1(phy);
+		reg_hi = NETXEN_NIU_GB_STATION_ADDR_0(phy);
+	}
+
+	/* write twice to flush */
+	if (NXWR32(adapter, reg_lo, mac_lo) || NXWR32(adapter, reg_hi, mac_hi))
+		return -EIO;
+	if (NXWR32(adapter, reg_lo, mac_lo) || NXWR32(adapter, reg_hi, mac_hi))
+		return -EIO;
+
+	return 0;
+}
-- 
1.6.0.2

Re: [patch next 3/4] netxen: fix bonding support

[David Miller]

From: Dhananjay Phadke <dhananjay@netxen.com>
Date: Tue,  5 May 2009 22:05:08 -0700

> o Pause traffic during mac addr change.
> o Enable setting mac address for NX3031.
> 
> Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>

Applied.

Re: [patch next 3/4] netxen: fix bonding support

[Eric W. Biederman]

Dhananjay Phadke <dhananjay@netxen.com> writes:

> o Pause traffic during mac addr change.
> o Enable setting mac address for NX3031.

In 2.6.31 when I try and use bonding with the netxen or
even simply set the mac address after the driver has
first loaded but before the nic is brought up I get:

As I read that oops. The problem is that netxen_send_cmd_descs
is being called before tx_ring->txq has been initialized.  
tx_ring->txq happens in open, when the device is brought up.

Any chance you can look at the problem?

Eric


BUG: unable to handle kernel NULL pointer dereference at 0000000000000020
IP: [<ffffffffa0128942>] netxen_send_cmd_descs.clone.2+0x26/0xfe [netxen_nic]
PGD 1fe76067 PUD b4d05067 PMD 0 
Oops: 0000 [#1] SMP 
last sysfs file: /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq
CPU 2 
Modules linked in: ipt_REDIRECT vfat fat loop appletalk ipx p8023 rose ax25 tulip xt_hl ipt_LOG xt_limit ipt_REJECT xt_state xt_tcpudp dummy iptable_filter veth macvlan nfsd lockd nfs_acl auth_rpcgss exportfs sco bridge stp bnep l2cap bluetooth sunrpc cpufreq_ondemand acpi_cpufreq freq_table ext4 jbd2 crc16 dm_mirror dm_region_hash dm_log dm_multipath dm_mod uinput bonding ipv6 kvm_intel kvm fuse xt_multiport iptable_nat ip_tables nf_nat x_tables nf_conntrack_ipv4 nf_conntrack nf_defrag_ipv4 tun 8021q i2c_i801 igb i2c_core i5k_amb hwmon i5400_edac edac_core iTCO_wdt iTCO_vendor_support pcspkr shpchp myri10ge floppy netxen_nic sr_mod cdrom serio_raw sg ata_generic pata_acpi ata_piix libata sd_mod scsi_mod ext3 jbd mbcache uhci_hcd ohci_hcd ehci_hcd [last unloaded: microcode]
Pid: 22313, comm: ip Not tainted 2.6.31.2-193574.2006Arora.fc11.x86_64 #1 X7DWU
RIP: 0010:[<ffffffffa0128942>]  [<ffffffffa0128942>] netxen_send_cmd_descs.clone.2+0x26/0xfe [netxen_nic]
RSP: 0018:ffff88005753d638  EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000001 RSI: ffff88005753d688 RDI: ffff88012dc8f580
RBP: ffff88005753d678 R08: ffff88012dc8f580 R09: ffff88005753d688
R10: 00000000a388069b R11: 0000000000000246 R12: ffff88012dc8f580
R13: ffff88001fcb6c00 R14: ffff88005753d738 R15: 0000000000000020
FS:  00007f4360f656f0(0000) GS:ffff88002805a000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000020 CR3: 00000000b4de5000 CR4: 00000000000026e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process ip (pid: 22313, threadinfo ffff88005753c000, task ffff88002fbdb700)
Stack:
 0000000000000000 00000000a388069b ffffea000136a578 00000000a388069b
<0> ffff88005753d778 ffff88012dc8f580 ffff88012e2cf910 ffff88001fcb6c00
<0> ffff88005753d6d8 ffffffffa0128ae5 000000000a000000 0000000000010001
Call Trace:
 [<ffffffffa0128ae5>] nx_p3_sre_macaddr_change+0x60/0x76 [netxen_nic]
 [<ffffffffa0128ca8>] nx_p3_nic_add_mac+0x129/0x14c [netxen_nic]
 [<ffffffffa0128d57>] netxen_p3_nic_set_multi+0x8c/0x169 [netxen_nic]
 [<ffffffffa0128e54>] netxen_p3_nic_set_mac_addr+0x20/0x38 [netxen_nic]
 [<ffffffffa0129ed2>] netxen_nic_set_mac+0x96/0xd6 [netxen_nic]
 [<ffffffff813128b1>] do_setlink+0x17c/0x360
 [<ffffffff81312e97>] rtnl_newlink+0x2e1/0x4e6
 [<ffffffff81312c53>] ? rtnl_newlink+0x9d/0x4e6
 [<ffffffff810ec697>] ? page_add_new_anon_rmap+0x4d/0x73
 [<ffffffff812f8920>] ? sock_rmalloc+0x39/0xa8
 [<ffffffff810fdc8c>] ? virt_to_head_page+0x1c/0x51
 [<ffffffff813122fe>] rtnetlink_rcv_msg+0x1d0/0x201
 [<ffffffff81327856>] ? netlink_sendmsg+0x18f/0x2ac
 [<ffffffff8131212e>] ? rtnetlink_rcv_msg+0x0/0x201
 [<ffffffff81327b7d>] netlink_rcv_skb+0x4d/0xb4
 [<ffffffff81312113>] rtnetlink_rcv+0x30/0x4b
 [<ffffffff8132764a>] netlink_unicast+0x12f/0x1ac
 [<ffffffff81327950>] netlink_sendmsg+0x289/0x2ac
 [<ffffffff812f4146>] __sock_sendmsg+0x6b/0x8a
 [<ffffffff812f4abf>] sock_sendmsg+0xd6/0x103
 [<ffffffff812f494b>] ? sock_recvmsg+0xd9/0x106
 [<ffffffff8106a257>] ? autoremove_wake_function+0x0/0x5a
 [<ffffffff812ff6b4>] ? verify_iovec+0x5b/0xaf
 [<ffffffff812f4d08>] sys_sendmsg+0x21c/0x2a0
 [<ffffffff810e3711>] ? handle_mm_fault+0x2d6/0x681
 [<ffffffff811ced89>] ? __up_read+0x9c/0xbb
 [<ffffffff8106e264>] ? up_read+0x1c/0x32
 [<ffffffff813a9c3f>] ? do_page_fault+0x260/0x2a4
 [<ffffffff8100bdc2>] system_call_fastpath+0x16/0x1b
Code: 5b 41 5c c9 c3 55 48 89 e5 41 55 41 54 49 89 fc 53 48 83 ec 28 65 48 8b 04 25 28 00 00 00 48 89 45 d8 31 c0 48 8b 9f 28 01 00 00 <4c> 8b 6b 20 48 89 75 c8 49 8d 7d 40 e8 5e e9 27 e1 65 8b 04 25 
RIP  [<ffffffffa0128942>] netxen_send_cmd_descs.clone.2+0x26/0xfe [netxen_nic]
 RSP <ffff88005753d638>
CR2: 0000000000000020

RE: [patch next 3/4] netxen: fix bonding support

[Dhananjay Phadke]

Yes, this was a bug until recent times.

It should be fixed in 2.6.32 development cycle by this commit -
db4cfd8a6149e778befb2ff6e6f91cdc6394cbe6 ("netxen: handle firmware load errors").

This added a check for adapter->is_up before touching txq lock.

-Dhananjay
________________________________________
From: Eric W. Biederman [ebiederm@xmission.com]
Sent: Thursday, October 22, 2009 10:57 PM
To: Dhananjay Phadke
Cc: netdev@vger.kernel.org
Subject: Re: [patch next 3/4] netxen: fix bonding support

Dhananjay Phadke <dhananjay@netxen.com> writes:

> o Pause traffic during mac addr change.
> o Enable setting mac address for NX3031.

In 2.6.31 when I try and use bonding with the netxen or
even simply set the mac address after the driver has
first loaded but before the nic is brought up I get:

As I read that oops. The problem is that netxen_send_cmd_descs
is being called before tx_ring->txq has been initialized.
tx_ring->txq happens in open, when the device is brought up.

Any chance you can look at the problem?

Eric


BUG: unable to handle kernel NULL pointer dereference at 0000000000000020
IP: [<ffffffffa0128942>] netxen_send_cmd_descs.clone.2+0x26/0xfe [netxen_nic]
PGD 1fe76067 PUD b4d05067 PMD 0
Oops: 0000 [#1] SMP
last sysfs file: /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq
CPU 2
Modules linked in: ipt_REDIRECT vfat fat loop appletalk ipx p8023 rose ax25 tulip xt_hl ipt_LOG xt_limit ipt_REJECT xt_state xt_tcpudp dummy iptable_filter veth macvlan nfsd lockd nfs_acl auth_rpcgss exportfs sco bridge stp bnep l2cap bluetooth sunrpc cpufreq_ondemand acpi_cpufreq freq_table ext4 jbd2 crc16 dm_mirror dm_region_hash dm_log dm_multipath dm_mod uinput bonding ipv6 kvm_intel kvm fuse xt_multiport iptable_nat ip_tables nf_nat x_tables nf_conntrack_ipv4 nf_conntrack nf_defrag_ipv4 tun 8021q i2c_i801 igb i2c_core i5k_amb hwmon i5400_edac edac_core iTCO_wdt iTCO_vendor_support pcspkr shpchp myri10ge floppy netxen_nic sr_mod cdrom serio_raw sg ata_generic pata_acpi ata_piix libata sd_mod scsi_mod ext3 jbd mbcache uhci_hcd ohci_hcd ehci_hcd [last unloaded: microcode]
Pid: 22313, comm: ip Not tainted 2.6.31.2-193574.2006Arora.fc11.x86_64 #1 X7DWU
RIP: 0010:[<ffffffffa0128942>]  [<ffffffffa0128942>] netxen_send_cmd_descs.clone.2+0x26/0xfe [netxen_nic]
RSP: 0018:ffff88005753d638  EFLAGS: 00010246
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000001 RSI: ffff88005753d688 RDI: ffff88012dc8f580
RBP: ffff88005753d678 R08: ffff88012dc8f580 R09: ffff88005753d688
R10: 00000000a388069b R11: 0000000000000246 R12: ffff88012dc8f580
R13: ffff88001fcb6c00 R14: ffff88005753d738 R15: 0000000000000020
FS:  00007f4360f656f0(0000) GS:ffff88002805a000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000020 CR3: 00000000b4de5000 CR4: 00000000000026e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process ip (pid: 22313, threadinfo ffff88005753c000, task ffff88002fbdb700)
Stack:
 0000000000000000 00000000a388069b ffffea000136a578 00000000a388069b
<0> ffff88005753d778 ffff88012dc8f580 ffff88012e2cf910 ffff88001fcb6c00
<0> ffff88005753d6d8 ffffffffa0128ae5 000000000a000000 0000000000010001
Call Trace:
 [<ffffffffa0128ae5>] nx_p3_sre_macaddr_change+0x60/0x76 [netxen_nic]
 [<ffffffffa0128ca8>] nx_p3_nic_add_mac+0x129/0x14c [netxen_nic]
 [<ffffffffa0128d57>] netxen_p3_nic_set_multi+0x8c/0x169 [netxen_nic]
 [<ffffffffa0128e54>] netxen_p3_nic_set_mac_addr+0x20/0x38 [netxen_nic]
 [<ffffffffa0129ed2>] netxen_nic_set_mac+0x96/0xd6 [netxen_nic]
 [<ffffffff813128b1>] do_setlink+0x17c/0x360
 [<ffffffff81312e97>] rtnl_newlink+0x2e1/0x4e6
 [<ffffffff81312c53>] ? rtnl_newlink+0x9d/0x4e6
 [<ffffffff810ec697>] ? page_add_new_anon_rmap+0x4d/0x73
 [<ffffffff812f8920>] ? sock_rmalloc+0x39/0xa8
 [<ffffffff810fdc8c>] ? virt_to_head_page+0x1c/0x51
 [<ffffffff813122fe>] rtnetlink_rcv_msg+0x1d0/0x201
 [<ffffffff81327856>] ? netlink_sendmsg+0x18f/0x2ac
 [<ffffffff8131212e>] ? rtnetlink_rcv_msg+0x0/0x201
 [<ffffffff81327b7d>] netlink_rcv_skb+0x4d/0xb4
 [<ffffffff81312113>] rtnetlink_rcv+0x30/0x4b
 [<ffffffff8132764a>] netlink_unicast+0x12f/0x1ac
 [<ffffffff81327950>] netlink_sendmsg+0x289/0x2ac
 [<ffffffff812f4146>] __sock_sendmsg+0x6b/0x8a
 [<ffffffff812f4abf>] sock_sendmsg+0xd6/0x103
 [<ffffffff812f494b>] ? sock_recvmsg+0xd9/0x106
 [<ffffffff8106a257>] ? autoremove_wake_function+0x0/0x5a
 [<ffffffff812ff6b4>] ? verify_iovec+0x5b/0xaf
 [<ffffffff812f4d08>] sys_sendmsg+0x21c/0x2a0
 [<ffffffff810e3711>] ? handle_mm_fault+0x2d6/0x681
 [<ffffffff811ced89>] ? __up_read+0x9c/0xbb
 [<ffffffff8106e264>] ? up_read+0x1c/0x32
 [<ffffffff813a9c3f>] ? do_page_fault+0x260/0x2a4
 [<ffffffff8100bdc2>] system_call_fastpath+0x16/0x1b
Code: 5b 41 5c c9 c3 55 48 89 e5 41 55 41 54 49 89 fc 53 48 83 ec 28 65 48 8b 04 25 28 00 00 00 48 89 45 d8 31 c0 48 8b 9f 28 01 00 00 <4c> 8b 6b 20 48 89 75 c8 49 8d 7d 40 e8 5e e9 27 e1 65 8b 04 25
RIP  [<ffffffffa0128942>] netxen_send_cmd_descs.clone.2+0x26/0xfe [netxen_nic]
 RSP <ffff88005753d638>
CR2: 0000000000000020

Re: [patch next 3/4] netxen: fix bonding support

[Eric W. Biederman]

Dhananjay Phadke <dhananjay.phadke@qlogic.com> writes:

> Yes, this was a bug until recent times.
>
> It should be fixed in 2.6.32 development cycle by this commit -
> db4cfd8a6149e778befb2ff6e6f91cdc6394cbe6 ("netxen: handle firmware load errors").
>
> This added a check for adapter->is_up before touching txq lock.

Yes.  That should prevent the null pointer deference.  Will it also
allow setting the mac address when the NIC is down?

Eric

RE: [patch next 3/4] netxen: fix bonding support

[Dhananjay Phadke]

> Yes.  That should prevent the null pointer deference.  Will it also
> allow setting the mac address when the NIC is down?
> 
> Eric

Yes, we do save new address in netdev->dev_addr.
This is later on programmed in hardware when interface is brought up.

-Dhannajay

Re: [patch next 3/4] netxen: fix bonding support

[Eric W. Biederman]

Dhananjay Phadke <dhananjay.phadke@qlogic.com> writes:

>> Yes.  That should prevent the null pointer deference.  Will it also
>> allow setting the mac address when the NIC is down?
>> 
>> Eric
>
> Yes, we do save new address in netdev->dev_addr.
> This is later on programmed in hardware when interface is brought up.

Yep. Thanks.  I just tested it and confirmed the crash is no longer there.

Eric

RE: [patch next 3/4] netxen: fix bonding support

[Dhananjay Phadke]

Eric,

Thanks for reporting and verifying.

-Dhananjay
________________________________________
From: Eric W. Biederman [ebiederm@xmission.com]
Sent: Monday, October 26, 2009 10:50 PM
To: Dhananjay Phadke
Cc: netdev@vger.kernel.org
Subject: Re: [patch next 3/4] netxen: fix bonding support

Dhananjay Phadke <dhananjay.phadke@qlogic.com> writes:

>> Yes.  That should prevent the null pointer deference.  Will it also
>> allow setting the mac address when the NIC is down?
>>
>> Eric
>
> Yes, we do save new address in netdev->dev_addr.
> This is later on programmed in hardware when interface is brought up.

Yep. Thanks.  I just tested it and confirmed the crash is no longer there.

Eric

[patch next 4/4] netxen: disable rss for GbE ports

[Dhananjay Phadke]

There's no advantage of receive side scaling on GbE ports, so
restrict rss support to 10GbE ports.

Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
---
 drivers/net/netxen/netxen_nic_main.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index 83dadfd..1349cf8 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -292,12 +292,12 @@ static void netxen_check_options(struct netxen_adapter *adapter)
 	adapter->msix_supported = 0;
 	if (NX_IS_REVISION_P3(adapter->ahw.revision_id)) {
 		adapter->msix_supported = !!use_msi_x;
-		adapter->rss_supported = !!use_msi_x;
+		adapter->rss_supported = !!use_msi_x &&
+			(adapter->ahw.port_type == NETXEN_NIC_XGBE);
 	} else if (adapter->fw_version >= NETXEN_VERSION_CODE(3, 4, 336)) {
 		switch (adapter->ahw.board_type) {
 		case NETXEN_BRDTYPE_P2_SB31_10G:
 		case NETXEN_BRDTYPE_P2_SB31_10G_CX4:
-		case NETXEN_BRDTYPE_P2_SB31_10G_HMEZ:
 			adapter->msix_supported = !!use_msi_x;
 			adapter->rss_supported = !!use_msi_x;
 			break;
@@ -496,6 +496,7 @@ netxen_setup_intr(struct netxen_adapter *adapter)
 {
 	struct netxen_legacy_intr_set *legacy_intrp;
 	struct pci_dev *pdev = adapter->pdev;
+	int num_msix = adapter->rss_supported ? MSIX_ENTRIES_PER_ADAPTER : 1;
 
 	adapter->flags &= ~(NETXEN_NIC_MSI_ENABLED | NETXEN_NIC_MSIX_ENABLED);
 
@@ -513,8 +514,7 @@ netxen_setup_intr(struct netxen_adapter *adapter)
 	if (adapter->msix_supported) {
 
 		netxen_init_msix_entries(adapter);
-		if (pci_enable_msix(pdev, adapter->msix_entries,
-					MSIX_ENTRIES_PER_ADAPTER))
+		if (pci_enable_msix(pdev, adapter->msix_entries, num_msix))
 			goto request_msi;
 
 		adapter->flags |= NETXEN_NIC_MSIX_ENABLED;
-- 
1.6.0.2

Re: [patch next 4/4] netxen: disable rss for GbE ports

[David Miller]

From: Dhananjay Phadke <dhananjay@netxen.com>
Date: Tue,  5 May 2009 22:05:09 -0700

> There's no advantage of receive side scaling on GbE ports, so
> restrict rss support to 10GbE ports.
> 
> Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>

This is your opinion.  And even if it is more than opinion,
you give zero technical arguments in your commit message so
we can see why this should be done.

Tell that opinion to the person with 10 netxen cards in their
machine running at full tilt.

Tell that opinion to the person with extremely complicated and
cpu intensive netfilter and routing rules in use.

I'm not applying this patch.

Re: [patch next 4/4] netxen: disable rss for GbE ports

[Dhananjay Phadke]

The reason for RSS was to distribute the ingress traffic
to different CPUs. At 10Gbps, without RSS, one CPU was
saturating, acting as bottleneck. I can do 1Gbps with
only one CPU only 50% saturated, this to on a low end
server.

The fallout is more msix vectors being consumed for RSS,
when they don't really bring any performance gain.

Since NetXen has single driver, unlike others, it needs
to be dealt based on NIC board type.

-Dhananjay

David Miller wrote:
> From: Dhananjay Phadke <dhananjay@netxen.com>
> Date: Tue,  5 May 2009 22:05:09 -0700
> 
>> There's no advantage of receive side scaling on GbE ports, so
>> restrict rss support to 10GbE ports.
>>
>> Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
> 
> This is your opinion.  And even if it is more than opinion,
> you give zero technical arguments in your commit message so
> we can see why this should be done.
> 
> Tell that opinion to the person with 10 netxen cards in their
> machine running at full tilt.
> 
> Tell that opinion to the person with extremely complicated and
> cpu intensive netfilter and routing rules in use.
> 
> I'm not applying this patch.

Re: [patch next 4/4] netxen: disable rss for GbE ports

[David Miller]

From: Dhananjay Phadke <dhananjay@netxen.com>
Date: Wed, 6 May 2009 16:06:30 -0700

> The reason for RSS was to distribute the ingress traffic
> to different CPUs. At 10Gbps, without RSS, one CPU was
> saturating, acting as bottleneck. I can do 1Gbps with
> only one CPU only 50% saturated, this to on a low end
> server.
> 
> The fallout is more msix vectors being consumed for RSS,
> when they don't really bring any performance gain.
> 
> Since NetXen has single driver, unlike others, it needs
> to be dealt based on NIC board type.

Thanks for ignoring my email.

I'll say it again, maybe you'll listen this time.

If a user has very cpu intensive netfilter or routing
rules installed, the RSS flow seperation to different
CPUs can help even at 1GB speeds.

Therefore, your change will introduce performance regressions.

Re: [patch next 4/4] netxen: disable rss for GbE ports

[Dhananjay Phadke]

David Miller wrote:
> Thanks for ignoring my email.
> 
> I'll say it again, maybe you'll listen this time.
> 
> If a user has very cpu intensive netfilter or routing
> rules installed, the RSS flow seperation to different
> CPUs can help even at 1GB speeds.
> 
> Therefore, your change will introduce performance regressions.

I got your point, but there's another reason I have put
forward. With four (not even 10) 4-port netxen NICs
installed, they will consume 64 or more msi-x vectors.
When all this for only 1Gbps per port, restricted by
physical media speed.

This is only per port accounting, when it has to be
per pci function. The virtual NICs have more than one
pci function per physical port (=> 32 vectors per card).

There are other ways to balance the load, like moving
tx ring clean up to separate (one) msi-x vector.

I do see a reason for conserving msix vectors, unless
they are bringing performance gain.

Middle ground can be using 2 vectors per pci func, instead
of 4 (which has been tested to benefit 10Gbps NICs).

Call it my opinion, but may be it's necessary if system has
limited msi vectors.

-Dhananjay

Re: [patch next 4/4] netxen: disable rss for GbE ports

[David Miller]

From: Dhananjay Phadke <dhananjay@netxen.com>
Date: Wed, 6 May 2009 17:08:06 -0700

> I do see a reason for conserving msix vectors, unless
> they are bringing performance gain.

Then scale the number of RX queues in use by port speed.

Re: [patch next 4/4] netxen: disable rss for GbE ports

[Dhananjay Phadke]

David Miller wrote:
>> I do see a reason for conserving msix vectors, unless
>> they are bringing performance gain.
> 
> Then scale the number of RX queues in use by port speed.

Yes, I was proposing one rx queue per port, which is implied
by disabling rss. May be two ...


-Dhananjay

Re: [patch next 4/4] netxen: disable rss for GbE ports

[David Miller]

From: Dhananjay Phadke <dhananjay@netxen.com>
Date: Wed, 6 May 2009 17:31:57 -0700

> David Miller wrote:
>>> I do see a reason for conserving msix vectors, unless
>>> they are bringing performance gain.
>> 
>> Then scale the number of RX queues in use by port speed.
> 
> Yes, I was proposing one rx queue per port, which is implied
> by disabling rss. May be two ...

I personally think 4 would be more appropriate.

That's what I end up using in 4-port 1G NIU configurations.

Re: [patch next 4/4] netxen: disable rss for GbE ports

[Dhananjay Phadke]

David Miller wrote:
> I personally think 4 would be more appropriate.
> 
> That's what I end up using in 4-port 1G NIU configurations.

Ok, I will leave it at 4 for now, will revisit this when
sriov stuff gets submitted.

Thanks,
Dhananjay

Re: [patch next 4/4] netxen: disable rss for GbE ports

[Jesper Dangaard Brouer]

On Wed, 6 May 2009, Dhananjay Phadke wrote:

> The reason for RSS was to distribute the ingress traffic
> to different CPUs. At 10Gbps, without RSS, one CPU was
> saturating, acting as bottleneck. I can do 1Gbps with
> only one CPU only 50% saturated, this to on a low end
> server.

1Gbit/s throughput tests with MTU size frames is a bad measurement target.
Packets Per Sec is the key factor.

Hilsen
   Jesper Brouer

--
-------------------------------------------------------------------
MSc. Master of Computer Science
Dept. of Computer Science, University of Copenhagen
Author of http://www.adsl-optimizer.dk
-------------------------------------------------------------------

Re: [patch next 4/4] netxen: disable rss for GbE ports

[Dhananjay Phadke]

Jesper Dangaard Brouer wrote:
> 1Gbit/s throughput tests with MTU size frames is a bad measurement target.
> Packets Per Sec is the key factor.
> 

Yes, I will run some small packet tests before deciding on the number of
rx queues. I am going to revisit the msix vector utilization a bit later,
that's when I will address this.

Thanks,
Dhananjay