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

From: Haakon Bugge
Date: Thu Aug 12 2021 - 12:15:44 EST




> 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
>