[ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[James Smart]

I'd like to announce the availability of the Broadcom (Emulex) FC Target 
driver - efct.
This driver has been part of the Emulex OneCore Storage SDK tool kit for 
Emulex
SLI-4 adapters. The SLI-4 adapters support 16Gb/s and higher adapters. 
Although this
kit has supported FCoE in the past, it is currently limited to FC support.

This driver provides the following:
- Target mode operation:
   - Functional with LIO-based interfaces
   - Extensive use of hw offloads such as auto-xfer_rdy, auto-rsp, cmd 
cpu spreading
   - High login mode - thousands of logins
   - T-10 DIF/PI support  (inline and separate)
   - NPIV support
- Concurrent Initiator support if needed
- Discovery engine has F_Port and fabric services emulation.
- Extended mgmt interfaces:
   - firmware dump api, including dump to host memory for faster dumps
   - Healthcheck operations and watchdogs
- Extended driver behaviors such as:
   - polled mode operation
   - multi-queue: cpu, roundrobin, or priority  (but not tied  to scsi-mq)
   - long chained sgl's
   - extensive internal logging and statistics
   - Tuning parameters on modes and resource allocation to different 
features

Broadcom is looking to upstream this driver and would like review and 
feedback.
The driver may be found at the following git repository:
     git@gitlab.com:jsmart/efct-Emulex_FC_Target.git


Some of the key questions we have are with lpfc :
1) Coexistence vs integration
Currently, the efct driver maps to a different set of PCI ids than lpfc.
It's very clear there's an overlap with lpfc, both on SLI-4 hw as well 
as initiator support.
Although target mode support can be simplistically added to lpfc, what 
we've found is
that doing so means a lot of tradeoffs. Some of the target mode 
features, when enabled,
impact the initiator support and how it would operate.

2) SLI-3 support
lpfc provides SLI-3 support so that all FC adapters are supported, 
including the older ones.
The form of the driver, based on its history, is SLI-3 with SLI-3 
adapted to SLI-4 at the point
it hits the hardware. efct does not support SLI-3.

3) complexity of configuration knobs caused by the kitchen-sink of 
features in lpfc ?
we are pushing the limit on needing per-instance attributes rather than 
global module
parameters.

-- james

Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[Hannes Reinecke]

Hi James,

On 02/28/2017 12:28 AM, James Smart wrote:
> I'd like to announce the availability of the Broadcom (Emulex) FC Target
> driver - efct.
> This driver has been part of the Emulex OneCore Storage SDK tool kit for
> Emulex
> SLI-4 adapters. The SLI-4 adapters support 16Gb/s and higher adapters.
> Although this
> kit has supported FCoE in the past, it is currently limited to FC support.
> 
> This driver provides the following:
> - Target mode operation:
>   - Functional with LIO-based interfaces
>   - Extensive use of hw offloads such as auto-xfer_rdy, auto-rsp, cmd
> cpu spreading
>   - High login mode - thousands of logins
>   - T-10 DIF/PI support  (inline and separate)
>   - NPIV support
> - Concurrent Initiator support if needed
> - Discovery engine has F_Port and fabric services emulation.
> - Extended mgmt interfaces:
>   - firmware dump api, including dump to host memory for faster dumps
>   - Healthcheck operations and watchdogs
> - Extended driver behaviors such as:
>   - polled mode operation
>   - multi-queue: cpu, roundrobin, or priority  (but not tied  to scsi-mq)
>   - long chained sgl's
>   - extensive internal logging and statistics
>   - Tuning parameters on modes and resource allocation to different
> features
> 
Yay! Finally!

> Broadcom is looking to upstream this driver and would like review and
> feedback.
> The driver may be found at the following git repository:
>     git@gitlab.com:jsmart/efct-Emulex_FC_Target.git
> 
> 
> Some of the key questions we have are with lpfc :
> 1) Coexistence vs integration
> Currently, the efct driver maps to a different set of PCI ids than lpfc.
> It's very clear there's an overlap with lpfc, both on SLI-4 hw as well
> as initiator support.
> Although target mode support can be simplistically added to lpfc, what
> we've found is that doing so means a lot of tradeoffs. Some of the
> target mode features, when enabled,
> impact the initiator support and how it would operate.
> 
Can you clarify these?
Are these 'just' resource allocation problems or something else, too?

> 2) SLI-3 support
> lpfc provides SLI-3 support so that all FC adapters are supported,
> including the older ones.
> The form of the driver, based on its history, is SLI-3 with SLI-3
> adapted to SLI-4 at the point
> it hits the hardware. efct does not support SLI-3.
> 
I personally wouldn't have a problem with _not_ enabling SLI-3; after
all, this is (conceptually :-) new code, so we do not _need_ to support
older HW here.

