RFC: The be2iscsi driver support for bsg
Subsystems (3)
Quoted 20 lines
Quoted 11 lines · On Thu, 18 Mar 2010 16:02:52 -0500 Mike Christie <[email protected]> wrote:
Quoted 3 lines · FUJITA Tomonori
- You invent your hardware specific data structure for the simplest operation such as setting IP address.
I think this is what Jay is not trying to do. I think the patch has some extra code like the
ISCSI_BSG_HST_VENDORparts that makes it confusing - it got me too. TheISCSI_BSG_HST_VENDORcode in be2iscsi looks like it is basically disabled (should remove for a formal patch when he sends for merging).Quoted 5 lines
It looks like there is a common struct
iscsi_bsg_common_formatthat is getting passed around, and then in be2iscsi the driver is using that info to make a be2iscsi specific command. Soscsi_transport_iscsi/ISCSI_SET_IP_ADDR/iscsi_bsg_common_formatgets translated by b2iscsi to b2iscsi /OPCODE_COMMON_ISCSI_NTWK_MODIFY_IP_ADDR/be_modify_ip_addr.
Quoted 7 lines
Yeah, seems you are right. But looks like this patchset also adds the vendor specific message support (
ISCSI_BSG_HST_VENDOR)?I still want to know why vendors can't do this via the existing netlink interface. open-iscsi uses the netlink interface for some pdu so I guess that having a different channel for management might be a good idea.
In the current state *iscsi netlink interface* does not allow the user space apps for iSCSI H/W offload solution (in our case its qla4xxx) to pass down bunch of parameter's in one shot for configuration purposes. As of know one can pass one param at a time using netlink but that does not fit very well for offload solution. Basically on the driver/FW side, it needs to use MBX interface for most of the configuration part which requires payload to be passed back and forth.
Looking into what was done on bsg side for FC looks a better fit and provides the flexibility to add vendor specific stuff nicely and supports dma payloads etc. So I agree with what has been discussed so far, about the need for *iSCSI bsg interface* support for H/W iSCSI offload solution.
Ditto for flash management stuff - using bsg vendor mechanism makes more sense.
Here's an early snapshot of the patch which we are working on to add the support using bsg vendor specific part - btw this is based on the previous iscsi bsg patch and need to be synced up with what was posted recently. So we hope that this gives you an idea why this needed.
(Please discard the *PING changes* for right now)
drivers/scsi/qla4xxx/ql4_def.h +32 -0
@@ -443,6 +443,38 @@ struct scsi_qla_host {struct srb *status_srb; }; +struct qla4xxx_bsg_cmd { +uint32_t cmd; +uint32_t sub_cmd; +uint32_t data_len; +uint32_t instance; +}; + +enum ql4_bsg_vendor_priv_cmd { +QL4_GET_DATA , +QL4_SET_DATA , +QL4_RESTORE_FACTORY_DEF , +QL4_DISABLE_ACB , +}; + +enum ql4_get_data { +QL4_GET_FLASH , +QL4_GET_DDB_DEF , +QL4_GET_DDB , +QL4_GET_IFCB_DEF , +QL4_GET_IFCB , +QL4_GET_ACB , +QL4_GET_ISCSI_STAT , +}; + +enum ql4_set_data { +QL4_SET_FLASH , +QL4_SET_DDB , +QL4_SET_IFCB , +QL4_SET_ACB , +QL4_RST_ISCSI_STAT , +}; +static inline int is_qla4010 (struct scsi_qla_host *ha) {return ha->pdev->device ==PCI_DEVICE_ID_QLOGIC_ISP4010 ;
drivers/scsi/qla4xxx/ql4_glbl.h +18 -0
@@ -34,6 +34,24 @@ int qla4xxx_get_flash(struct scsi_qla_host * ha, dma_addr_t dma_addr,int qla4xxx_get_firmware_status (struct scsi_qla_host * ha);int qla4xxx_get_firmware_state (struct scsi_qla_host * ha);int qla4xxx_initialize_fw_cb (struct scsi_qla_host * ha); +int qla4xxx_get_default_ddb (struct scsi_qla_host *ha, +dma_addr_t dma_addr); +int qla4xxx_set_flash (struct scsi_qla_host *ha,dma_addr_t dma_addr, +uint32_t offset,uint32_t option,uint32_t len); +int qla4xxx_get_acb (struct scsi_qla_host *ha,struct qla4xxx_bsg_cmd vendor_cmd, +dma_addr_t data_dma); +int qla4xxx_bsg_get_ifcb (struct scsi_qla_host *ha,uint32_t sub_cmd, +dma_addr_t data_dma); +int qla4xxx_set_ifcb (struct scsi_qla_host *ha,dma_addr_t data_dma); +int qla4xxx_set_acb (struct scsi_qla_host *ha,uint32_t instance,uint32_t acb_len, +dma_addr_t data_dma); +int qla4xxx_restore_factory_defaults (struct scsi_qla_host *ha, +struct iscsi_bsg_job *job); +int qla4xxx_ping (struct iscsi_bsg_job *job); +int qla4xxx_rst_iscsi_stat (struct scsi_qla_host *ha,uint32_t device_index); +int qla4xxx_get_iscsi_stat (struct scsi_qla_host *ha,uint32_t device_index, +dma_addr_t data_dma); +int qla4xxx_disable_acb (struct scsi_qla_host *ha,uint32_t acb_instance);/* FIXME: Goodness! this really wants a small struct to hold the * parameters. On x86 the args will get passed on the stack! */
drivers/scsi/qla4xxx/ql4_isr.c +1 -0
@@ -529,6 +529,7 @@ static void qla4xxx_isr_decode_mailbox(struct scsi_qla_host * ha,/* No action */ DEBUG2 (printk ("scsi%ld: AEN %04x\n" , ha->host_no, mbox_status)); +clear_bit (AF_DISABLE_ACB_COMPLETE , &ha->flags);break ;case MBOX_ASTS_IP_ADDR_STATE_CHANGED :
drivers/scsi/qla4xxx/ql4_mbx.c +308 -1
324 lines not on this page · raw message
drivers/scsi/qla4xxx/ql4_os.c +206 -0
232 lines not on this page · raw message
drivers/scsi/scsi_transport_iscsi.c +7 -4
@@ -403,9 +403,9 @@ iscsi_bsg_map_buffer(struct iscsi_bsg_buffer *buf, struct request *req)/** * iscsi_req_to_bsgjob - Allocate/create the iscsi_bsg_job structure for the -* bsg request -* @req: BSG request that needs a job structure -* @shost: SCSI Host corresponding to the bsg object +* bsg request +* @req: BSG request that needs a job structure +* @shost: SCSI Host corresponding to the bsg object */ static int iscsi_req_to_bsgjob (struct request *req,struct Scsi_Host *shost)
@@ -428,7 +428,7 @@ iscsi_req_to_bsgjob(struct request *req, struct Scsi_Host *shost)* then gets reformatted as a blk request, which then gets * reformatted as a iscsi bsg request. And on completion, we have * to wrap return results such that SGIO v4 thinks it was a scsi -* status. I hope this was all worth it. +* status. I hope this was all worth it. */ req->special = job;
@@ -500,6 +500,9 @@ iscsi_bsg_host_dispatch(struct request_queue *q, struct Scsi_Host *shost, goto fail_host_msg; }break ; +case ISCSI_BSG_HST_PING : + cmdlen +=sizeof (struct iscsi_bsg_host_ping ); +break ;default : ret = -EBADR ;
Thanks
Ravi
Quoted 6 lines
--
You received this message because you are subscribed to the Google Groups "open-iscsi" group. To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/open-iscsi?hl=en.
Quoted 23 lines · Ravi Anand
Quoted 23 lines
Quoted 11 lines · On Thu, 18 Mar 2010 16:02:52 -0500 Mike Christie<[email protected]> wrote:
Quoted 3 lines · FUJITA Tomonori
- You invent your hardware specific data structure for the simplest operation such as setting IP address.
I think this is what Jay is not trying to do. I think the patch has some extra code like the
ISCSI_BSG_HST_VENDORparts that makes it confusing - it got me too. TheISCSI_BSG_HST_VENDORcode in be2iscsi looks like it is basically disabled (should remove for a formal patch when he sends for merging).Quoted 5 lines
It looks like there is a common struct
iscsi_bsg_common_formatthat is getting passed around, and then in be2iscsi the driver is using that info to make a be2iscsi specific command. Soscsi_transport_iscsi/ISCSI_SET_IP_ADDR/iscsi_bsg_common_formatgets translated by b2iscsi to b2iscsi /OPCODE_COMMON_ISCSI_NTWK_MODIFY_IP_ADDR/be_modify_ip_addr.Yeah, seems you are right. But looks like this patchset also adds the vendor specific message support (
ISCSI_BSG_HST_VENDOR)?
.......
Quoted 3 lines
Here's an early snapshot of the patch which we are working on to add the support using bsg vendor specific part - btw this is based on the previous iscsi bsg patch and need to be synced up with what was posted recently.
Just to make sure we are all on the same page. Tomo's comments above means that for your comment about updating to the new code does not mean that you should just use the new HST_VENDOR cmd, and it does not mean that we only should use a common struct/cmd for net operations that Jay handled.
For other common operations we should do like Tomo suggested, and Jay started with the network stuff, and make common operations. You guys should also not feel like you have to use the format Jay posted with. We can modify it so it make sense for everyone.
Quoted 11 lines
drivers/scsi/qla4xxx/ql4_os.c +11
-0+switch (msgcode) { +case ISCSI_BSG_HST_VENDOR : + rval =qla4xxx_process_vendor_specific (job); +break ; +case ISCSI_BSG_HST_PING : + rval =qla4xxx_ping (job); +break ; +default : +break ; + } +