2022-11-09 13:38:44

by David Wysochanski

[permalink] [raw]
Subject: [PATCH] Documentation: Fix sysfs path for the NFSv4 client identifier

The sysfs path for the NFS4 client identfier should start with
the path component of 'nfs' for the kset, and then the 'net'
path component for the netns object, followed by the
'nfs_client' path component for the NFS client kobject,
and ending with 'identifier' for the netns_client_id
kobj_attribute.

Fixes: a28faaddb2be ("Documentation: Add an explanation of NFSv4 client identifiers")
Signed-off-by: Dave Wysochanski <[email protected]>
---
Documentation/filesystems/nfs/client-identifier.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/nfs/client-identifier.rst b/Documentation/filesystems/nfs/client-identifier.rst
index 5147e15815a1..a94c7a9748d7 100644
--- a/Documentation/filesystems/nfs/client-identifier.rst
+++ b/Documentation/filesystems/nfs/client-identifier.rst
@@ -152,7 +152,7 @@ string:
via the kernel command line, or when the "nfs" module is
loaded.

- /sys/fs/nfs/client/net/identifier
+ /sys/fs/nfs/net/nfs_client/identifier
This virtual file, available since Linux 5.3, is local to the
network namespace in which it is accessed and so can provide
distinction between network namespaces (containers) when the
@@ -164,7 +164,7 @@ then that uniquifier can be used. For example, a uniquifier might
be formed at boot using the container's internal identifier:

sha256sum /etc/machine-id | awk '{print $1}' \\
- > /sys/fs/nfs/client/net/identifier
+ > /sys/fs/nfs/net/nfs_client/identifier

Security considerations
-----------------------
--
2.31.1



2022-11-09 14:12:03

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH] Documentation: Fix sysfs path for the NFSv4 client identifier



> On Nov 9, 2022, at 8:33 AM, Dave Wysochanski <[email protected]> wrote:
>
> The sysfs path for the NFS4 client identfier should start with
> the path component of 'nfs' for the kset, and then the 'net'
> path component for the netns object, followed by the
> 'nfs_client' path component for the NFS client kobject,
> and ending with 'identifier' for the netns_client_id
> kobj_attribute.

Seems like the redundancy has some purpose and is baked-in to
the path. I'd rather leave the sleeping dog as it is, enjoying
the morning sun.

Reviewed-by: Chuck Lever <[email protected]>


> Fixes: a28faaddb2be ("Documentation: Add an explanation of NFSv4 client identifiers")
> Signed-off-by: Dave Wysochanski <[email protected]>
> ---
> Documentation/filesystems/nfs/client-identifier.rst | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/filesystems/nfs/client-identifier.rst b/Documentation/filesystems/nfs/client-identifier.rst
> index 5147e15815a1..a94c7a9748d7 100644
> --- a/Documentation/filesystems/nfs/client-identifier.rst
> +++ b/Documentation/filesystems/nfs/client-identifier.rst
> @@ -152,7 +152,7 @@ string:
> via the kernel command line, or when the "nfs" module is
> loaded.
>
> - /sys/fs/nfs/client/net/identifier
> + /sys/fs/nfs/net/nfs_client/identifier
> This virtual file, available since Linux 5.3, is local to the
> network namespace in which it is accessed and so can provide
> distinction between network namespaces (containers) when the
> @@ -164,7 +164,7 @@ then that uniquifier can be used. For example, a uniquifier might
> be formed at boot using the container's internal identifier:
>
> sha256sum /etc/machine-id | awk '{print $1}' \\
> - > /sys/fs/nfs/client/net/identifier
> + > /sys/fs/nfs/net/nfs_client/identifier
>
> Security considerations
> -----------------------
> --
> 2.31.1
>

--
Chuck Lever




2023-01-23 13:53:57

by David Wysochanski

[permalink] [raw]
Subject: Re: [PATCH] Documentation: Fix sysfs path for the NFSv4 client identifier

