[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <85b02061798a1b750a87b0302681b86651d0c7a3.camel@kernel.org>
Date: Tue, 23 Jan 2024 06:17:15 -0500
From: Jeff Layton <jlayton@...nel.org>
To: Lorenzo Bianconi <lorenzo@...nel.org>, Chuck Lever
<chuck.lever@...cle.com>
Cc: NeilBrown <neilb@...e.de>, linux-nfs@...r.kernel.org,
lorenzo.bianconi@...hat.com, kuba@...nel.org, horms@...nel.org,
netdev@...r.kernel.org
Subject: Re: [PATCH v6 3/3] NFSD: add write_ports to netlink command
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, userspace is
> > > > > expected to provide a NFS listeners list it wants to enable (all the
> > > > > other ports will be closed).
> > > > >
> > > >
> > > > 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 change.
> > >
> > > I agree it is better, and thanks for highlighting the change.
> > >
> > > > > + /* 2- remove stale listeners */
> > > >
> > > >
> > > > The old portlist interface was weird, in that it was only additive. You
> > > > couldn't use it to close a listening socket (AFAICT). We may be able to
> > > > support that now with this interface, but we'll need to test that case
> > > > carefully.
> > >
> > > Do we ever want/need to remove listening sockets?
> >
> > 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.
> >
> > But for now, our socket listeners are "any". Wondering how net
> > namespaces play into this.
> >
> >
> > > 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 suspect
> > > anything that is not used by normal service start will hardly ever be
> > > used in practice, so will not be tested.
> >
> > Well, there is that. I guess until we have test coverage for NFSD
> > administrative interfaces, we should leave well enough alone.
>
> 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 remove
> them (the user/admin will need to stop/start the server).
>
> Agree? If so I will work on it and post v7.
>
>
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).
>
> >
> >
> > > 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.
> > >
> > > Thanks,
> > > NeilBrown
> >
> > --
> > Chuck Lever
--
Jeff Layton <jlayton@...nel.org>
Powered by blists - more mailing lists