Use xpt_path_sbuf() in few drivers

xpt_path_string() is now a wrapper around xpt_path_sbuf(). Using it
to than concatenate result to another sbuf makes no sense. Just call
xpt_path_sbuf() directly

Use xpt_path_sbuf() in few drivers

xpt_path_string() is now a wrapper around xpt_path_sbuf(). Using it
to than concatenate result to another sbuf makes no sense. Just call
xpt_path_sbuf() directly.

MFC after: 1 month

show more ...

sys: Remove $FreeBSD$: one-line .c pattern

Remove /^[\s*]*__FBSDID\("\$FreeBSD\$"\);?\s*\n/

mpr: Fix minor 'typos' comment

moving -> removing (we're removing the device)
CAM_REQ_CMO_ERROR -> CAM_REQ_ERR (the former isn't a thing)

Sponsored by: Netflix

mpr/mps: when sending reset on removal, include target in message

It's possible for muliple drives to be departing at the same time (if
the common power rail the share goes dark, for example). To un

mpr/mps: when sending reset on removal, include target in message

It's possible for muliple drives to be departing at the same time (if
the common power rail the share goes dark, for example). To understand
what's going on better, include target and handle in the messages
announcing the reset to allow matching with other corresponding events.

MFC After: 3 days
Sponsored by: Netflix
Reviewed by: mav
Differential Revision: https://reviews.freebsd.org/D35092

show more ...

mpr/mps: Fix a race in diagnostic reset

There's a small race in freezing the simq when performing a diagnostic
reset. During this time, a transaction can slip through and encounter
the target id of

mpr/mps: Fix a race in diagnostic reset

There's a small race in freezing the simq when performing a diagnostic
reset. During this time, a transaction can slip through and encounter
the target id of 0. If we're still in diagnostic reset when we detect
this, return a CAM_DEVICE_NOT_THERE status. Instead, freeze the queue
and return a requeue status, similar to what we do when we're resetting
a target and a transaction get here. The race is unavoidable due to
separate locks for queue and SIM, but easy enough to detect and make
harmless.

Sponsored by: Netflix
Reviewed by: scottl, mav
Differential Revision: https://reviews.freebsd.org/D34017

show more ...

mpr/mps: Remove write-only flag and callout

The discovery callout is initialized and cancelled only, making it
write-only. Remove a state flag associated with it being pending as well
as two defines

mpr/mps: Remove write-only flag and callout

The discovery callout is initialized and cancelled only, making it
write-only. Remove a state flag associated with it being pending as well
as two defines that aren't used that are associated with it. Remove
MP?SAS_SHUTDOWN flag, which is unused.

Sponsored by: Netflix
Reviewed by: ken, scottl, mav
Differential Revision: https://reviews.freebsd.org/D33925

show more ...

Fix "set but not used" warnings in the mpr driver. This fixes a minor
bug in error handling.

mpr: fix freeze / release mismatch in timeout code

So, if we're processing a timeout, and we've sent an ABORT to the
firmware for that timeout, but not yet received the response from the
firmware, A

mpr: fix freeze / release mismatch in timeout code

So, if we're processing a timeout, and we've sent an ABORT to the
firmware for that timeout, but not yet received the response from the
firmware, AND we get another timeout, we queue the timeout and freeze
the queue. However, when we've finally processed them all, we only
release the queue once. This causes all I/O to halt as the devq remains
frozen forever.

Instead, only freeze the queue when we start the process (eg set INRESET
on the target). This will allow the release when all the timed out I/Os
have finished ABORTing.

Sponsored by: Netflix
Reviewed by: mav
Differential Revision: https://reviews.freebsd.org/D33054

show more ...

mpr: Minor formatting changes to match mps.

Minor reformatting nits to make mprsas_scsiio_timeout match
mpssas_scsiio_timeout more closely. The differences aren't necessary and
are distracting when

mpr: Minor formatting changes to match mps.

Minor reformatting nits to make mprsas_scsiio_timeout match
mpssas_scsiio_timeout more closely. The differences aren't necessary and
are distracting when comparing the routines. No functional changes.

Sponsored by: Netflix

show more ...

mps/mpr(4): Move xpt_register_async() out of lock.

It fixes lock ordere reversal between SIM and device locks. Also
remove registration for AC_FOUND_DEVICE, unused for a while now.

MFC after: 1 mo

mps/mpr(4): Move xpt_register_async() out of lock.

It fixes lock ordere reversal between SIM and device locks. Also
remove registration for AC_FOUND_DEVICE, unused for a while now.

MFC after: 1 month

show more ...

mpr(4): Fix unmatched devq release.