On Wed, Nov 9, 2022 at 8:58 AM Chuck Lever III <[email protected]> wrote:
>
>
>
> > On Nov 9, 2022, at 8:33 AM, Dave Wysochanski <[email protected]> wrote:
> >
> > The sysfs path for the NFS4 client identfier should start with
> > the path component of 'nfs' for the kset, and then the 'net'
> > path component for the netns object, followed by the
> > 'nfs_client' path component for the NFS client kobject,
> > and ending with 'identifier' for the netns_client_id
> > kobj_attribute.
>
> Seems like the redundancy has some purpose and is baked-in to
> the path. I'd rather leave the sleeping dog as it is, enjoying
> the morning sun.
>
> Reviewed-by: Chuck Lever <[email protected]>
>
Hey Chuck,

I just realized the patch is still outstanding. Now when I re-read
your comment, I'm not sure what it means. Your comment sounds
like you feel the patch may not be necessary, but then you have
a "Reviewed-by". I am not sure if you mean this should be applied or not.

To be clear, the existing sysfs path does not exist, and we got a
complaint about this documentation inaccuracy, hence my patch.


>
> > Fixes: a28faaddb2be ("Documentation: Add an explanation of NFSv4 client identifiers")
> > Signed-off-by: Dave Wysochanski <[email protected]>
> > ---
> > Documentation/filesystems/nfs/client-identifier.rst | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/filesystems/nfs/client-identifier.rst b/Documentation/filesystems/nfs/client-identifier.rst
> > index 5147e15815a1..a94c7a9748d7 100644
> > --- a/Documentation/filesystems/nfs/client-identifier.rst
> > +++ b/Documentation/filesystems/nfs/client-identifier.rst
> > @@ -152,7 +152,7 @@ string:
> > via the kernel command line, or when the "nfs" module is
> > loaded.
> >
> > - /sys/fs/nfs/client/net/identifier
> > + /sys/fs/nfs/net/nfs_client/identifier
> > This virtual file, available since Linux 5.3, is local to the
> > network namespace in which it is accessed and so can provide
> > distinction between network namespaces (containers) when the
> > @@ -164,7 +164,7 @@ then that uniquifier can be used. For example, a uniquifier might
> > be formed at boot using the container's internal identifier:
> >
> > sha256sum /etc/machine-id | awk '{print $1}' \\
> > - > /sys/fs/nfs/client/net/identifier
> > + > /sys/fs/nfs/net/nfs_client/identifier
> >
> > Security considerations
> > -----------------------
> > --
> > 2.31.1
> >
>
> --
> Chuck Lever
>
>
>


2023-01-23 14:49:00

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH] Documentation: Fix sysfs path for the NFSv4 client identifier

Hi DW -

> On Jan 23, 2023, at 8:52 AM, David Wysochanski <[email protected]> wrote:
>
> On Wed, Nov 9, 2022 at 8:58 AM Chuck Lever III <[email protected]> wrote:
>>
>>
>>
>>> On Nov 9, 2022, at 8:33 AM, Dave Wysochanski <[email protected]> wrote:
>>>
>>> The sysfs path for the NFS4 client identfier should start with
>>> the path component of 'nfs' for the kset, and then the 'net'
>>> path component for the netns object, followed by the
>>> 'nfs_client' path component for the NFS client kobject,
>>> and ending with 'identifier' for the netns_client_id
>>> kobj_attribute.
>>
>> Seems like the redundancy has some purpose and is baked-in to
>> the path. I'd rather leave the sleeping dog as it is, enjoying
>> the morning sun.
>>
>> Reviewed-by: Chuck Lever <[email protected]>
>>
> Hey Chuck,
>
> I just realized the patch is still outstanding. Now when I re-read
> your comment, I'm not sure what it means. Your comment sounds
> like you feel the patch may not be necessary, but then you have
> a "Reviewed-by".

I was responding to Ben's suggestion elsewhere that the pathname
had redundant components and should be simplified. After reviewing
the code (and your patch) it appears that all the components are
necessary to have, so the document change you proposed is
appropriate.


> I am not sure if you mean this should be applied or not.

Reviewed-by: means "OK to apply". The documentation is incorrect.


