[PATCH net-next 1/6] cnic: Finetune iSCSI connection set up.

[PATCH net-next 1/6] cnic: Finetune iSCSI connection set up.

[Michael Chan]

From: Eddie Wai <waie@broadcom.com>

Initialize IP ID and handle some additional connection errors.

Signed-off-by: Eddie Wai <waie@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
Signed-off-by: Benjamin Li <benli@broadcom.com>
---
 drivers/net/cnic.c |   17 ++++++++++++++++-
 drivers/net/cnic.h |    2 +-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index 6aecef9..0fe8371 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -2507,7 +2507,7 @@ static int cnic_cm_offload_pg(struct cnic_sock *csk)
 	l4kwqe->sa5 = dev->mac_addr[5];
 
 	l4kwqe->etype = ETH_P_IP;
-	l4kwqe->ipid_count = DEF_IPID_COUNT;
+	l4kwqe->ipid_start = DEF_IPID_START;
 	l4kwqe->host_opaque = csk->l5_cid;
 
 	if (csk->vlan_id) {
@@ -3046,6 +3046,14 @@ static void cnic_cm_process_offld_pg(struct cnic_dev *dev, struct l4_kcq *kcqe)
 		clear_bit(SK_F_OFFLD_SCHED, &csk->flags);
 		goto done;
 	}
+	/* Possible PG kcqe status:  SUCCESS, OFFLOADED_PG, or CTX_ALLOC_FAIL */
+	if (kcqe->status == L4_KCQE_COMPLETION_STATUS_CTX_ALLOC_FAIL) {
+		clear_bit(SK_F_OFFLD_SCHED, &csk->flags);
+		cnic_cm_upcall(cp, csk,
+			       L4_KCQE_OPCODE_VALUE_CONNECT_COMPLETE);
+		goto done;
+	}
+
 	csk->pg_cid = kcqe->pg_cid;
 	set_bit(SK_F_PG_OFFLD_COMPLETE, &csk->flags);
 	cnic_cm_conn_req(csk);
@@ -3083,6 +3091,13 @@ static void cnic_cm_process_kcqe(struct cnic_dev *dev, struct kcqe *kcqe)
 	}
 
 	switch (opcode) {
+	case L5CM_RAMROD_CMD_ID_TCP_CONNECT:
+		if (l4kcqe->status != 0) {
+			clear_bit(SK_F_OFFLD_SCHED, &csk->flags);
+			cnic_cm_upcall(cp, csk,
+				       L4_KCQE_OPCODE_VALUE_CONNECT_COMPLETE);
+		}
+		break;
 	case L4_KCQE_OPCODE_VALUE_CONNECT_COMPLETE:
 		if (l4kcqe->status == 0)
 			set_bit(SK_F_OFFLD_COMPLETE, &csk->flags);
diff --git a/drivers/net/cnic.h b/drivers/net/cnic.h
index 241d09a..1921597 100644
--- a/drivers/net/cnic.h
+++ b/drivers/net/cnic.h
@@ -101,7 +101,7 @@ struct cnic_redirect_entry {
 #define BNX2X_KWQ_DATA(cp, x)						\
 	&(cp)->kwq_16_data[BNX2X_KWQ_DATA_PG(cp, x)][BNX2X_KWQ_DATA_IDX(cp, x)]
 
-#define DEF_IPID_COUNT		0xc001
+#define DEF_IPID_START		0x8000
 
 #define DEF_KA_TIMEOUT		10000
 #define DEF_KA_INTERVAL		300000
-- 
1.6.4.GIT

[PATCH net-next 2/6] cnic: Finetune iSCSI connection reset.

[Michael Chan]

From: Eddie Wai <waie@broadcom.com>

For bnx2 devices, always send notification to bnx2i to let it initiate
the cleanup when RST is received.

For bnx2x devices, add unsolicited RST_COMP handling to start the cleanup.

Signed-off-by: Eddie Wai <waie@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
Signed-off-by: Benjamin Li <benli@broadcom.com>
---
 drivers/net/cnic.c |   21 ++++++++++++++++-----
 1 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index 0fe8371..40865aa 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -3108,7 +3108,10 @@ static void cnic_cm_process_kcqe(struct cnic_dev *dev, struct kcqe *kcqe)
 		break;
 
 	case L4_KCQE_OPCODE_VALUE_RESET_RECEIVED:
-		if (test_and_clear_bit(SK_F_OFFLD_COMPLETE, &csk->flags))
+		if (test_bit(CNIC_F_BNX2_CLASS, &dev->flags)) {
+			cnic_cm_upcall(cp, csk, opcode);
+			break;
+		} else if (test_and_clear_bit(SK_F_OFFLD_COMPLETE, &csk->flags))
 			csk->state = opcode;
 		/* fall through */
 	case L4_KCQE_OPCODE_VALUE_CLOSE_COMP:
@@ -3172,6 +3175,16 @@ static int cnic_ready_to_close(struct cnic_sock *csk, u32 opcode)
 		if (!test_and_set_bit(SK_F_CLOSING, &csk->flags))
 			return 1;
 	}
+	/* 57710+ only  workaround to handle unsolicited RESET_COMP
+	 * which will be treated like a RESET RCVD notification
+	 * which triggers the clean up procedure
+	 */
+	else if (opcode == L4_KCQE_OPCODE_VALUE_RESET_COMP) {
+		if (!test_and_set_bit(SK_F_CLOSING, &csk->flags)) {
+			csk->state = L4_KCQE_OPCODE_VALUE_RESET_RECEIVED;
+			return 1;
+		}
+	}
 	return 0;
 }
 
