[PATCH 1/5 net-next] cnic: Fix interrupt logic

[PATCH 1/5 net-next] cnic: Fix interrupt logic

[Michael Chan]

We need to keep looping until cnic_get_kcqes() returns 0.  cnic_get_kcqes()
returns a maximum of 64 entries.  If there are more entries in the queue
and we don't loop back, the remaining entries may not be serviced for a
long time.

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

diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index 11a92af..5c0a669 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -2778,13 +2778,10 @@ static u32 cnic_service_bnx2_queues(struct cnic_dev *dev)
 
 		/* Tell compiler that status_blk fields can change. */
 		barrier();
-		if (status_idx != *cp->kcq1.status_idx_ptr) {
-			status_idx = (u16) *cp->kcq1.status_idx_ptr;
-			/* status block index must be read first */
-			rmb();
-			cp->kwq_con_idx = *cp->kwq_con_idx_ptr;
-		} else
-			break;
+		status_idx = (u16) *cp->kcq1.status_idx_ptr;
+		/* status block index must be read first */
+		rmb();
+		cp->kwq_con_idx = *cp->kwq_con_idx_ptr;
 	}
 
 	CNIC_WR16(dev, cp->kcq1.io_addr, cp->kcq1.sw_prod_idx);
@@ -2908,8 +2905,6 @@ static u32 cnic_service_bnx2x_kcq(struct cnic_dev *dev, struct kcq_info *info)
 
 		/* Tell compiler that sblk fields can change. */
 		barrier();
-		if (last_status == *info->status_idx_ptr)
-			break;
 
 		last_status = *info->status_idx_ptr;
 		/* status block index must be read before reading the KCQ */
-- 
1.6.4.GIT

[PATCH 2/5 net-next] cnic: Fix race conditions with firmware

[Michael Chan]

During iSCSI connection terminations, if the target is also terminating
at about the same time, the firmware may not complete the driver's
request to close or reset the connection.  This is fixed by handling
other events (instead of the expected completion event) as an indication
that the driver's request has been rejected.

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

diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index 5c0a669..ea7245c 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -3767,7 +3767,13 @@ static void cnic_cm_process_kcqe(struct cnic_dev *dev, struct kcqe *kcqe)
 		break;
 
 	case L4_KCQE_OPCODE_VALUE_CLOSE_RECEIVED:
-		cnic_cm_upcall(cp, csk, opcode);
+		/* after we already sent CLOSE_REQ */
+		if (test_bit(CNIC_F_BNX2X_CLASS, &dev->flags) &&
+		    !test_bit(SK_F_OFFLD_COMPLETE, &csk->flags) &&
+		    csk->state == L4_KCQE_OPCODE_VALUE_CLOSE_COMP)
+			cp->close_conn(csk, L4_KCQE_OPCODE_VALUE_RESET_COMP);
+		else
+			cnic_cm_upcall(cp, csk, opcode);
 		break;
 	}
 	csk_put(csk);
