* [PATCH 1/1] NFSD: don't start nfsd if sv_permsocks is empty
@ 2025-10-30 16:35 Olga Kornievskaia
2025-10-30 17:53 ` Jeff Layton
2025-10-31 12:51 ` Chuck Lever
0 siblings, 2 replies; 9+ messages in thread
From: Olga Kornievskaia @ 2025-10-30 16:35 UTC (permalink / raw)
To: chuck.lever, jlayton; +Cc: linux-nfs, neilb, Dai.Ngo, tom
Previously, while trying to create a server instance, if no
listening sockets were present then default parameter udp
and tcp listeners were created. It's unclear what purpose
was of starting these listeners were and how this could have
been triggered by the userland setup. This patch proposed
to ensure the reverse that we never end in a situation where
no listener sockets are created and we are trying to create
nfsd threads.
The problem it solves is: when nfs.conf only has tcp=n (and
nothing else for the choice of transports), nfsdctl would
still start the server and create udp and tcp listeners.
Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
---
fs/nfsd/nfssvc.c | 28 +++++-----------------------
1 file changed, 5 insertions(+), 23 deletions(-)
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index 7057ddd7a0a8..40592b61b04b 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -249,27 +249,6 @@ int nfsd_nrthreads(struct net *net)
return rv;
}
-static int nfsd_init_socks(struct net *net, const struct cred *cred)
-{
- int error;
- struct nfsd_net *nn = net_generic(net, nfsd_net_id);
-
- if (!list_empty(&nn->nfsd_serv->sv_permsocks))
- return 0;
-
- error = svc_xprt_create(nn->nfsd_serv, "udp", net, PF_INET, NFS_PORT,
- SVC_SOCK_DEFAULTS, cred);
- if (error < 0)
- return error;
-
- error = svc_xprt_create(nn->nfsd_serv, "tcp", net, PF_INET, NFS_PORT,
- SVC_SOCK_DEFAULTS, cred);
- if (error < 0)
- return error;
-
- return 0;
-}
-
static int nfsd_users = 0;
static int nfsd_startup_generic(void)
@@ -377,9 +356,12 @@ static int nfsd_startup_net(struct net *net, const struct cred *cred)
ret = nfsd_startup_generic();
if (ret)
return ret;
- ret = nfsd_init_socks(net, cred);
- if (ret)
+
+ if (list_empty(&nn->nfsd_serv->sv_permsocks)) {
+ pr_warn("NFSD: not starting because no listening sockets found\n");
+ ret = -EIO;
goto out_socks;
+ }
if (nfsd_needs_lockd(nn) && !nn->lockd_up) {
ret = lockd_up(net, cred);
--
2.47.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] NFSD: don't start nfsd if sv_permsocks is empty
2025-10-30 16:35 [PATCH 1/1] NFSD: don't start nfsd if sv_permsocks is empty Olga Kornievskaia
@ 2025-10-30 17:53 ` Jeff Layton
2025-10-30 23:05 ` NeilBrown
2025-10-31 17:26 ` Olga Kornievskaia
2025-10-31 12:51 ` Chuck Lever
1 sibling, 2 replies; 9+ messages in thread
From: Jeff Layton @ 2025-10-30 17:53 UTC (permalink / raw)
To: Olga Kornievskaia, chuck.lever; +Cc: linux-nfs, neilb, Dai.Ngo, tom
On Thu, 2025-10-30 at 12:35 -0400, Olga Kornievskaia wrote:
> Previously, while trying to create a server instance, if no
> listening sockets were present then default parameter udp
> and tcp listeners were created. It's unclear what purpose
> was of starting these listeners were and how this could have
> been triggered by the userland setup. This patch proposed
> to ensure the reverse that we never end in a situation where
> no listener sockets are created and we are trying to create
> nfsd threads.
>
> The problem it solves is: when nfs.conf only has tcp=n (and
> nothing else for the choice of transports), nfsdctl would
> still start the server and create udp and tcp listeners.
>
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> ---
> fs/nfsd/nfssvc.c | 28 +++++-----------------------
> 1 file changed, 5 insertions(+), 23 deletions(-)
>
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index 7057ddd7a0a8..40592b61b04b 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -249,27 +249,6 @@ int nfsd_nrthreads(struct net *net)
> return rv;
> }
>
> -static int nfsd_init_socks(struct net *net, const struct cred *cred)
> -{
> - int error;
> - struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> -
> - if (!list_empty(&nn->nfsd_serv->sv_permsocks))
> - return 0;
> -
> - error = svc_xprt_create(nn->nfsd_serv, "udp", net, PF_INET, NFS_PORT,
> - SVC_SOCK_DEFAULTS, cred);
> - if (error < 0)
> - return error;
> -
> - error = svc_xprt_create(nn->nfsd_serv, "tcp", net, PF_INET, NFS_PORT,
> - SVC_SOCK_DEFAULTS, cred);
> - if (error < 0)
> - return error;
> -
> - return 0;
> -}
> -
> static int nfsd_users = 0;
>
> static int nfsd_startup_generic(void)
> @@ -377,9 +356,12 @@ static int nfsd_startup_net(struct net *net, const struct cred *cred)
> ret = nfsd_startup_generic();
> if (ret)
> return ret;
> - ret = nfsd_init_socks(net, cred);
> - if (ret)
> +
> + if (list_empty(&nn->nfsd_serv->sv_permsocks)) {
> + pr_warn("NFSD: not starting because no listening sockets found\n");
> + ret = -EIO;
> goto out_socks;
> + }
>
> if (nfsd_needs_lockd(nn) && !nn->lockd_up) {
> ret = lockd_up(net, cred);
Assuming that there is no regression with rpc.nfsd startup, this looks
good to me.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] NFSD: don't start nfsd if sv_permsocks is empty
2025-10-30 17:53 ` Jeff Layton
@ 2025-10-30 23:05 ` NeilBrown
2025-10-31 17:26 ` Olga Kornievskaia
1 sibling, 0 replies; 9+ messages in thread
From: NeilBrown @ 2025-10-30 23:05 UTC (permalink / raw)
To: Jeff Layton
Cc: Olga Kornievskaia, chuck.lever, linux-nfs, neilb, Dai.Ngo, tom
On Fri, 31 Oct 2025, Jeff Layton wrote:
> On Thu, 2025-10-30 at 12:35 -0400, Olga Kornievskaia wrote:
> > Previously, while trying to create a server instance, if no
> > listening sockets were present then default parameter udp
> > and tcp listeners were created. It's unclear what purpose
> > was of starting these listeners were and how this could have
> > been triggered by the userland setup. This patch proposed
> > to ensure the reverse that we never end in a situation where
> > no listener sockets are created and we are trying to create
> > nfsd threads.
> >
> > The problem it solves is: when nfs.conf only has tcp=n (and
> > nothing else for the choice of transports), nfsdctl would
> > still start the server and create udp and tcp listeners.
> >
> > Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> > ---
> > fs/nfsd/nfssvc.c | 28 +++++-----------------------
> > 1 file changed, 5 insertions(+), 23 deletions(-)
> >
> > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> > index 7057ddd7a0a8..40592b61b04b 100644
> > --- a/fs/nfsd/nfssvc.c
> > +++ b/fs/nfsd/nfssvc.c
> > @@ -249,27 +249,6 @@ int nfsd_nrthreads(struct net *net)
> > return rv;
> > }
> >
> > -static int nfsd_init_socks(struct net *net, const struct cred *cred)
> > -{
> > - int error;
> > - struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> > -
> > - if (!list_empty(&nn->nfsd_serv->sv_permsocks))
> > - return 0;
> > -
> > - error = svc_xprt_create(nn->nfsd_serv, "udp", net, PF_INET, NFS_PORT,
> > - SVC_SOCK_DEFAULTS, cred);
> > - if (error < 0)
> > - return error;
> > -
> > - error = svc_xprt_create(nn->nfsd_serv, "tcp", net, PF_INET, NFS_PORT,
> > - SVC_SOCK_DEFAULTS, cred);
> > - if (error < 0)
> > - return error;
> > -
> > - return 0;
> > -}
> > -
> > static int nfsd_users = 0;
> >
> > static int nfsd_startup_generic(void)
> > @@ -377,9 +356,12 @@ static int nfsd_startup_net(struct net *net, const struct cred *cred)
> > ret = nfsd_startup_generic();
> > if (ret)
> > return ret;
> > - ret = nfsd_init_socks(net, cred);
> > - if (ret)
> > +
> > + if (list_empty(&nn->nfsd_serv->sv_permsocks)) {
> > + pr_warn("NFSD: not starting because no listening sockets found\n");
> > + ret = -EIO;
> > goto out_socks;
> > + }
> >
> > if (nfsd_needs_lockd(nn) && !nn->lockd_up) {
> > ret = lockd_up(net, cred);
>
>
> Assuming that there is no regression with rpc.nfsd startup, this looks
> good to me.
>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
>
>
Agreed.
Reviewed-by: NeilBrown <neil@brown.name>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] NFSD: don't start nfsd if sv_permsocks is empty
2025-10-30 17:53 ` Jeff Layton
2025-10-30 23:05 ` NeilBrown
@ 2025-10-31 17:26 ` Olga Kornievskaia
1 sibling, 0 replies; 9+ messages in thread
From: Olga Kornievskaia @ 2025-10-31 17:26 UTC (permalink / raw)
To: Jeff Layton
Cc: Olga Kornievskaia, chuck.lever, linux-nfs, neilb, Dai.Ngo, tom
On Thu, Oct 30, 2025 at 1:55 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> On Thu, 2025-10-30 at 12:35 -0400, Olga Kornievskaia wrote:
> > Previously, while trying to create a server instance, if no
> > listening sockets were present then default parameter udp
> > and tcp listeners were created. It's unclear what purpose
> > was of starting these listeners were and how this could have
> > been triggered by the userland setup. This patch proposed
> > to ensure the reverse that we never end in a situation where
> > no listener sockets are created and we are trying to create
> > nfsd threads.
> >
> > The problem it solves is: when nfs.conf only has tcp=n (and
> > nothing else for the choice of transports), nfsdctl would
> > still start the server and create udp and tcp listeners.
> >
> > Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> > ---
> > fs/nfsd/nfssvc.c | 28 +++++-----------------------
> > 1 file changed, 5 insertions(+), 23 deletions(-)
> >
> > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> > index 7057ddd7a0a8..40592b61b04b 100644
> > --- a/fs/nfsd/nfssvc.c
> > +++ b/fs/nfsd/nfssvc.c
> > @@ -249,27 +249,6 @@ int nfsd_nrthreads(struct net *net)
> > return rv;
> > }
> >
> > -static int nfsd_init_socks(struct net *net, const struct cred *cred)
> > -{
> > - int error;
> > - struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> > -
> > - if (!list_empty(&nn->nfsd_serv->sv_permsocks))
> > - return 0;
> > -
> > - error = svc_xprt_create(nn->nfsd_serv, "udp", net, PF_INET, NFS_PORT,
> > - SVC_SOCK_DEFAULTS, cred);
> > - if (error < 0)
> > - return error;
> > -
> > - error = svc_xprt_create(nn->nfsd_serv, "tcp", net, PF_INET, NFS_PORT,
> > - SVC_SOCK_DEFAULTS, cred);
> > - if (error < 0)
> > - return error;
> > -
> > - return 0;
> > -}
> > -
> > static int nfsd_users = 0;
> >
> > static int nfsd_startup_generic(void)
> > @@ -377,9 +356,12 @@ static int nfsd_startup_net(struct net *net, const struct cred *cred)
> > ret = nfsd_startup_generic();
> > if (ret)
> > return ret;
> > - ret = nfsd_init_socks(net, cred);
> > - if (ret)
> > +
> > + if (list_empty(&nn->nfsd_serv->sv_permsocks)) {
> > + pr_warn("NFSD: not starting because no listening sockets found\n");
> > + ret = -EIO;
> > goto out_socks;
> > + }
> >> > if (nfsd_needs_lockd(nn) && !nn->lockd_up) {
> > ret = lockd_up(net, cred);
>
>
> Assuming that there is no regression with rpc.nfsd startup, this looks
> good to me.
I haven't found a combination of parameters to rpc.nfsd that would
ever trigger nfsd_init_socks() to have an empty list. Looking at the
nfs-utils nfsd code I see code that fails nfsd start if there were no
sockets.
I've been digging into code evolution. And day0 was nfsd would
svc_create() and then call svc_makesock(UDP) and svc_makesock(TPC)
(originally ifdef-ed). So sockets were always created right there.
Then starting from this commit 02a375f0ac4bc "knfsd: separate out some
parts of nfsd_svc, which start nfs servers", nfsd_init_socks() was
born with the check for sv_permsock being empty. I'm assuming it's
because function is called multiple times and socket creation needed
to be done once... anyway looking at old code, it looked like sockets
were created in the kernel. Then later socket creation was moved into
userspace. Then rpc.nfsd would make sure there would be sockets before
calling into the kernel. At that point I believe nfsd_init_sock()
would always guarantee that sv_permsocks would have sockets there
making the code.
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/1] NFSD: don't start nfsd if sv_permsocks is empty
2025-10-30 16:35 [PATCH 1/1] NFSD: don't start nfsd if sv_permsocks is empty Olga Kornievskaia
2025-10-30 17:53 ` Jeff Layton
@ 2025-10-31 12:51 ` Chuck Lever
2025-10-31 17:26 ` Olga Kornievskaia
2025-10-31 23:41 ` NeilBrown
1 sibling, 2 replies; 9+ messages in thread
From: Chuck Lever @ 2025-10-31 12:51 UTC (permalink / raw)
To: Olga Kornievskaia, jlayton; +Cc: linux-nfs, neilb, Dai.Ngo, tom
On 10/30/25 12:35 PM, Olga Kornievskaia wrote:
> Previously, while trying to create a server instance, if no
> listening sockets were present then default parameter udp
> and tcp listeners were created. It's unclear what purpose
> was of starting these listeners were and how this could have
> been triggered by the userland setup. This patch proposed
> to ensure the reverse that we never end in a situation where
> no listener sockets are created and we are trying to create
> nfsd threads.
>
> The problem it solves is: when nfs.conf only has tcp=n (and
> nothing else for the choice of transports), nfsdctl would
> still start the server and create udp and tcp listeners.
>
Fixes: ?
One more below.
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> ---
> fs/nfsd/nfssvc.c | 28 +++++-----------------------
> 1 file changed, 5 insertions(+), 23 deletions(-)
>
> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> index 7057ddd7a0a8..40592b61b04b 100644
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -249,27 +249,6 @@ int nfsd_nrthreads(struct net *net)
> return rv;
> }
>
> -static int nfsd_init_socks(struct net *net, const struct cred *cred)
> -{
> - int error;
> - struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> -
> - if (!list_empty(&nn->nfsd_serv->sv_permsocks))
> - return 0;
> -
> - error = svc_xprt_create(nn->nfsd_serv, "udp", net, PF_INET, NFS_PORT,
> - SVC_SOCK_DEFAULTS, cred);
> - if (error < 0)
> - return error;
> -
> - error = svc_xprt_create(nn->nfsd_serv, "tcp", net, PF_INET, NFS_PORT,
> - SVC_SOCK_DEFAULTS, cred);
> - if (error < 0)
> - return error;
> -
> - return 0;
> -}
> -
> static int nfsd_users = 0;
>
> static int nfsd_startup_generic(void)
> @@ -377,9 +356,12 @@ static int nfsd_startup_net(struct net *net, const struct cred *cred)
> ret = nfsd_startup_generic();
> if (ret)
> return ret;
> - ret = nfsd_init_socks(net, cred);
> - if (ret)
> +
> + if (list_empty(&nn->nfsd_serv->sv_permsocks)) {
> + pr_warn("NFSD: not starting because no listening sockets found\n");
I know the code refers to sockets, but the term doesn't refer to RDMA
listeners at all, and this warning seems applicable to both socket-based
and RDMA transports. How about:
NFSD: No available listeners
> + ret = -EIO;
> goto out_socks;
> + }
>
> if (nfsd_needs_lockd(nn) && !nn->lockd_up) {
> ret = lockd_up(net, cred);
--
Chuck Lever
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] NFSD: don't start nfsd if sv_permsocks is empty
2025-10-31 12:51 ` Chuck Lever
@ 2025-10-31 17:26 ` Olga Kornievskaia
2025-10-31 23:41 ` NeilBrown
1 sibling, 0 replies; 9+ messages in thread
From: Olga Kornievskaia @ 2025-10-31 17:26 UTC (permalink / raw)
To: Chuck Lever; +Cc: Olga Kornievskaia, jlayton, linux-nfs, neilb, Dai.Ngo, tom
On Fri, Oct 31, 2025 at 8:52 AM Chuck Lever <chuck.lever@oracle.com> wrote:
>
> On 10/30/25 12:35 PM, Olga Kornievskaia wrote:
> > Previously, while trying to create a server instance, if no
> > listening sockets were present then default parameter udp
> > and tcp listeners were created. It's unclear what purpose
> > was of starting these listeners were and how this could have
> > been triggered by the userland setup. This patch proposed
> > to ensure the reverse that we never end in a situation where
> > no listener sockets are created and we are trying to create
> > nfsd threads.
> >
> > The problem it solves is: when nfs.conf only has tcp=n (and
> > nothing else for the choice of transports), nfsdctl would
> > still start the server and create udp and tcp listeners.
> >
>
> Fixes: ?
I don't think it deserves a Fixes. There wasn't a way (that I have
discovered) that would ever exercise that code because the checking
that we had socket(s) (or even setting up a socket) was done in
userpand by rpc.nfsd. nfsdctl doesn't do that and thus exposed the
situation where we are starting a thread and had no listeners. While
there was an option to change nfsdctl to do the checking, the kernel
change made more sense as I don't see a reason why the code is useful.
>
> One more below.
>
>
> > Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> > ---
> > fs/nfsd/nfssvc.c | 28 +++++-----------------------
> > 1 file changed, 5 insertions(+), 23 deletions(-)
> >
> > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> > index 7057ddd7a0a8..40592b61b04b 100644
> > --- a/fs/nfsd/nfssvc.c
> > +++ b/fs/nfsd/nfssvc.c
> > @@ -249,27 +249,6 @@ int nfsd_nrthreads(struct net *net)
> > return rv;
> > }
> >
> > -static int nfsd_init_socks(struct net *net, const struct cred *cred)
> > -{
> > - int error;
> > - struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> > -
> > - if (!list_empty(&nn->nfsd_serv->sv_permsocks))
> > - return 0;
> > -
> > - error = svc_xprt_create(nn->nfsd_serv, "udp", net, PF_INET, NFS_PORT,
> > - SVC_SOCK_DEFAULTS, cred);
> > - if (error < 0)
> > - return error;
> > -
> > - error = svc_xprt_create(nn->nfsd_serv, "tcp", net, PF_INET, NFS_PORT,
> > - SVC_SOCK_DEFAULTS, cred);
> > - if (error < 0)
> > - return error;
> > -
> > - return 0;
> > -}
> > -
> > static int nfsd_users = 0;
> >
> > static int nfsd_startup_generic(void)
> > @@ -377,9 +356,12 @@ static int nfsd_startup_net(struct net *net, const struct cred *cred)
> > ret = nfsd_startup_generic();
> > if (ret)
> > return ret;
> > - ret = nfsd_init_socks(net, cred);
> > - if (ret)
> > +
> > + if (list_empty(&nn->nfsd_serv->sv_permsocks)) {
> > + pr_warn("NFSD: not starting because no listening sockets found\n");
>
> I know the code refers to sockets, but the term doesn't refer to RDMA
> listeners at all, and this warning seems applicable to both socket-based
> and RDMA transports. How about:
>
> NFSD: No available listeners
This doesn't convey to me the fact that we didn't start the server. How about
"NFSD: not started because no listeners were found" or
"NFSD: not started, no available listeners"?
>
>
> > + ret = -EIO;
> > goto out_socks;
> > + }
> >
> > if (nfsd_needs_lockd(nn) && !nn->lockd_up) {
> > ret = lockd_up(net, cred);
>
>
> --
> Chuck Lever
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] NFSD: don't start nfsd if sv_permsocks is empty
2025-10-31 12:51 ` Chuck Lever
2025-10-31 17:26 ` Olga Kornievskaia
@ 2025-10-31 23:41 ` NeilBrown
2025-11-02 15:46 ` Chuck Lever
1 sibling, 1 reply; 9+ messages in thread
From: NeilBrown @ 2025-10-31 23:41 UTC (permalink / raw)
To: Chuck Lever; +Cc: Olga Kornievskaia, jlayton, linux-nfs, neilb, Dai.Ngo, tom
On Fri, 31 Oct 2025, Chuck Lever wrote:
> On 10/30/25 12:35 PM, Olga Kornievskaia wrote:
> > Previously, while trying to create a server instance, if no
> > listening sockets were present then default parameter udp
> > and tcp listeners were created. It's unclear what purpose
> > was of starting these listeners were and how this could have
> > been triggered by the userland setup. This patch proposed
> > to ensure the reverse that we never end in a situation where
> > no listener sockets are created and we are trying to create
> > nfsd threads.
> >
> > The problem it solves is: when nfs.conf only has tcp=n (and
> > nothing else for the choice of transports), nfsdctl would
> > still start the server and create udp and tcp listeners.
> >
>
> Fixes: ?
>
> One more below.
>
>
> > Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> > ---
> > fs/nfsd/nfssvc.c | 28 +++++-----------------------
> > 1 file changed, 5 insertions(+), 23 deletions(-)
> >
> > diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> > index 7057ddd7a0a8..40592b61b04b 100644
> > --- a/fs/nfsd/nfssvc.c
> > +++ b/fs/nfsd/nfssvc.c
> > @@ -249,27 +249,6 @@ int nfsd_nrthreads(struct net *net)
> > return rv;
> > }
> >
> > -static int nfsd_init_socks(struct net *net, const struct cred *cred)
> > -{
> > - int error;
> > - struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> > -
> > - if (!list_empty(&nn->nfsd_serv->sv_permsocks))
> > - return 0;
> > -
> > - error = svc_xprt_create(nn->nfsd_serv, "udp", net, PF_INET, NFS_PORT,
> > - SVC_SOCK_DEFAULTS, cred);
> > - if (error < 0)
> > - return error;
> > -
> > - error = svc_xprt_create(nn->nfsd_serv, "tcp", net, PF_INET, NFS_PORT,
> > - SVC_SOCK_DEFAULTS, cred);
> > - if (error < 0)
> > - return error;
> > -
> > - return 0;
> > -}
> > -
> > static int nfsd_users = 0;
> >
> > static int nfsd_startup_generic(void)
> > @@ -377,9 +356,12 @@ static int nfsd_startup_net(struct net *net, const struct cred *cred)
> > ret = nfsd_startup_generic();
> > if (ret)
> > return ret;
> > - ret = nfsd_init_socks(net, cred);
> > - if (ret)
> > +
> > + if (list_empty(&nn->nfsd_serv->sv_permsocks)) {
> > + pr_warn("NFSD: not starting because no listening sockets found\n");
>
> I know the code refers to sockets, but the term doesn't refer to RDMA
> listeners at all, and this warning seems applicable to both socket-based
> and RDMA transports. How about:
>
> NFSD: No available listeners
"configured" rather than "available" ??
"network listeners"? "network request listeners" ??
"ports" rather than "sockets" ??
NFSD: No network ports configured for listening
??
I did consider suggesting that the message isn't needed.
NeilBrown
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] NFSD: don't start nfsd if sv_permsocks is empty
2025-10-31 23:41 ` NeilBrown
@ 2025-11-02 15:46 ` Chuck Lever
2025-11-02 22:11 ` NeilBrown
0 siblings, 1 reply; 9+ messages in thread
From: Chuck Lever @ 2025-11-02 15:46 UTC (permalink / raw)
To: NeilBrown, Olga Kornievskaia; +Cc: jlayton, linux-nfs, neilb, Dai.Ngo, tom
On 10/31/25 7:41 PM, NeilBrown wrote:
> On Fri, 31 Oct 2025, Chuck Lever wrote:
>> On 10/30/25 12:35 PM, Olga Kornievskaia wrote:
>>> Previously, while trying to create a server instance, if no
>>> listening sockets were present then default parameter udp
>>> and tcp listeners were created. It's unclear what purpose
>>> was of starting these listeners were and how this could have
>>> been triggered by the userland setup. This patch proposed
>>> to ensure the reverse that we never end in a situation where
>>> no listener sockets are created and we are trying to create
>>> nfsd threads.
>>>
>>> The problem it solves is: when nfs.conf only has tcp=n (and
>>> nothing else for the choice of transports), nfsdctl would
>>> still start the server and create udp and tcp listeners.
>>>
>>
>> Fixes: ?
>>
>> One more below.
>>
>>
>>> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
>>> ---
>>> fs/nfsd/nfssvc.c | 28 +++++-----------------------
>>> 1 file changed, 5 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
>>> index 7057ddd7a0a8..40592b61b04b 100644
>>> --- a/fs/nfsd/nfssvc.c
>>> +++ b/fs/nfsd/nfssvc.c
>>> @@ -249,27 +249,6 @@ int nfsd_nrthreads(struct net *net)
>>> return rv;
>>> }
>>>
>>> -static int nfsd_init_socks(struct net *net, const struct cred *cred)
>>> -{
>>> - int error;
>>> - struct nfsd_net *nn = net_generic(net, nfsd_net_id);
>>> -
>>> - if (!list_empty(&nn->nfsd_serv->sv_permsocks))
>>> - return 0;
>>> -
>>> - error = svc_xprt_create(nn->nfsd_serv, "udp", net, PF_INET, NFS_PORT,
>>> - SVC_SOCK_DEFAULTS, cred);
>>> - if (error < 0)
>>> - return error;
>>> -
>>> - error = svc_xprt_create(nn->nfsd_serv, "tcp", net, PF_INET, NFS_PORT,
>>> - SVC_SOCK_DEFAULTS, cred);
>>> - if (error < 0)
>>> - return error;
>>> -
>>> - return 0;
>>> -}
>>> -
>>> static int nfsd_users = 0;
>>>
>>> static int nfsd_startup_generic(void)
>>> @@ -377,9 +356,12 @@ static int nfsd_startup_net(struct net *net, const struct cred *cred)
>>> ret = nfsd_startup_generic();
>>> if (ret)
>>> return ret;
>>> - ret = nfsd_init_socks(net, cred);
>>> - if (ret)
>>> +
>>> + if (list_empty(&nn->nfsd_serv->sv_permsocks)) {
>>> + pr_warn("NFSD: not starting because no listening sockets found\n");
>>
>> I know the code refers to sockets, but the term doesn't refer to RDMA
>> listeners at all, and this warning seems applicable to both socket-based
>> and RDMA transports. How about:
>>
>> NFSD: No available listeners
>
> "configured" rather than "available" ??
> "network listeners"? "network request listeners" ??
> "ports" rather than "sockets" ??
>
> NFSD: No network ports configured for listening
> ??
>
> I did consider suggesting that the message isn't needed.
I lean towards having a notice that something didn't go as planned.
My final pitch:
NFSD: Failed to start, no listeners configured.
--
Chuck Lever
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 1/1] NFSD: don't start nfsd if sv_permsocks is empty
2025-11-02 15:46 ` Chuck Lever
@ 2025-11-02 22:11 ` NeilBrown
0 siblings, 0 replies; 9+ messages in thread
From: NeilBrown @ 2025-11-02 22:11 UTC (permalink / raw)
To: Chuck Lever; +Cc: Olga Kornievskaia, jlayton, linux-nfs, neilb, Dai.Ngo, tom
On Mon, 03 Nov 2025, Chuck Lever wrote:
> On 10/31/25 7:41 PM, NeilBrown wrote:
> > On Fri, 31 Oct 2025, Chuck Lever wrote:
> >> On 10/30/25 12:35 PM, Olga Kornievskaia wrote:
> >>> Previously, while trying to create a server instance, if no
> >>> listening sockets were present then default parameter udp
> >>> and tcp listeners were created. It's unclear what purpose
> >>> was of starting these listeners were and how this could have
> >>> been triggered by the userland setup. This patch proposed
> >>> to ensure the reverse that we never end in a situation where
> >>> no listener sockets are created and we are trying to create
> >>> nfsd threads.
> >>>
> >>> The problem it solves is: when nfs.conf only has tcp=n (and
> >>> nothing else for the choice of transports), nfsdctl would
> >>> still start the server and create udp and tcp listeners.
> >>>
> >>
> >> Fixes: ?
> >>
> >> One more below.
> >>
> >>
> >>> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> >>> ---
> >>> fs/nfsd/nfssvc.c | 28 +++++-----------------------
> >>> 1 file changed, 5 insertions(+), 23 deletions(-)
> >>>
> >>> diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
> >>> index 7057ddd7a0a8..40592b61b04b 100644
> >>> --- a/fs/nfsd/nfssvc.c
> >>> +++ b/fs/nfsd/nfssvc.c
> >>> @@ -249,27 +249,6 @@ int nfsd_nrthreads(struct net *net)
> >>> return rv;
> >>> }
> >>>
> >>> -static int nfsd_init_socks(struct net *net, const struct cred *cred)
> >>> -{
> >>> - int error;
> >>> - struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> >>> -
> >>> - if (!list_empty(&nn->nfsd_serv->sv_permsocks))
> >>> - return 0;
> >>> -
> >>> - error = svc_xprt_create(nn->nfsd_serv, "udp", net, PF_INET, NFS_PORT,
> >>> - SVC_SOCK_DEFAULTS, cred);
> >>> - if (error < 0)
> >>> - return error;
> >>> -
> >>> - error = svc_xprt_create(nn->nfsd_serv, "tcp", net, PF_INET, NFS_PORT,
> >>> - SVC_SOCK_DEFAULTS, cred);
> >>> - if (error < 0)
> >>> - return error;
> >>> -
> >>> - return 0;
> >>> -}
> >>> -
> >>> static int nfsd_users = 0;
> >>>
> >>> static int nfsd_startup_generic(void)
> >>> @@ -377,9 +356,12 @@ static int nfsd_startup_net(struct net *net, const struct cred *cred)
> >>> ret = nfsd_startup_generic();
> >>> if (ret)
> >>> return ret;
> >>> - ret = nfsd_init_socks(net, cred);
> >>> - if (ret)
> >>> +
> >>> + if (list_empty(&nn->nfsd_serv->sv_permsocks)) {
> >>> + pr_warn("NFSD: not starting because no listening sockets found\n");
> >>
> >> I know the code refers to sockets, but the term doesn't refer to RDMA
> >> listeners at all, and this warning seems applicable to both socket-based
> >> and RDMA transports. How about:
> >>
> >> NFSD: No available listeners
> >
> > "configured" rather than "available" ??
> > "network listeners"? "network request listeners" ??
> > "ports" rather than "sockets" ??
> >
> > NFSD: No network ports configured for listening
> > ??
> >
> > I did consider suggesting that the message isn't needed.
>
> I lean towards having a notice that something didn't go as planned.
> My final pitch:
>
> NFSD: Failed to start, no listeners configured.
Works for me - good compromise.
NeilBrown
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-11-02 22:11 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-30 16:35 [PATCH 1/1] NFSD: don't start nfsd if sv_permsocks is empty Olga Kornievskaia
2025-10-30 17:53 ` Jeff Layton
2025-10-30 23:05 ` NeilBrown
2025-10-31 17:26 ` Olga Kornievskaia
2025-10-31 12:51 ` Chuck Lever
2025-10-31 17:26 ` Olga Kornievskaia
2025-10-31 23:41 ` NeilBrown
2025-11-02 15:46 ` Chuck Lever
2025-11-02 22:11 ` NeilBrown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).