Re: [RFC-v4 0/3] qla2xxx: v3.4 target mode LLD changes + tcm_qla2xxx fabric module

Re: [RFC-v4 0/3] qla2xxx: v3.4 target mode LLD changes + tcm_qla2xxx fabric module

[Bart Van Assche]

On Sunday December 18, 2011 Nicholas Bellinger wrote:
> So to get the ball rolling on remaining items, one question is still how to
> resolve mixed target/initiator mode operation on a HW per port context basis..?
> 
> This is currently done with a qla2xxx module parameter, but to do mixed mode
> properly we will need something smarter between scsi-core and target-core ports.
> Note we currently set qlini_mode = QLA2XXX_INI_MODE_STR_DISABLED, so by default
> patch #1 will effectively disable initiator mode by skipping scsi_scan_host()
> from being called in to avoid scsi-core timeouts when performing immediate
> transition from initiator mode -> target mode via ISP reset.
> 
> What we would like to eventually do is run qla2xxx LLD to allow both initiator
> and target mode access based on the physical HW port.  We tried some simple
> qla_target.c changes this make this work, but to really do it properly
> and address current qlini_mode = QLA2XXX_INI_MODE_STR_DISABLED usage it will
> need to involve scsi-core so that individual HW port can be configured and
> dynamically changed across different access modes.

(sorry for the late reply)

Dynamically switching between initiator mode, target mode or even using both
modes simultaneously is already possible for iSCSI over Ethernet and for SRP
over InfiniBand with the current SCSI core. Why would the SCSI core have to be
modified in order to make the same possible for FCP ? Am I missing something ?
If SCSI initiator timeouts during initiator-to-target mode transitions are an
issue, why not to abort pending SCSI commands before performing the transition ?

Bart.

Re: [RFC-v4 0/3] qla2xxx: v3.4 target mode LLD changes + tcm_qla2xxx fabric module

[Nicholas A. Bellinger]

On Sun, 2012-05-13 at 15:55 +0000, Bart Van Assche wrote:
> On Sunday December 18, 2011 Nicholas Bellinger wrote:
> > So to get the ball rolling on remaining items, one question is still how to
> > resolve mixed target/initiator mode operation on a HW per port context basis..?
> > 
> > This is currently done with a qla2xxx module parameter, but to do mixed mode
> > properly we will need something smarter between scsi-core and target-core ports.
> > Note we currently set qlini_mode = QLA2XXX_INI_MODE_STR_DISABLED, so by default
> > patch #1 will effectively disable initiator mode by skipping scsi_scan_host()
> > from being called in to avoid scsi-core timeouts when performing immediate
> > transition from initiator mode -> target mode via ISP reset.
> > 
> > What we would like to eventually do is run qla2xxx LLD to allow both initiator
> > and target mode access based on the physical HW port.  We tried some simple
> > qla_target.c changes this make this work, but to really do it properly
> > and address current qlini_mode = QLA2XXX_INI_MODE_STR_DISABLED usage it will
> > need to involve scsi-core so that individual HW port can be configured and
> > dynamically changed across different access modes.
> 
> (sorry for the late reply)
> 
> Dynamically switching between initiator mode, target mode or even using both
> modes simultaneously is already possible for iSCSI over Ethernet and for SRP
> over InfiniBand with the current SCSI core.

Correct, because the implementation of these two examples is not
dependent upon the registering of any LLD logic within scsi-core in
order to function in target mode.  We've always been able to avoid these
types of conflicts between scsi-core and target-core subsystems up until
now when initiator + target have been implemented as logically separate
drivers.

> Why would the SCSI core have to be modified in order to make the same possible
> for FCP ? Am I missing something ?

So it's nothing fabric specific to FCP of course, look at software +
Intel 82599 HW offload for example in tcm_fc(FCoE) where initiator and
target mode are logically separate drivers based upon common libfc
code..

But the underlying issue with the current implementation of HW FC HBAs
such as qla2xxx is that only a small subset of code is actually made
common (at least in the fabric I/O codepath sense) amongst HW FC LLDs
within libfc.  It's my understanding that since this type of code has
historically been very driver/vendor specific, trying to make this
(more) common for individual HW FC HBAs hasn't been interesting enough
for vendors to pursue benefit wise given the amount of re-factoring +
regressions involved to existing initiator-mode operation. 

That being the case, qla_target.c ends up depending upon alot of LLD
specific code (some of which is not initiator specific or made common in
libfc), so at least in the current implementation it really doesn't make
sense to try to push this logic back out into tcm_qla2xxx code..

> If SCSI initiator timeouts during initiator-to-target mode transitions are an
> issue, why not to abort pending SCSI commands before performing the transition ?
> 

That would probably work, but I think it's still just a band-aid on the
underlying issue of SCSI LLDs always kicking off initiator mode enable /
disable operations from within PCI *_probe_one() / *_remove_code() code.

