Received-SPF: pass (google.com: domain of linux-nfs+bounces-1794-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Mon, 5 Feb 2024 18:44:25 +0100
From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
To: Jeff Layton <jlayton@kernel.org>
Cc: Chuck Lever <chuck.lever@oracle.com>, NeilBrown <neilb@suse.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?
Message-ID: <ZcEeeRrYOQsKOPw_@lore-desk>
References: <8a7bbc05b6515109692cb88ad68374d14fc01eca.camel@kernel.org>
 <Zb0hlnQmgVikeNpi@lore-rh-laptop>
 <e706a77b4ddbceeb8b0ebac4dcbca03f8196e8a5.camel@kernel.org>
 <ZcEQzyrZpyrMJGKg@lore-desk>
 <799dc278c5d15605e1535303a7a15dee4eef82b3.camel@kernel.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="ayO4eR3f6pk4ILW8"
Content-Disposition: inline
In-Reply-To: <799dc278c5d15605e1535303a7a15dee4eef82b3.camel@kernel.org>


--ayO4eR3f6pk4ILW8
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

> On Mon, 2024-02-05 at 17:46 +0100, 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 tim=
e, when
> > > > > 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=
 nfsd.
> > > > >=20
> > > > > Here's what I'm thinking:
> > > > >=20
> > > > > Let's build a swiss-army-knife kind of interface like git or virs=
h:
> > > > >=20
> > > > > # nfsdctl=A0stats			<--- fetch the new stats that got merged
> > > > > # nfsdctl add_listener		<--- add a new listen socket, by address =
or hostname
> > > > > # nfsdctl set v3 on		<--- enable NFSv3
> > > > > # nfsdctl set splice_read off	<--- disable splice reads (per Chuc=
k's recent patch)
> > > > > # nfsdctl set threads 128	<--- spin up the threads
> > > > >=20
> > > > > We could start with just the bare minimum for now (the stats inte=
rface),
> > > > > 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.nf=
sd to
> > > > > start and stop the server. systemd will also need to fall back to=
 using
> > > > > 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 the
> > > > > plumbing we'd need:
> > > > >=20
> > > > > =A0=A0=A0=A0https://github.com/LorenzoBianconi/nfsd-netlink
> > > >=20
> > > > This is something I developed just for testing the new interface bu=
t I agree we
> > > > could start from it.
> > > >=20
> > > > Regarding the kernel part I addressed the comments I received upstr=
eam on v6 and
> > > > pushed the code here [0].
> > > > How do you guys prefer to proceed? Is the better to post v7 upstrea=
m and continue
> > > > the discussion in order to have something usable to develop the use=
r-space 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-net=
link-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 wi=
th
> > > 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=A0stats                                                 <--- =
fetch the new stats that got merged
> > $ nfsdctl xprt add proto <udp|tcp> host <host> [port <port>]    <--- ad=
d a new listen socket, by address or hostname
>=20
> Those look fine.
>=20
> All of the commands should display the current state too when run with
> no arguments. So running "nfsctl xprt" should dump out all of the
> listening sockets. Ditto for the ones below too.

ack

>=20
> > $ nfsdctl proto v3.0 v4.0 v4.1                                  <--- en=
able NFSv3 and v4.1
>=20
> The above would also enable v4.0 too?
>=20
> For this we might want to use the +/- syntax, actually. Also, there were
> no minorversions before v4, so it probably better not to accept that for
> v3:
>=20
>     $ nfsdctl proto +v3 -v4.0 +4.1

according to the previous discussion we agreed to pass to the kernel just t=
he
protocol versions we want to enable (v3.0 v4.0 v4.1 in the previous example)
and disable all the other ones..but I am fine to define a different semanti=
cs.
What do you think it is the best one?

>=20
> So to me, that would mean enable v3 and v4.1, and disable v4.0.
> v2 (if supported) and v4.2 would be left unchanged.
>=20
> > $ nfsdctl set threads 128                                       <--- sp=
in up the threads
> >=20
>=20
> Maybe:
>=20
>     $ nfsdctl threads 128

ack

>=20
> ?
> =20
> > Can we start from [0] to develop them?
> >=20
>=20
> Yeah, that seems like a good place to start.

ack, I will work in it.

Regards,
Lorenzo

>=20
> > Regards,
> > Lorenzo
> >=20
> > [0] https://github.com/LorenzoBianconi/nfsd-netlink
> --=20
> Jeff Layton <jlayton@kernel.org>
>=20

--ayO4eR3f6pk4ILW8
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQTquNwa3Txd3rGGn7Y6cBh0uS2trAUCZcEeeQAKCRA6cBh0uS2t
rA/aAP9Natx9nC8aLKnzmXABa4eW5gvRmb7PbD34Mt/ZEJwvgwD9FEDh4QeBzfRZ
4KP1Un6GuWaBNndgTmjeklrSwLtwAgA=
=0aJ1
-----END PGP SIGNATURE-----

--ayO4eR3f6pk4ILW8--