@@ -3181,10 +3194,8 @@ static void cnic_close_bnx2_conn(struct cnic_sock *csk, u32 opcode)
 	struct cnic_local *cp = dev->cnic_priv;
 
 	clear_bit(SK_F_CONNECT_START, &csk->flags);
-	if (cnic_ready_to_close(csk, opcode)) {
-		cnic_close_conn(csk);
-		cnic_cm_upcall(cp, csk, opcode);
-	}
+	cnic_close_conn(csk);
+	cnic_cm_upcall(cp, csk, opcode);
 }
 
 static void cnic_cm_stop_bnx2_hw(struct cnic_dev *dev)
-- 
1.6.4.GIT

[PATCH net-next 3/6] cnic: Fix panic in cnic_iscsi_nl_msg_recv() when device is down.

[Michael Chan]

Some data structures are freed when the device is down and it will
crash if an ISCSI netlink message is received.  Add RCU protection
to prevent this.  In the shutdown path, ulp_ops[CNIC_ULP_L4] is
assigned NULL and rcu_synchronized before freeing the data
structures.

Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/cnic.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index 40865aa..4558444 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -327,6 +327,12 @@ static int cnic_iscsi_nl_msg_recv(struct cnic_dev *dev, u32 msg_type,
 		if (l5_cid >= MAX_CM_SK_TBL_SZ)
 			break;
 
+		rcu_read_lock();
+		if (!rcu_dereference(cp->ulp_ops[CNIC_ULP_L4])) {
+			rc = -ENODEV;
+			rcu_read_unlock();
+			break;
+		}
 		csk = &cp->csk_tbl[l5_cid];
 		csk_hold(csk);
 		if (cnic_in_use(csk)) {
@@ -341,6 +347,7 @@ static int cnic_iscsi_nl_msg_recv(struct cnic_dev *dev, u32 msg_type,
 				cnic_cm_set_pg(csk);
 		}
 		csk_put(csk);
+		rcu_read_unlock();
 		rc = 0;
 	}
 	}
-- 
1.6.4.GIT

[PATCH net-next 4/6] cnic: Simplify route checking during iSCSI connection.

[Michael Chan]

With a separate IP address for iSCSI, connections should proceed
whether or not we can get a route to the target from the network stack.
It is possible that the network IP address may not reach the iSCSI target.

Signed-off-by: Michael Chan <mchan@broadcom.com>
Signed-off-by: Benjamin Li <benli@broadcom.com>
Signed-off-by: Eddie Wai <waie@broadcom.com>
---
 drivers/net/cnic.c |   37 +++++++++++++------------------------
 1 files changed, 13 insertions(+), 24 deletions(-)

diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index 4558444..0defe61 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -2860,8 +2860,8 @@ static int cnic_get_route(struct cnic_sock *csk, struct cnic_sockaddr *saddr)
 {
 	struct cnic_dev *dev = csk->dev;
 	struct cnic_local *cp = dev->cnic_priv;
-	int is_v6, err, rc = -ENETUNREACH;
-	struct dst_entry *dst;
+	int is_v6, rc = 0;
+	struct dst_entry *dst = NULL;
 	struct net_device *realdev;
 	u32 local_port;
 
@@ -2877,39 +2877,31 @@ static int cnic_get_route(struct cnic_sock *csk, struct cnic_sockaddr *saddr)
 	clear_bit(SK_F_IPV6, &csk->flags);
 
 	if (is_v6) {
-#if defined(CONFIG_IPV6) || (defined(CONFIG_IPV6_MODULE) && defined(MODULE))
 		set_bit(SK_F_IPV6, &csk->flags);
-		err = cnic_get_v6_route(&saddr->remote.v6, &dst);
-		if (err)
-			return err;
-
-		if (!dst || dst->error || !dst->dev)
-			goto err_out;
+		cnic_get_v6_route(&saddr->remote.v6, &dst);
 
 		memcpy(&csk->dst_ip[0], &saddr->remote.v6.sin6_addr,
 		       sizeof(struct in6_addr));
 		csk->dst_port = saddr->remote.v6.sin6_port;
 		local_port = saddr->local.v6.sin6_port;
-#else
-		return rc;
-#endif
 
 	} else {
-		err = cnic_get_v4_route(&saddr->remote.v4, &dst);
-		if (err)
-			return err;
-
-		if (!dst || dst->error || !dst->dev)
-			goto err_out;
+		cnic_get_v4_route(&saddr->remote.v4, &dst);
 
 		csk->dst_ip[0] = saddr->remote.v4.sin_addr.s_addr;
 		csk->dst_port = saddr->remote.v4.sin_port;
 		local_port = saddr->local.v4.sin_port;
 	}
 
-	csk->vlan_id = cnic_get_vlan(dst->dev, &realdev);
-	if (realdev != dev->netdev)
-		goto err_out;
+	csk->vlan_id = 0;
+	csk->mtu = dev->netdev->mtu;
+	if (dst && dst->dev) {
+		u16 vlan = cnic_get_vlan(dst->dev, &realdev);
+		if (realdev == dev->netdev) {
+			csk->vlan_id = vlan;
+			csk->mtu = dst_mtu(dst);
+		}
+	}
 
 	if (local_port >= CNIC_LOCAL_PORT_MIN &&
 	    local_port < CNIC_LOCAL_PORT_MAX) {
@@ -2927,9 +2919,6 @@ static int cnic_get_route(struct cnic_sock *csk, struct cnic_sockaddr *saddr)
 	}
 	csk->src_port = local_port;
 
-	csk->mtu = dst_mtu(dst);
-	rc = 0;
-
 err_out:
 	dst_release(dst);
 	return rc;
-- 
1.6.4.GIT

[PATCH net-next 5/6] cnic: Use union for the status blocks of different devices.

[Michael Chan]

We only need to assign the status block address once and it also saves
space in the structure.

Signed-off-by: Michael Chan <mchan@broadcom.com>
Signed-off-by: Benjamin Li <benli@broadcom.com>
Signed-off-by: Eddie Wai <waie@broadcom.com>
---
 drivers/net/cnic.c |   35 +++++++++++++++++------------------
 drivers/net/cnic.h |    9 ++++++---
 2 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index 0defe61..463408f 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -900,7 +900,8 @@ static int cnic_alloc_uio(struct cnic_dev *dev) {
 	uinfo->mem[0].memtype = UIO_MEM_PHYS;
 
 	if (test_bit(CNIC_F_BNX2_CLASS, &dev->flags)) {
-		uinfo->mem[1].addr = (unsigned long) cp->status_blk & PAGE_MASK;
+		uinfo->mem[1].addr = (unsigned long) cp->status_blk.gen &
+			PAGE_MASK;
 		if (cp->ethdev->drv_state & CNIC_DRV_STATE_USING_MSIX)
 			uinfo->mem[1].size = BNX2_SBLK_MSIX_ALIGN_SIZE * 9;
 		else
@@ -1103,10 +1104,9 @@ static int cnic_alloc_bnx2x_resc(struct cnic_dev *dev)
 	if (ret)
 		goto error;
 
-	cp->bnx2x_status_blk = cp->status_blk;
 	cp->bnx2x_def_status_blk = cp->ethdev->irq_arr[1].status_blk;
 
-	memset(cp->bnx2x_status_blk, 0, sizeof(struct host_status_block));
+	memset(cp->status_blk.bnx2x, 0, sizeof(*cp->status_blk.bnx2x));
 
 	cp->l2_rx_ring_size = 15;
 
@@ -2205,7 +2205,7 @@ static void cnic_service_bnx2_msix(unsigned long data)
 {
 	struct cnic_dev *dev = (struct cnic_dev *) data;
 	struct cnic_local *cp = dev->cnic_priv;
-	struct status_block_msix *status_blk = cp->bnx2_status_blk;
+	struct status_block_msix *status_blk = cp->status_blk.bnx2;
 	u32 status_idx = status_blk->status_idx;
 	u16 hw_prod, sw_prod;
 	int kcqe_cnt;
@@ -2251,7 +2251,7 @@ static irqreturn_t cnic_irq(int irq, void *dev_instance)
 	if (cp->ack_int)
 		cp->ack_int(dev);
 
-	prefetch(cp->status_blk);
+	prefetch(cp->status_blk.gen);
 	prefetch(&cp->kcq[KCQ_PG(prod)][KCQ_IDX(prod)]);
 
 	if (likely(test_bit(CNIC_F_CNIC_UP, &dev->flags)))
@@ -2292,7 +2292,7 @@ static void cnic_service_bnx2x_bh(unsigned long data)
 	struct cnic_local *cp = dev->cnic_priv;
 	u16 hw_prod, sw_prod;
 	struct cstorm_status_block_c *sblk =
-		&cp->bnx2x_status_blk->c_status_block;
+		&cp->status_blk.bnx2x->c_status_block;
 	u32 status_idx = sblk->status_block_index;
 	int kcqe_cnt;
 
@@ -2334,7 +2334,7 @@ static int cnic_service_bnx2x(void *data, void *status_blk)
 	struct cnic_local *cp = dev->cnic_priv;
 	u16 prod = cp->kcq_prod_idx & MAX_KCQ_IDX;
 
-	prefetch(cp->status_blk);
+	prefetch(cp->status_blk.bnx2x);
 	prefetch(&cp->kcq[KCQ_PG(prod)][KCQ_IDX(prod)]);
 
 	if (likely(test_bit(CNIC_F_CNIC_UP, &dev->flags)))
@@ -3409,8 +3409,7 @@ static int cnic_init_bnx2_irq(struct cnic_dev *dev)
 		CNIC_WR(dev, base + BNX2_HC_COM_TICKS_OFF, (64 << 16) | 220);
 		CNIC_WR(dev, base + BNX2_HC_CMD_TICKS_OFF, (64 << 16) | 220);
 
-		cp->bnx2_status_blk = cp->status_blk;
-		cp->last_status_idx = cp->bnx2_status_blk->status_idx;
+		cp->last_status_idx = cp->status_blk.bnx2->status_idx;
 		tasklet_init(&cp->cnic_irq_task, cnic_service_bnx2_msix,
 			     (unsigned long) dev);
 		err = request_irq(ethdev->irq_arr[0].vector, cnic_irq, 0,
@@ -3419,7 +3418,7 @@ static int cnic_init_bnx2_irq(struct cnic_dev *dev)
 			tasklet_disable(&cp->cnic_irq_task);
 			return err;
 		}
-		while (cp->bnx2_status_blk->status_completion_producer_index &&
+		while (cp->status_blk.bnx2->status_completion_producer_index &&
 		       i < 10) {
 			CNIC_WR(dev, BNX2_HC_COALESCE_NOW,
 				1 << (11 + sblk_num));
@@ -3427,13 +3426,13 @@ static int cnic_init_bnx2_irq(struct cnic_dev *dev)
 			i++;
 			barrier();
 		}
-		if (cp->bnx2_status_blk->status_completion_producer_index) {
+		if (cp->status_blk.bnx2->status_completion_producer_index) {
 			cnic_free_irq(dev);
 			goto failed;
 		}
 
 	} else {
-		struct status_block *sblk = cp->status_blk;
+		struct status_block *sblk = cp->status_blk.gen;
 		u32 hc_cmd = CNIC_RD(dev, BNX2_HC_COMMAND);
 		int i = 0;
 
@@ -3490,7 +3489,7 @@ static void cnic_init_bnx2_tx_ring(struct cnic_dev *dev)
 	int i;
 	struct tx_bd *txbd;
 	dma_addr_t buf_map;
-	struct status_block *s_blk = cp->status_blk;
+	struct status_block *s_blk = cp->status_blk.gen;
 
 	sb_id = cp->status_blk_num;
 	tx_cid = 20;
@@ -3498,7 +3497,7 @@ static void cnic_init_bnx2_tx_ring(struct cnic_dev *dev)
 	cnic_init_context(dev, tx_cid + 1);
 	cp->tx_cons_ptr = &s_blk->status_tx_quick_consumer_index2;
 	if (ethdev->drv_state & CNIC_DRV_STATE_USING_MSIX) {
-		struct status_block_msix *sblk = cp->status_blk;
+		struct status_block_msix *sblk = cp->status_blk.bnx2;
 
 		tx_cid = TX_TSS_CID + sb_id - 1;
 		cnic_init_context(dev, tx_cid);
@@ -3554,7 +3553,7 @@ static void cnic_init_bnx2_rx_ring(struct cnic_dev *dev)
 	u32 cid_addr, sb_id, val, coal_reg, coal_val;
 	int i;
 	struct rx_bd *rxbd;
-	struct status_block *s_blk = cp->status_blk;
+	struct status_block *s_blk = cp->status_blk.gen;
 
 	sb_id = cp->status_blk_num;
 	cnic_init_context(dev, 2);
@@ -3562,7 +3561,7 @@ static void cnic_init_bnx2_rx_ring(struct cnic_dev *dev)
 	coal_reg = BNX2_HC_COMMAND;
 	coal_val = CNIC_RD(dev, coal_reg);
 	if (ethdev->drv_state & CNIC_DRV_STATE_USING_MSIX) {
-		struct status_block_msix *sblk = cp->status_blk;
+		struct status_block_msix *sblk = cp->status_blk.bnx2;
 
 		cp->rx_cons_ptr = &sblk->status_rx_quick_consumer_index;
 		coal_reg = BNX2_HC_COALESCE_NOW;
@@ -3661,7 +3660,7 @@ static int cnic_start_bnx2_hw(struct cnic_dev *dev)
 {
 	struct cnic_local *cp = dev->cnic_priv;
 	struct cnic_eth_dev *ethdev = cp->ethdev;
-	struct status_block *sblk = cp->status_blk;
+	struct status_block *sblk = cp->status_blk.gen;
 	u32 val;
 	int err;
 
@@ -4250,7 +4249,7 @@ static int cnic_start_hw(struct cnic_dev *dev)
 	cp->chip_id = ethdev->chip_id;
 	pci_dev_get(dev->pcidev);
 	cp->func = PCI_FUNC(dev->pcidev->devfn);
-	cp->status_blk = ethdev->irq_arr[0].status_blk;
+	cp->status_blk.gen = ethdev->irq_arr[0].status_blk;
 	cp->status_blk_num = ethdev->irq_arr[0].status_blk_num;
 
 	err = cp->alloc_resc(dev);
diff --git a/drivers/net/cnic.h b/drivers/net/cnic.h
index 1921597..d566390 100644
--- a/drivers/net/cnic.h
+++ b/drivers/net/cnic.h
@@ -224,9 +224,12 @@ struct cnic_local {
 	u16		kcq_prod_idx;
 	u32		kcq_io_addr;
 
-	void				*status_blk;
-	struct status_block_msix	*bnx2_status_blk;
-	struct host_status_block	*bnx2x_status_blk;
+	union {
+		void				*gen;
+		struct status_block_msix	*bnx2;
+		struct host_status_block	*bnx2x;
+	} status_blk;
+
 	struct host_def_status_block	*bnx2x_def_status_blk;
 
 	u32				status_blk_num;
-- 
1.6.4.GIT

[PATCH net-next 6/6] cnic: Update version to 2.1.1.

[Michael Chan]

And update copyright to 2010.

Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/cnic.c      |    2 +-
 drivers/net/cnic.h      |    2 +-
 drivers/net/cnic_defs.h |    2 +-
 drivers/net/cnic_if.h   |    6 +++---
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index 463408f..9781942 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -1,6 +1,6 @@
 /* cnic.c: Broadcom CNIC core network driver.
  *
- * Copyright (c) 2006-2009 Broadcom Corporation
+ * Copyright (c) 2006-2010 Broadcom Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
diff --git a/drivers/net/cnic.h b/drivers/net/cnic.h
index d566390..a0d853d 100644
--- a/drivers/net/cnic.h
+++ b/drivers/net/cnic.h
@@ -1,6 +1,6 @@
 /* cnic.h: Broadcom CNIC core network driver.
  *
- * Copyright (c) 2006-2009 Broadcom Corporation
+ * Copyright (c) 2006-2010 Broadcom Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
diff --git a/drivers/net/cnic_defs.h b/drivers/net/cnic_defs.h
index 9827b27..7ce694d 100644
--- a/drivers/net/cnic_defs.h
+++ b/drivers/net/cnic_defs.h
@@ -1,7 +1,7 @@
 
 /* cnic.c: Broadcom CNIC core network driver.
  *
- * Copyright (c) 2006-2009 Broadcom Corporation
+ * Copyright (c) 2006-2010 Broadcom Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
diff --git a/drivers/net/cnic_if.h b/drivers/net/cnic_if.h
index 8aaf98b..110c620 100644
--- a/drivers/net/cnic_if.h
+++ b/drivers/net/cnic_if.h
@@ -1,6 +1,6 @@
 /* cnic_if.h: Broadcom CNIC core network driver.
  *
- * Copyright (c) 2006 Broadcom Corporation
+ * Copyright (c) 2006-2010 Broadcom Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -12,8 +12,8 @@
 #ifndef CNIC_IF_H
 #define CNIC_IF_H
 
-#define CNIC_MODULE_VERSION	"2.1.0"
-#define CNIC_MODULE_RELDATE	"Oct 10, 2009"
+#define CNIC_MODULE_VERSION	"2.1.1"
+#define CNIC_MODULE_RELDATE	"Feb 22, 2010"
 
 #define CNIC_ULP_RDMA		0
 #define CNIC_ULP_ISCSI		1
-- 
1.6.4.GIT

Re: [PATCH net-next 3/6] cnic: Fix panic in cnic_iscsi_nl_msg_recv() when device is down.

[Simon Horman]

On Wed, Feb 24, 2010 at 04:42:06PM -0800, Michael Chan wrote:
> Some data structures are freed when the device is down and it will
> crash if an ISCSI netlink message is received.  Add RCU protection
> to prevent this.  In the shutdown path, ulp_ops[CNIC_ULP_L4] is
> assigned NULL and rcu_synchronized before freeing the data
> structures.

Is rcu_assign_pointer() unnecessary in cnic_cm_open()?
It doesn't seem to be followed by rcu_synchronized() and the pointer
doesn't seem to be accessible anywhere else at that time.

Re: [PATCH net-next 3/6] cnic: Fix panic in cnic_iscsi_nl_msg_recv() when device is down.

[Michael Chan]

Simon Horman wrote:

> On Wed, Feb 24, 2010 at 04:42:06PM -0800, Michael Chan wrote:
> > Some data structures are freed when the device is down and it will
> > crash if an ISCSI netlink message is received.  Add RCU protection
> > to prevent this.  In the shutdown path, ulp_ops[CNIC_ULP_L4] is
> > assigned NULL and rcu_synchronized before freeing the data
> > structures.
>
> Is rcu_assign_pointer() unnecessary in cnic_cm_open()?
> It doesn't seem to be followed by rcu_synchronized() and the pointer
> doesn't seem to be accessible anywhere else at that time.

We assign a valid pointer in cnic_cm_open() so that it can be used
during run-time (in service_kcqes() for example).  During shutdown in
cnic_stop_hw(), we assign NULL followed by rcu_synchronize().

Re: [PATCH net-next 1/6] cnic: Finetune iSCSI connection set up.

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 24 Feb 2010 16:42:04 -0800

> From: Eddie Wai <waie@broadcom.com>
> 
> Initialize IP ID and handle some additional connection errors.
> 
> Signed-off-by: Eddie Wai <waie@broadcom.com>
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> Signed-off-by: Benjamin Li <benli@broadcom.com>

Applied.

Re: [PATCH net-next 2/6] cnic: Finetune iSCSI connection reset.

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 24 Feb 2010 16:42:05 -0800

> From: Eddie Wai <waie@broadcom.com>
> 
> For bnx2 devices, always send notification to bnx2i to let it initiate
> the cleanup when RST is received.
> 
> For bnx2x devices, add unsolicited RST_COMP handling to start the cleanup.
> 
> Signed-off-by: Eddie Wai <waie@broadcom.com>
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> Signed-off-by: Benjamin Li <benli@broadcom.com>

Applied.

Re: [PATCH net-next 3/6] cnic: Fix panic in cnic_iscsi_nl_msg_recv() when device is down.

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 24 Feb 2010 16:42:06 -0800

> Some data structures are freed when the device is down and it will
> crash if an ISCSI netlink message is received.  Add RCU protection
> to prevent this.  In the shutdown path, ulp_ops[CNIC_ULP_L4] is
> assigned NULL and rcu_synchronized before freeing the data
> structures.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>

Applied.

Re: [PATCH net-next 4/6] cnic: Simplify route checking during iSCSI connection.

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 24 Feb 2010 16:42:07 -0800

> With a separate IP address for iSCSI, connections should proceed
> whether or not we can get a route to the target from the network stack.
> It is possible that the network IP address may not reach the iSCSI target.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> Signed-off-by: Benjamin Li <benli@broadcom.com>
> Signed-off-by: Eddie Wai <waie@broadcom.com>

Applied.

Re: [PATCH net-next 5/6] cnic: Use union for the status blocks of different devices.

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 24 Feb 2010 16:42:08 -0800

> We only need to assign the status block address once and it also saves
> space in the structure.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> Signed-off-by: Benjamin Li <benli@broadcom.com>
> Signed-off-by: Eddie Wai <waie@broadcom.com>

Applied.

Re: [PATCH net-next 6/6] cnic: Update version to 2.1.1.

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 24 Feb 2010 16:42:09 -0800

> And update copyright to 2010.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>

Applied.

Re: [PATCH net-next 3/6] cnic: Fix panic in cnic_iscsi_nl_msg_recv() when device is down.

[Paul E. McKenney]

On Thu, Feb 25, 2010 at 11:01:59PM -0800, Michael Chan wrote:
> Simon Horman wrote:
> 
> > On Wed, Feb 24, 2010 at 04:42:06PM -0800, Michael Chan wrote:
> > > Some data structures are freed when the device is down and it will
> > > crash if an ISCSI netlink message is received.  Add RCU protection
> > > to prevent this.  In the shutdown path, ulp_ops[CNIC_ULP_L4] is
> > > assigned NULL and rcu_synchronized before freeing the data
> > > structures.
> >
> > Is rcu_assign_pointer() unnecessary in cnic_cm_open()?
> > It doesn't seem to be followed by rcu_synchronized() and the pointer
> > doesn't seem to be accessible anywhere else at that time.
> 
> We assign a valid pointer in cnic_cm_open() so that it can be used
> during run-time (in service_kcqes() for example).  During shutdown in
> cnic_stop_hw(), we assign NULL followed by rcu_synchronize().

So you are saying that when the pointer is assigned in cnic_cm_open(),
there cannot possibly be any concurrent reading threads?

Use of an explicit rcu_assign_pointer() would be better if so.

							Thanx, Paul

Re: [PATCH net-next 3/6] cnic: Fix panic in cnic_iscsi_nl_msg_recv() when device is down.

[Michael Chan]

On Fri, 2010-02-26 at 10:40 -0800, Paul E. McKenney wrote:
> On Thu, Feb 25, 2010 at 11:01:59PM -0800, Michael Chan wrote:
> > Simon Horman wrote:
> > 
> > > On Wed, Feb 24, 2010 at 04:42:06PM -0800, Michael Chan wrote:
> > > > Some data structures are freed when the device is down and it will
> > > > crash if an ISCSI netlink message is received.  Add RCU protection
> > > > to prevent this.  In the shutdown path, ulp_ops[CNIC_ULP_L4] is
> > > > assigned NULL and rcu_synchronized before freeing the data
> > > > structures.
> > >
> > > Is rcu_assign_pointer() unnecessary in cnic_cm_open()?
> > > It doesn't seem to be followed by rcu_synchronized() and the pointer
> > > doesn't seem to be accessible anywhere else at that time.
> > 
> > We assign a valid pointer in cnic_cm_open() so that it can be used
> > during run-time (in service_kcqes() for example).  During shutdown in
> > cnic_stop_hw(), we assign NULL followed by rcu_synchronize().
> 
> So you are saying that when the pointer is assigned in cnic_cm_open(),
> there cannot possibly be any concurrent reading threads?

Right.  The hardware has not been started yet so there will be no events
to process from the hardware.  The pointer is read when processing these
hardware events.

> 
> Use of an explicit rcu_assign_pointer() would be better if so.

Yes, we use rcu_assign_pointer in cnic_cm_open() when assigning the
valid pointer in cnic_cm_open().  Thanks.

Re: [PATCH net-next 3/6] cnic: Fix panic in cnic_iscsi_nl_msg_recv() when device is down.

[Paul E. McKenney]

On Fri, Feb 26, 2010 at 11:11:09AM -0800, Michael Chan wrote:
> 
> On Fri, 2010-02-26 at 10:40 -0800, Paul E. McKenney wrote:
> > On Thu, Feb 25, 2010 at 11:01:59PM -0800, Michael Chan wrote:
> > > Simon Horman wrote:
> > > 
> > > > On Wed, Feb 24, 2010 at 04:42:06PM -0800, Michael Chan wrote:
> > > > > Some data structures are freed when the device is down and it will
> > > > > crash if an ISCSI netlink message is received.  Add RCU protection
> > > > > to prevent this.  In the shutdown path, ulp_ops[CNIC_ULP_L4] is
> > > > > assigned NULL and rcu_synchronized before freeing the data
> > > > > structures.
> > > >
> > > > Is rcu_assign_pointer() unnecessary in cnic_cm_open()?
> > > > It doesn't seem to be followed by rcu_synchronized() and the pointer
> > > > doesn't seem to be accessible anywhere else at that time.
> > > 
> > > We assign a valid pointer in cnic_cm_open() so that it can be used
> > > during run-time (in service_kcqes() for example).  During shutdown in
> > > cnic_stop_hw(), we assign NULL followed by rcu_synchronize().
> > 
> > So you are saying that when the pointer is assigned in cnic_cm_open(),
> > there cannot possibly be any concurrent reading threads?
> 
> Right.  The hardware has not been started yet so there will be no events
> to process from the hardware.  The pointer is read when processing these
> hardware events.
> 
> > 
> > Use of an explicit rcu_assign_pointer() would be better if so.
> 
> Yes, we use rcu_assign_pointer in cnic_cm_open() when assigning the
> valid pointer in cnic_cm_open().  Thanks.

Sounds very good, thank you!

							Thanx, Paul