Before this change devq was frozen only if some command was sent to
the target after reset started, but release was called always. This
change freezes the devq i

mpr(4): Fix unmatched devq release.

Before this change devq was frozen only if some command was sent to
the target after reset started, but release was called always. This
change freezes the devq immediately, leaving mprsas_action_scsiio()
check only to cover race condition due to different lock devq use.

This should also avoid unnecessary requeue of the commands, creating
additional log noise and confusing some broken apps.

MFC after: 2 weeks
Sponsored by: iXsystems, Inc.

show more ...

mpr(4): Handle mprsas_alloc_tm() errors on device removal.

SAS9305-16e with firmware 16.00.01.00 report HighPriorityCredit of
only 8, while for comparison some other combinations I have report
100 o

mpr(4): Handle mprsas_alloc_tm() errors on device removal.

SAS9305-16e with firmware 16.00.01.00 report HighPriorityCredit of
only 8, while for comparison some other combinations I have report
100 or even 128. In case of large JBOD detach requirement to send
target reset command to each target same time overflows the limit,
and without adequate handling makes devices stuck in half-detached
state, preventing later re-attach.

To handle that in case of allocation error mark the target with new
MPRSAS_TARGET_TOREMOVE flag, and retry the removal attempt next time
something else free high priority command. With this patch I can
successfully detach/attach 102 disk JBOD from/to the SAS9305-16e.

MFC after: 2 weeks
Sponsored by: iXsystems, Inc.

show more ...

Fix mpr(4) and mps(4) state transitions and a use-after-free panic.

When the mpr(4) and mps(4) drivers probe a SATA device, they issue an
ATA Identify command (via mp{s,r}sas_get_sata_identify()) be

Fix mpr(4) and mps(4) state transitions and a use-after-free panic.

When the mpr(4) and mps(4) drivers probe a SATA device, they issue an
ATA Identify command (via mp{s,r}sas_get_sata_identify()) before the
target is fully setup in the driver. The drivers wait for completion of
the identify command, and have a 5 second timeout. If the timeout
fires, the command is marked with the SATA_ID_TIMEOUT flag so it can be
freed later.

That is where the use-after-free problem comes in. Once the ATA
Identify times out, the driver sends a target reset, and then frees any
identify commands that have timed out. But, once the target reset
completes, commands that were queued to the drive are returned to the
driver by the controller.

At that point, the driver (in mp{s,r}_intr_locked()) looks up the
command descriptor for that particular SMID, marks it CM_STATE_BUSY and
sends it on for completion handling.

The problem at this stage is that the command has already been freed,
and put on the free queue, so its state is CM_STATE_FREE. If INVARIANTS
are turned on, we get a panic as soon as this command is allocated,
because its state is no longer CM_STATE_FREE, but rather CM_STATE_BUSY.

So, the solution is to not free ATA Identify commands that get stuck
until they actually return from the controller. Hopefully this works
correctly on older firmware versions. If not, it could result in
commands hanging around indefinitely. But, the alternative is a
use-after-free panic or assertion (in the INVARIANTS case).

This also tightens up the state transitions between CM_STATE_FREE,
CM_STATE_BUSY and CM_STATE_INQUEUE, so that the state transitions happen
once, and we have assertions to make sure that commands are in the
correct state before transitioning to the next state. Also, for each
state assertion, we print out the current state of the command if it is
incorrect.

mp{s,r}.c: Add a new sysctl variable, dump_reqs_alltypes,
that controls the behavior of the dump_reqs sysctl.
If dump_reqs_alltypes is non-zero, it will dump
all commands, not just the commands that are in the
CM_STATE_INQUEUE state. (You can see the commands
that are in the queue by using mp{s,r}util debug
dumpreqs.)

Make sure that the INQUEUE -> BUSY state transition
happens in one place, the mp{s,r}_complete_command
routine.

mp{s,r}_sas.c: Make sure we print the current command type in
command state assertions.

mp{s,r}_sas_lsi.c:
Add a new completion handler,
mp{s,r}sas_ata_id_complete. This completion
handler will free data allocated for an ATA
Identify command and free the command structure.

In mp{s,r}_ata_id_timeout, do not set the command
state to CM_STATE_BUSY. The command is still in
queue in the controller. Since we were blocking
waiting for this command to complete, there was
no completion handler previously. Set the
completion handler, so that whenever the command
does come back, it will get freed properly.

Do not free ATA Identify commands that have timed
out in mp{s,r}sas_add_device(). Wait for them
to actually come back from the controller.

mp{s,r}var.h: Add a dump_reqs_alltypes variable for the new
dump_reqs_alltypes sysctl.

Make sure we print the current state for state
transition asserts.

