From: jsmart2021@gmail.com (James Smart)
Subject: [PATCH v2 4/5] nvmet_fc: Rework target side abort handling
Date: Wed, 19 Apr 2017 16:19:41 -0700 [thread overview]
Message-ID: <00a1f313-d5d2-d750-9993-e815fa780ffe@gmail.com> (raw)
In-Reply-To: <20170419193625.GE18191@infradead.org>
On 4/19/2017 12:36 PM, Christoph Hellwig wrote:
> This looks ok as a change to the existing code:
>
> Reviewed-by: Christoph Hellwig <hch at lst.de>
>
> But we really need to go to the NVMe technical working group about
> how to cater for the fact that the FC transport does transport aborts
> (and thus probably doesn't use NVMe aborts at all, although we'd need
> to clarify that). Can you reach out to the working group and the T11
> folks?
>
Thanks.
I think there's a mis-understanding. T11 doesn't use transport aborts
in lieu of an NVMe Abort. In fact, it's written that if it has an I/O
failure and can't recover from it by retransmission/preserving the
exchange (and currently it can't, as the T11 1.0 spec deferred
retransmission support until 1.1), then it falls back to terminating the
connection, which also terminates the association - which is per the
language in the NVMe Fabric spec sec 7.1. So, if it sees an ABTS for an
exchange, it kills the association. Note: there would be several issues
if T11 tried to use ABTS's in lieu of Abort, or ABTS and cmd retry in
lieu of real retransmission. So neither are allowed.
What you're probably seeing is the error being detected on the io, and
the ABTS being pre-emptively sent for that io, and then that escalating
to the connection/association failure, which usually spits out lots more
ABTS's. On the target side implementation in linux, the one io gets
aborted, and it currently doesn't escalate to other commands - it
expects the initiator to get the ABTS, thus an io error, thus the
initiator to teardown the connection/association and send all the
ABTS's. This may need to be revisited after the T11 1.0 spec comes
out, which I believe requires the target to also ABTS things on
connection failure. I do need to check that if the linux nvmet layer
kills the association/connection its returning all the outstanding cmds
to the transport so I can meet that requirement.
There are perhaps 2 things that could be improved on the linux initiator
fc transport:
1) Use NVMe Aborts instead of defaulting to resetting the controller
(like rdma). This was held off as: ) NVME Abort are "best effort" and
there were a lot of comments in the tech group promoting lazy abort
support by always returning Dword 0 bit 0 =1 (cmd not aborted); b) Abort
vs SQ cmd delivery is even more asynchronous than on other transports,
creating more hit/miss conditions and requiring Abort command retries
(what is a reasonable policy?); and c) it really should be something in
the core layer and managing vs the Abort Command Limit is a real pain.
This can always change in the future.
2) There are a couple of io error cases that detect a transport error
and set status to NVME_SC_FC_TRANSPORT and don't ABTS the cmds. They
should per T11 spec.
-- james
next prev parent reply other threads:[~2017-04-19 23:19 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-11 18:32 [PATCH v2 0/5] nvmet_fc: bug fixes and lldd api mods jsmart2021
2017-04-11 18:32 ` [PATCH v2 1/5] nvmet_fc: add target feature flags for upcall isr contexts jsmart2021
2017-04-19 19:33 ` Christoph Hellwig
2017-04-11 18:32 ` [PATCH v2 2/5] nvmet_fc: add req_release to lldd api jsmart2021
2017-04-19 19:34 ` Christoph Hellwig
2017-04-11 18:32 ` [PATCH v2 3/5] nvme_fcloop: split job struct from transport for req_release jsmart2021
2017-04-19 19:34 ` Christoph Hellwig
2017-04-11 18:32 ` [PATCH v2 4/5] nvmet_fc: Rework target side abort handling jsmart2021
2017-04-19 19:36 ` Christoph Hellwig
2017-04-19 23:19 ` James Smart [this message]
2017-04-11 18:32 ` [PATCH v2 5/5] nvmet_fc: add missing reference in add_port jsmart2021
2017-04-19 19:36 ` Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=00a1f313-d5d2-d750-9993-e815fa780ffe@gmail.com \
--to=jsmart2021@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.