>>> Mike Christie <michael.chris...@oracle.com> schrieb am 08.10.2020 um 22:54 >>> in Nachricht <47eca384-b54e-63cc-0f84-7ed6501f4...@oracle.com>: > On 10/8/20 12:11 PM, Mike Christie wrote: >> On 9/25/20 1:41 PM, ldun...@suse.com wrote: >>> From: Lee Duncan <ldun...@suse.com> >>> >>> iSCSI NOPs are sometimes "lost", mistakenly sent to the >>> user-land iscsid daemon instead of handled in the kernel, >>> as they should be, resulting in a message from the daemon like: >>> >>>> iscsid: Got nop in, but kernel supports nop handling. >>> >>> This can occur because of the forward- and back-locks >>> in the kernel iSCSI code, and the fact that an iSCSI NOP >>> response can be processed before processing of the NOP send >>> is complete. This can result in "conn->ping_task" being NULL >>> in iscsi_nop_out_rsp(), when the pointer is actually in >>> the process of being set. >>> >>> To work around this, we add a new state to the "ping_task" >>> pointer. In addition to NULL (not assigned) and a pointer >>> (assigned), we add the state "being set", which is signaled >>> with an INVALID pointer (using "-1"). >>> >>> Signed-off-by: Lee Duncan <ldun...@suse.com> >>> --- >>> drivers/scsi/libiscsi.c | 13 ++++++++++--- >>> include/scsi/libiscsi.h | 3 +++ >>> 2 files changed, 13 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c >>> index 1e9c3171fa9f..cade108c33b6 100644 >>> --- a/drivers/scsi/libiscsi.c >>> +++ b/drivers/scsi/libiscsi.c >>> @@ -738,6 +738,9 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct > iscsi_hdr *hdr, >>> task->conn->session->age); >>> } >>> >>> + if (unlikely(READ_ONCE(conn->ping_task) == INVALID_SCSI_TASK)) >>> + WRITE_ONCE(conn->ping_task, task); >>> + >>> if (!ihost->workq) { >>> if (iscsi_prep_mgmt_task(conn, task)) >>> goto free_task; >> >> I think the API gets a little weird now where in some cases >> __iscsi_conn_send_pdu checks the opcode to see what type of request >> it is but above we the caller sets the ping_task. >> >> For login, tmfs and passthrough, we assume the __iscsi_conn_send_pdu >> has sent or cleaned up everything. I think it might be nicer to just >> have __iscsi_conn_send_pdu set the ping_task field before doing the >> xmit/queue call. It would then work similar to the conn->login_task >> case where that function knows about that special task too. >> >> So in __iscsi_conn_send_pdu add a "if (opcode == ISCSI_OP_NOOP_OUT)", >> and check if it's a nop we need to track. If so set conn->ping_task. >> > Ignore this. It won't work nicely either. To figure out if the nop is > our internal transport test ping vs a userspace ping that also needs > a reply, we would need to do something like you did above so there is > no point.
I don't know the implementation details, but if ICMP package data would contain some "unlikely" value for iSCSI pings, you could use the packet data to decide. I guess 32 random bits could be "unlikely enough". > > -- > You received this message because you are subscribed to the Google Groups > "open-iscsi" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to open-iscsi+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/open-iscsi/47eca384-b54e-63cc-0f84-7ed6501f > 427e%40oracle.com. -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/5F801050020000A10003BD6F%40gwsmtp.uni-regensburg.de.