The SanDisk SD8SB8U1 and likely others pad their serial number with
spaces on the end rather than the start (at least when connected to a
SAS3008). This makes them difficult to wire unit numbers to with the
serial because you have to specify the trailing spaces. Instead, strip
out the trailing spaces.
We already strip leading spaces both here. In addition, when glabel
creates the devfs device nodes, leading and trailing spaces are removed
already (so there will be no change there either).
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D32684
This turns debugging printf() into a KASSERT().
Reviewed By: imp
Sponsored by: NetApp, Inc.
Sponsored by: Klara, Inc.
Differential Revision: https://reviews.freebsd.org/D31523
If some device is plugged back in after unplug before the probe periph
destroyed, it will just restart the probe process. But I've found that
PROBE_INQUIRY_CKSUM flag not cleared between the iterations may cause
AC_FOUND_DEVICE not reported on the second iteration, and because of
AC_LOST_DEVICE reported during the first iteration, the device end up
configured, but without any periphs attached.
We've found that enabled serial console and 102-disk JBOD cause enough
probe delays to easily trigger the issue for half of the disks. This
change fixes it reliably on my tests.
MFC after: 2 weeks
Sponsored by: iXsystems, Inc.
No functional changes. Do not MFC this, it changes kernel ABI.
Sponsored by: NetApp, Inc.
Sponsored by: Klara, Inc.
Differential Revision: https://reviews.freebsd.org/D30698
It looks like I've missed a couple of places where we don't clear
stack-allocated CCBs. Don't panic when that happens, just print
a warning.
This is a temporary measure until I get those cases fixed.
Reviewed By: markj
Sponsored by: NetApp, Inc.
Sponsored by: Klara, Inc.
Differential Revision: https://reviews.freebsd.org/D30296
This patch makes it possible for CAM to use small CCBs allocated
from an periph-specific UMA zone instead of the usual, huge ones.
The end result is that CCBs issued via da(4) take 544B (size of
ccb_scsiio) instead of the usual 2kB (size of 'union ccb', ~1.5kB,
rounded up by malloc(9)). For ATA it's 272B. We waste less
memory, we avoid zeroing the unused 1kB, and it should be easier
to allocate those CCBs in low memory conditions. It should also
be possible to use uma_zone_reserve(9) to improve behaviour
in low memory conditions even further.
Note that this does not change the size, or the layout, of CCBs
as such. CCBs get allocated in various different ways, in particular
on the stack, and I don't want to redo all that. Instead, this
provides an opt-in mechanism for the periph to declare "my start()
callback is fine with receiving a CCB allocated from this UMA zone".
In other words, most of the code works exactly as it used to; the
change only happens to IOs issued by xpt_run_allockq(), which
is - conveniently - pretty much all that matters for performance.
The reason for doing it this way is that it's pretty small, localized
change, and can be implemented gradually and iteratively: take a
periph, make sure its start() callback only casts the CCBs it takes
to a particular type of CCB, for example ccb_scsiio, and that it only
casts CCBs returned by cam_periph_getccb() to that type, then add UMA
zone for that size, and declare it safe to XPT.
This is disabled by default. Set 'kern.cam.ada.enable_uma_ccbs=1'
and 'kern.cam.da.enable_uma_ccbs=1' tunables to enable it. Testing
is welcome; I will flip the default to enable in two weeks from now.
Reviewed By: imp
Sponsored by: NetApp, Inc.
Sponsored by: Klara, Inc.
Differential Revision: https://reviews.freebsd.org/D28674
From my understanding this could happen with iSCSI LUNs with
unusually long names. The bug would make CAM fail to retrieve
the full inquiry data. Instead of bumping the size of the local
variable, just use a macro.
Reviewed By: imp, mav
Sponsored by: NetApp, Inc.
Sponsored by: Klara, Inc.
X-NetApp-PR: #50
Differential Revision: https://reviews.freebsd.org/D29991
This is my second pass, this time over all of CAM except
for the SCSI target bits. There should be no functional
changes.
Reviewed By: imp
Sponsored by: NetApp, Inc.
Sponsored by: Klara, Inc.
Differential Revision: https://reviews.freebsd.org/D29549
This is required for small CCBs support, where we need to track
whether the CCB was allocated from an UMA zone or not. There are
no (intended) functional changes with the current source.
Reviewed By: imp
Sponsored by: NetApp, Inc.
Sponsored by: Klara, Inc.
Differential Revision: https://reviews.freebsd.org/D29484
r357614 added CTLFLAG_NEEDGIANT to make it easier to find nodes that are
still not MPSAFE (or already are but aren’t properly marked).
Use it in preparation for a general review of all nodes.
This is non-functional change that adds annotations to SYSCTL_NODE and
SYSCTL_PROC nodes using one of the soon-to-be-required flags.
Mark all obvious cases as MPSAFE. All entries that haven't been marked
as MPSAFE before are by default marked as NEEDGIANT
Approved by: kib (mentor, blanket)
Commented by: kib, gallatin, melifaro
Differential Revision: https://reviews.freebsd.org/D23718
XPT_DEV_ADVINFO call should be protected by the lock of the specific
device it is addressed to, not the lock of SES device. In some weird
case, probably with hardware violating standards, it sometimes caused
NULL dereference due to race.
To protect from it further, add lock assertion to *_dev_advinfo().
MFC after: 1 week
Sponsored by: iXsystems, Inc.
Even if we do not expect retries, we better be sure, since otherwise it
may result in use after free kernel panic. I've noticed that it retries
SCSI_STATUS_BUSY even with SF_NO_RECOVERY | SF_NO_RETRY.
MFC after: 1 week
Sponsored by: iXsystems, Inc.
In `probedone()`, for the `PROBE_REPORT_LUNS` case, all paths that
fall to the bottom of the case set `lp` to `NULL`, so the test for a
non-NULL value of `lp` and call to `free()` if true is dead code as
the test can never be true. Fix by eliminating the whole if
statement. To guard against a possible future change that accidentally
violates this assumption, use a `KASSERT()` to catch if `lp` is
non-NULL.
Reviewed by: cem
MFC after: 1 week
Sponsored by: Dell EMC Isilon
Differential Revision: https://reviews.freebsd.org/D19109
It's likely that the header was needed in the past for swi(9).
But now that code does not use swi(9) or any other interfaces defined
in sys/interrupt.h.
MFC after: 1 week
There's no compelling reason to return a cam_status type for this
function and doing so only creates confusion with normal C
coding practices. It's technically an API change, but the periph API
isn't widely used. No efffective change to operation.
Reviewed by: imp, mav, ken
Sponsored by: Netflix
Differential Revision: D14063
This provides a nice wrarpper around the XPT_PATH_INQ ccb creation and
calling.
Sponsored by: Netflix
Differential Revision: https://reviews.freebsd.org/D13387
Mainly focus on files that use BSD 2-Clause license, however the tool I
was using misidentified many licenses so this was mostly a manual - error
prone - task.
The Software Package Data Exchange (SPDX) group provides a specification
to make it easier for automated tools to detect and summarize well known
opensource licenses. We are gradually adopting the specification, noting
that the tags are considered only advisory and do not, in any way,
superceed or replace the license texts.
In scsi_dev_advinfo(), if the physical path is being stored and there is a
malloc failure (malloc(9) is called with M_NOWAIT), we could wind up in a
situation where the device's physpath_len is set to the length the user
provided, but the physpath itself is NULL.
If another context then comes in to fetch the physical path value, we would
wind up trying to memcpy a NULL pointer into the caller's buffer.
So, set the physpath_len to 0 when we free the physpath on entry into the
store case for the physical path. Reset the length to a non-zero value only
after we've successfully malloced a buffer to hold it.
Submitted by: ken
Reviewed by: asomers
MFC after: 3 weeks
Sponsored by: Spectra Logic Corp
The Genesys chip is failing when issueing READ_CAP(16) command.
Force a quirk to disable it and use READ_CAP(10) instead.
Also, depending on used firmware, GL3224 can be recognized
either as 'storage device' or 'mass storage class' -
enable both variants in scsi_quirk_table.
Submitted by: Wojciech Macek <wma@semihalf.com>
Konrad Adamczyk <ka@semihalf.com>
Obtained from: Semihalf
Sponsored by: Stormshield
Reviewed by: mav
Differential revision: https://reviews.freebsd.org/D10902
using a driver-supplied sbuf for printing device discovery
announcements. This helps ensure that messages to the console
will be properly serialized (through sbuf_putbuf) and not be
truncated and interleaved with other messages. The
infrastructure mirrors the existing xpt_announce_periph()
entry point and is opt-in for now. No content or formatting
changes are visible to the operator other than the new coherency.
While here, eliminate the stack usage of the temporary
announcement buffer in some of the drivers. It's moved to the
softc for now, but future work will eliminate it entirely by
making the code flow more linear. Future work will also address
locking so that the sbufs can be dynamically sized.
The scsi_da, scs_cd, scsi_ses, and ata_da drivers are converted
at this point, other drivers can be converted at a later date.
A tunable+sysctl, kern.cam.announce_nosbuf, exists for testing
purposes but will be removed later.
TODO:
Eliminate all of the code duplication and temporary buffers. The
old printf-based methods will be retired, and xpt_announce_periph()
will just be a wrapper that uses a dynamically sized sbuf. This
requires that the register and deregister paths be made malloc-safe,
which they aren't currently.
Sponsored by: Netflix
A bug in CAM's serial number hash logic resulted in SATA drives behind a SAS
controller getting removed and readded anytime the drive was rescanned for
any reason.
PR: 212914
Submitted by: kadesai
Reported by: kadesai
Reviewed by: asomers, ken
MFC after: 4 weeks
Sponsored by: Spectra Logic Corp
per-protocol. This reduces the number scsi symbols references by
cam_xpt significantly, and eliminates all ata / nvme symbols. There's
still some NVME / ATA specific code for dealing with XPT_NVME_IO and
XPT_ATA_IO respectively, and a bunch of scsi-specific code, but this
is progress.
Differential Revision: https://reviews.freebsd.org/D7289
eliminates the need to special case everything in cam_xpt for new
transports. It is now a failure to not have a transport object when
registering the bus as well. You can still, however, create a
transport that's unspecified (XPT_)
Differential Revision: https://reviews.freebsd.org/D7289
sys/cam/scsi/scsi_xpt.c
Strip leading spaces off of a SCSI disk's reported serial number
when populating the CAM serial number. This affects the output of
"diskinfo -v" and the names of /dev/diskid/DISK-* device nodes,
among other things.
SPC5r05 says that the Product Serial Number field from the Unit
Serial Number VPD page is right-aligned. So any leading spaces are
not part of the actual serial number. Most devices don't left-pad
their serial numbers, but some do. In particular, the SN VPD page
that an LSI HBA emulates for a SATA drive contains enough
left-padding to fill a 20-byte field.
UPDATING
Add a note to UPDATING, because some users may have to update
/etc/fstab or geom labels.
Reviewed by: ken, mav
MFC after: Never
Sponsored by: Spectra Logic Corp
Differential Revision: https://reviews.freebsd.org/D6516
and a retry is scheduled.
Instead of leaving the device queue frozen, unfreeze the device queue so
that the retry can happen.
Sponsored by: Spectra Logic
MFC after: 3 days
Previously such LUNs were silently ignored. But while they indeed unable
to process most of SCSI commands, some, like RTPG, they still can.
MFC after: 1 month
referenced. I think that there does exist an unlikely edge case for a
memory leak, but only if a driver is incorrectly written and specifies no
valid range of targets to scan. That can be fixed in a follow-up commit.
Obtained from: Netflix, Inc.
This VPD page is effectively an extension of the standard Inquiry
data page, and includes lots of additional bits.
This commit includes support for probing the page in the SCSI probe code,
and an additional request type for the XPT_DEV_ADVINFO CCB. CTL already
supports the Extended Inquiry page.
Support for querying this page in the sa(4) driver will come later.
sys/cam/scsi/scsi_xpt.c:
Probe the Extended Inquiry page, if the device supports it, and
return it in response to a XPT_DEV_ADVINFO CCB if it is requested.
sys/cam/scsi/cam_ccb.h:
Define a new advanced information CCB data type, CDAI_TYPE_EXT_INQ.
sys/cam/cam_xpt.c:
Free the extended inquiry data in a device when the device goes
away.
sys/cam/cam_xpt_internal.h:
Add an extended inquiry data pointer and length to struct cam_ed.
sys/sys/param.h
Bump __FreeBSD_version for the addition of the new
CDAI_TYPE_EXT_INQ advanced information type.
Sponsored by: Spectra Logic
MFC after: 1 week
data to go undetected.
The probe code does an MD5 checksum of the inquiry data (and page
0x80 serial number if available) before doing a reprobe of an
existing device, and then compares a checksum after the probe to
see whether the device has changed.
This check was broken in January, 2000 by change 56146 when the extended
inquiry probe code was added.
In the extended inquiry probe case, it was calculating the checksum
a second time. The second time it included the updated inquiry
data from the short inquiry probe (first 36 bytes). So it wouldn't
catch cases where the vendor, product, revision, etc. changed.
This change will have the effect that when a device's inquiry data is
updated and a rescan is issued, it will disappear and then reappear.
This is the appropriate action, because if the inquiry data or serial
number changes, it is either a different device or the device
configuration may have changed significantly. (e.g. with updated
firmware.)
scsi_xpt.c: Don't calculate the initial MD5 checksum on
standard inquiry data and the page 0x80 serial
number if we have already calculated it.
MFC after: 1 week
Sponsored by: Spectra Logic
SPC-2 tells REPORT LUNS shall be supported by devices supporting LUNs other
then LUN 0. If we see LUN 0 disconnected, guess there may be others, and
so REPORT LUNS shall be supported.
MFC after: 1 month
Previous logic was not differentiating disconnected LUNs and absent targets.
That made it to stop scan if LUN 0 was not found for any reason. That made
problematic, for example, using iSCSI targets declaring SPC-2 compliance and
having no LUN 0 configured.
The new logic continues sequential LUN scan if:
-- we have more configured LUNs that need recheck;
-- this LUN is connected and its SCSI version allows more LUNs;
-- this LUN is disconnected, its SCSI version allows more LUNs and we
guess they may be connected (we haven't scanned first 8 LUNs yet or
kern.cam.cam_srch_hi sysctl is set to scan more).
Reported by: trasz
MFC after: 1 month
These changes prevent sysctl(8) from returning proper output,
such as:
1) no output from sysctl(8)
2) erroneously returning ENOMEM with tools like truss(1)
or uname(1)
truss: can not get etype: Cannot allocate memory
there is an environment variable which shall initialize the SYSCTL
during early boot. This works for all SYSCTL types both statically and
dynamically created ones, except for the SYSCTL NODE type and SYSCTLs
which belong to VNETs. A new flag, CTLFLAG_NOFETCH, has been added to
be used in the case a tunable sysctl has a custom initialisation
function allowing the sysctl to still be marked as a tunable. The
kernel SYSCTL API is mostly the same, with a few exceptions for some
special operations like iterating childrens of a static/extern SYSCTL
node. This operation should probably be made into a factored out
common macro, hence some device drivers use this. The reason for
changing the SYSCTL API was the need for a SYSCTL parent OID pointer
and not only the SYSCTL parent OID list pointer in order to quickly
generate the sysctl path. The motivation behind this patch is to avoid
parameter loading cludges inside the OFED driver subsystem. Instead of
adding special code to the OFED driver subsystem to post-load tunables
into dynamically created sysctls, we generalize this in the kernel.
Other changes:
- Corrected a possibly incorrect sysctl name from "hw.cbb.intr_mask"
to "hw.pcic.intr_mask".
- Removed redundant TUNABLE statements throughout the kernel.
- Some minor code rewrites in connection to removing not needed
TUNABLE statements.
- Added a missing SYSCTL_DECL().
- Wrapped two very long lines.
- Avoid malloc()/free() inside sysctl string handling, in case it is
called to initialize a sysctl from a tunable, hence malloc()/free() is
not ready when sysctls from the sysctl dataset are registered.
- Bumped FreeBSD version to indicate SYSCTL API change.
MFC after: 2 weeks
Sponsored by: Mellanox Technologies
