Clarification on PyNFS Test Case Behavior for st_lookupp.testLink in Versions 4.0 and 4.1

Clarification on PyNFS Test Case Behavior for st_lookupp.testLink in Versions 4.0 and 4.1

[Gaikwad, Shubham]

Hi Bruce/PyNFS team,
I hope this email finds you well.

I am reaching out to seek clarification regarding the PyNFS test case 'st_lookupp.testLink' (flag: lookupp, code: LOOKP2a/LKPP1a) included in the PyNFS test suite for v4.0 and v4.1. Specifically, I would like to understand the expected behavior relating to the error codes for this test case.

In the PyNFS test suite for v4.0, the test case LOOKP2a (located at nfs4.0/servertests/st_lookupp.py::testLink) initially checked for the error code NFS4ERR_NOTDIR. Subsequently, an update was made to this test case to also expect NFS4ERR_SYMLINK in addition to NFS4ERR_NOTDIR [reference: git.linux-nfs.org Git - bfields/pynfs.git/commitdiff]. However, in the PyNFS test suite for v4.1, the corresponding test case LKPP1a (located at nfs4.1/server41tests/st_lookupp.py::testLink) continues to check only for NFS4ERR_SYMLINK as the expected error code.

Given the discrepancy, should the test case for v4.1 (LKPP1a) be updated to also check for NFS4ERR_NOTDIR, similar to the modification made for the v4.0 test case (LOOKP2a)? Additionally, while the RFC 8881 section on the lookupp operation [reference: Section 18.14: Operation 16: LOOKUPP - Lookup Parent Directory] does not explicitly mention the error code NFS4ERR_SYMLINK, it is noted in Sections 15.2 [reference: Operations and Their Valid Errors] and section 15.4 [reference: Errors and the Operations That Use Them]. Therefore, would it be reasonable to update the test case LKPP1a to allow both NFS4ERR_SYMLINK and NFS4ERR_NOTDIR as valid error codes, ensuring the test case passes if either error code is received from the server?

Your insight on this matter would be greatly appreciated.

References:
1. git.linux-nfs.org Git - bfields/pynfs.git/commitdiff -- https://git.linux-nfs.org/?p=bfields/pynfs.git;a=commitdiff;h=6044afcc8ab7deea1beb77be53956fc36713a5b3;hp=19e4c878f8538881af2b6e83672fb94d785aea33
2. Section 18.14: Operation 16: LOOKUPP - Lookup Parent Directory -- https://www.rfc-editor.org/rfc/rfc8881.html#name-operation-16-lookupp-lookup
3. Operations and Their Valid Errors -- https://www.rfc-editor.org/rfc/rfc8881.html#name-operations-and-their-valid-
4. Errors and the Operations That Use Them -- https://www.rfc-editor.org/rfc/rfc8881.html#name-errors-and-the-operations-t

Best regards,
Shubham Gaikwad


Internal Use - Confidential

Clarification on PyNFS Test Case Behavior for st_lookupp.testLink in Versions 4.0 and 4.1

[Gaikwad, Shubham]

Hi Bruce/PyNFS team,
I hope this email finds you well.

I am reaching out to seek clarification regarding the PyNFS test case 'st_lookupp.testLink' (flag: lookupp, code: LOOKP2a/LKPP1a) included in the PyNFS test suite for v4.0 and v4.1. Specifically, I would like to understand the expected behavior relating to the error codes for this test case.

In the PyNFS test suite for v4.0, the test case LOOKP2a (located at nfs4.0/servertests/st_lookupp.py::testLink) initially checked for the error code NFS4ERR_NOTDIR. Subsequently, an update was made to this test case to also expect NFS4ERR_SYMLINK in addition to NFS4ERR_NOTDIR [reference: git.linux-nfs.org Git - bfields/pynfs.git/commitdiff]. However, in the PyNFS test suite for v4.1, the corresponding test case LKPP1a (located at nfs4.1/server41tests/st_lookupp.py::testLink) continues to check only for NFS4ERR_SYMLINK as the expected error code.

