From:   Chuck Lever III <chuck.lever@oracle.com>
To:     "trondmy@kernel.org" <trondmy@kernel.org>
CC:     Bruce Fields <bfields@redhat.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v2 10/10] nfsd: Ignore rpcbind errors on nfsd startup
Thread-Topic: [PATCH v2 10/10] nfsd: Ignore rpcbind errors on nfsd startup
Thread-Index: AQHX9HoJf0UrhpbILEawWnDLCGoAp6w6H74A
Date:   Sun, 19 Dec 2021 18:15:07 +0000
Message-ID: <831659F6-3005-459B-92ED-933BBCEE6FE9@oracle.com>
References: <20211219013803.324724-1-trondmy@kernel.org>
 <20211219013803.324724-2-trondmy@kernel.org>
 <20211219013803.324724-3-trondmy@kernel.org>
 <20211219013803.324724-4-trondmy@kernel.org>
 <20211219013803.324724-5-trondmy@kernel.org>
 <20211219013803.324724-6-trondmy@kernel.org>
 <20211219013803.324724-7-trondmy@kernel.org>
 <20211219013803.324724-8-trondmy@kernel.org>
 <20211219013803.324724-9-trondmy@kernel.org>
 <20211219013803.324724-10-trondmy@kernel.org>
 <20211219013803.324724-11-trondmy@kernel.org>
