2022-04-29 20:30:19

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v2 3/4] SUNRPC: Ensure gss-proxy connects on setup

From: Trond Myklebust <[email protected]>

For reasons best known to the author, gss-proxy does not implement a
NULL procedure, and returns RPC_PROC_UNAVAIL. However we still want to
ensure that we connect to the service at setup time.
So add a quirk-flag specially for this case.

Fixes: 1d658336b05f ("SUNRPC: Add RPC based upcall mechanism for RPCGSS auth")
Cc: [email protected]
Signed-off-by: Trond Myklebust <[email protected]>
---
include/linux/sunrpc/clnt.h | 1 +
net/sunrpc/auth_gss/gss_rpc_upcall.c | 2 +-
net/sunrpc/clnt.c | 3 +++
3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
index 267b7aeaf1a6..db5149567305 100644
--- a/include/linux/sunrpc/clnt.h
+++ b/include/linux/sunrpc/clnt.h
@@ -160,6 +160,7 @@ struct rpc_add_xprt_test {
#define RPC_CLNT_CREATE_NO_RETRANS_TIMEOUT (1UL << 9)
#define RPC_CLNT_CREATE_SOFTERR (1UL << 10)
#define RPC_CLNT_CREATE_REUSEPORT (1UL << 11)
+#define RPC_CLNT_CREATE_IGNORE_NULL_UNAVAIL (1UL << 12)

struct rpc_clnt *rpc_create(struct rpc_create_args *args);
struct rpc_clnt *rpc_bind_new_program(struct rpc_clnt *,
diff --git a/net/sunrpc/auth_gss/gss_rpc_upcall.c b/net/sunrpc/auth_gss/gss_rpc_upcall.c
index 61c276bddaf2..8ca1d809b78d 100644
--- a/net/sunrpc/auth_gss/gss_rpc_upcall.c
+++ b/net/sunrpc/auth_gss/gss_rpc_upcall.c
@@ -97,7 +97,7 @@ static int gssp_rpc_create(struct net *net, struct rpc_clnt **_clnt)
* timeout, which would result in reconnections being
* done without the correct namespace:
*/
- .flags = RPC_CLNT_CREATE_NOPING |
+ .flags = RPC_CLNT_CREATE_IGNORE_NULL_UNAVAIL |
RPC_CLNT_CREATE_NO_IDLE_TIMEOUT
};
struct rpc_clnt *clnt;
diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 98133aa54f19..22c28cf43eba 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -479,6 +479,9 @@ static struct rpc_clnt *rpc_create_xprt(struct rpc_create_args *args,

if (!(args->flags & RPC_CLNT_CREATE_NOPING)) {
int err = rpc_ping(clnt);
+ if ((args->flags & RPC_CLNT_CREATE_IGNORE_NULL_UNAVAIL) &&
+ err == -EOPNOTSUPP)
+ err = 0;
if (err != 0) {
rpc_shutdown_client(clnt);
return ERR_PTR(err);
--
2.35.1


2022-04-30 09:16:28

by Trond Myklebust

[permalink] [raw]
Subject: [PATCH v2 4/4] Revert "SUNRPC: attempt AF_LOCAL connect on setup"

From: Trond Myklebust <[email protected]>

This reverts commit 7073ea8799a8cf73db60270986f14e4aae20fa80.

We must not try to connect the socket while the transport is under
construction, because the mechanisms to safely tear it down are not in
place.

Cc: [email protected]
Signed-off-by: Trond Myklebust <[email protected]>
---
net/sunrpc/xprtsock.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 25b8a8ead56b..650102a9c86a 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -2875,9 +2875,6 @@ static struct rpc_xprt *xs_setup_local(struct xprt_create *args)
}
xprt_set_bound(xprt);
xs_format_peer_addresses(xprt, "local", RPCBIND_NETID_LOCAL);
- ret = ERR_PTR(xs_local_setup_socket(transport));
- if (ret)
- goto out_err;
break;
default:
ret = ERR_PTR(-EAFNOSUPPORT);
--
2.35.1

2022-05-01 18:31:56

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] Revert "SUNRPC: attempt AF_LOCAL connect on setup"

On Fri, 2022-04-29 at 13:36 -0400, [email protected] wrote:
> From: Trond Myklebust <[email protected]>
>
> This reverts commit 7073ea8799a8cf73db60270986f14e4aae20fa80.
>
> We must not try to connect the socket while the transport is under
> construction, because the mechanisms to safely tear it down are not
> in
> place.
>
> Cc: [email protected]
> Signed-off-by: Trond Myklebust <[email protected]>

Sorry. I intended to add a

"Reported-by: wanghai (M) <[email protected]>"

That has been added to the version in my "testing" branch.


--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]


2022-05-05 23:56:03

by Wang Hai

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] Revert "SUNRPC: attempt AF_LOCAL connect on setup"


在 2022/4/30 8:56, Trond Myklebust 写道:
> On Fri, 2022-04-29 at 13:36 -0400, [email protected] wrote:
>> From: Trond Myklebust <[email protected]>
>>
>> This reverts commit 7073ea8799a8cf73db60270986f14e4aae20fa80.
>>
>> We must not try to connect the socket while the transport is under
>> construction, because the mechanisms to safely tear it down are not
>> in
>> place.
>>
>> Cc: [email protected]
>> Signed-off-by: Trond Myklebust <[email protected]>
> Sorry. I intended to add a
>
> "Reported-by: wanghai (M) <[email protected]>"
>
> That has been added to the version in my "testing" branch.
>
Thanks for your help, I tested it carefully and this patchset can solve
my problem.