--nab

Re: [RFC-v4 0/3] qla2xxx: v3.4 target mode LLD changes + tcm_qla2xxx fabric module

[Bart Van Assche]

On 05/14/12 03:29, Nicholas A. Bellinger wrote:

> That would probably work, but I think it's still just a band-aid on the
> underlying issue of SCSI LLDs always kicking off initiator mode enable /
> disable operations from within PCI *_probe_one() / *_remove_code() code.


Maybe the following makes sense (I'm not familiar with the qla2xxx
driver nor with FC): split the qla2xxx driver in three kernel modules -
a kernel module with the code that is shared by initiator and target
mode, a kernel module with the initiator functionality and a kernel
module with the target functionality. This will allow users to choose
which functionality to enable by loading the proper kernel module(s).
Note: in the kernel module with the shared code an interface will have
to be added that allows the initiator and the target module to enumerate
qla2xxx HBA ports. Maybe it's a good idea to add an interface similar to
the add_one() / remove_one() callback functions present in the Linux
InfiniBand stack.

Bart.

Re: [RFC-v4 0/3] qla2xxx: v3.4 target mode LLD changes + tcm_qla2xxx fabric module

[Nicholas A. Bellinger]

On Mon, 2012-05-14 at 12:50 +0000, Bart Van Assche wrote:
> On 05/14/12 03:29, Nicholas A. Bellinger wrote:
> 
> > That would probably work, but I think it's still just a band-aid on the
> > underlying issue of SCSI LLDs always kicking off initiator mode enable /
> > disable operations from within PCI *_probe_one() / *_remove_code() code.
> 
> 
> Maybe the following makes sense (I'm not familiar with the qla2xxx
> driver nor with FC): split the qla2xxx driver in three kernel modules -
> a kernel module with the code that is shared by initiator and target
> mode, a kernel module with the initiator functionality and a kernel
> module with the target functionality. This will allow users to choose
> which functionality to enable by loading the proper kernel module(s).

Seriously, this involves high-level re-factoring of FC HBA drivers with
10x the moving parts and 100x the mission critical install base of your
other HW example.

So yes, wanting to re-factor logic out into common code for initiator /
target mode is good starting point, but without getting into the
real-deal FC HW specifics of how this would work for running code along
with tangible HBA vendor interest backing it, this type of statement
quickly falls into the realm of wishful thinking.

> Note: in the kernel module with the shared code an interface will have
> to be added that allows the initiator and the target module to enumerate
> qla2xxx HBA ports. Maybe it's a good idea to add an interface similar to
> the add_one() / remove_one() callback functions present in the Linux
> InfiniBand stack.
> 

Roland has had a few thoughts to this end about incremental improvements
post 3.5 merge, but the target team has been primarily interested in
fixing tcm_qla2xxx bugs and ensuring the qla2xxx LLD changes for target
mode do not cause FC initiator mode regressions.  

Considering the time scales involved with doing this type of testing
(esp the latter, which can take months) plus given the amount of LLD
code involved (40K LOC), a large re-org that effects existing FC
initiator mode operation is something we'd like to avoid for now.

Thanks,

--nab

Re: [RFC-v4 0/3] qla2xxx: v3.4 target mode LLD changes + tcm_qla2xxx fabric module

[Bart Van Assche]

On 05/14/12 23:12, Nicholas A. Bellinger wrote:

> On Mon, 2012-05-14 at 12:50 +0000, Bart Van Assche wrote:
>> Note: in the kernel module with the shared code an interface will have
>> to be added that allows the initiator and the target module to enumerate
>> qla2xxx HBA ports. Maybe it's a good idea to add an interface similar to
>> the add_one() / remove_one() callback functions present in the Linux
>> InfiniBand stack.
> 
> Considering the time scales involved with doing this type of testing
> (esp the latter, which can take months) plus given the amount of LLD
> code involved (40K LOC), a large re-org that effects existing FC
> initiator mode operation is something we'd like to avoid for now.


Let me summarize what I've noticed after I had a (very) short look at
the proposed qla2xxx driver changes:
- The proposed changes do not allow to enable initiator and target mode
  simultaneously on a single FC port.
- Most of the initiator-mode SCSI host sysfs attributes are relevant in
  target mode too (e.g. NV-RAM access) and hence should be moved from
  the SCSI host to somewhere else. A good example is e.g. the Linux IB
  stack - in the IB stack there is a clean separation between
  HCA-specific sysfs attributes and sysfs attributes associated with
  higher-level services.
