Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <10c0ad38-d28c-1916-9dcc-6f3fd5e19ab8@huawei.com>
Date:   Mon, 9 May 2022 18:28:16 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [PATCH v2 3/4] SUNRPC: Ensure gss-proxy connects on setup
To:     <trondmy@kernel.org>, "J. Bruce Fields" <bfields@fieldses.org>
CC:     Chuck Lever <chuck.lever@oracle.com>, <linux-nfs@vger.kernel.org>
References: <20220429173629.621418-1-trondmy@kernel.org>
 <20220429173629.621418-2-trondmy@kernel.org>
 <20220429173629.621418-3-trondmy@kernel.org>
From:   "wanghai (M)" <wanghai38@huawei.com>
In-Reply-To: <20220429173629.621418-3-trondmy@kernel.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


在 2022/4/30 1:36, trondmy@kernel.org 写道:
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>
> 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: stable@vger.kernel.org
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
>   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