Fiber channel driver for Broadcom/Emulex FC host bus adapters.

[ram]

Summary

First cut of ocs_fc driver.

[ram]

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.

Hi Ken,

We support following topologies:

hint.ocs_fc.X.topology: FC topology:
• 0 – auto-select; first tries N_Port, then loop. (default)
• 1 – N_Port (point-to-point)
• 2 – loop

N_Port topology covers both Fabric and point-to-point mode. Driver decides the Port type based on the PLOGI response.

In your test (hint.ocs_fc.0.topology=2) means loop topology. So, Tape drive attach worked with loop topology.

Panic with hint.ocs_fc.0.topology=3: Driver initialization failed specifying "unsupported Topology". I fixed the driver panic here.

Yes, we have SRR and REC support. Our Firmware will do REC and SRR and perform the timers. I missed a change here to update the FW that FCP Error Recovery Protocol should be performed. Fixed the issue.

Regarding CRN, After discussing internally in our team, we thought of ignoring it as only few targets support it. Please let know if ignoring may cause any issue.

Task Retry ID - which is used by sequence level error recovery is automatically stamped by our Firmware.

Regarding Tape drive failure with default topology, From the trace I am not able to find the root cause for link toggle.

Thanks,
Ram

[ram]

sys/dev/ocs_fc/ocs_cam.c

1691 ↗

