Before this ioctl frontend always replaced tags with sequential ones.
It was done for ctladm, that can not keep track of global tag list.
But in case of virtio-scsi in bhyve we can pass provided tags as-is.
It should be on virtio-scsi initiator to provide us valid tags. It
should allow proper task management, error reporting, etc. In case
of several virtio-scsi devices, they should use different CTL ports
or initiator IDs to avoid conflicts, but this is expected by design.
PR: 267539
SAM-5 specification states maximum size of command identifier (tag),
defined by specific transports, should not be larger than 64 bits.
While most of supported transports use 32 bits or less, it was
reported that virtio-scsi uses 64 bits. Truncation to 32 bits in
bhyve code caused false tag conflict errors reported and possibly
other issues.
This changes CTL ABI and HA protocol, so CTL_HA_VERSION is bumped.
While we make HA protocol incompatible, increase default maximum
number of ports in CTL from 256 to 1024, matching number of LUNs.
There are many reports from people who need many iSCSI targets with
only one LUN each. Increased memory consumption should be less of
a problem these days.
PR: 267539
It was possible to cause kernel panic by passing too large args_len
or non-NULL result_nvl.
Though since the /dev/cam/ctl device is accessible only by root and
used only by limited number of tools it was not a big problem.
PR: 266115
PR: 266136
Reported by: Robert Morris <rtm@lcs.mit.edu>
MFC after: 1 week
I've found couple cases when CTL_FLAG_SENT_2OTHER_SC flags were not
cleared on commands return from active node or the send failure. It
created races when ctl_failover_lun() call before ctl_process_done()
could cause second ctl_done() and ctl_process_done() calls, causing
all sorts of problems.
MFC after: 2 weeks
Sponsored by: iXsystems, Inc.
cxgbei needs the ability to return different limits based on the
connection (e.g. if the connection is over a T5 adapter or a T6
adapter as well as factoring in the MTU).
This change plumbs through the changes in the ioctls without changing
any of the backends. The limits callback passed to icl_register now
accepts a second socket argument which holds the integer file
descriptor. To support ABI compatiblity for old binaries, the
callback should return "global" values if the socket fd is zero.
The CTL_ISCSI_LIMITS argument used with CTL_ISCSI by ctld(8) now
accepts the socket fd in a field that was previously part of a
reserved spare field. Old binaries zero this request which results in
passing a socket fd of 0 to the limits callback.
The ISCSIDREQUEST ioctl no longer returns limits. Instead, iscsid(8)
invokes a new ISCSIDLIMITS ioctl after establishing the connection via
connect(2). For ABI compat, if the old ISCSIDREQUEST is invoked, the
global limits are still fetched (with a socket fd of 0) and returned.
Reviewed by: mav
Sponsored by: Chelsio Communications
Differential Revision: https://reviews.freebsd.org/D34928
This should fix uninitialized memory reads when working with broken
HA peer, like one fixed in 1a8d8a3a90. Instead print error message
and kill the HA link.
MFC after: 3 days
Sponsored by: iXsystems, Inc.
For a compare request, the ramdisk backend allocates a temporary
buffer to hold the I/O data and then compares it against the LUN's
pages in ctl_backend_ramdisk_cmp after the data has been filled.
However, the tempory buffer was leaked when after the comparison was
complete. Fix this by freeing the buffer after the comparison.
Reviewed by: mav
Sponsored by: Chelsio Communications
Differential Revision: https://reviews.freebsd.org/D34316
The dummy PDU needs to be freed before marking task abortion complete
as otherwise cfiscsi_session_terminate_tasks can return and destroy
the session in another thread before the PDU is freed.
Fixes: 2e8d1a5525 iscsi: Allocate a dummy PDU for the internal nexus reset task.
Sponsored by: Chelsio Communications
Differential Revision: https://reviews.freebsd.org/D34176
Due to variable size of struct ctl_ha_msg_mode ctl_isc_announce_mode()
sent only first 4 bytes of modified mode page to the other HA side,
that caused its corruption there, noticeable only after failover.
I've found alike bug also in ctl_isc_announce_lun(), but there it was
sending slightly more than needed, that is a smaller problem.
MFC after: 1 week
Sponsored by: iXsystems, Inc.
When an iSCSI target session is terminated, an internal nexus reset
task is posted to abort existing tasks belonging to the session.
Previously, the ctl_io for this internal nexus reset stored a pointer
to the session in the slot that normally holds a pointer to the PDU
from the initiator that triggered the I/O request. The completion
handler then assumed that any nexus reset I/O was due to an internal
request and fetched the session pointer (instead of the PDU pointer)
from the ctl_io. However, it is possible to trigger a nexus reset via
an on-the-wire task management PDU. If such a PDU were sent to the
target, then the completion handler would incorrectly treat this
request as an internal request and treat the pointer to the received
PDU as a pointer to the session instead.
To fix, allocate a dummy PDU for the internal reset task and use an
invalid opcode to differentiate internal nexus resets from resets
requested by the initiator.
PR: 260449
Reported by: Robert Morris <rtm@lcs.mit.edu>
Reviewed by: mav
Sponsored by: Chelsio Communications
Differential Revision: https://reviews.freebsd.org/D34055
This matches icl_conn_task_setup() which passes the PDU and avoids the
need for a layering violation in cxgbei to fetch the request PDU from
the ctl_io.
Reviewed by: mav
Sponsored by: Chelsio Communications
Differential Revision: https://reviews.freebsd.org/D33746
For years CTL block backend limited I/O size to 1MB, splitting larger
requests into sequentially processed chunks. It is sufficient for
most of use cases, since typical initiators rarely use bigger I/Os.
One of known exceptions is VMWare VAAI offload, by default sending up
to 8 4MB EXTENDED COPY requests same time. CTL internally converted
those into 32 1MB READ/WRITE requests, that could overwhelm the block
backend, having finite number of processing threads and making more
important interactive I/Os to wait in its queue. Previously it was
partially covered by CTL core serializing sequential reads to help
ZFS speculative prefetcher, but that serialization was significantly
relaxed after recent ZFS improvements.
With the new settings block backend receives 8 4MB requests, that
should be easier for both CTL itself and the underlying storage.
MFC after: 2 weeks
Sponsored by: iXsystems, Inc.
cfiscsi_datamove_out() can race with cfiscsi_session_terminate_tasks()
and enqueue a new task after the latter function has aborted existing
tasks. This could result in a deadlock as
cfiscsi_session_terminate_tasks() waited forever for this task to
complete.
Reviewed by: mav
Sponsored by: Chelsio Communications
Differential Revision: https://reviews.freebsd.org/D31892
This adds support for SCSI UNMAP command to file-backed LUNs, if the
underlying file system has a non-zerofilling VOP_DEALLOCATE
implementation where some or all parts of the requested operation range
may be deallocated.
Sponsored by: The FreeBSD Foundation
Reviewed by: mav
Differential Revision: https://reviews.freebsd.org/D31922
When using iSCSI PDU offload (cxgbei) on T6 adapters, a burst of
received PDUs can be reported via a single message to the driver.
Previously the driver passed these multi-PDU bursts up to the iSCSI
stack up as a single "large" PDU by rewriting the buffer offset, data
segment length, and DataSN fields in the iSCSI header. The DataSN
field in particular was rewritten so that each of the "large" PDUs
used consecutively increasing values. While this worked, the forged
DataSN values did not match the ExpDataSN value in the subsequent SCSI
Response PDU. The initiator does not currently verify this value, but
the forged DataSN values prevent adding a check.
To avoid this, allow a logical iSCSI PDU (struct icl_pdu) to describe
a burst of PDUs via a new 'ip_additional_pdus' field. Normally this
field is set to zero when 'struct icl_pdu' represents a single PDU.
If logical PDU represents a burst of on-the-wire PDUs, then 'ip_npdus'
contains the count of additional on-the-wire PDUs. The header of this
"large" PDU is still modified, but the DataSN field now contains the
DataSN value of the first on-the-wire PDU in the burst.
Reviewed by: mav
Sponsored by: Chelsio Communications
Differential Revision: https://reviews.freebsd.org/D31577
Similar to TSO, iSCSI segmentation offload permits the upper layers to
submit a "large" virtual PDU which is split up into multiple segments
(PDUs) on the wire. Similar to how the TCP/IP headers are used as
templates for TSO, the BHS at the start of a large PDU is used as a
template to construct the specific BHS at the start of each PDU. In
particular, the DataSN is incremented for each subsequent PDU, and the
'F' flag is only set on the last PDU.
struct icl_conn has a new 'ic_hw_isomax' field which defaults to 0,
but can be set to the largest virtual PDU a backend supports. If this
value is non-zero, the iSCSI target and initiator use this size
instead of 'ic_max_send_data_segment_length' to determine the maximum
size for SCSI Data-In and SCSI Data-Out PDUs. Note that since PDUs
can be constructed from multiple buffers before being dispatched, the
target and initiator must wait for the PDU to be fully constructed
before determining the number of DataSN values were consumed (and thus
updating the per-transfer DataSN value used for the start of the next
PDU).
The target generates large PDUs for SCSI Data-In PDUs in
cfiscsi_datamove_in(). The initiator generates large PDUs for SCSI
Data-Out PDUs generated in response to an R2T.
Reviewed by: mav
Sponsored by: Chelsio Communications
Differential Revision: https://reviews.freebsd.org/D31222
and remove repetetive code that calculates vnode locking type for write.
Reviewed by: khng, markj
Sponsored by: The FreeBSD Foundation
MFC after: 1 week
Differential revision: https://reviews.freebsd.org/D31405
Note that, as pointed out by scottl@, this code should really look
a bit different, in that the stack allocations should be replaced
with dynamic allocation, and the periph creation should be moved
to a context where one can use M_WAITOK. See the review for more
details. For now let's go with a minimal fix until we're done with
UMA CCBs.
Reviewed By: mav, imp
Sponsored by: NetApp, Inc.
Sponsored by: Klara, Inc.
Differential Revision: https://reviews.freebsd.org/D30298
This fixes a few bugs in iSCSI backends where the backends were using
the limits they advertised initially during the login phase as the
final values instead of the values negotiated with the other end.
Reported by: Jithesh Arakkan @ Chelsio
Reviewed by: mav
Differential Revision: https://reviews.freebsd.org/D30271
cxgbei stores state about a target transfer in the ctl_private[] array
of a ctl_io that is freed when a target transfer (represented by the
cdw) is freed. As such, freeing a ctl_io before a cdw that references
it can result in a use after free in cxgbei. Two of the four places
freed the cdw first, and the other two freed the ctl_io first. Fix
the latter two places to free the cdw first.
Reported by: Jithesh Arakkan @ Chelsio
Reviewed by: mav
Differential Revision: https://reviews.freebsd.org/D30270
With new ZFS prefetcher improvements it is no longer needed to fully
serialize reads to reach decent prediction hit rate. Softer variant
only creates small time window to reduce races instead of completely
blocking following reads while previous is running. It much less
hurts the performance in case of prediction miss.
MFC after: 1 month
Return while there are any I/Os in a queue may result in them stuck
indefinitely, since there is only one taskqueue task for all of them.
I think I've reproduced this by switching ha_role to secondary under
heavy load.
MFC after: 3 days
- Move ctl_get_cmd_entry() calls from every OOA traversal to when
the requests first inserted, storing seridx in struct ctl_scsiio.
- Move some checks out of the loop in ctl_check_ooa().
- Replace checks for errors that can not happen with asserts.
- Transpose ctl_serialize_table, so that any OOA traversal accessed
only one row (cache line). Compact it from enum to uint8_t.
- Optimize static branch predictions in hottest places.
Due to O(n) nature on deep LUN queues this can be the hottest code
path in CTL, and additional 20% of IOPS I see in some 4KB I/O tests
are good to have in reserve. About 50% of CPU time here according
to the profiles is now spent in two memory accesses per traversed
request in OOA.
Sponsored by: iXsystems, Inc.
MFC after: 2 weeks
This makes random read benchmarks look better on a wide ZFS pools.
I am not sure where the original value goes from, but it is there
for too long now.
MFC after: 1 week
- Make frontends call unified CTL core method ctl_datamove_done()
to report move completion. It allows to reduce code duplication
in differerent backends by accounting DMA time in common code.
- Add to ctl_datamove_done() and be_move_done() callback samethr
argument, reporting whether the callback is called in the same
context as ctl_datamove(). It allows for some cases like iSCSI
write with immediate data or camsim frontend write save one context
switch, since we know that the context is sleepable.
- Remove data_move_done() methods from struct ctl_backend_driver,
unused since forever.
MFC after: 1 month
Switch OOA queue from TAILQ to LIST and change its direction, so that
we traverse it forward, not backward. There is only one place where
we really need other direction, and it is not critical.
Use STAILQ_REMOVE_HEAD() instead of STAILQ_REMOVE() in backends.
Replace few impossible conditions with assertions.
MFC after: 1 month
Introduce new CTL core KPI ctl_run(), preprocessing I/Os in the caller
context instead of scheduling another thread just for that. This call
may sleep, that is not acceptable for some frontends like the original
CAM/FC one, but iSCSI already has separate sleepable per-connection RX
threads, and another thread scheduling is mostly just a waste of time.
IOCTL frontend actually waits for the I/O completion in the caller
thread, so the use of another thread for this has even less sense.
With this change I can measure ~5% IOPS improvement on 4KB iSCSI I/Os
to ZFS.
MFC after: 1 month
RFC 7143 (11.7.4):
The Target Transfer Tag values are not specified by this protocol,
except that the value 0xffffffff is reserved and means that the
Target Transfer Tag is not supplied.
MFC after: 1 month
Replace MAXPHYS by runtime variable maxphys. It is initialized from
MAXPHYS by default, but can be also adjusted with the tunable kern.maxphys.
Make b_pages[] array in struct buf flexible. Size b_pages[] for buffer
cache buffers exactly to atop(maxbcachebuf) (currently it is sized to
atop(MAXPHYS)), and b_pages[] for pbufs is sized to atop(maxphys) + 1.
The +1 for pbufs allow several pbuf consumers, among them vmapbuf(),
to use unaligned buffers still sized to maxphys, esp. when such
buffers come from userspace (*). Overall, we save significant amount
of otherwise wasted memory in b_pages[] for buffer cache buffers,
while bumping MAXPHYS to desired high value.
Eliminate all direct uses of the MAXPHYS constant in kernel and driver
sources, except a place which initialize maxphys. Some random (and
arguably weird) uses of MAXPHYS, e.g. in linuxolator, are converted
straight. Some drivers, which use MAXPHYS to size embeded structures,
get private MAXPHYS-like constant; their convertion is out of scope
for this work.
Changes to cam/, dev/ahci, dev/ata, dev/mpr, dev/mpt, dev/mvs,
dev/siis, where either submitted by, or based on changes by mav.
Suggested by: mav (*)
Reviewed by: imp, mav, imp, mckusick, scottl (intermediate versions)
Tested by: pho
Sponsored by: The FreeBSD Foundation
Differential revision: https://reviews.freebsd.org/D27225
Before this CTL always allocated MAXPHYS-sized buffers, even for 4KB I/O,
that is even more overkill for MAXPHYS of 1MB. This change limits maximum
allocation to 512KB if MAXPHYS is bigger, plus if one is above 128KB, adds
new 128KB UMA zone for smaller I/Os. The patch factors out alloc/free,
so later we could make it use more zones or malloc() if we'd like.
MFC after: 1 week
Sponsored by: iXsystems, Inc.
Foundation copyrights, approved by emaste@. It does not include
files which carry other people's copyrights; if you're one
of those people, feel free to make similar change.
Reviewed by: emaste, imp, gbe (manpages)
Differential Revision: https://reviews.freebsd.org/D26980
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.