Given the discrepancy, should the test case for v4.1 (LKPP1a) be updated to also check for NFS4ERR_NOTDIR, similar to the modification made for the v4.0 test case (LOOKP2a)? Additionally, while the RFC 8881 section on the lookupp operation [reference: Section 18.14: Operation 16: LOOKUPP - Lookup Parent Directory] does not explicitly mention the error code NFS4ERR_SYMLINK, it is noted in Sections 15.2 [reference: Operations and Their Valid Errors] and section 15.4 [reference: Errors and the Operations That Use Them]. Therefore, would it be reasonable to update the test case LKPP1a to allow both NFS4ERR_SYMLINK and NFS4ERR_NOTDIR as valid error codes, ensuring the test case passes if either error code is received from the server?

Your insight on this matter would be greatly appreciated.

References:
1. git.linux-nfs.org Git - bfields/pynfs.git/commitdiff -- https://git.linux-nfs.org/?p=bfields/pynfs.git;a=commitdiff;h=6044afcc8ab7deea1beb77be53956fc36713a5b3;hp=19e4c878f8538881af2b6e83672fb94d785aea33
2. Section 18.14: Operation 16: LOOKUPP - Lookup Parent Directory -- https://www.rfc-editor.org/rfc/rfc8881.html#name-operation-16-lookupp-lookup
3. Operations and Their Valid Errors -- https://www.rfc-editor.org/rfc/rfc8881.html#name-operations-and-their-valid-
4. Errors and the Operations That Use Them -- https://www.rfc-editor.org/rfc/rfc8881.html#name-errors-and-the-operations-t

Best regards,
Shubham Gaikwad

Re: Clarification on PyNFS Test Case Behavior for st_lookupp.testLink in Versions 4.0 and 4.1

[J. Bruce Fields]

On Wed, Jan 01, 2025 at 06:28:12AM +0000, Gaikwad, Shubham wrote:
> Hi Bruce/PyNFS team,
> I hope this email finds you well.

Thanks, yes, but I'm not actually maintaining pynfs anymore and I don't
remember who is....

> I am reaching out to seek clarification regarding the PyNFS test case
> 'st_lookupp.testLink' (flag: lookupp, code: LOOKP2a/LKPP1a) included
> in the PyNFS test suite for v4.0 and v4.1. Specifically, I would like
> to understand the expected behavior relating to the error codes for
> this test case.
> 
> In the PyNFS test suite for v4.0, the test case LOOKP2a (located at
> nfs4.0/servertests/st_lookupp.py::testLink) initially checked for the
> error code NFS4ERR_NOTDIR. Subsequently, an update was made to this
> test case to also expect NFS4ERR_SYMLINK in addition to NFS4ERR_NOTDIR
> [reference: git.linux-nfs.org Git - bfields/pynfs.git/commitdiff].
> However, in the PyNFS test suite for v4.1, the corresponding test case
> LKPP1a (located at nfs4.1/server41tests/st_lookupp.py::testLink)
> continues to check only for NFS4ERR_SYMLINK as the expected error
> code.
> 
> Given the discrepancy, should the test case for v4.1 (LKPP1a) be
> updated to also check for NFS4ERR_NOTDIR, similar to the modification
> made for the v4.0 test case (LOOKP2a)? Additionally, while the RFC
> 8881 section on the lookupp operation [reference: Section 18.14:
> Operation 16: LOOKUPP - Lookup Parent Directory] does not explicitly
> mention the error code NFS4ERR_SYMLINK, it is noted in Sections 15.2
> [reference: Operations and Their Valid Errors] and section 15.4
> [reference: Errors and the Operations That Use Them]. Therefore, would
> it be reasonable to update the test case LKPP1a to allow both
> NFS4ERR_SYMLINK and NFS4ERR_NOTDIR as valid error codes, ensuring the
> test case passes if either error code is received from the server?

Not sure!  In theory I guess 4.1 could be stricter about the error code
than 4.0.  Language for other operations (e.g. LOOKUP, 18.13.4) does
seem to prefer ERR_SYMLINK in the case of a symlink.  I think the Linux
client probably only uses LOOKUPP in the export code, where the choice
of error is unlikely to matter.

I'd guess it doesn't matter much.  What motivates the question?

--b.

> 
> Your insight on this matter would be greatly appreciated.
> 
> References: 1. git.linux-nfs.org Git - bfields/pynfs.git/commitdiff --
> https://git.linux-nfs.org/?p=bfields/pynfs.git;a=commitdiff;h=6044afcc8ab7deea1beb77be53956fc36713a5b3;hp=19e4c878f8538881af2b6e83672fb94d785aea33
> 2. Section 18.14: Operation 16: LOOKUPP - Lookup Parent Directory --
> https://www.rfc-editor.org/rfc/rfc8881.html#name-operation-16-lookupp-lookup
> 3. Operations and Their Valid Errors --
> https://www.rfc-editor.org/rfc/rfc8881.html#name-operations-and-their-valid-
> 4. Errors and the Operations That Use Them --
> https://www.rfc-editor.org/rfc/rfc8881.html#name-errors-and-the-operations-t
> 
> Best regards, Shubham Gaikwad

Re: Clarification on PyNFS Test Case Behavior for st_lookupp.testLink in Versions 4.0 and 4.1

[Chuck Lever]

On 1/1/25 4:38 PM, J. Bruce Fields wrote:
> On Wed, Jan 01, 2025 at 06:28:12AM +0000, Gaikwad, Shubham wrote:
>> Hi Bruce/PyNFS team,
>> I hope this email finds you well.
> 
> Thanks, yes, but I'm not actually maintaining pynfs anymore and I don't
> remember who is....

Calum Mackay, copied.


>> I am reaching out to seek clarification regarding the PyNFS test case
>> 'st_lookupp.testLink' (flag: lookupp, code: LOOKP2a/LKPP1a) included
>> in the PyNFS test suite for v4.0 and v4.1. Specifically, I would like
>> to understand the expected behavior relating to the error codes for
>> this test case.
>>
>> In the PyNFS test suite for v4.0, the test case LOOKP2a (located at
>> nfs4.0/servertests/st_lookupp.py::testLink) initially checked for the
>> error code NFS4ERR_NOTDIR. Subsequently, an update was made to this
>> test case to also expect NFS4ERR_SYMLINK in addition to NFS4ERR_NOTDIR
>> [reference: git.linux-nfs.org Git - bfields/pynfs.git/commitdiff].
>> However, in the PyNFS test suite for v4.1, the corresponding test case
>> LKPP1a (located at nfs4.1/server41tests/st_lookupp.py::testLink)
>> continues to check only for NFS4ERR_SYMLINK as the expected error
>> code.
>>
>> Given the discrepancy, should the test case for v4.1 (LKPP1a) be
>> updated to also check for NFS4ERR_NOTDIR, similar to the modification
>> made for the v4.0 test case (LOOKP2a)? Additionally, while the RFC
>> 8881 section on the lookupp operation [reference: Section 18.14:
>> Operation 16: LOOKUPP - Lookup Parent Directory] does not explicitly
>> mention the error code NFS4ERR_SYMLINK, it is noted in Sections 15.2
>> [reference: Operations and Their Valid Errors] and section 15.4
>> [reference: Errors and the Operations That Use Them]. Therefore, would
>> it be reasonable to update the test case LKPP1a to allow both
>> NFS4ERR_SYMLINK and NFS4ERR_NOTDIR as valid error codes, ensuring the
>> test case passes if either error code is received from the server?
> 
> Not sure!  In theory I guess 4.1 could be stricter about the error code
> than 4.0.  Language for other operations (e.g. LOOKUP, 18.13.4) does
> seem to prefer ERR_SYMLINK in the case of a symlink.  I think the Linux
> client probably only uses LOOKUPP in the export code, where the choice
> of error is unlikely to matter.

There are some differences between NFSv4.0 and NFSv4.1. Neil recently
did some work in this area. See:

https://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git/commit/?id=438f81e0e92a780b117097503599eb030b77dabe

Perhaps he missed a spot.


> I'd guess it doesn't matter much.  What motivates the question?
> 
> --b.
> 
>>
>> Your insight on this matter would be greatly appreciated.
>>
>> References: 1. git.linux-nfs.org Git - bfields/pynfs.git/commitdiff --
>> https://git.linux-nfs.org/?p=bfields/pynfs.git;a=commitdiff;h=6044afcc8ab7deea1beb77be53956fc36713a5b3;hp=19e4c878f8538881af2b6e83672fb94d785aea33
>> 2. Section 18.14: Operation 16: LOOKUPP - Lookup Parent Directory --
>> https://www.rfc-editor.org/rfc/rfc8881.html#name-operation-16-lookupp-lookup
>> 3. Operations and Their Valid Errors --
>> https://www.rfc-editor.org/rfc/rfc8881.html#name-operations-and-their-valid-
>> 4. Errors and the Operations That Use Them --
>> https://www.rfc-editor.org/rfc/rfc8881.html#name-errors-and-the-operations-t
>>
>> Best regards, Shubham Gaikwad
> 


-- 
Chuck Lever

Re: Clarification on PyNFS Test Case Behavior for st_lookupp.testLink in Versions 4.0 and 4.1

[Trond Myklebust]

On Wed, 2025-01-01 at 06:28 +0000, Gaikwad, Shubham wrote:
> Hi Bruce/PyNFS team,
> I hope this email finds you well.
> 
> I am reaching out to seek clarification regarding the PyNFS test case
> 'st_lookupp.testLink' (flag: lookupp, code: LOOKP2a/LKPP1a) included
> in the PyNFS test suite for v4.0 and v4.1. Specifically, I would like
> to understand the expected behavior relating to the error codes for
> this test case.
> 
> In the PyNFS test suite for v4.0, the test case LOOKP2a (located at
> nfs4.0/servertests/st_lookupp.py::testLink) initially checked for the
> error code NFS4ERR_NOTDIR. Subsequently, an update was made to this
> test case to also expect NFS4ERR_SYMLINK in addition to
> NFS4ERR_NOTDIR [reference: git.linux-nfs.org Git -
> bfields/pynfs.git/commitdiff]. However, in the PyNFS test suite for
> v4.1, the corresponding test case LKPP1a (located at
> nfs4.1/server41tests/st_lookupp.py::testLink) continues to check only
> for NFS4ERR_SYMLINK as the expected error code.
> 
> Given the discrepancy, should the test case for v4.1 (LKPP1a) be
> updated to also check for NFS4ERR_NOTDIR, similar to the modification
> made for the v4.0 test case (LOOKP2a)? Additionally, while the RFC
> 8881 section on the lookupp operation [reference: Section 18.14:
> Operation 16: LOOKUPP - Lookup Parent Directory] does not explicitly
> mention the error code NFS4ERR_SYMLINK, it is noted in Sections 15.2
> [reference: Operations and Their Valid Errors] and section 15.4
> [reference: Errors and the Operations That Use Them]. Therefore,
> would it be reasonable to update the test case LKPP1a to allow both
> NFS4ERR_SYMLINK and NFS4ERR_NOTDIR as valid error codes, ensuring the
> test case passes if either error code is received from the server?
> 
> Your insight on this matter would be greatly appreciated.
> 
> References:
> 1. git.linux-nfs.org Git - bfields/pynfs.git/commitdiff --
> https://git.linux-nfs.org/?p=bfields/pynfs.git;a=commitdiff;h=6044afcc8ab7deea1beb77be53956fc36713a5b3;hp=19e4c878f8538881af2b6e83672fb94d785aea33
> 2. Section 18.14: Operation 16: LOOKUPP - Lookup Parent Directory --
> https://www.rfc-editor.org/rfc/rfc8881.html#name-operation-16-lookupp-lookup
> 3. Operations and Their Valid Errors --
> https://www.rfc-editor.org/rfc/rfc8881.html#name-operations-and-their-valid
> -
> 4. Errors and the Operations That Use Them --
> https://www.rfc-editor.org/rfc/rfc8881.html#name-errors-and-the-operations-t
> 
> Best regards,
> Shubham Gaikwad
> 
> 

Both RFC7530 Section 16.14.5 and RFC8881 Section 18.14.3 are adamant
that:

   If the current filehandle is not a directory or named attribute
   directory, the error NFS4ERR_NOTDIR is returned.

While that doesn't say MUST return NFS4ERR_NOTDIR, it is pretty clear
that conforming servers need to have a good reason for why they would
prefer to return NFS4ERR_SYMLINK. It's not as if the client can handle
things differently in the case where it knows it has a symlink as
opposed to a regular file.

As for LOOKUP, there again, the spec is clear but fails to use
normative language:
RFC7530 Section 16.13.5 and RFC8881 Section 18.13.4 both state that

   If the current filehandle supplied is not a directory but a symbolic
   link, the error NFS4ERR_SYMLINK is returned as the error.  For all
   other non-directory file types, the error NFS4ERR_NOTDIR is
returned.

Here, there is a very good reason to want to return NFS4ERR_SYMLINK
rather than NFS4ERR_NOTDIR: the client will need to resolve the symlink
using READLINK in order to resolve the path (i.e. it handles the
NFS4ERR_SYMLINK error differently than it would resolve
NFS4ERR_NOTDIR).
Note that OPEN has the exact same requirements as LOOKUP and for the
same reason.


So ideally, pynfs should reflect both these requirements:

Yes, it is true that NFS4ERR_SYMLINK is an allowed error for LOOKUPP,
but it makes no sense to return it, so pynfs should at least warn that
the server is doing something stupid.

As for the return value from LOOKUP, it should probably disallow
altogether returning NFS4ERR_NOTDIR in the case where the filehandle
represents a symlink, for the above mentioned very good reason.

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

Re: Clarification on PyNFS Test Case Behavior for st_lookupp.testLink in Versions 4.0 and 4.1

[Calum Mackay]