In-Reply-To: <20211219013803.324724-11-trondmy@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?rTPn3jAnKxF4+Ry3Yu5y5cBCpjuu3iv/8TpG1ys0WWMFIduaXU4KN6Rznm0z?=
 =?us-ascii?Q?mmwpd63qD1f9eP6NhYiGHXcijrq4xce7XBbXGLMWt75LbO5kJjVOoZRnSUP5?=
 =?us-ascii?Q?aPJPKxnk6vdaYL5GDducejxzzSnZm6OT24wrbpQWUksT5Bx2svByySaaBegG?=
 =?us-ascii?Q?Zv/SQnzEx+Qmd39vsJoN3VW2CWjOLChl7rDee/xpjBkjJR5ddUTHDtbKuaG9?=
 =?us-ascii?Q?6h5J7CBOwXr0fP+0ClZbGhZnopA8uHgHI6LuYZ6h2Ani/5zC+EEx0LVUqr+k?=
 =?us-ascii?Q?+5yZjTfLrSnFzrd30oQIeiQGhXUgqw/kZR6eqXNhE/IZKX4q3KXDHI5e9yGs?=
 =?us-ascii?Q?Wma4kLkwX4QMqvIlWKlCoz9fP2cliDMOcAS3wFEosgDtNk1ZBF8y6x3xiBtZ?=
 =?us-ascii?Q?XRJqpFtm6PYstwKwt2X5RD027Ez0l1NIR9KYpnGATxi7SuADj2cSDEa8e+tP?=
 =?us-ascii?Q?G8sxLbg6uc3IBn5pzywZLuyxYUN5o1RO2ahMjmkgosxEjdC7HG07SQdH8yt3?=
 =?us-ascii?Q?itAP+prDBI7GhvLnP/nqD2F0ByhvLV/rpY6mz5HhFHFjwB4mntkMXdZJvt0D?=
 =?us-ascii?Q?9Niu8vLFCVtzBTyzvgNJvfy8/K2YaMJBhrWLOqoaolPQU9TQOacvhMSYgfu2?=
 =?us-ascii?Q?buypAh7RSAOH31PNZfU/2usTkkdV0z/2A1G/J2CRr+efFK30hhAYLOPYZxAJ?=
 =?us-ascii?Q?iEXACeMtZ5AOsg0O8nDSoC9j18FnCwvGSmI7IVuX4JSjncthQXb21EKfeZhn?=
 =?us-ascii?Q?ryO3+iMnX55EtQesEmw3kIkIQJsgFumVm3iDeU5ZH3ehF9vCvCw4vl+/bTrM?=
 =?us-ascii?Q?6XuFOn/WkLJMln39VXy6+VbGeTEoj/mcfPptTEP5EPegx+11Jo3WWAEEDXOy?=
 =?us-ascii?Q?VJhYE07w/xdLfYJ/TLIhBEWelAccHYlRjCBxv1au6uCpraOMM1oRClMRGqZa?=
 =?us-ascii?Q?em7/tlYyJvmaITB+vyoZBuVkbHFlagJ1TZy9LyKrVjjg+bpfqAAfw3v5V202?=
 =?us-ascii?Q?eT+ZAskxVOlx8zneEMrhAib9rrYwh29Boa6JhQqPmkQPVKriUdN4P8iIMNf6?=
 =?us-ascii?Q?EaWDlRMooF7C4QNSt1ZZLBTYb+KxQPfMelSl3D2q5dEQlBH4vHplNJmR5PKt?=
 =?us-ascii?Q?zULjsKzsCXLv90U5Y92ipQQ1CpK+lBbh1WAt3U7R3S212Z58lHqlaawVqAKv?=
 =?us-ascii?Q?lUBhQ1iSPj7IUIZZFAheStceolRVrqbOq2oGiE3rl2lKRAJ+nZQvLvmH/1L1?=
 =?us-ascii?Q?03xTDp8BxLZE1NZK2V5uQ3BlQdk5qWrgqf011J4Sfcw2AkzstAd0aJQ5DP7y?=
 =?us-ascii?Q?brb6Y8dT4CiXE2hCmG32LSXU3t/Gc/boMJ2eXRDa7rvqhuSsjI1inX9cuWSm?=
 =?us-ascii?Q?ymKGb5Jix1bOHtK1JTt4PoO/feMn2SbTrUlgBREsJWyl7B/7PIFASlfZVs4A?=
 =?us-ascii?Q?lBqC1598IMrTkImjBbNfF7BLDzMLABW+nM4isP0Qhy147cRCLoUtLkFc8K18?=
 =?us-ascii?Q?SlCB0lC/b5IOMVwusLEaU/qCvP3EBzcqbI0Pe7QZF8kiDEyMDHeNTZWqjdoc?=
 =?us-ascii?Q?KTTsa4cEpahdg4ZTyv+oi1i58uQwFwSld3AcqAW1O0rdHwX4s1BNnyVps9MK?=
 =?us-ascii?Q?lAHVICYUZlkGgNmZ/tGuCHQ=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <C73087796AEA3E4F8AEDB6868ABF16ED@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CH0PR10MB4858.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 863b112d-6a0b-47f0-ff56-08d9c31b7cbd
X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Dec 2021 18:15:07.3058
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: oWNsdYuQyKYolwC3DnbU1NslvulH4PY/TTMNBdazlehnXhJBAkk1AflhmTC7NNDpfbkVi1/tgIbb+sY73a1IOQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR10MB3784
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10203 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 malwarescore=0
 spamscore=0 phishscore=0 mlxscore=0 bulkscore=0 adultscore=0
 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2110150000 definitions=main-2112190114
X-Proofpoint-GUID: -BcKmiIgBLi5biAWIimiV_cTXVlD0Ob8
X-Proofpoint-ORIG-GUID: -BcKmiIgBLi5biAWIimiV_cTXVlD0Ob8
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Dec 18, 2021, at 8:38 PM, trondmy@kernel.org wrote:
>=20
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>=20
> NFSv4 doesn't need rpcbind, so let's not refuse to start up just because
> the rpcbind registration failed.

