Re: [PATCH RFC] SUNRPC: Backchannel RPCs don't fail when the transport disconnects

[Chuck Lever <chuck.lever@oracle.com>]

> On Apr 3, 2020, at 7:11 PM, Chuck Lever <chuck.lever@oracle.com> wrote:
> 
> 
> 
>> On Apr 3, 2020, at 6:43 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
>> 
>> On Fri, 2020-04-03 at 17:46 -0400, Chuck Lever wrote:
>>>> On Apr 3, 2020, at 4:05 PM, Chuck Lever <chuck.lever@oracle.com>
>>>> wrote:
>>>> 
>>>> Hi Trond, thanks for the look!
>>>> 
>>>>> On Apr 3, 2020, at 4:00 PM, Trond Myklebust <
>>>>> trondmy@hammerspace.com> wrote:
>>>>> 
>>>>> On Fri, 2020-04-03 at 15:42 -0400, Chuck Lever wrote:
>>>>>> Commit 3832591e6fa5 ("SUNRPC: Handle connection issues
>>>>>> correctly on
>>>>>> the back channel") intended to make backchannel RPCs fail
>>>>>> immediately when there is no forward channel connection. What's
>>>>>> currently happening is, when the forward channel conneciton
>>>>>> goes
>>>>>> away, backchannel operations are causing hard loops because
>>>>>> call_transmit_status's SOFTCONN logic ignores ENOTCONN.
>>>>> 
>>>>> Won't RPC_TASK_NOCONNECT do the right thing? It should cause the
>>>>> request to exit with an ENOTCONN error when it hits
>>>>> call_connect().
>>>> 
>>>> OK, so does that mean SOFTCONN is entirely the wrong semantic here?
>>>> 
>>>> Was RPC_TASK_NOCONNECT available when 3832591e6fa5 was merged?
>>> 
>>> It turns out 3832591e6fa5 is not related. It's 58255a4e3ce5 that
>>> added
>>> RPC_TASK_SOFTCONN on NFSv4 callback Calls.
>>> 
>>> However, the server uses nfsd4_run_cb_work() for both NFSv4.0 and
>>> NFSv4.1 callbacks. IMO a fix for this will have to take care that
>>> RPC_TASK_NOCONNECT is not set on NFSv4.0 callback tasks.
>> 
>> Possibly, but don't we really want to let such a NFSv4.0 request fail
>> and send another CB_NULL? There is already version-specific code in
>> nfsd4_process_cb_update() to set up the callback client.
> 
> A not unreasonable conclusion. But it's hard to test the NFSv4.0 case,
> since it's instability on the forward channel that is tickling this
> problem. The NFSv4.0 callback connection is not affected by that.
> 
> Maybe Bruce has a thought? Otherwise we can try an unconditional
> NOCONNECT for now. RPC_TASK_NOCONNECT was added three years after
> 58255a4e3ce5, fwiw...

I confirmed that NFSv4.0 callback does not tolerate using the
RPC_TASK_NOCONNECT flag in nfsd4_run_cb_work(). After replacing
SOFTCONN with NOCONNECT, the NFSv4.0 mount operates without using
delegation.

setup_callback_client creates the callback rpc_clnt with NOPING. The
current callback mechanism depends on the next RPC Call to initiate
connection establishment.

Setting NOCONNECT by itself is still not enough to prevent a soft
lockup, btw. The rpc_xprt for the backchannel is still marked
connected, so the NOCONNECT check in call_connect is skipped entirely
on subsequent retransmits. My fix now includes some new code to
ensure that the backchannel rpc_xprt is marked closed by
svc_delete_xprt.

I'll post the updated patch soon.


--
Chuck Lever