Re: [PATCH for-next v2] RDMA/core/sa_query: Retry SA queries
From: Haakon Bugge
Date: Thu Aug 26 2021 - 11:59:56 EST
> On 25 Aug 2021, at 19:49, Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:
>
> On Thu, Aug 12, 2021 at 06:12:35PM +0200, Håkon Bugge 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(-)
>
> I'm nervous about this, mostly because the mad layer is very
> complicated, but it does seem aligned with the spec.
>
> However, it seems quite wrong that the timeout comes in from outside,
> the SA timeout should be integral to the SA layer..
They are quite different (timeout in ms):
iser: 1000
rtrs: 30000
srp: 1000
nvme: 3000
samba: 5000
p9: 30000
rds: 5000
xprtrdma: 5000
Dividing 30 seconds by ten and get 3, seems OK. But for iser/srp, we get 100ms, which is in the low end for some system I would expect.
> Anyhow, applied to for-next
Thanks!
Håkon