2021-04-14 18:20:26

by Steve Dickson

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

This is a tweak of the patch set Alice Mitchell posted last July [1].
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


2021-04-15 00:50:39

by Chuck Lever III

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

Hi Steve-

> On Apr 14, 2021, at 2:10 PM, Steve Dickson <[email protected]> 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).


> 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
>

2021-04-15 15:33:06

by Steve Dickson

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

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 <[email protected]> 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. 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?

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.

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... Actual I have customers asking for this type
of functionality

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
>>

2021-04-15 16:38:11

by Chuck Lever III

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



> On Apr 15, 2021, at 11:33 AM, Steve Dickson <[email protected]> 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 <[email protected]> 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.


> 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



2021-04-15 23:58:19

by Trond Myklebust

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

On Thu, 2021-04-15 at 16:37 +0000, Chuck Lever III wrote:
>
>
> > On Apr 15, 2021, at 11:33 AM, Steve Dickson <[email protected]>
> > 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 <[email protected]>
> > > > 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
[email protected]


2021-04-16 02:21:26

by Trond Myklebust

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

On Thu, 2021-04-15 at 23:30 +0000, Trond Myklebust wrote:
> On Thu, 2021-04-15 at 16:37 +0000, Chuck Lever III wrote:
> >
> >
> > > On Apr 15, 2021, at 11:33 AM, Steve Dickson <[email protected]>
> > > 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 <[email protected]>
> > > > > 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.
>

Here is a skeleton example:

[[email protected] rules.d]# cat /etc/udev/rules.d/50-nfs4.rules
ACTION=="add" KERNEL=="nfs_client" ATTR{identifier}=="(null)" PROGRAM="/usr/sbin/nfs4_uuid" ATTR{identifier}="%c"

[[email protected] rules.d]# cat /usr/sbin/nfs4_uuid
#!/bin/bash
#
if [ ! -f /etc/nfs4_uuid ]
then
uuid="$(uuidgen -r)"
echo -n ${uuid} > /etc/nfs4_uuid
else
uuid="$(cat /etc/nfs4_uuid)"
fi
echo ${uuid}


Obviously, the /usr/sbin/nfs4_uuid would need to be fleshed out a
little more to ensure that the file /etc/nfs4_uuid actually contains a
uuid in the right format, but you get the gist...

With the above additions, I end up with a repeatable

[[email protected] rules.d]# modprobe nfs4
[[email protected] rules.d]# cat /sys/fs/nfs/net/nfs_client/identifier
7f9f211b-0253-4ef8-a970-b1b0f600af02
[[email protected] rules.d]# cat /etc/nfs4_uuid
7f9f211b-0253-4ef8-a970-b1b0f600af02

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]


2021-04-17 16:17:22

by Steve Dickson

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



On 4/15/21 12:37 PM, Chuck Lever III wrote:
>
>
>> On Apr 15, 2021, at 11:33 AM, Steve Dickson <[email protected]> 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 <[email protected]> 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 request was to set that parameter...

