Re: [PATCH for-next] RDMA/core/sa_query: Retry SA queries

[Haakon Bugge]

> On 12 Aug 2021, at 17:41, Håkon Bugge <Haakon.Bugge@xxxxxxxxxx> wrote:
> 
> A MAD packet is sent as an unreliable datagram (UD). SA requests are
> sent as MAD packets. As such, SA requests or responses may be silently
> dropped.
> 
> IB Core's MAD layer has a timeout and retry mechanism, which amongst
> other, is used by RDMA CM. But it is not used by SA queries. The lack
> of retries of SA queries leads to long specified timeout, and error
> being returned in case of packet loss. The ULP or user-land process
> has to perform the retry.
> 
> Fix this by taking advantage of the MAD layer's retry mechanism.
> 
> First, a check against a zero timeout is added in
> rdma_resolve_route(). In send_mad(), we set the MAD layer timeout to
> one tenth of the specified timeout and the number of retries to
> 10. The special case when timeout is less than 10 is handled.
> 
> With this fix:
> 
> # ucmatose -c 1000 -S 1024 -C 1
> 
> runs stable on an Infiniband fabric. Without this fix, we see an
> intermittent behavior and it errors out with:
> 
> cmatose: event: RDMA_CM_EVENT_ROUTE_ERROR, error: -110
> 
> (110 is ETIMEDOUT)
> 
> Fixes: f75b7a529494 ("[PATCH] IB: Add automatic retries to MAD layer")
> Signed-off-by: Håkon Bugge <haakon.bugge@xxxxxxxxxx>
> ---
> drivers/infiniband/core/cma.c      | 3 +++
> drivers/infiniband/core/sa_query.c | 9 ++++++++-
> 2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 515a7e9..bbcbab7 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -3117,6 +3117,9 @@ int rdma_resolve_route(struct rdma_cm_id *id, unsigned long timeout_ms)
> 	struct rdma_id_private *id_priv;
> 	int ret;
> 
> +	if (timeout_ms)

Too fat fingers. v2 on its way!

-h


> +		return -EINVAL;
> +
> 	id_priv = container_of(id, struct rdma_id_private, id);
> 	if (!cma_comp_exch(id_priv, RDMA_CM_ADDR_RESOLVED, RDMA_CM_ROUTE_QUERY))
> 		return -EINVAL;
> diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
> index b61576f..5a56082 100644
> --- a/drivers/infiniband/core/sa_query.c
> +++ b/drivers/infiniband/core/sa_query.c
> @@ -1358,6 +1358,7 @@ static int send_mad(struct ib_sa_query *query, unsigned long timeout_ms,
> {
> 	unsigned long flags;
> 	int ret, id;
> +	const int nmbr_sa_query_retries = 10;
> 
> 	xa_lock_irqsave(&queries, flags);
> 	ret = __xa_alloc(&queries, &id, query, xa_limit_32b, gfp_mask);
> @@ -1365,7 +1366,13 @@ static int send_mad(struct ib_sa_query *query, unsigned long timeout_ms,
> 	if (ret < 0)
> 		return ret;
> 
> -	query->mad_buf->timeout_ms  = timeout_ms;
> +	query->mad_buf->timeout_ms  = timeout_ms / nmbr_sa_query_retries;
> +	query->mad_buf->retries = nmbr_sa_query_retries;
> +	if (!query->mad_buf->timeout_ms) {
> +		/* Special case, very small timeout_ms */
> +		query->mad_buf->timeout_ms = 1;
> +		query->mad_buf->retries = timeout_ms;
> +	}
> 	query->mad_buf->context[0] = query;
> 	query->id = id;
> 
> -- 
> 1.8.3.1
>