On Thu, 18 Mar 2010 16:02:52 -0500
Mike Christie micha...@cs.wisc.edu wrote:
On 03/18/2010 08:58 AM, FUJITA Tomonori wrote:
- 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_VENDOR parts that makes it confusing -
it got me too. The ISCSI_BSG_HST_VENDOR code in be2iscsi looks like it
is basically disabled (should remove for a formal patch when he sends
for merging).
It looks like there is a common struct iscsi_bsg_common_format that is
getting passed around, and then in be2iscsi the driver is using that
info to make a be2iscsi specific command. So scsi_transport_iscsi /
ISCSI_SET_IP_ADDR / iscsi_bsg_common_format gets 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)?
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)
diff --git a/drivers/scsi/qla4xxx/ql4_def.h b/drivers/scsi/qla4xxx/ql4_def.h
index 81b5f29..47c3f45 100644
--- a/drivers/scsi/qla4xxx/ql4_def.h
+++ b/drivers/scsi/qla4xxx/ql4_def.h
@@ -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;
diff --git a/drivers/scsi/qla4xxx/ql4_glbl.h b/drivers/scsi/qla4xxx/ql4_glbl.h
index 96ebfb0..b5f3178 100644
--- a/drivers/scsi/qla4xxx/ql4_glbl.h
+++ b/drivers/scsi/qla4xxx/ql4_glbl.h
@@ -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