> 3) complexity of configuration knobs caused by the kitchen-sink of
> features in lpfc ?
> we are pushing the limit on needing per-instance attributes rather than
> global module
> parameters.
> 
Yes, this is always a good idea. Also one might want to look at which of
those knobs are there for pure historical reasons, maybe we can cut down
on the number of knobs already :-)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[James Smart]

On 2/28/2017 8:34 AM, Hannes Reinecke wrote:
> Can you clarify these?
> Are these 'just' resource allocation problems or something else, too?

Most are resource allocation - buffer pools, dma pools, pages for 
resources, and hw resource allocation splits. However, async receive RQ 
policies are a case where initiator and target have to share the policy 
and perhaps a buffer pool, so they have to be careful. Another area is 
in ABTS handling. Initiators typically leave things up to the hardware, 
and do little if any ABTS handling. Most targets though, as CMD IU may 
be received without an assign exchange context and be buffered until the 
target is ready to do something, require that they handle ABTS's.  Some 
of the target features, when enabled, dictate host ownership of ABTS 
policy. So, if running I+T, it gets rather tricky.

-- james

Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[Hannes Reinecke]

On 02/28/2017 07:14 PM, James Smart wrote:
> 
> On 2/28/2017 8:34 AM, Hannes Reinecke wrote:
>> Can you clarify these?
>> Are these 'just' resource allocation problems or something else, too?
> 
> Most are resource allocation - buffer pools, dma pools, pages for
> resources, and hw resource allocation splits. However, async receive RQ
> policies are a case where initiator and target have to share the policy
> and perhaps a buffer pool, so they have to be careful. Another area is
> in ABTS handling. Initiators typically leave things up to the hardware,
> and do little if any ABTS handling. Most targets though, as CMD IU may
> be received without an assign exchange context and be buffered until the
> target is ready to do something, require that they handle ABTS's.  Some
> of the target features, when enabled, dictate host ownership of ABTS
> policy. So, if running I+T, it gets rather tricky.
> 
Ah, okay.
However, I still would favour having both integrated into the same
driver; otherwise we run into the tricky issue of having to manually
unbind drivers from a given PCI device (or declare some PCI devices as
target capable on a rather arbitrary manner).
Can't it be modelled like the NVME-FC target, with having the same
driver supporting all modes, but the admin having to decide which PCI
function is doing what?
(NB: What about SR-IOV? Are the resources shared across functions or
does each function have its own set? If so than I'd be perfectly happy
if we could set each _function_ to a given role, much like
VirtualConnect does nowadays).

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[Nicholas A. Bellinger]

Hi James & Co,

Adding target-devel and Sebastian CC'

On Mon, 2017-02-27 at 15:28 -0800, James Smart wrote:
> I'd like to announce the availability of the Broadcom (Emulex) FC Target 
> driver - efct.
> This driver has been part of the Emulex OneCore Storage SDK tool kit for 
> Emulex
> SLI-4 adapters. The SLI-4 adapters support 16Gb/s and higher adapters. 
> Although this
> kit has supported FCoE in the past, it is currently limited to FC support.
> 
> This driver provides the following:
> - Target mode operation:
>    - Functional with LIO-based interfaces

Glad to see the upstream push for this after all these years.  :)

>    - Extensive use of hw offloads such as auto-xfer_rdy, auto-rsp, cmd 
> cpu spreading
>    - High login mode - thousands of logins
>    - T-10 DIF/PI support  (inline and separate)
>    - NPIV support
> - Concurrent Initiator support if needed
> - Discovery engine has F_Port and fabric services emulation.
> - Extended mgmt interfaces:
>    - firmware dump api, including dump to host memory for faster dumps
>    - Healthcheck operations and watchdogs
> - Extended driver behaviors such as:
>    - polled mode operation
>    - multi-queue: cpu, roundrobin, or priority  (but not tied  to scsi-mq)
>    - long chained sgl's
>    - extensive internal logging and statistics
>    - Tuning parameters on modes and resource allocation to different 
> features
> 
> Broadcom is looking to upstream this driver and would like review and 
> feedback.
> The driver may be found at the following git repository:
>      git@gitlab.com:jsmart/efct-Emulex_FC_Target.git
> 

Can we get the patch series posted to linux-scsi and target-devel to
start giving some initial feedback..?

> 
> Some of the key questions we have are with lpfc :
> 1) Coexistence vs integration
> Currently, the efct driver maps to a different set of PCI ids than lpfc.
> It's very clear there's an overlap with lpfc, both on SLI-4 hw as well 
> as initiator support.
> Although target mode support can be simplistically added to lpfc, what 
> we've found is
> that doing so means a lot of tradeoffs. Some of the target mode 
> features, when enabled,
> impact the initiator support and how it would operate.
> 

I don't really have much preference either way.  

