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

On Tue, 06 Feb 2024, Chuck Lever III wrote:
>=20
>=20
> > On Feb 5, 2024, at 2:44=E2=80=AFPM, NeilBrown <neilb@suse.de> wrote:
> >=20
> > 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 stats <--- fetch the new stats that got merged
> >>>>> # nfsdctl add_listener <--- add a new listen socket, by address or ho=
stname
> >>>>> # 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
> >>>>>     https://github.com/LorenzoBianconi/nfsd-netlink
> >>>>=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 a=
nd continue
> >>>> the discussion in order to have something usable to develop the user-s=
pace 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-netlin=
k-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 stats                                                 <--- fet=
ch 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                                  <--- ena=
ble NFSv3 and v4.1
> >> $ nfsdctl set threads 128                                       <--- spi=
n up the threads
> >=20
> > My preference would be:
> >=20
> >   nfsdctl start
> > and=20
> >   nfsdctl stop
> >=20
> > 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.
> >=20
> > Possibly nfsctl would accept config on the command line:
> >  nfsdctl start proto=3D3,4.1 threads=3D42 proto=3Dtcp:localhost:2049
> > or similar.
>=20
> You've got proto=3D listed twice here.

Yep - the second was meant to be xprt=3D

>=20
> I'm more in favor of having more subcommands, each of which do
> something simple. Easier to understand, easier to test.

Fewer subcommands are easier to test than more.
OK - forget the "config on command line" idea.  Just read config from
/etc/nfs.conf and action that.

>=20
>=20
> > 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.
> >=20
> > In my experience working with our customers and support team, they are
> > not at all interested in fine control.
>=20
> This is an interface to be used by systemctl. I don't think customers
> or support teams would ever need to make use of it directly.

If it's to be used by systemctl, then there is zero justification for
anything other than start/reload/stop.  We already have
/etc/nfs.conf.d/* for configuring nfsd.  Requiring tools to edit
systemctl unit files as well is a retrograde step.

NeilBrown


>=20
>=20
> > "systemctl restart nfs-server" is
> > their second preference when "reboot" isn't appropriate for some reason.
> >=20
> > 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.
> >=20
> > NeilBrown
>=20
>=20
> --
> Chuck Lever
>=20
>=20
>=20