Received-SPF: pass (google.com: domain of linux-nfs+bounces-3612-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Mike Snitzer <snitzer@kernel.org>
To: linux-nfs@vger.kernel.org
Cc: Jeff Layton <jlayton@kernel.org>,
	Chuck Lever <chuck.lever@oracle.com>,
	Trond Myklebust <trondmy@hammerspace.com>,
	snitzer@hammerspace.com
Subject: [for-6.11 PATCH 27/29] nfs/nfsd: switch GETUUID to using {encode,decode}_opaque_fixed
Date: Fri,  7 Jun 2024 10:26:44 -0400
Message-ID: <20240607142646.20924-28-snitzer@kernel.org>
In-Reply-To: <20240607142646.20924-1-snitzer@kernel.org>
References: <20240607142646.20924-1-snitzer@kernel.org>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

The uuid is always UUID_SIZE (16) so there is no need to use less
efficient variable sized opaque encode and decode XDR methods.

Also, XDR buffer size requirements were audited and reduced
accordingly.

Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/nfs/localio.c        |  6 ++---
 fs/nfs/nfs3xdr.c        | 13 +++++++--
 fs/nfs/nfs4xdr.c        |  5 ++--
 fs/nfsd/localio.c       | 60 +++++++++++++++++------------------------
 include/linux/nfs_xdr.h |  3 +--
 5 files changed, 41 insertions(+), 46 deletions(-)

diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c
index ab92c92f04e5..ff28a7315470 100644
--- a/fs/nfs/localio.c
+++ b/fs/nfs/localio.c
@@ -246,9 +246,9 @@ static bool nfs_local_server_getuuid(struct nfs_client *clp, uuid_t *nfsd_uuid)
 	dprintk("%s: NFS issuing getuuid\n", __func__);
 	msg.rpc_proc = &clp->cl_rpcclient_localio->cl_procinfo[LOCALIOPROC_GETUUID];
 	status = rpc_call_sync(clp->cl_rpcclient_localio, &msg, 0);
-	dprintk("%s: NFS reply getuuid: status=%d uuid=%pU uuid_len=%u\n",
-		__func__, status, res.uuid, res.len);
-	if (status || res.len != UUID_SIZE)
+	dprintk("%s: NFS reply getuuid: status=%d uuid=%pU\n",
+		__func__, status, res.uuid);
+	if (status)
 		return false;
 
 	import_uuid(nfsd_uuid, res.uuid);
diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
index 49689a9a2111..d2a17ecd12b8 100644
--- a/fs/nfs/nfs3xdr.c
+++ b/fs/nfs/nfs3xdr.c
@@ -2582,7 +2582,7 @@ const struct rpc_version nfsacl_version3 = {
 
 #if defined(CONFIG_NFS_V3_LOCALIO)
 
-#define LOCALIO3_getuuidres_sz	(1+NFS3_filename_sz)
+#define LOCALIO3_getuuidres_sz	(1+XDR_QUADLEN(UUID_SIZE))
 
 static void nfs3_xdr_enc_getuuidargs(struct rpc_rqst *req,
 				struct xdr_stream *xdr,
@@ -2591,10 +2591,19 @@ static void nfs3_xdr_enc_getuuidargs(struct rpc_rqst *req,
 	/* void function */
 }
 
+// FIXME: factor out from fs/nfs/nfs4xdr.c
+static int decode_opaque_fixed(struct xdr_stream *xdr, void *buf, size_t len)
+{
+	ssize_t ret = xdr_stream_decode_opaque_fixed(xdr, buf, len);
+	if (unlikely(ret < 0))
+		return -EIO;
+	return 0;
+}
+
 static inline int nfs3_decode_getuuidresok(struct xdr_stream *xdr,
 					struct nfs_getuuidres *result)
 {
-	return decode_inline_filename3(xdr, &result->uuid, &result->len);
+	return decode_opaque_fixed(xdr, result->uuid, UUID_SIZE);
 }
 
 static int nfs3_xdr_dec_getuuidres(struct rpc_rqst *req,
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index e6f3556a320e..d3b4fa3245f0 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -7731,8 +7731,7 @@ const struct rpc_version nfs_version4 = {
 
 #if defined(CONFIG_NFS_V4_LOCALIO)
 
-#define NFS4_filename_sz	(1+(NFS4_MAXNAMLEN>>2))
-#define LOCALIO4_getuuidres_sz	(1+NFS4_filename_sz)
+#define LOCALIO4_getuuidres_sz	(op_decode_hdr_maxsz+XDR_QUADLEN(UUID_SIZE))
 
 static void nfs4_xdr_enc_getuuidargs(struct rpc_rqst *req,
 				struct xdr_stream *xdr,
@@ -7744,7 +7743,7 @@ static void nfs4_xdr_enc_getuuidargs(struct rpc_rqst *req,
 static inline int nfs4_decode_getuuidresok(struct xdr_stream *xdr,
 					struct nfs_getuuidres *result)
 {
-	return decode_opaque_inline(xdr, &result->len, (char **)&result->uuid);
+	return decode_opaque_fixed(xdr, result->uuid, UUID_SIZE);
 }
 
 static int nfs4_xdr_dec_getuuidres(struct rpc_rqst *req,
diff --git a/fs/nfsd/localio.c b/fs/nfsd/localio.c
index e4d2adf9531f..ace99f371c13 100644
--- a/fs/nfsd/localio.c
+++ b/fs/nfsd/localio.c
@@ -201,17 +201,23 @@ static __be32 nfsd_proc_getuuid(struct svc_rqst *rqstp)
 	return rpc_success;
 }
 
+#define NFS_getuuid_sz XDR_QUADLEN(UUID_SIZE)
+
+static inline void encode_opaque_fixed(struct xdr_stream *xdr, const void *buf, size_t len)
+{
+	WARN_ON_ONCE(xdr_stream_encode_opaque_fixed(xdr, buf, len) < 0);
+}
+
+static void encode_uuid(struct xdr_stream *xdr, uuid_t *src_uuid)
+{
+	u8 uuid[UUID_SIZE];
+
+	export_uuid(uuid, src_uuid);
+	encode_opaque_fixed(xdr, uuid, UUID_SIZE);
+	dprintk("%s: uuid=%pU\n", __func__, uuid);
+}
+
 #if defined(CONFIG_NFSD_V3_LOCALIO)
-
-static void encode_uuid(struct xdr_stream *xdr,
-			const char *name, u32 length)
-{
-	__be32 *p;
-
-	p = xdr_reserve_space(xdr, 4 + length);
-	xdr_encode_opaque(p, name, length);
-}
-
 static bool nfs3svc_encode_getuuidres(struct svc_rqst *rqstp,
 				      struct xdr_stream *xdr)
 {
@@ -219,14 +225,8 @@ static bool nfs3svc_encode_getuuidres(struct svc_rqst *rqstp,
 
 	if (!svcxdr_encode_nfsstat3(xdr, resp->status))
 		return false;
-	if (resp->status == nfs_ok) {
-		u8 uuid[UUID_SIZE];
-
-		export_uuid(uuid, &resp->uuid);
-		encode_uuid(xdr, uuid, UUID_SIZE);
-		dprintk("%s: nfs_ok uuid=%pU uuid_len=%lu\n",
-			__func__, uuid, sizeof(uuid));
-	}
+	if (resp->status == nfs_ok)
+		encode_uuid(xdr, &resp->uuid);
 
 	return true;
 }
@@ -242,7 +242,7 @@ static const struct svc_procedure nfsd_localio_procedures3[2] = {
 		.pc_argsize = sizeof(struct nfsd_voidargs),
 		.pc_ressize = sizeof(struct nfsd_voidres),
 		.pc_cachetype = RC_NOCACHE,
-		.pc_xdrressize = ST,
+		.pc_xdrressize = 1,
 		.pc_name = "NULL",
 	},
 	[LOCALIOPROC_GETUUID] = {
@@ -252,7 +252,7 @@ static const struct svc_procedure nfsd_localio_procedures3[2] = {
 		.pc_argsize = sizeof(struct nfsd_voidargs),
 		.pc_ressize = sizeof(struct nfsd_getuuidres),
 		.pc_cachetype = RC_NOCACHE,
-		.pc_xdrressize = ST+NFS3_filename_sz,
+		.pc_xdrressize = 1+NFS_getuuid_sz,
 		.pc_name = "GETUUID",
 	},
 };
@@ -282,24 +282,12 @@ static bool nfs4svc_encode_getuuidres(struct svc_rqst *rqstp,
 	*p++ = cpu_to_be32(LOCALIOPROC_GETUUID);
 	*p++ = resp->status;
 
-	if (resp->status == nfs_ok) {
-		u8 uuid[UUID_SIZE];
-
-		export_uuid(uuid, &resp->uuid);
-		p = xdr_reserve_space(xdr, 4 + UUID_SIZE);
-		if (!p)
-			return 0;
-		xdr_encode_opaque(p, uuid, UUID_SIZE);
-		dprintk("%s: nfs_ok uuid=%pU uuid_len=%lu\n",
-			__func__, uuid, sizeof(uuid));
-	}
+	if (resp->status == nfs_ok)
+		encode_uuid(xdr, &resp->uuid);
 
 	return 1;
 }
 
-#define ST 1		/* status */
-#define NFS4_filename_sz	(1+(NFS4_MAXNAMLEN>>2))
-
 static const struct svc_procedure nfsd_localio_procedures4[2] = {
 	[LOCALIOPROC_NULL] = {
 		.pc_func = nfsd_proc_null,
@@ -308,7 +296,7 @@ static const struct svc_procedure nfsd_localio_procedures4[2] = {
 		.pc_argsize = sizeof(struct nfsd_voidargs),
 		.pc_ressize = sizeof(struct nfsd_voidres),
 		.pc_cachetype = RC_NOCACHE,
-		.pc_xdrressize = ST,
+		.pc_xdrressize = 1,
 		.pc_name = "NULL",
 	},
 	[LOCALIOPROC_GETUUID] = {
@@ -318,7 +306,7 @@ static const struct svc_procedure nfsd_localio_procedures4[2] = {
 		.pc_argsize = sizeof(struct nfsd_voidargs),
 		.pc_ressize = sizeof(struct nfsd_getuuidres),
 		.pc_cachetype = RC_NOCACHE,
-		.pc_xdrressize = ST+NFS4_filename_sz,
+		.pc_xdrressize = 2+NFS_getuuid_sz,
 		.pc_name = "GETUUID",
 	},
 };
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index b6a16eca4664..2a438f4c2d6d 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1003,8 +1003,7 @@ struct nfs3_getaclres {
 };
 
 struct nfs_getuuidres {
-	const char *		uuid;
-	unsigned int		len;
+	__u8 *			uuid;
 };
 
 #if IS_ENABLED(CONFIG_NFS_V4)
-- 
2.44.0