Refactor the code to use these new functions.
Change-Id: I21ee7e9a96f30fbd60106add5e8b071e86bf93c9
Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
This preps for some upcoming patches as well as
removing two levels of indentation.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I4f685c1e44ec4aa261e68af1786cfc110f451ed5
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17960
Reviewed-by: Konrad Sztyber <konrad.sztyber@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-by: Aleksey Marchuk <alexeymar@nvidia.com>
While userspace probes have a high overhead when enabled due
to the trap, it is still cleaner and slightly more efficient
to not have all of the SPDK_DTRACE_PROBE macros implicitly
capture the tsc counter as an argument.
So rename the existing SPDK_DTRACE_PROBE macros to
SPDK_DTRACE_PROBE_TICKS, and create new SPDK_DTRACE_PROBE
macros without the implicit ticks argument.
Note this does cause slight breakage if there is any
out-of-tree code that using SPDK_DTRACE_PROBE previously,
and programs written against those probes would need to
adjust their arguments. But the likelihood of such code
existing is practically nil, so I'm just renaming the
macros to their ideal state.
All of the nvmf SPDK_DTRACE_PROBE calls are changed to
use the new _TICKS variants. The event one is left
without _TICKS - we have no in-tree scripts that use
the tsc for that event.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Icb965b7b8f13c23d671263326029acb88c82d9df
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17669
Community-CI: Mellanox Build Bot
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Mike Gerdts <mgerdts@nvidia.com>
Reviewed-by: Konrad Sztyber <konrad.sztyber@intel.com>
When removing a listener, for example with
nvmf_subsystem_remove_listener RPC, we use the concept of a
"listen trid" to determine which existing connections
should be disconnected.
This listen trid has the trtype, adrfam, traddr and trsvcid
defined, but *not* the subnqn. We use the subsystem pointer
itself to match the subsystem.
nvmf_stop_listen_disconnect_qpairs gets the listen trid
for each qpair, compares it to the trid passed by the
RPC, and if it matches, then it compares the subsystem
pointers and will disconnect the qpair if it matches.
The problem is that the spdk_nvmf_qpair_get_listen_trid
path does not initialize the subnqn to an empty string,
and in this case the caller does not initialize it either.
So sometimes the subnqn on the stack used to get the
qpair's listen trid ends up with some garbage as the subnqn
string, which causes the transport_id_compare to fail, and
then the qpair won't get disconnected even if the other
trid fields and subsystem pointers match.
For the failover.sh test, this means that the qpair doesn't
get disconnected, so we never go down the reset path
on the initiator side and don't see the "Resetting" strings
expected in the log.
This similarly impacts the host/timeout.sh test, which is
also fixed by this patch. There were multiple failing
signatures, all related to remove_listener not working
correctly due to this bug.
While the get_listen_trid path is the one that caused
these bugs, the get_local_trid and get_peer_trid paths
have similar problems, so they are similarly fixed in
this patch.
Fixes issue #2862.
Fixes issue #2595.
Fixes issue #2865.
Fixes issue #2864.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I36eb519cd1f434d50eebf724ecd6dbc2528288c3
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17788
Reviewed-by: Aleksey Marchuk <alexeymar@nvidia.com>
Reviewed-by: Shuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-by: Mike Gerdts <mgerdts@nvidia.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Konrad Sztyber <konrad.sztyber@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: <sebastian.brzezinka@intel.com>
Handling this callback is quite complex and may lead to
various problems. In most of places, the actual event
when qpair is dosconnected is not importnat for the
app logic. Only in shutdown path we need to be sure
that all qpairs are disconnected, it can be achieved
by checking poll_group::qpairs list
Signed-off-by: Alexey Marchuk <alexeymar@nvidia.com>
Change-Id: I453961299f67342c1193dc622685aefb46bfceb6
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17165
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
If the qpair is already in the process of disconnect,
the spdk_nvmf_qpair_disconnect API now return -EINPROGRESS
and doesn't call the callback passed by the user.
Signed-off-by: Alexey Marchuk <alexeymar@nvidia.com>
Change-Id: If996b0496bf15729654d18771756b736e41812ae
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17164
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Current implementation of spdk_nvmf_qpair_disconnect
saves and calls user's callback correctly only on
the first call. If this function is called when
qpair is already in the process of disconnect, the
cb_fn is called immediately, that may lead to stack
overflow.
In most of places this function is called with
cb_fn = NULL, that means that the real qpair disconnect
is not important for the app logic. Only in several
places (nvmf tgt shutdown flow) that is important to
wait for all qpairs to be disconnected.
Taking into account complexity related to possible stack
overflow, do not pass the cb_fn to spdk_nvmf_qpair_disconnect.
Instead, wait until a list of qpairs is empty in shutdown path.
Next patches will change spdk_nvmf_qpair_disconnect behaviour
when disconnect is in progress and deprecate cb_fn and ctx
parameters.
Fixes issue #2765
Signed-off-by: Alexey Marchuk <alexeymar@nvidia.com>
Change-Id: Ie8d49c88cc009b774b45adab3e37c4dde4395549
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17163
Reviewed-by: Shuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
This will be useful in upcoming patch, where we
use the number of poll groups to dynamically pick
the buf_cache_size for each transport poll group.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Id166098244287c56f12cdd88ba27a17fa34a4348
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17331
Reviewed-by: Aleksey Marchuk <alexeymar@nvidia.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Let us consider the following process:
1. one fabric connect request A comes but the subsystem is paused
due to adding/removing ns or other operations, so this request A
will be put into sgroup->queued until the subsystem becomes active;
2. the subsystem is paused for a long time until the connect timeout,
related qpair is destroyed, the sgroup->queued will not be cleaned
because qpair's ctrlr is NULL;
3. if a new request B comes, it is more likely to be allocated to the
same memory as the previous fabric command request. And it will be
put into sgroup->queued again, where has already exists the exactly
same pointer with request B.
This leads to the pointer hanging problem and it will cause infinitely
loop when traversing sgroup->queued!
So this patch avoids the ptr-hanging problem by checking and cleaning
all sgroups queued req whose qpair is the being destroyed qpair in
_nvmf_qpair_destroy when ctrlr is NULL.
This problem is already described in issue #2133.
Signed-off-by: Peng Lian<peng.lian@smartx.com>
Change-Id: I909d673b5050f21fa193914cc4ffe6634232fa7d
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17147
Community-CI: Mellanox Build Bot
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Konrad Sztyber <konrad.sztyber@intel.com>
There is a way to pause/resume spdk pollers, however there is no way
to achieve that using public API for the given target which has
a hook behaving similar to pollers. Exposing such functionality can
be used for pausing and restoring target pollers during
reset, e.g. new commands should not be fetched to assure
that all internal resources can be cleared/reinitialized safety.
Pausing target poller during the reset will assure that, without
need for destroying transport or adding condition statements in IO path.
Similar use case might be hitless upgrade. Depending on implementation
there might be need that no new command can be submitted when
secondary processes are being switched to upgraded versions.
Pausing target pollers should be useful in this case.
Signed-off-by: Kamuda Szymon <szymon.kamuda@intel.com>
Change-Id: I419816552c710c43e02197ebcc20a967fb23b3bd
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/15911
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Konrad Sztyber <konrad.sztyber@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Basic IO completion counting can be done at the common
layer, to enable some level of stat tracking even for
transports that don't have transport-specific tracking
yet.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: If04f854b97440089b8ad149b64cb59173c73975c
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/15912
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@nvidia.com>
Reviewed-by: Shuhei Matsumoto <smatsumoto@nvidia.com>
Community-CI: Mellanox Build Bot
We make decisions on how to pick a poll group for a new
qpair by looking at each poll group's current_io_qpairs
count. But this count isn't always accurate since it
doesn't get updated until after the CONNECT has
been received.
This means that if we accept a bunch of connections
all at once, they may all get assigned the same poll
group, because the target poll groups counter doesn't
get immediately incremented.
So add a new counter, current_unassociated_qpairs,
to account for these qpairs. We protect this counter
with a lock, since the accept thread will increment
the counter, and the poll group thread will
decrement it when the qpair receives the CONNECT
allowing us to associated with a subsystem/controller..
If the qpair gets destroyed before the CONNECT is
received, we can use the qpair->connect_received
flag to decrement current_unassociated_qpairs.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I8bba8da2abfe225b3b9f981cd71b6f49e2b87391
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/15693
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@nvidia.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Konrad Sztyber <konrad.sztyber@intel.com>
Currently we use qpair->ctrlr at qpair destroy
time to decide if we need to decrement the
qpair's poll group's qpair count. But this is
not correct - these counters get incremented
when the CONNECT is received, but qpair->ctrlr
doesn't get set until later.
So add a new connect_received bool to the spdk_nvmf_qpair.
Use this instead to determine when we should decrement
the poll group qpair counters.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I174a0fda36c4558171953bf58f2f5117bc074f76
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/15692
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@nvidia.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Konrad Sztyber <konrad.sztyber@intel.com>
per Intel policy to include file commit date using git cmd
below. The policy does not apply to non-Intel (C) notices.
git log --follow -C90% --format=%ad --date default <file> | tail -1
and then pull just the 4 digit year from the result.
Intel copyrights were not added to files where Intel either had
no contribution ot the contribution lacked substance (ie license
header updates, formatting changes, etc). Contribution date used
"--follow -C95%" to get the most accurate date.
Note that several files in this patch didn't end the license/(c)
block with a blank comment line so these were added as the vast
majority of files do have this last blank line. Simply there for
consistency.
Signed-off-by: paul luse <paul.e.luse@intel.com>
Change-Id: Id5b7ce4f658fe87132f14139ead58d6e285c04d4
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/15192
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Community-CI: Mellanox Build Bot
When emitting the JSON-RPC text for saving the
current configuration, add the listeners last.
This is usually the preferred order when
configuring a new subsystem - it is better to have
all of the namespaces and hosts added to the subsystem
before adding the listener to allow hosts to connect
to it. We support namespace hotplug but there's
no need to unnecessarily generate hotplug events
if we can avoid it.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I79e8a0a496eeb128efbb7e314ac835b6110d3cc8
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/14586
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@nvidia.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
While running into this function, even the subsystem can't be
destroyed due to error subsystem state, it's better to continue
the execution.
Continue to fix#2590, QEMU is stuck for the failure case, and
nvmf target should process such error because it may support other
normal subsystems at the same time.
Change-Id: Ib05e24996378b52070d2b760519f476f9b2d7e76
Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/13839
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@nvidia.com>
Reviewed-by: Dong Yi <dongx.yi@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
it is impl and used only in nvmf.c source file
Signed-off-by: Jacek Kalwas <jacek.kalwas@intel.com>
Change-Id: I1236f9ede28c5da313d118ce73e1da64381379c5
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/13664
Community-CI: Mellanox Build Bot
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Dong Yi <dongx.yi@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@nvidia.com>
Reviewed-by: Konrad Sztyber <konrad.sztyber@intel.com>
In SPDK, declarations have the return type on the same line. Definitions
have the return type on a separate line. Astyle has an option for
enforcing this. Unfortunately, it seems to have two bugs:
1) It doesn't work correctly at all on C++ files.
2) It often fails on functions that return enums, or long type names
Deal with 1) by adjusting the check_format.sh script to only tell astyle
to fix return type line breaks for C files and not C++. Deal with 2) by
adding a few typedefs to work around the problem.
Change-Id: Idf28281466cab8411ce252d5f02ab384166790c6
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/13437
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Dong Yi <dongx.yi@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
If the broadcast NSID is supplied, every namespace is paused.
Change-Id: I40cc3e04b5a75b731ab0c8946ed8146275cc8ee4
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/13394
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Many open source projects have moved to using SPDX identifiers
to specify license information, reducing the amount of
boilerplate code in every source file. This patch replaces
the bulk of SPDK .c, .cpp and Makefiles with the BSD-3-Clause
identifier.
Almost all of these files share the exact same license text,
and this patch only modifies the files that contain the
most common license text. There can be slight variations
because the third clause contains company names - most say
"Intel Corporation", but there are instances for Nvidia,
Samsung, Eideticom and even "the copyright holder".
Used a bash script to automate replacement of the license text
with SPDX identifier which is checked into scripts/spdx.sh.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Iaa88ab5e92ea471691dc298cfe41ebfb5d169780
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/12904
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@nvidia.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Dong Yi <dongx.yi@intel.com>
Reviewed-by: Konrad Sztyber <konrad.sztyber@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: <qun.wan@intel.com>
For the benefit of forthcoming vfio-user changes, register the poll
group poller prior to calling the transport create callback, and pass in
a pointer to the poll group itself.
Signed-off-by: John Levon <john.levon@nutanix.com>
Change-Id: Idbc24126c9d46f8162e4ded07c5a0ecf074fc7dd
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10718
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Shuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
On failure, we weren't cleaning up the poll group data properly, and in
one place, we were trying to remove ourselves from the tgt-> list prior
to being on it.
Signed-off-by: John Levon <john.levon@nutanix.com>
Change-Id: I9bbe5847b3703eba1ee1d762392ad3159a74ac8b
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10717
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
There's no need to forward-declare this, when we can just place it
before its consumers, and this will also help follow-up fixes.
Signed-off-by: John Levon <john.levon@nutanix.com>
Change-Id: I201bd966371db76a3b789473041799bf55b13c95
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/11437
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
There is a race condition between controller destruction and
subsystem state change, e.g. admin qpair may already be freed
when a namespace is added or removed. As result in function
poll_group_update_subsystem we may get heap-use-after-free error
Another problem is that some qpair's live time may exceed controller's
life time. To avoid it, start controller destruction process when the last
qpair finished the disconnect process (previously controller started
the descruction process before the last qpair starts to disconnect
and it could lead to raise conditions)
Fixes#2055
Signed-off-by: Alexey Marchuk <alexeymar@mellanox.com>
Change-Id: Ibc99b1d840e4796e1588cc217d65834bb556b909
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9995
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Add dtrace probes aroung qpair/controller/subsystem management
to help with debugging issue #2055.
Change-Id: I0b981bffadee3fe4172ad6916c059bf357959dde
Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10237
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Zero-copy requests are kept on the outstanding queue for the whole
duration of the request - from the initial zcopy_start submission to the
completion of zcopy_end. This means, that there's a period in which a
request doesn't wait for a completion from the bdev layer, but is still
on the oustanding queue (after zcopy_start callback, before zcopy_end
submit). If a qpair gets disconnected while a request is in this state,
we need to manually force its completion, as otherwise it might hang
indefinitely (e.g. waiting for host data).
Signed-off-by: Konrad Sztyber <konrad.sztyber@intel.com>
Change-Id: I53731b8e363b725efa564ca3c7d89b46f5fb2a24
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10793
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
The zero-copy requests can also be queued when a subsystem is paused, so
we need to properly resume and submit them by using zcopy_start.
Since only requests that haven't received the zero-copy buffer (i.e.
before zcopy_start was called) can be queued, we don't need to bother
with checking zcopy_phase.
Signed-off-by: Konrad Sztyber <konrad.sztyber@intel.com>
Change-Id: Ie629688f6961eb2ae05741df496720b91be4d80d
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10792
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Not every transport requires accept poller - transport specific
layer can have its own policy and way of handling new connection.
APIs to notify generic layer are already in place
- spdk_nvmf_poll_group_add
- spdk_nvmf_tgt_new_qpair
Having accept poller removed should simplify interrupt mode impl
in transport specific layer.
Fixes issue #1876
Change-Id: Ia6cac0c2da67a298e88956734c50fb6e6b7521f1
Signed-off-by: Jacek Kalwas <jacek.kalwas@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/7268
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
in case of failure groups shall be destroyed
Signed-off-by: Jacek Kalwas <jacek.kalwas@intel.com>
Change-Id: I8933f4128a7a3361bbb55d6a9c08a540521e5bda
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10435
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
adding transport to tgt should be the last step
also there is an issue before change i.e. if calloc failed then
transport remains on the list
Signed-off-by: Jacek Kalwas <jacek.kalwas@intel.com>
Change-Id: Iaf29cfc7b0f535d40160c6fdf9ef6a7e6bfb127c
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10429
Community-CI: Mellanox Build Bot
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
SPDK nvmf target reports all listeners on all subsystems
in discovery pages, kernel target reports only subsystems
listening on a port where discovery command is received.
NVMEoF specification allows to specify any addresses/
transport types. Ch 5: The set of Discovery Log entries should
include all applicable addresses on the same fabric as the
Discovery Service and may include addresses on other fabrics.
To align SPDK and kernel targets behaviour, add filtering
rules to allow flexible configuration of what should be
listed in discovery log page entries.
Fixes#2082
Signed-off-by: Alexey Marchuk <alexeymar@mellanox.com>
Change-Id: Ie981edebb29206793d3310940034dcbb22c52441
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9185
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
This reverts commit 3b1f13ef29.
It seems like this particular commit is causing failures on the
CI side related to the following issue:
https://github.com/spdk/spdk/issues/2214
Reverting for now to make the CI stable.
Signed-off-by: Michal Berger <michalx.berger@intel.com>
Change-Id: I7f32c612b8549477de0b9079d114cec4ec9bda59
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9986
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
There is a race condition between controller destruction and
subsystem state change, e.g. admin qpair may already be freed
when a namespace is added or removed. As result in function
poll_group_update_subsystem we may get heap-use-after-free error
Another problem is that some qpair's live time may exceed controller's
life time. To avoid it, start controller destruction process when the last
qpair finished the disconnect process (previously controller started
the descruction process before the last qpair starts to disconnect
and it could lead to raise conditions)
Fixes#2055
Change-Id: I11612979eb914a5fdcc7b9e3c812bf1e450b6120
Signed-off-by: Alexey Marchuk <alexeymar@mellanox.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8963
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Matt Dumm <matt.dumm@hpe.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
An error might occur after succesful transport creation
when the new transport is added to nvmf poll groups, e.g.
in nvmf_transport_poll_group_create. In that case
transport is not detroyed and poll groups are not fully
functional. To correct this behaviour, destroy transport if
spdk_nvmf_tgt_add_transport fails. Also update nvmf_tgt
initialization step to check that all poll groups were
created.
Change-Id: I116e6944729d846c1755c2844c77825f65db8c12
Signed-off-by: Alexey Marchuk <alexeymar@mellanox.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9255
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Jacek Kalwas <jacek.kalwas@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
When a subsystem is being deleted, we disconnect all qpairs
and when the last qpair for some controller is disconnected,
we start controller desctruction process. This process requires
to send a message to subsystem's thread to remove the controller
from the list in the subsystem and after that send a message to
controller's thread to release resources.
The problem is that the subsystem also destroys all attached
controllers. This order is unpredictable and we may get
heap-use-after-free or double free.
To fix this problem we can rely on the fact that the subsystem
can only be destroyed in incative state, that means that all
qpairs linked to the subsystem are already disconnected and
all controllers are already destroyed or in the process of
destruction.
spdk_nvmf_subsystem_destroy API is now can be asyncrhonous,
it accepts a callback with cb argument.
Change-Id: Ic72d69200bc8302dae2f8cd8ca44bc640c6a8116
Signed-off-by: Alexey Marchuk <alexeymar@mellanox.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6660
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
We could not restore the setting of ana_reporting because it was not
included in the JSON config dump.
Add the parameter ana_reporting into JSON config dump by adding and
using a new helper function nvmf_subsystem_get_ana_reporting().
Besides, previously the JSON RPC nvmf_subsystem_get_listeners had
ana_state regardless of the value of ana_reporting. We make it
conditional in this patch. The JSON RPC nvmf_subsystem_get_listeners
had not been used in the test code in the repository. Hence this
change will be acceptable.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ia4e04600c969c254e0a816d3eb34983ee951091e
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9111
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Monica Kenguva <monica.kenguva@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Implemented nvmf code to allow transports to use ZCOPY. Note ZCOPY
has to be enabled within the individual transport layer
Signed-off-by: matthewb <matthew.burbridge@hpe.com>
Change-Id: I273b3d4ab44d882c916ac39e821505e1f4211ded
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6817
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Xiaodong Liu <xiaodong.liu@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
It is possible for a controller to get added to the
subsystem before its admin_qpair has been assigned.
We need to account for that when traversing the subsystem's
ctrlr list when determining ns and ana_changes that need
to be reported for the ctrlr.
Found while doing stress testing with connects and
subsystem ns add/remove.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Ie54dc6ac202faeaeace054e6599f2dea2f30211e
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8570
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Pollers are supposed to return SPDK_POLLER_{BUSY,IDLE}.
Signed-off-by: John Levon <john.levon@nutanix.com>
Change-Id: I92bd184aaba9e3efb730b68a6024ebc9757ffd8b
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8559
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Konrad Sztyber <konrad.sztyber@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
When a qpair is destroyed and the qpair is the last,
_nvmf_ctrlr_free_from_qpair() (in lib/nvmf/nvmf.c) sends two messages,
one is for _nvmf_ctrlr_destruct() and another is for
_nvmf_transport_qpair_fini().
We do not know which of two completes earlier.
_nvmf_ctrlr_destruct() frees the qpair->ctrlr in the end.
On the other hand, _nvmf_ctrlr_free_from_qpair() calls
spdk_nvmf_poll_group_remove() in the end, and spdk_nvmf_poll_group_remove()
accesses the qpair->ctrlr to free queued requests to the qpair.
Before one recent change, spdk_nvmf_poll_group_remove() had been called
before _nvmf_ctrlr_free_from_qpair() was called.
Hence extrace the operation to free queued requests from
spdk_nvmf_poll_group_remove() and inline it into _nvmf_qpair_destroy().
Fixes one showstopper error to investigate the issue reported in #1819.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I29c43ff7b289fc77a5de9c33e0266301c412e208
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/8438
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: <dongx.yi@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Community-CI: Mellanox Build Bot