> 2) SLI-3 support
> lpfc provides SLI-3 support so that all FC adapters are supported, 
> including the older ones.
> The form of the driver, based on its history, is SLI-3 with SLI-3 
> adapted to SLI-4 at the point
> it hits the hardware. efct does not support SLI-3.

AFAIK I think Sebastian was using SLI-3, so he might have some comments
here.

Since he's been the main person using the original tcm_lpfc code from
way back when, maybe it would be a good idea to send him a couple of
SLI-4 HBAs to help with the upstreaming of efct..?

> 
> 3) complexity of configuration knobs caused by the kitchen-sink of 
> features in lpfc ?
> we are pushing the limit on needing per-instance attributes rather than 
> global module
> parameters.

This is exactly what
/sys/kernel/config/target/efct/$WWPN/tpgt_1/attribute/ is intended for.

Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[Sebastian Herbszt]

Nicholas A. Bellinger wrote:
> Hi James & Co,
> 
> Adding target-devel and Sebastian CC'
> 
> On Mon, 2017-02-27 at 15:28 -0800, James Smart wrote:
> > I'd like to announce the availability of the Broadcom (Emulex) FC
> > Target driver - efct.
> > This driver has been part of the Emulex OneCore Storage SDK tool
> > kit for Emulex
> > SLI-4 adapters. The SLI-4 adapters support 16Gb/s and higher
> > adapters. Although this
> > kit has supported FCoE in the past, it is currently limited to FC
> > support.
> > 
> > This driver provides the following:
> > - Target mode operation:
> >    - Functional with LIO-based interfaces  
> 
> Glad to see the upstream push for this after all these years.  :)

How does this compare to ocs_fc_lio? Does efct replace it?
 
> >    - Extensive use of hw offloads such as auto-xfer_rdy, auto-rsp,
> > cmd cpu spreading
> >    - High login mode - thousands of logins
> >    - T-10 DIF/PI support  (inline and separate)
> >    - NPIV support
> > - Concurrent Initiator support if needed
> > - Discovery engine has F_Port and fabric services emulation.
> > - Extended mgmt interfaces:
> >    - firmware dump api, including dump to host memory for faster
> > dumps
> >    - Healthcheck operations and watchdogs
> > - Extended driver behaviors such as:
> >    - polled mode operation
> >    - multi-queue: cpu, roundrobin, or priority  (but not tied  to
> > scsi-mq)
> >    - long chained sgl's
> >    - extensive internal logging and statistics
> >    - Tuning parameters on modes and resource allocation to
> > different features
> > 
> > Broadcom is looking to upstream this driver and would like review
> > and feedback.
> > The driver may be found at the following git repository:
> >      git@gitlab.com:jsmart/efct-Emulex_FC_Target.git
> >   
> 
> Can we get the patch series posted to linux-scsi and target-devel to
> start giving some initial feedback..?
> 
> > 
> > Some of the key questions we have are with lpfc :
> > 1) Coexistence vs integration
> > Currently, the efct driver maps to a different set of PCI ids than
> > lpfc. It's very clear there's an overlap with lpfc, both on SLI-4
> > hw as well as initiator support.
> > Although target mode support can be simplistically added to lpfc,
> > what we've found is
> > that doing so means a lot of tradeoffs. Some of the target mode 
> > features, when enabled,
> > impact the initiator support and how it would operate.
> >   
> 
> I don't really have much preference either way.  

Just like Hannes I do favour integration. I guess it could be
comparable to qla2xxx + tcm_qla2xxx, lpfc + lpfc_scst and
lpfc + tcm_lpfc. That approach might even help Bart with his
target driver unification if he didn't give up on that topic.

> > 2) SLI-3 support
> > lpfc provides SLI-3 support so that all FC adapters are supported, 
> > including the older ones.
> > The form of the driver, based on its history, is SLI-3 with SLI-3 
> > adapted to SLI-4 at the point
> > it hits the hardware. efct does not support SLI-3.  
> 
> AFAIK I think Sebastian was using SLI-3, so he might have some
> comments here.

I do care for SLI-3. Currently I still only got access to 8GFC HBAs and
even our new cards are SLI-3 and not Advanced-8 with SLI-4. Seems like
SLI-3 is not dead yet.

> Since he's been the main person using the original tcm_lpfc code from
> way back when, maybe it would be a good idea to send him a couple of
> SLI-4 HBAs to help with the upstreaming of efct..?

The code I got here is actually a direct port from lpfc + lpfc_scst to
TCM. As of now it kind of works on kernel version 4.10.

> > 
> > 3) complexity of configuration knobs caused by the kitchen-sink of 
> > features in lpfc ?
> > we are pushing the limit on needing per-instance attributes rather
> > than global module
> > parameters.  
> 
> This is exactly what
> /sys/kernel/config/target/efct/$WWPN/tpgt_1/attribute/ is intended
> for.
> 

