diff mbox series

[v2,1/1] NFSv4.1: fix handling of backchannel binding in BIND_CONN_TO_SESSION

Message ID 20200424214550.30462-1-olga.kornievskaia@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/1] NFSv4.1: fix handling of backchannel binding in BIND_CONN_TO_SESSION | expand

Commit Message

Olga Kornievskaia April 24, 2020, 9:45 p.m. UTC 
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@vger.kernel.org
Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs4proc.c           | 8 ++++++++
 include/linux/nfs_xdr.h     | 2 ++
 include/linux/sunrpc/clnt.h | 5 +++++
 3 files changed, 15 insertions(+)

Comments

Sasha Levin April 26, 2020, 3:03 p.m. UTC | #1 
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?
Olga Kornievskaia April 26, 2020, 3:25 p.m. UTC | #2 
On Sun, Apr 26, 2020 at 11:03 AM Sasha Levin <sashal@kernel.org> 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
Sasha Levin April 26, 2020, 6:17 p.m. UTC | #3 
On Sun, Apr 26, 2020 at 11:25:25AM -0400, Olga Kornievskaia wrote:
>On Sun, Apr 26, 2020 at 11:03 AM Sasha Levin <sashal@kernel.org> 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: stable@kernel.org # 5.3+
diff mbox series

Patch

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 */