6432 Commits

Author SHA1 Message Date
Shuhei Matsumoto
3f1e18cbbf lib/iscsi: Factor out operation to open each LUN for connection
This is a preparation to the subsequent patches.

Subsequent patches will introduce struct spdk_iscsi_lun
to refine LUN hotplug process and fix critical issues of it.

This change will make us easy to introduce struct spdk_iscsi_lun.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I75db59d88bb09ee2ea94e8c02e0e87003352850c
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/476112
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>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
2019-12-05 12:21:09 +00:00
Shuhei Matsumoto
22adcd1487 lib/iscsi: Fix orphaned PDUs when freeing PDUs and tasks for LUN hotplug
In _iscsi_conn_free_tasks(), we had parsed conn->write_pdu_list
and then parsed conn->queued_datain_tasks.  However when we parsed
conn->write_pdu_list, if there was any task in conn->queued_datain_tasks,
some PDUs were inserted conn->write_pdu_list.  Hence after parsing
conn->write_pdu_list, new PDUs were in conn->write_pdu_list as orphan.
Then orphaned PDUs were freed later but LUN was already freed and
critical failure occurred.

This patch swaps the order of conn->queued_datain_tasks and
conn->write_pdu_list, and add comment to explain the change.

Additionally, this patch adds unit test which fails if it runs
without this fix.

Fixes issue #1030.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Icb0ffbbbac70792a62939dc55a69df05d2ab9128
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475453
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>
2019-12-05 12:21:09 +00:00
Shuhei Matsumoto
9ccf32d64e lib/iscsi: Fix double free of primary task when write I/O is split
When a iSCSI write is large and split, if LUN is removed between
creating and submitting the last subtask, spdk_clear_all_transfer_task()
completes the primary task and then process_non_read_task_completion()
tries to complete the primary task.

This is the double free case, and the later have to be skipped.

We add a flag is_r2t_active to struct spdk_iscsi_task and use it to
check the duplication. We may be able to use primary's initiator task tag
(ITT) instead but we can not rely on ITT because it is set by the initiator.

We clear is_r2t_active even when primary is removed from
conn->queued_r2t_tasks but it will be no harm.

Fixes the issue #1064.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ia6511bd7adaa8fcb9a07bc40d498e8ee0b7a7ccf
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475044
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>
2019-12-05 12:21:09 +00:00
Shuhei Matsumoto
8238265548 lib/iscsi: Separate split and non-split in process_non_read_task_completion()
In process_non_read_task_completion(), when the current I/O is
not split, we have to call only spdk_iscsi_task_response().

The next patch will fix the github issue by changing the path
executed when the current I/O is split.

Hence to make the fix easier, this patch separates split case and
non-split case.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ic1603609f760c4bdd41272ba6146e260f668b059
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475043
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>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
2019-12-05 12:21:09 +00:00
Shuhei Matsumoto
acab54c1f4 iscsi: correctly free the deferred pdu for ERL > 0 case
The type of pdu deferred to be free only have
two types, R2T or DATA_IN. And the two types of pdus
are all assoicated a task, so updateing both the code and unit test case.

Also for all pdu free, we should use spdk_iscsi_conn_free function since
for normal pdu free, we all use this function.

PS: I also tested the calsoft local, it does not trigger the assert.

Fixes #1074.

Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I0524965baf5349a100210ef717aedaa5f8ff105e
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475657
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
2019-12-04 15:34:54 +00:00
Shuhei Matsumoto
64e0e380a3 Revert: "Replace conn_free_pdu() by task_put() and put_pdu() in remove_acked_pdu()"
This revert commit 6e4e85dcef91e6793b554a5d15c72ba93e56a4ab.

Causing intermittent build pool failures. We had different operations
in remove_acked_pdu() and _iscsi_conn_free_tasks(). The patch tried
to unify them but was wrong. The operation in removed_acked_pdu()
was correct. This patch restores it. The next patch will fix the
operation in _iscsi_conn_free_tasks().

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ia398fd295769b786ba4777cc9f7df6e134f15e48
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475791
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
2019-12-04 15:34:54 +00:00
Ziye Yang
fe5a969662 iscsi: put data_cnt_in operation in iscsi task get and free
Purpose: Simply the code, doing data_cnt_in every where
will make the code diffcult to maintain. If we put the
management in iscsi task get and free related function, then
the code will be easy to be read and easy to maintain.