Sebastian

Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[Bart Van Assche]

On Sun, 2017-03-05 at 17:35 +0100, Sebastian Herbszt wrote:
> Just like Hannes I do favour integration. I guess it could be
> comparable to qla2xxx + tcm_qla2xxx, lpfc + lpfc_scst and
> lpfc + tcm_lpfc. That approach might even help Bart with his
> target driver unification if he didn't give up on that topic.

Hello Sebastian,

That project is still on my to-do list :-) However, while I was
working on that project I hit bugs related to abort handling in
the QLogic FC and also in the iSCSI target drivers. I want to fix
these bugs first because otherwise if my tests fail it is not
possible to figure out whether I hit a regression or an existing
bug.

Bart.

Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[Roland Dreier]

On Sun, Mar 5, 2017 at 8:35 AM, Sebastian Herbszt <herbszt@gmx.de> wrote:
> Just like Hannes I do favour integration. I guess it could be
> comparable to qla2xxx + tcm_qla2xxx, lpfc + lpfc_scst and
> lpfc + tcm_lpfc. That approach might even help Bart with his
> target driver unification if he didn't give up on that topic.

Resurrecting this old topic - sorry for not seeing this go by initially.

For context, I have a lot of experience debugging the qla2xxx target
code - we did a lot of work to get error/exception paths correct.
Basic FC target support is pretty straightforward but handling SAN log
in / log out events and other strange things that initiators do took a
lot of effort.

Anyway, my feeling is that the integration of tcm_qla2xxx and qla2xxx
was overall a net negative.  Having the target driver grafted onto the
side of an already-complex driver that has a bunch of code not
relevant to the target (SCSI error handling, logging into and timing
out remote target ports, etc) made the code harder to debug and harder
to get right.

Of course I'm in favor of making common code really common.  So
certainly we should have a common library of SLI-4 code that both the
initiator and target driver share.  And if there is more commonality,
that's great.  But any code similar to "if (initiator) ... else ..."
is really suspect to me - grepping for "qla_ini_mode_enabled" shows
great examples like

                if (fcport->scan_state == QLA_FCPORT_SCAN) {
                        if ((qla_dual_mode_enabled(vha) ||
                            qla_ini_mode_enabled(vha)) &&
                            atomic_read(&fcport->state) == FCS_ONLINE) {
                                qla2x00_mark_device_lost(vha, fcport,
                                        ql2xplogiabsentdevice, 0);
                                if (fcport->loop_id != FC_NO_LOOP_ID &&
                                    (fcport->flags & FCF_FCP2_DEVICE) == 0 &&
                                    fcport->port_type != FCT_INITIATOR &&
                                    fcport->port_type != FCT_BROADCAST) {
                                        ql_dbg(ql_dbg_disc, vha, 0xffff,
                                            "%s %d %8phC post del sess\n",
                                            __func__, __LINE__,
                                            fcport->port_name);

                                        qlt_schedule_sess_for_deletion_lock
                                                (fcport);
                                        continue;
                                }
                        }
                }

Handling "dual mode" (both initiator and target on the same port at
the same time) is a design challenge, but I don't think the current
qla2xxx driver is an example of a maintainable way to do that.

(I'm agnostic about what to do about SLI-3 - perhaps the cleanest
thing to do is split the driver between SLI-4 and SLI-3, and handle
the initiator and target drivers for those two cases as appropriate)

I'd love to discuss this further and come up with a design that meets
the concerns about integration but also learns the lessons from
tcm_qla2xxx.

 - R.

Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[James Smart]

On 5/16/2017 12:59 PM, Roland Dreier wrote:
> On Sun, Mar 5, 2017 at 8:35 AM, Sebastian Herbszt <herbszt@gmx.de> wrote:
>> Just like Hannes I do favour integration. I guess it could be
>> comparable to qla2xxx + tcm_qla2xxx, lpfc + lpfc_scst and
>> lpfc + tcm_lpfc. That approach might even help Bart with his
>> target driver unification if he didn't give up on that topic.
> Resurrecting this old topic - sorry for not seeing this go by initially.
>
> For context, I have a lot of experience debugging the qla2xxx target
> code - we did a lot of work to get error/exception paths correct.
> Basic FC target support is pretty straightforward but handling SAN log
> in / log out events and other strange things that initiators do took a
> lot of effort.
>
> Anyway, my feeling is that the integration of tcm_qla2xxx and qla2xxx
> was overall a net negative.  Having the target driver grafted onto the
> side of an already-complex driver that has a bunch of code not
> relevant to the target (SCSI error handling, logging into and timing
> out remote target ports, etc) made the code harder to debug and harder
> to get right.
>
> Of course I'm in favor of making common code really common.  So
> certainly we should have a common library of SLI-4 code that both the
> initiator and target driver share.  And if there is more commonality,
> that's great.  But any code similar to "if (initiator) ... else ..."
> is really suspect to me - grepping for "qla_ini_mode_enabled" shows
> great examples like
>
> ...
>
> Handling "dual mode" (both initiator and target on the same port at
> the same time) is a design challenge, but I don't think the current
> qla2xxx driver is an example of a maintainable way to do that.
>
> (I'm agnostic about what to do about SLI-3 - perhaps the cleanest
> thing to do is split the driver between SLI-4 and SLI-3, and handle
> the initiator and target drivers for those two cases as appropriate)
>
> I'd love to discuss this further and come up with a design that meets
> the concerns about integration but also learns the lessons from
> tcm_qla2xxx.
>
>   - R.


