| Message ID | 541B8CF0.3090502@akamai.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Sep 18, 2014 at 9:54 PM, Jason Baron <jbaron@akamai.com> wrote: > On 09/18/2014 05:20 PM, Trond Myklebust wrote: >> On Thu, Sep 18, 2014 at 5:02 PM, Jason Baron <jbaron@akamai.com> wrote: >>> On 09/18/2014 04:51 PM, Trond Myklebust wrote: >>>> On Thu, Sep 18, 2014 at 3:51 PM, Jason Baron <jbaron@akamai.com> 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 <nfs server ip> -j DROP >>>>> 3) write to /nfs/foo >>>>> 4) close /nfs/foo >>>>> 5) iptables -D OUTPUT -d <nfs server ip> -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.
On 09/19/2014 03:41 PM, Trond Myklebust wrote: > On Thu, Sep 18, 2014 at 9:54 PM, Jason Baron <jbaron@akamai.com> wrote: >> On 09/18/2014 05:20 PM, Trond Myklebust wrote: >>> On Thu, Sep 18, 2014 at 5:02 PM, Jason Baron <jbaron@akamai.com> wrote: >>>> On 09/18/2014 04:51 PM, Trond Myklebust wrote: >>>>> On Thu, Sep 18, 2014 at 3:51 PM, Jason Baron <jbaron@akamai.com> 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 <nfs server ip> -j DROP >>>>>> 3) write to /nfs/foo >>>>>> 4) close /nfs/foo >>>>>> 5) iptables -D OUTPUT -d <nfs server ip> -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 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <jbaron@akamai.com> wrote: >>> On 09/18/2014 05:20 PM, Trond Myklebust wrote: >>>> On Thu, Sep 18, 2014 at 5:02 PM, Jason Baron <jbaron@akamai.com> wrote: >>>>> On 09/18/2014 04:51 PM, Trond Myklebust wrote: >>>>>> On Thu, Sep 18, 2014 at 3:51 PM, Jason Baron <jbaron@akamai.com> 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 <nfs server ip> -j DROP >>>>>>> 3) write to /nfs/foo >>>>>>> 4) close /nfs/foo >>>>>>> 5) iptables -D OUTPUT -d <nfs server ip> -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 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 22, 2014 at 1:55 PM, Jason Baron <jbaron@akamai.com> wrote: > 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 <jbaron@akamai.com> wrote: >>>> On 09/18/2014 05:20 PM, Trond Myklebust wrote: >>>>> On Thu, Sep 18, 2014 at 5:02 PM, Jason Baron <jbaron@akamai.com> wrote: >>>>>> On 09/18/2014 04:51 PM, Trond Myklebust wrote: >>>>>>> On Thu, Sep 18, 2014 at 3:51 PM, Jason Baron <jbaron@akamai.com> 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 <nfs server ip> -j DROP >>>>>>>> 3) write to /nfs/foo >>>>>>>> 4) close /nfs/foo >>>>>>>> 5) iptables -D OUTPUT -d <nfs server ip> -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? > Please do. Thanks! Trond
--- 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);