Received-SPF: pass (google.com: domain of linux-nfs+bounces-1797-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
MIME-Version: 1.0
From: "NeilBrown" <neilb@suse.de>
To: "Lorenzo Bianconi" <lorenzo.bianconi@redhat.com>
Cc: "Jeff Layton" <jlayton@kernel.org>, "Chuck Lever" <chuck.lever@oracle.com>,
 "Dai Ngo" <dai.ngo@oracle.com>,
 "olga.kornievskaia" <olga.kornievskaia@gmail.com>,
 "Tom Talpey" <tom@talpey.com>, "linux-nfs" <linux-nfs@vger.kernel.org>
Subject: Re: Should we establish a new nfsdctl userland program?
In-reply-to: <ZcEQzyrZpyrMJGKg@lore-desk>
References: <8a7bbc05b6515109692cb88ad68374d14fc01eca.camel@kernel.org>,
 <Zb0hlnQmgVikeNpi@lore-rh-laptop>,
 <e706a77b4ddbceeb8b0ebac4dcbca03f8196e8a5.camel@kernel.org>,
 <ZcEQzyrZpyrMJGKg@lore-desk>
Date: Tue, 06 Feb 2024 06:44:39 +1100
Message-id: <170716227903.13976.3838162060222642711@noble.neil.brown.name>

On Tue, 06 Feb 2024, Lorenzo Bianconi wrote:
> > On Fri, 2024-02-02 at 18:08 +0100, Lorenzo Bianconi wrote:
> > > > The existing rpc.nfsd program was designed during a different time, w=
hen
> > > > we just didn't require that much control over how it behaved. It's
> > > > klunky to work with.
> > > >=20
> > > > In a response to Chuck's recent RFC patch to add knob to disable
> > > > READ_PLUS calls, I mentioned that it might be a good time to make a
> > > > clean break from the past and start a new program for controlling nfs=
d.
> > > >=20
> > > > Here's what I'm thinking:
> > > >=20
> > > > Let's build a swiss-army-knife kind of interface like git or virsh:
> > > >=20
> > > > # nfsdctl=C2=A0stats			<--- fetch the new stats that got merged
> > > > # nfsdctl add_listener		<--- add a new listen socket, by address or h=
ostname
> > > > # nfsdctl set v3 on		<--- enable NFSv3
> > > > # nfsdctl set splice_read off	<--- disable splice reads (per Chuck's =
recent patch)
> > > > # nfsdctl set threads 128	<--- spin up the threads
> > > >=20
> > > > We could start with just the bare minimum for now (the stats interfac=
e),
> > > > and then expand on it. Once we're at feature parity with rpc.nfsd, we=
'd
> > > > want to have systemd preferentially use nfsdctl instead of rpc.nfsd to
> > > > start and stop the server. systemd will also need to fall back to usi=
ng
> > > > rpc.nfsd if nfsdctl or the netlink program isn't present.
> > > >=20
> > > > Note that I think this program will have to be a compiled binary vs. a
> > > > python script or the like, given that it'll be involved in system
> > > > startup.
> > > >=20
> > > > It turns out that Lorenzo already has a C program that has a lot of t=
he
> > > > plumbing we'd need:
> > > >=20
> > > > =C2=A0=C2=A0=C2=A0=C2=A0https://github.com/LorenzoBianconi/nfsd-netli=
nk
> > >=20
> > > This is something I developed just for testing the new interface but I =
agree we
> > > could start from it.
> > >=20
> > > Regarding the kernel part I addressed the comments I received upstream =
on v6 and
> > > pushed the code here [0].
> > > How do you guys prefer to proceed? Is the better to post v7 upstream an=
d continue
> > > the discussion in order to have something usable to develop the user-sp=
ace part or
> > > do you prefer to have something for the user-space first?
> > > I do not have a strong opinion on it.
> > >=20
> > > Regards,
> > > Lorenzo
> > >=20
> > > [0] https://github.com/LorenzoBianconi/nfsd-next/tree/nfsd-next-netlink=
-new-cmds-public-v7
> > >=20
> > >=20
> >=20
> > My advice?
> >=20
> > Step back and spend some time working on the userland bits before
> > posting another revision. Experience has shown that you never realize
> > what sort of warts an interface like this has until you have to work
> > with it.
> >=20
> > You may find that you want to tweak it some once you do, and it's much
> > easier to do that before we merge anything. This will be part of the
> > kernel ABI, so once it's in a shipping kernel, we're sort of stuck with
> > it.
> >=20
> > Having a userland program ready to go will allow us to do things like
> > set up the systemd service for this too, which is primarily how this new
> > program will be called.
>=20
> I agree on it. In order to proceed I guess we should define a list of
> requirements/expected behaviour on this new user-space tool used to
> configure nfsd. I am not so familiar with the user-space requirements
> for nfsd so I am just copying what you suggested, something like:
>=20
> $ nfsdctl=C2=A0stats                                                 <--- f=
etch the new stats that got merged
> $ nfsdctl xprt add proto <udp|tcp> host <host> [port <port>]    <--- add a =
new listen socket, by address or hostname
> $ nfsdctl proto v3.0 v4.0 v4.1                                  <--- enable=
 NFSv3 and v4.1
> $ nfsdctl set threads 128                                       <--- spin u=
p the threads

My preference would be:

   nfsdctl start
and=20
   nfsdctl stop

where nfsdctl reads a config file to discover what setting are required.
I cannot see any credible use case for 'xprt' or 'proto' or 'threads'
commands.

Possibly nfsctl would accept config on the command line:
  nfsdctl start proto=3D3,4.1 threads=3D42 proto=3Dtcp:localhost:2049
or similar.

It would also be helpful to have "nfsdinfo" or similar which has "stats"
and "status" commands.  Maybe that could be combined with "nfsdctl", but
extracting info is not a form of "control".  Or we could find a more
generic verb.  For soft-raid I wrote "mdadm" "adm" for "administer"
which seemed less specific than "control" (mdctl).  Though probably the
main reason was that I like palindromes and "mdadm" was a bit easier to
say.  nfsdadm ??  (see also udevadm, drdbadm, fsadm ....) But maybe I'm
just too fuss.

In my experience working with our customers and support team, they are
not at all interested in fine control.  "systemctl restart nfs-server" is
their second preference when "reboot" isn't appropriate for some reason.

You might be able to convince me that "nfdctl reload" was useful - it
could be called from "systemctl reload nfs-server".  That would then
justify kernel interfaces to remove xprts.  But having all the
fine-control just increases the required testing needed with little
practical gain.

NeilBrown