Change-Id: Ib9af067326630657877a94afc2eb0db28f5d5fd1
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/474914
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>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-12-04 15:34:54 +00:00
Shuhei Matsumoto
1435ee36fa lib/iscsi: Make the data_in_cnt only used for subread tasks.
Purpose: Do not let the primary task do the I/O if it is
splitted into subtasks. This will make the code simplier,
when all the sub I/Os are finished, we can free the
primary task.

Update the corresponding unit test also.

As a result of this change, when read I/O is split into subtasks,
the primary task uses only some of its data. Hence separate
iscsi_pdu_payload_op_scsi_read() into split and non-split
case explicitly.

Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Change-Id: I9bbe4b8dd92a2996f35ad810b33676e34670c77e
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/473532
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
2019-12-04 15:34:54 +00:00
Jim Harris
d7b5ca749b env: reuse set_translation code for clear_translation
The code is exactly the same - we can just have
spdk_mem_map_clear_translation call spdk_mem_map_set_translation
with translation = map->default_translation.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I6a2ce39b0397be9d29b1a4c1cdfba15025afba7a

Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/476529
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
2019-12-04 15:29:57 +00:00
GangCao
d453c9400e RPC: properly set the address in error cases
Change-Id: Ie1757ef7b3e382492448f2fca0f5a165cdfac08c
Signed-off-by: GangCao <gang.cao@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475946
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Xiaodong Liu <xiaodong.liu@intel.com>
Reviewed-by: Liang Yan <liang.z.yan@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
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>
2019-12-04 15:11:55 +00:00
Karol Latecki
ebe62e1453 Revert "env_dpdk: Detect DPDK's iova mode when programming the IOMMU"
This reverts commit a68effe70977348e94a3c4a0f57293c873c6ff69.

Reason: introduces assertion:
vhost: rte_vhost_compat.c:103: vhost_session_mem_unregister: Assertion `false' failed.

Reported in GH issue 1085.

Change-Id: I00926844c1e00f19547f03487156f7b4238b446c
Signed-off-by: Karol Latecki <karol.latecki@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/476133
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
2019-11-29 09:31:39 +00:00
Tomasz Zawadzki
0d1aa0252d blob: fix sequentially allocated clusters starting from 0
When serializing extents, run-length encoding is supposed to
1) RLE all sequential LBAs
2) RLE zero LBAs (unallocated)

There is one special case, with sequential LBAs that start
with 0 LBA. This is RLE as 1) case, but results in descriptor
matching case 2). Which causes loss of allocated clusters.

This requires following conditions to be met:
- blobstore has just a single cluster reserved for MD
- blob is thin provisioned
- first allocation occurs on cluster_num=1

For last part to be true, very first write for blob has to be
issued to LBA between cluster_size and 2*cluster_size.
Causing allocation of second cluster in blobstore and assiging
it LBA equal to number of LBAs per cluster.

To fix this, case 1) disallows to RLE zeroes.

Signed-off-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Change-Id: I136282407966310c882ca97c960e9a71c442c469
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475494
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>
2019-11-28 12:38:03 +00:00
paul luse
dc29e75b1c lib/blob: minor refactor around clear_method
In prep for storing a clear_method in the blob metadata:

* Set the default to DEFAULT and let the switch statement choose
UNMAP
* Use switch statements to make it clearer which method we are
using and why. (ie previously we set the default to UNMAP and
then had an UNMAP || DEFAULT condition to choose UNMAP.

Later in the patch series it will become clearer why this makes
sense.

Signed-off-by: paul luse <paul.e.luse@intel.com>
Change-Id: I216cb97fd8eaa772437a36c2c7a47e66618bbfbd
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472202
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
2019-11-28 12:37:46 +00:00
paul luse
27fbf685bb lib/lvol: no longer store clear_method as an xattr
Part of a longer series to make clear_method a lot easier to use and
understand. We're going to store this parm now in the blob metadata
instead of lvol xattr. That way it will be usable without having
to remove the option from lvol_create or adding to lvol_delete
(which was what the last version of this patch did for those keeping
score at home)

Note: the current use of this xattr is broken anyway, its missing
plumbing to use it on a subsequent delete so this doesn't break
anything new.

Signed-off-by: paul luse <paul.e.luse@intel.com>
Change-Id: Ie77a25ecdc52fd4aef6c090756c6cafe79486bc8
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470410
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
2019-11-28 12:37:46 +00:00
Tomasz Kulasek
fd2af7afa9 lib/nvme: stop all NVMe io producers on detach
Now all registered producers should be stopped (unregistered) before
NVMe detach, otherwise NVMe controller cannot be safely detached.

This patch allows to stop all not unregistered io producers before
NVMe detach:

1. Callback to the "struct nvme_io_msg_producer" to stop producer
   started on selected controller.
2. On nvme_io_msg_ctrlr_detach() if there's some unregistered producers,
   stop all before freeing resources.


This approach also fixes issue with not to stop CUSE device when
NVMe controller is detached without unregistering producer (github
issue #1033).

	Fixes #1033

Change-Id: Ia1ffef566bb745edb55c54d6786ea481a35bbefd
Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/474273
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
2019-11-28 12:36:58 +00:00
Jacek Kalwas
155c3babce nvmf/tcp: rm qpair destroy from poll_group_add
Destroy in poll_group_add results in heap-use-after-free because
upper layer calls qpair_fini in case poll_group_add returns
error.

Signed-off-by: Jacek Kalwas <jacek.kalwas@intel.com>
Change-Id: I3e921a21b7ab5f7c15c80bc5919cb97cbda0b5d2
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475858
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>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
2019-11-28 12:36:36 +00:00
Ben Walker
a68effe709 env_dpdk: Detect DPDK's iova mode when programming the IOMMU
If DPDK is using virtual addresses, we should use virtual addresses.
If DPDK is using physical addresses, we should use physical addresses.
This way there can never be a conflict and everything is consistent.

Change-Id: Ie4b0e885e9a52dd6cbc81000a87908102a9771cb
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475928
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-11-28 12:36:20 +00:00
Ziye Yang
3ad192e3b5 iscsi: change timeout from int to uint64_t
Avoid the running converting timeout from sec to TSC, thus
make the behaviour same as last_nopin, i.e., initialize
when constructing the connection.

Change-Id: Ibc120fed24d2208cab9ae8a876856e9d05363075
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475711
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>
2019-11-27 07:09:31 +00:00
Ben Walker
97b0f7733f env: Check supported iommu address width before using iova-mode=va
DPDK by default guesses that it should be using iova-mode=va
so that it can support running as an unprivileged user. However,
some systems (especially virtual machines) don't have an IOMMU capable
of handling the full virtual address space and DPDK doesn't
currently catch that. Add a check in SPDK and force iova-mode=pa
here.

Change-Id: Ib3a5691a584190feaab4b9064b5a500e361328f2
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475149
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>
2019-11-27 07:08:32 +00:00
Ben Walker
f1d0c5aba0 sock: Simply spdk_sock_close
When this was originally implemented, it only accessed
*sock once or twice. As more stuff is added, it becomes
worthwhile to dereference the first level of **sock.

Change-Id: Ie31bb0210008f6341b071ba472aaedf897fa459a
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475310
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-11-22 13:54:58 +00:00
Tomasz Kulasek
88808c5ab7 lib/nvme: cuse device avoid using signals
This patch uses lowlevel fuse functions to process messages to
eliminate the need to use signals to interrupt blocking read
operation in fuse_session_loop().

  Fixes #1032

Change-Id: Ie9c9ea76cc135c383f5757864aa2d84ac9eb3da3
Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/473233
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2019-11-22 13:53:38 +00:00
Ben Walker
07ca02210a env: Force iova-mode=pa on ppc
In DPDK, the ppc iommu support does not currently allow for
iova-mode=va, but DPDK doesn't detect ppc and so still attempts
to guess iova-mode=va in some modes. Force iova-mode=pa from
SPDK to fix this.

Change-Id: I6a1ee25ab74873826ac211c3e0dfdf54afc74502
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475148
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: JinYu <jin.yu@intel.com>
2019-11-21 14:34:42 +00:00
Wojciech Malikowski
93402cd144 lib/ftl: Keep state of single band relocation
Added states for keeping track on which reloc
queue band actually is.

Change-Id: Ib05ac4e925002728ddfed3195891f5328eebb0d0
Signed-off-by: Wojciech Malikowski <wojciech.malikowski@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/465072
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>
2019-11-21 14:34:11 +00:00
Wojciech Malikowski
7012a2760e lib/ftl: Minor cleanup in reloc module
This patch removes some unused variables from
ftl_reloc_init().

Change-Id: I483b28ad2f6331088c44bb5b215b4db50546a06b
Signed-off-by: Wojciech Malikowski <wojciech.malikowski@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472044
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>
Reviewed-by: Mateusz Kozlowski <mateusz.kozlowski@intel.com>
Reviewed-by: Konrad Sztyber <konrad.sztyber@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-11-21 14:34:11 +00:00
Seth Howell
e9e3f61525 nvme/rdma: add connect retry in edge case
If the initiator dies without a disconnecting a qpair, the target can
possibly retain the state of the connection. In this case, it will
inform us that the connection is stale, and we need to try again.

Change-Id: I4d349c634aee59ce9ea4af795b07dd8649db56b3
Signed-off-by: Seth Howell <seth.howell@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/473063
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-11-21 08:03:33 +00:00
Tomasz Kulasek
82a54d29f6 lib/nvme: fix nvme_io_msg_ctrlr_unregister with no io_msg_producer
Change-Id: If2e0d00560311948570a2d7113dc5f730d937c0c
Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/474257
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: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
2019-11-21 08:01:57 +00:00
Tomasz Kulasek
9eb0ffa90c lib/nvme: change api for io message
API changes in this patch:

 1) nvme_io_msg_ctrlr_start                         => nvme_io_msg_ctrlr_register
 2) nvme_io_msg_ctrlr_stop with (shutdown == false) => nvme_io_msg_ctrlr_unregister
 3) nvme_io_msg_ctrlr_stop with (shutdown == true)  => nvme_io_msg_ctrlr_detach


Change-Id: I60153ebbfb0d0b22575128d106f9333c3887213d
Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/474096
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-11-21 08:01:57 +00:00
Tomasz Kulasek
53184430a5 lib/nvme: fix do not use external_io_msg_qpair after free
Change-Id: I20ef8303c2fae6abf43d15ebb025ea368c0dfd67
Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/473282
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Seth Howell <seth.howell@intel.com>
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>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
2019-11-21 08:01:57 +00:00
Shuhei Matsumoto
e79a29b264 lib/iscsi: Make spdk_del_transfer_task() return success/failure
After recent refinement of LUN hotplug, it is possible that
for large write I/O, primary task is freed doubly as a github issue
is reported.

However we could not notice the case because spdk_del_transfer_task()
had not return success/failure, and to make matters worse,
the second call of TAILQ_REMOVE() to the same header and instance
caused no error if the first call succeeded.

This patch changes spdk_del_transfer_task() to return success/failure.

Besides, the next after patch expects the stub of spdk_del_transfer_task()
returns true in the unit test, and hence do that.

The next after patch will fix the issue of double free of primary task
by using this patch.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ibc0b65723050362d5fafa913417b64393feb874e
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475042
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
2019-11-21 08:01:29 +00:00
Shuhei Matsumoto
fa2dd2722d lib/scsi: Move scsi_task_free_data() in a file to remove function declaration
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I9c98ded51bf88cd199bace7b3703b86ed4a9c25b
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472519
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>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
2019-11-21 08:01:29 +00:00
Shuhei Matsumoto
16dbc19508 lib/scsi: Consolidate error paths in bdev_scsi_readwrite()
This simplifies the code a little.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Idf53616bda21c82b82f9e9438f4dcb87ffaa59a6
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472518
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
2019-11-21 08:01:29 +00:00
Shuhei Matsumoto
ca0972d3b8 lib/scsi: Match parameters of bdev_scsi_readwrite() with bdev_scsi_sync/unmap()
We can pass bdev, lun->bdev_desc, and lun->io_channel to
bdev_scsi_readwrite() as same as bdev_scsi_sync/unmap().

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: Ib8b368da4da54b057ae09a91ab60a50388a5c684
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472517
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>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
2019-11-21 08:01:29 +00:00
Shuhei Matsumoto
84e64cc9d7 lib/scsi: Pass SCSI task to SCSI layer to get DIF context
By the recent refactoring, SCSI task is configured when getting
DIF context from SCSI layer. Passing not CDB and offset separately
but SCSI task to SCSI layer is more concise and do in this patch.

In iscsi_send_datain(), we have to update task->scsi.offset for the
case that data is split into a sequence, but the update is no harm
because task has completed what it must to do.

Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I153352dfa7aa7325db4452f03d863df11b3e0cfa
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/472510
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>
Reviewed-by: Paul Luse <paul.e.luse@intel.com>
2019-11-21 08:01:29 +00:00
Tomasz Zawadzki
4ea996ce19 nvmf/rdma: dont refer to rtransport when poll_group_create failed
This issue was found by code inspection.
It only occurs when pdk_nvmf_transport_poll_group_create()
itself calls spdk_nvmf_transport_poll_group_destroy().
Other transports at this time do not show this issue.

spdk_nvmf_rdma_poll_group_destroy() depends on
rgroup being assigned a transport, which is only being
done on generic nvmf layer using
spdk_nvmf_transport_poll_group_create()
after successful spdk_nvmf_rdma_poll_group_create().
When failure occurs during create, such assignment was not
performed so any references to rtransport will segfault.

Reported-by: Jacek Kalwas <jacek.kalwas@intel.com>
Signed-off-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Change-Id: Id54482d562bd6d7c71371306cf1de93bc05f4e8a
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475002
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: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Alexey Marchuk <alexeymar@mellanox.com>
2019-11-20 10:01:43 +00:00
Ben Walker
e475586b2a util: Add spdk_iovcpy, an iovec to iovec copy function.
Like memcpy, but works on two iovecs.

Change-Id: Ia1cf462a95690286f0c19325fc10937b9ba6baf3
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/473976
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-11-20 09:35:32 +00:00
Ben Walker
063252fa66 pipe: Add a utility for buffering data from sockets
When dealing with sockets most code in SPDK buffers
data into large chunks to minimize the number of
syscalls made. The pipe utility is designed to make
that easy.

Change-Id: Ie29966712bbfb43fb49457e042903cf45864e6c6
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/465707
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-11-20 09:35:32 +00:00
Jim Harris
aa4856632e bdev: remove _spdk_bdev function prefix
The _ was supposed to indicate an internal function,
but really leaving off the spdk in the function
is the standard way to denote an internal function.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I74facafb67b793502838b9c1b5f90aec2c88c69b

Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475033
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Alexey Marchuk <alexeymar@mellanox.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
2019-11-20 09:34:18 +00:00
Jim Harris
0e58d1549b bdev: remove spdk_ prefix from static functions
We try to reserve the spdk_ prefix for functions that
are part of the public SPDK API.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I0a95ba0f9db04b588a2e948427f8a9f53a7b5740

Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475032
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: Alexey Marchuk <alexeymar@mellanox.com>
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
2019-11-20 09:34:18 +00:00
Jim Harris
fdfb4e1270 bdev: rename spdk_bdev_get_io to bdev_channel_get_io
This function is not part of the SPDK public API, so
remove the spdk_ prefix from it to make it more clear
this is internal to the SPDK bdev library.

Also add "channel" to the name since this function is
operating on the channel parameter.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I57897ab4bf601b90551259b7cf6efa63152ed02f

Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475031
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: Alexey Marchuk <alexeymar@mellanox.com>
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
2019-11-20 09:34:18 +00:00
Jim Harris
a1d68e9c02 bdev: remove spdk_ prefix from spdk_bdev_io_init
This is not part of the public SPDK API, so remove
the spdk_ prefix to make it clear this is internal
to the bdev library.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Ibda76e4d8e34dde0c2fe638cb965e5ba2d9e47b5

Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475030
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: Alexey Marchuk <alexeymar@mellanox.com>
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
2019-11-20 09:34:18 +00:00
Jim Harris
6c07e9d1a8 bdev: remove spdk_ prefix from spdk_bdev_io_submit
This function is not part of the SPDK public API, so
remove the spdk_ prefix to make that more clear when
reading the code.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I5ea4c04c474a6cf0862f11921daa97c9df728e96

Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475029
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: Alexey Marchuk <alexeymar@mellanox.com>
2019-11-20 09:34:18 +00:00
Jim Harris
c3aaaa0181 nvme: don't monitor hotplug events in secondary process
NVMe hotplug must be monitored in the primary process -
DPDK doesn't support trying to handle it in the
secondary process.

This issue was somewhat masked previously in secondary
processes, since usually it would just probe(NULL) which
meant probe all attached NVMe controllers.  So in the
secondary process, we would probe just once, and create
the hotplug fd - it would never actually try to monitor
it.

But when explicitly specifying multiple trids in a
secondary process, probe would get called multiple
times.  First time would be fine since it only creates
the hotplug fd.  But second time would segfault since
monitoring for hotplug requires checking the DPDK-allocated
context which doesn't exist in the secondary process.

Fixes issue #1063.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I2a9a91e222c206034293d90e30e3f598c8d7baa8

Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475015
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: Alexey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
2019-11-20 09:34:18 +00:00
Jim Harris
27e88b8d91 nvme: add g_ prefix to hotplug_fd
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I8cc03e1a8b5d2eb28bf945115f3c9b3980b30f1c

Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475014
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: Alexey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
2019-11-20 09:34:18 +00:00
Vitaliy Mysak
506a90a90b vhost: wrap some of rte_vhost functions
This patch introduces indirection layer for session management
functions that makes it possible to switch their underlying implementation
if we want to (in unit tests for example).

Change-Id: I563c97bc65d55cc42fecbd1b7eb6679e394784a2
Signed-off-by: Vitaliy Mysak <vitaliy.mysak@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470459
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-11-20 09:33:43 +00:00
Vitaliy Mysak
0a089121f6 vhost: move memory registration functions to vhost_compat.c
This change will allow us to move other DPDK specific functions
to rte_vhost_compat.c, such as session callbacks which are
the only consumers of these memory management functions.

Change-Id: Ieb7b3f08ddf2e7cf04ecf18e8af4ad04124ccfea
Signed-off-by: Vitaliy Mysak <vitaliy.mysak@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470458
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-11-20 09:33:43 +00:00
Vitaliy Mysak
3f7e2ba18e vhost: move global definitions to common header
Move FLOOR_2MB() and CEIL_2MB() definitions to spdk_internal/memory.h
because that is where common memory functions are located.

Change-Id: I0d366686f86520e5564be07254d98a579faa3650
Signed-off-by: Vitaliy Mysak <vitaliy.mysak@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471713
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-11-20 09:33:43 +00:00
Darek Stojaczyk
6231a197f5 vhost: simplify memory registration functions
Make them accept a struct rte_vhost_memory * param
instead of a full spdk session object.

Change-Id: Ibe12f29146d2e971ef58fd5f7366cb059123224d
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470457
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>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: John Kariuki <John.K.Kariuki@intel.com>
2019-11-20 09:33:43 +00:00
Vitaliy Mysak
30998f9b1f vhost: return error codes for all session callbacks
Change type of `vhost_stop_device_cb()` and `vhost_destroy_connection_cb()`
to return response code instead of "void".

While DPDK callbacks `stop_device()` and `destroy_connection()`
do not have response code, it does make sense to have them in
our VHOST wrappers because those actions can fail.

Practical benefit we get by adopting this change is that we can
now use high level `vhost_stop_device_cb()` and `vhost_destroy_connection_cb()`
in unittests and check if they succeeded or not.

Change-Id: I2cd1886728b1edce7946e87db7ca0ac435e83a41
Signed-off-by: Vitaliy Mysak <vitaliy.mysak@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/471712
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>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-11-20 09:33:43 +00:00
Vitaliy Mysak
0e96d724d8 vhost: move session callbacks to rte_vhost_compat
This will allow us to write some more interesting unit tests
  because we can now mock high level callbacks instead of
  low level DPDK API and also, in future, we won't have to deal with sem_wait()
  in our mocked implementation since it's a DPDK specific thing.

Change-Id: I9ed5cff216e750685c00e718025ff1802fbe32c8
Signed-off-by: Vitaliy Mysak <vitaliy.mysak@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470456
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>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-11-20 09:33:43 +00:00
Darek Stojaczyk
1dae563373 vhost: move rte_vhost socket creation to rte_vhost_compat.c
rte_vhost_compat.c will now not only handle vhost-user
messages over the unix domain socket, but also setup
that unix domain socket with rte_vhost's APIs.

What was previously called vhost_dev_install_rte_compat_hooks()
is now called vhost_register_unix_socket() and is responsible
for creating the entire unix domain socket.

This enables us to write more advanced unit tests for vhost.
Instead of mocking low-level rte_vhost APIs, we could
now potentially mock vhost_register_unix_socket() and
create vhost devices and sessions without any actual
unix domain sockets involved.

Change-Id: Ifb18b92b37915c3f683b6d4fcdcc9259a3770561
Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Signed-off-by: Vitaliy Mysak <vitaliy.mysak@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/470455
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>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
2019-11-20 09:33:43 +00:00