iscsi_create_context() calls srand() every time a new context is
generated. That practice is questionable, as the seed does not need to
change before each call to rand(). As a matter of fact, doing so defeats
the purpose of using rand() altogether. Furthermore, the current
implementation is not thread safe.
This improves ISID generation by using /dev/urandom (when available) as
a seed, and calling srand() only once. In case of errors, fallback to
using something similar to the previous implementation (albeit
thread-safe).
Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
The old code is effectively always posting iser_conn->min_posted_rx
descriptors, since it is
if (outstanding + iser_conn->min_posted_rx <= iser_conn->qp_max_recv_dtos) {
if(iser_conn->qp_max_recv_dtos - outstanding > iser_conn->min_posted_rx)
count = iser_conn->min_posted_rx;
else
count = iser_conn->qp_max_recv_dtos - outstanding;
which is equivalent to
if(iser_conn->qp_max_recv_dtos - outstanding >= iser_conn->min_posted_rx)
if(iser_conn->qp_max_recv_dtos - outstanding > iser_conn->min_posted_rx)
count = iser_conn->min_posted_rx;
else
count = iser_conn->min_posted_rx;
So the "if" is redundant and the "min_posted_rx" is actually behaving more
like a _maximum_ number of posted descriptors in one iser_post_recvm.
Fix it with the (presumably) intended logic and remove a goto along
the way.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The current random seed for determining a new context's ISID is
calculated by XOR'ing time(), getpid() and "iscsi". When invoked from
iscsi_reconnect(), all three inputs are likely to be identical,
resulting on identical ISIDs.
That happens because iscsi_reconnect() malloc()s a temporary "iscsi"
which is then free()d at the end of the call. Successive calls to
malloc() (from that function) are therefore likely to reuse the same
address for the context.
When multiple sessions are used for different LUNs of the same target,
and reconnects happen within the same second (the precision given by
time()), then multiple login attempts will happen with identical values,
violating the ISID RULE as described in Section 3.4.3 of RFC3270.
This fixes the issue by introducing a sequence number to the ISID seed
generation.
Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
The current iscsi context in iscsi_reconnect() is called "old_iscsi",
whilst the temporary context is called "iscsi". That is rather
confusing, and this fixes that by calling the current context "iscsi"
and the temporary context "tmp_iscsi".
Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
This fixes some identation in iscsi_reconnect_cb() where whitespaces
were used instead of hard tabs.
Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
When unmarshalling a SCSI_PERSISTENT_RESERVE_READ_KEYS response,
scsi_persistentreservein_datain_unmarshall() assumes that the ADDITIONAL
LENGTH field represents the number of keys packed in the key array.
This is incorrect as key array data buffer may be truncated while
ADDITIONAL LENGTH is left in tact, as per SPC5r17 4.2.5.6:
If the information being transferred to the Data-In Buffer includes
fields containing counts ..., then the contents of these fields shall
not be altered to reflect the truncation, if any, that results from an
insufficient ALLOCATION LENGTH value, unless the standard that
describes the Data-In Buffer format states otherwise.
Determine the number of keys returned based on the minimum of the
data-in length and the ADDITIONAL LENGTH value.
Signed-off-by: David Disseldorp <ddiss@suse.de>
The code was implicitly dependent on container_of from
inifiniband/verbs.h, however that's been removed in rdma-core
latest release:
ce0274acff
Define container_of locally if it's not already defined
The set of sync connect calls use a stack variable to track the
connection status. This is ok because such calls block on event_poll()
until the connection is established. However, event_poll may return
early in case of errors (or timeout) while PDUs are still queued on the
context (and pointing to a local stack).
This cancels any pending PDUs before returning from sync connect calls.
Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
Introduce a helper exported from lib/pdu.c which cancels all pdus for a
given context. This patch eliminates repeated code from various other
files which have the same purpose. The only functional difference is
that the cancellation done from iscsi-command.c was (incorrectly) not
checking for iscsi->is_loggedin before issuing callbacks.
Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
Using WIN32 depends on the build environment defining the variable.
_WIN32 is a predefined MSVC macro and is always available.
Signed-off-by: Tim Crawford <crawfxrd@gmail.com>
The primary issue is that in MSVC 14.00 (VS2015) Microsoft added
snprintf as a function to the standard library and prevents users from
defining it to something else (typically, this was _snprintf). So, only
define it when using _MSC_VER < 1900.
Other changes are:
- Fix macro definition of dup2
- Add macro for getpid
- Add function definition for win32_dup
- Add missing EXTERNs
Signed-off-by: Tim Crawford <crawfxrd@gmail.com>
Win32 has been rotting for a while. This patch adds vs17 build files
as well as fixing up all build errors that have accumulated.
There are still build warnings but those can be addressed in a followup
patch.
Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
all sync commands that return an integer value are
supposed to return a negative value on error.
However, state.status is positive non-zero on error.
Fix this by returning -1 if the state.status is
not SCSI_STATUS_GOOD.
Reported-by: Sitsofe Wheeler <sitsofe@yahoo.com>
Signed-off-by: Peter Lieven <pl@kamp.de>
we mangled the PDU header after calculating the checksum which
effectively broke CRC32C header digests completely.
Signed-off-by: Peter Lieven <pl@kamp.de>
commit bc64420 introduced an extra smalloc for the in->hdr,
however it did use iscsi_free instead of iscsi_sfree to free it.
Signed-off-by: Peter Lieven <pl@kamp.de>
Build on existing scsi_cdb_extended_copy() functionality. This operation
can be used to offload inter and intra LU copies to the target.
The API is rather primitive, in that the caller needs to construct the
parameter buffer, including CSCD and segment descriptor lists, etc.
Signed-off-by: David Disseldorp <ddiss@suse.de>
Build on existing scsi_cdb_receive_copy_results() functionality. This
request is most commonly used to determine target-side EXTENDED COPY
operational parameters.
Signed-off-by: David Disseldorp <ddiss@suse.de>
If the length of iscsi->waitpdu and iscsi->inqueue are the same
then except for any target initiated NOPs or async messages
we should have received any and all possible pdus from this
socket and can abort early.
This avoids running the loop one more time just to fail with EAGAIN
at the recs/readv. Just avoiding that recv/readv syscall will shave
at least 10us off this function and thus the latency.
Suggested-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Signed-off-by: Peter Lieven <pl@kamp.de>
iscsi_read_from_socket can currently only read one PDU in each iscsi_service invocation even
if there is more data available on the socket. This patch reads all PDUs until the socket
would block. It enqueues all complete read PDUs and then processes them in order of arrival.
Signed-off-by: Peter Lieven <pl@kamp.de>
- Various updates to the test utility
- Add transport abstraction
- Add support for iSER
- Add iscsi_discovery_sync()
Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