Thanks for the feedback.  I believe you echo many of our concerns as we 
look at "merging them into one".  I agree with your statements on the 
number of if-else roles and know that we made this even more complicated 
by the driver doing fc-nvme initiator and fc-nvme target as well.  Your 
small list of "mode_enabled" hits pales in comparison to a hit list in 
the current driver if looking for SCSI initiator support 
(LPFC_ENABLE_FCP), NVME initiator support (LPFC_ENABLE_NVME), or NVME 
target support (phba->nvmet_support). And that's before adding SCSI 
target support.   We're also concerned about the discovery engines as 
not only are there lots of different paths for the different roles as 
well as support for fcoe, but there are a lot of carefully managed 
accommodations for various oem and switch environments. It's very 
difficult to replicate and retest all these different configurations and 
scenarios.

Here's what I'd like to propose for a direction:
1) Create an initiator driver and a target driver.  For now, initiator 
would support both SCSI and NVME initiator. Target would support SCSI 
and NVME target.
2) SLI3 support would be contained only within the initiator driver and 
limited to SCSI (as it is today in lpfc).
3) SLI4 support would be library-ized,so that the code can be shared 
between the two drivers.  Library-izing SLI-4 means SLI-3 will also be 
library-ized.
4) Discovery support would be librarized so it can be shared. As part of 
this effort we will minimally move generic functions from the library to 
drivers/scsi/libfc (example: setting RPA payloads, etc).  At this time, 
the drivers will not attempt to use libfc for discovery. There is too 
much sensitive code tied to interlocks with adapter api design that are 
visible in the discovery state machine. Use of libfc can be a future, 
but for the short term, the goal is a single library for the broadcom 
initiator/target drivers.
5) lpfc will be refactored, addressing concerns that have been desired 
for a while.


To start this effort, I'd like a bcmlpfc directory to be made within the 
drivers staging tree. The directory would be populated with the efct 
driver and a copy of the existing lpfc driver.   Work can then commence 
on refactoring lpfc and creating the libraries and integrating the 
libraries into both drivers.  As lpfc is updated in the main tree, 
patches would be posted to the staging version of lpfc to keep them on par.


Questions:
a) How best to deal with overlapping pci id's ?  E.g. if we do (1) and 
we have an initiator and target driver, there is a lot of adapters that 
are fully functional for target operation, but were sold as primarily an 
initiator adapter. How could we manage target mode enablement without 
code mod or hard pci id partitioning ?   I know individual pci 
unbind/bind could work, but its been frowned upon as a long term option. 
Same thing goes for module parameters to select which ports do what role.

b) Assuming we have the lpfc copy in the bcmlpfc directory in the 
staging tree: are there any issues with having a version of lpfc in the 
main tree and another in the staging tree ?     For many reasons, I'd 
like to keep the name lpfc on the initiator driver in the staging tree. 
But is that possible ? I assume we would need to develop in the staging 
tree as a new name and pci id space separate from the base driver, and 
we can rename the staging driver to the lpfc name when it merges into 
the main kernel and replaces the existing driver.

Thoughts ? Nic ?   James ?  Martin ?

-- james

Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[Christoph Hellwig]

Hi James,

I like the plan except for one bit:  The staging tree will be a major
pain due to the rules for applying to it and all the whitespace
cleanup junk you'll get.  I would storngly recommend to you to
do this with a non-mainline tree instead.  I'd be happy to help you
maintaining it if you want.

> Questions:
> a) How best to deal with overlapping pci id's ?  E.g. if we do (1) and we
> have an initiator and target driver, there is a lot of adapters that are
> fully functional for target operation, but were sold as primarily an
> initiator adapter. How could we manage target mode enablement without code
> mod or hard pci id partitioning ?   I know individual pci unbind/bind could
> work, but its been frowned upon as a long term option. Same thing goes for
> module parameters to select which ports do what role.