(On Diff #30250)

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

1986 ↗

(On Diff #30250)

I will submit the indentation changes in the next update.

sys/dev/ocs_fc/ocs_device.c

1741 ↗

(On Diff #30250)

Replaced GID FT with GIT PT to support this case.

[mav]

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.

[ram]

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.

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.

I will do the changes to support EXTLUNS.

• 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;

[mav]

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.

[mav]

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.

[mav]

sys/dev/ocs_fc/ocs_cam.c

790 ↗

(On Diff #30774)

This and above/below looks like leaked debugging.

2154 ↗

(On Diff #30774)

I suppose here should be called ocs_hw_flush() to allow kernel dumping on FC storage.

2274 ↗

(On Diff #30774)

This is overly verbose for normal operation.

[ram]

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#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.

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.

[mav]

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?

[ram]

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?

Yes, you are right. I got it. I will do the necessary changes.
Our approach worked until now because I used to load driver, create LUNs and enable ctl port using script, So initiators used to work with out any issue.

[ram]

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

[ken]

I tried this out, and this breaks automatic tape drive recognition if I haven't set the topology to 2 (loop mode). If I set the topology to 2, it sees the tape drive without a problem.

I'm looking at traces, and it still looks like REC isn't getting sent. You can trigger a REC by doing a 'mt fsf' to go to the next filemark, if the next filemark is far enough away. e.g.:

dd if=big_file bs=1m | dd of=/dev/nsa0 bs=128k
mt rewind
mt fsf

I do see that REC is getting sent in the trace you sent me on October 10th. I don't see the FC setup, though, so I don't know whether your initiator / tape drive are doing the same thing as mine.

[ram]

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.

In D11423#270158, @ken wrote:

I tried this out, and this breaks automatic tape drive recognition if I haven't set the topology to 2 (loop mode). If I set the topology to 2, it sees the tape drive without a problem.

I'm looking at traces, and it still looks like REC isn't getting sent. You can trigger a REC by doing a 'mt fsf' to go to the next filemark, if the next filemark is far enough away. e.g.:

dd if=big_file bs=1m | dd of=/dev/nsa0 bs=128k
mt rewind
mt fsf

I do see that REC is getting sent in the trace you sent me on October 10th. I don't see the FC setup, though, so I don't know whether your initiator / tape drive are doing the same thing as mine.

[ram]

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.

[ken]

The new diffs fixed both issues: auto negotiation with tape drives and sending RECs for non-data commands. I got a trace to confirm that RECs are getting sent during a long-running SPACE command. It looks like the card sends a REC every 4 seconds, which should be fine. (QLogic seems to send them every 3 seconds. I don't think the spec mandates a specific time.)

I'll try to test target mode a little bit today. One other thing that needs to get done is write a man page for the driver. The man page should cover the following topics at least:

1. General description of the driver.
2. What hardware it works with.
3. Driver kernel configuration options (if any).
4. Loader tunables and sysctl variables. Include information on what the different settings do.

For examples of device driver man pages, see:

share/man/man4/isp.4
share/man/man4/mpr.4
share/man/man4/mps.4

One thing to remember when writing man pages is that every sentence should start on a new line.

[ram]

Added man page for ocs_fc driver.

[ram]

Updated man page.

[wblock]

share/man/man4/ocs_fc.4

49 ↗

(On Diff #35188)

This whole introductory section is the old style with halting and confusing wording. Please see https://www.freebsd.org/doc/en_US.ISO8859-1/books/fdp-primer/manpages-sample-structures.html#manpages-sample-structures-section-4 for a suggested newer style.

52 ↗

(On Diff #35188)

s/FibreChannel/Fibre Channel/

57 ↗

(On Diff #35188)

Use the serial comma: s/Point-to-Point/Point-to-Point,/

61 ↗

(On Diff #35188)

Nonspecific "It" here... probably means FC-Tape and would be clearer just saying that. Also, "encompasses" might be a technical term here, but would probably be clearer with just "includes":

FC-Tape includes four elements from the T-10 FCP-4 specification:

75 ↗

(On Diff #35188)

Another non-specific "it". Does this refer to "link level error recover"?

76 ↗

(On Diff #35188)

Use the serial comma: s/part/part,/

78 ↗

(On Diff #35188)

Too many "it"s.

FC-Tape is automatically enabled when both the controller and target support it."

84 ↗

(On Diff #35188)

s/Driver/driver/
s/the following/these/
s/FibreChannel/Fibre Channel/

88 ↗

(On Diff #35188)

These would probably look better without the periods.

93 ↗

(On Diff #35188)

As above, periods are probably not needed.

100 ↗

(On Diff #35188)

s/the steps below/these steps/

103 ↗

(On Diff #35188)

Copy this code to a
.Pa Makefile :

110 ↗

(On Diff #35188)

Replace
.Pa imagename
with the name of the GRP file.

112 ↗

(On Diff #35188)

Copy the
.Pa Makefile
and GRP file to a local directory

114 ↗

(On Diff #35188)

Execute
.Cm make
and copy the generated
.Pa ocsflash.ko
file to

117 ↗

(On Diff #35188)

I think this also needs .Cm

125 ↗

(On Diff #35188)

Options are controlled by setting values in

131 ↗

(On Diff #35188)

This type of thing is usually clearer to read when the items are just the description rather than a full sentence. For example,

Enable initiator functionality.
Default 1 (enabled), 0 to disable.

It's not clear to me whether "hint" is necessary or useful.

134 ↗

(On Diff #35188)

Enable target functionality.
Default 1 (enabled), 0 to disable.

137 ↗

(On Diff #35188)

Topology: 0 for Auto, 1 for NPort only, 2 for Loop only.

139 ↗

(On Diff #35188)

As above, but also please start new sentences on new lines.

Link speed in megabits per second.
Possible values include:

140 ↗

(On Diff #35188)

Needs ending period.

145 ↗

(On Diff #35188)

As above: skip the introductory part to make it a sentence, and just give the description the reader is looking for anyway. Also, start sentences on new lines.
Not sure about the parameters. Ideally, we avoid showing stuff in quotes to avoid the reader having to ask whether the quotes are part of the parameter.

Port state (read/write).
Valid values are
.Li online
and
.Li offline .

147 ↗

(On Diff #35188)

As above.

149 ↗

(On Diff #35188)

As above.

151 ↗

(On Diff #35188)

Firmware revision (read-only).

153 ↗

(On Diff #35188)

Adapter serial number (read-only).

155 ↗

(On Diff #35188)

As above.

157 ↗

(On Diff #35188)

As above.

159 ↗

(On Diff #35188)

As above.

161 ↗

(On Diff #35188)

As above.

[ken]

Another thing to note here is that the man page has replaced the entire set of diffs that included the code. Both need to be included in the diffs, and the man page hooked up to the build.

[ram]

Updated makefile as per the review commnets.

[ram]

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

Updated makefile as per the review commnets.

Thanks Warren for the review.

Hi Ken,
Sorry , I messed the diffs update. I uploaded all the code now. Please let me know if you want me to create a new revision.

Thanks ,
Ram

[ken]

One thing that needs to get fixed is cosmetic. The reported speeds are 10X what they should be.

da45 at ocs_fc0 bus 0 scbus11 target 2 lun 4
da45: <FREEBSD CTLDISK 0001> Fixed Direct Access SPC-5 SCSI device
da45: Serial Number MYSERIAL0004
da45: 16000.000MB/s transfers WWNN 0x2000000e1e222a31 WWPN 0x2100000e1e222a31 Po
rtID 0xef
da45: Command Queueing enabled
da45: 100000MB (204800000 512 byte sectors)
da44 at ocs_fc0 bus 0 scbus11 target 2 lun 3
da44: <FREEBSD CTLDISK 0001> Fixed Direct Access SPC-5 SCSI device
da44: Serial Number MYSERIAL0003
da44: 16000.000MB/s transfers WWNN 0x2000000e1e222a31 WWPN 0x2100000e1e222a31 PortID 0xef
da44: Command Queueing enabled

sa0 at ocs_fc1 bus 0 scbus12 target 0 lun 0
sa0: <IBM ULTRIUM-HH5 G9N1> Removable Sequential Access SPC-4 SCSI device
sa0: Serial Number 101500520A
sa0: 8000.000MB/s transfers WWNN 0x21150090a500520a WWPN 0x21150090a500520a PortID 0x26
sa0: Command Queueing enabled
ch0 at ocs_fc1 bus 0 scbus12 target 0 lun 1
ch0: <SPECTRA PYTHON 2000> Removable Changer SCSI-3 device
ch0: Serial Number 921400520A
ch0: 8000.000MB/s transfers WWNN 0x21150090a500520a WWPN 0x21150090a500520a PortID 0x26
ch0: 10 slots, 1 drive, 1 picker, 1 portal

[ram]

Hi Ken,

Please clarify,

"da44: 16000.000MB/s transfers"
should be
"da44: 16000.000Mb/s transfers" --> This seems correct to me but camcontrol utility always prints in MB/s transfers.
or
"da44: 1600.000MB/s transfers" --> Do you want me to report this speed?

Thanks,
Ram

In D11423#281162, @ken wrote:

One thing that needs to get fixed is cosmetic. The reported speeds are 10X what they should be.

da45 at ocs_fc0 bus 0 scbus11 target 2 lun 4
da45: <FREEBSD CTLDISK 0001> Fixed Direct Access SPC-5 SCSI device
da45: Serial Number MYSERIAL0004
da45: 16000.000MB/s transfers WWNN 0x2000000e1e222a31 WWPN 0x2100000e1e222a31 Po
rtID 0xef
da45: Command Queueing enabled
da45: 100000MB (204800000 512 byte sectors)
da44 at ocs_fc0 bus 0 scbus11 target 2 lun 3
da44: <FREEBSD CTLDISK 0001> Fixed Direct Access SPC-5 SCSI device
da44: Serial Number MYSERIAL0003
da44: 16000.000MB/s transfers WWNN 0x2000000e1e222a31 WWPN 0x2100000e1e222a31 PortID 0xef
da44: Command Queueing enabled

sa0 at ocs_fc1 bus 0 scbus12 target 0 lun 0
sa0: <IBM ULTRIUM-HH5 G9N1> Removable Sequential Access SPC-4 SCSI device
sa0: Serial Number 101500520A
sa0: 8000.000MB/s transfers WWNN 0x21150090a500520a WWPN 0x21150090a500520a PortID 0x26
sa0: Command Queueing enabled
ch0 at ocs_fc1 bus 0 scbus12 target 0 lun 1
ch0: <SPECTRA PYTHON 2000> Removable Changer SCSI-3 device
ch0: Serial Number 921400520A
ch0: 8000.000MB/s transfers WWNN 0x21150090a500520a WWPN 0x21150090a500520a PortID 0x26
ch0: 10 slots, 1 drive, 1 picker, 1 portal

[ken]

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

Hi Ken,

Please clarify,

"da44: 16000.000MB/s transfers"
should be
"da44: 16000.000Mb/s transfers" --> This seems correct to me but camcontrol utility always prints in MB/s transfers.
or
"da44: 1600.000MB/s transfers" --> Do you want me to report this speed?

Yes, the third one is what we need to report. The numbers printed are in mebibytes/sec (2^20 bytes/sec). Back when we wrote it, the name "mebibytes" and the MiB abbreviation didn't exist. :) So a tape drive attached to a Qlogic controller and running at 8Gb, for example, shows:

sa0 at isp0 bus 0 scbus2 target 0 lun 0
sa0: <IBM ULTRIUM-HH5 G9N1> Removable Sequential Access SPC-4 SCSI device
sa0: Serial Number 101300520A
sa0: 800.000MB/s transfers WWNN 0x21130090a500520a WWPN 0x21130090a500520a PortID 0xef

[ram]

Fixed issue in reporting the transfer speed.

[ram]

Added NPIV support to ocs_fc driver.

[ram]

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

[ram]

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

[ram]

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

[ken]

So found some problems when I did a 'make universe'. See the inline comments for more details.

I don't think you need to fix everything, but it would be good to have the driver compiling on 32-bit and 64-bit little endian platforms. We can just default to building the module on i386 and amd64 for now. I don't expect anyone to run it on actual i386 hardware, but building it there will make sure that we continue to keep it building in 32-bit mode.

sys/modules/Makefile

296 ↗

(On Diff #40489)

This is good, but it does cause several problems:

1. The driver doesn't build on 32-bit platforms because of printf format warnings.
2. The driver doesn't build on big-endian platforms because the headers (sli4.h, ocs_fcp.h) don't have big endian versions defined.
3. The driver doesn't build on non-Intel platforms because it uses assembly to get at the TSC value. Use get_cyclecount(9) instead.

So, I think what I would suggest here is:

1. Don't both fixing the big endian problems right now.
2. Do fix the 32-bit printf format warnings that keep it from compiling.
3. Use get_cyclecount().
4. Just enable building the module for amd64 and i386. People can enable it for arm64 later if they're able to test it. (Or if you're able.)

[ram]

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.

[ken]

Looks good. This passes make universe now. I'm working on getting it into the tree.