Linux NFS development
 help / color / mirror / Atom feed
* [PATCH 0/2] Address minor issues with status codes
@ 2025-12-10  0:28 Chuck Lever
  2025-12-10  0:28 ` [PATCH 1/2] NFSD: Remove NFSERR_EAGAIN Chuck Lever
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Chuck Lever @ 2025-12-10  0:28 UTC (permalink / raw)
  To: NeilBrown, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: linux-nfs, Chuck Lever

From: Chuck Lever <chuck.lever@oracle.com>

Here are two examples of minor bugs I found when comparing NFSD's
human-generated status code definitions with equivalent xdrgen-
generated headers.

Chuck Lever (2):
  NFSD: Remove NFSERR_EAGAIN
  NFS: NFSERR_INVAL is not defined by NFSv2

 fs/nfs_common/common.c   | 1 -
 fs/nfsd/nfs4proc.c       | 2 +-
 fs/nfsd/nfsd.h           | 1 -
 fs/nfsd/nfsproc.c        | 2 +-
 include/trace/misc/nfs.h | 2 --
 include/uapi/linux/nfs.h | 3 +--
 6 files changed, 3 insertions(+), 8 deletions(-)

-- 
2.52.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] NFSD: Remove NFSERR_EAGAIN
  2025-12-10  0:28 [PATCH 0/2] Address minor issues with status codes Chuck Lever
@ 2025-12-10  0:28 ` Chuck Lever
  2025-12-10  0:28 ` [PATCH 2/2] NFS: NFSERR_INVAL is not defined by NFSv2 Chuck Lever
  2025-12-10  0:49 ` [PATCH 0/2] Address minor issues with status codes Jeff Layton
  2 siblings, 0 replies; 5+ messages in thread
From: Chuck Lever @ 2025-12-10  0:28 UTC (permalink / raw)
  To: NeilBrown, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: linux-nfs, Chuck Lever

From: Chuck Lever <chuck.lever@oracle.com>

I haven't found an NFSERR_EAGAIN in RFCs 1094, 1813, 7530, or 8881.
None of these RFCs have an NFS status code that match the numeric
value "11".

Based on the meaning of the EAGAIN errno, I presume the use of this
status in NFSD means NFS4ERR_DELAY. So replace the one usage of
nfserr_eagain, and remove it from NFSD's NFS status conversion
tables.