By the way, when can this patchset be applied to linux mainline?

--
Wang Hai


2022-05-06 13:21:29

by Wang Hai

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] Revert "SUNRPC: attempt AF_LOCAL connect on setup"


在 2022/4/30 8:56, Trond Myklebust 写道:
> On Fri, 2022-04-29 at 13:36 -0400, [email protected] wrote:
>> From: Trond Myklebust <[email protected]>
>>
>> This reverts commit 7073ea8799a8cf73db60270986f14e4aae20fa80.
>>
>> We must not try to connect the socket while the transport is under
>> construction, because the mechanisms to safely tear it down are not
>> in
>> place.
>>
>> Cc: [email protected]
>> Signed-off-by: Trond Myklebust <[email protected]>
> Sorry. I intended to add a
>
> "Reported-by: wanghai (M) <[email protected]>"
>
> That has been added to the version in my "testing" branch.
>
Hi, Trond.

If it is just to fix my problem, is it enough to apply only
patch3 and patch4? I tested that if only patch3 and patch4
are applied, the problem seems to be fixed.

--
Wang Hai


2022-05-09 11:38:24

by Wang Hai

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] SUNRPC: Ensure gss-proxy connects on setup


在 2022/4/30 1:36, [email protected] 写道:
> From: Trond Myklebust <[email protected]>
>
> For reasons best known to the author, gss-proxy does not implement a
> NULL procedure, and returns RPC_PROC_UNAVAIL. However we still want to
> ensure that we connect to the service at setup time.
> So add a quirk-flag specially for this case.
>
> Fixes: 1d658336b05f ("SUNRPC: Add RPC based upcall mechanism for RPCGSS auth")
> Cc: [email protected]
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> include/linux/sunrpc/clnt.h | 1 +
> net/sunrpc/auth_gss/gss_rpc_upcall.c | 2 +-
> net/sunrpc/clnt.c | 3 +++
> 3 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/sunrpc/clnt.h b/include/linux/sunrpc/clnt.h
> index 267b7aeaf1a6..db5149567305 100644
> --- a/include/linux/sunrpc/clnt.h
> +++ b/include/linux/sunrpc/clnt.h
> @@ -160,6 +160,7 @@ struct rpc_add_xprt_test {
> #define RPC_CLNT_CREATE_NO_RETRANS_TIMEOUT (1UL << 9)
> #define RPC_CLNT_CREATE_SOFTERR (1UL << 10)
> #define RPC_CLNT_CREATE_REUSEPORT (1UL << 11)
> +#define RPC_CLNT_CREATE_IGNORE_NULL_UNAVAIL (1UL << 12)
>
> struct rpc_clnt *rpc_create(struct rpc_create_args *args);
> struct rpc_clnt *rpc_bind_new_program(struct rpc_clnt *,
> diff --git a/net/sunrpc/auth_gss/gss_rpc_upcall.c b/net/sunrpc/auth_gss/gss_rpc_upcall.c
> index 61c276bddaf2..8ca1d809b78d 100644
> --- a/net/sunrpc/auth_gss/gss_rpc_upcall.c
> +++ b/net/sunrpc/auth_gss/gss_rpc_upcall.c
> @@ -97,7 +97,7 @@ static int gssp_rpc_create(struct net *net, struct rpc_clnt **_clnt)
> * timeout, which would result in reconnections being
> * done without the correct namespace:
> */
> - .flags = RPC_CLNT_CREATE_NOPING |
> + .flags = RPC_CLNT_CREATE_IGNORE_NULL_UNAVAIL |
> RPC_CLNT_CREATE_NO_IDLE_TIMEOUT
> };
> struct rpc_clnt *clnt;
> diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
> index 98133aa54f19..22c28cf43eba 100644
> --- a/net/sunrpc/clnt.c
> +++ b/net/sunrpc/clnt.c
> @@ -479,6 +479,9 @@ static struct rpc_clnt *rpc_create_xprt(struct rpc_create_args *args,
>
> if (!(args->flags & RPC_CLNT_CREATE_NOPING)) {
> int err = rpc_ping(clnt);
> + if ((args->flags & RPC_CLNT_CREATE_IGNORE_NULL_UNAVAIL) &&
> + err == -EOPNOTSUPP)
> + err = 0;

Hi, Trond and Bruce

After I apply this patch, gssproxy.service does not work properly.


The following is the abnormal working log
[root@localhost ~]# systemctl restart gssproxy.service
[root@localhost ~]# cat /proc/net/rpc/use-gss-proxy
-1
[root@localhost ~]#

On failure, rpc_ping() returns -EIO. should the following change be made
here

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 22c28cf43eba..314b6fd60e2f 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -480,7 +480,7 @@ static struct rpc_clnt *rpc_create_xprt(struct
rpc_create_args *args,
        if (!(args->flags & RPC_CLNT_CREATE_NOPING)) {
                int err = rpc_ping(clnt);
                if ((args->flags & RPC_CLNT_CREATE_IGNORE_NULL_UNAVAIL) &&
-                   err == -EOPNOTSUPP)
+                   (err == -EOPNOTSUPP || err == -EIO))
                        err = 0;
                if (err != 0) {
                        rpc_shutdown_client(clnt);

> if (err != 0) {
> rpc_shutdown_client(clnt);
> return ERR_PTR(err);

--
Wang Hai