RFC: The be2iscsi driver support for bsg
Subsystems (3)
PATCH 2/2 RFC: The be2iscsi driver support for bsg
This patch contains the necessary changes to support
the bsg interface
--- drivers/scsi/be2iscsi/be_cmds.h | 137 ++++++++++++++++++++--- drivers/scsi/be2iscsi/be_iscsi.c | 3 +- drivers/scsi/be2iscsi/be_main.c | 99 ++++++++++++++-- drivers/scsi/be2iscsi/be_main.h | 6 +- drivers/scsi/be2iscsi/be_mgmt.c | 230 +++++++++++++++++++++++++++++++++++++- drivers/scsi/be2iscsi/be_mgmt.h | 107 ++++++++++++++++++ include/scsi/scsi_bsg_iscsi.h | 1 + 7 files changed, 547 insertions(+), 36 deletions(-)
drivers/scsi/be2iscsi/be_cmds.h +120 -17
@@ -18,6 +18,7 @@#ifndef BEISCSI_CMDS_H #define BEISCSI_CMDS_H +#include <scsi/scsi_bsg_iscsi.h> /** * The driver sends configuration and managements command requests to the * firmware in the BE. These requests are communicated to the processor
@@ -162,6 +163,13 @@ struct be_mcc_mailbox {#define OPCODE_COMMON_ISCSI_CFG_POST_SGL_PAGES 2#define OPCODE_COMMON_ISCSI_CFG_REMOVE_SGL_PAGES 3#define OPCODE_COMMON_ISCSI_NTWK_GET_NIC_CONFIG 7 +#define OPCODE_COMMON_ISCSI_NTWK_SET_VLAN 14 +#define OPCODE_COMMON_ISCSI_NTWK_CONFIGURE_STATELESS_IP_ADDR 17 +#define OPCODE_COMMON_ISCSI_NTWK_MODIFY_IP_ADDR 21 +#define OPCODE_COMMON_ISCSI_NTWK_GET_DEFAULT_GATEWAY 22 +#define OPCODE_COMMON_ISCSI_NTWK_MODIFY_DEFAULT_GATEWAY 23 +#define OPCODE_COMMON_ISCSI_NTWK_GET_ALL_IF_ID 24 +#define OPCODE_COMMON_ISCSI_NTWK_GET_IF_INFO 25#define OPCODE_COMMON_ISCSI_SET_FRAGNUM_BITS_FOR_SGL_CRA 61#define OPCODE_COMMON_ISCSI_DEFQ_CREATE 64#define OPCODE_COMMON_ISCSI_DEFQ_DESTROY 65
@@ -237,6 +245,86 @@ struct be_cmd_resp_eq_create {u16 rsvd0;/* sword */ } __packed; +struct mgmt_chap_format { +u32 flags; +u8 intr_chap_name[256 ]; +u8 intr_secret[16 ]; +u8 target_chap_name[256 ]; +u8 target_secret[16 ]; +u16 intr_chap_name_length; +u16 intr_secret_length; +u16 target_chap_name_length; +u16 target_secret_length; +} __packed; + +struct mgmt_auth_method_format { +u8 auth_method_type; +u8 padding[3 ]; +struct mgmt_chap_format chap; +} __packed; + +struct mgmt_conn_login_options { +u8 flags; +u8 header_digest; +u8 data_digest; +u8 rsvd0; +u32 max_recv_datasegment_len_ini; +u32 max_recv_datasegment_len_tgt; +u32 tcp_mss; +u32 tcp_window_size; +struct mgmt_auth_method_format auth_data; +} __packed; + +struct mgmt_conn_info { +u32 connection_handle; +u32 connection_status; +u16 src_port; +u16 dest_port; +u16 dest_port_redirected; +u16 cid; +u32 estimated_throughput; +struct ip_address_format src_ipaddr; +struct ip_address_format dest_ipaddr; +struct ip_address_format dest_ipaddr_redirected; +struct mgmt_conn_login_options negotiated_login_options; +} __packed; + +struct mgmt_session_login_options { +u8 flags; +u8 error_recovery_level; +u16 rsvd0; +u32 first_burst_length; +u32 max_burst_length; +u16 max_connections; +u16 max_outstanding_r2t; +u16 default_time2wait; +u16 default_time2retain; +} __packed; + +struct mgmt_session_info { +u32 session_handle; +u32 status; +u8 isid[6 ]; +u16 tsih; +u32 session_flags; +u16 conn_count; +u16 pad; +u8 target_name[224 ]; +u8 initiator_iscsiname[224 ]; +struct mgmt_session_login_options negotiated_login_options; +struct mgmt_conn_info conn_list[1 ]; +} __packed; + +struct be_cmd_req_geta_session { +struct be_cmd_resp_hdr resp_hdr; +u32 session_handle; +} __packed; + +struct be_cmd_resp_geta_session { +struct be_cmd_resp_hdr resp_hdr; +struct mgmt_session_info session_info; +} __packed; +struct mac_addr {u16 size_of_struct;u8 addr[ETH_ALEN ];
@@ -254,6 +342,16 @@ struct be_cmd_resp_mac_query {struct mac_addr mac; }; +struct be_cmd_req_get_boot_target { +struct be_cmd_req_hdr hdr; +} __packed; + +struct be_cmd_resp_get_boot_target { +struct be_cmd_resp_hdr hdr; +u32 boot_session_count; +u32 boot_session_handle; +}; +/******************** Create CQ ***************************/ /** * Pseudo amap definition in which each bit of the actual structure is defined
@@ -601,14 +699,6 @@ struct be_eq_delay_params_in {struct eq_delay delay[8 ]; } __packed; -struct ip_address_format { -u16 size_of_structure; -u8 reserved; -u8 ip_type; -u8 ip_address[16 ]; -u32 rsvd0; -} __packed; -struct tcp_connect_and_offload_in {struct be_cmd_req_hdr hdr;struct ip_address_format ip_address;
@@ -688,18 +778,31 @@ struct be_fw_cfg {u32 function_caps; } __packed; -#define CMD_ISCSI_COMMAND_INVALIDATE 1 -#define ISCSI_OPCODE_SCSI_DATA_OUT 5 +struct be_all_if_id { +struct be_cmd_req_hdr hdr; +u32 if_count; +u32 if_hndl_list[1 ]; +} __packed; + + +#define ISCSI_OPCODE_SCSI_DATA_OUT 5#define OPCODE_COMMON_ISCSI_TCP_CONNECT_AND_OFFLOAD 70 -#define OPCODE_ISCSI_INI_DRIVER_OFFLOAD_SESSION 41 -#define OPCODE_COMMON_MODIFY_EQ_DELAY 41 -#define OPCODE_COMMON_ISCSI_CLEANUP 59 -#define OPCODE_COMMON_TCP_UPLOAD 56 +#define OPCODE_COMMON_MODIFY_EQ_DELAY 41 +#define OPCODE_COMMON_ISCSI_CLEANUP 59 +#define OPCODE_COMMON_TCP_UPLOAD 56#define OPCODE_COMMON_ISCSI_ERROR_RECOVERY_INVALIDATE_COMMANDS 1 -/* --- CMD_ISCSI_INVALIDATE_CONNECTION_TYPE --- */ -#define CMD_ISCSI_CONNECTION_INVALIDATE 0x8001 -#define CMD_ISCSI_CONNECTION_ISSUE_TCP_RST 0x8002 +#define OPCODE_ISCSI_INI_CFG_GET_HBA_NAME 6 +#define OPCODE_ISCSI_INI_CFG_SET_HBA_NAME 7 +#define OPCODE_ISCSI_INI_SESSION_GET_A_SESSION 14 +#define OPCODE_ISCSI_INI_DRIVER_OFFLOAD_SESSION 41#define OPCODE_ISCSI_INI_DRIVER_INVALIDATE_CONNECTION 42 +#define OPCODE_ISCSI_INI_BOOT_GET_BOOT_TARGET 52 + +/* --- CMD_ISCSI_INVALIDATE_CONNECTION_TYPE --- */ +#define CMD_ISCSI_COMMAND_INVALIDATE 1 +#define CMD_ISCSI_CONNECTION_INVALIDATE 0x8001 +#define CMD_ISCSI_CONNECTION_ISSUE_TCP_RST 0x8002 +#define INI_WR_CMD 1/* Initiator write command */ #define INI_TMF_CMD 2/* Initiator TMF command */
drivers/scsi/be2iscsi/be_iscsi.c +1 -2
@@ -696,8 +696,7 @@ void beiscsi_conn_stop(struct iscsi_cls_conn *cls_conn, int flag)SE_DEBUG (DBG_LVL_8 ,"In beiscsi_conn_stop ep_cid = %d\n" , beiscsi_ep->ep_cid); tag =mgmt_invalidate_connection (phba, beiscsi_ep, - beiscsi_ep->ep_cid,1 , - savecfg_flag); + beiscsi_ep->ep_cid,0 , savecfg_flag);if (!tag) { SE_DEBUG(DBG_LVL_1, "mgmt_invalidate_connection Failed for cid=%d \n",
drivers/scsi/be2iscsi/be_main.c +87 -12
201 lines not on this page · raw message
drivers/scsi/be2iscsi/be_main.h +4 -2
@@ -62,7 +62,7 @@#define BEISCSI_SGLIST_ELEMENTS 30#define BEISCSI_CMD_PER_LUN 128/* scsi_host->cmd_per_lun */ -#define BEISCSI_MAX_SECTORS 256/* scsi_host->max_sectors */ +#define BEISCSI_MAX_SECTORS 2048/* scsi_host->max_sectors */ #define BEISCSI_MAX_CMD_LEN 16/* scsi_host->max_cmd_len */ #define BEISCSI_NUM_MAX_LUN 256/* scsi_host->max_lun */
@@ -77,7 +77,7 @@#define MAX_CMD_SZ 65536#define IIOC_SCSI_DATA 0x05/* Write Operation */ -#define DBG_LVL 0x00000001 +#define DBG_LVL 0x000000FF#define DBG_LVL_1 0x00000001#define DBG_LVL_2 0x00000002#define DBG_LVL_3 0x00000004
@@ -334,6 +334,7 @@ struct beiscsi_hba {struct work_struct work_cqs;/* The work being queued */ struct be_ctrl_info ctrl;unsigned int generation; +unsigned int interface_handle;struct invalidate_command_table inv_tbl[128 ]; };
@@ -353,6 +354,7 @@ struct beiscsi_conn {struct beiscsi_endpoint *ep;unsigned short login_in_progress;struct sgl_handle *plogin_sgl_handle; +struct wrb_handle *plogin_wrb_handle;struct beiscsi_session *beiscsi_sess;struct iscsi_task *task; };
drivers/scsi/be2iscsi/be_mgmt.c +227 -3
285 lines not on this page · raw message
drivers/scsi/be2iscsi/be_mgmt.h +107 -0
123 lines not on this page · raw message
include/scsi/scsi_bsg_iscsi.h +1 -0
@@ -56,6 +56,7 @@#define ISCSI_GET_HBA_NAME 6#define ISCSI_GET_IF_INFO 7#define ISCSI_GET_DEFAULT_GATEWAY 8 +#define ISCSI_GET_MAC_ADDRESS 9/* * iSCSI Host Messages
Signature · 1 line
1.6.4
Quoted 27 lines · On Wed, 17 Mar 2010 23:37:07 +0530 Jayamohan Kallickal…
This patch contains the necessary changes to support
the bsg interfaceSigned-off-by Jayamohan Kallickal <[email protected]>--- drivers/scsi/be2iscsi/be_cmds.h | 137 ++++++++++++++++++++--- drivers/scsi/be2iscsi/be_iscsi.c | 3 +- drivers/scsi/be2iscsi/be_main.c | 99 ++++++++++++++-- drivers/scsi/be2iscsi/be_main.h | 6 +- drivers/scsi/be2iscsi/be_mgmt.c | 230 +++++++++++++++++++++++++++++++++++++- drivers/scsi/be2iscsi/be_mgmt.h | 107 ++++++++++++++++++ include/scsi/scsi_bsg_iscsi.h | 1 + 7 files changed, 547 insertions(+), 36 deletions(-)drivers/scsi/be2iscsi/be_cmds.h +1
-0@@ -18,6 +18,7 @@#ifndef BEISCSI_CMDS_H #define BEISCSI_CMDS_H +#include <scsi/scsi_bsg_iscsi.h> /** * The driver sends configuration and managements command requests to the * firmware in the BE. These requests are communicated to the processor
drivers/scsi/be2iscsi/be_cmds.h struct be_mcc_mailbox { · line 172
Quoted 10 lines
drivers/scsi/be2iscsi/be_cmds.h +7
-0@@ -162,6 +163,13 @@ struct be_mcc_mailbox {#define OPCODE_COMMON_ISCSI_CFG_POST_SGL_PAGES 2#define OPCODE_COMMON_ISCSI_CFG_REMOVE_SGL_PAGES 3#define OPCODE_COMMON_ISCSI_NTWK_GET_NIC_CONFIG 7 +#define OPCODE_COMMON_ISCSI_NTWK_SET_VLAN 14 +#define OPCODE_COMMON_ISCSI_NTWK_CONFIGURE_STATELESS_IP_ADDR 17 +#define OPCODE_COMMON_ISCSI_NTWK_MODIFY_IP_ADDR 21 +#define OPCODE_COMMON_ISCSI_NTWK_GET_DEFAULT_GATEWAY 22 +#define OPCODE_COMMON_ISCSI_NTWK_MODIFY_DEFAULT_GATEWAY 23 +#define OPCODE_COMMON_ISCSI_NTWK_GET_ALL_IF_ID 24 +#define OPCODE_COMMON_ISCSI_NTWK_GET_IF_INFO 25
So this patchset adds the user-kernel space interface for management via bsg, right?
Then I have two questions.
- open-iscsi already has the user-kernel space interface for management via netlink. Why do you need another via bsg? IOW, why can't you do this via the existing netlink interface?
- You invent your hardware specific data structure for the simplest operation such as setting IP address. It means that every vendor invents their hardware specific data structure for management. Users have to use the vendor specific management software or open-iscsi management tool (iscsiadm) needs all the vendor specific code to build the vendor specific data structure (and parse the response). I think that it is exactly what open-iscsi tries to avoid. iscsiadm builds the generic management data structure and scsi_transport_iscsi.c passes it to the drivers. Then the drivers have to handle it. That's the way open-iscsi works now.
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_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.
So the approach using a common struct/cmd is ok with me, and if we can get a struct format works for everyone (qla4xxx, be2iscsi, cxgb4i) then I am fine.
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)?
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.
A message that is not in the archive (sent off-list or not yet received): 20100319081016W.fujita.tomonori-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org
Quoted 10 lines · FUJITA Tomonori
Quoted 6 lines · On Thu, 18 Mar 2010 16:02:52 -0500 Mike Christie…
Quoted 2 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).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.
Separate this request into two needs:
The first need is for the iscsi driver to have some kind of entry point to kick off a vendor specific thing - primarily diagnostics and internal f/w and flash mgmt items. Here, using the same mechanism that we had on the FC side, which also supports dma payloads, made a lot of sense. I like and prefer the symmetry.
The second need is for "common iscsi link/stack mgmt". All vendors would be expected to implement the same items the same way - thus the formalization of the api in the transport. It also makes sense that all use of these common interfaces comes via the open-iscsi mgmt utilities. Given the data set, it could be done by netlink or bsg. I gave some pros/cons on the interfaces in (http://marc.info/?l=linux-scsi&m=124811693510903&w=2). In my mind, the main reason these settings ended up in bsg vs netlink is - the functionality is typically migrating from a vendor-specific ioctl set, which maps rather easily to the bsg model. Not that netlink is that more difficult (although to NLA_ or not may confuse some of the contributors). And, if you already had the bsg infrastructure for the first need, you had to add very little to support it.
Thus, the main reason they are together is one of expediency. The first had to be done, so it was very easy to use the same methodology for the second.
-- james s
Signature · 4 lines
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-/JYPxA39Uh5TLH3MbocFF+G/[email protected] To unsubscribe from this group, send email to open-iscsi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected] For more options, visit this group at http://groups.google.com/group/open-iscsi?hl=en.
Quoted 25 lines · On Fri, 19 Mar 2010 08:56:30 -0400 James Smart <[email protected]> wrote:
Quoted 4 lines
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.
Separate this request into two needs:
The first need is for the iscsi driver to have some kind of entry point to kick off a vendor specific thing - primarily diagnostics and internal f/w and flash mgmt items. Here, using the same mechanism that we had on the FC side, which also supports dma payloads, made a lot of sense. I like and prefer the symmetry.
The second need is for "common iscsi link/stack mgmt". All vendors would be expected to implement the same items the same way - thus the formalization of the api in the transport. It also makes sense that all use of these common interfaces comes via the open-iscsi mgmt utilities. Given the data set, it could be done by netlink or bsg. I gave some pros/cons on the interfaces in
(http://marc.info/?l=linux-scsi&m=124811693510903&w=2). In my mind, the main reason these settings ended up in bsg vs netlink is - the functionality is typically migrating from a vendor-specific ioctl set, which maps rather easily to the bsg model. Not that netlink is that more difficult (although toNLA_or not may confuse some of the contributors). And, if you already had the bsg infrastructure for the first need, you had to add very little to support it.
Quoted 2 lines
Thus, the main reason they are together is one of expediency. The first had to be done, so it was very easy to use the same methodology for the second.
If vendors use the common data structures via bsg, it's totally fine by me. I see why bsg is preferable. The only thing that I care about is managing any iSCSI HBA with iscsiadm instead of various vendor specific utilities.
About the implementation, I think that it's better to have the common library code rather than just copying the fs bsg code into iscsi.
A message that is not in the archive (sent off-list or not yet received): 20100322175704Y.fujita.tomonori-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org
Quoted 4 lines · FUJITA Tomonori
If vendors use the common data structures via bsg, it's totally fine by me. I see why bsg is preferable. The only thing that I care about is managing any iSCSI HBA with iscsiadm instead of various vendor specific utilities.
agreed
Quoted 2 lines
About the implementation, I think that it's better to have the common library code rather than just copying the fs bsg code into iscsi.
Note: I tried to library-ize the transport implementation on the first pass of the RFC. But it was making things more complex. I tried to explain this, perhaps not very well (http://marc.info/?l=linux-scsi&m=125725904205688&w=2).
-- james s
Signature · 4 lines
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-/JYPxA39Uh5TLH3MbocFF+G/[email protected] To unsubscribe from this group, send email to open-iscsi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected] For more options, visit this group at http://groups.google.com/group/open-iscsi?hl=en.
Quoted 6 lines · On Mon, 22 Mar 2010 11:16:31 -0400 James Smart <[email protected]> wrote:
Quoted 2 lines
About the implementation, I think that it's better to have the common library code rather than just copying the fs bsg code into iscsi.
Note: I tried to library-ize the transport implementation on the first pass of the RFC. But it was making things more complex. I tried to explain this, perhaps not very well (http://marc.info/?l=linux-scsi&m=125725904205688&w=2).
Ah, I overlooked it. I'll think about it later. Maybe Mike already has some ideas about it. Anyway, library-izing is not urgent at all. We can work on it later.
Quoted 14 lines · FUJITA Tomonori
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 8 lines
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)?
Yeah, you are right. That should go in a separate patch and sent later.