> To be clear, the existing sysfs path does not exist, and we got a
> complaint about this documentation inaccuracy, hence my patch.

Can you dig up the complaint bug and suggest a Link: tag to add?


>>> Fixes: a28faaddb2be ("Documentation: Add an explanation of NFSv4 client identifiers")
>>> Signed-off-by: Dave Wysochanski <[email protected]>
>>> ---
>>> Documentation/filesystems/nfs/client-identifier.rst | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/filesystems/nfs/client-identifier.rst b/Documentation/filesystems/nfs/client-identifier.rst
>>> index 5147e15815a1..a94c7a9748d7 100644
>>> --- a/Documentation/filesystems/nfs/client-identifier.rst
>>> +++ b/Documentation/filesystems/nfs/client-identifier.rst
>>> @@ -152,7 +152,7 @@ string:
>>> via the kernel command line, or when the "nfs" module is
>>> loaded.
>>>
>>> - /sys/fs/nfs/client/net/identifier
>>> + /sys/fs/nfs/net/nfs_client/identifier
>>> This virtual file, available since Linux 5.3, is local to the
>>> network namespace in which it is accessed and so can provide
>>> distinction between network namespaces (containers) when the
>>> @@ -164,7 +164,7 @@ then that uniquifier can be used. For example, a uniquifier might
>>> be formed at boot using the container's internal identifier:
>>>
>>> sha256sum /etc/machine-id | awk '{print $1}' \\
>>> - > /sys/fs/nfs/client/net/identifier
>>> + > /sys/fs/nfs/net/nfs_client/identifier
>>>
>>> Security considerations
>>> -----------------------
>>> --
>>> 2.31.1
>>>
>>
>> --
>> Chuck Lever

--
Chuck Lever




2023-01-23 16:09:38

by David Wysochanski

[permalink] [raw]
Subject: Re: [PATCH] Documentation: Fix sysfs path for the NFSv4 client identifier

On Mon, Jan 23, 2023 at 9:48 AM Chuck Lever III <[email protected]> wrote:
>
> Hi DW -
>
> > On Jan 23, 2023, at 8:52 AM, David Wysochanski <[email protected]> wrote:
> >
> > On Wed, Nov 9, 2022 at 8:58 AM Chuck Lever III <[email protected]> wrote:
> >>
> >>
> >>
> >>> On Nov 9, 2022, at 8:33 AM, Dave Wysochanski <[email protected]> wrote:
> >>>
> >>> The sysfs path for the NFS4 client identfier should start with
> >>> the path component of 'nfs' for the kset, and then the 'net'
> >>> path component for the netns object, followed by the
> >>> 'nfs_client' path component for the NFS client kobject,
> >>> and ending with 'identifier' for the netns_client_id
> >>> kobj_attribute.
> >>
> >> Seems like the redundancy has some purpose and is baked-in to
> >> the path. I'd rather leave the sleeping dog as it is, enjoying
> >> the morning sun.
> >>
> >> Reviewed-by: Chuck Lever <[email protected]>
> >>
> > Hey Chuck,
> >
> > I just realized the patch is still outstanding. Now when I re-read
> > your comment, I'm not sure what it means. Your comment sounds
> > like you feel the patch may not be necessary, but then you have
> > a "Reviewed-by".
>
> I was responding to Ben's suggestion elsewhere that the pathname
> had redundant components and should be simplified. After reviewing
> the code (and your patch) it appears that all the components are
> necessary to have, so the document change you proposed is
> appropriate.
>
Ah ok yes I remember now, thanks for explaining.

>
> > I am not sure if you mean this should be applied or not.
>
> Reviewed-by: means "OK to apply". The documentation is incorrect.
>
Ok.

>
> > To be clear, the existing sysfs path does not exist, and we got a
> > complaint about this documentation inaccuracy, hence my patch.
>
> Can you dig up the complaint bug and suggest a Link: tag to add?
>
It was such a small issue I was not planning to create a separate
bug, but I can do so easily if you would like that.

