Changes look good.

In D34423#779963, @mav wrote:

It makes some sense to me. I suppose other cases could be NOP and BA_ACC/BA_REJ, which do not require action if unsolicited. I just guess incoming BA_* for outgoing ABTS are handled somewhere else, if they reach the driver at all.

In D34422#779953, @mav wrote:

I don't think I can properly review this change without diving in deeply. It just looks weird to me first print "shutting down sport" but then "p2p winner, domain already attached\n" and post OCS_EVT_DOMAIN_ATTACH_OK.

The changes look good.

In D16196#345939, @mav wrote:

The general motivation is good for me. Very alike approach is used in isp(4) driver, may be just for some additional reasons. But unless I missed something, I think this patch of yours is incomplete -- it implements the translation table for initiator role, but not for target. If the HBA is capable of playing both roles same time, there will be a mess in CAM and CTL. isp(4) driver originally also had the same issue, but I fixed it there few years ago, and now target and initiator roles may successfully coexist.

Fixed as per the review comments.

Thanks Ken. Changes look good.

1. Fixed 32-bit print format warnings.
2. Enabled building ocs_fc module for amd64 and i386.
3. get_cyclecount() to get the TSC value.

Added ocs_fc module to sys/modules/Makefile.
Removed unintended files under ocs_fc.

Increased max remote nodes/targets supported based on firmware resources.

Fixed issue in ABTS handling where lun number is not passed correctly.

Added NPIV support to ocs_fc driver.

Fixed issue in reporting the transfer speed.

Hi Ken,

In D11423#273617, @ram.vegesna_broadcom.com wrote:

Updated makefile as per the review commnets.

Updated makefile as per the review commnets.

Updated man page.

Added man page for ocs_fc driver.

1. Changed auto topology detection to try FC-AL first and failover to Nport topology.
2. Fixed issue of not sending REC for no data commands, by setting the Error Recovery bit in command wrb.

Thanks Ken. Sorry missed the auto-topology fix in this drop. I will update the code.
I will try to trigger the REC with the commands you provided, will update you the status.

1. Added support for live driver role change via CTL.
2. Removed some info prints.
3. Did some bug fixes.

In D11423#244184, @mav wrote:

In D11423#244162, @ram.vegesna_broadcom.com wrote:

In D11423#243919, @mav wrote:

As of now, we are not supporting live role change.

It should better be supported. I've tried to enable target mode, but found expected problems: If target mode enabled via device hints, it won't be able to reply to initiators until target mode is enabled in CTL, leading to command timeouts on initiators. Enabling target in CTL after makes it work, but it seems CTL doesn't receive or drops AC_CONTRACT notifications, as result of what ctladm portlist -i does not report connected initiators until they disconnect and reconnect.

I think if we support the live role change also this issue will happen. As enabling target mode in CTL should be done in any case. So, I think we need to debug why CTL doesn't receive or drops AC_CONTRACT notifications.
Please correct me if I am missing any thing.

I'm sorry, it seems I was wrong about AC_CONTRACT, it does work.

But I still see a problem in fact that target mode is not controller by CTL: when system boots and driver applies target mode tunable, there is no LUNs configured/enabled till ctld start. It makes initiators to see the target which does not respond on requests. I am not sure it is a nice behavior. in case of isp(4) driver FC link remains down until ctld enable the target role when all LUNs are ready. Could you describe how your approach supposed to work in case I miss something?

In D11423#240180, @mav wrote:

In D11423#240081, @ram.vegesna_broadcom.com wrote:

Sorry missed that, Is this what you intended here: I am still not updating the target id as I am not saving that value now, will add that code.

• ocs_cam.c (revision 40610)

+++ ocs_cam.c (working copy)
@@ -586,6 +586,7 @@

if ((lun < OCS_MAX_LUN) && ocs->targ_rsrc[lun].enabled) {
        trsrc = &ocs->targ_rsrc[lun];
} else if (ocs->targ_rsrc_wildcard.enabled) {

+ lun = CAM_LUN_WILDCARD;

        trsrc = &ocs->targ_rsrc_wildcard;
}

@@ -601,6 +602,9 @@

tmfio->tgt_io.app = abortio;

STAILQ_REMOVE_HEAD(&trsrc->inot, sim_links.stqe);

+
+ inot->ccb_h.status = CAM_CDB_RECVD;
+ inot->ccb_h.target_lun = lun;

inot->tag_id = tmfio->tag;

Only one last line would probably be enough. Previous one seems not from here.

In D11423#240075, @mav wrote:

In D11423#240042, @ram.vegesna_broadcom.com wrote:

4. Fixed sending correct target id when request handled by wildcard.

This is not what I was thinking about. When request is handled by wildcard LUN, CAM and CTL should receive real target and LUN IDs. As I understand, your new code still does not set target_lun value, leaving it wildcard.

By the way, looking wider, I've noticed that your code in ocs_fc_decode_lun() decodes 8-byte LUN into flat space. But there is no reason to do it now. Recent FreeBSD versions support full 8-byte LUNs if PIM_EXTLUNS flag is set. While CTL still does not make use of that, I was thinking about making it support LUN conglomerates at some point.

In D11423#237113, @ken wrote:

I already sent Ram email about this, from from a purely functional standpoint:

I have been able to get a tape drive / library to probe and run in point to point mode when the library / drive are directly attached to the card. (I have not tried going through a switch.) This is with a Spectra Logic T120 exported via an IBM LTO-5 drive.

I have found several issues with the latest code:

