Every function that returns COMPST_ERROR must set wqe->status to another value than IB_WC_SUCCESS before returning COMPST_ERROR. Fix the only code path for which this is not yet the case.
Signed-off-by: Bart Van Assche bart.vanassche@wdc.com Cc: Zhu Yanjun yanjun.zhu@oracle.com Cc: Jianchao Wang jianchao.w.wang@oracle.com Cc: Yuval Shaia yuval.shaia@oracle.com Cc: stable@vger.kernel.org ---
Changes compared to v1: left out a pr_err() statement as requested by Jason.
drivers/infiniband/sw/rxe/rxe_comp.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c index 98d470d1f3fc..83311dd07019 100644 --- a/drivers/infiniband/sw/rxe/rxe_comp.c +++ b/drivers/infiniband/sw/rxe/rxe_comp.c @@ -276,6 +276,7 @@ static inline enum comp_state check_ack(struct rxe_qp *qp, case IB_OPCODE_RC_RDMA_READ_RESPONSE_MIDDLE: if (wqe->wr.opcode != IB_WR_RDMA_READ && wqe->wr.opcode != IB_WR_RDMA_READ_WITH_INV) { + wqe->status = IB_WC_FATAL_ERR; return COMPST_ERROR; } reset_retry_counters(qp);
On Tue, Jun 26, 2018 at 08:39:36AM -0700, Bart Van Assche wrote:
Every function that returns COMPST_ERROR must set wqe->status to another value than IB_WC_SUCCESS before returning COMPST_ERROR. Fix the only code path for which this is not yet the case.
It is unclear to me why other places left out from this fix (e.x. lines 248, 288 etc).
Signed-off-by: Bart Van Assche bart.vanassche@wdc.com Cc: Zhu Yanjun yanjun.zhu@oracle.com Cc: Jianchao Wang jianchao.w.wang@oracle.com Cc: Yuval Shaia yuval.shaia@oracle.com Cc: stable@vger.kernel.org
Changes compared to v1: left out a pr_err() statement as requested by Jason.
drivers/infiniband/sw/rxe/rxe_comp.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c index 98d470d1f3fc..83311dd07019 100644 --- a/drivers/infiniband/sw/rxe/rxe_comp.c +++ b/drivers/infiniband/sw/rxe/rxe_comp.c @@ -276,6 +276,7 @@ static inline enum comp_state check_ack(struct rxe_qp *qp, case IB_OPCODE_RC_RDMA_READ_RESPONSE_MIDDLE: if (wqe->wr.opcode != IB_WR_RDMA_READ && wqe->wr.opcode != IB_WR_RDMA_READ_WITH_INV) {
wqe->status = IB_WC_FATAL_ERR; return COMPST_ERROR;-- 2.17.1
On 06/27/18 09:32, Yuval Shaia wrote:
On Tue, Jun 26, 2018 at 08:39:36AM -0700, Bart Van Assche wrote:
Every function that returns COMPST_ERROR must set wqe->status to another value than IB_WC_SUCCESS before returning COMPST_ERROR. Fix the only code path for which this is not yet the case.
It is unclear to me why other places left out from this fix (e.x. lines 248, 288 etc).
Hello Yuval,
wqe->status is already set before the code that you mentioned has been reached.
Bart.
wqe->status is already set before the code that you mentioned has been reached.
can you be more specific? for instance I didn't find similar behavior for the example below (also in check_ack())
case IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE: syn = aeth_syn(pkt);
if ((syn & AETH_TYPE_MASK) != AETH_ACK) return COMPST_ERROR;
if (wqe->wr.opcode != IB_WR_ATOMIC_CMP_AND_SWP && wqe->wr.opcode != IB_WR_ATOMIC_FETCH_AND_ADD) return COMPST_ERROR; <<<<<<<<<<<<<<<<<<<<<<<<<< reset_retry_counters(qp); return COMPST_ATOMIC;
On 06/28/18 01:58, Moni Shoua wrote:
wqe->status is already set before the code that you mentioned has been reached.
can you be more specific? for instance I didn't find similar behavior for the example below (also in check_ack())
case IB_OPCODE_RC_ATOMIC_ACKNOWLEDGE: syn = aeth_syn(pkt); if ((syn & AETH_TYPE_MASK) != AETH_ACK) return COMPST_ERROR; if (wqe->wr.opcode != IB_WR_ATOMIC_CMP_AND_SWP && wqe->wr.opcode != IB_WR_ATOMIC_FETCH_AND_ADD) return COMPST_ERROR; <<<<<<<<<<<<<<<<<<<<<<<<<< reset_retry_counters(qp); return COMPST_ATOMIC;
Hello Moni,
I may have been wrong when I claimed that all other paths that return COMPST_ERROR already set wqe->status to an error value. However, I only have a test case for the path I modified. Hence my patch.
Bart.
On Tue, Jun 26, 2018 at 08:39:36AM -0700, Bart Van Assche wrote:
Every function that returns COMPST_ERROR must set wqe->status to another value than IB_WC_SUCCESS before returning COMPST_ERROR. Fix the only code path for which this is not yet the case.
Signed-off-by: Bart Van Assche bart.vanassche@wdc.com Cc: Zhu Yanjun yanjun.zhu@oracle.com Cc: Jianchao Wang jianchao.w.wang@oracle.com Cc: Yuval Shaia yuval.shaia@oracle.com Cc: stable@vger.kernel.org Reviewed-by: Yuval Shaia yuval.shaia@oracle.com
Changes compared to v1: left out a pr_err() statement as requested by Jason.
drivers/infiniband/sw/rxe/rxe_comp.c | 1 + 1 file changed, 1 insertion(+)
applied to for-next
Thanks, Jason
linux-stable-mirror@lists.linaro.org
-
Bart Van Assche -
Jason Gunthorpe -
Moni Shoua -
Yuval Shaia