| Message ID | 20210414181040.7108-1-steved@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Enable the setting of a kernel module parameter from nfs.conf | expand |
Hi Steve- > On Apr 14, 2021, at 2:10 PM, Steve Dickson <SteveD@redhat.com> wrote: > > This is a tweak of the patch set Alice Mitchell posted last July [1]. That approach was dropped last July because it is not container-aware. It should be simple for someone to write a udev script that uses the existing sysfs API that can update nfs4_client_id in a namespace. I would prefer the sysfs/udev approach for setting nfs4_client_id, since it is container-aware and makes this setting completely automatic (zero touch). > 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 >
Hey Chuck! On 4/14/21 7:26 PM, Chuck Lever III wrote: > Hi Steve- > >> On Apr 14, 2021, at 2:10 PM, Steve Dickson <SteveD@redhat.com> wrote: >> >> This is a tweak of the patch set Alice Mitchell posted last July [1]. > > That approach was dropped last July because it is not container-aware. > It should be simple for someone to write a udev script that uses the > existing sysfs API that can update nfs4_client_id in a namespace. I > would prefer the sysfs/udev approach for setting nfs4_client_id, > since it is container-aware and makes this setting completely > automatic (zero touch). As I said in in my cover letter, I see this more as introduction of a mechanism more than a way to set the unique id. 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 >>
> On Apr 15, 2021, at 11:33 AM, Steve Dickson <SteveD@RedHat.com> wrote: > > Hey Chuck! > > On 4/14/21 7:26 PM, Chuck Lever III wrote: >> Hi Steve- >> >>> On Apr 14, 2021, at 2:10 PM, Steve Dickson <SteveD@redhat.com> wrote: >>> >>> This is a tweak of the patch set Alice Mitchell posted last July [1]. >> >> That approach was dropped last July because it is not container-aware. >> It should be simple for someone to write a udev script that uses the >> existing sysfs API that can update nfs4_client_id in a namespace. I >> would prefer the sysfs/udev approach for setting nfs4_client_id, >> since it is container-aware and makes this setting completely >> automatic (zero touch). > As I said in in my cover letter, I see this more as introduction of > a mechanism more than a way to set the unique id. Yep, I got that. I'm not addressing the question of whether adding a mechanism to set a module parameter in nfs.conf is good or not. I'm saying nfs4_client_id is not an appropriate parameter to add to nfs.conf. Can you pick another module parameter as an example for your mechanism? > The mechanism being > a way to set kernel module params from nfs.conf. The setting of > the id is just a side effect... > > Why spread out the NFS configuration? Why not > just keep it in one place... aka nfs.conf? We need to understand whether a module parameter is not going to work in nfs.conf because that setting needs to be namespace-aware. In this case, this setting does indeed need to be namespace-aware. nfs.conf is not aware of network namespaces. > Plus we could document all the kernel params in nfs.conf > and the nfs.conf man page. The only documentation I know > of is in the kernel tree. OK, but that's not relevant to whether nfs.conf is the right place to set nfs4_client_id. > As far as not being container-aware... that might true > but it does not mean its not useful to set the id from > nfs.conf... Yes, it does mean that in that case. It's completely broken to use the same nfs4_client_id in every network namespace. > Actual I have customers asking for this type > of functionality Ask yourself why they might want it. It's probably because we don't set it correctly currently. If we have a way to automatically get it right every time, there's really no need for this setting to be exposed. I do agree that it's long past time we should be setting nfs4_client_id properly. I would rather see a udev script developed (you, me, or Alice could do it in an afternoon) first. If that doesn't meet the actual customer need, then we can revisit. > 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
On Thu, 2021-04-15 at 16:37 +0000, Chuck Lever III wrote: > > > > On Apr 15, 2021, at 11:33 AM, Steve Dickson <SteveD@RedHat.com> > > wrote: > > > > Hey Chuck! > > > > On 4/14/21 7:26 PM, Chuck Lever III wrote: > > > Hi Steve- > > > > > > > On Apr 14, 2021, at 2:10 PM, Steve Dickson <SteveD@redhat.com> > > > > wrote: > > > > > > > > This is a tweak of the patch set Alice Mitchell posted last > > > > July [1]. > > > > > > That approach was dropped last July because it is not container- > > > aware. > > > It should be simple for someone to write a udev script that uses > > > the > > > existing sysfs API that can update nfs4_client_id in a namespace. > > > I > > > would prefer the sysfs/udev approach for setting nfs4_client_id, > > > since it is container-aware and makes this setting completely > > > automatic (zero touch). > > As I said in in my cover letter, I see this more as introduction of > > a mechanism more than a way to set the unique id. > > Yep, I got that. > > I'm not addressing the question of whether adding a > mechanism to set a module parameter in nfs.conf is good > or not. I'm saying nfs4_client_id is not an appropriate > parameter to add to nfs.conf. Can you pick another > module parameter as an example for your mechanism? > > > > The mechanism being > > a way to set kernel module params from nfs.conf. The setting of > > the id is just a side effect... > > > > Why spread out the NFS configuration? Why not > > just keep it in one place... aka nfs.conf? > > We need to understand whether a module parameter is not > going to work in nfs.conf because that setting needs to > be namespace-aware. In this case, this setting does indeed > need to be namespace-aware. nfs.conf is not aware of > network namespaces. > > > > Plus we could document all the kernel params in nfs.conf > > and the nfs.conf man page. The only documentation I know > > of is in the kernel tree. > > OK, but that's not relevant to whether nfs.conf is the > right place to set nfs4_client_id. > > > > As far as not being container-aware... that might true > > but it does not mean its not useful to set the id from > > nfs.conf... > > Yes, it does mean that in that case. It's completely > broken to use the same nfs4_client_id in every network > namespace. > > > > Actual I have customers asking for this type > > of functionality > > Ask yourself why they might want it. It's probably because > we don't set it correctly currently. If we have a way to > automatically get it right every time, there's really no > need for this setting to be exposed. > > I do agree that it's long past time we should be setting > nfs4_client_id properly. I would rather see a udev script > developed (you, me, or Alice could do it in an afternoon) > first. If that doesn't meet the actual customer need, then > we can revisit. > Right. The only sensible solution in a containerised world is a udev script that sets /sys/fs/nfs/net/nfs_client/identifier when triggered. Note that we really want something that generates a random uuid, and then persists it so that it can be retrieved on reboot or restart of the container. Something similar to systemd-machine-id-setup, but that can be called from udev. > > > steved. > > > > > > > > > > It enables the setting of the nfs4_unique_id kernel module > > > > parameter from /etc/nfs.conf. > > > > > > > Things I tweaked: > > > > > > > > * Introduce a new [kernel] section in nfs.conf which only > > > > contains the nfs4_unique_id setting... For now... > > > > > > > > * nfs4_unique_id can be set to two different values > > > > > > > > - nfs4_unique_id = ${machine-id} will use /etc/machine-id > > > > as the unique id. > > > > - nfs4_unique_id = ${hostname} will use the system's > > > > hostname > > > > as the unique id. > > > > > > > > * The new nfs-config systemd service need to be enabled for > > > > the > > > > /etc/modprobe.d/nfs.conf file to be created with > > > > the "options nfs nfs4_unique_id=" set. > > > > > > > > I see this patch set is not a way to set the nfs4_unique_id > > > > module parameter... I see it as a beginning of a way to set > > > > all module parameters from /etc/nfs.conf, which I think > > > > is a good thing... > > > > > > > > [1] https://www.spinics.net/lists/linux-nfs/msg78658.html > > > > > > > > Alice Mitchell (3): > > > > nfs-utils: Enable the retrieval of raw config settings without > > > > expansion > > > > nfs-utils: Add support for further ${variable} expansions in > > > > nfs.conf > > > > nfs-utils: Update nfs4_unique_id module parameter from the > > > > nfs.conf > > > > value > > > > > > > > configure.ac | 1 + > > > > nfs.conf | 4 +- > > > > support/include/conffile.h | 1 + > > > > support/nfs/conffile.c | 283 > > > > ++++++++++++++++++++++++++++++++-- > > > > systemd/Makefile.am | 3 + > > > > systemd/nfs-client.target | 3 + > > > > systemd/nfs-conf-export.sh | 28 ++++ > > > > systemd/nfs-config.service.in | 18 +++ > > > > systemd/nfs.conf.man | 19 ++- > > > > tools/nfsconf/nfsconf.man | 10 +- > > > > tools/nfsconf/nfsconfcli.c | 22 ++- > > > > 11 files changed, 372 insertions(+), 20 deletions(-) > > > > create mode 100755 systemd/nfs-conf-export.sh > > > > create mode 100644 systemd/nfs-config.service.in > > > > > > > > -- > > > > 2.30.2 > > > > > > > > -- > Chuck Lever > > >
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 <SteveD@RedHat.com> > > > wrote: > > > > > > Hey Chuck! > > > > > > On 4/14/21 7:26 PM, Chuck Lever III wrote: > > > > Hi Steve- > > > > > > > > > On Apr 14, 2021, at 2:10 PM, Steve Dickson <SteveD@redhat.com> > > > > > wrote: > > > > > > > > > > This is a tweak of the patch set Alice Mitchell posted last > > > > > July [1]. > > > > > > > > That approach was dropped last July because it is not container- > > > > aware. > > > > It should be simple for someone to write a udev script that uses > > > > the > > > > existing sysfs API that can update nfs4_client_id in a namespace. > > > > I > > > > would prefer the sysfs/udev approach for setting nfs4_client_id, > > > > since it is container-aware and makes this setting completely > > > > automatic (zero touch). > > > As I said in in my cover letter, I see this more as introduction of > > > a mechanism more than a way to set the unique id. > > > > Yep, I got that. > > > > I'm not addressing the question of whether adding a > > mechanism to set a module parameter in nfs.conf is good > > or not. I'm saying nfs4_client_id is not an appropriate > > parameter to add to nfs.conf. Can you pick another > > module parameter as an example for your mechanism? > > > > > > > The mechanism being > > > a way to set kernel module params from nfs.conf. The setting of > > > the id is just a side effect... > > > > > > Why spread out the NFS configuration? Why not > > > just keep it in one place... aka nfs.conf? > > > > We need to understand whether a module parameter is not > > going to work in nfs.conf because that setting needs to > > be namespace-aware. In this case, this setting does indeed > > need to be namespace-aware. nfs.conf is not aware of > > network namespaces. > > > > > > > Plus we could document all the kernel params in nfs.conf > > > and the nfs.conf man page. The only documentation I know > > > of is in the kernel tree. > > > > OK, but that's not relevant to whether nfs.conf is the > > right place to set nfs4_client_id. > > > > > > > As far as not being container-aware... that might true > > > but it does not mean its not useful to set the id from > > > nfs.conf... > > > > Yes, it does mean that in that case. It's completely > > broken to use the same nfs4_client_id in every network > > namespace. > > > > > > > Actual I have customers asking for this type > > > of functionality > > > > Ask yourself why they might want it. It's probably because > > we don't set it correctly currently. If we have a way to > > automatically get it right every time, there's really no > > need for this setting to be exposed. > > > > I do agree that it's long past time we should be setting > > nfs4_client_id properly. I would rather see a udev script > > developed (you, me, or Alice could do it in an afternoon) > > first. If that doesn't meet the actual customer need, then > > we can revisit. > > > > Right. The only sensible solution in a containerised world is a udev > script that sets /sys/fs/nfs/net/nfs_client/identifier when triggered. > > Note that we really want something that generates a random uuid, and > then persists it so that it can be retrieved on reboot or restart of > the container. Something similar to systemd-machine-id-setup, but that > can be called from udev. > Here is a skeleton example: [root@leira 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" [root@leira 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 [root@leira rules.d]# modprobe nfs4 [root@leira rules.d]# cat /sys/fs/nfs/net/nfs_client/identifier 7f9f211b-0253-4ef8-a970-b1b0f600af02 [root@leira rules.d]# cat /etc/nfs4_uuid 7f9f211b-0253-4ef8-a970-b1b0f600af02
On 4/15/21 12:37 PM, Chuck Lever III wrote: > > >> On Apr 15, 2021, at 11:33 AM, Steve Dickson <SteveD@RedHat.com> wrote: >> >> Hey Chuck! >> >> On 4/14/21 7:26 PM, Chuck Lever III wrote: >>> Hi Steve- >>> >>>> On Apr 14, 2021, at 2:10 PM, Steve Dickson <SteveD@redhat.com> wrote: >>>> >>>> This is a tweak of the patch set Alice Mitchell posted last July [1]. >>> >>> That approach was dropped last July because it is not container-aware. >>> It should be simple for someone to write a udev script that uses the >>> existing sysfs API that can update nfs4_client_id in a namespace. I >>> would prefer the sysfs/udev approach for setting nfs4_client_id, >>> since it is container-aware and makes this setting completely >>> automatic (zero touch). >> As I said in in my cover letter, I see this more as introduction of >> a mechanism more than a way to set the unique id. > > Yep, I got that. > > I'm not addressing the question of whether adding a > mechanism to set a module parameter in nfs.conf is good > or not. I'm saying nfs4_client_id is not an appropriate > parameter to add to nfs.conf. Can you pick another > module parameter as an example for your mechanism? The 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.
Hey! On 4/15/21 8:40 PM, Trond Myklebust wrote: > Here is a skeleton example: > > [root@leira 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" > > [root@leira 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 > > [root@leira rules.d]# modprobe nfs4 > [root@leira rules.d]# cat /sys/fs/nfs/net/nfs_client/identifier > 7f9f211b-0253-4ef8-a970-b1b0f600af02 > [root@leira 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.
> On Apr 17, 2021, at 12:18 PM, Steve Dickson <SteveD@RedHat.com> wrote: > > > > On 4/15/21 12:37 PM, Chuck Lever III wrote: >> >> >>> On Apr 15, 2021, at 11:33 AM, Steve Dickson <SteveD@RedHat.com> wrote: >>> >>> Hey Chuck! >>> >>> On 4/14/21 7:26 PM, Chuck Lever III wrote: >>>> Hi Steve- >>>> >>>>> On Apr 14, 2021, at 2:10 PM, Steve Dickson <SteveD@redhat.com> wrote: >>>>> >>>>> This is a tweak of the patch set Alice Mitchell posted last July [1]. >>>> >>>> That approach was dropped last July because it is not container-aware. >>>> It should be simple for someone to write a udev script that uses the >>>> existing sysfs API that can update nfs4_client_id in a namespace. I >>>> would prefer the sysfs/udev approach for setting nfs4_client_id, >>>> since it is container-aware and makes this setting completely >>>> automatic (zero touch). >>> As I said in in my cover letter, I see this more as introduction of >>> a mechanism more than a way to set the unique id. >> >> Yep, I got that. >> >> I'm not addressing the question of whether adding a >> mechanism to set a module parameter in nfs.conf is good >> or not. I'm saying nfs4_client_id is not an appropriate >> parameter to add to nfs.conf. Can you pick another >> module parameter as an example for your mechanism? > The 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
On 4/17/21 12:36 PM, Chuck Lever III wrote: > > >> On Apr 17, 2021, at 12:18 PM, Steve Dickson <SteveD@RedHat.com> wrote: >> >> >> >> On 4/15/21 12:37 PM, Chuck Lever III wrote: >>> >>> >>>> On Apr 15, 2021, at 11:33 AM, Steve Dickson <SteveD@RedHat.com> wrote: >>>> >>>> Hey Chuck! >>>> >>>> On 4/14/21 7:26 PM, Chuck Lever III wrote: >>>>> Hi Steve- >>>>> >>>>>> On Apr 14, 2021, at 2:10 PM, Steve Dickson <SteveD@redhat.com> wrote: >>>>>> >>>>>> This is a tweak of the patch set Alice Mitchell posted last July [1]. >>>>> >>>>> That approach was dropped last July because it is not container-aware. >>>>> It should be simple for someone to write a udev script that uses the >>>>> existing sysfs API that can update nfs4_client_id in a namespace. I >>>>> would prefer the sysfs/udev approach for setting nfs4_client_id, >>>>> since it is container-aware and makes this setting completely >>>>> automatic (zero touch). >>>> As I said in in my cover letter, I see this more as introduction of >>>> a mechanism more than a way to set the unique id. >>> >>> Yep, I got that. >>> >>> I'm not addressing the question of whether adding a >>> mechanism to set a module parameter in nfs.conf is good >>> or not. I'm saying nfs4_client_id is not an appropriate >>> parameter to add to nfs.conf. Can you pick another >>> module parameter as an example for your mechanism? >> The 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 > > >
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: > > > > [root@leira 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" > > > > [root@leira 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 > > > > [root@leira rules.d]# modprobe nfs4 > > [root@leira rules.d]# cat /sys/fs/nfs/net/nfs_client/identifier > > 7f9f211b-0253-4ef8-a970-b1b0f600af02 > > [root@leira 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.
> On Apr 17, 2021, at 1:50 PM, Steve Dickson <SteveD@RedHat.com> wrote: > > > > On 4/17/21 12:36 PM, Chuck Lever III wrote: >> >> >>> On Apr 17, 2021, at 12:18 PM, Steve Dickson <SteveD@RedHat.com> wrote: >>> >>> >>> >>> On 4/15/21 12:37 PM, Chuck Lever III wrote: >>>> >>>> >>>>> On Apr 15, 2021, at 11:33 AM, Steve Dickson <SteveD@RedHat.com> wrote: >>>>> >>>>> Hey Chuck! >>>>> >>>>> On 4/14/21 7:26 PM, Chuck Lever III wrote: >>>>>> Hi Steve- >>>>>> >>>>>>> On Apr 14, 2021, at 2:10 PM, Steve Dickson <SteveD@redhat.com> wrote: >>>>>>> >>>>>>> This is a tweak of the patch set Alice Mitchell posted last July [1]. >>>>>> >>>>>> That approach was dropped last July because it is not container-aware. >>>>>> It should be simple for someone to write a udev script that uses the >>>>>> existing sysfs API that can update nfs4_client_id in a namespace. I >>>>>> would prefer the sysfs/udev approach for setting nfs4_client_id, >>>>>> since it is container-aware and makes this setting completely >>>>>> automatic (zero touch). >>>>> As I said in in my cover letter, I see this more as introduction of >>>>> a mechanism more than a way to set the unique id. >>>> >>>> Yep, I got that. >>>> >>>> I'm not addressing the question of whether adding a >>>> mechanism to set a module parameter in nfs.conf is good >>>> or not. I'm saying nfs4_client_id is not an appropriate >>>> parameter to add to nfs.conf. Can you pick another >>>> module parameter as an example for your mechanism? >>> The 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 # [cel@manet 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, [cel@manet 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, [cel@manet 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
On 4/18/21 12:51 PM, Chuck Lever III wrote: > > >> On Apr 17, 2021, at 1:50 PM, Steve Dickson <SteveD@RedHat.com> wrote: >> >> >> >> On 4/17/21 12:36 PM, Chuck Lever III wrote: >>> >>> >>>> On Apr 17, 2021, at 12:18 PM, Steve Dickson <SteveD@RedHat.com> wrote: >>>> >>>> >>>> >>>> On 4/15/21 12:37 PM, Chuck Lever III wrote: >>>>> >>>>> >>>>>> On Apr 15, 2021, at 11:33 AM, Steve Dickson <SteveD@RedHat.com> wrote: >>>>>> >>>>>> Hey Chuck! >>>>>> >>>>>> On 4/14/21 7:26 PM, Chuck Lever III wrote: >>>>>>> Hi Steve- >>>>>>> >>>>>>>> On Apr 14, 2021, at 2:10 PM, Steve Dickson <SteveD@redhat.com> wrote: >>>>>>>> >>>>>>>> This is a tweak of the patch set Alice Mitchell posted last July [1]. >>>>>>> >>>>>>> That approach was dropped last July because it is not container-aware. >>>>>>> It should be simple for someone to write a udev script that uses the >>>>>>> existing sysfs API that can update nfs4_client_id in a namespace. I >>>>>>> would prefer the sysfs/udev approach for setting nfs4_client_id, >>>>>>> since it is container-aware and makes this setting completely >>>>>>> automatic (zero touch). >>>>>> As I said in in my cover letter, I see this more as introduction of >>>>>> a mechanism more than a way to set the unique id. >>>>> >>>>> Yep, I got that. >>>>> >>>>> I'm not addressing the question of whether adding a >>>>> mechanism to set a module parameter in nfs.conf is good >>>>> or not. I'm saying nfs4_client_id is not an appropriate >>>>> parameter to add to nfs.conf. Can you pick another >>>>> module parameter as an example for your mechanism? >>>> The 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 > # > > [cel@manet 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, > [cel@manet 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, > [cel@manet 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 > > >
> On Apr 20, 2021, at 9:11 AM, Steve Dickson <SteveD@RedHat.com> wrote: > > > > On 4/18/21 12:51 PM, Chuck Lever III wrote: >> >> >>> On Apr 17, 2021, at 1:50 PM, Steve Dickson <SteveD@RedHat.com> wrote: >>> >>> >>> >>> On 4/17/21 12:36 PM, Chuck Lever III wrote: >>>> >>>> >>>>> On Apr 17, 2021, at 12:18 PM, Steve Dickson <SteveD@RedHat.com> wrote: >>>>> >>>>> >>>>> >>>>> On 4/15/21 12:37 PM, Chuck Lever III wrote: >>>>>> >>>>>> >>>>>>> On Apr 15, 2021, at 11:33 AM, Steve Dickson <SteveD@RedHat.com> wrote: >>>>>>> >>>>>>> Hey Chuck! >>>>>>> >>>>>>> On 4/14/21 7:26 PM, Chuck Lever III wrote: >>>>>>>> Hi Steve- >>>>>>>> >>>>>>>>> On Apr 14, 2021, at 2:10 PM, Steve Dickson <SteveD@redhat.com> wrote: >>>>>>>>> >>>>>>>>> This is a tweak of the patch set Alice Mitchell posted last July [1]. >>>>>>>> >>>>>>>> That approach was dropped last July because it is not container-aware. >>>>>>>> It should be simple for someone to write a udev script that uses the >>>>>>>> existing sysfs API that can update nfs4_client_id in a namespace. I >>>>>>>> would prefer the sysfs/udev approach for setting nfs4_client_id, >>>>>>>> since it is container-aware and makes this setting completely >>>>>>>> automatic (zero touch). >>>>>>> As I said in in my cover letter, I see this more as introduction of >>>>>>> a mechanism more than a way to set the unique id. >>>>>> >>>>>> Yep, I got that. >>>>>> >>>>>> I'm not addressing the question of whether adding a >>>>>> mechanism to set a module parameter in nfs.conf is good >>>>>> or not. I'm saying nfs4_client_id is not an appropriate >>>>>> parameter to add to nfs.conf. Can you pick another >>>>>> module parameter as an example for your mechanism? >>>>> The 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 >> # >> >> [cel@manet 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, >> [cel@manet 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, >> [cel@manet 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
On Tue, 2021-04-20 at 14:09 +0000, Chuck Lever III wrote: > > > > On Apr 20, 2021, at 9:11 AM, Steve Dickson <SteveD@RedHat.com> > > wrote: > > > > > > > > On 4/18/21 12:51 PM, Chuck Lever III wrote: > > > > > > > > > > On Apr 17, 2021, at 1:50 PM, Steve Dickson <SteveD@RedHat.com> > > > > wrote: > > > > > > > > > > > > > > > > On 4/17/21 12:36 PM, Chuck Lever III wrote: > > > > > > > > > > > > > > > > On Apr 17, 2021, at 12:18 PM, Steve Dickson < > > > > > > SteveD@RedHat.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > On 4/15/21 12:37 PM, Chuck Lever III wrote: > > > > > > > > > > > > > > > > > > > > > > On Apr 15, 2021, at 11:33 AM, Steve Dickson < > > > > > > > > SteveD@RedHat.com> wrote: > > > > > > > > > > > > > > > > Hey Chuck! > > > > > > > > > > > > > > > > On 4/14/21 7:26 PM, Chuck Lever III wrote: > > > > > > > > > Hi Steve- > > > > > > > > > > > > > > > > > > > On Apr 14, 2021, at 2:10 PM, Steve Dickson < > > > > > > > > > > SteveD@redhat.com> wrote: > > > > > > > > > > > > > > > > > > > > This is a tweak of the patch set Alice Mitchell > > > > > > > > > > posted last July [1]. > > > > > > > > > > > > > > > > > > That approach was dropped last July because it is not > > > > > > > > > container-aware. > > > > > > > > > It should be simple for someone to write a udev > > > > > > > > > script that uses the > > > > > > > > > existing sysfs API that can update nfs4_client_id in > > > > > > > > > a namespace. I > > > > > > > > > would prefer the sysfs/udev approach for setting > > > > > > > > > nfs4_client_id, > > > > > > > > > since it is container-aware and makes this setting > > > > > > > > > completely > > > > > > > > > automatic (zero touch). > > > > > > > > As I said in in my cover letter, I see this more as > > > > > > > > introduction of > > > > > > > > a mechanism more than a way to set the unique id. > > > > > > > > > > > > > > Yep, I got that. > > > > > > > > > > > > > > I'm not addressing the question of whether adding a > > > > > > > mechanism to set a module parameter in nfs.conf is good > > > > > > > or not. I'm saying nfs4_client_id is not an appropriate > > > > > > > parameter to add to nfs.conf. Can you pick another > > > > > > > module parameter as an example for your mechanism? > > > > > > The 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 > > > # > > > > > > [cel@manet 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, > > > [cel@manet 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, > > > [cel@manet 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?
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.
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.
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.
On Tue, 2021-04-20 at 13:40 -0400, bfields@fieldses.org 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.
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.
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.
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.
On 4/20/21 2:16 PM, bfields@fieldses.org 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.
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 <SteveD@redhat.com> wrote: > >> > >> This is a tweak of the patch set Alice Mitchell posted last July [1]. > > > > That approach was dropped last July because it is not container-aware. > > It should be simple for someone to write a udev script that uses the > > existing sysfs API that can update nfs4_client_id in a namespace. I > > would prefer the sysfs/udev approach for setting nfs4_client_id, > > since it is container-aware and makes this setting completely > > automatic (zero touch). > As I said in in my cover letter, I see this more as introduction of > a mechanism more than a way to set the unique id. 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
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 <SteveD@redhat.com> wrote: >>>> >>>> This is a tweak of the patch set Alice Mitchell posted last July [1]. >>> >>> That approach was dropped last July because it is not container-aware. >>> It should be simple for someone to write a udev script that uses the >>> existing sysfs API that can update nfs4_client_id in a namespace. I >>> would prefer the sysfs/udev approach for setting nfs4_client_id, >>> since it is container-aware and makes this setting completely >>> automatic (zero touch). >> As I said in in my cover letter, I see this more as introduction of >> a mechanism more than a way to set the unique id. 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.
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 <SteveD@redhat.com> wrote: > >>>> > >>>> This is a tweak of the patch set Alice Mitchell posted last July [1]. > >>> > >>> That approach was dropped last July because it is not container-aware. > >>> It should be simple for someone to write a udev script that uses the > >>> existing sysfs API that can update nfs4_client_id in a namespace. I > >>> would prefer the sysfs/udev approach for setting nfs4_client_id, > >>> since it is container-aware and makes this setting completely > >>> automatic (zero touch). > >> As I said in in my cover letter, I see this more as introduction of > >> a mechanism more than a way to set the unique id. 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
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