From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Christie Subject: Re: [PATCH v3 2/2][BNX2]: Add iSCSI support to BNX2 devices. Date: Fri, 07 Sep 2007 17:23:07 -0500 Message-ID: <46E1CF4B.9080407@cs.wisc.edu> References: <1188599815.5176.12.camel@dell> <46DEF69D.3090901@redhat.com> <1189027622.19638.42.camel@dhcp-10-13-106-205.broadcom.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Mike Christie , Michael Chan , davem@davemloft.net, netdev@vger.kernel.org, open-iscsi@googlegroups.com, talm@broadcom.com, lusinsky@broadcom.com, uri@broadcom.com, SCSI Mailing List To: Anil Veerabhadrappa Return-path: In-Reply-To: <1189027622.19638.42.camel@dhcp-10-13-106-205.broadcom.com> Sender: linux-scsi-owner@vger.kernel.org List-Id: netdev.vger.kernel.org Anil Veerabhadrappa wrote: >> >>> + >>> +/* iSCSI stages */ >>> +#define ISCSI_STAGE_SECURITY_NEGOTIATION (0) >>> +#define ISCSI_STAGE_LOGIN_OPERATIONAL_NEGOTIATION (1) >>> +#define ISCSI_STAGE_FULL_FEATURE_PHASE (3) >>> +/* Logout response codes */ >>> +#define ISCSI_LOGOUT_RESPONSE_CONNECTION_CLOSED (0) >>> +#define ISCSI_LOGOUT_RESPONSE_CID_NOT_FOUND (1) >>> +#define ISCSI_LOGOUT_RESPONSE_CLEANUP_FAILED (3) >>> + >>> +/* iSCSI task types */ >>> +#define ISCSI_TASK_TYPE_READ (0) >>> +#define ISCSI_TASK_TYPE_WRITE (1) >>> +#define ISCSI_TASK_TYPE_MPATH (2) >> >> >> >> All of these iscsi code shoulds be in iscsi_proto.h or should be added >> there. > This is a very tricky proposal as this header file is automatically > generated by a well defined process and is shared between various driver > supporting multiple platform/OS and the firmware. If it is not of a big > issue I would like to keep it the way it is. The values that are iscsi RFC values should come from the iscsi_proto.h file and not be duplicated for each driver. >>> +/* >>> + * hardware reset >>> + */ >>> +int bnx2i_reset(struct scsi_cmnd *sc) >>> +{ >>> + return 0; >>> +} >> >> So what is up with this one? It seems like if there is a way to reset >> hardware then you would want it as the scsi eh host reset callout >> instead of dropping the session. We could add some transport level >> recovery callouts for the iscsi specifics. > > We may not be able to support HBA cold reset as bnx2 driver is the > primary owner of chip reset and initialization. This is the drawback of > sharing network interface with the NIC driver. If there is a need for > administrator to reset the iSCSI port same can be achieved by running > 'ifdown eth#' and 'ifup eth#'. > Current driver even allows ethernet interface reset when there are > active iSCSI connection, all active iscsi sessions will be reinstated > when the network link comes back live > > If you cannot support it or it does not make sense just remove the stub then. I say it is not a big deal now, but hopefully we do not hit fun like with qla3xxx and qla4xxx :) >>> + >>> +void bnx2i_sysfs_cleanup(void) >>> +{ >>> + class_device_unregister(&port_class_dev); >>> + class_unregister(&bnx2i_class); >>> +} >> The sysfs bits related to the hba should be use one of the scsi sysfs >> facilities or if they are related to iscsi bits and are generic then >> through the iscsi hba > > bnx2i needs 2 sysfs entries - > 1. QP size info - this is used to size per connection shared data > structures to issue work requests to chip (login, scsi cmd, tmf, nopin) > and get completions from the chip (scsi completions, async messages, > etc'). This is a iSCSI HBA attribute > 2. port mapper - we can be more flexible on classifying this as either > iSCSI HBA attribute or bnx2i driver global attribute > Can hooks be added to iSCSI transport class to include these? > Which ones were they exactly? I think JamesB wanted only common transport values in the transport class. If it is driver specific then it should go on the host or target or device with the scsi_host_template attrs.