| Message ID | 20220705155705.21094-1-yangx.jy@fujitsu.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | RDMA/rxe: Use correct ATOMIC Acknowledge opcode in BTH | expand |
On 2022/7/5 23:57, Xiao Yang wrote: > When responder processed an Atomic requeset and got a NAK, > the opcode field in BTH should be ATOMIC Acknowledge instead > of Acknowledge. Hi, Sorry, please ignore the wrong patch because it will lead to a kernel panic. Best Regards, Xiao Yang > > Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com> > --- > drivers/infiniband/sw/rxe/rxe_resp.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c > index 265e46fe050f..592d73c37d48 100644 > --- a/drivers/infiniband/sw/rxe/rxe_resp.c > +++ b/drivers/infiniband/sw/rxe/rxe_resp.c > @@ -1080,10 +1080,10 @@ static enum resp_states acknowledge(struct rxe_qp *qp, > if (qp_type(qp) != IB_QPT_RC) > return RESPST_CLEANUP; > > - if (qp->resp.aeth_syndrome != AETH_ACK_UNLIMITED) > + if (pkt->mask & RXE_ATOMIC_MASK) > + send_atomic_ack(qp, qp->resp.aeth_syndrome, pkt->psn); > + else if (qp->resp.aeth_syndrome != AETH_ACK_UNLIMITED) > send_ack(qp, qp->resp.aeth_syndrome, pkt->psn); > - else if (pkt->mask & RXE_ATOMIC_MASK) > - send_atomic_ack(qp, AETH_ACK_UNLIMITED, pkt->psn); > else if (bth_ack(pkt)) > send_ack(qp, AETH_ACK_UNLIMITED, pkt->psn); >
On 7/5/22 10:57, Xiao Yang wrote: > When responder processed an Atomic requeset and got a NAK, > the opcode field in BTH should be ATOMIC Acknowledge instead > of Acknowledge. > > Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com> > --- > drivers/infiniband/sw/rxe/rxe_resp.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c > index 265e46fe050f..592d73c37d48 100644 > --- a/drivers/infiniband/sw/rxe/rxe_resp.c > +++ b/drivers/infiniband/sw/rxe/rxe_resp.c > @@ -1080,10 +1080,10 @@ static enum resp_states acknowledge(struct rxe_qp *qp, > if (qp_type(qp) != IB_QPT_RC) > return RESPST_CLEANUP; > > - if (qp->resp.aeth_syndrome != AETH_ACK_UNLIMITED) > + if (pkt->mask & RXE_ATOMIC_MASK) > + send_atomic_ack(qp, qp->resp.aeth_syndrome, pkt->psn); > + else if (qp->resp.aeth_syndrome != AETH_ACK_UNLIMITED) > send_ack(qp, qp->resp.aeth_syndrome, pkt->psn); > - else if (pkt->mask & RXE_ATOMIC_MASK) > - send_atomic_ack(qp, AETH_ACK_UNLIMITED, pkt->psn); > else if (bth_ack(pkt)) > send_ack(qp, AETH_ACK_UNLIMITED, pkt->psn); > Xaio, Just saw this. The difference between the two opcodes tells if there is an AtomicAckETH present. If the operation fails there is no data to send back so there is only an AETH in the reply packet and Acknowledge is the correct opcode. When the packet is parsed if you use AtomicAcknowledge for a failed atomic operation you will get the wrong offsets when you lookup the header offsets in rxe_opcode[] which not surprisingly leads to problems. Bob
diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/infiniband/sw/rxe/rxe_resp.c index 265e46fe050f..592d73c37d48 100644 --- a/drivers/infiniband/sw/rxe/rxe_resp.c +++ b/drivers/infiniband/sw/rxe/rxe_resp.c @@ -1080,10 +1080,10 @@ static enum resp_states acknowledge(struct rxe_qp *qp, if (qp_type(qp) != IB_QPT_RC) return RESPST_CLEANUP; - if (qp->resp.aeth_syndrome != AETH_ACK_UNLIMITED) + if (pkt->mask & RXE_ATOMIC_MASK) + send_atomic_ack(qp, qp->resp.aeth_syndrome, pkt->psn); + else if (qp->resp.aeth_syndrome != AETH_ACK_UNLIMITED) send_ack(qp, qp->resp.aeth_syndrome, pkt->psn); - else if (pkt->mask & RXE_ATOMIC_MASK) - send_atomic_ack(qp, AETH_ACK_UNLIMITED, pkt->psn); else if (bth_ack(pkt)) send_ack(qp, AETH_ACK_UNLIMITED, pkt->psn);
When responder processed an Atomic requeset and got a NAK, the opcode field in BTH should be ATOMIC Acknowledge instead of Acknowledge. Signed-off-by: Xiao Yang <yangx.jy@fujitsu.com> --- drivers/infiniband/sw/rxe/rxe_resp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)