Currently, if the client sends BIND_CONN_TO_SESSION with
NFS4_CDFC4_FORE_OR_BOTH but only gets NFS4_CDFS4_FORE back it ignores
that it wasn't able to enable a backchannel.
To make sure, the client sends BIND_CONN_TO_SESSION as the first
operation on the connections (ie., no other session compounds haven't
been sent before), and if the client's request to bind the backchannel
is not satisfied, then reset the connection and retry.
Cc: [email protected]
Signed-off-by: Olga Kornievskaia <[email protected]>
---
fs/nfs/nfs4proc.c | 6 ++++++
include/linux/sunrpc/clnt.h | 5 +++++
2 files changed, 11 insertions(+)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 512afb1..69a5487 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7891,6 +7891,7 @@ static int nfs4_check_cl_exchange_flags(u32 flags)
nfs4_bind_one_conn_to_session_done(struct rpc_task *task, void *calldata)
{
struct nfs41_bind_conn_to_session_args *args = task->tk_msg.rpc_argp;
+ struct nfs41_bind_conn_to_session_res *res = task->tk_msg.rpc_resp;
struct nfs_client *clp = args->client;
switch (task->tk_status) {
@@ -7899,6 +7900,11 @@ static int nfs4_check_cl_exchange_flags(u32 flags)
nfs4_schedule_session_recovery(clp->cl_session,
task->tk_status);
}
+ if (args->dir == NFS4_CDFC4_FORE_OR_BOTH &&
+ res->dir != NFS4_CDFS4_BOTH) {
+ rpc_task_close_connection(task);
+ task->tk_status = -NFS4ERR_DELAY;
+ }
}
static const struct rpc_call_ops nfs4_bind_one_conn_to_session_ops = {
diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index ca7e108..cc20a08 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -236,4 +236,9 @@ static inline int rpc_reply_expected(struct rpc_task *task)
(task->tk_msg.rpc_proc->p_decode != NULL);
}
+static inline void rpc_task_close_connection(struct rpc_task *task)
+{
+ if (task->tk_xprt)
+ xprt_force_disconnect(task->tk_xprt);
+}
#endif /* _LINUX_SUNRPC_CLNT_H */
--
1.8.3.1
Hi Olga
On Fri, 2020-04-24 at 15:08 -0400, Olga Kornievskaia wrote:
> Currently, if the client sends BIND_CONN_TO_SESSION with
> NFS4_CDFC4_FORE_OR_BOTH but only gets NFS4_CDFS4_FORE back it ignores
> that it wasn't able to enable a backchannel.
>
> To make sure, the client sends BIND_CONN_TO_SESSION as the first
> operation on the connections (ie., no other session compounds haven't
> been sent before), and if the client's request to bind the
> backchannel
> is not satisfied, then reset the connection and retry.
>
> Cc: [email protected]
> Signed-off-by: Olga Kornievskaia <[email protected]>
> ---
> fs/nfs/nfs4proc.c | 6 ++++++
> include/linux/sunrpc/clnt.h | 5 +++++
> 2 files changed, 11 insertions(+)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 512afb1..69a5487 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -7891,6 +7891,7 @@ static int nfs4_check_cl_exchange_flags(u32
> flags)
> nfs4_bind_one_conn_to_session_done(struct rpc_task *task, void
> *calldata)
> {
> struct nfs41_bind_conn_to_session_args *args = task-
> >tk_msg.rpc_argp;
> + struct nfs41_bind_conn_to_session_res *res = task-
> >tk_msg.rpc_resp;
> struct nfs_client *clp = args->client;
>
> switch (task->tk_status) {
> @@ -7899,6 +7900,11 @@ static int nfs4_check_cl_exchange_flags(u32
> flags)
> nfs4_schedule_session_recovery(clp->cl_session,
> task->tk_status);
> }
> + if (args->dir == NFS4_CDFC4_FORE_OR_BOTH &&
> + res->dir != NFS4_CDFS4_BOTH) {
> + rpc_task_close_connection(task);
> + task->tk_status = -NFS4ERR_DELAY;
We don't have any error handling for NFS4ERR_DELAY in this function, so
if your intention is to replay the RPC call, then maybe you want to
replace this line with a call to rpc_restart_call()?
However it would be good to ensure that we only do this once or twice
so that we don't loop forever. Maybe also add a variable to the struct
nfs41_bind_conn_to_session_args that can set the number of retries
remaining?
> + }
> }
>
> static const struct rpc_call_ops nfs4_bind_one_conn_to_session_ops =
> {
> diff --git a/include/linux/sunrpc/clnt.h
> b/include/linux/sunrpc/clnt.h
> index ca7e108..cc20a08 100644
> --- a/include/linux/sunrpc/clnt.h
> +++ b/include/linux/sunrpc/clnt.h
> @@ -236,4 +236,9 @@ static inline int rpc_reply_expected(struct
> rpc_task *task)
> (task->tk_msg.rpc_proc->p_decode != NULL);
> }
>
> +static inline void rpc_task_close_connection(struct rpc_task *task)
> +{
> + if (task->tk_xprt)
> + xprt_force_disconnect(task->tk_xprt);
> +}
> #endif /* _LINUX_SUNRPC_CLNT_H */
Thanks
Trond
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]
On Fri, Apr 24, 2020 at 4:51 PM Trond Myklebust <[email protected]> wrote:
>
> Hi Olga
>
> On Fri, 2020-04-24 at 15:08 -0400, Olga Kornievskaia wrote:
> > Currently, if the client sends BIND_CONN_TO_SESSION with
> > NFS4_CDFC4_FORE_OR_BOTH but only gets NFS4_CDFS4_FORE back it ignores
> > that it wasn't able to enable a backchannel.
> >
> > To make sure, the client sends BIND_CONN_TO_SESSION as the first
> > operation on the connections (ie., no other session compounds haven't
> > been sent before), and if the client's request to bind the
> > backchannel
> > is not satisfied, then reset the connection and retry.
> >
> > Cc: [email protected]
> > Signed-off-by: Olga Kornievskaia <[email protected]>
> > ---
> > fs/nfs/nfs4proc.c | 6 ++++++
> > include/linux/sunrpc/clnt.h | 5 +++++
> > 2 files changed, 11 insertions(+)
> >
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 512afb1..69a5487 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -7891,6 +7891,7 @@ static int nfs4_check_cl_exchange_flags(u32
> > flags)
> > nfs4_bind_one_conn_to_session_done(struct rpc_task *task, void
> > *calldata)
> > {
> > struct nfs41_bind_conn_to_session_args *args = task-
> > >tk_msg.rpc_argp;
> > + struct nfs41_bind_conn_to_session_res *res = task-
> > >tk_msg.rpc_resp;
> > struct nfs_client *clp = args->client;
> >
> > switch (task->tk_status) {
> > @@ -7899,6 +7900,11 @@ static int nfs4_check_cl_exchange_flags(u32
> > flags)
> > nfs4_schedule_session_recovery(clp->cl_session,
> > task->tk_status);
> > }
> > + if (args->dir == NFS4_CDFC4_FORE_OR_BOTH &&
> > + res->dir != NFS4_CDFS4_BOTH) {
> > + rpc_task_close_connection(task);
> > + task->tk_status = -NFS4ERR_DELAY;
>
> We don't have any error handling for NFS4ERR_DELAY in this function, so
> if your intention is to replay the RPC call, then maybe you want to
> replace this line with a call to rpc_restart_call()?
What happens is nfs4_proc_bind_conn_to_session() is errored with
ERR_DELAY which goes back into nfs4state.c which has a case for
ERR_DELAY which resets the bit and does the call again. But I can try
the rpc_restart_call().
> However it would be good to ensure that we only do this once or twice
> so that we don't loop forever. Maybe also add a variable to the struct
> nfs41_bind_conn_to_session_args that can set the number of retries
> remaining?
If I recode using rpc_restart_call, then I can add something to the
args. Otherwise, number of retries would need to be handled some other
way that I'd need to think of.
Thank you for all the help.
>
> > + }
> > }
> >
> > static const struct rpc_call_ops nfs4_bind_one_conn_to_session_ops =
> > {
> > diff --git a/include/linux/sunrpc/clnt.h
> > b/include/linux/sunrpc/clnt.h
> > index ca7e108..cc20a08 100644
> > --- a/include/linux/sunrpc/clnt.h
> > +++ b/include/linux/sunrpc/clnt.h
> > @@ -236,4 +236,9 @@ static inline int rpc_reply_expected(struct
> > rpc_task *task)
> > (task->tk_msg.rpc_proc->p_decode != NULL);
> > }
> >
> > +static inline void rpc_task_close_connection(struct rpc_task *task)
> > +{
> > + if (task->tk_xprt)
> > + xprt_force_disconnect(task->tk_xprt);
> > +}
> > #endif /* _LINUX_SUNRPC_CLNT_H */
>
> Thanks
> Trond
>
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> [email protected]
>
>