RDMA/mlx5: Clear old rate limit when closing QP

From: Rafi Wiener <[email protected]>

Before QP is closed it changes to ERROR state, when this happens the QP was left with old rate limit that was already removed from the table.

---
 drivers/infiniband/hw/mlx5/qp.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

index 8937d72ddcf6..5fd071c05944 100644
@@ -3249,10 +3249,12 @@ static int modify_raw_packet_qp_sq(
 	}
 
 	/* Only remove the old rate after new rate was set */
-	if ((old_rl.rate &&
-	     !mlx5_rl_are_equal(&old_rl, &new_rl)) ||
-	    (new_state != MLX5_SQC_STATE_RDY))
+	if ((old_rl.rate && !mlx5_rl_are_equal(&old_rl, &new_rl)) ||
+	    (new_state != MLX5_SQC_STATE_RDY)) {
 		mlx5_rl_remove_rate(dev, &old_rl);
+		if (new_state != MLX5_SQC_STATE_RDY)
+			memset(&new_rl, 0, sizeof(new_rl));
+	}
 
 	ibqp->rl = new_rl;
 	sq->state = new_state;

2.20.1

If you are in the process of closing the queue pair, does this solve some sort of multi-close race, or is it just being pedantic before freeing the qp struct?

I took it regardless, just curious.

Doug Ledford <[email protected]>
    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

It fixes real bug with panic, I didn't add splat, because it had debug info needed to find this problem.

The nutshell of this bug is how we are storing rate limits:
in one table of global mlx5_core_dev and struct (not pointer) of mlx5_rate_limit inside mlx5_ib_qp. Such combination still allows access to rate limit (old one) for ibqp, for example for compare (mlx5_rl_are_equal).

The best solution is to rewrite rl logic to use pointers, but it was too much to demand from Oleg and Rafi, who stepped on this bug with their user space application.

Thanks

Hi Doug,
The issue is when you have a few QPs with the same rate limit you remove the rate twice from the table instead of once. This causes issue, no racing or stressing occurs. Rafi

-----Original Message-----
From: Doug Ledford <[email protected]>
Sent: Thursday, October 17, 2019 11:12 PM
To: Leon Romanovsky <[email protected]>; Jason Gunthorpe <[email protected]>
Cc: Rafi Wiener <[email protected]>; RDMA mailing list <[email protected]>; Bodong Wang <[email protected]>; Oleg Kuporosov <[email protected]>; Leon Romanovsky <[email protected]>
Subject: Re: [PATCH rdma-rc] RDMA/mlx5: Clear old rate limit when closing QP

If you are in the process of closing the queue pair, does this solve some sort of multi-close race, or is it just being pedantic before freeing the qp struct?

I took it regardless, just curious.

--
Doug Ledford <[email protected]>

    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD