Return-Path: linux-nfs-owner@vger.kernel.org Received: from prod-mail-xrelay02.akamai.com ([72.246.2.14]:58799 "EHLO prod-mail-xrelay02.akamai.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754285AbaIVRzq (ORCPT ); Mon, 22 Sep 2014 13:55:46 -0400 Message-ID: <542062A0.4030103@akamai.com> Date: Mon, 22 Sep 2014 13:55:44 -0400 From: Jason Baron MIME-Version: 1.0 To: Trond Myklebust CC: Bruce Fields , Linux NFS Mailing List Subject: Re: [PATCH 2/2] rpc: Add -EPERM processing for xs_udp_send_request() References:

<541B484E.90202@akamai.com> <541B8CF0.3090502@akamai.com> <541C9D16.7010800@akamai.com> In-Reply-To: <541C9D16.7010800@akamai.com> Content-Type: text/plain; charset=UTF-8 Sender: linux-nfs-owner@vger.kernel.org List-ID: On 09/19/2014 05:16 PM, Jason Baron wrote: > On 09/19/2014 03:41 PM, Trond Myklebust wrote: >> On Thu, Sep 18, 2014 at 9:54 PM, Jason Baron wrote: >>> On 09/18/2014 05:20 PM, Trond Myklebust wrote: >>>> On Thu, Sep 18, 2014 at 5:02 PM, Jason Baron wrote: >>>>> On 09/18/2014 04:51 PM, Trond Myklebust wrote: >>>>>> On Thu, Sep 18, 2014 at 3:51 PM, Jason Baron wrote: >>>>>>> If an iptables drop rule is added for an nfs server, the client can end up in >>>>>>> a softlockup. Because of the way that xs_sendpages() is structured, the -EPERM >>>>>>> is ignored since the prior bits of the packet may have been successfully queued >>>>>>> and thus xs_sendpages() returns a non-zero value. Then, xs_udp_send_request() >>>>>>> thinks that because some bits were queued it should return -EAGAIN. We then try >>>>>>> the request and again and a softlockup occurs. The test sequence is simply: >>>>>>> >>>>>>> 1) open a file on the nfs server '/nfs/foo' (mounted using udp) >>>>>>> 2) iptables -A OUTPUT -d -j DROP >>>>>>> 3) write to /nfs/foo >>>>>>> 4) close /nfs/foo >>>>>>> 5) iptables -D OUTPUT -d -j DROP >>>>>>> >>>>>>> The softlockup occurs in step 4 above. >>>>>> For UDP, the expected and documented behaviour in the case above is as follows: >>>>>> - if the mount is soft, then return EIO on the first major timeout. >>>>> yeah - so this case is a softlockup in my testing :( >>>>> >>>>>> - if the mount is hard, then retry indefinitely on timeout. >>>>>> >>>>>> Won't these 2 patches end up propagating an EPERM to the application? >>>>>> That would be a definite violation of both hard and soft semantics. >>>>> ok, yeah it does propogate the -EPERM up - I wasn't aware of the correct >>>>> semantics - thanks. >>>>> >>>>> I can rework the patches such that they return -EIO instead for a soft mount, >>>>> and verify that we keep retrying for a hard one. >>>>> >>>> Doesn't the soft timeout currently trigger after the major timeout? If >>>> not, do we understand why it isn't doing so? >>> >>> No, the soft timeout does not currently trigger after the major timeout. Instead, >>> the kernel spins indefinitely, and triggers a softlockup. >>> >>> The reason is that xs_sendpages() returns a positive value in this case >>> and xs_udp_send_request() turns it in an -EAGAIN for the write operation. >>> Subsequently, we call call_transmit_status() and then call_status() which >>> sees the EAGAIN, which just starts all over again with a 'call_transmit()'. >>> So we are stuck spinning indefinitely in kernel space. >>> >>> Simply moving the -EPERM up in this patch, results in the behavior you >>> described above - EIO after a major timeout on a soft mount, and indefinte >>> retries on a hard mount - but without the cpu consumption. IE applying >>> this on top of this patch: >>> >>> --- a/net/sunrpc/clnt.c >>> +++ b/net/sunrpc/clnt.c >>> @@ -2019,6 +2019,7 @@ call_status(struct rpc_task *task) >>> case -EHOSTDOWN: >>> case -EHOSTUNREACH: >>> case -ENETUNREACH: >>> + case -EPERM: >>> if (RPC_IS_SOFTCONN(task)) { >>> rpc_exit(task, status); >>> break; >>> @@ -2048,7 +2049,6 @@ call_status(struct rpc_task *task) >>> case -EAGAIN: >>> task->tk_action = call_transmit; >>> break; >>> - case -EPERM: >>> case -EIO: >>> /* shutdown or soft timeout */ >>> rpc_exit(task, status); >>> >>> We could also 'translate' the -EPERM into an '-ENETUNREACH' or such, >>> in the return from xs_udp_send_request(), if you think that would make >>> more sense? >>> >>> Hopefully, I've explained things better. >>> >>> >> >> Yep. Can you please resend the patch with the above fix? I think we >> can live with the EPERM in the RPC_IS_SOFTCONN case, given that it is >> in practice only ever passed back to the 'mount' syscall. >> > > Hi, > > So after some more testing on this new patch, the test sequence I described > works fine - but if I set the firewall rule first and then do an open, it > appears that the open() wouldn't time out even on a soft mount (whereas > with the previous patch it incorrectly returned -EPERM almost immediately). > It appears that the rpc request is getting queued up onto one of the wait > queues (xprt_backlog or xprt_sending) in that case, but I'm not sure why. > I'll have to look more into it next week. > > Thanks, > > -Jason > > Hi Trond, Ok, so they do timeout now with this patch (for a soft mount) - I simply wasn't waiting long enough (took around 30 minutes in some cases). So I think this patch is ok. If it makes sense I can clean it up based on the comments, and re-submit? Thanks, -Jason