Date: Mon, 3 Jan 2022 11:21:12 -0800 From: John Baldwin <jhb@FreeBSD.org> To: Alexander Motin <mav@FreeBSD.org> Cc: =?UTF-8?Q?Edward_Tomasz_Napiera=c5=82a?= <trasz@freebsd.org>, scsi@FreeBSD.org, Ken Merry <ken@freebsd.org> Subject: Re: iSCSI target: Handling in-flight requests during ctld shutdown Message-ID: <6c6a7236-3f54-efcf-7996-9e6c796bf5f0@FreeBSD.org> In-Reply-To: <5f5eac1f-7581-c19d-b2a3-ccf927471cca@FreeBSD.org> References: <fd383f6f-5a19-e2bb-5383-e559271eb3cd@FreeBSD.org> <b6c090ac-6cb0-6173-422d-9aef0b37b8ee@FreeBSD.org> <42e175d9-1693-29e2-0b5b-3fa513aa2a2d@FreeBSD.org> <7f1a1a65-199d-858a-792c-42871d1df13e@FreeBSD.org> <68ad0ffd-79fb-973d-e5ba-92deee352d44@FreeBSD.org> <2ce80c64-6954-21d0-74eb-eeb88e289350@FreeBSD.org> <10687910-7500-008c-aed5-61f76ae90b3d@FreeBSD.org> <1df3d9cf-6456-bcff-4fbe-c36136692efa@FreeBSD.org> <253e4d65-4c24-20be-480d-026c35d10ae5@FreeBSD.org> <5f5eac1f-7581-c19d-b2a3-ccf927471cca@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 12/31/21 3:07 PM, Alexander Motin wrote:
> On 31.12.2021 17:17, John Baldwin wrote:
>> So my remaining question I guess is if I add a new
>> 'cs->cs_terminating_tasks'
>> or the like, how does cfiscsi_datamove_out ensure that no response is sent?
>
> By the time cs_terminating_tasks is set, there is no connection any
> more, so nothing will be sent any way. The problem with cs_terminating
> is that it is set too early, when connection may still be alive.
Ok.
> On top of that, probably not important, but CTL_TASK_I_T_NEXUS_RESET
> call by cfiscsi_session_terminate_tasks() will make the code below to
> also block the status reporting for the most commands.
Well, this was more about if there are potential commands (perhaps in the
future) that get aborted in cfiscsi_datamove_out that weren't cancelled
by CTL_TASK_I_T_NEXUS_RESET due to not being on the OOA queue. However,
I think we agree that currently those shouldn't exist. Perhaps I can just
assert that CTL_FLAG_ABORT is set.
>> The only thing I've seen so far is this code in cfiscsi_scsi_command_done:
>>
>> /*
>> * Do not return status for aborted commands.
>> * There are exceptions, but none supported by CTL yet.
>> */
>> if (((io->io_hdr.flags & CTL_FLAG_ABORT) &&
>> (io->io_hdr.flags & CTL_FLAG_ABORT_STATUS) == 0) ||
>> (io->io_hdr.flags & CTL_FLAG_STATUS_SENT)) {
>> ctl_free_io(io);
>> icl_pdu_free(request);
>> return;
>> }
>>
>> Would you prefer checking cs_terminating_tasks in this function as well to
>> avoid sending the peudo-aborted responses instead of forcing CTL_FLAG_ABORT
>> on?
>
> CTL_FLAG_ABORT flags there are correct. The tasks are really aborted.
> But if I remember it right, CTL_TASK_I_T_NEXUS_RESET should not set
> CTL_FLAG_ABORT_STATUS, so abort statuses should not be sent. Abort
> statuses are only sent when initiator does not expect aborts, for
> example, when some other initiator requested LUN or target reset.
I was more asking about the potential future use case of there being a command
we data abort that doesn't already have CTL_FLAG_ABORT set, but just asserting
it is already set it perhaps the simplest approach instead.
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6c6a7236-3f54-efcf-7996-9e6c796bf5f0>