>
>
>> 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.
You probably can say that for every /etc/*.conf file...
not being namespace aware... how can they be...

In the kernel document you say "Specifying a uniquifier string is
not support for NFS clients running in containers."

Why isn't that enough?

>
>
>> 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.
Point.

>
>
>> 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.
How does this break? If all the clients have unique ids
what breaks?

Or are you saying setting the unique id like this:

options nfs nfs4_unique_id="64fd26280451566d648ab0e0b7384421"

in modprobe.d/nfs.conf is not namespace safe?

>
>> 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.
If we shouldn't expose it... Lets get rid of it...
You added the param in the fall 2012... If it hasn't
been used correctly or can't be used correctly for
all theses years... why does it exist?

>
> 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.
I'll address this in Trond's reply...

thanks!

steved.

2021-04-17 16:32:00

by Steve Dickson

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

Hey!

On 4/15/21 8:40 PM, Trond Myklebust wrote:
> Here is a skeleton example:
>
> [[email protected] rules.d]# cat /etc/udev/rules.d/50-nfs4.rules
> ACTION=="add" KERNEL=="nfs_client" ATTR{identifier}=="(null)" PROGRAM="/usr/sbin/nfs4_uuid" ATTR{identifier}="%c"
>
> [[email protected] rules.d]# cat /usr/sbin/nfs4_uuid
> #!/bin/bash
> #
> if [ ! -f /etc/nfs4_uuid ]
> then
> uuid="$(uuidgen -r)"
> echo -n ${uuid} > /etc/nfs4_uuid
> else
> uuid="$(cat /etc/nfs4_uuid)"
> fi
> echo ${uuid}
>
>
> Obviously, the /usr/sbin/nfs4_uuid would need to be fleshed out a
> little more to ensure that the file /etc/nfs4_uuid actually contains a
> uuid in the right format, but you get the gist...
>
> With the above additions, I end up with a repeatable
>
> [[email protected] rules.d]# modprobe nfs4
> [[email protected] rules.d]# cat /sys/fs/nfs/net/nfs_client/identifier
> 7f9f211b-0253-4ef8-a970-b1b0f600af02
> [[email protected] rules.d]# cat /etc/nfs4_uuid
> 7f9f211b-0253-4ef8-a970-b1b0f600af02

I see that this example does populate nfs_client/identifier and
I'm sure we could beef up the mechanism but the may question
is this....

How does populating nfs_client/identifier via udev
actually setting the nfs4_unique_id parameter which is used to set
the unique id? I look and i've must have missed it...

If the answer is we need to change the client to look a
the nfs_client/identifier... then we should get rid of the
nfs4_unique_id param all together...

steved.

2021-04-17 16:36:28

by Chuck Lever III

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



> On Apr 17, 2021, at 12:18 PM, Steve Dickson <[email protected]> wrote:
>
>
>
> On 4/15/21 12:37 PM, Chuck Lever III wrote:
>>
>>
>>> On Apr 15, 2021, at 11:33 AM, Steve Dickson <[email protected]> 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 <[email protected]> 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 request was to set that parameter...

The cover letter and the quoted e-mail above state that
you are just demonstrating a mechanism to set module
parameters via nfs.conf. I guess that statement was not
entirely accurate, then. Thanks for clarifying.

If there's a bug report somewhere that requests a
feature, it's normal practice to include a URL pointing
to that report in the patch description.


>>> 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.
> You probably can say that for every /etc/*.conf file...
> not being namespace aware... how can they be...
>
> In the kernel document you say "Specifying a uniquifier string is
> not support for NFS clients running in containers."
>
> Why isn't that enough?

Because that statement is noting a limitation which is a
bug that has to be addressed to support NFSv4 properly in
containers.


>>> 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.
> How does this break? If all the clients have unique ids
> what breaks?
>
> Or are you saying setting the unique id like this:
>
> options nfs nfs4_unique_id="64fd26280451566d648ab0e0b7384421"
>
> in modprobe.d/nfs.conf is not namespace safe?

Setting the client_id via module parameter is not
namespace-aware. That's the bug that the sysfs/udev
contraption is designed to fix.


>>> 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.
> If we shouldn't expose it... Lets get rid of it...
> You added the param in the fall 2012... If it hasn't
> been used correctly or can't be used correctly for
> all theses years... why does it exist?

Because back then we didn't care about container awareness
enough to make it an initial part of the feature. We were
trying to address the problem of how to ensure that the
nfs4_client_id is unique. But clearly it was only half a
solution.

The module parameter still exists because the general rule
about these things is that module parameters are part of the
kernel API, and can't just be removed. If there's a process
for removing it, then I would agree to that now that we have
a sysfs API to manage the nfs4_client_id setting.


>> 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.
> I'll address this in Trond's reply...
>
> thanks!
>
> steved.

--
Chuck Lever



2021-04-17 17:51:29

by Steve Dickson

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



On 4/17/21 12:36 PM, Chuck Lever III wrote:
>
>
>> On Apr 17, 2021, at 12:18 PM, Steve Dickson <[email protected]> wrote:
>>
>>
>>
>> On 4/15/21 12:37 PM, Chuck Lever III wrote:
>>>
>>>
>>>> On Apr 15, 2021, at 11:33 AM, Steve Dickson <[email protected]> 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 <[email protected]> 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 request was to set that parameter...
>
> The cover letter and the quoted e-mail above state that
> you are just demonstrating a mechanism to set module
> parameters via nfs.conf. I guess that statement was not
> entirely accurate, then. Thanks for clarifying.
I was trying to keep the conversation off of what
was being set to how it was being set...

I had no idea the entire "options nfs" API is compromised
when it comes to containers... Not sure why... but I will
believe you... But is the API compromised in a
non-container env? Other than the cloud, isn't non-containers
envs the majority of our install based?

>
> If there's a bug report somewhere that requests a
> feature, it's normal practice to include a URL pointing
> to that report in the patch description.
I just assumed, since it had a customer case, the bz was
private... it turns out not to be the case
https://bugzilla.redhat.com/show_bug.cgi?id=1801326

>
>
>>>> 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.
>> You probably can say that for every /etc/*.conf file...
>> not being namespace aware... how can they be...
>>
>> In the kernel document you say "Specifying a uniquifier string is
>> not support for NFS clients running in containers."
>>
>> Why isn't that enough?
>
> Because that statement is noting a limitation which is a
> bug that has to be addressed to support NFSv4 properly in
> containers.
It seems to me we could use similar documentation when
setting the id from nfs.conf.

>
>
>>>> 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.
>> How does this break? If all the clients have unique ids
>> what breaks?
>>
>> Or are you saying setting the unique id like this:
>>
>> options nfs nfs4_unique_id="64fd26280451566d648ab0e0b7384421"
>>
>> in modprobe.d/nfs.conf is not namespace safe?
>
> Setting the client_id via module parameter is not
> namespace-aware. That's the bug that the sysfs/udev
> contraption is designed to fix.
Fair enough... So we know the sysfs/udev is
actually setting the id?

>
>
>>>> 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.
>> If we shouldn't expose it... Lets get rid of it...
>> You added the param in the fall 2012... If it hasn't
>> been used correctly or can't be used correctly for
>> all theses years... why does it exist?
>
> Because back then we didn't care about container awareness
> enough to make it an initial part of the feature. We were
> trying to address the problem of how to ensure that the
> nfs4_client_id is unique. But clearly it was only half a
> solution.
Right... I was just trying to build a mechanism to
set the value, and not worrying (yet) about what the
value is set to...

So at this point, all of the nfs kernel module parameter
can be set through the sysfs/udev interface?

If that is the cause... have the variables in
nfs.conf create sysfs/udev file would work better?

steved.

>
> The module parameter still exists because the general rule
> about these things is that module parameters are part of the
> kernel API, and can't just be removed. If there's a process
> for removing it, then I would agree to that now that we have
> a sysfs API to manage the nfs4_client_id setting.
>
>
>>> 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.
>> I'll address this in Trond's reply...
>>
>> thanks!
>>
>> steved.
>
> --
> Chuck Lever
>
>
>

2021-04-17 18:12:06

by Trond Myklebust

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

On Sat, 2021-04-17 at 12:33 -0400, Steve Dickson wrote:
> Hey!
>
> On 4/15/21 8:40 PM, Trond Myklebust wrote:
> > Here is a skeleton example:
> >
> > [[email protected] rules.d]# cat /etc/udev/rules.d/50-nfs4.rules
> > ACTION=="add" KERNEL=="nfs_client" ATTR{identifier}=="(null)"
> > PROGRAM="/usr/sbin/nfs4_uuid" ATTR{identifier}="%c"
> >
> > [[email protected] rules.d]# cat /usr/sbin/nfs4_uuid
> > #!/bin/bash
> > #
> > if [ ! -f /etc/nfs4_uuid ]
> > then
> >         uuid="$(uuidgen -r)"
> >         echo -n ${uuid} > /etc/nfs4_uuid
> > else
> >         uuid="$(cat /etc/nfs4_uuid)"
> > fi
> > echo ${uuid}
> >
> >
> > Obviously, the /usr/sbin/nfs4_uuid would need to be fleshed out a
> > little more to ensure that the file /etc/nfs4_uuid actually
> > contains a
> > uuid in the right format, but you get the gist...
> >
> > With the above additions, I end up with a repeatable
> >
> > [[email protected] rules.d]# modprobe nfs4
> > [[email protected] rules.d]# cat /sys/fs/nfs/net/nfs_client/identifier
> > 7f9f211b-0253-4ef8-a970-b1b0f600af02
> > [[email protected] rules.d]# cat /etc/nfs4_uuid
> > 7f9f211b-0253-4ef8-a970-b1b0f600af02
>
> I see that this example does populate nfs_client/identifier and
> I'm sure we could beef up the mechanism but the may question
> is this....
>
> How does populating nfs_client/identifier via udev
> actually setting the nfs4_unique_id parameter which is used to set
> the unique id? I look and i've must have missed it...
>
> If the answer is we need to change the client to look a
> the nfs_client/identifier... then we should get rid of the
> nfs4_unique_id param all together...
>

Commit 39d43d164127 ("NFSv4: Use the net namespace uniquifier if it is
set") should default to using the nfs_client identifier if it is set.
Otherwise it falls back to using the nfs4_unique_id. So kernels >= 5.10
are ready to use this udev-based mechanism.

The reason why I added udev support is, as I said, because we need
something that works correctly inside containers. Unfortunately, module
parameters are system-wide, so the older mechanism works just fine
right up to the moment where you fire up 'docker', 'kubernetes' or
'podman' (which is an increasingly important use case for NFS).

We do need something a little more sophisticated than the naive script
that I provided. As I said, that was intended as a skeleton example
just to demonstrate the basic mechanism and how to configure udev. It
would be very good to have something similar to what I showed there be
installed by default when you install nfs-utils.

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]


2021-04-18 16:51:47

by Chuck Lever III

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



> On Apr 17, 2021, at 1:50 PM, Steve Dickson <[email protected]> wrote:
>
>
>
> On 4/17/21 12:36 PM, Chuck Lever III wrote:
>>
>>
>>> On Apr 17, 2021, at 12:18 PM, Steve Dickson <[email protected]> wrote:
>>>
>>>
>>>
>>> On 4/15/21 12:37 PM, Chuck Lever III wrote:
>>>>
>>>>
>>>>> On Apr 15, 2021, at 11:33 AM, Steve Dickson <[email protected]> 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 <[email protected]> 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 request was to set that parameter...
>>
>> The cover letter and the quoted e-mail above state that
>> you are just demonstrating a mechanism to set module
>> parameters via nfs.conf. I guess that statement was not
>> entirely accurate, then. Thanks for clarifying.
> I was trying to keep the conversation off of what
> was being set to how it was being set...
>
> I had no idea the entire "options nfs" API is compromised
> when it comes to containers... Not sure why... but I will
> believe you...

Module parameters take effect for all namespaces. It's
not just nfs.ko that has this issue.


>> If there's a bug report somewhere that requests a
>> feature, it's normal practice to include a URL pointing
>> to that report in the patch description.
> I just assumed, since it had a customer case, the bz was
> private... it turns out not to be the case
> https://bugzilla.redhat.com/show_bug.cgi?id=1801326

>>>>> Actual I have customers asking for this type
>>>>> of functionality

Hrm. The reporter of 1801326 is dwyso, a Red Hat employee,
not a customer.

Also, I see nothing that requires it to be set in nfs.conf.
So what actual functionality is being requested, and why
can't it be addressed with a udev script, which has been
the design already in the works for many months?


>>>> 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.
>>> If we shouldn't expose it... Lets get rid of it...
>>> You added the param in the fall 2012... If it hasn't
>>> been used correctly or can't be used correctly for
>>> all theses years... why does it exist?
>>
>> Because back then we didn't care about container awareness
>> enough to make it an initial part of the feature. We were
>> trying to address the problem of how to ensure that the
>> nfs4_client_id is unique. But clearly it was only half a
>> solution.
> Right... I was just trying to build a mechanism to
> set the value, and not worrying (yet) about what the
> value is set to...
>
> So at this point, all of the nfs kernel module parameter
> can be set through the sysfs/udev interface?

The only module parameter that has been explicitly replaced
is the one that sets nfs4_client_id, as far as I am aware.
There might be some other settings available in /sys:

# ls /sys/module/nfsv4/parameters
delegation_watermark layoutstats_timer
#

[[email protected] linux]$ git grep MODULE_PARM -- fs/nfs/
fs/nfs/cache_lib.c:MODULE_PARM_DESC(cache_getent, "Path to the client cache upcall program");
fs/nfs/cache_lib.c:MODULE_PARM_DESC(cache_getent_timeout, "Timeout (in seconds) after which "
fs/nfs/dir.c:MODULE_PARM_DESC(nfs_access_max_cachesize, "NFS access maximum total cache length");
fs/nfs/filelayout/filelayoutdev.c:MODULE_PARM_DESC(dataserver_retrans, "The number of times the NFSv4.1 client "
fs/nfs/filelayout/filelayoutdev.c:MODULE_PARM_DESC(dataserver_timeo, "The time (in tenths of a second) the "
fs/nfs/flexfilelayout/flexfilelayout.c:MODULE_PARM_DESC(io_maxretrans, "The number of times the NFSv4.1 client "
fs/nfs/flexfilelayout/flexfilelayoutdev.c:MODULE_PARM_DESC(dataserver_retrans, "The number of times the NFSv4.1 client "
fs/nfs/flexfilelayout/flexfilelayoutdev.c:MODULE_PARM_DESC(dataserver_timeo, "The time (in tenths of a second) the "
fs/nfs/namespace.c:MODULE_PARM_DESC(nfs_mountpoint_expiry_timeout,
fs/nfs/super.c:MODULE_PARM_DESC(callback_nr_threads, "Number of threads that will be "
fs/nfs/super.c:MODULE_PARM_DESC(nfs4_disable_idmapping,
fs/nfs/super.c:MODULE_PARM_DESC(max_session_slots, "Maximum number of outstanding NFSv4.1 "
fs/nfs/super.c:MODULE_PARM_DESC(max_session_cb_slots, "Maximum number of parallel NFSv4.1 "
fs/nfs/super.c:MODULE_PARM_DESC(send_implementation_id,
fs/nfs/super.c:MODULE_PARM_DESC(nfs4_unique_id, "nfs_client_id4 uniquifier string");
fs/nfs/super.c:MODULE_PARM_DESC(recover_lost_locks,
[[email protected] linux]$ git grep MODULE_PARM -- fs/nfsd/
fs/nfsd/nfs4idmap.c:MODULE_PARM_DESC(nfs4_disable_idmapping,
fs/nfsd/nfs4proc.c:MODULE_PARM_DESC(inter_copy_offload_enable,
fs/nfsd/nfs4recover.c:MODULE_PARM_DESC(cltrack_prog, "Path to the nfsdcltrack upcall program");
fs/nfsd/nfs4recover.c:MODULE_PARM_DESC(cltrack_legacy_disable,
[[email protected] linux]$

Each of the above has to be considered on a case-by-case
basis, IMO.


> If that is the cause... have the variables in
> nfs.conf create sysfs/udev file would work better?

Seems to me we have the same argument for a separate file
to store the nfs4_unique_id that we have for breaking
/etc/exports into a directory of individual files: no
parsing is necessary. Scripts and applications can simply
read the string right out of the file, or update it just
by writing the string into that file.

Conversely, there's no clear need for administrators to be
aware of the setting, once it is being set properly.


--
Chuck Lever



2021-04-20 13:09:27

by Steve Dickson

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



On 4/18/21 12:51 PM, Chuck Lever III wrote:
>
>
>> On Apr 17, 2021, at 1:50 PM, Steve Dickson <[email protected]> wrote:
>>
>>
>>
>> On 4/17/21 12:36 PM, Chuck Lever III wrote:
>>>
>>>
>>>> On Apr 17, 2021, at 12:18 PM, Steve Dickson <[email protected]> wrote:
>>>>
>>>>
>>>>
>>>> On 4/15/21 12:37 PM, Chuck Lever III wrote:
>>>>>
>>>>>
>>>>>> On Apr 15, 2021, at 11:33 AM, Steve Dickson <[email protected]> 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 <[email protected]> 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 request was to set that parameter...
>>>
>>> The cover letter and the quoted e-mail above state that
>>> you are just demonstrating a mechanism to set module
>>> parameters via nfs.conf. I guess that statement was not
>>> entirely accurate, then. Thanks for clarifying.
>> I was trying to keep the conversation off of what
>> was being set to how it was being set...
>>
>> I had no idea the entire "options nfs" API is compromised
>> when it comes to containers... Not sure why... but I will
>> believe you...
>
> Module parameters take effect for all namespaces. It's
> not just nfs.ko that has this issue.
Right...
>
>
>>> If there's a bug report somewhere that requests a
>>> feature, it's normal practice to include a URL pointing
>>> to that report in the patch description.
>> I just assumed, since it had a customer case, the bz was
>> private... it turns out not to be the case
>> https://bugzilla.redhat.com/show_bug.cgi?id=1801326
>
>>>>>> Actual I have customers asking for this type
>>>>>> of functionality
>
> Hrm. The reporter of 1801326 is dwyso, a Red Hat employee,
> not a customer.
>
> Also, I see nothing that requires it to be set in nfs.conf.
> So what actual functionality is being requested, and why
> can't it be addressed with a udev script, which has been
> the design already in the works for many months?
The bz was open by a request of a customer... There is a
lot of info in bzs that are not public...

Having a one, centralized place to configure NFS
I thought was a good idea...

>
>
>>>>> 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.
>>>> If we shouldn't expose it... Lets get rid of it...
>>>> You added the param in the fall 2012... If it hasn't
>>>> been used correctly or can't be used correctly for
>>>> all theses years... why does it exist?
>>>
>>> Because back then we didn't care about container awareness
>>> enough to make it an initial part of the feature. We were
>>> trying to address the problem of how to ensure that the
>>> nfs4_client_id is unique. But clearly it was only half a
>>> solution.
>> Right... I was just trying to build a mechanism to
>> set the value, and not worrying (yet) about what the
>> value is set to...
>>
>> So at this point, all of the nfs kernel module parameter
>> can be set through the sysfs/udev interface?
>
> The only module parameter that has been explicitly replaced
> is the one that sets nfs4_client_id, as far as I am aware.
> There might be some other settings available in /sys:
>
> # ls /sys/module/nfsv4/parameters
> delegation_watermark layoutstats_timer
> #
>
> [[email protected] linux]$ git grep MODULE_PARM -- fs/nfs/
> fs/nfs/cache_lib.c:MODULE_PARM_DESC(cache_getent, "Path to the client cache upcall program");
> fs/nfs/cache_lib.c:MODULE_PARM_DESC(cache_getent_timeout, "Timeout (in seconds) after which "
> fs/nfs/dir.c:MODULE_PARM_DESC(nfs_access_max_cachesize, "NFS access maximum total cache length");
> fs/nfs/filelayout/filelayoutdev.c:MODULE_PARM_DESC(dataserver_retrans, "The number of times the NFSv4.1 client "
> fs/nfs/filelayout/filelayoutdev.c:MODULE_PARM_DESC(dataserver_timeo, "The time (in tenths of a second) the "
> fs/nfs/flexfilelayout/flexfilelayout.c:MODULE_PARM_DESC(io_maxretrans, "The number of times the NFSv4.1 client "
> fs/nfs/flexfilelayout/flexfilelayoutdev.c:MODULE_PARM_DESC(dataserver_retrans, "The number of times the NFSv4.1 client "
> fs/nfs/flexfilelayout/flexfilelayoutdev.c:MODULE_PARM_DESC(dataserver_timeo, "The time (in tenths of a second) the "
> fs/nfs/namespace.c:MODULE_PARM_DESC(nfs_mountpoint_expiry_timeout,
> fs/nfs/super.c:MODULE_PARM_DESC(callback_nr_threads, "Number of threads that will be "
> fs/nfs/super.c:MODULE_PARM_DESC(nfs4_disable_idmapping,
> fs/nfs/super.c:MODULE_PARM_DESC(max_session_slots, "Maximum number of outstanding NFSv4.1 "
> fs/nfs/super.c:MODULE_PARM_DESC(max_session_cb_slots, "Maximum number of parallel NFSv4.1 "
> fs/nfs/super.c:MODULE_PARM_DESC(send_implementation_id,
> fs/nfs/super.c:MODULE_PARM_DESC(nfs4_unique_id, "nfs_client_id4 uniquifier string");
> fs/nfs/super.c:MODULE_PARM_DESC(recover_lost_locks,
> [[email protected] linux]$ git grep MODULE_PARM -- fs/nfsd/
> fs/nfsd/nfs4idmap.c:MODULE_PARM_DESC(nfs4_disable_idmapping,
> fs/nfsd/nfs4proc.c:MODULE_PARM_DESC(inter_copy_offload_enable,
> fs/nfsd/nfs4recover.c:MODULE_PARM_DESC(cltrack_prog, "Path to the nfsdcltrack upcall program");
> fs/nfsd/nfs4recover.c:MODULE_PARM_DESC(cltrack_legacy_disable,
> [[email protected] linux]$
>
> Each of the above has to be considered on a case-by-case
> basis, IMO.
>
>
>> If that is the cause... have the variables in
>> nfs.conf create sysfs/udev file would work better?
>
> Seems to me we have the same argument for a separate file
> to store the nfs4_unique_id that we have for breaking
> /etc/exports into a directory of individual files: no
> parsing is necessary. Scripts and applications can simply
> read the string right out of the file, or update it just
> by writing the string into that file.
I'm sure I'm following this analogy with the export...
but being able to set things up from one configuration
file and command is key.

nfsconf does an excellent job parsing nfs.conf and I would
like to build on that...

I understand we have to work well in containers which
is one of the reason I was trying to come up with a
nfsv4 only nfs-utils... That went over like a lead balloon ;-)

What I don't understand is why we can't come up with a
solution that uniquely set a param that is set by
nfsconf using nfs.conf.

steved.
>
> Conversely, there's no clear need for administrators to be
> aware of the setting, once it is being set properly.
>
>
> --
> Chuck Lever
>
>
>

2021-04-20 14:10:36

by Chuck Lever III

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



> On Apr 20, 2021, at 9:11 AM, Steve Dickson <[email protected]> wrote:
>
>
>
> On 4/18/21 12:51 PM, Chuck Lever III wrote:
>>
>>
>>> On Apr 17, 2021, at 1:50 PM, Steve Dickson <[email protected]> wrote:
>>>
>>>
>>>
>>> On 4/17/21 12:36 PM, Chuck Lever III wrote:
>>>>
>>>>
>>>>> On Apr 17, 2021, at 12:18 PM, Steve Dickson <[email protected]> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 4/15/21 12:37 PM, Chuck Lever III wrote:
>>>>>>
>>>>>>
>>>>>>> On Apr 15, 2021, at 11:33 AM, Steve Dickson <[email protected]> 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 <[email protected]> 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 request was to set that parameter...
>>>>
>>>> The cover letter and the quoted e-mail above state that
>>>> you are just demonstrating a mechanism to set module
>>>> parameters via nfs.conf. I guess that statement was not
>>>> entirely accurate, then. Thanks for clarifying.
>>> I was trying to keep the conversation off of what
>>> was being set to how it was being set...
>>>
>>> I had no idea the entire "options nfs" API is compromised
>>> when it comes to containers... Not sure why... but I will
>>> believe you...
>>
>> Module parameters take effect for all namespaces. It's
>> not just nfs.ko that has this issue.
> Right...
>>
>>
>>>> If there's a bug report somewhere that requests a
>>>> feature, it's normal practice to include a URL pointing
>>>> to that report in the patch description.
>>> I just assumed, since it had a customer case, the bz was
>>> private... it turns out not to be the case
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1801326
>>
>>>>>>> Actual I have customers asking for this type
>>>>>>> of functionality
>>
>> Hrm. The reporter of 1801326 is dwyso, a Red Hat employee,
>> not a customer.
>>
>> Also, I see nothing that requires it to be set in nfs.conf.
>> So what actual functionality is being requested, and why
>> can't it be addressed with a udev script, which has been
>> the design already in the works for many months?
> The bz was open by a request of a customer... There is a
> lot of info in bzs that are not public...
>
> Having a one, centralized place to configure NFS
> I thought was a good idea...
>
>>
>>
>>>>>> 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.
>>>>> If we shouldn't expose it... Lets get rid of it...
>>>>> You added the param in the fall 2012... If it hasn't
>>>>> been used correctly or can't be used correctly for
>>>>> all theses years... why does it exist?
>>>>
>>>> Because back then we didn't care about container awareness
>>>> enough to make it an initial part of the feature. We were
>>>> trying to address the problem of how to ensure that the
>>>> nfs4_client_id is unique. But clearly it was only half a
>>>> solution.
>>> Right... I was just trying to build a mechanism to
>>> set the value, and not worrying (yet) about what the
>>> value is set to...
>>>
>>> So at this point, all of the nfs kernel module parameter
>>> can be set through the sysfs/udev interface?
>>
>> The only module parameter that has been explicitly replaced
>> is the one that sets nfs4_client_id, as far as I am aware.
>> There might be some other settings available in /sys:
>>
>> # ls /sys/module/nfsv4/parameters
>> delegation_watermark layoutstats_timer
>> #
>>
>> [[email protected] linux]$ git grep MODULE_PARM -- fs/nfs/
>> fs/nfs/cache_lib.c:MODULE_PARM_DESC(cache_getent, "Path to the client cache upcall program");
>> fs/nfs/cache_lib.c:MODULE_PARM_DESC(cache_getent_timeout, "Timeout (in seconds) after which "
>> fs/nfs/dir.c:MODULE_PARM_DESC(nfs_access_max_cachesize, "NFS access maximum total cache length");
>> fs/nfs/filelayout/filelayoutdev.c:MODULE_PARM_DESC(dataserver_retrans, "The number of times the NFSv4.1 client "
>> fs/nfs/filelayout/filelayoutdev.c:MODULE_PARM_DESC(dataserver_timeo, "The time (in tenths of a second) the "
>> fs/nfs/flexfilelayout/flexfilelayout.c:MODULE_PARM_DESC(io_maxretrans, "The number of times the NFSv4.1 client "
>> fs/nfs/flexfilelayout/flexfilelayoutdev.c:MODULE_PARM_DESC(dataserver_retrans, "The number of times the NFSv4.1 client "
>> fs/nfs/flexfilelayout/flexfilelayoutdev.c:MODULE_PARM_DESC(dataserver_timeo, "The time (in tenths of a second) the "
>> fs/nfs/namespace.c:MODULE_PARM_DESC(nfs_mountpoint_expiry_timeout,
>> fs/nfs/super.c:MODULE_PARM_DESC(callback_nr_threads, "Number of threads that will be "
>> fs/nfs/super.c:MODULE_PARM_DESC(nfs4_disable_idmapping,
>> fs/nfs/super.c:MODULE_PARM_DESC(max_session_slots, "Maximum number of outstanding NFSv4.1 "
>> fs/nfs/super.c:MODULE_PARM_DESC(max_session_cb_slots, "Maximum number of parallel NFSv4.1 "
>> fs/nfs/super.c:MODULE_PARM_DESC(send_implementation_id,
>> fs/nfs/super.c:MODULE_PARM_DESC(nfs4_unique_id, "nfs_client_id4 uniquifier string");
>> fs/nfs/super.c:MODULE_PARM_DESC(recover_lost_locks,
>> [[email protected] linux]$ git grep MODULE_PARM -- fs/nfsd/
>> fs/nfsd/nfs4idmap.c:MODULE_PARM_DESC(nfs4_disable_idmapping,
>> fs/nfsd/nfs4proc.c:MODULE_PARM_DESC(inter_copy_offload_enable,
>> fs/nfsd/nfs4recover.c:MODULE_PARM_DESC(cltrack_prog, "Path to the nfsdcltrack upcall program");
>> fs/nfsd/nfs4recover.c:MODULE_PARM_DESC(cltrack_legacy_disable,
>> [[email protected] linux]$
>>
>> Each of the above has to be considered on a case-by-case
>> basis, IMO.
>>
>>
>>> If that is the cause... have the variables in
>>> nfs.conf create sysfs/udev file would work better?
>>
>> Seems to me we have the same argument for a separate file
>> to store the nfs4_unique_id that we have for breaking
>> /etc/exports into a directory of individual files: no
>> parsing is necessary. Scripts and applications can simply
>> read the string right out of the file, or update it just
>> by writing the string into that file.
> I'm sure I'm following this analogy with the export...
> but being able to set things up from one configuration
> file and command is key.

I find that to be a red herring. We're not ever going to be
at a place where everything is configured in one file. Are
you going to replace /etc/exports.d and automounter.conf
and krb5.conf and all the other things with just /etc/nfs.conf?
Probably not. So let's stop using this strange logic to
insist that /etc/nfs.conf is the only place for the clientid
uniqifier.

Please, let's just focus on what's right for this one setting.


> nfsconf does an excellent job parsing nfs.conf and I would
> like to build on that...

Just because it is technically possible to save the uniqifier
in /etc/nfs.conf doesn't mean that's what's best for our users.
We're in better shape if we can guarantee that setting is
correct and administrators can't break it.


> I understand we have to work well in containers which
> is one of the reason I was trying to come up with a
> nfsv4 only nfs-utils... That went over like a lead balloon ;-)

I didn't have a problem with an nfsv4-only configuration.
What I didn't like about that is that you were making the
administrative interface more complex, and it didn't need to
be.


> What I don't understand is why we can't come up with a
> solution that uniquely set a param that is set by
> nfsconf using nfs.conf.

Once we have an automated mechanism to set the uniqifier,
it does not need to be set by humans. Let's keep it out of
nfs.conf.

I'm in favor of making this as automatic as possible. No
setting is better than an exposed setting that is never
touched.

I prefer no change to nfs.conf, and put the uniqifier in a
separate file.


--
Chuck Lever



2021-04-20 14:32:29

by Trond Myklebust

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

On Tue, 2021-04-20 at 14:09 +0000, Chuck Lever III wrote:
>
>
> > On Apr 20, 2021, at 9:11 AM, Steve Dickson <[email protected]>
> > wrote:
> >
> >
> >
> > On 4/18/21 12:51 PM, Chuck Lever III wrote:
> > >
> > >
> > > > On Apr 17, 2021, at 1:50 PM, Steve Dickson <[email protected]>
> > > > wrote:
> > > >
> > > >
> > > >
> > > > On 4/17/21 12:36 PM, Chuck Lever III wrote:
> > > > >
> > > > >
> > > > > > On Apr 17, 2021, at 12:18 PM, Steve Dickson <
> > > > > > [email protected]> wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > On 4/15/21 12:37 PM, Chuck Lever III wrote:
> > > > > > >
> > > > > > >
> > > > > > > > On Apr 15, 2021, at 11:33 AM, Steve Dickson <
> > > > > > > > [email protected]> 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 <
> > > > > > > > > > [email protected]> 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 request was to set that parameter...
> > > > >
> > > > > The cover letter and the quoted e-mail above state that
> > > > > you are just demonstrating a mechanism to set module
> > > > > parameters via nfs.conf. I guess that statement was not
> > > > > entirely accurate, then. Thanks for clarifying.
> > > > I was trying to keep the conversation off of what
> > > > was being set to how it was being set...
> > > >
> > > > I had no idea the entire "options nfs" API is compromised
> > > > when it comes to containers... Not sure why... but I will
> > > > believe you...
> > >
> > > Module parameters take effect for all namespaces. It's
> > > not just nfs.ko that has this issue.
> > Right...
> > >
> > >
> > > > > If there's a bug report somewhere that requests a
> > > > > feature, it's normal practice to include a URL pointing
> > > > > to that report in the patch description.
> > > > I just assumed, since it had a customer case, the bz was
> > > > private... it turns out not to be the case
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=1801326
> > >
> > > > > > > > Actual I have customers asking for this type
> > > > > > > > of functionality
> > >
> > > Hrm. The reporter of 1801326 is dwyso, a Red Hat employee,
> > > not a customer.
> > >
> > > Also, I see nothing that requires it to be set in nfs.conf.
> > > So what actual functionality is being requested, and why
> > > can't it be addressed with a udev script, which has been
> > > the design already in the works for many months?
> > The bz was open by a request of a customer... There is a
> > lot of info in bzs that are not public...
> >
> > Having a one, centralized place to configure NFS
> > I thought was a good idea...
> >
> > >
> > >
> > > > > > > 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.
> > > > > > If we shouldn't expose it... Lets get rid of it...
> > > > > > You added the param in the fall 2012... If it hasn't
> > > > > > been used correctly or can't be used correctly for
> > > > > > all theses years... why does it exist?
> > > > >
> > > > > Because back then we didn't care about container awareness
> > > > > enough to make it an initial part of the feature. We were
> > > > > trying to address the problem of how to ensure that the
> > > > > nfs4_client_id is unique. But clearly it was only half a
> > > > > solution.
> > > > Right... I was just trying to build a mechanism to
> > > > set the value, and not worrying (yet) about what the
> > > > value is set to...
> > > >
> > > > So at this point, all of the nfs kernel module parameter
> > > > can be set through the sysfs/udev interface?
> > >
> > > The only module parameter that has been explicitly replaced
> > > is the one that sets nfs4_client_id, as far as I am aware.
> > > There might be some other settings available in /sys:
> > >
> > > # ls /sys/module/nfsv4/parameters
> > > delegation_watermark  layoutstats_timer
> > > #
> > >
> > > [[email protected] linux]$ git grep MODULE_PARM -- fs/nfs/
> > > fs/nfs/cache_lib.c:MODULE_PARM_DESC(cache_getent, "Path to the
> > > client cache upcall program");
> > > fs/nfs/cache_lib.c:MODULE_PARM_DESC(cache_getent_timeout,
> > > "Timeout (in seconds) after which "
> > > fs/nfs/dir.c:MODULE_PARM_DESC(nfs_access_max_cachesize, "NFS
> > > access maximum total cache length");
> > > fs/nfs/filelayout/filelayoutdev.c:MODULE_PARM_DESC(dataserver_ret
> > > rans, "The  number of times the NFSv4.1 client "
> > > fs/nfs/filelayout/filelayoutdev.c:MODULE_PARM_DESC(dataserver_tim
> > > eo, "The time (in tenths of a second) the "
> > > fs/nfs/flexfilelayout/flexfilelayout.c:MODULE_PARM_DESC(io_maxret
> > > rans, "The  number of times the NFSv4.1 client "
> > > fs/nfs/flexfilelayout/flexfilelayoutdev.c:MODULE_PARM_DESC(datase
> > > rver_retrans, "The  number of times the NFSv4.1 client "
> > > fs/nfs/flexfilelayout/flexfilelayoutdev.c:MODULE_PARM_DESC(datase
> > > rver_timeo, "The time (in tenths of a second) the "
> > > fs/nfs/namespace.c:MODULE_PARM_DESC(nfs_mountpoint_expiry_timeout
> > > ,
> > > fs/nfs/super.c:MODULE_PARM_DESC(callback_nr_threads, "Number of
> > > threads that will be "
> > > fs/nfs/super.c:MODULE_PARM_DESC(nfs4_disable_idmapping,
> > > fs/nfs/super.c:MODULE_PARM_DESC(max_session_slots, "Maximum
> > > number of outstanding NFSv4.1 "
> > > fs/nfs/super.c:MODULE_PARM_DESC(max_session_cb_slots, "Maximum
> > > number of parallel NFSv4.1 "
> > > fs/nfs/super.c:MODULE_PARM_DESC(send_implementation_id,
> > > fs/nfs/super.c:MODULE_PARM_DESC(nfs4_unique_id, "nfs_client_id4
> > > uniquifier string");
> > > fs/nfs/super.c:MODULE_PARM_DESC(recover_lost_locks,
> > > [[email protected] linux]$ git grep MODULE_PARM -- fs/nfsd/
> > > fs/nfsd/nfs4idmap.c:MODULE_PARM_DESC(nfs4_disable_idmapping,
> > > fs/nfsd/nfs4proc.c:MODULE_PARM_DESC(inter_copy_offload_enable,
> > > fs/nfsd/nfs4recover.c:MODULE_PARM_DESC(cltrack_prog, "Path to the
> > > nfsdcltrack upcall program");
> > > fs/nfsd/nfs4recover.c:MODULE_PARM_DESC(cltrack_legacy_disable,
> > > [[email protected] linux]$
> > >
> > > Each of the above has to be considered on a case-by-case
> > > basis, IMO.
> > >
> > >
> > > > If that is the cause... have the variables in
> > > > nfs.conf create sysfs/udev file would work better?
> > >
> > > Seems to me we have the same argument for a separate file
> > > to store the nfs4_unique_id that we have for breaking
> > > /etc/exports into a directory of individual files: no
> > > parsing is necessary. Scripts and applications can simply
> > > read the string right out of the file, or update it just
> > > by writing the string into that file.
> > I'm sure I'm following this analogy with the export...
> > but being able to set things up from one configuration
> > file and command is key.
>
> I find that to be a red herring. We're not ever going to be
> at a place where everything is configured in one file. Are
> you going to replace /etc/exports.d and automounter.conf
> and krb5.conf and all the other things with just /etc/nfs.conf?
> Probably not. So let's stop using this strange logic to
> insist that /etc/nfs.conf is the only place for the clientid
> uniqifier.
>
> Please, let's just focus on what's right for this one setting.
>
>
> > nfsconf does an excellent job parsing nfs.conf and I would
> > like to build on that...
>
> Just because it is technically possible to save the uniqifier
> in /etc/nfs.conf doesn't mean that's what's best for our users.
> We're in better shape if we can guarantee that setting is
> correct and administrators can't break it.
>
>
> > I understand we have to work well in containers which
> > is one of the reason I was trying to come up with a
> > nfsv4 only nfs-utils... That went over like a lead balloon ;-)
>
> I didn't have a problem with an nfsv4-only configuration.
> What I didn't like about that is that you were making the
> administrative interface more complex, and it didn't need to
> be.
>
>
> > What I don't understand is why we can't come up with a
> > solution that uniquely set a param that is set by
> > nfsconf using nfs.conf.
>
> Once we have an automated mechanism to set the uniqifier,
> it does not need to be set by humans. Let's keep it out of
> nfs.conf.
>
> I'm in favor of making this as automatic as possible. No
> setting is better than an exposed setting that is never
> touched.
>
> I prefer no change to nfs.conf, and put the uniqifier in a
> separate file.
>

I think the important thing is, as Chuck said, that the setting of the
uniquifier has to be automated. There are too many instances out there
of people who get confused because they are using a default hostname,
such as 'localhost.localdomain' and are setting no uniquifier.

So the point is that it needs to be persisted by an automated script if
unset.

While that script could use nfsconf to get/set the persisted
uniquifier, the worry is that such an automated change might be made
while the user is performing some other edit of nfs.conf. What happens
then?

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]


2021-04-20 17:18:26

by Bruce Fields

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

On Tue, Apr 20, 2021 at 02:31:58PM +0000, Trond Myklebust wrote:
> I think the important thing is, as Chuck said, that the setting of the
> uniquifier has to be automated. There are too many instances out there
> of people who get confused because they are using a default hostname,
> such as 'localhost.localdomain' and are setting no uniquifier.
>
> So the point is that it needs to be persisted by an automated script if
> unset.
>
> While that script could use nfsconf to get/set the persisted
> uniquifier, the worry is that such an automated change might be made
> while the user is performing some other edit of nfs.conf. What happens
> then?

The one thing I'm a little uneasy about is ignoring /etc/machine-id.
Seems like distros *should* be creating it for us. And it would be
convenient to have one source of machine identity rather than separate
ones for different subsystems.

Maybe we could use that if it exists, and fall back on generating our
own only if it doesn't?

(Well, where "use it" actually means take a hash of it, as explained in
machine-id(5).)

--b.

2021-04-20 17:29:05

by Trond Myklebust

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

On Tue, 2021-04-20 at 13:18 -0400, J. Bruce Fields wrote:
> On Tue, Apr 20, 2021 at 02:31:58PM +0000, Trond Myklebust wrote:
> > I think the important thing is, as Chuck said, that the setting of
> > the
> > uniquifier has to be automated. There are too many instances out
> > there
> > of people who get confused because they are using a default
> > hostname,
> > such as 'localhost.localdomain' and are setting no uniquifier.
> >
> > So the point is that it needs to be persisted by an automated
> > script if
> > unset.
> >
> > While that script could use nfsconf to get/set the persisted
> > uniquifier, the worry is that such an automated change might be
> > made
> > while the user is performing some other edit of nfs.conf. What
> > happens
> > then?
>
> The one thing I'm a little uneasy about is ignoring /etc/machine-id.
> Seems like distros *should* be creating it for us.  And it would be
> convenient to have one source of machine identity rather than
> separate
> ones for different subsystems.
>
> Maybe we could use that if it exists, and fall back on generating our
> own only if it doesn't?
>
> (Well, where "use it" actually means take a hash of it, as explained
> in
> machine-id(5).)
>

Maybe, but that ties the nfs-utils package irrevocably to systemd.

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]


2021-04-20 17:41:08

by Bruce Fields

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

On Tue, Apr 20, 2021 at 05:28:08PM +0000, Trond Myklebust wrote:
> On Tue, 2021-04-20 at 13:18 -0400, J. Bruce Fields wrote:
> > On Tue, Apr 20, 2021 at 02:31:58PM +0000, Trond Myklebust wrote:
> > > I think the important thing is, as Chuck said, that the setting of
> > > the
> > > uniquifier has to be automated. There are too many instances out
> > > there
> > > of people who get confused because they are using a default
> > > hostname,
> > > such as 'localhost.localdomain' and are setting no uniquifier.
> > >
> > > So the point is that it needs to be persisted by an automated
> > > script if
> > > unset.
> > >
> > > While that script could use nfsconf to get/set the persisted
> > > uniquifier, the worry is that such an automated change might be
> > > made
> > > while the user is performing some other edit of nfs.conf. What
> > > happens
> > > then?
> >
> > The one thing I'm a little uneasy about is ignoring /etc/machine-id.
> > Seems like distros *should* be creating it for us.  And it would be
> > convenient to have one source of machine identity rather than
> > separate
> > ones for different subsystems.
> >
> > Maybe we could use that if it exists, and fall back on generating our
> > own only if it doesn't?
> >
> > (Well, where "use it" actually means take a hash of it, as explained
> > in
> > machine-id(5).)
> >
>
> Maybe, but that ties the nfs-utils package irrevocably to systemd.

Well, like I say, we could have a fallback. Or even provide alternative
scripts in nfs-utils and let the distro decide which to install
depending on whether they use systemd.

But, whatever, those two alternatives (machine-id or vs. nfs generating
its own uuid) are basically the same on some level.

I agree with the basic idea that this should be automated rather than
living in a configuration file that humans might have to deal with.

--b.

2021-04-20 17:54:59

by Trond Myklebust

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

On Tue, 2021-04-20 at 13:40 -0400, [email protected] wrote:
> On Tue, Apr 20, 2021 at 05:28:08PM +0000, Trond Myklebust wrote:
> > On Tue, 2021-04-20 at 13:18 -0400, J. Bruce Fields wrote:
> > > On Tue, Apr 20, 2021 at 02:31:58PM +0000, Trond Myklebust wrote:
> > > > I think the important thing is, as Chuck said, that the setting
> > > > of
> > > > the
> > > > uniquifier has to be automated. There are too many instances
> > > > out
> > > > there
> > > > of people who get confused because they are using a default
> > > > hostname,
> > > > such as 'localhost.localdomain' and are setting no uniquifier.
> > > >
> > > > So the point is that it needs to be persisted by an automated
> > > > script if
> > > > unset.
> > > >
> > > > While that script could use nfsconf to get/set the persisted
> > > > uniquifier, the worry is that such an automated change might be
> > > > made
> > > > while the user is performing some other edit of nfs.conf. What
> > > > happens
> > > > then?
> > >
> > > The one thing I'm a little uneasy about is ignoring /etc/machine-
> > > id.
> > > Seems like distros *should* be creating it for us.  And it would
> > > be
> > > convenient to have one source of machine identity rather than
> > > separate
> > > ones for different subsystems.
> > >
> > > Maybe we could use that if it exists, and fall back on generating
> > > our
> > > own only if it doesn't?
> > >
> > > (Well, where "use it" actually means take a hash of it, as
> > > explained
> > > in
> > > machine-id(5).)
> > >
> >
> > Maybe, but that ties the nfs-utils package irrevocably to systemd.
>
> Well, like I say, we could have a fallback.  Or even provide
> alternative
> scripts in nfs-utils and let the distro decide which to install
> depending on whether they use systemd.
>
> But, whatever, those two alternatives (machine-id or vs. nfs
> generating
> its own uuid) are basically the same on some level.

Not quite. They cause the behaviour to differ depending on whether or
not systemd is installed. So if you imagine a system that gets updated
from "traditional init" to systemd, then that could cause the NFS
identity of the machine to change.

It would be better to be able to specify the identity in a form that is
independent of the platform.

So if the machine-id exists, then maybe we could indeed generate the
identity using the uuid in that file (although the question remains as
to why you'd want that?). However the generated value should then be
persisted separately so that it can be platform independent.

>
> I agree with the basic idea that this should be automated rather than
> living in a configuration file that humans might have to deal with.
>
> --b.

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]


2021-04-20 18:17:09

by Bruce Fields

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

On Tue, Apr 20, 2021 at 05:53:34PM +0000, Trond Myklebust wrote:
> So if the machine-id exists, then maybe we could indeed generate the
> identity using the uuid in that file (although the question remains as
> to why you'd want that?).

I was assuming: When you clone a machine image, either you want the
clone to have the same identity (maybe it's a backup, or you're doing
some sort of migration) or you want it to act like a new machine (say
you've got a base image that you're using to make a bunch of hosts). In
the latter case you've got to track down everything on the filesystem
that needs to differ between hosts and fix it up. The fewer of those,
the better.

> However the generated value should then be persisted separately so
> that it can be platform independent.

That'd be OK.

I don't think switching a host between systemd and not systemd-based is
a common case.

If somebody has a really weird case they can always write their own
script.

--b.

2021-04-20 18:24:23

by Steve Dickson

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



On 4/20/21 10:09 AM, Chuck Lever III wrote:
>>>> If that is the cause... have the variables in
>>>> nfs.conf create sysfs/udev file would work better?
>>>
>>> Seems to me we have the same argument for a separate file
>>> to store the nfs4_unique_id that we have for breaking
>>> /etc/exports into a directory of individual files: no
>>> parsing is necessary. Scripts and applications can simply
>>> read the string right out of the file, or update it just
>>> by writing the string into that file.
>> I'm sure I'm following this analogy with the export...
>> but being able to set things up from one configuration
>> file and command is key.
>
> I find that to be a red herring. We're not ever going to be
> at a place where everything is configured in one file. Are
> you going to replace /etc/exports.d and automounter.conf
> and krb5.conf and all the other things with just /etc/nfs.conf?
Obviously not... and you for got /etc/idmapd.conf ;-)
but I have thought about rolling nfsmount.conf into nfs.conf.

> Probably not. So let's stop using this strange logic to
> insist that /etc/nfs.conf is the only place for the clientid
> uniqifier.
Maybe this is splitting hairs... but the actual uniqifier does
exist in nfs.conf... Patches introduce a way to generate one
for nfs.conf.

>
> Please, let's just focus on what's right for this one setting.
>
>
>> nfsconf does an excellent job parsing nfs.conf and I would
>> like to build on that...
>
> Just because it is technically possible to save the uniqifier
> in /etc/nfs.conf doesn't mean that's what's best for our users.
> We're in better shape if we can guarantee that setting is
> correct and administrators can't break it.
Again... the id is not saved in nfs.conf... Just a couple
methods to generate one.

>
>
>> I understand we have to work well in containers which
>> is one of the reason I was trying to come up with a
>> nfsv4 only nfs-utils... That went over like a lead balloon ;-)
>
> I didn't have a problem with an nfsv4-only configuration.
> What I didn't like about that is that you were making the
> administrative interface more complex, and it didn't need to
> be.
I'm sure what you mean... but that is for another day 8-)

>
>
>> What I don't understand is why we can't come up with a
>> solution that uniquely set a param that is set by
>> nfsconf using nfs.conf.
>
> Once we have an automated mechanism to set the uniqifier,
> it does not need to be set by humans. Let's keep it out of
> nfs.conf.
>
> I'm in favor of making this as automatic as possible. No
> setting is better than an exposed setting that is never
> touched.
>
> I prefer no change to nfs.conf, and put the uniqifier in a
> separate file.
Again the id will end up in a different file... Trond
wants it in the sysfs filesystem verses the /etc/modprob.d/nfs.conf
file... Which is fine...

To me this is sound more like a distro issue of how the
uniqifier is created and what should be used to create it
when nfs-utils is installed.

steved.

2021-04-20 18:46:34

by Steve Dickson

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



On 4/20/21 10:31 AM, Trond Myklebust wrote:
>>> What I don't understand is why we can't come up with a
>>> solution that uniquely set a param that is set by
>>> nfsconf using nfs.conf.
>> Once we have an automated mechanism to set the uniqifier,
>> it does not need to be set by humans. Let's keep it out of
>> nfs.conf.
>>
>> I'm in favor of making this as automatic as possible. No
>> setting is better than an exposed setting that is never
>> touched.
>>
>> I prefer no change to nfs.conf, and put the uniqifier in a
>> separate file.
>>
> I think the important thing is, as Chuck said, that the setting of the
> uniquifier has to be automated. There are too many instances out there
> of people who get confused because they are using a default hostname,
> such as 'localhost.localdomain' and are setting no uniquifier.
The current patches use either /etc/machine-id or hostname
to generate the uniquifier. Alice's patches also included
/proc/sys/kernel/random/uuid as as way generate.

People could have those choices... and we (aka nfs-utils) would be
doing the generations.

>
> So the point is that it needs to be persisted by an automated script if
> unset.
Yes this is one thing that is missing... Making sure it is not already set.

>
> While that script could use nfsconf to get/set the persisted
> uniquifier, the worry is that such an automated change might be made
> while the user is performing some other edit of nfs.conf. What happens
> then?
Cat will start dating Dogs??? IDK! :-)

So it sound like we need a way to generate an uniquifier
which the patches do (we could add your sysfs one)
but you don't what that way tied to /etc/nfs.conf.

So that means we will generate the uniquifier one
way and only one way that has to work on all distro
that happens automatically... If id is not already
set...

There should be a way for distro to decide who the
uniquifier is generated?

steved.

2021-04-20 19:28:19

by Steve Dickson

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



On 4/20/21 2:16 PM, [email protected] wrote:
> On Tue, Apr 20, 2021 at 05:53:34PM +0000, Trond Myklebust wrote:
>> So if the machine-id exists, then maybe we could indeed generate the
>> identity using the uuid in that file (although the question remains as
>> to why you'd want that?).
>
> I was assuming: When you clone a machine image, either you want the
> clone to have the same identity (maybe it's a backup, or you're doing
> some sort of migration) or you want it to act like a new machine (say
> you've got a base image that you're using to make a bunch of hosts). In
> the latter case you've got to track down everything on the filesystem
> that needs to differ between hosts and fix it up. The fewer of those,
> the better.
For the record... I cloned a VM and the /etc/machine-id were the same.
The later would have to happen.

>
>> However the generated value should then be persisted separately so
>> that it can be platform independent.
>
> That'd be OK.
>
> I don't think switching a host between systemd and not systemd-based is
> a common case.
>
> If somebody has a really weird case they can always write their own
> script.
+1

steved.

2021-05-13 00:30:25

by NeilBrown

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

On Fri, 16 Apr 2021, Steve Dickson 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 <[email protected]> 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. The mechanism being
> a way to set kernel module params from nfs.conf. The setting of
> the id is just a side effect...

I wonder if this is the best approach for setting module parameters.

rpc.nfsd already sets grace-time and lease-time - which aren't
exactly module parameters, but are similar - using values from nfs.conf.
Similarly statd sets /proc/fs/nfs/nlm_tcport based on nfs.conf.

I don't think these things should appear in nfs.conf as "kernel
parameters", but as service parameters for the particular service.
How they are communicate to the kernel is an internal implementation
detail. Maybe it will involve setting module parameters (at least on
older kernels).

For the "identity" setting, I think it would be best if this were
checked and updated by mount.nfs (similar to the way mount.nfs will
check if statd is running, and will start it if necessary). So should
it go in nfsmount.conf instead of nfs.conf?? I'm not sure.

It isn't clear to me where the identity should come from.
In some circumstances it might make sense to take it from nfs.conf.
In that case we would want to support reading /etc/netnfs/NAME/nfs.conf
where NAME was determined in much the same way that "ip netns identify"
determines a name. (Compare inum of /proc/self/ns/net with the inum of
each name in /run/netns/).
If we did that, we could then support "$netns" in the conf file, and
allow

[nfs]
identity = ${hostname}-${netns}

in /etc/nfs.conf, and it would Do The Right Thing for many cases.

We have a partner who wants to make use of 'nconnect' but is
particularly inconvenienced by the fact that once there is any mount
from a given server it is no longer possible to change the nconnect
setting. I have suggested they explore setting up a separate
net-namespace for "their" mounts which can be independent from "other"
mounts on the same machine. If we could make that work with a degree of
transparency - maybe even a "-o netfs=foobar" mount option - that would
be a big help.

Thanks,
NeilBrown

2021-05-19 18:06:34

by Steve Dickson

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

Sorry for the delay... I took some PTO...

On 5/12/21 8:29 PM, NeilBrown wrote:
> On Fri, 16 Apr 2021, Steve Dickson 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 <[email protected]> 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. The mechanism being
>> a way to set kernel module params from nfs.conf. The setting of
>> the id is just a side effect...
>
> I wonder if this is the best approach for setting module parameters.
>
> rpc.nfsd already sets grace-time and lease-time - which aren't
> exactly module parameters, but are similar - using values from nfs.conf.
> Similarly statd sets /proc/fs/nfs/nlm_tcport based on nfs.conf.
>
> I don't think these things should appear in nfs.conf as "kernel
> parameters", but as service parameters for the particular service.
> How they are communicate to the kernel is an internal implementation
> detail. Maybe it will involve setting module parameters (at least on
> older kernels).
I think I understand you idea of look at thing as "service parameters"
instead of "kernel parameters", but looking at the actual parameters
that might be a bit difficult.

Some do map to a service like nfs4_disable_idmapping could be set
from /etc/idmapd.conf, but things like send_implementation_id or
delegation_watermark do not really map to a particular service
or am I missing something?

>
> For the "identity" setting, I think it would be best if this were
> checked and updated by mount.nfs (similar to the way mount.nfs will
> check if statd is running, and will start it if necessary). So should
> it go in nfsmount.conf instead of nfs.conf?? I'm not sure.
Interesting idea...I would think nfsmount.conf would be the
right place.

>
> It isn't clear to me where the identity should come from.
> In some circumstances it might make sense to take it from nfs.conf.
> In that case we would want to support reading /etc/netnfs/NAME/nfs.conf
> where NAME was determined in much the same way that "ip netns identify"
> determines a name. (Compare inum of /proc/self/ns/net with the inum of
> each name in /run/netns/).
I think supporting configs per namespaces is a good idea. I don't
think it would be too difficult to do since we already support
the nfs.d directory.


> If we did that, we could then support "$netns" in the conf file, and
> allow
>
> [nfs]
> identity = ${hostname}-${netns}
>
> in /etc/nfs.conf, and it would Do The Right Thing for many cases.
I'm a bit namespace challenged... but as I see it using
"ip netns identify" (w/out the [PID]) would return all of
the current network network namespaces. Then we would run through
the /etc/nfs.conf.d/ directory looking for a matching directory
for any of the returned namespaces. If found that config
would be used. Something along those lines?

With multiple namespaces, how would we know which one to use?

>
> We have a partner who wants to make use of 'nconnect' but is
> particularly inconvenienced by the fact that once there is any mount
> from a given server it is no longer possible to change the nconnect
> setting. I have suggested they explore setting up a separate
> net-namespace for "their" mounts which can be independent from "other"
> mounts on the same machine. If we could make that work with a degree of
> transparency - maybe even a "-o netfs=foobar" mount option - that would
> be a big help.
I think I would like to continue exploring the namespace patch verse
adding another mount option.

steved.


2021-05-21 09:22:59

by NeilBrown

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

On Tue, 18 May 2021, Steve Dickson wrote:
> Sorry for the delay... I took some PTO...
>
> On 5/12/21 8:29 PM, NeilBrown wrote:
> > On Fri, 16 Apr 2021, Steve Dickson 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 <[email protected]> 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. The mechanism being
> >> a way to set kernel module params from nfs.conf. The setting of
> >> the id is just a side effect...
> >
> > I wonder if this is the best approach for setting module parameters.
> >
> > rpc.nfsd already sets grace-time and lease-time - which aren't
> > exactly module parameters, but are similar - using values from nfs.conf.
> > Similarly statd sets /proc/fs/nfs/nlm_tcport based on nfs.conf.
> >
> > I don't think these things should appear in nfs.conf as "kernel
> > parameters", but as service parameters for the particular service.
> > How they are communicate to the kernel is an internal implementation
> > detail. Maybe it will involve setting module parameters (at least on
> > older kernels).
> I think I understand you idea of look at thing as "service parameters"
> instead of "kernel parameters", but looking at the actual parameters
> that might be a bit difficult.
>
> Some do map to a service like nfs4_disable_idmapping could be set
> from /etc/idmapd.conf, but things like send_implementation_id or
> delegation_watermark do not really map to a particular service
> or am I missing something?

There are two "nfs4_disable_idmapping" parameters. One for server, one
for client.
The server one should, I think, be set by rpc.nfsd based on a setting in
the [nfsd] section of nfs.conf.

The client one should (I think) be set by mount.nfs using whatever
config language we decide is appropriate.

>
> >
> > For the "identity" setting, I think it would be best if this were
> > checked and updated by mount.nfs (similar to the way mount.nfs will
> > check if statd is running, and will start it if necessary). So should
> > it go in nfsmount.conf instead of nfs.conf?? I'm not sure.
> Interesting idea...I would think nfsmount.conf would be the
> right place.

Maybe... nfsmount.conf is currently only for mount options. These can
all be per-server or per-mountpoint, or global.
It might make sense to have other things in the global section ...
though it is named "NFSMount_Global_Options" which seems to explicitly
suggest that these are mount options.

I think I lean towards an [nfs] or possibly [mount] section of nfs.conf.

>
> >
> > It isn't clear to me where the identity should come from.
> > In some circumstances it might make sense to take it from nfs.conf.
> > In that case we would want to support reading /etc/netnfs/NAME/nfs.conf
> > where NAME was determined in much the same way that "ip netns identify"
> > determines a name. (Compare inum of /proc/self/ns/net with the inum of
> > each name in /run/netns/).
> I think supporting configs per namespaces is a good idea. I don't
> think it would be too difficult to do since we already support
> the nfs.d directory.

Yes, reading multiple files should be easy enough once we know what we
want to do.

>
>
> > If we did that, we could then support "$netns" in the conf file, and
> > allow
> >
> > [nfs]
> > identity = ${hostname}-${netns}
> >
> > in /etc/nfs.conf, and it would Do The Right Thing for many cases.
> I'm a bit namespace challenged... but as I see it using
> "ip netns identify" (w/out the [PID]) would return all of
> the current network network namespaces. Then we would run through
> the /etc/nfs.conf.d/ directory looking for a matching directory
> for any of the returned namespaces. If found that config
> would be used. Something along those lines?
>
> With multiple namespaces, how would we know which one to use?

(I'm only just coming up to speed on network namespaces too....)

A given process can only be in one network namespace. If it is in the
initial namespace (same as the 'init' process) then "ip netns identify"
reports nothing. If in some other namespace, then that namespace is
reported.

So if 'mount.nfs' is run in some other net-namespace, it should let
settings in /etc/netfs/NAME/nfs.conf over-ride settings in /etc/nfs.conf

I'm becoming less enamoured with the idea of using network namespaces to
ensure separate transports are used. Creating a new namespace means
that either you need a new IP address for that namespace, or you need to
set up NAT so processes in the namespace can access the network. Both
of these seem like a bit too much overhead just to get an independent
TCP connection (or set of connections) to the server.
I almost want an "NFS namespace" which shares the network but has
separate transports. I have something like that in our SLE12 kernels
(-o sharetransport=NN) but I'd like a better solution.

Being able to insisting on a separate transport is really useful for
problem analysis, and has other administrative uses.

NeilBrown