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

[Max Gurtovoy]

IMO any non-conforming target that sends an unnecessary remote invalidation deserves a dropped connection.

Something like:

static int iser_check_remote_inv(struct iser_conn *iser_conn, struct ib_wc *wc,
                                 struct iscsi_hdr *hdr)
  {
        if (wc->wc_flags & IB_WC_WITH_INVALIDATE) {
                struct iscsi_task *task;
                u32 rkey = wc->ex.invalidate_rkey;
  
                iser_dbg("conn %p: remote invalidation for rkey %#x\n",
                         iser_conn, rkey);
  
                if (unlikely(!iser_conn->snd_w_inv))
                        goto bad_inv;
                                 
                task = iscsi_itt_to_ctask(iser_conn->iscsi_conn, hdr->itt);
                if (likely(task)) {
                        struct iscsi_iser_task *iser_task = task->dd_data;
                        struct iser_fr_desc *desc;                                                       

                        if (iser_task->dir[ISER_DIR_IN]) {
                                desc = iser_task->rdma_reg[ISER_DIR_IN].desc;
                                if (unlikely(!desc))
                                        goto bad_inv;
                                if (unlikely(iser_inv_desc(desc, rkey)))
                                        return -EINVAL;
                        }
                
                        if (iser_task->dir[ISER_DIR_OUT]) { 
                                desc = iser_task->rdma_reg[ISER_DIR_OUT].desc;
                                if (unlikely(!desc))
                                        goto bad_inv;
                                if (unlikely(iser_inv_desc(desc, rkey)))
                                        return -EINVAL;
                        }               
                } else {        
                        iser_err("failed to get task for itt=%d\n", hdr->itt);
                        return -EINVAL;
                }       
        }               
                                
        return 0;               
                                        
  bad_inv:                      
        iser_err("conn %p: unexpected remote invalidation, terminating connection\n",
                 iser_conn);    
        return -EPROTO;
  }

-----Original Message-----
From: Yehyeong Lee <yhlee@xxxxxxxxxxxxxxxxxx> 
Sent: Tuesday, 18 August 2026 9:24
To: sagi@xxxxxxxxxxx
Cc: Yehyeong Lee <yhlee@xxxxxxxxxxxxxxxxxx>; Max Gurtovoy <mgurtovoy@xxxxxxxxxx>; jgg@xxxxxxxx; leon@xxxxxxxxxx; linux-rdma@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; dledford@xxxxxxxxxx
Subject: Re: [PATCH] IB/iser: skip an unregistered direction on remote invalidation

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