-----Original Message----- From: Mike Christie <[email protected]> Date: Monday 28 January 2013 4:47 PM To: "[email protected]" <[email protected]> Cc: Vikas <[email protected]>, Lalit Chandivade <[email protected]>, Ravi Anand <[email protected]>, Poornima Vonti <[email protected]>, Manish Rangankar <[email protected]>, Adheer Chandravanshi <[email protected]> Subject: Re: [RFC_V4 PATCH 1/2] scsi_transport_iscsi: Add flash target mgmt support
>On 01/11/2013 05:50 AM, [email protected] wrote: >> From: Adheer Chandravanshi <[email protected]> >> >> This patch allows iscsiadm to manage iSCSI target information stored on >> adapter flash on per host basis. >> >> The sysfs entries will look as cited below: >> /sys/class/iscsi_flash_tgt/tgt-<host_no>-<target_no>/ >> >> Signed-off-by: Adheer Chandravanshi <[email protected]> >> Signed-off-by: Vikas Chaudhary <[email protected]> >> --- >> drivers/scsi/scsi_transport_iscsi.c | 690 >>++++++++++++++++++++++++++++++++++- >> include/scsi/iscsi_if.h | 106 ++++++ >> include/scsi/scsi_transport_iscsi.h | 35 ++ >> 3 files changed, 830 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/scsi/scsi_transport_iscsi.c >>b/drivers/scsi/scsi_transport_iscsi.c >> index dac7f8d..028f2c3 100644 >> --- a/drivers/scsi/scsi_transport_iscsi.c >> +++ b/drivers/scsi/scsi_transport_iscsi.c >> @@ -25,6 +25,7 @@ >> #include <linux/slab.h> >> #include <linux/bsg-lib.h> >> #include <linux/idr.h> >> +#include <linux/list.h> >> #include <net/tcp.h> >> #include <scsi/scsi.h> >> #include <scsi/scsi_host.h> >> @@ -460,6 +461,431 @@ void iscsi_destroy_iface(struct iscsi_iface >>*iface) >> EXPORT_SYMBOL_GPL(iscsi_destroy_iface); >> >> /* >> + * Interface to display flash tgt params to sysfs >> + */ >> + >> +static void iscsi_flash_tgt_release(struct device *dev) >> +{ >> + struct iscsi_flash_tgt *tgt = iscsi_dev_to_flash_tgt(dev); >> + struct device *parent = tgt->dev.parent; >> + >> + kfree(tgt); >> + put_device(parent); >> +} >> + >> + >> +static struct class iscsi_flash_tgt_class = { >> + .name = "iscsi_flash_tgt", >> + .dev_release = iscsi_flash_tgt_release, >> +}; >> + > > >I guess we are supposed to be using buses now. See the recent fcoe >discussions and the code that got merged. Ok. We will submit next RFC patch using buses. > > >> +#define ISCSI_FLASHTGT_ATTR(_prefix, _name, _mode, _show, _store) \ >> +struct device_attribute dev_attr_##_prefix##_##_name = >> \ >> + __ATTR(_name, _mode, _show, _store) >> + >> +/* flash tgt attrs show */ >> +#define iscsi_flash_tgt_attr_show(type, name, param) >> \ >> +static ssize_t >> \ >> +show_##type##_##name(struct device *dev, struct device_attribute >>*attr, \ >> + char *buf) \ >> +{ \ >> + struct iscsi_flash_tgt *tgt = iscsi_dev_to_flash_tgt(dev); \ >> + struct iscsi_transport *t = tgt->transport; \ >> + return t->get_flash_tgt_param(tgt, param, buf); \ >> +} \ >> + >> + >> +#define iscsi_flash_tgt_attr(type, name, param) >> \ >> + iscsi_flash_tgt_attr_show(type, name, param) \ >> +static ISCSI_FLASHTGT_ATTR(type, name, S_IRUGO, >> \ >> + show_##type##_##name, NULL); >> + >> +/* Target attributes */ >> + >> +iscsi_flash_tgt_attr(tgt, is_fw_assigned_ipv6, >> + ISCSI_FLASHTGT_IS_FW_ASSIGNED_IPV6); > > >Could you give me more info on this one? I saw on the follow on pathc it >mentions link local ipv6 addrs. Can the card have multiple link local >ipv6 addrs some are assigned my fw and some are assigned by the driver? No, only single link local IPv6 address is supported at a time. This flag indicate if the address is assigned by FW or driver. > > >> +iscsi_flash_tgt_attr(tgt, dev_type, ISCSI_FLASHTGT_DEV_TYPE); > > >Could you rename to portal addr type or something like that so it is >clear it is for the portal address. Yes. > > >> +iscsi_flash_tgt_attr(tgt, dif, ISCSI_FLASHTGT_DIF_EN); >> +iscsi_flash_tgt_attr(tgt, auto_snd_tgt_disable, >> + ISCSI_FLASHTGT_AUTO_SND_TGT_DISABLE); >> +iscsi_flash_tgt_attr(tgt, discovery_session, >>ISCSI_FLASHTGT_DISCOVERY_SESS); > >This indicates if it is discovery session vs normal session right? Could >we rename to session_type? Yes. > > >> +iscsi_flash_tgt_attr(tgt, entry_enable, ISCSI_FLASHTGT_ENTRY_EN); >> +iscsi_flash_tgt_attr(tgt, dev_assoc_target, >>ISCSI_FLASHTGT_DEV_ASSOC_TARGET); >> +iscsi_flash_tgt_attr(tgt, dev_assoc_initiator, >> + ISCSI_FLASHTGT_DEV_ASSOC_INITIATOR); > >I did not get this. In the iscsi spec can we discover initiators like >how with FC we can see initiator and target ports? Is this for iSNS? >Does it work such that iSNS in fw, will discovery initaitor and target >nodes and they can be exported in this flash tgt mgmt interface? > >Can we rename to node_type? > >Also then, do we want to rename all of this to flash_node/FLASHNODE >instead of flash_tgt? Currently our firmware support only DEC_ASSOC_TARGET, we can remove both fields and by default this would be target entry > >> +iscsi_flash_tgt_attr(tgt, exec_throttle, ISCSI_FLASHTGT_EXEC_THROTTLE); > >Is this scsi commands or all types of iscsi pdus? So scsi commands with >say something like nops? > >If it is the limit of all iscsi and scsi commands combined, can we >rename to cmds_max so it matches the libiscsi/iscsid/iscsiadm name? > >If it is the limit of just the scsi commands, can we rename to can_queue >so it matches the scsi layer setting name. > This indicate how many scsi commands can be outstanding at any time. We can rename to cmds_max. > >> +iscsi_flash_tgt_attr(tgt, exec_count, ISCSI_FLASHTGT_EXEC_COUNT); > >I am not sure I understand this one. Is it the number of commands that >are currently running when you read the sysfs file? > >I see we write the value to the fw, but I do not see we ever update it >in qla4xxx. This will not give the current count, however just wahtever was saved in the flash when the target entry was written to the flash. We can remove this. > >> +iscsi_flash_tgt_attr(tgt, retry_count, ISCSI_FLASHTGT_RETRY_COUNT); > >Is this for scsi command retry type of things or for session relogin >type of events or both or neither? Same for delay. This was for legacy product and we are removing support for this. We can remove this parameter. > >> +iscsi_flash_tgt_attr(tgt, retry_delay, ISCSI_FLASHTGT_RETRY_DELAY); >> +iscsi_flash_tgt_attr(tgt, header_digest, ISCSI_FLASHTGT_HDR_DGST_EN); >> +iscsi_flash_tgt_attr(tgt, data_digest, ISCSI_FLASHTGT_DATA_DGST_EN); >> +iscsi_flash_tgt_attr(tgt, immediate_data, ISCSI_FLASHTGT_IMM_DATA_EN); >> +iscsi_flash_tgt_attr(tgt, initial_r2t, ISCSI_FLASHTGT_INITIAL_R2T_EN); >> +iscsi_flash_tgt_attr(tgt, data_seq_in_order, >>ISCSI_FLASHTGT_DATASEQ_INORDER); >> +iscsi_flash_tgt_attr(tgt, data_pdu_in_order, >>ISCSI_FLASHTGT_PDU_INORDER); >> +iscsi_flash_tgt_attr(tgt, chap_auth, ISCSI_FLASHTGT_CHAP_AUTH_EN); >> +iscsi_flash_tgt_attr(tgt, snack_req, ISCSI_FLASHTGT_SNACK_REQ_EN); >> +iscsi_flash_tgt_attr(tgt, discovery_logout, >> + ISCSI_FLASHTGT_DISCOVERY_LOGOUT_EN); > >If this is not set what happens? Do you guys keep the session logged in? >Is this like the cisco long lived discovery session feature in the >linux-iscsi driver? > >Why do you keep it open or why do you want to? > >It's fine. I am just wondering. > If not set, there will not be explicit logout. Just a tcp connection teardown. > >> +iscsi_flash_tgt_attr(tgt, bidi_chap, ISCSI_FLASHTGT_BIDI_CHAP_EN); >> +iscsi_flash_tgt_attr(tgt, discovery_auth_optional, >> + ISCSI_FLASHTGT_DISCOVERY_AUTH_OPTIONAL); >> +iscsi_flash_tgt_attr(tgt, erl, ISCSI_FLASHTGT_ERL); >> +iscsi_flash_tgt_attr(tgt, tcp_timestamp_stat, >> + ISCSI_FLASHTGT_TCP_TIMESTAMP_STAT); >> +iscsi_flash_tgt_attr(tgt, tcp_nagle_disable, >>ISCSI_FLASHTGT_TCP_NAGLE_DISABLE); >> +iscsi_flash_tgt_attr(tgt, tcp_wsf_disable, >>ISCSI_FLASHTGT_TCP_WSF_DISABLE); >> +iscsi_flash_tgt_attr(tgt, tcp_timer_scale, >>ISCSI_FLASHTGT_TCP_TIMER_SCALE); >> +iscsi_flash_tgt_attr(tgt, tcp_timestamp_enable, >> + ISCSI_FLASHTGT_TCP_TIMESTAMP_EN); >> +iscsi_flash_tgt_attr(tgt, ip_frag_disable, >>ISCSI_FLASHTGT_IP_FRAG_DISABLE); >> +iscsi_flash_tgt_attr(tgt, max_recv_dlength, >>ISCSI_FLASHTGT_MAX_RECV_DLENGTH); >> +iscsi_flash_tgt_attr(tgt, max_xmit_dlength, >>ISCSI_FLASHTGT_MAX_XMIT_DLENGTH); >> +iscsi_flash_tgt_attr(tgt, first_burst, ISCSI_FLASHTGT_FIRST_BURST); >> +iscsi_flash_tgt_attr(tgt, default_time2wait, >>ISCSI_FLASHTGT_DEF_TIME2WAIT); >> +iscsi_flash_tgt_attr(tgt, default_time2retain, >>ISCSI_FLASHTGT_DEF_TIME2RETAIN); >> +iscsi_flash_tgt_attr(tgt, max_r2t, ISCSI_FLASHTGT_MAX_R2T); >> +iscsi_flash_tgt_attr(tgt, keepalive_timeout, >>ISCSI_FLASHTGT_KEEPALIVE_TMO); >> +iscsi_flash_tgt_attr(tgt, isid, ISCSI_FLASHTGT_ISID); > >What format does this get printed out as? We will fix this to show in Network Byte order > >> +iscsi_flash_tgt_attr(tgt, tsid, ISCSI_FLASHTGT_TSID); >> +iscsi_flash_tgt_attr(tgt, port, ISCSI_FLASHTGT_PORT); >> +iscsi_flash_tgt_attr(tgt, max_burst, ISCSI_FLASHTGT_MAX_BURST); >> +iscsi_flash_tgt_attr(tgt, def_tmf_timeout, ISCSI_FLASHTGT_DEF_TMF_TMO); >> +iscsi_flash_tgt_attr(tgt, ipaddress, ISCSI_FLASHTGT_IPADDR); >> +iscsi_flash_tgt_attr(tgt, targetalias, ISCSI_FLASHTGT_ALIAS); >> +iscsi_flash_tgt_attr(tgt, redirect_ipaddr, >>ISCSI_FLASHTGT_REDIRECT_IPADDR); > >What is this? Is it the address we are redirected to or the address we >initially log in to? For the latter, isn't it then the same as ipaddress >above? > >In the other iscsi code the persistent_address/port is the one we >initially log into. The address/port is then the one we are currently >logged into. > This is equivalent to the address/port we are currently logged into. > >> +iscsi_flash_tgt_attr(tgt, max_segment_size, >>ISCSI_FLASHTGT_MAX_SEGMENT_SIZE); >> +iscsi_flash_tgt_attr(tgt, local_port, ISCSI_FLASHTGT_LOCAL_PORT); > >So why is the port printed but not the local ipaddress? It is due to how >your card supports addresses right? > >I think we want a link to the iface. Is that not possible? > This will not be relevant when the target entry is in the flash. This is here just for placeholder and not running copy. > > >> +iscsi_flash_tgt_attr(tgt, ipv4_tos, ISCSI_FLASHTGT_IPV4_TOS); >> +iscsi_flash_tgt_attr(tgt, ipv6_flow_label, >>ISCSI_FLASHTGT_IPV6_FLOW_LABEL); >> +iscsi_flash_tgt_attr(tgt, name, ISCSI_FLASHTGT_NAME); >> +iscsi_flash_tgt_attr(tgt, tpgt, ISCSI_FLASHTGT_TPGT); >> +iscsi_flash_tgt_attr(tgt, link_local_ipv6, >>ISCSI_FLASHTGT_LINK_LOCAL_IPV6); > >Is this the link local address we are connected through on the initiator >side? > Yes. > > >> +iscsi_flash_tgt_attr(tgt, discovery_parent, >>ISCSI_FLASHTGT_DISCOVERY_PARENT); > >Could you make this print out a string like "send_targets" and 'isns"? > This can be send_target entries index as well. For ISNS it would be a hex number like 0xFFFDh > > >> >> /** >> * struct iscsi_transport - iSCSI Transport template >> @@ -150,6 +151,16 @@ struct iscsi_transport { >> int (*get_chap) (struct Scsi_Host *shost, uint16_t chap_tbl_idx, >> uint32_t *num_entries, char *buf); >> int (*delete_chap) (struct Scsi_Host *shost, uint16_t chap_tbl_idx); >> + int (*get_flash_tgt_param) (struct iscsi_flash_tgt *tgt, int param, >> + char *buf); >> + int (*set_flash_tgt_param) (struct iscsi_flash_tgt *tgt, int param, >> + const char *buf, int len); > >Make this work like the iface set params where multiple params are set >in one call. Yes. > > >> + struct iscsi_flash_tgt *(*new_flash_tgt) (struct Scsi_Host *shost, >> + const char *buf, int len); >> + void (*del_flash_tgt) (struct iscsi_flash_tgt *tgt); >> + int (*apply_flash_tgt) (struct iscsi_flash_tgt *tgt); > >Kill this. Yes. > >> + int (*login_flash_tgt) (struct iscsi_flash_tgt *tgt); >> + int (*logout_flash_tgt) (struct iscsi_cls_session *cls_sess); >> }; > > >My only other comment is why are the portal/connection settings mixed in >with the node/session? I think it migt be cleaner to seperate them. Can >we just put the portal/conn attrs in another attribute group? It will >match how we read other sysfs conn/session params then too. Ok. ________________________________ This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message. -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/open-iscsi?hl=en. For more options, visit https://groups.google.com/groups/opt_out.
