| Message ID | 1628782891-26471-1-git-send-email-haakon.bugge@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [for-next] RDMA/core/sa_query: Retry SA queries | expand |
> On 12 Aug 2021, at 17:41, Håkon Bugge <Haakon.Bugge@oracle.com> 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@oracle.com> > --- > 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 >
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) + 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;
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@oracle.com> --- drivers/infiniband/core/cma.c | 3 +++ drivers/infiniband/core/sa_query.c | 9 ++++++++- 2 files changed, 11 insertions(+), 1 deletion(-)