Purpose: To make the pdu management consistent with other PDUs, then
we can easily adapt our code into some hardware offloading solution.
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Change-Id: Ic4a2847fd1b6cacda4cbaa52ff12c338f0394805
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3588
Community-CI: Mellanox Build Bot
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: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
RDMA target can't handle bidirectional xfer type, in debug build
it throws an assert in nvmf_rdma_setup_wr function. NVMF controller
performs checks od opcodes, but the failure happens before this
check. Add similar validation in TCP transport.
Change-Id: I14400b9c301295c0ae1d35a4330189d38aeee723
Signed-off-by: Alexey Marchuk <alexeymar@mellanox.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3436
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>
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
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>
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: 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>
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>
Poller should return status > 0 when it did some work
(CPU was used for some time) marking its call as busy
CPU time.
Active pollers should return BUSY status only if they
did any meangful work besides checking some conditions
(e.g. processing requests, do some complicated operations).
Signed-off-by: Maciej Szwed <maciej.szwed@intel.com>
Change-Id: Id4636a0997489b129cecfe785592cc97b50992ba
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2164
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
It should return "NVME_TCP_PDU_FATAL". I think that
this issue is introduced after we move the data
copy from tcp transport layer to the socket
layer. And it should return "NVME_TCP_PDU_FATAL now",
and it will be consistent with the logic in the same
function.
With this patch, it will fix the big I/O size write
from the initiator.
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Change-Id: Ide018adb603eb13d002fc98886258dd1e2424f7c
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3122
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>
Community-CI: Mellanox Build Bot
Community-CI: Broadcom CI
Having that transport can decide about particular ctrlr attributes not
globally but per ctrlr.
Signed-off-by: Jacek Kalwas <jacek.kalwas@intel.com>
Change-Id: Ia3fb0d4e576cb9f8ce6df75f775e2fd5727d7f48
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2757
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI
Community-CI: Mellanox Build Bot
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This parameter describes the number of admin and IO
qpairs while admin qpair always exists and should not
be configured explicitly.
Introduce a new parameter `max_io_qpairs_per_ctrlr`
which configures the number of IO qpairs.
Internal structure of NVMF transport is not changed,
both RPC parameters configure the same nvmf transport parameter.
Deprecate max_qpairs_per_ctrlr in spdkcli as well
Side change: update dif_insert_or_strip description -
it can be used by TCP and RDMA transports
Config files parsing is not changed since it is deprecated
Fixes#1378
Change-Id: I8403ee6fcf090bb5e86a32e4868fea5924daed23
Signed-off-by: Alexey Marchuk <alexeymar@mellanox.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2279
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Anil Veerabhadrappa <anil.veerabhadrappa@broadcom.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Community-CI: Broadcom CI
I missed a few files in this library the first time.
Signed-off-by: Seth Howell <seth.howell@intel.com>
Change-Id: I2ad55355e6348eaa10384a148dd45deb9f68fc2b
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2442
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Anil Veerabhadrappa <anil.veerabhadrappa@broadcom.com>
Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
In SPDK NVMe/TCP target, when initializing the socket, the low
watermark is set to sizeof(struct spdk_nvme_tcp_common_pdu_hdr),
which is 24 bytes. In our testing, some times there might be very
small data packet (as small as 16 bytes) be sent to wire. After
this, if there is no more data sent to the same socket, this small
data packet won’t be received by NVMe/TCP controller qpair thread
because the size hasn’t reached the low watermark. Because of this,
the qpair thread is waiting for more data come in and the initiator
is waiting for the IO request to be completed. Hence the delay
happens.
As the minimum data that allows target to determine the PDU type is
sizeof(struct spdk_nvme_tcp_common_pdu_hdr), which is 8 bytes, we
changed low watermark setting as below. With the change, the problem
was gone immediately.
Change-Id: I14ccc4c84b77e33a617726e7455304aca29d5d57
Signed-off-by: Wenhua Liu <liuw@vmware.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/2138
Community-CI: Mellanox Build Bot
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Virtual controller capabilities can be overridden on transport
specific layer. The current behavior shall be preserved.
This can be useful to limit or extend the default based on transport
type.
Signed-off-by: Jacek Kalwas <jacek.kalwas@intel.com>
Change-Id: I754f0d957a46f219adc1e55f792e79c7546ddb43
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1274
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>
Purpose: To set the priority of the NVMe-oF connection especially
for TCP connection.
For example, the previous example can be:
trtype:TCP adrfam:IPv4 traddr:10.67.110.181 trsvcid:4420
With the change, it could be:
trtype:TCP adrfam:IPv4 traddr:10.67.110.181 trsvcid:4420 priority:2
The priority is optional. We try to change
spdk_nvme_transport_id but not in spdk_nvme_ctrlr_opts since
the opts in spdk_nvme_ctrlr_opts will reflect in every nvme ctrlr,
this is short of flexibility.
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Signed-off-by: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>
Change-Id: I1ba364c714a95f2dbeab2b3fcc832b0222b48a15
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1875
Community-CI: Mellanox Build Bot
Community-CI: Broadcom CI
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>
Since the related feature is already contained in
spdk_sock_listen and spdk_sock_connect functions,
we no longer need this function.
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Change-Id: I1eafff0d139fa266a355fbee2bf0fc3947db69fc
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1876
Community-CI: Mellanox Build Bot
Community-CI: Broadcom CI
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>
We will be create fine name for each poller but it will need large
effort. Replacing spdk_poller_register by the macro SPDK_POLLER_REGISTER
will provide better name than function address with minimum effort.
Following patches may improve function name for clarification.
Signed-off-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: If862a274c5879065c3f7cb04dcb5ca7844523e68
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1781
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: Paul Luse <paul.e.luse@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Maciej Szwed <maciej.szwed@intel.com>
Community-CI: Broadcom CI
The code used to do this but it was removed when the buffering was
shifted down to the posix layer. Add a way for users of sockets
to still properly size the buffers.
This also means that by default, the receive buffering is not enabled
on sockets. That matches the behavior of the previous release.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Change-Id: I20ce875be2efd841fe3a900047b4655a317d7799
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1560
Community-CI: Broadcom CI
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>
It's actually faster to process them until you run out of data.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Change-Id: I9e81babdb9bdc405a8dbf03b2f701fe50bcc70f6
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1559
Community-CI: Broadcom CI
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>
This reverts commit ea5ad0b286.
This code is moving from the nvmf target to the posix sock
layer in this series.
Change-Id: I333bdf325848e726ab82a9e6916e1bbdcd34009c
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/446
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>
This reverts commit d50736776c.
This code is moving from the nvmf target to the posix sock
layer in this series.
Change-Id: I7cf477333f2a3fa4a0089394d5fa28142b262a7f
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/445
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>
This reverts commit 5e7b8d18f3.
This code is moving from the nvmf target down into the posix
sock layer in this series.
Change-Id: Iea9a7cef5bedd6a34edf7b4c87825897279829c3
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/444
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>
This was recently made asynchronous to support virtualized transports.
However, we're moving to add a new call to associated a listener with a
subsystem to transports and the operation that needed to be asynchronous
will actually be performed there. For simplicity, make this synchronous
again.
Change-Id: Ie98136a19c58f0f9bba0d140476de3bbb38e12d7
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/881
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
It is memory optimisation as transport id is 'heavy'. As a side effect
simpler handling of listen and stop_listen on transport specific layer.
Signed-off-by: Jacek Kalwas <jacek.kalwas@intel.com>
Change-Id: I4e9d0e0c5eee2d570ec4ac9079270c32d5afb8db
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/626
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 defines the official interface that NVMe-oF target
transports may use. For now, all code is just copied
from elsewhere. Eventually we'll want to add doxygen
comments.
Change-Id: I0cd9368607544be18c7c49188d071e38ceb59b8f
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/412
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jacek Kalwas <jacek.kalwas@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Some functions performed incorrect header/data digest
support check, align it with NVMEoF spec. Use a table
to check if PDU supports digest depending on its type.
Change-Id: I6170dd19ace017f37fda0a923f604732799460b9
Signed-off-by: Alexey Marchuk <alexeymar@mellanox.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/483375
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Community-CI: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
When a command arrives and no requests are available, the socket
recv state machine sits in the RECV_STATE_AWAIT_REQ state until another
network event occurs. If this I/O was the last one sent, this leaves the
target hung. To fix this, when a request is completed, kick the state
machine to make forward progress.
In practice, this can only occur once the pdu send acknowledgements are
asynchronous relative to arriving commands. That only begins happening
with the use of MSG_ZEROCOPY. When MSG_ZEROCOPY is turned on, it's
possible receive the next PDU in a chain for a command prior to seeing
the acknowledgement that the response that triggered that PDU actually
sent.
Change-Id: I556f31ad56970d36aa3538cfde375d35f3d4e551
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/480002
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Previously, the R2T was sent and if an H2C arrived prior
to seeing the R2T ack, it was processed anyway. Serialize
this process.
In practice, if the H2C arrives with a correctly functioning
initiator, that means the R2T already made it to the initiator.
But because the PDU hasn't been released yet, immediately processing the
PDU requires an extra PDU associated with the request. Basically, making
this change halves the worst-case number of PDUs required per
connection.
In the current sock layer implementations, it's not actually possible
for the R2T send ack to occur after that H2C arrives. But with the
upcoming addition of MSG_ZEROCOPY and other sock implementations, it's
best to fix this now.
Change-Id: Ifefaf48fcf2ff1dcc75e1686bbb9229b7ae3c219
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/479906
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>
This function was only called from one spot.
Change-Id: I856f564d3ef6c6157be7a32a2cd812c702516a8d
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/482003
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Alexey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This seems like a more descriptive name
Change-Id: Ia616865b3fb36d8f9ccc5fb2ca6185bdd8543cf8
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/482002
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>
Reviewed-by: Alexey Marchuk <alexeymar@mellanox.com>
With our target design, there's no advantage to sending
multiple R2T PDUs per nvme command. This patch starts by
setting up the math so that at most 1 R2T PDU is required
per request. This can be guaranteed because the maximum
data transfer size (MDTS) is pre-negotiated in NVMe-oF
to a reasonable size at start up.
It then proceeds to simplify all of the logic around mapping
requests to PDUs. It turns out that the mapping is now always
1:1. There are two additional cases where there is no request
object at all but a PDU is still needed - the connection response
and termination request. Put an extra PDU on the queue object
for that purpose.
This is a major simplification.
Change-Id: I8d41f9bf95e70c354ece8fb786793624bec757ea
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/479905
Community-CI: SPDK CI Jenkins <sys_sgci@intel.com>
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>
Reviewed-by: Alexey Marchuk <alexeymar@mellanox.com>
We can always accept up to the maximum I/O size in an H2C,
so eliminate the #define.
Change-Id: I349dab5f9b6ec482a7c580b1396e03c8d30a250b
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/482278
Community-CI: 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>
Reviewed-by: Alexey Marchuk <alexeymar@mellanox.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
The resources allocated to a queue pair do not need to be directly
correlated to the queue size requested by the initiator in NVMe-oF, as
long as enough resources are present. The RDMA transport, for instance,
does complex pooling of the resources behind the scenes when using a
shared receive queue.
Simplify the resource allocation for a TCP qpair to just always allocate
the max allowed queue size right away. This is a configurable parameter,
so system administrators can adjust for their needs. The initiator may
then request a queue size less than or equal to that, which will only be
enforced by queue depth counting and not impact the actual number of
resources allocated on the target.
This change relies on the MaxC2HSize being equal to the Maximum Data
Transfer Size (MDTS) reported. That is the default configuration, but
MDTS is configurable. Changing the MDTS with this patch to a value
larger than 128k will cause the target to break. This is addressed in
the next patch in this series.
Change-Id: Ibd4723785c6a4d8d444f9b7bbfa89f98de2320f5
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/479733
Community-CI: SPDK CI Jenkins <sys_sgci@intel.com>
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>
Reviewed-by: Alexey Marchuk <alexeymar@mellanox.com>
These values do not need to be negative.
Change-Id: Id9f798cf1c9da354448f9c6fbb90e599f877bb32
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/482277
Community-CI: 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>
Reviewed-by: Alexey Marchuk <alexeymar@mellanox.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
By releasing the just-completed PDU prior to calling the callback,
for flows that immediately submit another PDU inside the callback,
the just-released PDU can be immediately reused. This reduces the number
of PDUs required in the pool to continue forward progress to half of the
previous value, while also making it more CPU cache friendly.
Change-Id: I8031b8f9f57ac05f261d96433d9899fe5e31d318
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/479904
Community-CI: SPDK CI Jenkins <sys_sgci@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Alexey Marchuk <alexeymar@mellanox.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Or Gerlitz <gerlitz.or@gmail.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
This was calling a callback for another function which
attempted to release the request. The code only worked because
in the r2t case the cb_arg was set to NULL, and that makes
the request free function do nothing.
Change-Id: Id9ec30ceb0eaa41deb67aa995da5d6f786d9b9f0
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/479903
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
Community-CI: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Or Gerlitz <gerlitz.or@gmail.com>
Reviewed-by: Ziye Yang <ziye.yang@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This wasn't actually used. Every PDU only had a single reference.
Change-Id: I8adaa7edeca5fe175aa853c156df741170d76c10
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/479902
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
Community-CI: 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: Jim Harris <james.r.harris@intel.com>
This would allow to respond for add listener rpc request even
when there are async calls in transport specific function.
Signed-off-by: Jacek Kalwas <jacek.kalwas@intel.com>
Change-Id: I94a9f45b7ba9e8d46a60ae3785953cea12554732
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/479511
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
Community-CI: 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: Anil Veerabhadrappa <anil.veerabhadrappa@broadcom.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Make common code as part of successful return.
In rdma check if already listening first.
Signed-off-by: Jacek Kalwas <jacek.kalwas@intel.com>
Change-Id: Ib0c87ac11db7daff00dc4042c9e0ab20eb7ffd0f
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/478721
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
Community-CI: 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>
Purpose: With this patch,
(1)We can support using different sock implementations in
one application together.
(2)For one IP address managed by kernel, we can use different method
to listen/connect, e.g., posix, or uring. With this patch, we can
designate the specified sock implementation if impl_name is not NULL
and valid. Otherwise, spdk_sock_listen/connect will try to use the sock
implementations in the list by order if impl_name is NULL.
Without this patch, the app will always use the same type of sock implementation
if the order is fixed. For example, if we have posix and uring together,
the first one will always be uring.
Signed-off-by: Ziye Yang <ziye.yang@intel.com>
Change-Id: Ic49563f5025085471d356798e522ff7ab748f586
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/478140
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
Community-CI: SPDK CI Jenkins <sys_sgci@intel.com>
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: Jim Harris <james.r.harris@intel.com>
The trtype should be stored as both an enum and string. This is intended to
help pave the way for pluggable NVMe-oF transports.
Signed-off-by: Seth Howell <seth.howell@intel.com>
Change-Id: I6af658d7a17c405e191ff401b80ab704c65497e7
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/478744
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>
Reviewed-by: Alexey Marchuk <alexeymar@mellanox.com>
It is faster for the kernel to pin memory in hugepages, so allocate
the pdu pool from hugepages. This will help more
with upcoming changes to leverage MSG_ZEROCOPY.
Change-Id: I9ce581acca9c6edb71bd8119258966e3b405db77
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475801
Community-CI: Broadcom SPDK FC-NVMe CI <spdk-ci.pdl@broadcom.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Or Gerlitz <gerlitz.or@gmail.com>
