| From 0af429af3ef776318513d3f6eff134e0096d7e37 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Tue, 19 Dec 2023 09:23:11 +0200 |
| Subject: IB/iser: Prevent invalidating wrong MR |
| |
| From: Sergey Gorenko <sergeygo@nvidia.com> |
| |
| [ Upstream commit 2f1888281e67205bd80d3e8f54dbd519a9653f26 ] |
| |
| The iser_reg_resources structure has two pointers to MR but only one |
| mr_valid field. The implementation assumes that we use only *sig_mr when |
| pi_enable is true. Otherwise, we use only *mr. However, it is only |
| sometimes correct. Read commands without protection information occur even |
| when pi_enble is true. For example, the following SCSI commands have a |
| Data-In buffer but never have protection information: READ CAPACITY (16), |
| INQUIRY, MODE SENSE(6), MAINTENANCE IN. So, we use |
| *sig_mr for some SCSI commands and *mr for the other SCSI commands. |
| |
| In most cases, it works fine because the remote invalidation is applied. |
| However, there are two cases when the remote invalidation is not |
| applicable. |
| 1. Small write commands when all data is sent as an immediate. |
| 2. The target does not support the remote invalidation feature. |
| |
| The lazy invalidation is used if the remote invalidation is impossible. |
| Since, at the lazy invalidation, we always invalidate the MR we want to |
| use, the wrong MR may be invalidated. |
| |
| To fix the issue, we need a field per MR that indicates the MR needs |
| invalidation. Since the ib_mr structure already has such a field, let's |
| use ib_mr.need_inval instead of iser_reg_resources.mr_valid. |
| |
| Fixes: b76a439982f8 ("IB/iser: Use IB_WR_REG_MR_INTEGRITY for PI handover") |
| Link: https://lore.kernel.org/r/20231219072311.40989-1-sergeygo@nvidia.com |
| Acked-by: Max Gurtovoy <mgurtovoy@nvidia.com> |
| Signed-off-by: Sergey Gorenko <sergeygo@nvidia.com> |
| Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| drivers/infiniband/ulp/iser/iscsi_iser.h | 2 -- |
| drivers/infiniband/ulp/iser/iser_initiator.c | 5 ++++- |
| drivers/infiniband/ulp/iser/iser_memory.c | 8 ++++---- |
| drivers/infiniband/ulp/iser/iser_verbs.c | 1 - |
| 4 files changed, 8 insertions(+), 8 deletions(-) |
| |
| diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h b/drivers/infiniband/ulp/iser/iscsi_iser.h |
| index dee8c97ff056..d967d5532459 100644 |
| --- a/drivers/infiniband/ulp/iser/iscsi_iser.h |
| +++ b/drivers/infiniband/ulp/iser/iscsi_iser.h |
| @@ -317,12 +317,10 @@ struct iser_device { |
| * |
| * @mr: memory region |
| * @sig_mr: signature memory region |
| - * @mr_valid: is mr valid indicator |
| */ |
| struct iser_reg_resources { |
| struct ib_mr *mr; |
| struct ib_mr *sig_mr; |
| - u8 mr_valid:1; |
| }; |
| |
| /** |
| diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c b/drivers/infiniband/ulp/iser/iser_initiator.c |
| index 39ea73f69016..f5f090dc4f1e 100644 |
| --- a/drivers/infiniband/ulp/iser/iser_initiator.c |
| +++ b/drivers/infiniband/ulp/iser/iser_initiator.c |
| @@ -581,7 +581,10 @@ static inline int iser_inv_desc(struct iser_fr_desc *desc, u32 rkey) |
| return -EINVAL; |
| } |
| |
| - desc->rsc.mr_valid = 0; |
| + if (desc->sig_protected) |
| + desc->rsc.sig_mr->need_inval = false; |
| + else |
| + desc->rsc.mr->need_inval = false; |
| |
| return 0; |
| } |
| diff --git a/drivers/infiniband/ulp/iser/iser_memory.c b/drivers/infiniband/ulp/iser/iser_memory.c |
| index 29ae2c6a250a..6efcb79c8efe 100644 |
| --- a/drivers/infiniband/ulp/iser/iser_memory.c |
| +++ b/drivers/infiniband/ulp/iser/iser_memory.c |
| @@ -264,7 +264,7 @@ static int iser_reg_sig_mr(struct iscsi_iser_task *iser_task, |
| |
| iser_set_prot_checks(iser_task->sc, &sig_attrs->check_mask); |
| |
| - if (rsc->mr_valid) |
| + if (rsc->sig_mr->need_inval) |
| iser_inv_rkey(&tx_desc->inv_wr, mr, cqe, &wr->wr); |
| |
| ib_update_fast_reg_key(mr, ib_inc_rkey(mr->rkey)); |
| @@ -288,7 +288,7 @@ static int iser_reg_sig_mr(struct iscsi_iser_task *iser_task, |
| wr->access = IB_ACCESS_LOCAL_WRITE | |
| IB_ACCESS_REMOTE_READ | |
| IB_ACCESS_REMOTE_WRITE; |
| - rsc->mr_valid = 1; |
| + rsc->sig_mr->need_inval = true; |
| |
| sig_reg->sge.lkey = mr->lkey; |
| sig_reg->rkey = mr->rkey; |
| @@ -313,7 +313,7 @@ static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task, |
| struct ib_reg_wr *wr = &tx_desc->reg_wr; |
| int n; |
| |
| - if (rsc->mr_valid) |
| + if (rsc->mr->need_inval) |
| iser_inv_rkey(&tx_desc->inv_wr, mr, cqe, &wr->wr); |
| |
| ib_update_fast_reg_key(mr, ib_inc_rkey(mr->rkey)); |
| @@ -336,7 +336,7 @@ static int iser_fast_reg_mr(struct iscsi_iser_task *iser_task, |
| IB_ACCESS_REMOTE_WRITE | |
| IB_ACCESS_REMOTE_READ; |
| |
| - rsc->mr_valid = 1; |
| + rsc->mr->need_inval = true; |
| |
| reg->sge.lkey = mr->lkey; |
| reg->rkey = mr->rkey; |
| diff --git a/drivers/infiniband/ulp/iser/iser_verbs.c b/drivers/infiniband/ulp/iser/iser_verbs.c |
| index 95b8eebf7e04..6801b70dc9e0 100644 |
| --- a/drivers/infiniband/ulp/iser/iser_verbs.c |
| +++ b/drivers/infiniband/ulp/iser/iser_verbs.c |
| @@ -129,7 +129,6 @@ iser_create_fastreg_desc(struct iser_device *device, |
| goto err_alloc_mr_integrity; |
| } |
| } |
| - desc->rsc.mr_valid = 0; |
| |
| return desc; |
| |
| -- |
| 2.43.0 |
| |