@@ -3821,12 +3827,14 @@ static int cnic_ready_to_close(struct cnic_sock *csk, u32 opcode)
 	}
 
 	/* 1. If event opcode matches the expected event in csk->state
-	 * 2. If the expected event is CLOSE_COMP, we accept any event
+	 * 2. If the expected event is CLOSE_COMP or RESET_COMP, we accept any
+	 *    event
 	 * 3. If the expected event is 0, meaning the connection was never
 	 *    never established, we accept the opcode from cm_abort.
 	 */
 	if (opcode == csk->state || csk->state == 0 ||
-	    csk->state == L4_KCQE_OPCODE_VALUE_CLOSE_COMP) {
+	    csk->state == L4_KCQE_OPCODE_VALUE_CLOSE_COMP ||
+	    csk->state == L4_KCQE_OPCODE_VALUE_RESET_COMP) {
 		if (!test_and_set_bit(SK_F_CLOSING, &csk->flags)) {
 			if (csk->state == 0)
 				csk->state = opcode;
-- 
1.6.4.GIT

[PATCH 3/5 net-next] cnic: Randomize initial TCP port for iSCSI connections

[Michael Chan]

From: Eddie Wai <eddie.wai@broadcom.com>

This reduces the likelihood of port re-use when re-loading the driver.

Signed-off-by: Eddie Wai <eddie.wai@broadcom.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
---
 drivers/net/cnic.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index ea7245c..a529bde 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -605,11 +605,12 @@ static int cnic_unregister_device(struct cnic_dev *dev, int ulp_type)
 }
 EXPORT_SYMBOL(cnic_unregister_driver);
 
-static int cnic_init_id_tbl(struct cnic_id_tbl *id_tbl, u32 size, u32 start_id)
+static int cnic_init_id_tbl(struct cnic_id_tbl *id_tbl, u32 size, u32 start_id,
+			    u32 next)
 {
 	id_tbl->start = start_id;
 	id_tbl->max = size;
-	id_tbl->next = 0;
+	id_tbl->next = next;
 	spin_lock_init(&id_tbl->lock);
 	id_tbl->table = kzalloc(DIV_ROUND_UP(size, 32) * 4, GFP_KERNEL);
 	if (!id_tbl->table)
@@ -3804,14 +3805,17 @@ static void cnic_cm_free_mem(struct cnic_dev *dev)
 static int cnic_cm_alloc_mem(struct cnic_dev *dev)
 {
 	struct cnic_local *cp = dev->cnic_priv;
+	u32 port_id;
 
 	cp->csk_tbl = kzalloc(sizeof(struct cnic_sock) * MAX_CM_SK_TBL_SZ,
 			      GFP_KERNEL);
 	if (!cp->csk_tbl)
 		return -ENOMEM;
 
+	get_random_bytes(&port_id, sizeof(port_id));
+	port_id %= CNIC_LOCAL_PORT_RANGE;
 	if (cnic_init_id_tbl(&cp->csk_port_tbl, CNIC_LOCAL_PORT_RANGE,
-			     CNIC_LOCAL_PORT_MIN)) {
+			     CNIC_LOCAL_PORT_MIN, port_id)) {
 		cnic_cm_free_mem(dev);
 		return -ENOMEM;
 	}
@@ -4829,7 +4833,7 @@ static int cnic_start_bnx2x_hw(struct cnic_dev *dev)
 	pfid = cp->pfid;
 
 	ret = cnic_init_id_tbl(&cp->cid_tbl, MAX_ISCSI_TBL_SZ,
-			       cp->iscsi_start_cid);
+			       cp->iscsi_start_cid, 0);
 
 	if (ret)
 		return -ENOMEM;
@@ -4837,7 +4841,7 @@ static int cnic_start_bnx2x_hw(struct cnic_dev *dev)
 	if (BNX2X_CHIP_IS_E2(cp->chip_id)) {
 		ret = cnic_init_id_tbl(&cp->fcoe_cid_tbl,
 					BNX2X_FCOE_NUM_CONNECTIONS,
-					cp->fcoe_start_cid);
+					cp->fcoe_start_cid, 0);
 
 		if (ret)
 			return -ENOMEM;
-- 
1.6.4.GIT

[PATCH 4/5 net-next] cnic: Improve NETDEV_UP event handling

[Michael Chan]

During NETDEV_UP, we use symbol_get() to get the net driver's cnic
probe function.  This sometimes doesn't work if NETDEV_UP happens
right after NETDEV_REGISTER and the net driver is still running module
init code.  As a result, the cnic device may not be discovered.  We
fix this by probing on all NETDEV events if the device's netif_running
state is up.

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

diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index a529bde..6c544b3 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -5342,7 +5342,7 @@ static int cnic_netdev_event(struct notifier_block *this, unsigned long event,
 
 	dev = cnic_from_netdev(netdev);
 
-	if (!dev && (event == NETDEV_REGISTER || event == NETDEV_UP)) {
+	if (!dev && (event == NETDEV_REGISTER || netif_running(netdev))) {
 		/* Check for the hot-plug device */
 		dev = is_cnic_dev(netdev);
 		if (dev) {
@@ -5358,7 +5358,7 @@ static int cnic_netdev_event(struct notifier_block *this, unsigned long event,
 		else if (event == NETDEV_UNREGISTER)
 			cnic_ulp_exit(dev);
 
-		if (event == NETDEV_UP) {
+		if (event == NETDEV_UP || (new_dev && netif_running(netdev))) {
 			if (cnic_register_netdev(dev) != 0) {
 				cnic_put(dev);
 				goto done;
-- 
1.6.4.GIT

[PATCH 5/5 net-next] cnic, bnx2: Check iSCSI support early in bnx2_init_one()

[Michael Chan]

Based on earlier patch from Neil Horman <nhorman@tuxdriver.com>

If iSCSI is not supported on a bnx2 device, bnx2_cnic_probe() will
return NULL and the cnic device will not be visible to bnx2i.  This
will prevent bnx2i from registering and then unregistering during
cnic_start() and cause the warning message:

bnx2 0003:01:00.1: eth1: Failed waiting for ULP up call to complete

Signed-off-by: Michael Chan <mchan@broadcom.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
---
 drivers/net/bnx2.c |    7 +++++++
 drivers/net/cnic.c |   12 ++----------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 57d3293..74580bb 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -416,6 +416,9 @@ struct cnic_eth_dev *bnx2_cnic_probe(struct net_device *dev)
 	struct bnx2 *bp = netdev_priv(dev);
 	struct cnic_eth_dev *cp = &bp->cnic_eth_dev;
 
+	if (!cp->max_iscsi_conn)
+		return NULL;
+
 	cp->drv_owner = THIS_MODULE;
 	cp->chip_id = bp->chip_id;
 	cp->pdev = bp->pdev;
@@ -8177,6 +8180,10 @@ bnx2_init_board(struct pci_dev *pdev, struct net_device *dev)
 	bp->timer.data = (unsigned long) bp;
 	bp->timer.function = bnx2_timer;
 
+#ifdef BCM_CNIC
+	bp->cnic_eth_dev.max_iscsi_conn =
+		bnx2_reg_rd_ind(bp, BNX2_FW_MAX_ISCSI_CONN);
+#endif
 	pci_save_state(pdev);
 
 	return 0;
diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index 6c544b3..363c7f3 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -4225,14 +4225,6 @@ static void cnic_enable_bnx2_int(struct cnic_dev *dev)
 		BNX2_PCICFG_INT_ACK_CMD_INDEX_VALID | cp->last_status_idx);
 }
 
-static void cnic_get_bnx2_iscsi_info(struct cnic_dev *dev)
-{
-	u32 max_conn;
-
-	max_conn = cnic_reg_rd_ind(dev, BNX2_FW_MAX_ISCSI_CONN);
-	dev->max_iscsi_conn = max_conn;
-}
-
 static void cnic_disable_bnx2_int_sync(struct cnic_dev *dev)
 {
 	struct cnic_local *cp = dev->cnic_priv;
@@ -4557,8 +4549,6 @@ static int cnic_start_bnx2_hw(struct cnic_dev *dev)
 		return err;
 	}
 
-	cnic_get_bnx2_iscsi_info(dev);
-
 	return 0;
 }
 
@@ -5224,6 +5214,8 @@ static struct cnic_dev *init_bnx2_cnic(struct net_device *dev)
 	cdev->pcidev = pdev;
 	cp->chip_id = ethdev->chip_id;
 
+	cdev->max_iscsi_conn = ethdev->max_iscsi_conn;
+
 	cp->cnic_ops = &cnic_bnx2_ops;
 	cp->start_hw = cnic_start_bnx2_hw;
 	cp->stop_hw = cnic_stop_bnx2_hw;
-- 
1.6.4.GIT

Re: [PATCH 1/5 net-next] cnic: Fix interrupt logic

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 8 Jun 2011 22:29:32 -0700

> We need to keep looping until cnic_get_kcqes() returns 0.  cnic_get_kcqes()
> returns a maximum of 64 entries.  If there are more entries in the queue
> and we don't loop back, the remaining entries may not be serviced for a
> long time.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>

Applied.

Re: [PATCH 2/5 net-next] cnic: Fix race conditions with firmware

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 8 Jun 2011 22:29:33 -0700

> During iSCSI connection terminations, if the target is also terminating
> at about the same time, the firmware may not complete the driver's
> request to close or reset the connection.  This is fixed by handling
> other events (instead of the expected completion event) as an indication
> that the driver's request has been rejected.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>

Applied.

Re: [PATCH 3/5 net-next] cnic: Randomize initial TCP port for iSCSI connections

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 8 Jun 2011 22:29:34 -0700

> From: Eddie Wai <eddie.wai@broadcom.com>
> 
> This reduces the likelihood of port re-use when re-loading the driver.
> 
> Signed-off-by: Eddie Wai <eddie.wai@broadcom.com>
> Signed-off-by: Michael Chan <mchan@broadcom.com>

Applied.

Re: [PATCH 4/5 net-next] cnic: Improve NETDEV_UP event handling

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 8 Jun 2011 22:29:35 -0700

> During NETDEV_UP, we use symbol_get() to get the net driver's cnic
> probe function.  This sometimes doesn't work if NETDEV_UP happens
> right after NETDEV_REGISTER and the net driver is still running module
> init code.  As a result, the cnic device may not be discovered.  We
> fix this by probing on all NETDEV events if the device's netif_running
> state is up.
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>

Applied.

Re: [PATCH 5/5 net-next] cnic, bnx2: Check iSCSI support early in bnx2_init_one()

[David Miller]

From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 8 Jun 2011 22:29:36 -0700

> Based on earlier patch from Neil Horman <nhorman@tuxdriver.com>
> 
> If iSCSI is not supported on a bnx2 device, bnx2_cnic_probe() will
> return NULL and the cnic device will not be visible to bnx2i.  This
> will prevent bnx2i from registering and then unregistering during
> cnic_start() and cause the warning message:
> 
> bnx2 0003:01:00.1: eth1: Failed waiting for ULP up call to complete
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>

Applied.

Re: [PATCH 5/5 net-next] cnic, bnx2: Check iSCSI support early in bnx2_init_one()

[Neil Horman]

On Wed, Jun 08, 2011 at 10:29:36PM -0700, Michael Chan wrote:
> Based on earlier patch from Neil Horman <nhorman@tuxdriver.com>
> 
> If iSCSI is not supported on a bnx2 device, bnx2_cnic_probe() will
> return NULL and the cnic device will not be visible to bnx2i.  This
> will prevent bnx2i from registering and then unregistering during
> cnic_start() and cause the warning message:
> 
> bnx2 0003:01:00.1: eth1: Failed waiting for ULP up call to complete
> 
> Signed-off-by: Michael Chan <mchan@broadcom.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>
Thank you Michael.  This also lets you eliminate some dead code in bnx2i_start
now, since cdev->max_iscsi_conn will never be zero.

Regards
Neil

> 

Re: [PATCH 3/5 net-next] cnic: Randomize initial TCP port for iSCSI connections

[Stephen Hemminger]

On Wed, 8 Jun 2011 22:29:34 -0700
"Michael Chan" <mchan@broadcom.com> wrote:

>  static int cnic_cm_alloc_mem(struct cnic_dev *dev)
>  {
>  	struct cnic_local *cp = dev->cnic_priv;
> +	u32 port_id;
>  
>  	cp->csk_tbl = kzalloc(sizeof(struct cnic_sock) * MAX_CM_SK_TBL_SZ,
>  			      GFP_KERNEL);
>  	if (!cp->csk_tbl)
>  		return -ENOMEM;
>  
> +	get_random_bytes(&port_id, sizeof(port_id));

I think random32() or it's alias net_random() would be sufficient
for your needs here.

Re: [PATCH 5/5 net-next] cnic, bnx2: Check iSCSI support early in bnx2_init_one()

[Michael Chan]

On Thu, 2011-06-09 at 03:42 -0700, Neil Horman wrote:
> On Wed, Jun 08, 2011 at 10:29:36PM -0700, Michael Chan wrote:
> > Based on earlier patch from Neil Horman <nhorman@tuxdriver.com>
> > 
> > If iSCSI is not supported on a bnx2 device, bnx2_cnic_probe() will
> > return NULL and the cnic device will not be visible to bnx2i.  This
> > will prevent bnx2i from registering and then unregistering during
> > cnic_start() and cause the warning message:
> > 
> > bnx2 0003:01:00.1: eth1: Failed waiting for ULP up call to complete
> > 
> > Signed-off-by: Michael Chan <mchan@broadcom.com>
> > Cc: Neil Horman <nhorman@tuxdriver.com>
> Thank you Michael.  This also lets you eliminate some dead code in bnx2i_start
> now, since cdev->max_iscsi_conn will never be zero.
> 

This is true for bnx2.  For bnx2x, some devices may support FCoE only
and not iSCSI, so we still need code similar to what you proposed
yesterday.  We'll add that through the scsi tree.

Thanks.

Re: [PATCH 3/5 net-next] cnic: Randomize initial TCP port for iSCSI connections

[Michael Chan]

On Thu, 2011-06-09 at 08:27 -0700, Stephen Hemminger wrote:
> On Wed, 8 Jun 2011 22:29:34 -0700
> "Michael Chan" <mchan@broadcom.com> wrote:
> 
> >  static int cnic_cm_alloc_mem(struct cnic_dev *dev)
> >  {
> >  	struct cnic_local *cp = dev->cnic_priv;
> > +	u32 port_id;
> >  
> >  	cp->csk_tbl = kzalloc(sizeof(struct cnic_sock) * MAX_CM_SK_TBL_SZ,
> >  			      GFP_KERNEL);
> >  	if (!cp->csk_tbl)
> >  		return -ENOMEM;
> >  
> > +	get_random_bytes(&port_id, sizeof(port_id));
> 
> I think random32() or it's alias net_random() would be sufficient
> for your needs here.
> 

random32() is pseudo random so we can get the same number after reboot,
right?  One scenario is that we may be booting from an iSCSI target, but
due to some failure, the user may be rebooting multiple times rapidly.
We want to use a different port during each boot.

Re: [PATCH 3/5 net-next] cnic: Randomize initial TCP port for iSCSI connections

[Stephen Hemminger]

On Thu, 9 Jun 2011 18:08:00 -0700
"Michael Chan" <mchan@broadcom.com> wrote:

> 
> On Thu, 2011-06-09 at 08:27 -0700, Stephen Hemminger wrote:
> > On Wed, 8 Jun 2011 22:29:34 -0700
> > "Michael Chan" <mchan@broadcom.com> wrote:
> > 
> > >  static int cnic_cm_alloc_mem(struct cnic_dev *dev)
> > >  {
> > >  	struct cnic_local *cp = dev->cnic_priv;
> > > +	u32 port_id;
> > >  
> > >  	cp->csk_tbl = kzalloc(sizeof(struct cnic_sock) * MAX_CM_SK_TBL_SZ,
> > >  			      GFP_KERNEL);
> > >  	if (!cp->csk_tbl)
> > >  		return -ENOMEM;
> > >  
> > > +	get_random_bytes(&port_id, sizeof(port_id));
> > 
> > I think random32() or it's alias net_random() would be sufficient
> > for your needs here.
> > 
> 
> random32() is pseudo random so we can get the same number after reboot,
> right?  One scenario is that we may be booting from an iSCSI target, but
> due to some failure, the user may be rebooting multiple times rapidly.
> We want to use a different port during each boot.

random32 is seeded off get_random_bytes at boot.

Re: [PATCH 3/5 net-next] cnic: Randomize initial TCP port for iSCSI connections

[Eric Dumazet]

Le jeudi 09 juin 2011 à 21:01 -0700, Stephen Hemminger a écrit :
> On Thu, 9 Jun 2011 18:08:00 -0700
> "Michael Chan" <mchan@broadcom.com> wrote:
> 
> > 
> > On Thu, 2011-06-09 at 08:27 -0700, Stephen Hemminger wrote:
> > > On Wed, 8 Jun 2011 22:29:34 -0700
> > > "Michael Chan" <mchan@broadcom.com> wrote:
> > > 
> > > >  static int cnic_cm_alloc_mem(struct cnic_dev *dev)
> > > >  {
> > > >  	struct cnic_local *cp = dev->cnic_priv;
> > > > +	u32 port_id;
> > > >  
> > > >  	cp->csk_tbl = kzalloc(sizeof(struct cnic_sock) * MAX_CM_SK_TBL_SZ,
> > > >  			      GFP_KERNEL);
> > > >  	if (!cp->csk_tbl)
> > > >  		return -ENOMEM;
> > > >  
> > > > +	get_random_bytes(&port_id, sizeof(port_id));
> > > 
> > > I think random32() or it's alias net_random() would be sufficient
> > > for your needs here.
> > > 
> > 
> > random32() is pseudo random so we can get the same number after reboot,
> > right?  One scenario is that we may be booting from an iSCSI target, but
> > due to some failure, the user may be rebooting multiple times rapidly.
> > We want to use a different port during each boot.
> 
> random32 is seeded off get_random_bytes at boot.

cnic_cm_alloc_mem() is apparently called one time per boot, it seems OK
to call get_random_bytes() in this case ?

Re: [PATCH 3/5 net-next] cnic: Randomize initial TCP port for iSCSI connections

[Michael Chan]

Stephen Hemminger wrote:

> On Thu, 9 Jun 2011 18:08:00 -0700
> "Michael Chan" <mchan@broadcom.com> wrote:
> > random32() is pseudo random so we can get the same number after reboot,
> > right?  One scenario is that we may be booting from an iSCSI target, but
> > due to some failure, the user may be rebooting multiple times rapidly.
> > We want to use a different port during each boot.
> 
> random32 is seeded off get_random_bytes at boot.

Thanks.  We'll change it to random32() in our next patchset then.