Let's start with a manual bind for the target driver.  A slightly
more fancy option would be to add the PCI IDs to both and reject
the device with -ENXIO if not configured for the right role.

Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[Johannes Thumshirn]

On 06/13/2017 01:08 AM, James Smart wrote:
> Here's what I'd like to propose for a direction:
> 1) Create an initiator driver and a target driver.  For now, initiator
> would support both SCSI and NVME initiator. Target would support SCSI
> and NVME target.
> 2) SLI3 support would be contained only within the initiator driver and
> limited to SCSI (as it is today in lpfc).
> 3) SLI4 support would be library-ized,so that the code can be shared
> between the two drivers.  Library-izing SLI-4 means SLI-3 will also be
> library-ized.
> 4) Discovery support would be librarized so it can be shared. As part of
> this effort we will minimally move generic functions from the library to
> drivers/scsi/libfc (example: setting RPA payloads, etc).  At this time,
> the drivers will not attempt to use libfc for discovery. There is too
> much sensitive code tied to interlocks with adapter api design that are
> visible in the discovery state machine. Use of libfc can be a future,
> but for the short term, the goal is a single library for the broadcom
> initiator/target drivers.

Good idea, but be aware that libfc unfortunately is using skbs
internally which I don't like, but didn't have the time to clean it up.

> 5) lpfc will be refactored, addressing concerns that have been desired
> for a while.

\o/

When doing you refactoring can you please have a look at the levels of
indent (not more than 3 if possible), no unnecessary camelCase, etc...

The cyclomatic complexity GCC plugin we have in scripts/gcc-plugins
could be of help here, though I only have read about it and never really
used myself.

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[Martin K. Petersen]

James,

> To start this effort, I'd like a bcmlpfc directory to be made within
> the drivers staging tree. The directory would be populated with the
> efct driver and a copy of the existing lpfc driver.  Work can then
> commence on refactoring lpfc and creating the libraries and
> integrating the libraries into both drivers.  As lpfc is updated in
> the main tree, patches would be posted to the staging version of lpfc
> to keep them on par.

Sounds good to me.

> a) How best to deal with overlapping pci id's ?  E.g. if we do (1) and
> we have an initiator and target driver, there is a lot of adapters
> that are fully functional for target operation, but were sold as
> primarily an initiator adapter. How could we manage target mode
> enablement without code mod or hard pci id partitioning ?  I know
> individual pci unbind/bind could work, but its been frowned upon as a
> long term option. Same thing goes for module parameters to select
> which ports do what role.

I don't have a problem with the binding approach. A bit easier for the
sysadmin to script and manage compared to PCI ID-specific module
parameters that may come with initramfs rebuilds or grub tweaks and
reboots.

I also think the binding approach eases that pain for systems that want,
say, initiator for root fs on one port and target on the other. And
gives you the flexibility to use the WWN or sysfs attributes other than
the PCI ID to locate the ports whose affiliation you want to change.

> b) Assuming we have the lpfc copy in the bcmlpfc directory in the
> staging tree: are there any issues with having a version of lpfc in the
> main tree and another in the staging tree ?     For many reasons, I'd
> like to keep the name lpfc on the initiator driver in the staging
> tree. But is that possible ? I assume we would need to develop in the
> staging tree as a new name and pci id space separate from the base
> driver, and we can rename the staging driver to the lpfc name when it
> merges into the main kernel and replaces the existing driver.

I'm with Christoph in terms of avoiding staging. It's better to do out
of tree. And as far as naming is concerned, you have some flexibility in
terms of string prefixes for printk's, module aliases, etc. And then a
final tweak to get things shuffled into place before merging into
mainline.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[Hannes Reinecke]

On 06/13/2017 01:08 AM, James Smart wrote:
> On 5/16/2017 12:59 PM, Roland Dreier wrote:
>> On Sun, Mar 5, 2017 at 8:35 AM, Sebastian Herbszt <herbszt@gmx.de> wrote:
>>> Just like Hannes I do favour integration. I guess it could be
>>> comparable to qla2xxx + tcm_qla2xxx, lpfc + lpfc_scst and
>>> lpfc + tcm_lpfc. That approach might even help Bart with his
>>> target driver unification if he didn't give up on that topic.
>> Resurrecting this old topic - sorry for not seeing this go by initially.
>>
>> For context, I have a lot of experience debugging the qla2xxx target
>> code - we did a lot of work to get error/exception paths correct.
>> Basic FC target support is pretty straightforward but handling SAN log
>> in / log out events and other strange things that initiators do took a
>> lot of effort.
>>
>> Anyway, my feeling is that the integration of tcm_qla2xxx and qla2xxx
>> was overall a net negative.  Having the target driver grafted onto the
>> side of an already-complex driver that has a bunch of code not
>> relevant to the target (SCSI error handling, logging into and timing
>> out remote target ports, etc) made the code harder to debug and harder
>> to get right.
>>
>> Of course I'm in favor of making common code really common.  So
>> certainly we should have a common library of SLI-4 code that both the
>> initiator and target driver share.  And if there is more commonality,
>> that's great.  But any code similar to "if (initiator) ... else ..."
>> is really suspect to me - grepping for "qla_ini_mode_enabled" shows
>> great examples like
>>
>> ...
>>
>> Handling "dual mode" (both initiator and target on the same port at
>> the same time) is a design challenge, but I don't think the current
>> qla2xxx driver is an example of a maintainable way to do that.
>>
>> (I'm agnostic about what to do about SLI-3 - perhaps the cleanest
>> thing to do is split the driver between SLI-4 and SLI-3, and handle
>> the initiator and target drivers for those two cases as appropriate)
>>
>> I'd love to discuss this further and come up with a design that meets
>> the concerns about integration but also learns the lessons from
>> tcm_qla2xxx.
>>
>>   - R.
> 
> 
> Thanks for the feedback.  I believe you echo many of our concerns as we
> look at "merging them into one".  I agree with your statements on the
> number of if-else roles and know that we made this even more complicated
> by the driver doing fc-nvme initiator and fc-nvme target as well.  Your
> small list of "mode_enabled" hits pales in comparison to a hit list in
> the current driver if looking for SCSI initiator support
> (LPFC_ENABLE_FCP), NVME initiator support (LPFC_ENABLE_NVME), or NVME
> target support (phba->nvmet_support). And that's before adding SCSI
> target support.   We're also concerned about the discovery engines as
> not only are there lots of different paths for the different roles as
> well as support for fcoe, but there are a lot of carefully managed
> accommodations for various oem and switch environments. It's very
> difficult to replicate and retest all these different configurations and
> scenarios.
> 
> Here's what I'd like to propose for a direction:
> 1) Create an initiator driver and a target driver.  For now, initiator
> would support both SCSI and NVME initiator. Target would support SCSI
> and NVME target.
Well, _actually_ you only would need to move the NVMe target
functionality into a new driver...

> 2) SLI3 support would be contained only within the initiator driver and
> limited to SCSI (as it is today in lpfc).
> 3) SLI4 support would be library-ized,so that the code can be shared
> between the two drivers.  Library-izing SLI-4 means SLI-3 will also be
> library-ized.
> 4) Discovery support would be librarized so it can be shared. As part of
> this effort we will minimally move generic functions from the library to
> drivers/scsi/libfc (example: setting RPA payloads, etc).  At this time,
> the drivers will not attempt to use libfc for discovery. There is too
> much sensitive code tied to interlocks with adapter api design that are
> visible in the discovery state machine. Use of libfc can be a future,
> but for the short term, the goal is a single library for the broadcom
> initiator/target drivers.
> 5) lpfc will be refactored, addressing concerns that have been desired
> for a while.
> 
That all sound reasonable.

> 
> To start this effort, I'd like a bcmlpfc directory to be made within the
> drivers staging tree. The directory would be populated with the efct
> driver and a copy of the existing lpfc driver.   Work can then commence
> on refactoring lpfc and creating the libraries and integrating the
> libraries into both drivers.  As lpfc is updated in the main tree,
> patches would be posted to the staging version of lpfc to keep them on par.
> 
Ok.

> 
> Questions:
> a) How best to deal with overlapping pci id's ?  E.g. if we do (1) and
> we have an initiator and target driver, there is a lot of adapters that
> are fully functional for target operation, but were sold as primarily an
> initiator adapter. How could we manage target mode enablement without
> code mod or hard pci id partitioning ?   I know individual pci
> unbind/bind could work, but its been frowned upon as a long term option.
> Same thing goes for module parameters to select which ports do what role.
> 
That indeed is a problem.

Ideally we should be able to set the required mode on a per-port base;
having it per PCI device might be too coarse. Unless you represent each
port as a PCI function; not sure if that's the case, though.
If we were to allow to set the mode on a per-port base we could easily
implement kernel parameters like fctarget=WWPN and/or fcinitiator=WWPN;
NVMe could be treated similarly.
And have a config option specifying if the default FC mode should be
initiator or target.
With that we could configure the ports during boot-up and would avoid
the rebinding.

> b) Assuming we have the lpfc copy in the bcmlpfc directory in the
> staging tree: are there any issues with having a version of lpfc in the
> main tree and another in the staging tree ?     For many reasons, I'd
> like to keep the name lpfc on the initiator driver in the staging tree.
> But is that possible ? I assume we would need to develop in the staging
> tree as a new name and pci id space separate from the base driver, and
> we can rename the staging driver to the lpfc name when it merges into
> the main kernel and replaces the existing driver.
> 
Why not splitting the _existing_ lpfc driver along the above lines, and
keep the new target mode driver in staging?
That would be 'just' be a refactoring of the current lpfc driver, which
would be fully in keeping with the existing rules.
Hence I don't think that it would require to have the initiator driver
in staging, too.
And I do think that staging drivers can use library function from
'mainline' drivers.

Unless you want to go for a full split, and remove SLI-4 functionality
from the existing lpfc driver.
But then you'll be having a issue with driver renaming etc.
Not sure if I would want to go that route.

Cheers,

Hannes

Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[Sebastian Herbszt]

Hannes Reinecke wrote:
> On 06/13/2017 01:08 AM, James Smart wrote:

[snip]

> > Questions:
> > a) How best to deal with overlapping pci id's ?  E.g. if we do (1)
> > and we have an initiator and target driver, there is a lot of
> > adapters that are fully functional for target operation, but were
> > sold as primarily an initiator adapter. How could we manage target
> > mode enablement without code mod or hard pci id partitioning ?   I
> > know individual pci unbind/bind could work, but its been frowned
> > upon as a long term option. Same thing goes for module parameters
> > to select which ports do what role. 
> That indeed is a problem.
> 
> Ideally we should be able to set the required mode on a per-port base;
> having it per PCI device might be too coarse. Unless you represent
> each port as a PCI function; not sure if that's the case, though.

It seems to be the case.
At least a dual port FC HBA (SLI-3) has two PCI functions.

> If we were to allow to set the mode on a per-port base we could easily
> implement kernel parameters like fctarget=WWPN and/or
> fcinitiator=WWPN; NVMe could be treated similarly.
> And have a config option specifying if the default FC mode should be
> initiator or target.

The old lpfc+lpfc_scst combination and also qla2xxx+tcm_qla2xxx allow
simultaneous initiator and target mode on the same port. I guess this
won't be possible with a split driver.

Sebastian

Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[Hannes Reinecke]

On 06/14/2017 10:02 PM, Sebastian Herbszt wrote:
> Hannes Reinecke wrote:
>> On 06/13/2017 01:08 AM, James Smart wrote:
> 
> [snip]
> 
>>> Questions:
>>> a) How best to deal with overlapping pci id's ?  E.g. if we do (1)
>>> and we have an initiator and target driver, there is a lot of
>>> adapters that are fully functional for target operation, but were
>>> sold as primarily an initiator adapter. How could we manage target
>>> mode enablement without code mod or hard pci id partitioning ?   I
>>> know individual pci unbind/bind could work, but its been frowned
>>> upon as a long term option. Same thing goes for module parameters
>>> to select which ports do what role. 
>> That indeed is a problem.
>>
>> Ideally we should be able to set the required mode on a per-port base;
>> having it per PCI device might be too coarse. Unless you represent
>> each port as a PCI function; not sure if that's the case, though.
> 
> It seems to be the case.
> At least a dual port FC HBA (SLI-3) has two PCI functions.
> 
>> If we were to allow to set the mode on a per-port base we could easily
>> implement kernel parameters like fctarget=WWPN and/or
>> fcinitiator=WWPN; NVMe could be treated similarly.
>> And have a config option specifying if the default FC mode should be
>> initiator or target.
> 
> The old lpfc+lpfc_scst combination and also qla2xxx+tcm_qla2xxx allow
> simultaneous initiator and target mode on the same port. I guess this
> won't be possible with a split driver.
> 
Which was kinda the point of this entire conversation ...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.com			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [ANNOUNCE]: Broadcom (Emulex) FC Target driver - efct

[Bart Van Assche]

On Mon, 2017-02-27 at 15:28 -0800, James Smart wrote:
> I'd like to announce the availability of the Broadcom (Emulex) FC Target 
> driver - efct.

Hello James,

Sorry but I have not yet had the time to do a full review. But I would like
to already share the following feedback:
* efct_io_find_tgt_io() looks up an I/O request in a list. The lookup is
  protected by a spinlock but callers are expected to increase the I/O
  request reference count after the lock has been released. This looks wrong
  to me and most likely means that there are race conditions related to I/O
  request lookup. Have you considered to move the kref_get() call into
  efct_io_find_tgt_io() such that it can occur with the spinlock held?
* If an ABTS is received then efct_process_abts() (indirectly) calls
  target_submit_tmr(). That last function may or may not decide to call
  efct_lio_queue_status(), depending on whether or not the TAS bit has been
  set. efct_lio_queue_status() sends back a SCSI response to the initiator
  (see also __transport_check_aborted_status() and
  transport_send_task_abort()). Is it allowed by the FC spec to send back a
  SCSI response after an ABTS has been received? Is that SCSI response sent
  before or after the BLS ABTS response?

Thanks,

Bart.