Re: [PATCH] IB/iser: skip an unregistered direction on remote invalidation

[Yehyeong Lee]

The AI review on this patch asks two things: whether a bogus remote
invalidation is now silently accepted, and whether that desynchronises
the MR state.  The first is true by design; the second does not happen.

need_inval lives in the ib_mr inside struct iser_fr_desc
(iscsi_iser.h:320 and :333).  With no descriptor there is no flag to
clear, and the cleanup path already returns early on the same condition
this patch adds - iser_unreg_mem_fastreg() at iser_memory.c:156.  A
need_inval left true is cleared by the next registration, which posts
LOCAL_INV ahead of the registration work request (iser_memory.c:316).
The state that would be wrong is need_inval false with a live MR, and
skipping never clears it.

With the default always_register, desc is NULL only for a write sent
entirely as immediate data (iser_memory.c:364, iser_initiator.c:105),
and that path advertises no stag (iser_initiator.c:116), so a conforming
target has nothing to invalidate for such a task.  Returning an error
would fail the whole connection (iser_initiator.c:657).

Two corrections to what I wrote earlier.  The claim that returning an
error would break a bidirectional command, which I made under the ---
and again in a follow-up note, is wrong: ISCSI_FLAG_CMD_READ and
ISCSI_FLAG_CMD_WRITE are set in one if/else in libiscsi.c:368-417, so
they cannot both be set.  And the eight-versus-132 and ten-versus-three
counts in that note were each from a single run; two other runs of the
error-returning build saw three invalidations and no rejections.

Best regards,

Yehyeong Lee