As far as I can tell, NFSERR_EAGAIN has existed since the pre-git
era, but was not actually used by any code until commit f4e44b393389
("NFSD: delay unmount source's export after inter-server copy
completed."), at which time it become possible for NFSD to return
a status code of 11 (which is not valid NFS protocol).

Fixes: f4e44b393389 ("NFSD: delay unmount source's export after inter-server copy completed.")
X-Cc: stable@vger.kernel.org
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfs_common/common.c   | 1 -
 fs/nfsd/nfs4proc.c       | 2 +-
 fs/nfsd/nfsd.h           | 1 -
 include/trace/misc/nfs.h | 2 --
 include/uapi/linux/nfs.h | 1 -
 5 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/fs/nfs_common/common.c b/fs/nfs_common/common.c
index af09aed09fd2..0778743ae2c2 100644
--- a/fs/nfs_common/common.c
+++ b/fs/nfs_common/common.c
@@ -17,7 +17,6 @@ static const struct {
 	{ NFSERR_NOENT,		-ENOENT		},
 	{ NFSERR_IO,		-EIO		},
 	{ NFSERR_NXIO,		-ENXIO		},
-/*	{ NFSERR_EAGAIN,	-EAGAIN		}, */
 	{ NFSERR_ACCES,		-EACCES		},
 	{ NFSERR_EXIST,		-EEXIST		},
 	{ NFSERR_XDEV,		-EXDEV		},
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 814d78f23a03..4c708cf02849 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1504,7 +1504,7 @@ static __be32 nfsd4_ssc_setup_dul(struct nfsd_net *nn, char *ipaddr,
 					(schedule_timeout(20*HZ) == 0)) {
 				finish_wait(&nn->nfsd_ssc_waitq, &wait);
 				kfree(work);
-				return nfserr_eagain;
+				return nfserr_jukebox;
 			}
 			finish_wait(&nn->nfsd_ssc_waitq, &wait);
 			goto try_again;
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 50be785f1d2c..b0283213a8f5 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -233,7 +233,6 @@ void		nfsd_lockd_shutdown(void);
 #define	nfserr_noent		cpu_to_be32(NFSERR_NOENT)
 #define	nfserr_io		cpu_to_be32(NFSERR_IO)
 #define	nfserr_nxio		cpu_to_be32(NFSERR_NXIO)
-#define	nfserr_eagain		cpu_to_be32(NFSERR_EAGAIN)
 #define	nfserr_acces		cpu_to_be32(NFSERR_ACCES)
 #define	nfserr_exist		cpu_to_be32(NFSERR_EXIST)
 #define	nfserr_xdev		cpu_to_be32(NFSERR_XDEV)
diff --git a/include/trace/misc/nfs.h b/include/trace/misc/nfs.h
index c82233e950ac..a394b4d38e18 100644
--- a/include/trace/misc/nfs.h
+++ b/include/trace/misc/nfs.h
@@ -16,7 +16,6 @@ TRACE_DEFINE_ENUM(NFSERR_PERM);
 TRACE_DEFINE_ENUM(NFSERR_NOENT);
 TRACE_DEFINE_ENUM(NFSERR_IO);
 TRACE_DEFINE_ENUM(NFSERR_NXIO);
-TRACE_DEFINE_ENUM(NFSERR_EAGAIN);
 TRACE_DEFINE_ENUM(NFSERR_ACCES);
 TRACE_DEFINE_ENUM(NFSERR_EXIST);
 TRACE_DEFINE_ENUM(NFSERR_XDEV);
@@ -52,7 +51,6 @@ TRACE_DEFINE_ENUM(NFSERR_JUKEBOX);
 		{ NFSERR_NXIO,			"NXIO" }, \
 		{ ECHILD,			"CHILD" }, \
 		{ ETIMEDOUT,			"TIMEDOUT" }, \
-		{ NFSERR_EAGAIN,		"AGAIN" }, \
 		{ NFSERR_ACCES,			"ACCES" }, \
 		{ NFSERR_EXIST,			"EXIST" }, \
 		{ NFSERR_XDEV,			"XDEV" }, \
diff --git a/include/uapi/linux/nfs.h b/include/uapi/linux/nfs.h
index f356f2ba3814..71c7196d3281 100644
--- a/include/uapi/linux/nfs.h
+++ b/include/uapi/linux/nfs.h
@@ -49,7 +49,6 @@
 	NFSERR_NOENT = 2,		/* v2 v3 v4 */
 	NFSERR_IO = 5,			/* v2 v3 v4 */
 	NFSERR_NXIO = 6,		/* v2 v3 v4 */
-	NFSERR_EAGAIN = 11,		/* v2 v3 */
 	NFSERR_ACCES = 13,		/* v2 v3 v4 */
 	NFSERR_EXIST = 17,		/* v2 v3 v4 */
 	NFSERR_XDEV = 18,		/*    v3 v4 */
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] NFS: NFSERR_INVAL is not defined by NFSv2
  2025-12-10  0:28 [PATCH 0/2] Address minor issues with status codes Chuck Lever
  2025-12-10  0:28 ` [PATCH 1/2] NFSD: Remove NFSERR_EAGAIN Chuck Lever
@ 2025-12-10  0:28 ` Chuck Lever
  2025-12-10  0:49 ` [PATCH 0/2] Address minor issues with status codes Jeff Layton
  2 siblings, 0 replies; 5+ messages in thread
From: Chuck Lever @ 2025-12-10  0:28 UTC (permalink / raw)
  To: NeilBrown, Jeff Layton, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: linux-nfs, Chuck Lever

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.

NFSD returns NFSERR_INVAL for PROC_GETACL, which has no
specification (yet).

However, nfsd_map_status() maps nfserr_symlink and nfserr_wrong_type
to nfserr_inval, which does not align with RFC 1094. This logic was
introduced only recently by commit 438f81e0e92a ("nfsd: move error
choice for incorrect object types to version-specific code."). Given
that we have no INVAL or SERVERFAULT status in NFSv2, probably the
only choice is NFSERR_IO.

Fixes: 438f81e0e92a ("nfsd: move error choice for incorrect object types to version-specific code.")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfsproc.c        | 2 +-
 include/uapi/linux/nfs.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index 481e789a7697..8873033d1e82 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -33,7 +33,7 @@ static __be32 nfsd_map_status(__be32 status)
 		break;
 	case nfserr_symlink:
 	case nfserr_wrong_type:
-		status = nfserr_inval;
+		status = nfserr_io;
 		break;
 	}
 	return status;
diff --git a/include/uapi/linux/nfs.h b/include/uapi/linux/nfs.h
index 71c7196d3281..e629c4953534 100644
--- a/include/uapi/linux/nfs.h
+++ b/include/uapi/linux/nfs.h
@@ -55,7 +55,7 @@
 	NFSERR_NODEV = 19,		/* v2 v3 v4 */
 	NFSERR_NOTDIR = 20,		/* v2 v3 v4 */
 	NFSERR_ISDIR = 21,		/* v2 v3 v4 */
-	NFSERR_INVAL = 22,		/* v2 v3 v4 */
+	NFSERR_INVAL = 22,		/*    v3 v4 */
 	NFSERR_FBIG = 27,		/* v2 v3 v4 */
 	NFSERR_NOSPC = 28,		/* v2 v3 v4 */
 	NFSERR_ROFS = 30,		/* v2 v3 v4 */
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] Address minor issues with status codes
  2025-12-10  0:28 [PATCH 0/2] Address minor issues with status codes Chuck Lever
  2025-12-10  0:28 ` [PATCH 1/2] NFSD: Remove NFSERR_EAGAIN Chuck Lever
  2025-12-10  0:28 ` [PATCH 2/2] NFS: NFSERR_INVAL is not defined by NFSv2 Chuck Lever
@ 2025-12-10  0:49 ` Jeff Layton
  2025-12-10  1:01   ` NeilBrown
  2 siblings, 1 reply; 5+ messages in thread
From: Jeff Layton @ 2025-12-10  0:49 UTC (permalink / raw)
  To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
  Cc: linux-nfs, Chuck Lever

On Tue, 2025-12-09 at 19:28 -0500, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> Here are two examples of minor bugs I found when comparing NFSD's
> human-generated status code definitions with equivalent xdrgen-
> generated headers.
> 
> Chuck Lever (2):
>   NFSD: Remove NFSERR_EAGAIN
>   NFS: NFSERR_INVAL is not defined by NFSv2
> 
>  fs/nfs_common/common.c   | 1 -
>  fs/nfsd/nfs4proc.c       | 2 +-
>  fs/nfsd/nfsd.h           | 1 -
>  fs/nfsd/nfsproc.c        | 2 +-
>  include/trace/misc/nfs.h | 2 --
>  include/uapi/linux/nfs.h | 3 +--
>  6 files changed, 3 insertions(+), 8 deletions(-)

These both look fine to me.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] Address minor issues with status codes
  2025-12-10  0:49 ` [PATCH 0/2] Address minor issues with status codes Jeff Layton
@ 2025-12-10  1:01   ` NeilBrown
  0 siblings, 0 replies; 5+ messages in thread
From: NeilBrown @ 2025-12-10  1:01 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Chuck Lever, Olga Kornievskaia, Dai Ngo, Tom Talpey, linux-nfs,
	Chuck Lever

On Wed, 10 Dec 2025, Jeff Layton wrote:
> On Tue, 2025-12-09 at 19:28 -0500, Chuck Lever wrote:
> > From: Chuck Lever <chuck.lever@oracle.com>
> > 
> > Here are two examples of minor bugs I found when comparing NFSD's
> > human-generated status code definitions with equivalent xdrgen-
> > generated headers.
> > 
> > Chuck Lever (2):
> >   NFSD: Remove NFSERR_EAGAIN
> >   NFS: NFSERR_INVAL is not defined by NFSv2
> > 
> >  fs/nfs_common/common.c   | 1 -
> >  fs/nfsd/nfs4proc.c       | 2 +-
> >  fs/nfsd/nfsd.h           | 1 -
> >  fs/nfsd/nfsproc.c        | 2 +-
> >  include/trace/misc/nfs.h | 2 --
> >  include/uapi/linux/nfs.h | 3 +--
> >  6 files changed, 3 insertions(+), 8 deletions(-)
> 
> These both look fine to me.
> 
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
> 
Agreed.

Reviewed-by: NeilBrown <neil@brown.name>

I cannot easily find a path that would lead to NFSv2 being presented
with nfserr_symlink or nfserr_wrong_type, so that change probably isn't
visible.  I might have missed something though.

Thanks,
NeilBrown
 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-12-10  1:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-10  0:28 [PATCH 0/2] Address minor issues with status codes Chuck Lever
2025-12-10  0:28 ` [PATCH 1/2] NFSD: Remove NFSERR_EAGAIN Chuck Lever
2025-12-10  0:28 ` [PATCH 2/2] NFS: NFSERR_INVAL is not defined by NFSv2 Chuck Lever
2025-12-10  0:49 ` [PATCH 0/2] Address minor issues with status codes Jeff Layton
2025-12-10  1:01   ` NeilBrown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox