Re: [PATCH] mount.nfs: improve EPROTO error message for RDMA mounts

Steve Dickson <steved@xxxxxxxxxx> · Fri, 20 Sep 2024 13:18:30 -0400

On 9/19/24 11:10 AM, Dan Aloni wrote:
When mounting NFS shares using RDMA, users may encounter this rather
unclear error message:

     mount.nfs: Protocol error

Often there are either no RDMA interfaces existing, or that routing is
being done via other interfaces. This patch enhances the `mount_error`
function to provide a more informative message in such cases.

Signed-off-by: Dan Aloni <dan.aloni@xxxxxxxxxxxx>
Committed... (tag: nfs-utils-2-7-2-rc1)

steved.

---
  utils/mount/error.c     | 12 +++++++++++-
  utils/mount/error.h     |  4 +++-
  utils/mount/network.c   |  2 +-
  utils/mount/network.h   |  2 ++
  utils/mount/nfs4mount.c |  2 +-
  utils/mount/nfsmount.c  |  2 +-
  utils/mount/stropts.c   |  8 ++++----
  utils/mount/utils.c     |  6 +++---
  8 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/utils/mount/error.c b/utils/mount/error.c
index 9ddbcc096f72..d6cbdea1933b 100644
--- a/utils/mount/error.c
+++ b/utils/mount/error.c
@@ -40,6 +40,7 @@
  #include "nls.h"
  #include "mount.h"
  #include "error.h"
+#include "network.h"
  
  #ifdef HAVE_RPCSVC_NFS_PROT_H
  #include <rpcsvc/nfs_prot.h>
@@ -199,7 +200,8 @@ void sys_mount_errors(char *server, int error, int will_retry, int bg)
   * @error: errno value to report
   *
   */
