Re: [PATCH 0/3] Enable the setting of a kernel module parameter from nfs.conf

[Trond Myklebust <trondmy@hammerspace.com>]

On Thu, 2021-04-15 at 16:37 +0000, Chuck Lever III wrote:
> 
> 
> > On Apr 15, 2021, at 11:33 AM, Steve Dickson <SteveD@RedHat.com>
> > wrote:
> > 
> > Hey Chuck! 
> > 
> > On 4/14/21 7:26 PM, Chuck Lever III wrote:
> > > Hi Steve-
> > > 
> > > > On Apr 14, 2021, at 2:10 PM, Steve Dickson <SteveD@redhat.com>
> > > > wrote:
> > > > 
> > > > This is a tweak of the patch set Alice Mitchell posted last
> > > > July [1].
> > > 
> > > That approach was dropped last July because it is not container-
> > > aware.
> > > It should be simple for someone to write a udev script that uses
> > > the
> > > existing sysfs API that can update nfs4_client_id in a namespace.
> > > I
> > > would prefer the sysfs/udev approach for setting nfs4_client_id,
> > > since it is container-aware and makes this setting completely
> > > automatic (zero touch).
> > As I said in in my cover letter, I see this more as introduction of
> > a mechanism more than a way to set the unique id.
> 
> Yep, I got that.
> 
> I'm not addressing the question of whether adding a
> mechanism to set a module parameter in nfs.conf is good
> or not. I'm saying nfs4_client_id is not an appropriate
> parameter to add to nfs.conf. Can you pick another
> module parameter as an example for your mechanism?
> 
> 
> > The mechanism being
> > a way to set kernel module params from nfs.conf. The setting of
> > the id is just a side effect... 
> > 
> > Why spread out the NFS configuration?  Why not
> > just keep it in one place... aka nfs.conf?
> 
> We need to understand whether a module parameter is not
> going to work in nfs.conf because that setting needs to
> be namespace-aware. In this case, this setting does indeed
> need to be namespace-aware. nfs.conf is not aware of
> network namespaces.
> 
> 
> > Plus we could document all the kernel params in nfs.conf
> > and the nfs.conf man page. The only documentation I know 
> > of is in the kernel tree.
> 
> OK, but that's not relevant to whether nfs.conf is the
> right place to set nfs4_client_id.
> 
> 
> > As far as not being container-aware... that might true
> > but it does not mean its not useful to set the id from
> > nfs.conf...
> 
> Yes, it does mean that in that case. It's completely
> broken to use the same nfs4_client_id in every network
> namespace.
> 
> 
> > Actual I have customers asking for this type
> > of functionality
> 
> Ask yourself why they might want it. It's probably because
> we don't set it correctly currently. If we have a way to
> automatically get it right every time, there's really no
> need for this setting to be exposed.
> 
> I do agree that it's long past time we should be setting
> nfs4_client_id properly. I would rather see a udev script
> developed (you, me, or Alice could do it in an afternoon)
> first. If that doesn't meet the actual customer need, then
> we can revisit.
> 

Right. The only sensible solution in a containerised world is a udev
script that sets /sys/fs/nfs/net/nfs_client/identifier when triggered.

Note that we really want something that generates a random uuid, and
then persists it so that it can be retrieved on reboot or restart of
the container. Something similar to systemd-machine-id-setup, but that
can be called from udev.

> 
> > steved.
> > > 
> > > 
> > > > It enables the setting of the nfs4_unique_id kernel module 
> > > > parameter from /etc/nfs.conf.
> > > 
> > > > Things I tweaked:
> > > > 
> > > >   * Introduce a new [kernel] section in nfs.conf which only
> > > >     contains the nfs4_unique_id setting... For now... 
> > > > 
> > > >   * nfs4_unique_id can be set to two different values
> > > > 
> > > >       - nfs4_unique_id = ${machine-id} will use /etc/machine-id
> > > >           as the unique id.
> > > >       - nfs4_unique_id = ${hostname} will use the system's
> > > > hostname
> > > >           as the unique id.
> > > > 
> > > >   * The new nfs-config systemd service need to be enabled for
> > > > the
> > > >     /etc/modprobe.d/nfs.conf file to be created with 
> > > >     the "options nfs nfs4_unique_id=" set. 
> > > > 
> > > > I see this patch set is not a way to set the nfs4_unique_id 
> > > > module parameter... I see it as a beginning of a way to set 
> > > > all module parameters from /etc/nfs.conf, which I think
> > > > is a good thing... 
> > > > 
> > > > [1] https://www.spinics.net/lists/linux-nfs/msg78658.html
> > > > 
> > > > Alice Mitchell (3):
> > > > nfs-utils: Enable the retrieval of raw config settings without
> > > >   expansion
> > > > nfs-utils: Add support for further ${variable} expansions in
> > > > nfs.conf
> > > > nfs-utils: Update nfs4_unique_id module parameter from the
> > > > nfs.conf
> > > >   value
> > > > 
> > > > configure.ac                  |   1 +
> > > > nfs.conf                      |   4 +-
> > > > support/include/conffile.h    |   1 +
> > > > support/nfs/conffile.c        | 283
> > > > ++++++++++++++++++++++++++++++++--
> > > > systemd/Makefile.am           |   3 +
> > > > systemd/nfs-client.target     |   3 +
> > > > systemd/nfs-conf-export.sh    |  28 ++++
> > > > systemd/nfs-config.service.in |  18 +++
> > > > systemd/nfs.conf.man          |  19 ++-
> > > > tools/nfsconf/nfsconf.man     |  10 +-
> > > > tools/nfsconf/nfsconfcli.c    |  22 ++-
> > > > 11 files changed, 372 insertions(+), 20 deletions(-)
> > > > create mode 100755 systemd/nfs-conf-export.sh
> > > > create mode 100644 systemd/nfs-config.service.in
> > > > 
> > > > -- 
> > > > 2.30.2
> > > > 
> > 
> 
> --
> Chuck Lever
> 
> 
> 

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com