On Fri, Apr 24, 2020 at 4:51 PM Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> 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: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx>
> > ---
> > 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
> trond.myklebust@xxxxxxxxxxxxxxx
>
>