1. The tape library doesn’t probe in fabric mode when it is direct attached to the card. The driver defaults to fabric mode from what I can tell. Instead, the driver just bounces the link up and down (apparently), but from the traces, there are just 16Gb training sequences going over the wire from the initiator. The tape drive is 8Gb, so nothing really happens. I sent Ram two traces, one from a case where the initiator is just doing a training sequence, and the other from where the tape drive tries to do a FLOGI and then nothing happens after that. In both cases the driver is reporting that the link is going up and down.

2. Attempting to probe in loop mode (hint.ocs_fc.0.topology=3) when direct attached to the library yields a panic:

ocs_fc0: <Emulex LightPulse FC Adapter> mem 0xde418000-0xde41ffff,0xde420000-0xde420fff irq 32 at device 0.0 on pci2
OCS FC Driver (0.0.9999.0) Copyright (C) 2017 Broadcom. The term ?Broadcom? refers to Broadcom Limited and/or its subsidiaries.
ocs_fc0: Setting topology=0x3
ocs_fc: sli_get_config:3890:ocs_fc0:FW1:11.2.156.27 (2e313536) / FW2:11.2.156.27 (2e313536)
ocs_fc: sli_get_config:3893:ocs_fc0:HW1: 0000000b / HW2: 00000010
ocs_fc: ocs_xport_initialize:455:ocs_fc0:Emulex LightPulse FC Adapter: Can't set the toplogy
ocs_fc0: ocs_pci_attach: failed to initialize transport object
panic: XXX trying to free with un-initialized mtx!?!?

cpuid = 5
time = 1499104391
KDB: stack backtrace:
db_trace_self_wrapper() at 0xffffffff803756bb = db_trace_self_wrapper+0x2b/frame 0xfffffe08b3f58350
vpanic() at 0xffffffff8095569c = vpanic+0x19c/frame 0xfffffe08b3f583d0
panic() at 0xffffffff80955723 = panic+0x43/frame 0xfffffe08b3f58430
ocs_xport_free() at 0xffffffff827b4aa2 = ocs_xport_free+0xa2/frame 0xfffffe08b3f58460
ocs_pci_attach() at 0xffffffff8277f3bc = ocs_pci_attach+0x125c/frame 0xfffffe08b3f584d0
device_attach() at 0xffffffff80989a2e = device_attach+0x3de/frame 0xfffffe08b3f58510
pci_driver_added() at 0xffffffff8065fea9 = pci_driver_added+0xe9/frame 0xfffffe08b3f58550
devclass_driver_added() at 0xffffffff809878ed = devclass_driver_added+0x7d/frame 0xfffffe08b3f58590
devclass_add_driver() at 0xffffffff80987814 = devclass_add_driver+0x144/frame 0xfffffe08b3f585d0
module_register_init() at 0xffffffff8093a2c0 = module_register_init+0xc0/frame 0xfffffe08b3f58600
linker_load_module() at 0xffffffff8092d788 = linker_load_module+0xba8/frame 0xfffffe08b3f58910
kern_kldload() at 0xffffffff8092edc9 = kern_kldload+0xa9/frame 0xfffffe08b3f58950
sys_kldload() at 0xffffffff8092ee8b = sys_kldload+0x5b/frame 0xfffffe08b3f58980
amd64_syscall() at 0xffffffff80db1f69 = amd64_syscall+0x549/frame 0xfffffe08b3f58ab0
Xfast_syscall() at 0xffffffff80d92ecb = Xfast_syscall+0xfb/frame 0xfffffe08b3f58ab0

• syscall (304, FreeBSD ELF64, sys_kldload), rip = 0x800871a9a, rsp = 0x7fffffffde98, rbp = 0x7fffffffe410 ---

KDB: enter: panic
[ thread pid 9224 tid 102778 ]
Stopped at 0xffffffff80996fcb = kdb_enter+0x3b: movq $0,0xffffffff81887780 = kdb_why

3. Probing the tape library in point to point mode (hint.ocs_fc.0.topology=2) does work. I can talk to the drive just fine. The FC-Tape flags are turned on in the PRLI. Confirmed Completion is requested by the initiator, so the tape drive is sending completion confirmations. However, the CRN is not getting set; it is 0. That is another component of FC-tape, and needed to make sure commands don’t get out of order. Do you have SRR support? Will the driver send a REC if the tape drive takes too long to respond to a command?

I have confirmed that with the isp(4) driver, unloading the driver when you’ve created CTL LUNs with the driver loaded hangs. So the ocs_fc driver is no different in that regard. It's something that we need to fix with CTL in general.

Is there a way to get the board to start off in point to point mode and fall back to loop?

Starting off in fabric mode with no apparent fallback will likely yield some confused users who don’t know why their tape library isn’t attaching.

I haven’t gotten traces of target mode talking to the isp(4) driver yet.

Hopefully this gives something to go on for now. In summary:

1. We need to make sure that tape drives probe out of the box, whether direct or fabric attached.
2. Loop mode shouldn't cause a panic.
3. The CRN needs to be set properly, and SRR and REC should also be properly supported.

Fixed following issues as per the review comments:

In D11423#237221, @mav wrote:

I am happy to see FreeBSD getting one more FC driver. Thank you!

Without being able to try this code (the only Emulex cards I have now are ancient 9000-series which seems not supported here) I've added some comments based on very quick look. If you have some spare card samples for developers, I would happily get one to be able to test/improve CAM/CTL compatibility with it in my further work.