netdev - Re: [PATCH v3 2/2][BNX2]: Add iSCSI support to BNX2 devices.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <46E1CF4B.9080407@cs.wisc.edu>
Date:	Fri, 07 Sep 2007 17:23:07 -0500
From:	Mike Christie <michaelc@...wisc.edu>
To:	Anil Veerabhadrappa <anilgv@...adcom.com>
CC:	Mike Christie <mchristi@...hat.com>,
	Michael Chan <mchan@...adcom.com>, davem@...emloft.net,
	netdev@...r.kernel.org, open-iscsi@...glegroups.com,
	talm@...adcom.com, lusinsky@...adcom.com, uri@...adcom.com,
	SCSI Mailing List <linux-scsi@...r.kernel.org>
Subject: Re: [PATCH v3 2/2][BNX2]: Add iSCSI support to BNX2 devices.

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.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html