Re: [PATCH 2/2] NFS: NFSERR_INVAL is not defined by NFSv2

[Tom Rini <trini@konsulko.com>]

On Fri, Aug 28, 2026 at 05:06:14PM -0400, Chuck Lever wrote:
> 
> 
> On Fri, Aug 28, 2026, at 4:51 PM, Tom Rini wrote:
> > On Fri, Aug 28, 2026 at 09:27:41AM -0400, Chuck Lever wrote:
> >> 
> >> 
> >> On Fri, Aug 28, 2026, at 7:33 AM, NeilBrown wrote:
> >> > On Fri, 28 Aug 2026, Jörg Sommer wrote:
> >> >> Chuck Lever schrieb am Di 09. Dez, 19:28 (-0500):
> >> >> > From: Chuck Lever <chuck.lever@oracle.com>
> >> >> > 
> >> >> > A documenting comment in include/uapi/linux/nfs.h claims incorrectly
> >> >> > that NFSv2 defines NFSERR_INVAL. There is no such definition in either
> >> >> > RFC 1094 or https://pubs.opengroup.org/onlinepubs/9629799/chap7.htm
> >> >> > 
> >> >> > NFS3ERR_INVAL is introduced in RFC 1813.
> >> >> 
> >> >> Hello,
> >> >> 
> >> >> since this commit 0ac903d1bfdce8ff40657c2b7d996947b72b6645 was added, U-Boot
> >> >> 2022.04 (I don't know about newer versions) can no longer access symlinks in
> >> >> NFS shares. When I revert this commit, U-Boot can load the file behind the
> >> >> symlink. Real files are no problem.
> >> >> 
> >> >> The file I want to load is a symlink at the server:
> >> >> 
> >> >> ```
> >> >> % ls -l /srv/nfs/boot/Image.gz*
> >> >> lrwxrwxrwx 1 root root      41  7. Jul 16:29 /srv/nfs-con/boot/Image.gz -> Image.gz-5.15.213-imx8mm+gfffa4b6d4aea+p1
> >> >> -rw-r--r-- 1 root root 5279373  7. Jul 16:29 /srv/nfs-con/boot/Image.gz-5.15.213-imx8mm+gfffa4b6d4aea+p1
> >> >> ```
> >> >> 
> >> >> Without this commit:
> >> >> 
> >> >> ```
> >> >> u-boot=> nfs 0x42000000 /srv/nfs/boot/Image.gz
> >> >> Filename '/srv/nfs-con/boot/Image.gz'.
> >> >> Load address: 0x42000000
> >> >> Loading: #################################################################
> >> >> done
> >> >> Bytes transferred = 5279373 (508e8d hex)
> >> >> ```
> >> >> 
> >> >> But with this commit:
> >> >> 
> >> >> ```
> >> >> u-boot=> nfs 0x42000000 /srv/nfs/boot/Image.gz
> >> >> Filename '/srv/nfs-con/boot/Image.gz'.
> >> >> Load address: 0x42000000
> >> >> Loading:
> >> >> done
> >> >> ```
> >> >> 
> >> >> This is the network traffic:
> >> >> 
> >> >> ```
> >> >> No.	Time	Protocol	Length	Info
> >> >> 3	0.000370	Portmap	98	V2 GETPORT Call (Reply In 4) MOUNT(100005) V:1 UDP
> >> >> 4	0.000607	Portmap	70	V2 GETPORT Reply (Call In 3) Port:60340
> >> >> 5	0.000949	Portmap	98	V2 GETPORT Call (Reply In 6) NFS(100003) V:2 UDP
> >> >> 6	0.001046	Portmap	70	V2 GETPORT Reply (Call In 5) Port:2049
> >> >> 7	0.001368	MOUNT	126	V2 MNT Call (Reply In 8) /srv/nfs-con/boot
> >> >> 8	0.008026	MOUNT	102	V2 MNT Reply (Call In 7)
> >> >> 9	0.008389	NFS	146	V2 LOOKUP Call (Reply In 10), DH: 0x8c279135/Image.gz
> >> >> 10	0.008754	NFS	170	V2 LOOKUP Reply (Call In 9), FH: 0xcdddf154
> >> >> 11	0.009095	NFS	146	V2 READ Call (Reply In 12), FH: 0xcdddf154 Offset: 0 Count: 1024 TotalCount: 0
> >> >> 12	0.009189	NFS	70	V2 READ Reply (Call In 11) Error: NFS2ERR_IO
> >> >> 13	0.009511	MOUNT	102	V2 UMNTALL Call (Reply In 14)
> >> >> 14	0.010301	MOUNT	66	V2 UMNTALL Reply (Call In 13)
> >> >> ```
> >> >> 
> >> >> From packet 11 V2 READ Call
> >> >> 
> >> >> ```
> >> >> Remote Procedure Call, Type:Call XID:0x000050e1
> >> >>     XID: 0x000050e1 (20705)
> >> >>     Message Type: Call (0)
> >> >>     RPC Version: 2
> >> >>     Program: NFS (100003)
> >> >>     Program Version: 2
> >> >>     Procedure: READ (6)
> >> >>     [The reply to this request is in frame 12]
> >> >>     Credentials
> >> >>         Flavor: AUTH_UNIX (1)
> >> >>         Length: 20
> >> >>         Stamp: 0x00000000
> >> >>         Machine Name: <EMPTY>
> >> >>         UID: 0
> >> >>         GID: 0
> >> >>         Auxiliary GIDs (0)
> >> >>     Verifier
> >> >>         Flavor: AUTH_NULL (0)
> >> >>         Length: 0
> >> >> Network File System, READ Call FH: 0xcdddf154 Offset: 0 Count: 1024 TotalCount: 0
> >> >>     [Program Version: 2]
> >> >>     [V2 Procedure: READ (6)]
> >> >>     file
> >> >>         [hash (CRC-32): 0xcdddf154]
> >> >>         FileHandle: 010004010100540020bb3809040054002f2edde4000000000000000000000000
> >> >>     Offset: 0
> >> >>     Count: 1024
> >> >>     Total Count: 0
> >> >> ```
> >> >> 
> >> >> From packet 12 V2 READ Reply
> >> >> 
> >> >> ```
> >> >> Remote Procedure Call, Type:Reply XID:0x000050e1
> >> >>     XID: 0x000050e1 (20705)
> >> >>     Message Type: Reply (1)
> >> >>     [Program: NFS (100003)]
> >> >>     [Program Version: 2]
> >> >>     [Procedure: READ (6)]
> >> >>     Reply State: accepted (0)
> >> >>     [This is a reply to a request in frame 11]
> >> >>     [Time from request: 94.000 microseconds]
> >> >>     Verifier
> >> >>         Flavor: AUTH_NULL (0)
> >> >>         Length: 0
> >> >>     Accept State: RPC executed successfully (0)
> >> >> Network File System, READ Reply  Error: NFS2ERR_IO
> >> >>     [Program Version: 2]
> >> >>     [V2 Procedure: READ (6)]
> >> >>     Status: NFS2ERR_IO (5)
> >> >> ```
> >> >> 
> >> >> 
> >> >> I can locally revert this commit in our kernel, but is there any other way
> >> >> to get it working again?
> >> >
> >> > This is unfortunate.
> >> > U-boot has
> >> >
> >> > 		} else if ((rlen == -NFSERR_ISDIR) || (rlen == -NFSERR_INVAL)) {
> >> > 			/* symbolic link */
> >> > 			nfs_state = STATE_READLINK_REQ;
> >> > 			nfs_send();
> >> >
> >> > so we either need nfsdv2 to return NFSERR_ISDIR (for something that
> >> > isn't a directory) or NFSERR_INVAL (which is not a valid v2 error code),
> >> > or change u-boot to also check for NFSERR_IO (which is 5).
> >> 
> >> U-Boot's NFSERR_ISDIR check (quoted above) is the Sun-compatible
> >> path. The NFSERR_INVAL arm was presumably added for Linux NFSD?
> >> 
> >> So the pre-0ac903d1bfdc code is not something I want to go back to.
> >> Returning NFSERR_INVAL is incompatible with the reference NFSv2
> >> implementation, and so is NFSERR_IO.
> >> 
> >> Mapping nfserr_symlink to NFSERR_ISDIR affects the READ, WRITE, and
> >> SETATTR procedures. The Solaris NFS server returns NFSERR_ISDIR for
> >> READ and WRITE of a non-REG object, which suggests to me that is
> >> the behavior clients will expect.
> >> 
> >> IMO we should leave nfserr_wrong_type alone until we have a specific
> >> real-world complaint to address.
> >
> > On the U-Boot side of things, the code in question dates back to when
> > NFS support was originally added in 2004. The history here says that
> > someone added the symlink support on top of what was borrowed from
> > NetBSD, so yes, we're likely the originator of the incorrect behavior.
> > But also, this means that everything U-Boot that's ever been using this
> > feature now fails. And given the lag between the kernel change and the
> > bug report, it's not something that's in rapidly changing areas and
> > probably also in deployments that can't be updated. So perhaps things do
> > need to go back to the way it was.
> 
> That argument doesn't make sense to me, and moves NFSD backwards
> instead of forward.

I don't quite follow. The series here talks about correctness changes
that were found by comparison with the relevant documentation and not
about behavior with production / deployed systems. And it's not about
something new, it's about v2 support. On the U-Boot side I was ready to
drop it, except it turns out there's people still actively using it
today.

But also, doesn't this fall under the "you can't break ABI" side of
things? Especially since people have been using it for decades and
there's (seemingly) not some sort of security implication to the fix
(and we're talking about NFSv2, so...).

> Reverting the commit is fine for stable kernels, but getting the
> revert into mainline will take just as long as fixing it correctly.

I disagree about the speed of fixing it (being U-Boot) correctly. I have
a fix from Jörg today, and it could be in our October release, yes. When
will that actually be seen by people using U-Boot? Well, maybe newer
products based on... Fedora will pick up v2027.04, some semis will base
on v2027.01, but most places? Probably never.

> The "deployments can't be updated" also doesn't make sense: if
> they can't be updated, they won't be able to get the revert
> either.

By deployments I mean U-Boot deployments. It's notoriously hard to
update firmwares most of the time. Some testing labs refuse to because
of the value in testing the firmware most people likely have deployed
rather than the latest and greatest. So most of the time, it's never
updated. Can newer systems take advantage of good and specification
following update mechanisms? Sure. In practice? Only maybe.

-- 
Tom