- The posted patch does not allow to add more FC functionality in a
  clean way, e.g. IP over FC or FC-VI. (Note: IP over FC has been
  standardized through RFC 4338 - http://tools.ietf.org/html/rfc4338.)

So I'm not sure whether the proposed approach is acceptable for a
mainline kernel driver. However, I'm not a SCSI maintainer and hence I
do not have a decisive voice in this matter.

Bart.

Re: [RFC-v4 0/3] qla2xxx: v3.4 target mode LLD changes + tcm_qla2xxx fabric module

[Nicholas A. Bellinger]

On Thu, 2011-12-22 at 14:25 -0800, Andrew Vasquez wrote:
> On Wed, 21 Dec 2011, Christoph Hellwig wrote:
> 
> > I think the most important item is to sort out the mess around the old
> > generation qla23xx support.  The way the code currently sprinkles ifs
> > around that is a complete mess.  Given that the qla23xx support has as
> > far as I know zero test coverage, and has been EOLed by qlogic I see
> > no reason to keep it around.
> > 
> > Anyone disagreeing with that?
> > 
> 
> Christoph,
> 
> >From an engineering and support perspective it doesn't make sense to
> keep the (unsupported and untested) pre-ISP24xx codes around.  Going
> forward, once internal resources can be allocated, QLogic would like
> to see support for all ISP24xx and above hardware.
> 
> Regards,
> Andrew Vasquez
> 

Hi Andrew,

Thanks for the official clarification here from Qlogic wrt to pre 24xx
hardware support in qla_target.c.  We will go ahead and begin dropping
the legacy support and plan to have all the old code removed for an
RFC-v5 series to be posted after the holidays.

Thank you,

--nab

Re: [RFC-v4 0/3] qla2xxx: v3.4 target mode LLD changes + tcm_qla2xxx fabric module

[Andrew Vasquez]

On Wed, 21 Dec 2011, Christoph Hellwig wrote:

> I think the most important item is to sort out the mess around the old
> generation qla23xx support.  The way the code currently sprinkles ifs
> around that is a complete mess.  Given that the qla23xx support has as
> far as I know zero test coverage, and has been EOLed by qlogic I see
> no reason to keep it around.
> 
> Anyone disagreeing with that?
> 

Christoph,

>From an engineering and support perspective it doesn't make sense to
keep the (unsupported and untested) pre-ISP24xx codes around.  Going
forward, once internal resources can be allocated, QLogic would like
to see support for all ISP24xx and above hardware.

Regards,
Andrew Vasquez

Re: [RFC-v4 0/3] qla2xxx: v3.4 target mode LLD changes + tcm_qla2xxx fabric module

[Christoph Hellwig]

I think the most important item is to sort out the mess around the old
generation qla23xx support.  The way the code currently sprinkles ifs
around that is a complete mess.  Given that the qla23xx support has as
far as I know zero test coverage, and has been EOLed by qlogic I see
no reason to keep it around.

Anyone disagreeing with that?

The other bit is sorting out handling of the full command queues in the
hardware.  Currently the target core does a blind retry after some delay,
which isn't a good idea.  I had an RFC patch on how to move it into the
driver, but I'll really need some help to find the proper place to wake
up any process waiting once we get free slots.

[RFC-v4 0/3] qla2xxx: v3.4 target mode LLD changes + tcm_qla2xxx fabric module

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

Hi Andrew, James, Roland, Christoph & Co,

The following is the forth RFC series for adding qla2xxx LLD target mode support
into mainline 8.03.07.07-k @ 3.2-rc5 along with accompanying tcm_qla2xxx.ko
fabric module cut against target v4.1 infrastructure.  As before, this series
has been broken up into reviewable sections and should considered a 'for-3.4'
item as remaining TODO items are resolved.

The code is available directly against the most recent target-pending/for-next
changes using new v3.3 target_submit_cmd logic here:

  git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git qla_tgt-rfc-v4

There have been many improvements, bugfixes and cleanups since RFC-v3 during
the course of 2011.  Thanks to everyone who has contributed in order to get
this code worked into shape, as well as thanks to Vlad and the SCST community
for original qla2x00t implementation.

Here is a brief rundown of the changes since RFC-v3 earlier this year:

*) Convert to use scatterlist for all CDBS types for mainline target (andy + hch)
*) Fix for NULL s_id in qla_tgt_exec_sess_work() abort handling
   (roland via r3244 scst svn)
*) Disable EXPLICIT_CONFORM for FCP READ CHECK_CONDITION to address issue where
   SRR was being triggered during CHECK_CONDITION handling (nab + roland)
*) Address issues in qla_target.c request queue full handling (roland)
*) Add support for active I/O shutdown using generic target logic.  This includes
   numerous bugfixes related to shutdown. (roland + pure team + nab)
*) Kick target ATIO queue after qla2x00_fw_ready() completion. (roland + andrew)
*) Convert qla_target.c to use mainline v3.1 qla_dbg macros (nab)
*) Move to use command hardware structure definitions between > 24xx and older
   hardware. (madhu)