-void mount_error(const char *spec, const char *mount_point, int error)
+void mount_error(const char *spec, const char *mount_point, int error,
+		 struct mount_options *options)
  {
  	switch(error) {
  	case EACCES:
@@ -250,6 +252,14 @@ void mount_error(const char *spec, const char *mount_point, int error)
  	case EALREADY:
  		/* Error message has already been provided */
  		break;
+	case EPROTO:
+		if (options && po_rightmost(options, nfs_transport_opttbl) == 2)
+			nfs_error(_("%s: %s: is routing being done via an interface supporting RDMA?"),
+				  progname, strerror(error));
+		else
+			nfs_error(_("%s: %s"),
+				  progname, strerror(error));
+		break;
  	default:
  		nfs_error(_("%s: %s for %s on %s"),
  			  progname, strerror(error), spec, mount_point);
diff --git a/utils/mount/error.h b/utils/mount/error.h
index ef80fd079b48..f9f282233563 100644
--- a/utils/mount/error.h
+++ b/utils/mount/error.h
@@ -24,9 +24,11 @@
  #ifndef _NFS_UTILS_MOUNT_ERROR_H
  #define _NFS_UTILS_MOUNT_ERROR_H
  
+#include "parse_opt.h"
+
  char *nfs_strerror(unsigned int);
  
-void mount_error(const char *, const char *, int);
+void mount_error(const char *, const char *, int, struct mount_options *);
  void rpc_mount_errors(char *, int, int);
  void sys_mount_errors(char *, int, int, int);
  
diff --git a/utils/mount/network.c b/utils/mount/network.c
index 01ead49f0008..64293f6f8d51 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -88,7 +88,7 @@ static const char *nfs_nfs_pgmtbl[] = {
  	NULL,
  };
  
-static const char *nfs_transport_opttbl[] = {
+const char *nfs_transport_opttbl[] = {
  	"udp",
  	"tcp",
  	"rdma",
diff --git a/utils/mount/network.h b/utils/mount/network.h
index 0fc98acd4bcb..26f4eec775df 100644
--- a/utils/mount/network.h
+++ b/utils/mount/network.h
@@ -93,4 +93,6 @@ void mnt_closeclnt(CLIENT *, int);
  int nfs_umount_do_umnt(struct mount_options *options,
  		       char **hostname, char **dirname);
  
+extern const char *nfs_transport_opttbl[];
+
  #endif	/* _NFS_UTILS_MOUNT_NETWORK_H */
diff --git a/utils/mount/nfs4mount.c b/utils/mount/nfs4mount.c
index 3e4f1e255ca7..0fe142a7843e 100644
--- a/utils/mount/nfs4mount.c
+++ b/utils/mount/nfs4mount.c
@@ -469,7 +469,7 @@ int nfs4mount(const char *spec, const char *node, int flags,
  	if (!fake) {
  		if (mount(spec, node, "nfs4",
  				flags & ~(MS_USER|MS_USERS), &data)) {
-			mount_error(spec, node, errno);
+			mount_error(spec, node, errno, NULL);
  			goto fail;
  		}
  	}
diff --git a/utils/mount/nfsmount.c b/utils/mount/nfsmount.c
index 3d95da9456de..a1c92fe83fa0 100644
--- a/utils/mount/nfsmount.c
+++ b/utils/mount/nfsmount.c
@@ -858,7 +858,7 @@ noauth_flavors:
  	if (!fake) {
  		if (mount(spec, node, "nfs",
  				flags & ~(MS_USER|MS_USERS), &data)) {
-			mount_error(spec, node, errno);
+			mount_error(spec, node, errno, NULL);
  			goto fail;
  		}
  	}
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index a92c420011a4..2d5fa1f2e86e 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -1143,7 +1143,7 @@ static int nfsmount_fg(struct nfsmount_info *mi)
  		}
  	}
  
-	mount_error(mi->spec, mi->node, errno);
+	mount_error(mi->spec, mi->node, errno, mi->options);
  	return EX_FAIL;
  }
  
@@ -1160,7 +1160,7 @@ static int nfsmount_parent(struct nfsmount_info *mi)
  		return EX_SUCCESS;
  
  	if (nfs_is_permanent_error(errno)) {
-		mount_error(mi->spec, mi->node, errno);
+		mount_error(mi->spec, mi->node, errno, mi->options);
  		return EX_FAIL;
  	}
  
@@ -1237,7 +1237,7 @@ static int nfs_remount(struct nfsmount_info *mi)
  {
  	if (nfs_sys_mount(mi, mi->options))
  		return EX_SUCCESS;
-	mount_error(mi->spec, mi->node, errno);
+	mount_error(mi->spec, mi->node, errno, mi->options);
  	return EX_FAIL;
  }
  
@@ -1261,7 +1261,7 @@ static int nfsmount_start(struct nfsmount_info *mi)
  	 * NFS v2 has been deprecated
  	 */
  	if (mi->version.major == 2) {
-		mount_error(mi->spec, mi->node, EOPNOTSUPP);
+		mount_error(mi->spec, mi->node, EOPNOTSUPP, mi->options);
  		return EX_FAIL;
  	}
  
diff --git a/utils/mount/utils.c b/utils/mount/utils.c
index 865a4a05f3fc..b7562a474f88 100644
--- a/utils/mount/utils.c
+++ b/utils/mount/utils.c
@@ -123,15 +123,15 @@ int chk_mountpoint(const char *mount_point)
  	struct stat sb;
  
  	if (stat(mount_point, &sb) < 0){
-		mount_error(NULL, mount_point, errno);
+		mount_error(NULL, mount_point, errno, NULL);
  		return 1;
  	}
  	if (S_ISDIR(sb.st_mode) == 0){
-		mount_error(NULL, mount_point, ENOTDIR);
+		mount_error(NULL, mount_point, ENOTDIR, NULL);
  		return 1;
  	}
  	if (getuid() != 0 && geteuid() != 0 && access(mount_point, X_OK) < 0) {
-		mount_error(NULL, mount_point, errno);
+		mount_error(NULL, mount_point, errno, NULL);
  		return 1;
  	}
  








