Skip to content

Commit 5d122db

Browse files
Bob Pearsonjgunthorpe
authored andcommitted
RDMA/rxe: Fix incomplete state save in rxe_requester
If a send packet is dropped by the IP layer in rxe_requester() the call to rxe_xmit_packet() can fail with err == -EAGAIN. To recover, the state of the wqe is restored to the state before the packet was sent so it can be resent. However, the routines that save and restore the state miss a significnt part of the variable state in the wqe, the dma struct which is used to process through the sge table. And, the state is not saved before the packet is built which modifies the dma struct. Under heavy stress testing with many QPs on a fast node sending large messages to a slow node dropped packets are observed and the resent packets are corrupted because the dma struct was not restored. This patch fixes this behavior and allows the test cases to succeed. Fixes: 3050b99 ("IB/rxe: Fix race condition between requester and completer") Link: https://lore.kernel.org/r/20230721200748.4604-1-rpearsonhpe@gmail.com Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
1 parent cc28f35 commit 5d122db

File tree

1 file changed

+25
-20
lines changed

1 file changed

+25
-20
lines changed

drivers/infiniband/sw/rxe/rxe_req.c

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -578,21 +578,23 @@ static void save_state(struct rxe_send_wqe *wqe,
578578
struct rxe_send_wqe *rollback_wqe,
579579
u32 *rollback_psn)
580580
{
581-
rollback_wqe->state = wqe->state;
581+
rollback_wqe->state = wqe->state;
582582
rollback_wqe->first_psn = wqe->first_psn;
583-
rollback_wqe->last_psn = wqe->last_psn;
584-
*rollback_psn = qp->req.psn;
583+
rollback_wqe->last_psn = wqe->last_psn;
584+
rollback_wqe->dma = wqe->dma;
585+
*rollback_psn = qp->req.psn;
585586
}
586587

587588
static void rollback_state(struct rxe_send_wqe *wqe,
588589
struct rxe_qp *qp,
589590
struct rxe_send_wqe *rollback_wqe,
590591
u32 rollback_psn)
591592
{
592-
wqe->state = rollback_wqe->state;
593+
wqe->state = rollback_wqe->state;
593594
wqe->first_psn = rollback_wqe->first_psn;
594-
wqe->last_psn = rollback_wqe->last_psn;
595-
qp->req.psn = rollback_psn;
595+
wqe->last_psn = rollback_wqe->last_psn;
596+
wqe->dma = rollback_wqe->dma;
597+
qp->req.psn = rollback_psn;
596598
}
597599

598600
static void update_state(struct rxe_qp *qp, struct rxe_pkt_info *pkt)
@@ -797,6 +799,9 @@ int rxe_requester(struct rxe_qp *qp)
797799
pkt.mask = rxe_opcode[opcode].mask;
798800
pkt.wqe = wqe;
799801

802+
/* save wqe state before we build and send packet */
803+
save_state(wqe, qp, &rollback_wqe, &rollback_psn);
804+
800805
av = rxe_get_av(&pkt, &ah);
801806
if (unlikely(!av)) {
802807
rxe_dbg_qp(qp, "Failed no address vector\n");
@@ -829,29 +834,29 @@ int rxe_requester(struct rxe_qp *qp)
829834
if (ah)
830835
rxe_put(ah);
831836

832-
/*
833-
* To prevent a race on wqe access between requester and completer,
834-
* wqe members state and psn need to be set before calling
835-
* rxe_xmit_packet().
836-
* Otherwise, completer might initiate an unjustified retry flow.
837-
*/
838-
save_state(wqe, qp, &rollback_wqe, &rollback_psn);
837+
/* update wqe state as though we had sent it */
839838
update_wqe_state(qp, wqe, &pkt);
840839
update_wqe_psn(qp, wqe, &pkt, payload);
841840

842841
err = rxe_xmit_packet(qp, &pkt, skb);
843842
if (err) {
844-
qp->need_req_skb = 1;
843+
if (err != -EAGAIN) {
844+
wqe->status = IB_WC_LOC_QP_OP_ERR;
845+
goto err;
846+
}
845847

848+
/* the packet was dropped so reset wqe to the state
849+
* before we sent it so we can try to resend
850+
*/
846851
rollback_state(wqe, qp, &rollback_wqe, rollback_psn);
847852

848-
if (err == -EAGAIN) {
849-
rxe_sched_task(&qp->req.task);
850-
goto exit;
851-
}
853+
/* force a delay until the dropped packet is freed and
854+
* the send queue is drained below the low water mark
855+
*/
856+
qp->need_req_skb = 1;
852857

853-
wqe->status = IB_WC_LOC_QP_OP_ERR;
854-
goto err;
858+
rxe_sched_task(&qp->req.task);
859+
goto exit;
855860
}
856861

857862
update_state(qp, &pkt);

0 commit comments

Comments
 (0)