On 02/01/2025 2:18 am, Trond Myklebust wrote:
> On Wed, 2025-01-01 at 06:28 +0000, Gaikwad, Shubham wrote:
>> Hi Bruce/PyNFS team,
>> I hope this email finds you well.
>>
>> I am reaching out to seek clarification regarding the PyNFS test case
>> 'st_lookupp.testLink' (flag: lookupp, code: LOOKP2a/LKPP1a) included
>> in the PyNFS test suite for v4.0 and v4.1. Specifically, I would like
>> to understand the expected behavior relating to the error codes for
>> this test case.
>>
>> In the PyNFS test suite for v4.0, the test case LOOKP2a (located at
>> nfs4.0/servertests/st_lookupp.py::testLink) initially checked for the
>> error code NFS4ERR_NOTDIR. Subsequently, an update was made to this
>> test case to also expect NFS4ERR_SYMLINK in addition to
>> NFS4ERR_NOTDIR [reference: git.linux-nfs.org Git -
>> bfields/pynfs.git/commitdiff]. However, in the PyNFS test suite for
>> v4.1, the corresponding test case LKPP1a (located at
>> nfs4.1/server41tests/st_lookupp.py::testLink) continues to check only
>> for NFS4ERR_SYMLINK as the expected error code.
>>
>> Given the discrepancy, should the test case for v4.1 (LKPP1a) be
>> updated to also check for NFS4ERR_NOTDIR, similar to the modification
>> made for the v4.0 test case (LOOKP2a)? Additionally, while the RFC
>> 8881 section on the lookupp operation [reference: Section 18.14:
>> Operation 16: LOOKUPP - Lookup Parent Directory] does not explicitly
>> mention the error code NFS4ERR_SYMLINK, it is noted in Sections 15.2
>> [reference: Operations and Their Valid Errors] and section 15.4
>> [reference: Errors and the Operations That Use Them]. Therefore,
>> would it be reasonable to update the test case LKPP1a to allow both
>> NFS4ERR_SYMLINK and NFS4ERR_NOTDIR as valid error codes, ensuring the
>> test case passes if either error code is received from the server?
>>
>> Your insight on this matter would be greatly appreciated.
>>
>> References:
>> 1. git.linux-nfs.org Git - bfields/pynfs.git/commitdiff --
>> https://git.linux-nfs.org/?p=bfields/pynfs.git;a=commitdiff;h=6044afcc8ab7deea1beb77be53956fc36713a5b3;hp=19e4c878f8538881af2b6e83672fb94d785aea33
>> 2. Section 18.14: Operation 16: LOOKUPP - Lookup Parent Directory --
>> https://www.rfc-editor.org/rfc/rfc8881.html#name-operation-16-lookupp-lookup
>> 3. Operations and Their Valid Errors --
>> https://www.rfc-editor.org/rfc/rfc8881.html#name-operations-and-their-valid
>> -
>> 4. Errors and the Operations That Use Them --
>> https://www.rfc-editor.org/rfc/rfc8881.html#name-errors-and-the-operations-t
>>
>> Best regards,
>> Shubham Gaikwad
>>
>>
> 
> Both RFC7530 Section 16.14.5 and RFC8881 Section 18.14.3 are adamant
> that:
> 
>     If the current filehandle is not a directory or named attribute
>     directory, the error NFS4ERR_NOTDIR is returned.
> 
> While that doesn't say MUST return NFS4ERR_NOTDIR, it is pretty clear
> that conforming servers need to have a good reason for why they would
> prefer to return NFS4ERR_SYMLINK. It's not as if the client can handle
> things differently in the case where it knows it has a symlink as
> opposed to a regular file.
> 
> As for LOOKUP, there again, the spec is clear but fails to use
> normative language:
> RFC7530 Section 16.13.5 and RFC8881 Section 18.13.4 both state that
> 
>     If the current filehandle supplied is not a directory but a symbolic
>     link, the error NFS4ERR_SYMLINK is returned as the error.  For all
>     other non-directory file types, the error NFS4ERR_NOTDIR is
> returned.
> 
> Here, there is a very good reason to want to return NFS4ERR_SYMLINK
> rather than NFS4ERR_NOTDIR: the client will need to resolve the symlink
> using READLINK in order to resolve the path (i.e. it handles the
> NFS4ERR_SYMLINK error differently than it would resolve
> NFS4ERR_NOTDIR).
> Note that OPEN has the exact same requirements as LOOKUP and for the
> same reason.
> 
> 
> So ideally, pynfs should reflect both these requirements:
> 
> Yes, it is true that NFS4ERR_SYMLINK is an allowed error for LOOKUPP,
> but it makes no sense to return it, so pynfs should at least warn that
> the server is doing something stupid.
> 
> As for the return value from LOOKUP, it should probably disallow
> altogether returning NFS4ERR_NOTDIR in the case where the filehandle
> represents a symlink, for the above mentioned very good reason.

Thanks Trond; I'll adjust pynfs as required.

Thanks Shubham for raising this.


Happy new year, all; bliadhna mhath ùr

best wishes,
calum.