*) Make qla_target.c follow qla2xxx consistent code+naming conventions (madhu)
*) Add tcm_qla2xxx_free_wq for process context release instead of using
   TRANSPORT_FREE_CMD_INTR (hch + nab)
*) Refactor qla_tgt_send_term_exchange() in order to remove qla_tgt_cmd->locked_rsp
   exception path usage. (nab)
*) Conversion to use an internal workqueue in qla_target.c code for I/O dispatch
   into tcm_qla2xxx instead of legacy ->new_cmd_map(). (hch + nab)
*) Removal of unnecessary qla_hw_data->hardware_lock access in tcm_qla2xxx response
   path in tcm_qla2xxx_check_stop_free() and tcm_qla2xxx_release_cmd() (nab + joern)
*) Conversion to use target_submit_cmd() with for-next v3.3 code (nab + hch)
*) Merge into single tcm_qla2xxx.[c,h] files, and move into drivers/scsi/qla2xxx/

So to get the ball rolling on remaining items, one question is still how to
resolve mixed target/initiator mode operation on a HW per port context basis..?

This is currently done with a qla2xxx module parameter, but to do mixed mode
properly we will need something smarter between scsi-core and target-core ports.
Note we currently set qlini_mode = QLA2XXX_INI_MODE_STR_DISABLED, so by default
patch #1 will effectively disable initiator mode by skipping scsi_scan_host()
from being called in to avoid scsi-core timeouts when performing immediate
transition from initiator mode -> target mode via ISP reset.

What we would like to eventually do is run qla2xxx LLD to allow both initiator
and target mode access based on the physical HW port.  We tried some simple
qla_target.c changes this make this work, but to really do it properly
and address current qlini_mode = QLA2XXX_INI_MODE_STR_DISABLED usage it will
need to involve scsi-core so that individual HW port can be configured and
dynamically changed across different access modes.

(hch + james comments here..?)

So along with resolving this issue with mixed T/I mode, other TODO items
include:

*) Seperate out per qla_tgt session management from qla_hw_data->hardware_lock
   to use seperate lock. (nab)
*) Move write-pending abort checks into process-context (nab)
*) Finish up NPIV support for /sys/kernel/config/target/qla2xxx_npiv/ and determine
   remaining NPIV I/O path items. (nab + madhu)
*) FC jammer testing to verify bugfix for SRRs with non zero relative offsets before
   re-enabling them. (andrew + qlogic team)
*) Add proper support for ABORT_TASK in target-core (nab)
*) Multi-queue support with qla_hw_data->mqenable=1 (mq target fw support..?) 
*) Global event handling for active sessions in qla_tgt_reset()

Please have a look and let us know if you have any comments,

Thank you,

Nicholas Bellinger (3):
  qla2xxx: Add LLD internal target-mode support
  qla2xxx: Enable 2xxx series LLD target mode support
  qla2xxx: Add tcm_qla2xxx fabric module for mainline target

 drivers/scsi/qla2xxx/Kconfig       |    8 +
 drivers/scsi/qla2xxx/Makefile      |    3 +-
 drivers/scsi/qla2xxx/qla_attr.c    |    5 +-
 drivers/scsi/qla2xxx/qla_dbg.c     |   13 +-
 drivers/scsi/qla2xxx/qla_dbg.h     |    5 +
 drivers/scsi/qla2xxx/qla_def.h     |   70 +-
 drivers/scsi/qla2xxx/qla_gbl.h     |    7 +
 drivers/scsi/qla2xxx/qla_gs.c      |    4 +-
 drivers/scsi/qla2xxx/qla_init.c    |  101 +-
 drivers/scsi/qla2xxx/qla_iocb.c    |  105 +-
 drivers/scsi/qla2xxx/qla_isr.c     |   86 +-
 drivers/scsi/qla2xxx/qla_mbx.c     |  122 +-
 drivers/scsi/qla2xxx/qla_mid.c     |   21 +-
 drivers/scsi/qla2xxx/qla_os.c      |  126 +-
 drivers/scsi/qla2xxx/qla_target.c  | 5482 ++++++++++++++++++++++++++++++++++++
 drivers/scsi/qla2xxx/qla_target.h  | 1147 ++++++++
 drivers/scsi/qla2xxx/tcm_qla2xxx.c | 2059 ++++++++++++++
 drivers/scsi/qla2xxx/tcm_qla2xxx.h |  148 +
 18 files changed, 9454 insertions(+), 58 deletions(-)
 create mode 100644 drivers/scsi/qla2xxx/qla_target.c
 create mode 100644 drivers/scsi/qla2xxx/qla_target.h
 create mode 100644 drivers/scsi/qla2xxx/tcm_qla2xxx.c
 create mode 100644 drivers/scsi/qla2xxx/tcm_qla2xxx.h

-- 
1.7.2.3