The comment was a private comment that came later on in this
bug, which you commented on a while back when there was discussion
on how to generate a unique id.
https://bugzilla.redhat.com/show_bug.cgi?id=1801326#c45
The bug was repurposed to documentation only.


>
> >>> Fixes: a28faaddb2be ("Documentation: Add an explanation of NFSv4 client identifiers")
> >>> Signed-off-by: Dave Wysochanski <[email protected]>
> >>> ---
> >>> Documentation/filesystems/nfs/client-identifier.rst | 4 ++--
> >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/filesystems/nfs/client-identifier.rst b/Documentation/filesystems/nfs/client-identifier.rst
> >>> index 5147e15815a1..a94c7a9748d7 100644
> >>> --- a/Documentation/filesystems/nfs/client-identifier.rst
> >>> +++ b/Documentation/filesystems/nfs/client-identifier.rst
> >>> @@ -152,7 +152,7 @@ string:
> >>> via the kernel command line, or when the "nfs" module is
> >>> loaded.
> >>>
> >>> - /sys/fs/nfs/client/net/identifier
> >>> + /sys/fs/nfs/net/nfs_client/identifier
> >>> This virtual file, available since Linux 5.3, is local to the
> >>> network namespace in which it is accessed and so can provide
> >>> distinction between network namespaces (containers) when the
> >>> @@ -164,7 +164,7 @@ then that uniquifier can be used. For example, a uniquifier might
> >>> be formed at boot using the container's internal identifier:
> >>>
> >>> sha256sum /etc/machine-id | awk '{print $1}' \\
> >>> - > /sys/fs/nfs/client/net/identifier
> >>> + > /sys/fs/nfs/net/nfs_client/identifier
> >>>
> >>> Security considerations
> >>> -----------------------
> >>> --
> >>> 2.31.1
> >>>
> >>
> >> --
> >> Chuck Lever
>
> --
> Chuck Lever
>
>
>


2023-01-23 16:11:57

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH] Documentation: Fix sysfs path for the NFSv4 client identifier



> On Jan 23, 2023, at 11:07 AM, David Wysochanski <[email protected]> wrote:
>
> On Mon, Jan 23, 2023 at 9:48 AM Chuck Lever III <[email protected]> wrote:
>>
>> Hi DW -
>>
>>> On Jan 23, 2023, at 8:52 AM, David Wysochanski <[email protected]> wrote:
>>>
>>> On Wed, Nov 9, 2022 at 8:58 AM Chuck Lever III <[email protected]> wrote:
>>>>
>>>>
>>>>
>>>>> On Nov 9, 2022, at 8:33 AM, Dave Wysochanski <[email protected]> wrote:
>>>>>
>>>>> The sysfs path for the NFS4 client identfier should start with
>>>>> the path component of 'nfs' for the kset, and then the 'net'
>>>>> path component for the netns object, followed by the
>>>>> 'nfs_client' path component for the NFS client kobject,
>>>>> and ending with 'identifier' for the netns_client_id
>>>>> kobj_attribute.
>>>>
>>>> Seems like the redundancy has some purpose and is baked-in to
>>>> the path. I'd rather leave the sleeping dog as it is, enjoying
>>>> the morning sun.
>>>>
>>>> Reviewed-by: Chuck Lever <[email protected]>
>>>>
>>> Hey Chuck,
>>>
>>> I just realized the patch is still outstanding. Now when I re-read
>>> your comment, I'm not sure what it means. Your comment sounds
>>> like you feel the patch may not be necessary, but then you have
>>> a "Reviewed-by".
>>
>> I was responding to Ben's suggestion elsewhere that the pathname
>> had redundant components and should be simplified. After reviewing
>> the code (and your patch) it appears that all the components are
>> necessary to have, so the document change you proposed is
>> appropriate.
>>
> Ah ok yes I remember now, thanks for explaining.
>
>>
>>> I am not sure if you mean this should be applied or not.
>>
>> Reviewed-by: means "OK to apply". The documentation is incorrect.
>>
> Ok.
>
>>
>>> To be clear, the existing sysfs path does not exist, and we got a
>>> complaint about this documentation inaccuracy, hence my patch.
>>
>> Can you dig up the complaint bug and suggest a Link: tag to add?
>>
> It was such a small issue I was not planning to create a separate
> bug, but I can do so easily if you would like that.
>
> The comment was a private comment that came later on in this
> bug, which you commented on a while back when there was discussion
> on how to generate a unique id.
> https://bugzilla.redhat.com/show_bug.cgi?id=1801326#c45
> The bug was repurposed to documentation only.

