2020-04-24 21:46:30

by Olga Kornievskaia

[permalink] [raw]
Subject: [PATCH v2 1/1] NFSv4.1: fix handling of backchannel binding in BIND_CONN_TO_SESSION

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 | 8 ++++++++
include/linux/nfs_xdr.h | 2 ++
include/linux/sunrpc/clnt.h | 5 +++++
3 files changed, 15 insertions(+)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 512afb1..7e7c24e 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,12 @@ 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);
+ if (args->retries++ < MAX_BIND_CONN_TO_SESSION_RETRIES)
+ rpc_restart_call(task);
+ }
}

static const struct rpc_call_ops nfs4_bind_one_conn_to_session_ops = {
@@ -7921,6 +7928,7 @@ int nfs4_proc_bind_one_conn_to_session(struct rpc_clnt *clnt,
struct nfs41_bind_conn_to_session_args args = {
.client = clp,
.dir = NFS4_CDFC4_FORE_OR_BOTH,
+ .retries = 0,
};
struct nfs41_bind_conn_to_session_res res;
struct rpc_message msg = {
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 4402304..e5f3e7d 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1317,11 +1317,13 @@ struct nfs41_impl_id {
struct nfstime4 date;
};

+#define MAX_BIND_CONN_TO_SESSION_RETRIES 3
struct nfs41_bind_conn_to_session_args {
struct nfs_client *client;
struct nfs4_sessionid sessionid;
u32 dir;
bool use_conn_in_rdma_mode;
+ int retries;
};

struct nfs41_bind_conn_to_session_res {
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


2020-04-26 15:06:30

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] NFSv4.1: fix handling of backchannel binding in BIND_CONN_TO_SESSION

Hi

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v5.6.7, v5.4.35, v4.19.118, v4.14.177, v4.9.220, v4.4.220.

v5.6.7: Build OK!
v5.4.35: Failed to apply! Possible dependencies:
Unable to calculate

v4.19.118: Failed to apply! Possible dependencies:
07d02a67b7fa ("SUNRPC: Simplify lookup code")
3453d5708b33 ("NFSv4.1: Avoid false retries when RPC calls are interrupted")
5c441544f045 ("NFSv4.x: Handle bad/dead sessions correctly in nfs41_sequence_process()")
79b181810285 ("SUNRPC: Convert auth creds to use refcount_t")
8276c902bbe9 ("SUNRPC: remove uid and gid from struct auth_cred")
95cd623250ad ("SUNRPC: Clean up the AUTH cache code")
97f68c6b02e0 ("SUNRPC: add 'struct cred *' to auth_cred and rpc_cred")
a52458b48af1 ("NFS/NFSD/SUNRPC: replace generic creds with 'struct cred'.")
fc0664fd9bcc ("SUNRPC: remove groupinfo from struct auth_cred.")

v4.14.177: Failed to apply! Possible dependencies:
07d02a67b7fa ("SUNRPC: Simplify lookup code")
12f275cdd163 ("NFSv4: Retry CLOSE and DELEGRETURN on NFS4ERR_OLD_STATEID.")
1eb5d98f16f6 ("nfs: convert to new i_version API")
3453d5708b33 ("NFSv4.1: Avoid false retries when RPC calls are interrupted")
35156bfff3c0 ("NFSv4: Fix the nfs_inode_set_delegation() arguments")
5c441544f045 ("NFSv4.x: Handle bad/dead sessions correctly in nfs41_sequence_process()")
79b181810285 ("SUNRPC: Convert auth creds to use refcount_t")
95cd623250ad ("SUNRPC: Clean up the AUTH cache code")
97f68c6b02e0 ("SUNRPC: add 'struct cred *' to auth_cred and rpc_cred")
a52458b48af1 ("NFS/NFSD/SUNRPC: replace generic creds with 'struct cred'.")
b3dce6a2f060 ("pnfs/blocklayout: handle transient devices")
fc0664fd9bcc ("SUNRPC: remove groupinfo from struct auth_cred.")

v4.9.220: Failed to apply! Possible dependencies:
172d9de15a0d ("NFS: Change nfs4_get_session() to take an nfs_client structure")
3453d5708b33 ("NFSv4.1: Avoid false retries when RPC calls are interrupted")
3be0f80b5fe9 ("NFSv4.1: Fix up replays of interrupted requests")
42e1cca7e91e ("NFS: Change nfs4_setup_sequence() to take an nfs_client structure")
5c441544f045 ("NFSv4.x: Handle bad/dead sessions correctly in nfs41_sequence_process()")
6de7e12f53a1 ("NFS: Use nfs4_setup_sequence() everywhere")
7981c8a65914 ("NFS: Create a single nfs4_setup_sequence() function")
efc6f4aa742d ("NFS: Move nfs4_get_session() into nfs4_session.h")

v4.4.220: Failed to apply! Possible dependencies:
172d9de15a0d ("NFS: Change nfs4_get_session() to take an nfs_client structure")
3453d5708b33 ("NFSv4.1: Avoid false retries when RPC calls are interrupted")
3be0f80b5fe9 ("NFSv4.1: Fix up replays of interrupted requests")
42e1cca7e91e ("NFS: Change nfs4_setup_sequence() to take an nfs_client structure")
5c441544f045 ("NFSv4.x: Handle bad/dead sessions correctly in nfs41_sequence_process()")
5f83d86cf531 ("NFSv4.x: Fix wraparound issues when validing the callback sequence id")
68d264cf02b0 ("NFS42: handle layoutstats stateid error")
6de7e12f53a1 ("NFS: Use nfs4_setup_sequence() everywhere")
80f9642724af ("NFSv4.x: Enforce the ca_maxresponsesize_cached on the back channel")
810d82e68301 ("NFSv4.x: Allow multiple callbacks in flight")
9a0fe86745b8 ("pNFS: Handle NFS4ERR_OLD_STATEID correctly in LAYOUTSTAT calls")
efc6f4aa742d ("NFS: Move nfs4_get_session() into nfs4_session.h")
f74a834a0e1b ("NFSv4.x: CB_SEQUENCE should return NFS4ERR_DELAY if still executing")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

--
Thanks
Sasha

2020-04-26 15:27:10

by Olga Kornievskaia

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] NFSv4.1: fix handling of backchannel binding in BIND_CONN_TO_SESSION

On Sun, Apr 26, 2020 at 11:03 AM Sasha Levin <[email protected]> wrote:
>
> Hi
>
> [This is an automated email]
>
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
>
> The bot has tested the following trees: v5.6.7, v5.4.35, v4.19.118, v4.14.177, v4.9.220, v4.4.220.
>
> v5.6.7: Build OK!
> v5.4.35: Failed to apply! Possible dependencies:
> Unable to calculate
>
> v4.19.118: Failed to apply! Possible dependencies:
> 07d02a67b7fa ("SUNRPC: Simplify lookup code")
> 3453d5708b33 ("NFSv4.1: Avoid false retries when RPC calls are interrupted")
> 5c441544f045 ("NFSv4.x: Handle bad/dead sessions correctly in nfs41_sequence_process()")
> 79b181810285 ("SUNRPC: Convert auth creds to use refcount_t")
> 8276c902bbe9 ("SUNRPC: remove uid and gid from struct auth_cred")
> 95cd623250ad ("SUNRPC: Clean up the AUTH cache code")
> 97f68c6b02e0 ("SUNRPC: add 'struct cred *' to auth_cred and rpc_cred")
> a52458b48af1 ("NFS/NFSD/SUNRPC: replace generic creds with 'struct cred'.")
> fc0664fd9bcc ("SUNRPC: remove groupinfo from struct auth_cred.")
>
> v4.14.177: Failed to apply! Possible dependencies:
> 07d02a67b7fa ("SUNRPC: Simplify lookup code")
> 12f275cdd163 ("NFSv4: Retry CLOSE and DELEGRETURN on NFS4ERR_OLD_STATEID.")
> 1eb5d98f16f6 ("nfs: convert to new i_version API")
> 3453d5708b33 ("NFSv4.1: Avoid false retries when RPC calls are interrupted")
> 35156bfff3c0 ("NFSv4: Fix the nfs_inode_set_delegation() arguments")
> 5c441544f045 ("NFSv4.x: Handle bad/dead sessions correctly in nfs41_sequence_process()")
> 79b181810285 ("SUNRPC: Convert auth creds to use refcount_t")
> 95cd623250ad ("SUNRPC: Clean up the AUTH cache code")
> 97f68c6b02e0 ("SUNRPC: add 'struct cred *' to auth_cred and rpc_cred")
> a52458b48af1 ("NFS/NFSD/SUNRPC: replace generic creds with 'struct cred'.")
> b3dce6a2f060 ("pnfs/blocklayout: handle transient devices")
> fc0664fd9bcc ("SUNRPC: remove groupinfo from struct auth_cred.")
>
> v4.9.220: Failed to apply! Possible dependencies:
> 172d9de15a0d ("NFS: Change nfs4_get_session() to take an nfs_client structure")
> 3453d5708b33 ("NFSv4.1: Avoid false retries when RPC calls are interrupted")
> 3be0f80b5fe9 ("NFSv4.1: Fix up replays of interrupted requests")
> 42e1cca7e91e ("NFS: Change nfs4_setup_sequence() to take an nfs_client structure")
> 5c441544f045 ("NFSv4.x: Handle bad/dead sessions correctly in nfs41_sequence_process()")
> 6de7e12f53a1 ("NFS: Use nfs4_setup_sequence() everywhere")
> 7981c8a65914 ("NFS: Create a single nfs4_setup_sequence() function")
> efc6f4aa742d ("NFS: Move nfs4_get_session() into nfs4_session.h")
>
> v4.4.220: Failed to apply! Possible dependencies:
> 172d9de15a0d ("NFS: Change nfs4_get_session() to take an nfs_client structure")
> 3453d5708b33 ("NFSv4.1: Avoid false retries when RPC calls are interrupted")
> 3be0f80b5fe9 ("NFSv4.1: Fix up replays of interrupted requests")
> 42e1cca7e91e ("NFS: Change nfs4_setup_sequence() to take an nfs_client structure")
> 5c441544f045 ("NFSv4.x: Handle bad/dead sessions correctly in nfs41_sequence_process()")
> 5f83d86cf531 ("NFSv4.x: Fix wraparound issues when validing the callback sequence id")
> 68d264cf02b0 ("NFS42: handle layoutstats stateid error")
> 6de7e12f53a1 ("NFS: Use nfs4_setup_sequence() everywhere")
> 80f9642724af ("NFSv4.x: Enforce the ca_maxresponsesize_cached on the back channel")
> 810d82e68301 ("NFSv4.x: Allow multiple callbacks in flight")
> 9a0fe86745b8 ("pNFS: Handle NFS4ERR_OLD_STATEID correctly in LAYOUTSTAT calls")
> efc6f4aa742d ("NFS: Move nfs4_get_session() into nfs4_session.h")
> f74a834a0e1b ("NFSv4.x: CB_SEQUENCE should return NFS4ERR_DELAY if still executing")
>
>
> NOTE: The patch will not be queued to stable trees until it is upstream.
>
> How should we proceed with this patch?

Trond,

This is my first time trying to mark the patch as something that
should be back ported. What's the right approach?

I couldn't find a patch that would make sense to say that this patch
"fixes". Should I just pick one of them (maybe "SUNRPC: Allow creation
of RPC clients with multiple connections" (612b41f808a))?

I should have said that this only needs to be fixed up to when the
feature was included which was 5.3. The fact that I doesn't apply to
5.4 is the only problem I see needs to be looked at / addressed.


>
> --
> Thanks
> Sasha

2020-04-26 18:18:08

by Sasha Levin

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] NFSv4.1: fix handling of backchannel binding in BIND_CONN_TO_SESSION

On Sun, Apr 26, 2020 at 11:25:25AM -0400, Olga Kornievskaia wrote:
>On Sun, Apr 26, 2020 at 11:03 AM Sasha Levin <[email protected]> wrote:
>>
>> Hi
>>
>> [This is an automated email]
>>
>> This commit has been processed because it contains a -stable tag.
>> The stable tag indicates that it's relevant for the following trees: all
>>
>> The bot has tested the following trees: v5.6.7, v5.4.35, v4.19.118, v4.14.177, v4.9.220, v4.4.220.
>>
>> v5.6.7: Build OK!
>> v5.4.35: Failed to apply! Possible dependencies:
>> Unable to calculate
>>
>> v4.19.118: Failed to apply! Possible dependencies:
>> 07d02a67b7fa ("SUNRPC: Simplify lookup code")
>> 3453d5708b33 ("NFSv4.1: Avoid false retries when RPC calls are interrupted")
>> 5c441544f045 ("NFSv4.x: Handle bad/dead sessions correctly in nfs41_sequence_process()")
>> 79b181810285 ("SUNRPC: Convert auth creds to use refcount_t")
>> 8276c902bbe9 ("SUNRPC: remove uid and gid from struct auth_cred")
>> 95cd623250ad ("SUNRPC: Clean up the AUTH cache code")
>> 97f68c6b02e0 ("SUNRPC: add 'struct cred *' to auth_cred and rpc_cred")
>> a52458b48af1 ("NFS/NFSD/SUNRPC: replace generic creds with 'struct cred'.")
>> fc0664fd9bcc ("SUNRPC: remove groupinfo from struct auth_cred.")
>>
>> v4.14.177: Failed to apply! Possible dependencies:
>> 07d02a67b7fa ("SUNRPC: Simplify lookup code")
>> 12f275cdd163 ("NFSv4: Retry CLOSE and DELEGRETURN on NFS4ERR_OLD_STATEID.")
>> 1eb5d98f16f6 ("nfs: convert to new i_version API")
>> 3453d5708b33 ("NFSv4.1: Avoid false retries when RPC calls are interrupted")
>> 35156bfff3c0 ("NFSv4: Fix the nfs_inode_set_delegation() arguments")
>> 5c441544f045 ("NFSv4.x: Handle bad/dead sessions correctly in nfs41_sequence_process()")
>> 79b181810285 ("SUNRPC: Convert auth creds to use refcount_t")
>> 95cd623250ad ("SUNRPC: Clean up the AUTH cache code")
>> 97f68c6b02e0 ("SUNRPC: add 'struct cred *' to auth_cred and rpc_cred")
>> a52458b48af1 ("NFS/NFSD/SUNRPC: replace generic creds with 'struct cred'.")
>> b3dce6a2f060 ("pnfs/blocklayout: handle transient devices")
>> fc0664fd9bcc ("SUNRPC: remove groupinfo from struct auth_cred.")
>>
>> v4.9.220: Failed to apply! Possible dependencies:
>> 172d9de15a0d ("NFS: Change nfs4_get_session() to take an nfs_client structure")
>> 3453d5708b33 ("NFSv4.1: Avoid false retries when RPC calls are interrupted")
>> 3be0f80b5fe9 ("NFSv4.1: Fix up replays of interrupted requests")
>> 42e1cca7e91e ("NFS: Change nfs4_setup_sequence() to take an nfs_client structure")
>> 5c441544f045 ("NFSv4.x: Handle bad/dead sessions correctly in nfs41_sequence_process()")
>> 6de7e12f53a1 ("NFS: Use nfs4_setup_sequence() everywhere")
>> 7981c8a65914 ("NFS: Create a single nfs4_setup_sequence() function")
>> efc6f4aa742d ("NFS: Move nfs4_get_session() into nfs4_session.h")
>>
>> v4.4.220: Failed to apply! Possible dependencies:
>> 172d9de15a0d ("NFS: Change nfs4_get_session() to take an nfs_client structure")
>> 3453d5708b33 ("NFSv4.1: Avoid false retries when RPC calls are interrupted")
>> 3be0f80b5fe9 ("NFSv4.1: Fix up replays of interrupted requests")
>> 42e1cca7e91e ("NFS: Change nfs4_setup_sequence() to take an nfs_client structure")
>> 5c441544f045 ("NFSv4.x: Handle bad/dead sessions correctly in nfs41_sequence_process()")
>> 5f83d86cf531 ("NFSv4.x: Fix wraparound issues when validing the callback sequence id")
>> 68d264cf02b0 ("NFS42: handle layoutstats stateid error")
>> 6de7e12f53a1 ("NFS: Use nfs4_setup_sequence() everywhere")
>> 80f9642724af ("NFSv4.x: Enforce the ca_maxresponsesize_cached on the back channel")
>> 810d82e68301 ("NFSv4.x: Allow multiple callbacks in flight")
>> 9a0fe86745b8 ("pNFS: Handle NFS4ERR_OLD_STATEID correctly in LAYOUTSTAT calls")
>> efc6f4aa742d ("NFS: Move nfs4_get_session() into nfs4_session.h")
>> f74a834a0e1b ("NFSv4.x: CB_SEQUENCE should return NFS4ERR_DELAY if still executing")
>>
>>
>> NOTE: The patch will not be queued to stable trees until it is upstream.
>>
>> How should we proceed with this patch?
>
>Trond,
>
>This is my first time trying to mark the patch as something that
>should be back ported. What's the right approach?
>
>I couldn't find a patch that would make sense to say that this patch
>"fixes". Should I just pick one of them (maybe "SUNRPC: Allow creation
>of RPC clients with multiple connections" (612b41f808a))?
>
>I should have said that this only needs to be fixed up to when the
>feature was included which was 5.3. The fact that I doesn't apply to
>5.4 is the only problem I see needs to be looked at / addressed.

Hi Olga,

You can add annotate the stable tag with versions of the branches you
want it to be included in. For example here you could do:

CC: [email protected] # 5.3+

--
Thanks,
Sasha