| Message ID | 20180626153936.9576-1-bart.vanassche@wdc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
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; > } > reset_retry_counters(qp); > -- > 2.17.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > 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; -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
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(+)