Received-SPF: pass (google.com: domain of linux-nfs+bounces-1291-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Tue, 23 Jan 2024 14:21:03 +0100
From: Lorenzo Bianconi <lorenzo@kernel.org>
To: Jeff Layton <jlayton@kernel.org>
Cc: Chuck Lever <chuck.lever@oracle.com>, NeilBrown <neilb@suse.de>,
	linux-nfs@vger.kernel.org, lorenzo.bianconi@redhat.com,
	kuba@kernel.org, horms@kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
Message-ID: <Za-9P0NjlIsc1PcE@lore-desk>
References: <cover.1705771400.git.lorenzo@kernel.org>
 <f7c42dae2b232b3b06e54ceb3f00725893973e02.1705771400.git.lorenzo@kernel.org>
 <9e3ae337dcf168c60c4cfd51aa0b2fc7b24bcbfb.camel@kernel.org>
 <170595930799.23031.17998490973211605470@noble.neil.brown.name>
 <Za7zHvPJdei/vWm4@tissot.1015granger.net>
 <Za-N6BxOMXTGyxmW@lore-desk>
 <85b02061798a1b750a87b0302681b86651d0c7a3.camel@kernel.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="oGji4Uj0nBLpvmk1"
Content-Disposition: inline
In-Reply-To: <85b02061798a1b750a87b0302681b86651d0c7a3.camel@kernel.org>


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

> On Tue, 2024-01-23 at 10:59 +0100, Lorenzo Bianconi wrote:
> > > On Tue, Jan 23, 2024 at 08:35:07AM +1100, NeilBrown wrote:
> > > > On Tue, 23 Jan 2024, Jeff Layton wrote:
> > > > > On Sat, 2024-01-20 at 18:33 +0100, Lorenzo Bianconi wrote:
> > > > > > Introduce write_ports netlink command. For listener-set, usersp=
ace is
> > > > > > expected to provide a NFS listeners list it wants to enable (al=
l the
> > > > > > other ports will be closed).
> > > > > >=20
> > > > >=20
> > > > > Ditto here. This is a change to a declarative interface, which I =
think
> > > > > is a better way to handle this, but we should be aware of the cha=
nge.
> > > >=20
> > > > I agree it is better, and thanks for highlighting the change.
> > > >=20
> > > > > > +	/* 2- remove stale listeners */
> > > > >=20
> > > > >=20
> > > > > The old portlist interface was weird, in that it was only additiv=
e. You
> > > > > couldn't use it to close a listening socket (AFAICT). We may be a=
ble to
> > > > > support that now with this interface, but we'll need to test that=
 case
> > > > > carefully.
> > > >=20
> > > > Do we ever want/need to remove listening sockets?
> > >=20
> > > I think that might be an interesting use case. Disabling RDMA, for
> > > example, should kill the RDMA listening endpoints but leave
> > > listening sockets in place.
> > >=20
> > > But for now, our socket listeners are "any". Wondering how net
> > > namespaces play into this.
> > >=20
> > >=20
> > > > Normal practice when making any changes is to stop and restart where
> > > > "stop" removes all sockets, unexports all filesystems, disables all
> > > > versions.
> > > > I don't exactly object to supporting fine-grained changes, but I su=
spect
> > > > anything that is not used by normal service start will hardly ever =
be
> > > > used in practice, so will not be tested.
> > >=20
> > > Well, there is that. I guess until we have test coverage for NFSD
> > > administrative interfaces, we should leave well enough alone.
> >=20
> > So to summarize it:
> > - we will allow to remove enabled versions (as it is in patch v6 2/3)
> > - we will allow to add new listening sockets but we will not allow to r=
emove
> > =A0=A0them (the user/admin will need to stop/start the server).
> >=20
> > Agree? If so I will work on it and post v7.
> >=20
> >=20
>=20
> That sounds about right to me. We could eventually relax the restriction
> about removing sockets later, but for now it's probably best to prohibit
> it (like Neil suggests).

Do we want to add even the capability to specify the socket file descriptor
(similar to what we do in __write_ports_addfd())?

Regards,
Lorenzo

>=20
>=20
> >=20
> > >=20
> > >=20
> > > > So if it is easiest to support reverting previous configuration (as=
 it
> > > > probably is for version setting), then do so.  But if there is any
> > > > complexity (as maybe there is with listening sockets), then don't
> > > > add complexity that won't be used.
> > > >=20
> > > > Thanks,
> > > > NeilBrown
> > >=20
> > > --=20
> > > Chuck Lever
>=20
> --=20
> Jeff Layton <jlayton@kernel.org>

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

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

iHUEABYKAB0WIQTquNwa3Txd3rGGn7Y6cBh0uS2trAUCZa+9PwAKCRA6cBh0uS2t
rLjZAP95AZ1urFqF2KP8Eg9tgPCd28subpAoaeXd5sIAEcXj5QEAt7e4wWyzT2PN
nMr5DVs6HD66u+mtplaRBQfrkdEa8wk=
=jNqe
-----END PGP SIGNATURE-----

--oGji4Uj0nBLpvmk1--