This was tested in the Spectra Logic test bed (as described in the
review), as well Netflix's Open Connect fleet (where panics dropped from
a dozen or two a month to zero).

Reviewed by: imp@ (who is handling the commit with ken's OK)
Sponsored by: Spectra Logic
Differential Revision: https://reviews.freebsd.org/D25476

show more ...

mpr, mps: clear CCBs allocated on the stack

Reviewed By: imp
Sponsored by: NetApp, Inc.
Sponsored by: Klara, Inc.
Differential Revision: https://reviews.freebsd.org/D30301

mpr: big-endian support

This fixes mpr driver on big-endian devices.
Tested on powerpc64 and powerpc64le targets using a SAS9300-8i card
(LSISAS3008 pci vendor=0x1000 device=0x0097)

Submitted by: A

mpr: big-endian support

This fixes mpr driver on big-endian devices.
Tested on powerpc64 and powerpc64le targets using a SAS9300-8i card
(LSISAS3008 pci vendor=0x1000 device=0x0097)

Submitted by: Andre Fernando da Silva <andre.silva@eldorado.org.br>
Reviewed by: luporl, alfredo, Sreekanth Reddy <sreekanth.reddy@broadcom.com> (by email)
Sponsored by: Eldorado Research Institute (eldorado.org.br)
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D25785

show more ...

Introduce support of SCSI Command Priority.

SAM-3 specification introduced concept of Task Priority, that was renamed
to Command Priority in SAM-4, and supported by all modern SCSI transports.
It pr

Introduce support of SCSI Command Priority.

SAM-3 specification introduced concept of Task Priority, that was renamed
to Command Priority in SAM-4, and supported by all modern SCSI transports.
It provides 15 levels of relative priorities: 1 - highest, 15 - lowest and
0 - default. SAT specification for SATA devices translates priorities 1-3
into NCQ high priority.

This change adds new "priority" field into empty spots of struct ccb_scsiio
and struct ccb_accept_tio of CAM and struct ctl_scsiio of CTL. Respective
support is added into iscsi(4), isp(4), mpr(4), mps(4) and ocs_fc(4) drivers
for both initiator and where applicable target roles. Minimal support was
added to CTL to receive the priority value from different frontends, pass it
between HA controllers and report in few places.

This patch does not add consumers of this functionality, so nothing should
really change yet, since the field is still set to 0 (default) on initiator
and not actively used on target. Those are to be implemented separately.

I've confirmed priority working on WD Red SATA disks connected via mpr(4)
and properly transferred to CTL target via iscsi(4), isp(4) and ocs_fc(4).

While there, added missing tag_action support to ocs_fc(4) initiator role.

MFC after: 1 month
Relnotes: yes
Sponsored by: iXsystems, Inc.

show more ...

mpr: clean up empty lines in .c and .h files

MFH

Sponsored by: Rubicon Communications, LLC (netgate.com)

Merge ^/head r363739 through r363986.

Remove extra memset() left after r342388.

This memset() wiped MPI2_FUNCTION_SCSI_TASK_MGMT set by mprsas_alloc_tm(),
that broke target reset on device removal, making later re-insertion into
the sam

Remove extra memset() left after r342388.

This memset() wiped MPI2_FUNCTION_SCSI_TASK_MGMT set by mprsas_alloc_tm(),
that broke target reset on device removal, making later re-insertion into
the same slot impossible, since firmware was still waiting for the driver
to finish with the removed device.

MFC after: 1 week
Sponsored by: iXsystems, Inc.

show more ...

MFH

Sponsored by: Rubicon Communications, LLC (netgate.com)

Merge ^/head r363583 through r363738.

mpr(4), mps(4): Stop checking for failures from malloc(M_WAITOK).

PR: 240545
Submitted by: Andrew Reiter <arr@watson.org>
Reviewed by: imp
MFC after: 1 week
Differential Revision: https://reviews.f

mpr(4), mps(4): Stop checking for failures from malloc(M_WAITOK).

PR: 240545
Submitted by: Andrew Reiter <arr@watson.org>
Reviewed by: imp
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D25766

show more ...

Increase buffer in mprsas_log_command() from 192 to 224 bytes.

192 bytes are not enough to print long commands, such as ATA COMMAND PASS
THROUGH(16), that makes debug output difficult to read.

MFC

Increase buffer in mprsas_log_command() from 192 to 224 bytes.

192 bytes are not enough to print long commands, such as ATA COMMAND PASS
THROUGH(16), that makes debug output difficult to read.

MFC after: 2 weeks
Sponsored by: iXsystems, Inc.

show more ...

Merge ^/head r358269 through r358399.