PATCH] BNX2I : Bug fixes related to MTU change issue when there are active iscsi sessions
James, This patch is made against scsi-misc from this morning. Please apply. Thanks, Anil * bnx2i driver has to wait and cleanup all iscsi endpoints before returning from bnx2i_stop(). This is to make sure all chip resources are freed before chip is reset. * As the requirements for 1G and 10G chipsets are different, added per-device 'hba_shutdown_tmo' parameter to the adapter structure * If the connections are not torn down by the daemon within this timeout period, 'cid's will be leaked in 10G device. 1G devices are more flexible and do not leak any resources because the whole chip ports gets reset when MTU is changed or ethtool selftest is run * fixed a minor issue in bnx2i_ep_poll() which unnecessarily forced error return code when driver timed out waiting for TCP connect request to complete. This was also discovered while debugging MTU change issue Signed-off-by: Anil Veerabhadrappa ani...@broadcom.com --- drivers/scsi/bnx2i/bnx2i.h |2 ++ drivers/scsi/bnx2i/bnx2i_init.c | 13 - drivers/scsi/bnx2i/bnx2i_iscsi.c | 11 --- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/bnx2i/bnx2i.h b/drivers/scsi/bnx2i/bnx2i.h index 6cf9dc3..6b624e7 100644 --- a/drivers/scsi/bnx2i/bnx2i.h +++ b/drivers/scsi/bnx2i/bnx2i.h @@ -362,6 +362,7 @@ struct bnx2i_hba { u32 num_ccell; int ofld_conns_active; + wait_queue_head_t eh_wait; int max_active_conns; struct iscsi_cid_queue cid_que; @@ -381,6 +382,7 @@ struct bnx2i_hba { spinlock_t lock;/* protects hba structure access */ struct mutex net_dev_lock;/* sync net device access */ + int hba_shutdown_tmo; /* * PCI related info. */ diff --git a/drivers/scsi/bnx2i/bnx2i_init.c b/drivers/scsi/bnx2i/bnx2i_init.c index 6d8172e..5d9296c 100644 --- a/drivers/scsi/bnx2i/bnx2i_init.c +++ b/drivers/scsi/bnx2i/bnx2i_init.c @@ -177,11 +177,22 @@ void bnx2i_stop(void *handle) struct bnx2i_hba *hba = handle; /* check if cleanup happened in GOING_DOWN context */ - clear_bit(ADAPTER_STATE_UP, hba-adapter_state); if (!test_and_clear_bit(ADAPTER_STATE_GOING_DOWN, hba-adapter_state)) iscsi_host_for_each_session(hba-shost, bnx2i_drop_session); + + /* Wait for all endpoints to be torn down, Chip will be reset once +* control returns to network driver. So it is required to cleanup and +* release all connection resources before returning from this routine. +*/ + wait_event_interruptible_timeout(hba-eh_wait, +(hba-ofld_conns_active == 0), +hba-hba_shutdown_tmo); + /* This flag should be cleared last so that ep_disconnect() gracefully +* cleans up connection context +*/ + clear_bit(ADAPTER_STATE_UP, hba-adapter_state); } /** diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c index cb71dc9..974dfb6 100644 --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c @@ -819,6 +819,11 @@ struct bnx2i_hba *bnx2i_alloc_hba(struct cnic_dev *cnic) spin_lock_init(hba-lock); mutex_init(hba-net_dev_lock); + init_waitqueue_head(hba-eh_wait); + if (test_bit(BNX2I_NX2_DEV_57710, hba-cnic_dev_type)) + hba-hba_shutdown_tmo = 240 * HZ; + else/* 5706/5708/5709 */ + hba-hba_shutdown_tmo = 30 * HZ; if (iscsi_host_add(shost, hba-pcidev-dev)) goto free_dump_mem; @@ -1657,8 +1662,8 @@ static struct iscsi_endpoint *bnx2i_ep_connect(struct Scsi_Host *shost, */ hba = bnx2i_check_route(dst_addr); - if (!hba) { - rc = -ENOMEM; + if (!hba || test_bit(ADAPTER_STATE_GOING_DOWN, hba-adapter_state)) { + rc = -EINVAL; goto check_busy; } @@ -1803,7 +1808,7 @@ static int bnx2i_ep_poll(struct iscsi_endpoint *ep, int timeout_ms) (bnx2i_ep-state == EP_STATE_CONNECT_COMPL)), msecs_to_jiffies(timeout_ms)); - if (!rc || (bnx2i_ep-state == EP_STATE_OFLD_FAILED)) + if (bnx2i_ep-state == EP_STATE_OFLD_FAILED) rc = -1; if (rc 0) -- 1.6.5.1 -- You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-is...@googlegroups.com. To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/open
[RESEND PATCH] BNX2I - Bug fixes related to MTU change issue when there are active iscsi sessions
James, Please apply this patch to fix resources leak issue when device MTU is changed or ethtool selftest is executed while there are active iscsi sessions. Thanks, Anil * bnx2i driver has to wait and cleanup all iscsi endpoints before returning from bnx2i_stop(). This is to make sure all chip resources are freed before chip is reset. * As the requirements for 1G and 10G chipsets is different, added per-device 'hba_shutdown_tmo' parameter to adapter structure * If the connections are not torn down by the daemon within this timeout period, 'cid's will be leaked in 10G device. 1G devices are more flexible and do not leak any resources because the whole chip ports gets reset when MTU is changed or ethtool selftest is run * fixed a minor issue in bnx2i_ep_poll() which unnecessarily forced error return code when driver timed out waiting for TCP connect request to complete Signed-off-by: Anil Veerabhadrappa ani...@broadcom.com --- drivers/scsi/bnx2i/bnx2i.h |2 ++ drivers/scsi/bnx2i/bnx2i_init.c | 13 - drivers/scsi/bnx2i/bnx2i_iscsi.c | 13 ++--- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/bnx2i/bnx2i.h b/drivers/scsi/bnx2i/bnx2i.h index 6cf9dc3..6b624e7 100644 --- a/drivers/scsi/bnx2i/bnx2i.h +++ b/drivers/scsi/bnx2i/bnx2i.h @@ -362,6 +362,7 @@ struct bnx2i_hba { u32 num_ccell; int ofld_conns_active; + wait_queue_head_t eh_wait; int max_active_conns; struct iscsi_cid_queue cid_que; @@ -381,6 +382,7 @@ struct bnx2i_hba { spinlock_t lock;/* protects hba structure access */ struct mutex net_dev_lock;/* sync net device access */ + int hba_shutdown_tmo; /* * PCI related info. */ diff --git a/drivers/scsi/bnx2i/bnx2i_init.c b/drivers/scsi/bnx2i/bnx2i_init.c index 6d8172e..5d9296c 100644 --- a/drivers/scsi/bnx2i/bnx2i_init.c +++ b/drivers/scsi/bnx2i/bnx2i_init.c @@ -177,11 +177,22 @@ void bnx2i_stop(void *handle) struct bnx2i_hba *hba = handle; /* check if cleanup happened in GOING_DOWN context */ - clear_bit(ADAPTER_STATE_UP, hba-adapter_state); if (!test_and_clear_bit(ADAPTER_STATE_GOING_DOWN, hba-adapter_state)) iscsi_host_for_each_session(hba-shost, bnx2i_drop_session); + + /* Wait for all endpoints to be torn down, Chip will be reset once +* control returns to network driver. So it is required to cleanup and +* release all connection resources before returning from this routine. +*/ + wait_event_interruptible_timeout(hba-eh_wait, +(hba-ofld_conns_active == 0), +hba-hba_shutdown_tmo); + /* This flag should be cleared last so that ep_disconnect() gracefully +* cleans up connection context +*/ + clear_bit(ADAPTER_STATE_UP, hba-adapter_state); } /** diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c index cb71dc9..639e7d8 100644 --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c @@ -819,6 +819,11 @@ struct bnx2i_hba *bnx2i_alloc_hba(struct cnic_dev *cnic) spin_lock_init(hba-lock); mutex_init(hba-net_dev_lock); + init_waitqueue_head(hba-eh_wait); + if (test_bit(BNX2I_NX2_DEV_57710, hba-cnic_dev_type)) + hba-hba_shutdown_tmo = 240 * HZ; + else/* 5706/5708/5709 */ + hba-hba_shutdown_tmo = 30 * HZ; if (iscsi_host_add(shost, hba-pcidev-dev)) goto free_dump_mem; @@ -1657,8 +1662,8 @@ static struct iscsi_endpoint *bnx2i_ep_connect(struct Scsi_Host *shost, */ hba = bnx2i_check_route(dst_addr); - if (!hba) { - rc = -ENOMEM; + if (!hba || test_bit(ADAPTER_STATE_GOING_DOWN, hba-adapter_state)) { + rc = -EINVAL; goto check_busy; } @@ -1803,7 +1808,7 @@ static int bnx2i_ep_poll(struct iscsi_endpoint *ep, int timeout_ms) (bnx2i_ep-state == EP_STATE_CONNECT_COMPL)), msecs_to_jiffies(timeout_ms)); - if (!rc || (bnx2i_ep-state == EP_STATE_OFLD_FAILED)) + if (bnx2i_ep-state == EP_STATE_OFLD_FAILED) rc = -1; if (rc 0) @@ -1956,6 +1961,8 @@ return_bnx2i_ep: if (!hba-ofld_conns_active) bnx2i_unreg_dev_all(); + + wake_up_interruptible(hba-eh_wait); } -- 1.6.5.1 -- You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-is...@googlegroups.com. To unsubscribe from this group, send email
[PATCH 3/3] [BNX2I] update driver description and driver revision
[BNX2I] update driver description and driver revision * Add 10G devices to list of devices supported by bnx2i driver Signed-off-by: Anil Veerabhadrappa ani...@broadcom.com --- drivers/scsi/bnx2i/bnx2i_init.c |7 --- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/bnx2i/bnx2i_init.c b/drivers/scsi/bnx2i/bnx2i_init.c index 737dce0..ce0ee80 100644 --- a/drivers/scsi/bnx2i/bnx2i_init.c +++ b/drivers/scsi/bnx2i/bnx2i_init.c @@ -17,8 +17,8 @@ static struct list_head adapter_list = LIST_HEAD_INIT(adapter_list); static u32 adapter_count; #define DRV_MODULE_NAMEbnx2i -#define DRV_MODULE_VERSION 2.1.0 -#define DRV_MODULE_RELDATE Dec 06, 2009 +#define DRV_MODULE_VERSION 2.1.1 +#define DRV_MODULE_RELDATE Mar 24, 2010 static char version[] __devinitdata = Broadcom NetXtreme II iSCSI Driver DRV_MODULE_NAME \ @@ -26,7 +26,8 @@ static char version[] __devinitdata = MODULE_AUTHOR(Anil Veerabhadrappa ani...@broadcom.com); -MODULE_DESCRIPTION(Broadcom NetXtreme II BCM5706/5708/5709 iSCSI Driver); +MODULE_DESCRIPTION(Broadcom NetXtreme II BCM5706/5708/5709/57710/57711 + iSCSI Driver); MODULE_LICENSE(GPL); MODULE_VERSION(DRV_MODULE_VERSION); -- 1.6.5.1 -- You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-is...@googlegroups.com. To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/open-iscsi?hl=en.
Re: [PATCH 1/5] BNX2I - Add 5771E device support to bnx2i driver
On Thu, 2009-12-10 at 07:43 -0800, James Bottomley wrote: On Wed, 2009-12-09 at 20:45 -0600, Mike Christie wrote: Reviewed-by: Mike Christie micha...@cs.wisc.edu But not by checkpatch: ERROR: trailing whitespace #23: FILE: drivers/scsi/bnx2i/bnx2i_init.c:90: +^I^Iprintk(KERN_ALERT bnx2i: unknown device, 0x%x\n, $ total: 1 errors, 0 warnings, 13 lines checked I've fixed it up. And the several other whitespace problems in the rest of the patch set. Thanks James. In future will make sure not to repeat the slip-up. Thanks again. James -- You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-is...@googlegroups.com. To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/open-iscsi?hl=en.
[PATCH 1/5] BNX2I - Add 5771E device support to bnx2i driver
* Add code to enumerate 5771E device Signed-off-by: Anil Veerabhadrappa ani...@broadcom.com --- drivers/scsi/bnx2i/bnx2i_init.c |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/bnx2i/bnx2i_init.c b/drivers/scsi/bnx2i/bnx2i_init.c index 0c4210d..3c46458 100644 --- a/drivers/scsi/bnx2i/bnx2i_init.c +++ b/drivers/scsi/bnx2i/bnx2i_init.c @@ -83,8 +83,12 @@ void bnx2i_identify_device(struct bnx2i_hba *hba) set_bit(BNX2I_NX2_DEV_5709, hba-cnic_dev_type); hba-mail_queue_access = BNX2I_MQ_BIN_MODE; } else if (hba-pci_did == PCI_DEVICE_ID_NX2_57710 || - hba-pci_did == PCI_DEVICE_ID_NX2_57711) + hba-pci_did == PCI_DEVICE_ID_NX2_57711 || + hba-pci_did == PCI_DEVICE_ID_NX2_57711E) set_bit(BNX2I_NX2_DEV_57710, hba-cnic_dev_type); + else + printk(KERN_ALERT bnx2i: unknown device, 0x%x\n, + hba-pci_did); } -- 1.6.5.1 -- You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-is...@googlegroups.com. To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/open-iscsi?hl=en.
[PATCH 2/5 ] BNX2I - Adjust sq_size module parametr to power of 2 only if a non-zero value is specified
* This issue was discovered during 10G iscsi testing * Default value of 'sq_size' module parameter is '0' which means driver should use predefined SQ queue size when setting up iscsi connection. * roundup_pow_of_two(0) results in '1' and forces driver to setup connections with send queue size of '1' and results in lower performance as well Signed-off-by: Anil Veerabhadrappa ani...@broadcom.com --- drivers/scsi/bnx2i/bnx2i_init.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/bnx2i/bnx2i_init.c b/drivers/scsi/bnx2i/bnx2i_init.c index 3c46458..dc6b56c 100644 --- a/drivers/scsi/bnx2i/bnx2i_init.c +++ b/drivers/scsi/bnx2i/bnx2i_init.c @@ -367,7 +367,7 @@ static int __init bnx2i_mod_init(void) printk(KERN_INFO %s, version); - if (!is_power_of_2(sq_size)) + if (sq_size !is_power_of_2(sq_size)) sq_size = roundup_pow_of_two(sq_size); mutex_init(bnx2i_dev_lock); -- 1.6.5.1 -- You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-is...@googlegroups.com. To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/open-iscsi?hl=en.
[PATCH 3/5] BNX2I - update CQ arming algorith for 5771x chipsets
* Only affects 5771x (10G chipsets) devices * This is an optimized CQ arming algoritm which takes into account the number of outstanding tasks Signed-off-by: Anil Veerabhadrappa ani...@broadcom.com --- drivers/scsi/bnx2i/bnx2i.h |1 + drivers/scsi/bnx2i/bnx2i_hwi.c | 36 +++- drivers/scsi/bnx2i/bnx2i_init.c |4 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/drivers/scsi/bnx2i/bnx2i.h b/drivers/scsi/bnx2i/bnx2i.h index 2b973f3..6cf9dc3 100644 --- a/drivers/scsi/bnx2i/bnx2i.h +++ b/drivers/scsi/bnx2i/bnx2i.h @@ -684,6 +684,7 @@ extern unsigned int error_mask1, error_mask2; extern u64 iscsi_error_mask; extern unsigned int en_tcp_dack; extern unsigned int event_coal_div; +extern unsigned int event_coal_min; extern struct scsi_transport_template *bnx2i_scsi_xport_template; extern struct iscsi_transport bnx2i_iscsi_transport; diff --git a/drivers/scsi/bnx2i/bnx2i_hwi.c b/drivers/scsi/bnx2i/bnx2i_hwi.c index 5c8d763..57f4ca9 100644 --- a/drivers/scsi/bnx2i/bnx2i_hwi.c +++ b/drivers/scsi/bnx2i/bnx2i_hwi.c @@ -133,21 +133,39 @@ void bnx2i_arm_cq_event_coalescing(struct bnx2i_endpoint *ep, u8 action) { struct bnx2i_5771x_cq_db *cq_db; u16 cq_index; + u16 next_index; + u32 num_active_cmds; + + /* Coalesce CQ entries only on 10G devices */ if (!test_bit(BNX2I_NX2_DEV_57710, ep-hba-cnic_dev_type)) return; + /* Do not update CQ DB multiple times before firmware writes +* '0x' to CQDB-SQN field. Deviation may cause spurious +* interrupts and other unwanted results +*/ + cq_db = (struct bnx2i_5771x_cq_db *) ep-qp.cq_pgtbl_virt; + if (cq_db-sqn[0] cq_db-sqn[0] != 0x) + return; + if (action == CNIC_ARM_CQE) { - cq_index = ep-qp.cqe_exp_seq_sn + - ep-num_active_cmds / event_coal_div; - cq_index %= (ep-qp.cqe_size * 2 + 1); - if (!cq_index) { + num_active_cmds = ep-num_active_cmds; + if (num_active_cmds = event_coal_min) + next_index = 1; + else + next_index = event_coal_min + + (num_active_cmds - event_coal_min) / event_coal_div; + if (!next_index) + next_index = 1; + cq_index = ep-qp.cqe_exp_seq_sn + next_index - 1; + if (cq_index ep-qp.cqe_size * 2) + cq_index -= ep-qp.cqe_size * 2; + if (!cq_index) cq_index = 1; - cq_db = (struct bnx2i_5771x_cq_db *) - ep-qp.cq_pgtbl_virt; - cq_db-sqn[0] = cq_index; - } - } + + cq_db-sqn[0] = cq_index; +} } diff --git a/drivers/scsi/bnx2i/bnx2i_init.c b/drivers/scsi/bnx2i/bnx2i_init.c index dc6b56c..ad551c5 100644 --- a/drivers/scsi/bnx2i/bnx2i_init.c +++ b/drivers/scsi/bnx2i/bnx2i_init.c @@ -32,6 +32,10 @@ MODULE_VERSION(DRV_MODULE_VERSION); static DEFINE_MUTEX(bnx2i_dev_lock); +unsigned int event_coal_min = 24; +module_param(event_coal_min, int, 0664); +MODULE_PARM_DESC(event_coal_min, Event Coalescing Minimum Commands); + unsigned int event_coal_div = 1; module_param(event_coal_div, int, 0664); MODULE_PARM_DESC(event_coal_div, Event Coalescing Divide Factor); -- 1.6.5.1 -- You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-is...@googlegroups.com. To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/open-iscsi?hl=en.
[PATCH 4/5] BNX2I - Task management ABORT TASK fixes
* Due to typo error driver was failing TMF Abort Task request when ctask-sc != NULL. Fixed code to fail TMF ABORT Task request only when ctask-sc == NULL * Clear age component (19 most significant bits) of reference ITT carried in iSCSI TMF PDU. Age component is internal to initiator side and only lower bits of ITT as defined by ISCSI_ITT_MASK is is sent on wire * Retrieve LUN directly from the ref_sc and update SQ wqe as per chip HSI (Host Software Interface) specification Signed-off-by: Anil Veerabhadrappa ani...@broadcom.com --- drivers/scsi/bnx2i/bnx2i_hwi.c | 17 + 1 files changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/bnx2i/bnx2i_hwi.c b/drivers/scsi/bnx2i/bnx2i_hwi.c index 57f4ca9..af350b2 100644 --- a/drivers/scsi/bnx2i/bnx2i_hwi.c +++ b/drivers/scsi/bnx2i/bnx2i_hwi.c @@ -384,6 +384,7 @@ int bnx2i_send_iscsi_tmf(struct bnx2i_conn *bnx2i_conn, struct bnx2i_cmd *bnx2i_cmd; struct bnx2i_tmf_request *tmfabort_wqe; u32 dword; + u32 scsi_lun[2]; bnx2i_cmd = (struct bnx2i_cmd *)mtask-dd_data; tmfabort_hdr = (struct iscsi_tm *)mtask-hdr; @@ -394,27 +395,35 @@ int bnx2i_send_iscsi_tmf(struct bnx2i_conn *bnx2i_conn, tmfabort_wqe-op_attr = 0; tmfabort_wqe-op_attr = ISCSI_TMF_REQUEST_ALWAYS_ONE | ISCSI_TM_FUNC_ABORT_TASK; - tmfabort_wqe-lun[0] = be32_to_cpu(tmfabort_hdr-lun[0]); - tmfabort_wqe-lun[1] = be32_to_cpu(tmfabort_hdr-lun[1]); tmfabort_wqe-itt = (mtask-itt | (ISCSI_TASK_TYPE_MPATH 14)); tmfabort_wqe-reserved2 = 0; tmfabort_wqe-cmd_sn = be32_to_cpu(tmfabort_hdr-cmdsn); ctask = iscsi_itt_to_task(conn, tmfabort_hdr-rtt); - if (!ctask || ctask-sc) + if (!ctask || !ctask-sc) /* * the iscsi layer must have completed the cmd while this * was starting up. +* +* Note: In the case of a SCSI cmd timeout, the task's sc +* is still active; hence ctask-sc != 0 +* In this case, the task must be aborted */ return 0; + ref_sc = ctask-sc; + /* Retrieve LUN directly from the ref_sc */ + int_to_scsilun(ref_sc-device-lun, (struct scsi_lun *) scsi_lun); + tmfabort_wqe-lun[0] = be32_to_cpu(scsi_lun[0]); + tmfabort_wqe-lun[1] = be32_to_cpu(scsi_lun[1]); + if (ref_sc-sc_data_direction == DMA_TO_DEVICE) dword = (ISCSI_TASK_TYPE_WRITE ISCSI_CMD_REQUEST_TYPE_SHIFT); else dword = (ISCSI_TASK_TYPE_READ ISCSI_CMD_REQUEST_TYPE_SHIFT); - tmfabort_wqe-ref_itt = (dword | tmfabort_hdr-rtt); + tmfabort_wqe-ref_itt = (dword | (tmfabort_hdr-rtt ISCSI_ITT_MASK)); tmfabort_wqe-ref_cmd_sn = be32_to_cpu(tmfabort_hdr-refcmdsn); tmfabort_wqe-bd_list_addr_lo = (u32) bnx2i_conn-hba-mp_bd_dma; -- 1.6.5.1 -- You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-is...@googlegroups.com. To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/open-iscsi?hl=en.
[PATCH 5/5] BNX2I - minor code cleanup and update driver version
* Removed duplicate function call and not-so-useful comment line Signed-off-by: Anil Veerabhadrappa ani...@broadcom.com --- drivers/scsi/bnx2i/bnx2i_init.c |4 ++-- drivers/scsi/bnx2i/bnx2i_iscsi.c |2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/bnx2i/bnx2i_init.c b/drivers/scsi/bnx2i/bnx2i_init.c index ad551c5..1e111ac 100644 --- a/drivers/scsi/bnx2i/bnx2i_init.c +++ b/drivers/scsi/bnx2i/bnx2i_init.c @@ -17,8 +17,8 @@ static struct list_head adapter_list = LIST_HEAD_INIT(adapter_list); static u32 adapter_count; #define DRV_MODULE_NAMEbnx2i -#define DRV_MODULE_VERSION 2.0.1e -#define DRV_MODULE_RELDATE June 22, 2009 +#define DRV_MODULE_VERSION 2.1.0 +#define DRV_MODULE_RELDATE Dec 06, 2009 static char version[] __devinitdata = Broadcom NetXtreme II iSCSI Driver DRV_MODULE_NAME \ diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c index 070118a..54dc251 100644 --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c @@ -485,7 +485,6 @@ static int bnx2i_setup_cmd_pool(struct bnx2i_hba *hba, struct iscsi_task *task = session-cmds[i]; struct bnx2i_cmd *cmd = task-dd_data; - /* Anil */ task-hdr = cmd-hdr; task-hdr_max = sizeof(struct iscsi_hdr); @@ -765,7 +764,6 @@ struct bnx2i_hba *bnx2i_alloc_hba(struct cnic_dev *cnic) hba-pci_svid = hba-pcidev-subsystem_vendor; hba-pci_func = PCI_FUNC(hba-pcidev-devfn); hba-pci_devno = PCI_SLOT(hba-pcidev-devfn); - bnx2i_identify_device(hba); bnx2i_identify_device(hba); bnx2i_setup_host_queue_size(hba, shost); -- 1.6.5.1 -- You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-is...@googlegroups.com. To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/open-iscsi?hl=en.
[PATCH 1/1] BNX2I - Fix context mapping issue for architectures with PAGE_SIZE != 4096
[BNX2I] Fix context mapping issue for architectures with PAGE_SIZE != 4096 * 5706/5708/5709 devices allow driver/user to set page size. By default it is set to 4096 * Current drivers do not program this register based on architecture type (e.g. x86 = 4K, IA64 = 16K) and by choice lets device use the defaults * So while mapping connection context memory (doorebll registers), driver has to match page size used by the device. Included change fixes the issue we uncovered during IA64 testing Signed-off-by: Anil Veerabhadrappa ani...@broadcom.com --- drivers/scsi/bnx2i/bnx2i.h |2 ++ drivers/scsi/bnx2i/bnx2i_hwi.c |2 +- 2 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/bnx2i/bnx2i.h b/drivers/scsi/bnx2i/bnx2i.h index d7576f2..5edde1a 100644 --- a/drivers/scsi/bnx2i/bnx2i.h +++ b/drivers/scsi/bnx2i/bnx2i.h @@ -100,6 +100,8 @@ #define CTX_OFFSET 0x1 #define MAX_CID_CNT0x4000 +#define BNX2I_570X_PAGE_SIZE_DEFAULT 4096 + /* 5709 context registers */ #define BNX2_MQ_CONFIG20x3d00 #define BNX2_MQ_CONFIG2_CONT_SZ(0x7L4) diff --git a/drivers/scsi/bnx2i/bnx2i_hwi.c b/drivers/scsi/bnx2i/bnx2i_hwi.c index 906cef5..0afde71 100644 --- a/drivers/scsi/bnx2i/bnx2i_hwi.c +++ b/drivers/scsi/bnx2i/bnx2i_hwi.c @@ -2386,7 +2386,7 @@ int bnx2i_map_ep_dbell_regs(struct bnx2i_endpoint *ep) ctx_sz = (config2 BNX2_MQ_CONFIG2_CONT_SZ) 3; if (ctx_sz) reg_off = CTX_OFFSET + MAX_CID_CNT * MB_KERNEL_CTX_SIZE - + PAGE_SIZE * + + BNX2I_570X_PAGE_SIZE_DEFAULT * (((cid_num - first_l4l5) / ctx_sz) + 256); else reg_off = CTX_OFFSET + (MB_KERNEL_CTX_SIZE * cid_num); -- 1.5.4.3 --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: Use iscsi kernel passthrough interface for sendtargets
On Sun, 2009-08-30 at 14:31 -0700, Mike Christie wrote: For offlaod cards, bnx2i, cxgb3i and be2iscsi, we have to do discovery through a second network interface. The two attached patches has us instead do discovery through the iscsi class interface, so you do not have to set up a seperate nic for discovery and you do not have to worry about IP based ACLS on the target. The 0001-iscsiadm-use-kernel-pdu-passthrough-for-sendtargets.patch patch was made over the open-iscsi git tree. It is only userspce code. The 0001-libiscsi-fix-login-text-checks-in-pdu-injection-cod.patch patch is made over linux-2.6-iscsi's iscsi branch. It should probably apply to other kernels though. Please give the patches a try. Here are some known limits: MaxRecvSegmentLength is hard coded to 8K for discovery sessions, because the kernel conn-data buffer and driver related buffers were hard coded to this value and some netlink's buffers are too. I have only tested cxgb3i. Ben, this is not patched against the uip offload tree but I can do that for you. Michael and Anil, I was not sure where to handle text response pdus in bnx2i_process_new_cqes? I am currently sending the discovery session login and text pdus through the same interface you get nops or normal session login pdus, so I think bnx2i_mtask_xmit should just work (I limited MaxRecvSegmentLength to ISCSI_DEF_MAX_RECV_SEG_LEN, so the buf sizes are ok.). I will work on it and send it to you soon. where is the latest bnx2i code on your linux-2.6-iscsi tree? Jay, not sure how text response pdus are coming in? Are they like login or logout pdus? --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
[PATCH 2/2] bnx2i : Fix cid #n not valid issue
* when bnx2i_adapter_ready() fails, connection handle(cid) = 0 is wrongly freed because 'cid' is not yet allocated for the endpoint * Fix is to initialize bnx2i_ep-ep_iscsi_cid to '-1' in bnx2i_alloc_ep() and not in bnx2i_ep_connect() to avoid releasing invalid 'cid' * There is already a check in bnx2i_free_iscsi_cid() not to free invalid iscsi connection handle (-1) Signed-off-by: Anil Veerabhadrappa ani...@broadcom.com --- drivers/scsi/bnx2i/bnx2i_iscsi.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c index 9535bb6..08d0bfc 100644 --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c @@ -387,6 +387,7 @@ static struct iscsi_endpoint *bnx2i_alloc_ep(struct bnx2i_hba *hba) bnx2i_ep = ep-dd_data; INIT_LIST_HEAD(bnx2i_ep-link); bnx2i_ep-state = EP_STATE_IDLE; + bnx2i_ep-ep_iscsi_cid = (u16) -1; bnx2i_ep-hba = hba; bnx2i_ep-hba_age = hba-age; hba-ofld_conns_active++; @@ -1678,8 +1679,6 @@ static struct iscsi_endpoint *bnx2i_ep_connect(struct Scsi_Host *shost, goto net_if_down; } - bnx2i_ep-state = EP_STATE_IDLE; - bnx2i_ep-ep_iscsi_cid = (u16) -1; bnx2i_ep-num_active_cmds = 0; iscsi_cid = bnx2i_alloc_iscsi_cid(hba); if (iscsi_cid == -1) { -- 1.5.4.3 --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: [PATCH] BNX2I: register given device with cnic if shost != NULL in ep_connect()
On Thu, 2009-07-09 at 00:15 -0700, Michael Chan wrote: Mike Christie wrote: Anil Veerabhadrappa wrote: diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c index f741219..98148f3 100644 --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c @@ -1653,15 +1653,18 @@ static struct iscsi_endpoint *bnx2i_ep_connect(struct Scsi_Host *shost, struct iscsi_endpoint *ep; int rc = 0; - if (shost) + if (shost) { /* driver is given scsi host to work with */ hba = iscsi_host_priv(shost); - else + /* Register the device with cnic if not already done so */ + bnx2i_register_device(hba); + } else /* * check if the given destination can be reached through * a iscsi capable NetXtreme2 device */ hba = bnx2i_check_route(dst_addr); + if (!hba) { rc = -ENOMEM; goto check_busy; Do you want to do the test_bit(BNX2I_CNIC_REGISTERED, hba-reg_with_cnic) test here instead of down below. If it failed then you might try to do a cm_create on a cnic that is not properly registered. Good idea. cm_create() will properly be ok because there is no hardware interaction. bnx2i_send_conn_ofld_req() will fail if the cnic is not properly registered because there will be no call backs from the hardware events. following 2 conditions needs to be satisfied to offload an iscsi connection, 1) device has to be registered with cnic 2) device has to support iscsi offload bnx2i_adapter_ready() will return success only if both conditions (which is flagged by ADAPTER_STATE_UP bit in hba-adapter_state) are met. bnx2i_ep_connect() will bailout if bnx2i_adapter_ready() does not return correct status and this guarantees bnx2i will not make any cnic calls on an unregistered device. --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: [PATCH] BNX2I: register given device with cnic if shost != NULL in ep_connect()
On Thu, 2009-07-09 at 10:43 -0700, Mike Christie wrote: On 07/09/2009 09:39 AM, Anil Veerabhadrappa wrote: On Thu, 2009-07-09 at 00:15 -0700, Michael Chan wrote: Mike Christie wrote: Anil Veerabhadrappa wrote: diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c index f741219..98148f3 100644 --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c @@ -1653,15 +1653,18 @@ static struct iscsi_endpoint *bnx2i_ep_connect(struct Scsi_Host *shost, struct iscsi_endpoint *ep; int rc = 0; - if (shost) + if (shost) { /* driver is given scsi host to work with */ hba = iscsi_host_priv(shost); - else + /* Register the device with cnic if not already done so */ + bnx2i_register_device(hba); + } else /* * check if the given destination can be reached through * a iscsi capable NetXtreme2 device */ hba = bnx2i_check_route(dst_addr); + if (!hba) { rc = -ENOMEM; goto check_busy; Do you want to do the test_bit(BNX2I_CNIC_REGISTERED, hba-reg_with_cnic) test here instead of down below. If it failed then you might try to do a cm_create on a cnic that is not properly registered. Good idea. cm_create() will properly be ok because there is no hardware interaction. bnx2i_send_conn_ofld_req() will fail if the cnic is not properly registered because there will be no call backs from the hardware events. following 2 conditions needs to be satisfied to offload an iscsi connection, 1) device has to be registered with cnic 2) device has to support iscsi offload bnx2i_adapter_ready() will return success only if both conditions (which is flagged by ADAPTER_STATE_UP bit in hba-adapter_state) are met. bnx2i_ep_connect() will bailout if bnx2i_adapter_ready() does not return correct status and this guarantees bnx2i will not make any cnic calls on an unregistered device. Ok then. It seems fine to me. Reviewed-by Mike Christie micha...@cs.wisc.edu One other question though. Do you need the BNX2I_CNIC_REGISTERED tst in ep_connect then? If not then maybe in separate patch for the next feature window we can clean that up. It is not required to test for BNX2I_CNIC_REGISTERED because a successful return from bnx2i_adapter_ready() implicitly means the device is registered with cnic --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: [PATCH] BNX2I: register given device with cnic if shost != NULL in ep_connect()
On Thu, 2009-07-09 at 10:43 -0700, Mike Christie wrote: On 07/09/2009 09:39 AM, Anil Veerabhadrappa wrote: On Thu, 2009-07-09 at 00:15 -0700, Michael Chan wrote: Mike Christie wrote: Anil Veerabhadrappa wrote: diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c index f741219..98148f3 100644 --- a/drivers/scsi/bnx2i/bnx2i_iscsi.c +++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c @@ -1653,15 +1653,18 @@ static struct iscsi_endpoint *bnx2i_ep_connect(struct Scsi_Host *shost, struct iscsi_endpoint *ep; int rc = 0; - if (shost) + if (shost) { /* driver is given scsi host to work with */ hba = iscsi_host_priv(shost); - else + /* Register the device with cnic if not already done so */ + bnx2i_register_device(hba); + } else /* * check if the given destination can be reached through * a iscsi capable NetXtreme2 device */ hba = bnx2i_check_route(dst_addr); + if (!hba) { rc = -ENOMEM; goto check_busy; Do you want to do the test_bit(BNX2I_CNIC_REGISTERED, hba-reg_with_cnic) test here instead of down below. If it failed then you might try to do a cm_create on a cnic that is not properly registered. Good idea. cm_create() will properly be ok because there is no hardware interaction. bnx2i_send_conn_ofld_req() will fail if the cnic is not properly registered because there will be no call backs from the hardware events. following 2 conditions needs to be satisfied to offload an iscsi connection, 1) device has to be registered with cnic 2) device has to support iscsi offload bnx2i_adapter_ready() will return success only if both conditions (which is flagged by ADAPTER_STATE_UP bit in hba-adapter_state) are met. bnx2i_ep_connect() will bailout if bnx2i_adapter_ready() does not return correct status and this guarantees bnx2i will not make any cnic calls on an unregistered device. Ok then. It seems fine to me. Reviewed-by Mike Christie micha...@cs.wisc.edu One other question though. Do you need the BNX2I_CNIC_REGISTERED tst in ep_connect then? If not then maybe in separate patch for the next feature window we can clean that up. I agree there is no incentive to do the extra work if device registration failed. We will take care of this in the next feature window. Thanks!! --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: [PATCH] bind offloaded connection to port
On Tue, 2009-04-14 at 19:19 -0700, Mike Christie wrote: Hey offload guys, If we are using a offload card, then iface_set_param will match the iface info to a scsi_host and pass that info down to setup the net settings of the port (currently we just set the ip address). When we create the tcp/ip connection by calling ep_connect, we currently just go by the routing table info. I think there are two problems with this. 1. Some drivers do not have access to a routing table. Some drivers like qla4xxx do not even know about other ports. 2. If you have two initiator ports on the same subnet, the user may have set things up so that session1 was supposed to be run through port1. and session2 was supposed to be run through port2. It looks like we could end with both sessions going through one of the ports. Also how do you edit the routing table for the offload cards? You cannot use normal net tools like route can you? 3. If we set up hostA in the iface_set_param step, but then the routing info leads us to hostB, we are stuck. I did the attached patches to fix this. Basically we just pass down the scsi host we want to go through. Well, ok I began to fix this :) For qla4xxx or serverengines I think this will work fine. For bnx2i and cxgb3i, I am not sure. See the TODO and note in cxgb3i in kern-ep-connect-through-host.patch. bnx2i guys, you guys do somehting similar so will this work? In ep_connect can I control which host/port to use? this will work for bnx2i The patches were made against my iscsi tress. The kernel one was made over the iscsi brandh and that was just updated so you might want to reclone. The userspace one was made over the open-iscsi git tree head. --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
[RFC] : network configuration for iscsi hba
Hi, We'd like to propose a general scheme for configuring network parameters (IP address/mask/DHCP, etc) for iSCSI NICs that use a private IP address. The current scheme of using sysfs is not very good because bootproto, IP address, netmask, VLAN ID, etc all need to be set together. Having separate sysfs entries and updating them separately and independently will not work. Putting everything in a netlink message seems to be a better solution. After weighing different solutions, we feel expanding existing netlink family, NETLINK_ISCSI between iscsid and scsi_transport_iscsi is a flexible solution which allows information flow to be initiated from either side. Also this solution is flexible and elegantly handles network devices with multiple IP addresses. The objective of this proposal is to make this interface common for all iscsi offload solutions supported by open-iscsi. We would like to hear comments and suggestions from other iscsi offload vendors in defining this interface. Regards, Anil Veerabhadrappa Proposal to add netlink message type: - 3 new netlink message types are required to support network config message exchange between user and kernel components, 1. ISCSI_UEVENT_SET_NET_CONFIG - push iface info to driver to configure an iscsi network interface 2. ISCSI_UEVENT_GET_NET_CONFIG - get MAC address list, etc' 3. ISCSI_KEVENT_NET_CONFIG - propagate attribute changes from adapter to iscsid (e.g. advertise newly obtained dhcp address) iscsid will use this netlink messages to pass network configuration between user mode application and the driver. Once this message is received by scsi_transport_iscsi module it will call driver's newly added callback handlers in the iscsi_transport structure(net_config get_net_config) and also broadcast netlink message back to any hardware vendor's user level daemons ISCSI_XEVENT_NET_CONFIG message payload format: --- Payload consists of one or more config parameters defined in TLV (Type - Length - Value) format. struct net_cfg_tlv { uint32_t type; uint32_t length; uint8_t value[0]; }; Message types: -- This interface is envisioned to support standard network parameters such as netdev name, MAC address, IPv4/IPv6 address, VLAN, boot protocol (static or dhcp), etc'. Please refer to 'net_cfg_e' in proposed header file changes below. Advantages: --- This approach is much cleaner and delinks network configuration parameters currently bound to host attributes. Old scheme actually breaks the layering scheme as seen below, SCSI (net config parametes currently resides here) | iSCSI | TCP/IP This new approach is a deviation from current host attributes approach and places emphasis in iface components of iscsid and the LLD. Changes to iscsi transport headers: --- diff --git a/include/iscsi_if.h b/include/iscsi_if.h index afa86e8..76eea8c 100644 --- a/include/iscsi_if.h +++ b/include/iscsi_if.h @@ -51,6 +51,8 @@ enum iscsi_uevent_e { ISCSI_UEVENT_SET_HOST_PARAM = UEVENT_BASE + 16, ISCSI_UEVENT_UNBIND_SESSION = UEVENT_BASE + 17, ISCSI_UEVENT_CREATE_BOUND_SESSION = UEVENT_BASE + 18, + ISCSI_UEVENT_SET_NET_CONFIG = UEVENT_BASE + 19, + ISCSI_UEVENT_GET_NET_CONFIG = UEVENT_BASE + 20, /* up events */ ISCSI_KEVENT_RECV_PDU = KEVENT_BASE + 1, @@ -59,6 +61,7 @@ enum iscsi_uevent_e { ISCSI_KEVENT_DESTROY_SESSION= KEVENT_BASE + 4, ISCSI_KEVENT_UNBIND_SESSION = KEVENT_BASE + 5, ISCSI_KEVENT_CREATE_SESSION = KEVENT_BASE + 6, + ISCSI_KEVENT_NET_CONFIG = KEVENT_BASE + 7, }; enum iscsi_tgt_dscvr { @@ -317,6 +320,42 @@ enum iscsi_host_param { #define ISCSI_HOST_NETDEV_NAME (1ULL ISCSI_HOST_PARAM_NETDEV_NAME) #define ISCSI_HOST_IPADDRESS (1ULL ISCSI_HOST_PARAM_IPADDRESS) +/* iscsi network stack parameters */ +enum iscsi_net_cfg_e { + ISCSI_NET_CFG_UNKNOWN 0x00, + + ISCSI_NET_CFG_NETDEV_NAME ISCSI_NET_CFG_UNKNOWN + 1, + ISCSI_NET_CFG_MAC_ADDR ISCSI_NET_CFG_UNKNOWN + 2, + ISCSI_NET_CFG_IPV4_ADDR ISCSI_NET_CFG_UNKNOWN + 3, + ISCSI_NET_CFG_IPV6_ADDR ISCSI_NET_CFG_UNKNOWN + 4, + ISCSI_NET_CFG_IPV4_NETMASK ISCSI_NET_CFG_UNKNOWN + 5, + ISCSI_NET_CFG_IPV6_NETMASK ISCSI_NET_CFG_UNKNOWN + 6, + ISCSI_NET_CFG_IPV4_GATEWAY ISCSI_NET_CFG_UNKNOWN + 7, + ISCSI_NET_CFG_IPV6_GATEWAY ISCSI_NET_CFG_UNKNOWN + 8, + ISCSI_NET_CFG_BOOTPROTO ISCSI_NET_CFG_UNKNOWN + 9, + ISCSI_NET_CFG_IPV6_AUTO_CFG ISCSI_NET_CFG_UNKNOWN + 10, + ISCSI_NET_CFG_MTU
RE: [RFC] : network configuration for iscsi hba
On Thu, 2009-02-05 at 14:33 -0800, Karen Xie wrote: -Original Message- From: open-iscsi@googlegroups.com [mailto:open-is...@googlegroups.com] On Behalf Of Anil Veerabhadrappa Sent: Thursday, February 05, 2009 11:23 AM To: open-iscsi@googlegroups.com; micha...@cs.wisc.edu Cc: mchri...@redhat.com; mc...@broadcom.com; be...@broadcom.com; ani...@broadcom.com Subject: [RFC] : network configuration for iscsi hba Hi, We'd like to propose a general scheme for configuring network parameters (IP address/mask/DHCP, etc) for iSCSI NICs that use a private IP address. The current scheme of using sysfs is not very good because bootproto, IP address, netmask, VLAN ID, etc all need to be set together. Having separate sysfs entries and updating them separately and independently will not work. Putting everything in a netlink message seems to be a better solution. After weighing different solutions, we feel expanding existing netlink family, NETLINK_ISCSI between iscsid and scsi_transport_iscsi is a flexible solution which allows information flow to be initiated from either side. Also this solution is flexible and elegantly handles network devices with multiple IP addresses. The objective of this proposal is to make this interface common for all iscsi offload solutions supported by open-iscsi. We would like to hear comments and suggestions from other iscsi offload vendors in defining this interface. Regards, Anil Veerabhadrappa Proposal to add netlink message type: - 3 new netlink message types are required to support network config message exchange between user and kernel components, 1. ISCSI_UEVENT_SET_NET_CONFIG - push iface info to driver to configure an iscsi network interface 2. ISCSI_UEVENT_GET_NET_CONFIG - get MAC address list, etc' 3. ISCSI_KEVENT_NET_CONFIG - propagate attribute changes from adapter to iscsid (e.g. advertise newly obtained dhcp address) iscsid will use this netlink messages to pass network configuration between user mode application and the driver. Once this message is received by scsi_transport_iscsi module it will call driver's newly added callback handlers in the iscsi_transport structure(net_config get_net_config) and also broadcast netlink message back to any hardware vendor's user level daemons ISCSI_XEVENT_NET_CONFIG message payload format: --- Payload consists of one or more config parameters defined in TLV (Type - Length - Value) format. struct net_cfg_tlv { uint32_t type; uint32_t length; uint8_t value[0]; }; Message types: -- This interface is envisioned to support standard network parameters such as netdev name, MAC address, IPv4/IPv6 address, VLAN, boot protocol (static or dhcp), etc'. Please refer to 'net_cfg_e' in proposed header file changes below. Advantages: --- This approach is much cleaner and delinks network configuration parameters currently bound to host attributes. Old scheme actually breaks the layering scheme as seen below, SCSI (net config parametes currently resides here) | iSCSI | TCP/IP This new approach is a deviation from current host attributes approach and places emphasis in iface components of iscsid and the LLD. Changes to iscsi transport headers: --- diff --git a/include/iscsi_if.h b/include/iscsi_if.h index afa86e8..76eea8c 100644 --- a/include/iscsi_if.h +++ b/include/iscsi_if.h @@ -51,6 +51,8 @@ enum iscsi_uevent_e { ISCSI_UEVENT_SET_HOST_PARAM = UEVENT_BASE + 16, ISCSI_UEVENT_UNBIND_SESSION = UEVENT_BASE + 17, ISCSI_UEVENT_CREATE_BOUND_SESSION = UEVENT_BASE + 18, + ISCSI_UEVENT_SET_NET_CONFIG = UEVENT_BASE + 19, + ISCSI_UEVENT_GET_NET_CONFIG = UEVENT_BASE + 20, /* up events */ ISCSI_KEVENT_RECV_PDU = KEVENT_BASE + 1, @@ -59,6 +61,7 @@ enum iscsi_uevent_e { ISCSI_KEVENT_DESTROY_SESSION= KEVENT_BASE + 4, ISCSI_KEVENT_UNBIND_SESSION = KEVENT_BASE + 5, ISCSI_KEVENT_CREATE_SESSION = KEVENT_BASE + 6, + ISCSI_KEVENT_NET_CONFIG = KEVENT_BASE + 7, }; enum iscsi_tgt_dscvr { @@ -317,6 +320,42 @@ enum iscsi_host_param { #define ISCSI_HOST_NETDEV_NAME (1ULL ISCSI_HOST_PARAM_NETDEV_NAME) #define ISCSI_HOST_IPADDRESS (1ULL ISCSI_HOST_PARAM_IPADDRESS) +/* iscsi network stack parameters */ +enum iscsi_net_cfg_e { + ISCSI_NET_CFG_UNKNOWN 0x00, + + ISCSI_NET_CFG_NETDEV_NAME ISCSI_NET_CFG_UNKNOWN + 1, + ISCSI_NET_CFG_MAC_ADDR ISCSI_NET_CFG_UNKNOWN + 2, + ISCSI_NET_CFG_IPV4_ADDR ISCSI_NET_CFG_UNKNOWN + 3, + ISCSI_NET_CFG_IPV6_ADDR
RE: [RFC] : network configuration for iscsi hba
On Thu, 2009-02-05 at 16:00 -0800, Karen Xie wrote: Thanks, What are the possible values of ISCSI_NET_CFG_BOOTPROTO? static and dhcp Karen -Original Message- From: open-iscsi@googlegroups.com [mailto:open-is...@googlegroups.com] On Behalf Of Anil Veerabhadrappa Sent: Thursday, February 05, 2009 3:23 PM To: open-iscsi@googlegroups.com Cc: micha...@cs.wisc.edu; mchri...@redhat.com; Michael Chan; Benjamin Li Subject: RE: [RFC] : network configuration for iscsi hba On Thu, 2009-02-05 at 14:33 -0800, Karen Xie wrote: -Original Message- From: open-iscsi@googlegroups.com [mailto:open-is...@googlegroups.com] On Behalf Of Anil Veerabhadrappa Sent: Thursday, February 05, 2009 11:23 AM To: open-iscsi@googlegroups.com; micha...@cs.wisc.edu Cc: mchri...@redhat.com; mc...@broadcom.com; be...@broadcom.com; ani...@broadcom.com Subject: [RFC] : network configuration for iscsi hba Hi, We'd like to propose a general scheme for configuring network parameters (IP address/mask/DHCP, etc) for iSCSI NICs that use a private IP address. The current scheme of using sysfs is not very good because bootproto, IP address, netmask, VLAN ID, etc all need to be set together. Having separate sysfs entries and updating them separately and independently will not work. Putting everything in a netlink message seems to be a better solution. After weighing different solutions, we feel expanding existing netlink family, NETLINK_ISCSI between iscsid and scsi_transport_iscsi is a flexible solution which allows information flow to be initiated from either side. Also this solution is flexible and elegantly handles network devices with multiple IP addresses. The objective of this proposal is to make this interface common for all iscsi offload solutions supported by open-iscsi. We would like to hear comments and suggestions from other iscsi offload vendors in defining this interface. Regards, Anil Veerabhadrappa Proposal to add netlink message type: - 3 new netlink message types are required to support network config message exchange between user and kernel components, 1. ISCSI_UEVENT_SET_NET_CONFIG - push iface info to driver to configure an iscsi network interface 2. ISCSI_UEVENT_GET_NET_CONFIG - get MAC address list, etc' 3. ISCSI_KEVENT_NET_CONFIG - propagate attribute changes from adapter to iscsid (e.g. advertise newly obtained dhcp address) iscsid will use this netlink messages to pass network configuration between user mode application and the driver. Once this message is received by scsi_transport_iscsi module it will call driver's newly added callback handlers in the iscsi_transport structure(net_config get_net_config) and also broadcast netlink message back to any hardware vendor's user level daemons ISCSI_XEVENT_NET_CONFIG message payload format: --- Payload consists of one or more config parameters defined in TLV (Type - Length - Value) format. struct net_cfg_tlv { uint32_t type; uint32_t length; uint8_t value[0]; }; Message types: -- This interface is envisioned to support standard network parameters such as netdev name, MAC address, IPv4/IPv6 address, VLAN, boot protocol (static or dhcp), etc'. Please refer to 'net_cfg_e' in proposed header file changes below. Advantages: --- This approach is much cleaner and delinks network configuration parameters currently bound to host attributes. Old scheme actually breaks the layering scheme as seen below, SCSI (net config parametes currently resides here) | iSCSI | TCP/IP This new approach is a deviation from current host attributes approach and places emphasis in iface components of iscsid and the LLD. Changes to iscsi transport headers: --- diff --git a/include/iscsi_if.h b/include/iscsi_if.h index afa86e8..76eea8c 100644 --- a/include/iscsi_if.h +++ b/include/iscsi_if.h @@ -51,6 +51,8 @@ enum iscsi_uevent_e { ISCSI_UEVENT_SET_HOST_PARAM = UEVENT_BASE + 16, ISCSI_UEVENT_UNBIND_SESSION = UEVENT_BASE + 17, ISCSI_UEVENT_CREATE_BOUND_SESSION = UEVENT_BASE + 18, + ISCSI_UEVENT_SET_NET_CONFIG = UEVENT_BASE + 19, + ISCSI_UEVENT_GET_NET_CONFIG = UEVENT_BASE + 20, /* up events */ ISCSI_KEVENT_RECV_PDU = KEVENT_BASE + 1, @@ -59,6 +61,7 @@ enum iscsi_uevent_e { ISCSI_KEVENT_DESTROY_SESSION= KEVENT_BASE + 4, ISCSI_KEVENT_UNBIND_SESSION = KEVENT_BASE + 5, ISCSI_KEVENT_CREATE_SESSION = KEVENT_BASE + 6, + ISCSI_KEVENT_NET_CONFIG = KEVENT_BASE + 7, }; enum iscsi_tgt_dscvr
RE: [RFC] : network configuration for iscsi hba
On Thu, 2009-02-05 at 17:34 -0800, Karen Xie wrote: For dhcp, I am just thinking of the cases where one physical NIC have multiple IP addresses, maybe one is for iscsi only, one for RDMA, or another for admin/control traffic. They can operate in different subnet RDMA shares ip/mac address with host stack, right? Even in the example provided above the device should support 3 at least separate MAC addresses, so by specifying multiple iface definitions user should be able to get this configuration working. and managed by different DHCP servers. Maybe this is not typical setup? For the set_config, say a NIC does not support IPv6 but received a request to set it. So how should the driver treat any setting it does not support? Ignore or return an error? This should be considered as user error. Users are expected to create iface definitions based on guidelines (supported parameter list) outlined in driver's README file. -Original Message- From: open-iscsi@googlegroups.com [mailto:open-is...@googlegroups.com] On Behalf Of Anil Veerabhadrappa Sent: Thursday, February 05, 2009 5:01 PM To: open-iscsi@googlegroups.com Cc: micha...@cs.wisc.edu; mchri...@redhat.com; Michael Chan; Benjamin Li Subject: RE: [RFC] : network configuration for iscsi hba On Thu, 2009-02-05 at 16:36 -0800, Karen Xie wrote: If it is dhcp, does the dhcp server settings need to be provided? Or it is going to be handled outside of iscsi? dhcp setting should be zero-conf right? It works based on initial broadcast request, DHCPDISCOVER and subsequent unicast exchange between dhcp client and the server. Are you referring to choosing the preferred dhcp server if more than one is present in the network? Also a question about the set_net_config, suppose a list of parameters are send down, if one or more values are not supported, does the operation marked as fail and the user has re-do the iface file (to edit out the unsupported values)? Please provide specific examples for discussion. Any device specific limitation will be handled by the corresponding driver, our objective here is to define a generic interface that works for cxgb3i/qla4xxx/bnx2i/... Thanks, Karen -Original Message- From: open-iscsi@googlegroups.com [mailto:open-is...@googlegroups.com] On Behalf Of Anil Veerabhadrappa Sent: Thursday, February 05, 2009 4:21 PM To: open-iscsi@googlegroups.com Cc: micha...@cs.wisc.edu; mchri...@redhat.com; Michael Chan; Benjamin Li Subject: RE: [RFC] : network configuration for iscsi hba On Thu, 2009-02-05 at 16:00 -0800, Karen Xie wrote: Thanks, What are the possible values of ISCSI_NET_CFG_BOOTPROTO? static and dhcp Karen -Original Message- From: open-iscsi@googlegroups.com [mailto:open-is...@googlegroups.com] On Behalf Of Anil Veerabhadrappa Sent: Thursday, February 05, 2009 3:23 PM To: open-iscsi@googlegroups.com Cc: micha...@cs.wisc.edu; mchri...@redhat.com; Michael Chan; Benjamin Li Subject: RE: [RFC] : network configuration for iscsi hba On Thu, 2009-02-05 at 14:33 -0800, Karen Xie wrote: -Original Message- From: open-iscsi@googlegroups.com [mailto:open-is...@googlegroups.com] On Behalf Of Anil Veerabhadrappa Sent: Thursday, February 05, 2009 11:23 AM To: open-iscsi@googlegroups.com; micha...@cs.wisc.edu Cc: mchri...@redhat.com; mc...@broadcom.com; be...@broadcom.com; ani...@broadcom.com Subject: [RFC] : network configuration for iscsi hba Hi, We'd like to propose a general scheme for configuring network parameters (IP address/mask/DHCP, etc) for iSCSI NICs that use a private IP address. The current scheme of using sysfs is not very good because bootproto, IP address, netmask, VLAN ID, etc all need to be set together. Having separate sysfs entries and updating them separately and independently will not work. Putting everything in a netlink message seems to be a better solution. After weighing different solutions, we feel expanding existing netlink family, NETLINK_ISCSI between iscsid and scsi_transport_iscsi is a flexible solution which allows information flow to be initiated from either side. Also this solution is flexible and elegantly handles network devices with multiple IP addresses. The objective of this proposal is to make this interface common for all iscsi offload solutions supported by open-iscsi. We would like to hear comments and suggestions from other iscsi offload vendors in defining this interface. Regards, Anil Veerabhadrappa Proposal to add netlink message type: - 3 new netlink message types are required to support network config message exchange between user and kernel components, 1. ISCSI_UEVENT_SET_NET_CONFIG - push iface info
Re: [PATCH 3/3] bnx2i: Add bnx2i iSCSI driver.
On Fri, 2008-05-23 at 13:23 -0700, Roland Dreier wrote: Hi Michael, I was reading over the driver to try and figure out how you handle allocating source ports for the offloaded TCP connections you make so that they don't collide with the main network stack. It looks like you have: +/** + * bnx2i_alloc_tcp_port - allocates a tcp port from the free list + * + * Assumes this function is called with 'bnx2i_resc_lock' held. + */ +static u16 bnx2i_alloc_tcp_port(void) that has some failure code: + if (!tcp_port) { + printk(KERN_ERR bnx2i: run 'bnx2id' to alloc tcp ports\n); but I don't know what bnx2id is? 'bnx2id' is the user component in this solution. bnx2id daemon uses socket calls to bind tcp ports in high range and hands them over to driver. This is how iscsi driver tries to solve tcp port collision issue. User daemon communicates with the driver using sysfs and tcp port related functions are bnx2i_read_tcp_portd_*/bnx2i_write_tcp_portd_* (reference: bnx2i_sysfs.c) and I didn't see anywhere that bnx2i_get_tcp_port_requirements() is actually called, and it's not exported? +/** + * bnx2i_get_tcp_port_requirements - returns num tcp ports to alloc/bind + * + * driver returns the number of TCP ports to be allocated/bound by 'bnx2id' + *daemon. Return value of '0' means driver has everything to support + *max iscsi connections on enumerated NX2 devices + */ +int bnx2i_get_tcp_port_requirements(void) Actually this logic was simplified by adding a new member, 'num_required' to 'tcp_port_mgmt' structure. This call is no more required, will remove bnx2i_get_tcp_port_requirements() in the next driver revision --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups open-iscsi group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: [PATCH 3/3] bnx2i: Add bnx2i iSCSI driver.
On Thu, 2008-05-22 at 11:15 -0400, Konrad Rzeszutek wrote: A cursory glance.. .. snip.. +struct bnx2i_cleanup_request { +#if defined(__BIG_ENDIAN) + u8 op_code; + u8 reserved1; + u16 reserved0; +#elif defined(__LITTLE_ENDIAN) + u16 reserved0; + u8 reserved1; + u8 op_code; +#endif + u32 reserved2[3]; +#if defined(__BIG_ENDIAN) + u16 reserved3; + u16 itt; +#define ISCSI_CLEANUP_REQUEST_INDEX (0x3FFF0) +#define ISCSI_CLEANUP_REQUEST_INDEX_SHIFT 0 +#define ISCSI_CLEANUP_REQUEST_TYPE (0x314) +#define ISCSI_CLEANUP_REQUEST_TYPE_SHIFT 14 +#elif defined(__LITTLE_ENDIAN) + u16 itt; +#define ISCSI_CLEANUP_REQUEST_INDEX (0x3FFF0) +#define ISCSI_CLEANUP_REQUEST_INDEX_SHIFT 0 +#define ISCSI_CLEANUP_REQUEST_TYPE (0x314) +#define ISCSI_CLEANUP_REQUEST_TYPE_SHIFT 14 + u16 reserved3; +#endif Why the duplication of the #define values? They look the same. Sure we can take care of that by moving macros outside of #ifdef .. snip .. +/* + * mnc - lookup Jeff Garzack's rule about defining this Has this TODO been completed? + * type of junk. Base on enum and make it less error prone. + * Anil - these are bit masks, won't enum be little ugly? + */ .. snip .. + * bnx2i_iscsi_license_error - displays iscsi license related error message Doesn't look very license related. Just says 'not supported'. + * @hba: adapter instance pointer + * @error_code:error classification + * + * Puts out an error log when driver is unable to offload iscsi connection + * due to license restrictions Maybe adding in some extra information to the error, such as: Due to GPL restrictions, etc.. .. What does 'LOM' stand for? This has nothing to do with code licensing but related to whether iSCSI functionality is enabled on the device or not. iSCSI feature can be controlled through NVRAM configuration and some OEM's do not enable iscsi + */ +static void bnx2i_iscsi_license_error(struct bnx2i_hba *hba, u32 error_code) +{ + if (error_code == ISCSI_KCQE_COMPLETION_STATUS_ISCSI_NOT_SUPPORTED) + /* iSCSI offload not supported on this device */ + printk(KERN_ERR bnx2i: iSCSI not supported, dev=%s\n, + hba-netdev-name); + if (error_code == ISCSI_KCQE_COMPLETION_STATUS_LOM_ISCSI_NOT_ENABLED) + /* iSCSI offload not supported on this LOM device */ + printk(KERN_ERR bnx2i: LOM is not enable to -enabled. + offload iSCSI connections, dev=%s\n, + hba-netdev-name); + set_bit(ADAPTER_STATE_INIT_FAILED, hba-adapter_state); +} + +#ifdef _EVENT_COALESCE_ +#define CNIC_ARM_CQE 1 +#define CNIC_DISARM_CQE0 +extern unsigned int event_coal_div; + +/** + * bnx2i_arm_cq_event_coalescing - arms CQ to enable EQ notification + * @ep:endpoint (transport indentifier) structure + * @action:action, ARM or DISARM. For now only ARM_CQE is used + * + * Arm'ing CQ will enable chip to generate global EQ events inorder to interrupt + * the driver. EQ event is generated CQ index is hit or at least 1 CQ is + * outstanding and on chip timer expires + */ +static void bnx2i_arm_cq_event_coalescing(struct bnx2i_endpoint *ep, u8 action) +{ + u16 cq_index; + if ((action == CNIC_ARM_CQE) ep-sess) { + cq_index = ep-qp.cqe_exp_seq_sn + + ep-sess-num_active_cmds / event_coal_div; + cq_index %= (ep-qp.cqe_size * 2 + 1); + if (!cq_index) + cq_index = 1; + writew(cq_index, ep-qp.ctx_base + CNIC_EVENT_COAL_INDEX); + } + writeb(action, ep-qp.ctx_base + CNIC_EVENT_CQ_ARM); This code looks to be outside the function. Did you try to compile with the _EVENT_COALESCE_ define? function construction is ok, may be an additional blank line after writeb() the reason for confusion .. snip .. +int bnx2i_send_iscsi_tmf(struct bnx2i_conn *bnx2i_conn, +struct iscsi_task *mtask) .. snip + tmfabort_wqe-op_attr = 0; + tmfabort_wqe-op_attr = + ISCSI_TMF_REQUEST_ALWAYS_ONE | ISCSI_TM_FUNC_ABORT_TASK; Is the first assigment neccessary? .. snip .. +static int bnx2i_power_of2(u32 val) There are no macros for this? Did not find one. But there is is_power_of_2() to check if the given integer is an exact power of 2 .. snip .. +static void bnx2i_process_async_mesg(struct iscsi_session *session, +struct bnx2i_conn *bnx2i_conn, +struct cqe *cqe) +{ + struct bnx2i_async_msg *async_cqe; + struct iscsi_async *resp_hdr; + u8 async_event; + + bnx2i_unsol_pdu_adjust_rq(bnx2i_conn); + + async_cqe = (struct bnx2i_async_msg