Give this a more generic name. We're going to be using these
events for more than just hotremove coming up.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Change-Id: Ia5356e9ab809807ba4d85ecc212a496e96012bce
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3559
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
We can OR some of the bits ,and setting them one time.
Signed-off-by: yidong0635 <dongx.yi@intel.com>
Change-Id: Ie919ef66f3b8deea125df47d2194130b364c302a
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3489
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
This reverts commit 6194cb2e15.
It's unclear whether we need to add a new API for the env layer
for upcoming work. Nothing currently uses it. When we have a clear
need, we can add this back in.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Change-Id: I174276799d650a1365b37a737271a54a796cd455
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3561
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This will allow us to resize the backend bdev of vhost-blk
and notify the guest OS that the capactiy of virtio-blk
disk has been resized.
The spdk api entry is `spdk_bdev_notify_blockcnt_change`.
Any bdev if used as vhost-blk backend may need to implement
a rpc that calls this function.
Related DPDK patch has been merged and release in 20.02.
https://www.mail-archive.com/dev@dpdk.org/msg153365.html
Change-Id: I961c61de0fc03e210d776035a40f3a4adfa9b4f3
Signed-off-by: Li Feng <fengli@smartx.com>
Signed-off-by: Jin Yu <jin.yu@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1468
Community-CI: Mellanox Build Bot
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Added to the framework as well as all 3 engines. Needed by apps
in the event that they have to fail following the creation of a
batch, allows them to tell the framework to forget about the batch
as they have no intent to send it.
Signed-off-by: paul luse <paul.e.luse@intel.com>
Change-Id: Id94754ab1350e5a969a5fd2306bd59c38f0a0120
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3389
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: Ben Walker <benjamin.walker@intel.com>
When a process cleans up IO qpairs from another crashed
process in a multi-process environment, we must not try to
abort reqs for that IO qpair. Any reqs will contain callbacks
for the crashed process which we must not try to execute in
a different process.
Fixes issue #1509.
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I5e58cce7bdb86e3feb4084733815c086901f867e
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3536
Community-CI: Mellanox Build Bot
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Since rqpair->qpair.group is set to NULL when we remove the
qpair from poll group, we fail to send event to qpair's thread.
This patch adds a pointer to io_chaneel to spdk_nvmf_rdma_qpair
structure and a function to handle poll_group_remove transport
operation. In this function we get io_channel from nvmf_tgt,
this channel will be used to get a thread for sending
async event notification. This also guarantees that the thread
will be alive while we are destroying qpair.
Change-Id: I1222be9f9004304ba0a90edf6d56d316d014efda
Signed-off-by: Alexey Marchuk <alexeymar@mellanox.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3475
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jacek Kalwas <jacek.kalwas@intel.com>
Reviewed-by: Seth Howell <seth.howell@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Makefile support in DPDK was deprecated and will be removed soon,
so switch to the officially supported way of building DPDK -
with meson and ninja. Two new tools. Basically, our Makefiles
will invoke meson+ninja for DPDK, no other SPDK components are
affected.
Apparently DPDK wanted to move away from an octopus-like config
system and the ideology behind meson configuration is simple now:
build everything by default. Some PMDs can be explicitly disabled
with meson command line, but all libraries (both static and shared
versions) and test apps are built unconditionally.
How long does it take to build minimal DPDK with meson? Too much.
On my machine half of the total build time is spent on libraries
we don't need at all. (I have some hacks up my sleeve to disable
building those libraries - see the subsequent patch.) As for the
official way of building a minimal DPDK, there was a patch [1]
on dpdk mailing list to introduce more specific configuration,
but it was rejected:
> We talked about this a few times in the past, and it was actually one
> of the design goals to _avoid_ replicating the octopus-like config
> system of the makefiles. That's because it makes the test matrix
> insanely complicated, not to mention the harm to user friendliness,
> among other things.
>
> If someone doesn't want to use a PMD, they can just avoid installing it
> - it's simple enough.
>
> Sorry, but from me it's a very strong NACK.
Let's not follow that direction, hack the DPDK build system instead.
As for advantages of meson+ninja over Makefiles? I can't find any.
It's another build system that does a lot for you with some functions,
magic options, and a built-in dependency system. It seems nice if you know
the syntax, but it's another component that you need to learn, debug,
and possibly find bugs in (there's a lot of github issues open for meson).
I would compare it to CMake.
As for changes in this patch: rather that explicitly disabling
PMDs we don't need, specify a list of PMDs we do need and disable
everything else found in ./dpdk/drivers/*. This way we won't have
to disable the new PMDs as they're added to DPDK.
Meson configuration also sets RTE_EAL_PMD_PATH #define to a valid directory
with built PMD shared libs. When it's set, DPDK dynamically loads all shared
libraries inside. The drivers there depend on DPDK shared libs and fail to
load in static SPDK builds, so we disable them altogether by unsetting
RTE_EAL_PMD_PATH in the meson-generated config file - just like
DPDK Makefiles did. EAL checks for RTE_EAL_PMD_PATH being empty and skips
loading any external PMDs then. We do it for both static and shared libs.
We specify all PMDs at build time for now, so there's just no need to load
them dynamically.
We have three more hacks in our submodule:
* disable building dpdk apps by commenting-out a line in dpdk/meson.build
* disable building unnecessary libs (build everything that spdk *may*
need)
* build isa-l compress pmd with `-L[...] -lisal`. DPDK expects to find
libisal with pkg-config. We don't want to prepare a pkg-config file,
so comment-out a failing check in another meson.build file and provide
isa-l through CFLAGS and LDFLAGS.
We also need to make some changes to our test/external_code. First of
all, -ldpdk is no more. Meson build generates a pkg-config file with all
libs, but we'll switch to it in a separate patch - for now just specify
all -lrte_ libs one by one. -Wl,--no-as-needed has to be added to some
test cases, otherwise rte_mempool_ring isn't loaded. We don't use any
APIs from this library, it only has a static constructor that provides
a few callbacks used by rte_mempool_create(). Also, since DPDK now builds
both static and shared libraries, we need to add -Wl,-Bstatic to force
using static libswhere required. It's only needed for DPDK libs, but we
use it for SPDK libs as well since there's no harm.
As for performance:
$ ./configure --enable-debug --with-crypto --with-reduce
$ time make -j40 -C dpdkbuild all
with meson:
real 0m8.287s
user 1m7.983s
sys 0m10.548s
before, with the old DPDK makefiles:
real 0m20.232s
user 0m55.921s
sys 0m16.491s
The subsequent builds are much faster too:
$ time make -j40 -C dpdkbuild all
meson:
real 0m0.876s
user 0m0.663s
sys 0m0.217s
makefiles:
real 0m10.150s
user 0m11.740s
sys 0m6.772s
[1] http://inbox.dpdk.org/dev/1a07d1cd59d84dce84e56c10fdabf5e5504560a6.camel@debian.org/
Change-Id: Ic65db563014100bafb12e61ee0530cc2ae64401d
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Signed-off-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1440
Community-CI: Mellanox Build Bot
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>
Now that drivers can be registered from upper layers there's
no need to keep them centralized inside env.
(check_format.sh complains that spdk_pci_nvme_get_driver() shouldn't
start with the spdk_ prefix - to workaround that we move the function
declaration from one place in env.h to another - that's enough to
convince check_format it really is a public function)
Change-Id: If86aebd6c997349569c71430ec815b413eb44ef8
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3187
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Jacek Kalwas <jacek.kalwas@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
This allows SPDK apps to register new PCI drivers outside of
the env layer, enabling SPDK as a whole with new use cases.
Change-Id: I0c998a9ec249c3ca610b7b3b8b6caf616b16f64c
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3185
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Jacek Kalwas <jacek.kalwas@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
An alternative to spdk_pci_*_get_driver();
Change-Id: I20a80b3c655a37fb1c76da21c2b70d5678041fab
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3186
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Jacek Kalwas <jacek.kalwas@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
We want to make struct spdk_pci_driver public, so add env-agnostic
fields that define a driver directly to that struct.
PCI driver registration (potentially in upper layers) will only use
spdk_pci_id-s and spdk pci drv_flags, then those will be translated
to DPDK equivalents inside env_dpdk.
Change-Id: Ia24ecfc99ebf0f54f096eaf27bca5ed9c0dfe01d
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Signed-off-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3183
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Not needed with new scheme around accel_task in prior patches.
Signed-off-by: paul luse <paul.e.luse@intel.com>
Change-Id: Ic6f16654db62fe1dc51b0822f0ff92261da4494c
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3212
Community-CI: Mellanox Build Bot
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>
This was sort of a clunky interface requiring a couple of inline
functions in every app that wants to use the accel_fw moving
forward. By having the accel_fw public API accept a callback arg
instead of an accel_task combined with adding a pool of accel_tasks
in the accel_fw engine we can eliminate this.
After changing the parm to a cb_arg, changes were made to all accel_fw
interfaces to put cb_fn and cb_arg as the last parms in public and
private function calls.
Related bdev_malloc changes need to be in this patch in order to pass CI.
Signed-off-by: paul luse <paul.e.luse@intel.com>
Change-Id: I2b75764e534562d91484a094c3352266156d8425
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3209
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: Ben Walker <benjamin.walker@intel.com>
This function hasn't kept up properly with the states that
we use for tracking the qpair lifecycle.
Add checks for NVME_QPAIR_DISCONNECTING and NVME_QPAIR_DESTROYING.
Signed-off-by: Seth Howell <seth.howell@intel.com>
Change-Id: I51607d4f00e94937b08fca28e766163580d46461
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3359
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
With this patch, spdk_app_start/stop can be repeatedly
called by users based on their upper level application's
requirement.
Changes are:
* Add reinit ability inside spdk_env_init and related functions
* Clear g_shutdown_sig_received in spdk_app_setup_signal_handlers
* Clear malloc_disk_count in bdev_malloc_initialize
Change-Id: I2d7be52b0e4aac2cb6734cc1237ce72d33b6de0c
Signed-off-by: Xiaodong Liu <xiaodong.liu@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2260
Community-CI: Mellanox Build Bot
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Set g_external_init to false when the function calling
is succeed in the end of function spdk_env_init.
Change-Id: I3fc4b4c87b11ca34354efafdd9644c077309c638
Signed-off-by: Xiaodong Liu <xiaodong.liu@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3455
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: GangCao <gang.cao@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
The new abort functionality doesn't take custom admin cmd
handlers into account.
This commit allows setting a custom admin cmd handler
for abort that provides the ability to influence the
bdev lookup to which the abort is sent to.
Signed-off-by: Michael Haeuptle <michael.haeuptle@hpe.com>
Change-Id: I3a66c6f863f5ee4d89cb2194dffdc6855945fa8a
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3485
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
ABI dependency check tool complained the following:
00:06:10.012 ************************************
00:06:10.012 START TEST confirm_abi_deps
00:06:10.012 ************************************
00:06:11.559 No corresponding object for libspdk_bdev_uring.so in canonical directory. Skipping.
00:06:12.237 No corresponding object for libspdk_event_sock.so in canonical directory. Skipping.
00:06:12.485 Please update the major SO version for libspdk_ioat.so. A header accesible type has been modified since last release.
00:06:15.388 No corresponding object for libspdk_rdma.so in canonical directory. Skipping.
00:06:15.663 No corresponding object for libspdk_sock_uring.so in canonical directory. Skipping.
00:06:15.954 Processed 55 objects.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I2a6cc91ac56ed693ae0676cf64e01ed3ff620ffc
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3511
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Zero copy send can cause performance degradation with small
payloads. This patch adds an option to disable it if required. By
default zero copy is enabled.
Signed-off-by: Evgeniy Kochetov <evgeniik@mellanox.com>
Change-Id: I14f2b21ad375e770cb08f850360898bac675b351
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3344
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Receive pipe reduces number of system calls and gives significant
performance improvement with kernel TCP stack and relatively small IO
sizes. With user space TCP/IP implementations there are no system
calls and double buffering introduced by pipe has negative impact on
performance. Receive pipe remains enabled by default.
Signed-off-by: Evgeniy Kochetov <evgeniik@mellanox.com>
Change-Id: Ic5ddee42293df2c233ba7ffbe6662de7917ac586
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3343
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
struct spdk_nvmf_request holds req_to_abort and so passing req_to_abort
separately is not really necessary now. The internal API
nvmf_ctrlr_abort_request() was added at the stage of prototyping.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I9ef2467d6f92422f044650c62a0777b95c0fc1ab
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3488
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Now that we support only DPDK 18.11+ and always have
to register pci drivers to DPDK on initialization we
don't need that flag - it's always true.
Change-Id: Ibf1d79155595609fe9093f58e056bea25db6fdb2
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3446
Reviewed-by: Jacek Kalwas <jacek.kalwas@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
This follows struct rte_pci_id which had class_id as well.
We'll need it to make some additional DPDK APIs public through
the env abstraction.
Change-Id: I794a6cd6b17e48daf53b48fa5abe3d3dcfeaa403
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3182
Reviewed-by: Jacek Kalwas <jacek.kalwas@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
You don't get notified when someone starts using your hooked
device, so there's not much gain from knowing when someone
stops.
Remove that callback and also move DPDK device detach under
the same lock which sets the pending_removal flag. This eliminates
a data race window when hotremove notification could arrive
after device was detached, but before it was scheduled to be
removed.
vmd and ioat nest the spdk_pci_device struct and abigail complains
even though the parent structs only have forward declarations in
public headers. Adding those two structs to the suppression list
doesn't help though. Abidiff still complains about the pci device
struct being changed, probably because ioat.h and vmd.h both include
env.h. Abidiff suppresion list should eventually be split per-lib,
but for now ignore struct spdk_pci_device changes globally.
$ abidiff [...]/libspdk_ioat.so [...]
'struct spdk_pci_device at env.h:652:1' changed:
type size changed from 1024 to 960 (in bits)
1 data member deletion:
<SNIP>
Change-Id: I9b113572c661f0e0786b6d625e16dc07fe77e778
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2939
Community-CI: Mellanox Build Bot
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
A workaround for kernel deadlocks surfaced in #1275.
DPDK basically offers two APIs for hotplugging all PCI devices:
rte_bus_scan() and rte_bus_probe(). Scan iterates through
/sys/bus/pci/devices/* and creates corresponding rte_pci_device-s,
then rte_bus_probe() tries to initialize each device with the
supporting driver.
Previously we did scan and probe together, one after another, now
we'll have an intermediate step. After scanning the bus, we'll
iterate through all rte_pci_device-s and temporarily blacklist any
newly detected devices. We'll use devargs->data field to a store
a timeout value (integer) after which the device can be un-blacklisted
and initialized. devargs->data is documented in DPDK as "Device
string storage" and it's a char*, but it's not referenced anywhere
in DPDK. rte_bus_probe() respects the blacklist and doesn't do
absolutely anything with blacklisted ones.
The timeout value is 2 seconds, which should be plenty enough
for an NVMe device to reset, leave the critical lock sections in
kernel, and let us initialize it safely.
Note that direct attach by BDF doesn't respect the blacklist,
so an NVMe attach RPC won't be delayed in any way, it will continue
to work as it always did. Only the automatic discovery & enumeration
is deferred.
Change-Id: I62b719271bd0755bc2882331ea33f69897b1e5e5
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1733
Community-CI: Mellanox Build Bot
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>
Extensive testing showed it can fail:
> EAL: eal_parse_sysfs_value(): cannot open sysfs value
> /sys/bus/pci/devices/0000:02:00.0/vendor
> EAL: Scan for (pci) bus failed.
spdk_pci_enumerate() would previously return with error because
of this and e.g. the test nvme hotplug app could immediately exit
with failure. A mis-timed scan shouldn't cause this kind of failure,
so ignore it's return code. This shouldn't cause any issues.
Change-Id: I9253219c218981a747774a8632335963cfb0db53
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2941
Community-CI: Mellanox Build Bot
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: <dongx.yi@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
This is an attempt to workaround a kernel data race spotted in #1275.
We have to delay our VFIO interraction until after the kernel handles
the PCIe hotremoval. Otherwise we risk having a deadlock inside the
kernel. We could put a dummy delay in SPDK hotremove routine, but we
don't know exactly how long we have to wait, so wait for the VFIO
hotremove notification to arrive instead. From what I understand it
is always sent after the kerner already releases the critical locks.
This patch breaks hotremove for VFIO devices when running with
DPDK version < 18.05, but those reached EOL already and their support
is removed from SPDK later within this series.
It would be nice to wait for this VFIO notification inside lib/env_dpdk
e.g. inside the detach function, but we don't know there why the
detach is happening. It could be triggered by a user RPC, not physical
device hotremoval.
Change-Id: I2727de923bb2e3e05a83202465330a32a77d7cfc
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1732
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: Ben Walker <benjamin.walker@intel.com>
The hooked spdk_pci_device was being removed when someone
stopped using it (detached from it). The following didn't
work:
$ rpc.py bdev_nvme_attach_controller 10000:00:01.0
$ rpc.py bdev_nvme_detach_controller 10000:00:01.0
$ rpc.py bdev_nvme_attach_controller 10000:00:01.0
The second attach can't find PCI device 10000:00:01.0.
To fix it, simply don't do anything in VMD on device detach.
The entire callback will be removed soon. You don't get
notified when someone starts using your hooked device, so
there's not much gain from knowing when someone stops.
Change-Id: I147c24609077be24182ecfd2e273988a2e9e26b2
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2938
Community-CI: Mellanox Build Bot
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: Michael Haeuptle <michaelhaeuptle@gmail.com>
There was a chance we scheduled a device removal to the DPDK thread
while that thread was already removing the device from a VFIO hotremove
notification (on the DPDK interrupt thread). The second hotremove
attempt touches some freed memory and segfaults.
The VFIO hotremove notification already checks pending_removal flag
under a mutex and sets it to true, so do the same in spdk_detach_rte()
(called from the SPDK init thread).
Change-Id: Ib3f0eb7c0c5c6e1ab8cf253b7711fd149925a143
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1730
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Simplify the code path a bit. VFIO notification is the only
place where detach callback is called from the dpdk intr thread.
Detach checks the current thread and behaves differently in this
case, but it could be the VFIO notification that simply calls
a different function.
So instead of carrying the VFIO notification through the generic
detach routine, carry it just through the DPDK-thread specific
subset. This lets us remove some ifs in the generic routine.
Change-Id: I5e8866e4643ef08fb3cd12621e2d262b5e827c74
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1731
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This reverts commit 301c5aeec9.
The patch doesn't fix anything as the hotremoval could be still
called twice and the second call would do use-after-free.
Change-Id: I78a1120707dbdf36c871ec378a312c4a058fc76b
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1729
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This function can fail with -EAGAIN or -ENOMEM so we
should really check the return value.
Change-Id: I4a443351f3c85032f47e8af9e70b6b71ba3413f5
Signed-off-by: Seth Howell <seth.howell@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3240
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Vitaliy Mysak <vitaliy.mysak@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Also one small bug fix w/compare in accel_perf as a result
of changes made in accel_perf sicne base compare was added.
Signed-off-by: paul luse <paul.e.luse@intel.com>
Change-Id: Id8e67bd9de9cbd006ac148f4a77807cc3e8e662b
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2958
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: Mellanox Build Bot
Also update accel_perf to support it.
Signed-off-by: paul luse <paul.e.luse@intel.com>
Change-Id: Ida7b863de02c64239ec4cfbdc3b0235d4e0521f9
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2954
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Also as a minor drive-by, added verify capability for fill that
wasn't there before, useful in making sure the prep function
was working and really should have been there anyway.
idxd support for prep fill will follow.
Signed-off-by: paul luse <paul.e.luse@intel.com>
Change-Id: Ib54311c1fb98abd2fb61df6603cf3c5300b71161
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2952
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
When an engine has not filled out an entry point for an accel_fw
function, call the sw implmentation that's built into the accel_fw
transparently.
Also update the accel_perf app to no longer block attempts to use
capabilities not reported as supported by the selected engine but
instead to print an informational message.
The exception is batch related commands as their implementation
is specific to the engine.
Signed-off-by: paul luse <paul.e.luse@intel.com>
Change-Id: I0b3971d075ced20182f97e95147e8887103e68cd
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2930
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
Adds support for both the accel fw and the sw implementation.
Signed-off-by: paul luse <paul.e.luse@intel.com>
Change-Id: Ib8748f272a688a074ca475a86dea14179acc020d
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2928
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Add disable_sections_merge() procedure that will allow
to have multiple sections with a same name.
This behaviour is how FIO treats such sections
and so will be used in bdevperf config file.
Change-Id: If221daeb7753d91b5d2608d25ccbb16f2d43ccce
Signed-off-by: Vitaliy Mysak <vitaliy.mysak@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3433
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
When there are many Target information which exceeds the data
buffer, the initiator and target can use long text responses,
but SPDK doesn't support this feature now, so here we just
report an error here, we may add this feature in following
patches.
Fix issue #1442.
Change-Id: I5962c00943819cd11ce21e610e4b37265451f0cb
Signed-off-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3454
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
1 Change the default factor from 4 to 8, which can be used
to improve the performance.
2 Change the base buffer size in nvme_tcp.c,
we should not use sizeof(struct spdk_nvme_tcp_cmd),
it is 72 bytes. Normally, the initiator will receive
C2h pdus and R2T Pdus by most, so set the size of using
sizeof(struct spdk_nvme_tcp_c2h_data_hdr) is enough.
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Change-Id: I384f4cb026cb8d83e75b639f7256ee8cb8ed1df1
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3283
Community-CI: Mellanox Build Bot
Community-CI: Broadcom CI
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: John Kariuki <John.K.Kariuki@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
There is no reason to continue processing these requests if the
qpair is not still active. We should complete them and free
any resources they are still holding.
Also, not doing so can cause issues with trying to access pointers
in the qpair after they are invalid. See issue #1460.
Signed-off-by: Seth Howell <seth.howell@intel.com>
Change-Id: I6e570a576983dfedf726dc4a9a83316209403e00
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3451
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Includes a few related changes:
* slight refactor to have common prep function for batch prep
commands in the sw engine
* added support for dualcast batching in accel_perf example app
Signed-off-by: paul luse <paul.e.luse@intel.com>
Change-Id: If9f078a0f8eb509d296e60a94331c92a72f8cb10
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2913
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: Ben Walker <benjamin.walker@intel.com>
Doesn't provide any performance benefit but lets apps consistently
use the batch interface regardless of engine. Additional functions
and test code to follow.
Signed-off-by: paul luse <paul.e.luse@intel.com>
Change-Id: Ia737045560ddd5117a689f7715fa206268bbc13d
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2911
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Make the abort execution timeout value as optional.
Zero is acceptable and means immediate timeout.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ia4b03c65b8bd15899f48be9476ee657446147581
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3104
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
If the state of the request is TRANSFERRING_HOST_TO_CONTROLLER,
we cannot abort it now but may be able to abort it when its state
is EXECUTING. Hence wait until its state is EXECUTING, and then
retry aborting.
The following patch will make the timeout value configurable as
an new transport option.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ia4b43e79c3b0d9c53ed04b01a9eaa9b117b32d81
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3013
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>
If the request is queued and is not in completing, we can abort
it safely.
If the state of the request is NEED_BUFFERING, the request is
queued to tqpair->group->group.pending_buf_queue.
If the state of the request is DATA_TRANSFER_TO_CONTROLLER_PENDING,
the request is queued to rqpair->pending_rdma_read_queue.
If the state of the request is DATA_TRANSFER_TO_HOST_PENDING,
the request is queued to rqpair->pending_rdma_write_queue.
According to the current state, dequeue from the corresponding
queue, and then call an new helper function
nvmf_rdma_request_set_abort_status().
Using helper function will be easier to read.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Id0327f4d2c4728a11b3b6bbc7c2252f0b35263cf
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3012
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Call nvmf_ctrlr_abort_request() if the request whose CID matches
is found and its state is executing.
nvmf_rdma_qpair_abort_request() returns immediately if rc is
SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS, or calls
spdk_nvmf_request_complete() otherwise.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I1462a21db7270f3d63f8f293ad4be61d52e74da1
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3011
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
If the state of the request is TRANSFERRING_HOST_TO_CONTROLLER,
we cannot abort it now but may be able to abort it when its state
is EXECUTING. Hence wait until its state is EXECUTING, and then
retry aborting.
The following patch will make the timeout value configurable as
an new transport option.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I98347b68e8b6b4a804c47894964cb81eae215aaa
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3010
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
In my mind, we should firstly remove the sock from the group, then
all the call_cb of pdus will be handled, then we clear the task,
we can avoid the duplicated resource recycling.
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Change-Id: Ic5fad130e58c9dcb42a237973979b042eabdf43a
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3309
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
If the request is queued and is not in completing, we can abort
it safely.
If the state of the request is NEED_BUFFERING, the request is
queued to both tqpair->group->group.pending_buf_queue and
the queue per state.
If the state is AWAITING_R2T_ACK, the request is queued to the
queue per state.
Dequeueing from the queue per state is done in
nvmf_tcp_req_set_state(). Hence explicit dequeuing only when the
state of the request is NEED_BUFFERING.
Most abort operation is common between two cases. We can use fallthrough
in switch-case but factor out the common operation into a helper
function nvmf_tcp_req_set_abort_status() instead because we may use
the helper function in future and using helper function is easier to
read than fallthrough.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I1695b084d5d1f2537fbdd512bc3cd136e0f6a65b
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3009
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Call nvmf_ctrlr_abort_request() if the request whose CID matches
is found and its state is executing.
nvmf_tcp_qpair_abort_request() returns immediately if rc is
SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS or calls
spdk_nvmf_request_complete() otherwise.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I1abceecc211ee79d8ac18a82dc63b13d313a6f27
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3008
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: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
State machine is different among NVMe-oF transports and is
encapsulated to the transport neutral NVMe-oF controller and
NVMe-oF qpair.
To implement abort operation for each NVMe-oF transport,
add a function pointer qpair_abort_request to struct spdk_nvmf_transport_ops
and a stub nvmf_transport_qpair_abort_request() to encapsulate
which transport is used.
The following patches will implement qpair_abort_request for each
transport. Each qpair_abort_request() is responsible to call
spdk_nvmf_request_complete() for the abort request.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I2beac959ed428c5108cf33691226b7fae5cd24d6
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3007
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Factor out abort operation on the specific qpair into a helper
function nvmf_qpair_abort_request().
After this refactoring, nvmf_ctrlr_abort_done() calls
_nvmf_request_complete() only if the passed status is zero.
If the passed status is not zero, nvmf_qpair_abort() is responsible
for calling _nvmf_request_complete() instead.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I4828c0e21cc7650210675661d6e1c0fd54c7a2cb
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2991
Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Description is not clear but according to the NVMe specification,
always set the completion status to success and differentiate only
the bit 0 of CDW0 between success and failure for abort command.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I0195e72fe1d7fcc2592f47e9dcf92ac56912282c
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1965
Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
A new API spdk_nvme_ctrlr_cmd_abort_ext() gets cmd_cb_arg as a
parameter, and use it to abort requests whose cb_arg matches cmd_cb_arg.
The caller can set the parameter qpair to NULL if it wants to abort
requests on admin qpair.
Hold ctrlr->ctrlr_lock throughout because abort request is submitted
to admin qpair.
The API is not critical for performance, and so initialize parent
data first.
The API is for a specific qpair, and so hold SQID in the parent,
and it is copied to the children. On the other hand, CID is set
to child when request to abort is found.
Use an new helper function nvme_transport_qpair_iterate_requests()
to add abort request for each outstanding request which has
cmd_cb_arg as its callback context.
The case is possible such that the request to abort is not outstanding
yet but queued. Hence abort queued requests which has cmd_cb_arg
as its callback context too, but it is done only if there is no error
so far.
If only queued requests are aborted and there is no outstanding
request to abort, complete with success synchronously.
If there is no outstanding or queued request to abort, return -ENOENT.
When any abort request is submitted, the difference between success
and failure is only bit 0 of CDW0 according to the NVMe specification.
We cannot the existing helper functions nvme_request_add_child() and
nvme_cb_complete_child() but can use nvme_request_remove_child().
nvme_qpair_submit_request() may use only nvme_request_remove_child()
from these three helper functions. Hence we use req->parent as other
types of request do.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I3a271c6542f8e2e6b425b3bf6151f41e924bc200
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2039
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Use another list dedicated to hold queued requests being aborted
to avoid potential infinite recursive calls.
Add a helper function nvme_qpair_abort_queued_req() to move requests
whose cb_arg matches from qpair->queued_req to qpair->aborted_queued_req.
Then nvme_qpair_resubmit_requests() aborts all requests in
qpair->aborted_queued_req.
The first idea was that nvme_qpair_abort_queued_req() aborts queued
requests directly. However, this caused infinite recursive calls.
Hence separate requesting abort to queued requests and actually
aborting queued requests.
The detail of the infinite recursive calls is as follows:
Some SPDK tool submits the next request from the callback to the completion
of a request in the completion polling loop. For such tool, if the callback
submits a request and then aborts the request immediately, and the request
could not be submitted but queued, it will create infinite recursive calls
by request submit and abort, and it will not be able to get out of
completion polling loop.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I8196182b981bc52dee2074d7642498a5d6ef97d4
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2891
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
In INI config, split key based on whitespace AND symbol '='
This allows to have same format as FIO configs
FIO style config will be used for bdevperf config file
Change-Id: I92c501c839842919afb5c23ed935be010ae2b168
Signed-off-by: Vitaliy Mysak <vitaliy.mysak@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3151
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
We have no particular requirement to keep both conn->outstanding_r2t_tasks
array and conn->active_r2t_tasks list now.
To improve readability and maintaineability, unify two into the latter,
conn->outstanding_r2t_tasks list. Update unit test accordingly.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I25cf7cffbe39ac66e102eb3052340de6ef65c8f1
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3115
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Previously iscsi_del_transfer_task() dequeued the task only from
the array conn->outstanding_r2t_tasks[].
process_non_read_task_completion() had dequeued the task from
the tailq conn->active_r2t_tasks then.
However abort_transfer_task_in_task_mgmt_resp had not dequeued the
task from the tailq conn->active_r2t_tasks then.
This was an apparent bug, and is fixed here. Update unit tests
accordingly.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I93f02b2fb670dcee4c32d61c264e3ad5b4f9f43e
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3108
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Previously, we fixed a similar bug that iSCSI target got seg. fault
if connection is being exited between spdk_iscsi_conn_write_pdu()
and its callback iscsi_conn_login_pdu_success_complete() are executed.
This fix was not sufficient and we still saw similar error.
Not only socket write but also SPDK message is asynchronous and
connection may start exiting between iscsi_conn_schedule() and
iscsi_conn_full_feature_migrate().
This patch fix this new issue.
The previous fix was
https://review.spdk.io/gerrit/c/spdk/spdk/+/1275
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I5cc422529a335e5f1982693bdf910ac7debe6f17
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3074
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Following the last patch, add g_active_conns to manage active connections
by TAILQ. We cannot remove the is_valid bit and g_conns_array
to support iSCSI top application, but this change simplifies the
code a little.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I8b449f3056ffaed19f23f42d83c912dfba9a7b75
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3090
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: Changpeng Liu <changpeng.liu@intel.com>
Previously free connections had been managed by g_conns_array,
and allocate_conn() gets the lowest free connection. This had worked
almost as LIFO, and the just freed connection had been reused
immediately to the new connection.
Using TAILQ makes management of free connections FIFO, and this will
be more intuitive and simpler, and avoid potential issues due to the
fact that we do not know the state INVALID is the current connection
or the current connection is exited and the new connection is allocated.
This patch includes following updates.
Remove the test condition that the connection ID should be zero.
Connection ID is used as Target Transfer Tag (TTT) and TTT is opaque
number. Hence requiring connection ID to be zero is not meaningful.
iscsi_conn_free() calls free_conn() while holding g_conns_mutex, but
iscsi_conn_construct() does not call free_conn() without holding
g_conns_mutex. Hence add g_conns_mutex to the latter.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I204f66469f0bf54845c773da5b4ac86f3c8dca60
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3089
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
This patch only includes the basic framework for batching and the
ability to batch one type of command, copy. Follow-on patches will
add the ability to batch other commands and include an example of
how to do so via the accel perf tool. SW engine support for batching
will also come in a future patch. Documentation will also be coming.
Batching allows the application to submit a list of independent
descriptors to DSA with one single "batch" descriptor. This is beneficial
when the application is in a position to have several operations ready
at once; batching saves the overhead of submitting each one separately.
The way batching works in SPDK is as follows:
1) The app gets a handle to a new batch with spdk_accel_batch_create()
2) The app uses that handle to prepare a command to be included in the
batch. For copy the command is spdk_accel_batch_prep_copy(). The
app many continue to prep commands for the batch up to the max via
calling spdk_accel_batch_get_max()
3) The app then submits the batch with spdk_accel_batch_submit()
4) The callback provided for each command in the batch will be called as
they complete, the callback provided to the batch submit itself will be
called then the entire batch is done.
Signed-off-by: paul luse <paul.e.luse@intel.com>
Change-Id: I4102e9291fe59a245cedde6888f42a923b6dbafd
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2248
Community-CI: Mellanox Build Bot
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>
Logic error Dereference of null pointer ctrlr.c
nvmf_ctrlr_async_event_request 1522
Dereference of null pointer is not possible if sgroup obtained using
ctrlr obj. Adding corresponding asserts suppresses the warning.
Signed-off-by: Jacek Kalwas <jacek.kalwas@intel.com>
Change-Id: I78b32fadd5449ee9b533f65193c70e55cf9a8f1c
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3251
Community-CI: Mellanox Build Bot
Community-CI: Broadcom CI
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Having functions without qpair on the interface allows for wider usage
e.g. by nvmf layer.
Signed-off-by: Jacek Kalwas <jacek.kalwas@intel.com>
Change-Id: I3a51ad53f00eb29e2ba2681ef4ff0cc2a197b65d
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3176
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>
Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
In the case that the configuration field is not set, it will
return "-1" and then implicitly convert to the unsigned
case where the variable is wrongly set.
Change-Id: I7b2c64d653badd731d8e6df46629231343a0ae6d
Signed-off-by: GangCao <gang.cao@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3236
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Xiaodong Liu <xiaodong.liu@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Previous we fix the same issue in this commit:
cb98b2ab3e
But we forget to fix it here. And we also need to update here,
otherwise we will still face the same issue described in
commit: cb98b2ab3e
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Change-Id: I3660dbb6e97c92ea4cb347cfce4bf23c6dfe97ab
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3242
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
This allows users to configure the number of
connection requests outstanding to an rdma port
at once.
RPC included.
Signed-off-by: Seth Howell <seth.howell@intel.com>
Change-Id: I8a2bb86b2fb7565cb10288088d39af763b778703
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3097
Community-CI: Mellanox Build Bot
Community-CI: Broadcom CI
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
This is a preparation to the next patch.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I15356c69e676dc41d3af69caa6d12c1fcb282152
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3071
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>
Reviewed-by: Michael Haeuptle <michaelhaeuptle@gmail.com>