Link: https://bugzilla.redhat.com/show_bug.cgi?id=1801326

seems appropriate to me.



>>>>> Fixes: a28faaddb2be ("Documentation: Add an explanation of NFSv4 client identifiers")
>>>>> Signed-off-by: Dave Wysochanski <[email protected]>
>>>>> ---
>>>>> Documentation/filesystems/nfs/client-identifier.rst | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/filesystems/nfs/client-identifier.rst b/Documentation/filesystems/nfs/client-identifier.rst
>>>>> index 5147e15815a1..a94c7a9748d7 100644
>>>>> --- a/Documentation/filesystems/nfs/client-identifier.rst
>>>>> +++ b/Documentation/filesystems/nfs/client-identifier.rst
>>>>> @@ -152,7 +152,7 @@ string:
>>>>> via the kernel command line, or when the "nfs" module is
>>>>> loaded.
>>>>>
>>>>> - /sys/fs/nfs/client/net/identifier
>>>>> + /sys/fs/nfs/net/nfs_client/identifier
>>>>> This virtual file, available since Linux 5.3, is local to the
>>>>> network namespace in which it is accessed and so can provide
>>>>> distinction between network namespaces (containers) when the
>>>>> @@ -164,7 +164,7 @@ then that uniquifier can be used. For example, a uniquifier might
>>>>> be formed at boot using the container's internal identifier:
>>>>>
>>>>> sha256sum /etc/machine-id | awk '{print $1}' \\
>>>>> - > /sys/fs/nfs/client/net/identifier
>>>>> + > /sys/fs/nfs/net/nfs_client/identifier
>>>>>
>>>>> Security considerations
>>>>> -----------------------
>>>>> --
>>>>> 2.31.1
>>>>>
>>>>
>>>> --
>>>> Chuck Lever
>>
>> --
>> Chuck Lever

--
Chuck Lever




2023-01-23 16:59:37

by David Wysochanski

[permalink] [raw]
Subject: [PATCH] Documentation: Fix sysfs path for the NFSv4 client identifier

The sysfs path for the NFS4 client identfier should start with
the path component of 'nfs' for the kset, and then the 'net'
path component for the netns object, followed by the
'nfs_client' path component for the NFS client kobject,
and ending with 'identifier' for the netns_client_id
kobj_attribute.

Fixes: a28faaddb2be ("Documentation: Add an explanation of NFSv4 client identifiers")
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1801326
Reviewed-by: Chuck Lever <[email protected]>
Signed-off-by: Dave Wysochanski <[email protected]>
---
Documentation/filesystems/nfs/client-identifier.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/nfs/client-identifier.rst b/Documentation/filesystems/nfs/client-identifier.rst
index 5147e15815a1..a94c7a9748d7 100644
--- a/Documentation/filesystems/nfs/client-identifier.rst
+++ b/Documentation/filesystems/nfs/client-identifier.rst
@@ -152,7 +152,7 @@ string:
via the kernel command line, or when the "nfs" module is
loaded.

- /sys/fs/nfs/client/net/identifier
+ /sys/fs/nfs/net/nfs_client/identifier
This virtual file, available since Linux 5.3, is local to the
network namespace in which it is accessed and so can provide
distinction between network namespaces (containers) when the
@@ -164,7 +164,7 @@ then that uniquifier can be used. For example, a uniquifier might
be formed at boot using the container's internal identifier:

sha256sum /etc/machine-id | awk '{print $1}' \\
- > /sys/fs/nfs/client/net/identifier
+ > /sys/fs/nfs/net/nfs_client/identifier

Security considerations
-----------------------
--
2.31.1