Commit 7e55b59b2f32 ("SUNRPC/NFSD: Support a new option for ignoring
the result of svc_register") added vs_rpcb_optnl, which is already
set for nfsd4_version4. Is that not adequate?


> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
> fs/nfsd/nfsctl.c               |  7 ++++++-
> fs/nfsd/nfsd.h                 |  1 +
> fs/nfsd/nfssvc.c               | 18 ++++++++++++++++--
> include/linux/sunrpc/svcsock.h |  5 +++--
> net/sunrpc/svcsock.c           | 14 ++++++++------
> 5 files changed, 34 insertions(+), 11 deletions(-)
>=20
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 51a49e0cfe37..da9760479acd 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -727,6 +727,7 @@ static ssize_t __write_ports_addfd(char *buf, struct =
net *net, const struct cred
> 	char *mesg =3D buf;
> 	int fd, err;
> 	struct nfsd_net *nn =3D net_generic(net, nfsd_net_id);
> +	int flags =3D SVC_SOCK_DEFAULTS;
>=20
> 	err =3D get_int(&mesg, &fd);
> 	if (err !=3D 0 || fd < 0)
> @@ -741,7 +742,11 @@ static ssize_t __write_ports_addfd(char *buf, struct=
 net *net, const struct cred
> 	if (err !=3D 0)
> 		return err;
>=20
> -	err =3D svc_addsock(nn->nfsd_serv, fd, buf, SIMPLE_TRANSACTION_LIMIT, c=
red);
> +	if (!nfsd_rpcbind_error_is_fatal())
> +		flags |=3D SVC_SOCK_RPCBIND_NOERR;
> +
> +	err =3D svc_addsock(nn->nfsd_serv, fd, buf, SIMPLE_TRANSACTION_LIMIT,
> +			  flags, cred);
> 	if (err < 0) {
> 		nfsd_destroy(net);
> 		return err;
> diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
> index 498e5a489826..e0356d3ecf65 100644
> --- a/fs/nfsd/nfsd.h
> +++ b/fs/nfsd/nfsd.h
> @@ -134,6 +134,7 @@ int nfsd_vers(struct nfsd_net *nn, int vers, enum ver=
s_op change);
> int nfsd_minorversion(struct nfsd_net *nn, u32 minorversion, enum vers_op=
 change);
> void nfsd_reset_versions(struct nfsd_net *nn);
> int nfsd_create_serv(struct net *net);
> +extern bool nfsd_rpcbind_error_is_fatal(void);
>=20
> extern int nfsd_max_blksize;
>=20
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index 6815c70b06af..6f22c72f340d 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -289,17 +289,21 @@ static int nfsd_init_socks(struct net *net, const s=
truct cred *cred)
> {
> 	int error;
> 	struct nfsd_net *nn =3D net_generic(net, nfsd_net_id);
> +	int flags =3D SVC_SOCK_DEFAULTS;
>=20
> 	if (!list_empty(&nn->nfsd_serv->sv_permsocks))
> 		return 0;
>=20
> +	if (!nfsd_rpcbind_error_is_fatal())
> +		flags |=3D SVC_SOCK_RPCBIND_NOERR;
> +
> 	error =3D svc_create_xprt(nn->nfsd_serv, "udp", net, PF_INET, NFS_PORT,
> -					SVC_SOCK_DEFAULTS, cred);
> +				flags, cred);
> 	if (error < 0)
> 		return error;
>=20
> 	error =3D svc_create_xprt(nn->nfsd_serv, "tcp", net, PF_INET, NFS_PORT,
> -					SVC_SOCK_DEFAULTS, cred);
> +				flags, cred);
> 	if (error < 0)
> 		return error;
>=20
> @@ -340,6 +344,16 @@ static void nfsd_shutdown_generic(void)
> 	nfsd_file_cache_shutdown();
> }
>=20
> +static bool nfsd_rpcbind_error_fatal =3D false;
> +module_param(nfsd_rpcbind_error_fatal, bool, 0644);
> +MODULE_PARM_DESC(nfsd_rpcbind_error_fatal,
> +		 "rpcbind errors are fatal when starting nfsd.");
> +
> +bool nfsd_rpcbind_error_is_fatal(void)
> +{
> +	return nfsd_rpcbind_error_fatal;
> +}
> +
> /*
>  * Allow admin to disable lockd. This would typically be used to allow (e=
.g.)
>  * a userspace NLM server of some sort to be used.
> diff --git a/include/linux/sunrpc/svcsock.h b/include/linux/sunrpc/svcsoc=
k.h
> index bcc555c7ae9c..f34c222cee9d 100644
> --- a/include/linux/sunrpc/svcsock.h
> +++ b/include/linux/sunrpc/svcsock.h
> @@ -61,8 +61,8 @@ void		svc_drop(struct svc_rqst *);
> void		svc_sock_update_bufs(struct svc_serv *serv);
> bool		svc_alien_sock(struct net *net, int fd);
> int		svc_addsock(struct svc_serv *serv, const int fd,
> -					char *name_return, const size_t len,
> -					const struct cred *cred);
> +			    char *name_return, const size_t len, int flags,
> +			    const struct cred *cred);
> void		svc_init_xprt_sock(void);
> void		svc_cleanup_xprt_sock(void);
> struct svc_xprt *svc_sock_create(struct svc_serv *serv, int prot);
> @@ -74,5 +74,6 @@ void		svc_sock_destroy(struct svc_xprt *);
> #define SVC_SOCK_DEFAULTS	(0U)
> #define SVC_SOCK_ANONYMOUS	(1U << 0)	/* don't register with pmap */
> #define SVC_SOCK_TEMPORARY	(1U << 1)	/* flag socket as temporary */
> +#define SVC_SOCK_RPCBIND_NOERR	(1U << 2)	/* Ignore pmap errors */
>=20
> #endif /* SUNRPC_SVCSOCK_H */
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index 478f857cdaed..7f5b12a50bf9 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -1309,14 +1309,15 @@ static struct svc_sock *svc_setup_socket(struct s=
vc_serv *serv,
> 	inet =3D sock->sk;
>=20
> 	/* Register socket with portmapper */
> -	if (pmap_register)
> +	if (pmap_register) {
> 		err =3D svc_register(serv, sock_net(sock->sk), inet->sk_family,
> 				     inet->sk_protocol,
> 				     ntohs(inet_sk(inet)->inet_sport));
>=20
> -	if (err < 0) {
> -		kfree(svsk);
> -		return ERR_PTR(err);
> +		if (err < 0 && !(flags & SVC_SOCK_RPCBIND_NOERR)) {
> +			kfree(svsk);
> +			return ERR_PTR(err);
> +		}
> 	}
>=20
> 	svsk->sk_sock =3D sock;
> @@ -1364,6 +1365,7 @@ EXPORT_SYMBOL_GPL(svc_alien_sock);
>  * @fd: file descriptor of the new listener
>  * @name_return: pointer to buffer to fill in with name of listener
>  * @len: size of the buffer
> + * @flags: flags argument for svc_setup_socket()
>  * @cred: credential
>  *
>  * Fills in socket name and returns positive length of name if successful=
.
> @@ -1371,7 +1373,7 @@ EXPORT_SYMBOL_GPL(svc_alien_sock);
>  * value.
>  */
> int svc_addsock(struct svc_serv *serv, const int fd, char *name_return,
> -		const size_t len, const struct cred *cred)
> +		const size_t len, int flags, const struct cred *cred)
> {
> 	int err =3D 0;
> 	struct socket *so =3D sockfd_lookup(fd, &err);
> @@ -1395,7 +1397,7 @@ int svc_addsock(struct svc_serv *serv, const int fd=
, char *name_return,
> 	err =3D -ENOENT;
> 	if (!try_module_get(THIS_MODULE))
> 		goto out;
> -	svsk =3D svc_setup_socket(serv, so, SVC_SOCK_DEFAULTS);
> +	svsk =3D svc_setup_socket(serv, so, flags);
> 	if (IS_ERR(svsk)) {
> 		module_put(THIS_MODULE);
> 		err =3D PTR_ERR(svsk);
> --=20
> 2.33.1
>=20

--
Chuck Lever