All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] nfsuuid and udev examples
@ 2022-02-10 18:01 Benjamin Coddington
  2022-02-10 18:01 ` [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers Benjamin Coddington
  2022-02-10 18:01 ` [PATCH v2 2/2] nfsuuid: add some example udev rules Benjamin Coddington
  0 siblings, 2 replies; 36+ messages in thread
From: Benjamin Coddington @ 2022-02-10 18:01 UTC (permalink / raw)
  To: Steve Dickson; +Cc: linux-nfs

Changes on v2:
	- add forgotten aclocal/libuuid.m4

Benjamin Coddington (2):
  nfsuuid: a tool to create and persist nfs4 client uniquifiers
  nfsuuid: add some example udev rules

 .gitignore                       |   1 +
 aclocal/libuuid.m4               |  17 +++
 configure.ac                     |   4 +
 tools/Makefile.am                |   1 +
 tools/nfsuuid/Makefile.am        |   8 ++
 tools/nfsuuid/example_udev.rules |  28 +++++
 tools/nfsuuid/nfsuuid.c          | 203 +++++++++++++++++++++++++++++++
 tools/nfsuuid/nfsuuid.man        |  33 +++++
 8 files changed, 295 insertions(+)
 create mode 100644 aclocal/libuuid.m4
 create mode 100644 tools/nfsuuid/Makefile.am
 create mode 100644 tools/nfsuuid/example_udev.rules
 create mode 100644 tools/nfsuuid/nfsuuid.c
 create mode 100644 tools/nfsuuid/nfsuuid.man

-- 
2.31.1


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

* [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-10 18:01 [PATCH v2 0/2] nfsuuid and udev examples Benjamin Coddington
@ 2022-02-10 18:01 ` Benjamin Coddington
  2022-02-10 18:15   ` Chuck Lever III
  2022-02-10 18:01 ` [PATCH v2 2/2] nfsuuid: add some example udev rules Benjamin Coddington
  1 sibling, 1 reply; 36+ messages in thread
From: Benjamin Coddington @ 2022-02-10 18:01 UTC (permalink / raw)
  To: Steve Dickson; +Cc: linux-nfs

The nfsuuid program will either create a new UUID from a random source or
derive it from /etc/machine-id, else it returns a UUID that has already
been written to /etc/nfsuuid or the file specified.  This small,
lightweight tool is suitable for execution by systemd-udev in rules to
populate the nfs4 client uniquifier.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 .gitignore                |   1 +
 aclocal/libuuid.m4        |  17 ++++
 configure.ac              |   4 +
 tools/Makefile.am         |   1 +
 tools/nfsuuid/Makefile.am |   8 ++
 tools/nfsuuid/nfsuuid.c   | 203 ++++++++++++++++++++++++++++++++++++++
 tools/nfsuuid/nfsuuid.man |  33 +++++++
 7 files changed, 267 insertions(+)
 create mode 100644 aclocal/libuuid.m4
 create mode 100644 tools/nfsuuid/Makefile.am
 create mode 100644 tools/nfsuuid/nfsuuid.c
 create mode 100644 tools/nfsuuid/nfsuuid.man

diff --git a/.gitignore b/.gitignore
index c89d1cd2583d..4d63ee93b2dc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -61,6 +61,7 @@ utils/statd/statd
 tools/locktest/testlk
 tools/getiversion/getiversion
 tools/nfsconf/nfsconf
+tools/nfsuuid/nfsuuid
 support/export/mount.h
 support/export/mount_clnt.c
 support/export/mount_xdr.c
diff --git a/aclocal/libuuid.m4 b/aclocal/libuuid.m4
new file mode 100644
index 000000000000..f64085010d1d
--- /dev/null
+++ b/aclocal/libuuid.m4
@@ -0,0 +1,17 @@
+AC_DEFUN([AC_LIBUUID], [
+        LIBUUID=
+
+        AC_CHECK_LIB([uuid], [uuid_generate_random], [], [AC_MSG_FAILURE(
+                [Missing libuuid uuid_generate_random, needed to build nfs4id])])
+
+        AC_CHECK_LIB([uuid], [uuid_generate_sha1], [], [AC_MSG_FAILURE(
+                [Missing libuuid uuid_generate_sha1, needed to build nfs4id])])
+
+        AC_CHECK_LIB([uuid], [uuid_unparse], [], [AC_MSG_FAILURE(
+                [Missing libuuid uuid_unparse, needed to build nfs4id])])
+
+        AC_CHECK_HEADER([uuid/uuid.h], [], [AC_MSG_FAILURE(
+                [Missing uuid/uuid.h, needed to build nfs4id])])
+
+        AC_SUBST([LIBUUID], ["-luuid"])
+])
diff --git a/configure.ac b/configure.ac
index 50e9b321dcf3..1342c471f142 100644
--- a/configure.ac
+++ b/configure.ac
@@ -355,6 +355,9 @@ if test "$enable_nfsv4" = yes; then
   dnl check for the keyutils libraries and headers
   AC_KEYUTILS
 
+  dnl check for the libuuid library and headers
+  AC_LIBUUID
+
   dnl Check for sqlite3
   AC_SQLITE3_VERS
 
@@ -740,6 +743,7 @@ AC_CONFIG_FILES([
 	tools/nfsdclnts/Makefile
 	tools/nfsconf/Makefile
 	tools/nfsdclddb/Makefile
+	tools/nfsuuid/Makefile
 	utils/Makefile
 	utils/blkmapd/Makefile
 	utils/nfsdcld/Makefile
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 9b4b0803db39..a12b0f34f4e7 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -7,6 +7,7 @@ OPTDIRS += rpcgen
 endif
 
 OPTDIRS += nfsconf
+OPTDIRS += nfsuuid
 
 if CONFIG_NFSDCLD
 OPTDIRS += nfsdclddb
diff --git a/tools/nfsuuid/Makefile.am b/tools/nfsuuid/Makefile.am
new file mode 100644
index 000000000000..7b3a54c30d50
--- /dev/null
+++ b/tools/nfsuuid/Makefile.am
@@ -0,0 +1,8 @@
+## Process this file with automake to produce Makefile.in
+
+man8_MANS	= nfsuuid.man
+
+bin_PROGRAMS = nfsuuid
+
+nfsuuid_SOURCES = nfsuuid.c
+nfsuuid_LDADD = $(LIBUUID)
diff --git a/tools/nfsuuid/nfsuuid.c b/tools/nfsuuid/nfsuuid.c
new file mode 100644
index 000000000000..bbdec59f1afe
--- /dev/null
+++ b/tools/nfsuuid/nfsuuid.c
@@ -0,0 +1,203 @@
+/*
+ * nfsuuid.c -- create and persist uniquifiers for nfs4 clients
+ *
+ * Copyright (C) 2022  Red Hat, Benjamin Coddington <bcodding@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+
+#include <stdio.h>
+#include <stdarg.h>
+#include <getopt.h>
+#include <string.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <uuid/uuid.h>
+
+#define DEFAULT_ID_FILE "/etc/nfsuuid"
+
+UUID_DEFINE(nfs4_clientid_uuid_template,
+	0xa2, 0x25, 0x68, 0xb2, 0x7a, 0x5f, 0x49, 0x90,
+	0x8f, 0x98, 0xc5, 0xf0, 0x67, 0x78, 0xcc, 0xf1);
+
+static char *prog;
+static char *source = NULL;
+static char *id_file = DEFAULT_ID_FILE;
+static char nfs_unique_id[64];
+static int replace = 0;
+
+static void usage(void)
+{
+	fprintf(stderr, "usage: %s [-r|--replace] [-f <file> |--file <file> ] [machine]\n", prog);
+}
+
+static void fatal(const char *fmt, ...)
+{
+	int err = errno;
+	va_list args;
+	char fatal_msg[256] = "fatal: ";
+
+	va_start(args, fmt);
+	vsnprintf(&fatal_msg[7], 255, fmt, args);
+	if (err)
+		fprintf(stderr, "%s: %s\n", fatal_msg, strerror(err));
+	else
+		fprintf(stderr, "%s\n", fatal_msg);
+	exit(-1);
+}
+
+static int read_nfs_unique_id(void)
+{
+	int fd;
+	ssize_t len;
+
+	if (replace) {
+		errno = ENOENT;
+		return -1;
+	}
+
+	fd = open(id_file, O_RDONLY);
+	if (fd < 0)
+		return fd;
+	len = read(fd, nfs_unique_id, 64);
+	close(fd);
+	return len;
+}
+
+static void write_nfs_unique_id(void)
+{
+	int fd;
+
+	fd = open(id_file, O_RDWR|O_TRUNC|O_CREAT, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH);
+	if (fd < 0)
+		fatal("could not write id to %s", id_file);
+	write(fd, nfs_unique_id, 37);
+}
+
+static void print_nfs_unique_id(void)
+{
+	fprintf(stdout, "%s", nfs_unique_id);
+}
+
+static void check_or_make_id(void)
+{
+	int ret;
+	uuid_t uuid;
+
+	ret = read_nfs_unique_id();
+	if (ret < 1) {
+		if (errno != ENOENT )
+			fatal("reading file %s", id_file);
+		uuid_generate_random(uuid);
+		uuid_unparse(uuid, nfs_unique_id);
+		nfs_unique_id[36] = '\n';
+		nfs_unique_id[37] = '\0';
+		write_nfs_unique_id();
+	}
+	print_nfs_unique_id();
+}
+
+static void check_or_make_id_from_machine(void)
+{
+	int fd, ret;
+	char machineid[32];
+	uuid_t uuid;
+
+	ret = read_nfs_unique_id();
+	if (ret < 1) {
+		if (errno != ENOENT )
+			fatal("reading file %s", id_file);
+
+		fd = open("/etc/machine-id", O_RDONLY);
+		if (fd < 0)
+			fatal("unable to read /etc/machine-id");
+
+		read(fd, machineid, 32);
+		close(fd);
+
+		uuid_generate_sha1(uuid, nfs4_clientid_uuid_template, machineid, 32);
+		uuid_unparse(uuid, nfs_unique_id);
+		nfs_unique_id[36] = '\n';
+		nfs_unique_id[37] = '\0';
+		write_nfs_unique_id();
+	}
+	print_nfs_unique_id();
+}
+
+int main(int argc, char **argv)
+{
+	prog = argv[0];
+
+	while (1) {
+		int opt, prev_ind;
+		int option_index = 0;
+		static struct option long_options[] = {
+			{"replace",	no_argument,		0, 'r' },
+			{"file",	required_argument,	0, 'f' },
+			{ 0, 0, 0, 0 }
+		};
+
+		errno = 0;
+		prev_ind = optind;
+		opt = getopt_long(argc, argv, ":rf:", long_options, &option_index);
+		if (opt == -1)
+			break;
+
+		/* Let's detect missing options in the middle of an option list */
+		if (optind == prev_ind + 2 && *optarg == '-') {
+			opt = ':';
+			--optind;
+		}
+
+		switch (opt) {
+		case 'r':
+			replace = 1;
+			break;
+		case 'f':
+			id_file = optarg;
+			break;
+		case ':':
+			usage();
+			fatal("option \"%s\" requires an argument", argv[prev_ind]);
+			break;
+		case '?':
+			usage();
+			fatal("unexpected arg \"%s\"", argv[optind - 1]);
+			break;
+		}
+	}
+
+	argc -= optind;
+
+	if (argc > 1) {
+		usage();
+		fatal("Too many arguments");
+	}
+
+	if (argc)
+		source = argv[optind++];
+
+	if (!source)
+		check_or_make_id();
+	else if (strcmp(source, "machine") == 0)
+		check_or_make_id_from_machine();
+	else {
+		usage();
+		fatal("unrecognized source %s\n", source);
+	}
+}
diff --git a/tools/nfsuuid/nfsuuid.man b/tools/nfsuuid/nfsuuid.man
new file mode 100644
index 000000000000..856d2f383e0f
--- /dev/null
+++ b/tools/nfsuuid/nfsuuid.man
@@ -0,0 +1,33 @@
+.\"
+.\" nfsuuid(8)
+.\"
+.TH nfsuuid 8 "10 Feb 2022"
+.SH NAME
+nfsuuid \- Generate or return nfs client id uniquifiers
+.SH SYNOPSIS
+.B nfsuuid [ -r | --replace ] [ -f | --file <file> ] [<source>]
+
+.SH DESCRIPTION
+The
+.B nfsuuid
+command provides a simple utility to help NFS Version 4 clients use unique
+and persistent client id values.  The command checks for the existence of a
+file /etc/nfsuuid and returns the first 64 chars read from that file.  If
+the file is not found, a UUID is generated from the specified source and
+written to the file and returned.
+.SH OPTIONS
+.TP
+.BR \-r,\ \-\-replace
+Overwrite the <file> with a UUID generated from <source>.
+.TP
+.BR \-f,\ \-\-file
+Use the specified file to lookup or store any generated UUID.  If <file> is
+not specified, the default file used is /etc/nfsuuid.
+.SH Sources
+If <source> is not specified, nfsuuid will generate a new random UUID.
+
+If <source> is "machine", nfsuuid will generate a deterministic UUID value
+derived from a sha1 hash of the contents of /etc/machine-id and a static
+key.
+.SH SEE ALSO
+.BR machine-id (5)
-- 
2.31.1


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

* [PATCH v2 2/2] nfsuuid: add some example udev rules
  2022-02-10 18:01 [PATCH v2 0/2] nfsuuid and udev examples Benjamin Coddington
  2022-02-10 18:01 ` [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers Benjamin Coddington
@ 2022-02-10 18:01 ` Benjamin Coddington
  1 sibling, 0 replies; 36+ messages in thread
From: Benjamin Coddington @ 2022-02-10 18:01 UTC (permalink / raw)
  To: Steve Dickson; +Cc: linux-nfs

Provide several variations on the use of the nfsuuid tool in a udev rule in
order to automagically uniquify NFSv4 clients.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 tools/nfsuuid/example_udev.rules | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 tools/nfsuuid/example_udev.rules

diff --git a/tools/nfsuuid/example_udev.rules b/tools/nfsuuid/example_udev.rules
new file mode 100644
index 000000000000..31117387ac59
--- /dev/null
+++ b/tools/nfsuuid/example_udev.rules
@@ -0,0 +1,28 @@
+# This is a sample udev rules file that demonstrates how to get udev
+# to set a unique but persistent client id uniquifier for the kernel
+# NFS client.  The NFS client exposes a sysfs entry that can be used to
+# "uniquify" a client.  Values written to this entry are used to create
+# the client's identifier to distinguish a client from all other clients
+# that may claim state from a particular server.  Clients typically use
+# several sources of information to generate a client ID such as hostname,
+# and NFS version numbers, however these values may collide in certain
+# sitations.
+#
+# These examples use the `nfsuuid` helper available from nfs-utils, see
+# the man page nfsuuid(8).
+#
+# Write the default output of `nfsuuid` to /sys/fs/nfs/net/nfs_client/identifier
+# This either creates a new, random uuid or returns one previously saved:
+# KERNEL=="nfs_client", ATTR{identifier}=="(null)", PROGRAM="/usr/bin/nfsuuid", ATTR{identifier}="%c"
+#
+# Generate a new, random uuid on every boot:
+# KERNEL=="nfs_client", ATTR{identifier}=="(null)", PROGRAM="/usr/bin/nfsuuid -rf/dev/null", ATTR{identifier}="%c"
+#
+# Generate a deterministic uuid based on /etc/machine-id, or return the previously saved one:
+# KERNEL=="nfs_client", ATTR{identifier}=="(null)", PROGRAM="/usr/bin/nfsuuid machine", ATTR{identifier}="%c"
+#
+# Always use a deterministic uuid based on /etc/machine-id:
+# KERNEL=="nfs_client", ATTR{identifier}=="(null)", PROGRAM="/usr/bin/nfsuuid -r machine", ATTR{identifier}="%c"
+#
+# Generate a new, random uuid or return the one previously saved in /var/nfs/client_id:
+# KERNEL=="nfs_client", ATTR{identifier}=="(null)", PROGRAM="/usr/bin/nfsuuid -f/var/nfs/client_id", ATTR{identifier}="%c"
-- 
2.31.1


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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-10 18:01 ` [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers Benjamin Coddington
@ 2022-02-10 18:15   ` Chuck Lever III
  2022-02-11 13:44     ` Steve Dickson
  0 siblings, 1 reply; 36+ messages in thread
From: Chuck Lever III @ 2022-02-10 18:15 UTC (permalink / raw)
  To: Benjamin Coddington; +Cc: Steve Dickson, Linux NFS Mailing List



> On Feb 10, 2022, at 1:01 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> The nfsuuid program will either create a new UUID from a random source or
> derive it from /etc/machine-id, else it returns a UUID that has already
> been written to /etc/nfsuuid or the file specified.  This small,
> lightweight tool is suitable for execution by systemd-udev in rules to
> populate the nfs4 client uniquifier.

I like everything but the name. Without context, even if
I read NFS protocol specifications, I have no idea what
"nfsuuid" does.

Possible alternatives:

nfshostuniquifier
nfsuniquehost
nfshostuuid


> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> ---
> .gitignore                |   1 +
> aclocal/libuuid.m4        |  17 ++++
> configure.ac              |   4 +
> tools/Makefile.am         |   1 +
> tools/nfsuuid/Makefile.am |   8 ++
> tools/nfsuuid/nfsuuid.c   | 203 ++++++++++++++++++++++++++++++++++++++
> tools/nfsuuid/nfsuuid.man |  33 +++++++
> 7 files changed, 267 insertions(+)
> create mode 100644 aclocal/libuuid.m4
> create mode 100644 tools/nfsuuid/Makefile.am
> create mode 100644 tools/nfsuuid/nfsuuid.c
> create mode 100644 tools/nfsuuid/nfsuuid.man
> 
> diff --git a/.gitignore b/.gitignore
> index c89d1cd2583d..4d63ee93b2dc 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -61,6 +61,7 @@ utils/statd/statd
> tools/locktest/testlk
> tools/getiversion/getiversion
> tools/nfsconf/nfsconf
> +tools/nfsuuid/nfsuuid
> support/export/mount.h
> support/export/mount_clnt.c
> support/export/mount_xdr.c
> diff --git a/aclocal/libuuid.m4 b/aclocal/libuuid.m4
> new file mode 100644
> index 000000000000..f64085010d1d
> --- /dev/null
> +++ b/aclocal/libuuid.m4
> @@ -0,0 +1,17 @@
> +AC_DEFUN([AC_LIBUUID], [
> +        LIBUUID=
> +
> +        AC_CHECK_LIB([uuid], [uuid_generate_random], [], [AC_MSG_FAILURE(
> +                [Missing libuuid uuid_generate_random, needed to build nfs4id])])
> +
> +        AC_CHECK_LIB([uuid], [uuid_generate_sha1], [], [AC_MSG_FAILURE(
> +                [Missing libuuid uuid_generate_sha1, needed to build nfs4id])])
> +
> +        AC_CHECK_LIB([uuid], [uuid_unparse], [], [AC_MSG_FAILURE(
> +                [Missing libuuid uuid_unparse, needed to build nfs4id])])
> +
> +        AC_CHECK_HEADER([uuid/uuid.h], [], [AC_MSG_FAILURE(
> +                [Missing uuid/uuid.h, needed to build nfs4id])])
> +
> +        AC_SUBST([LIBUUID], ["-luuid"])
> +])
> diff --git a/configure.ac b/configure.ac
> index 50e9b321dcf3..1342c471f142 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -355,6 +355,9 @@ if test "$enable_nfsv4" = yes; then
>   dnl check for the keyutils libraries and headers
>   AC_KEYUTILS
> 
> +  dnl check for the libuuid library and headers
> +  AC_LIBUUID
> +
>   dnl Check for sqlite3
>   AC_SQLITE3_VERS
> 
> @@ -740,6 +743,7 @@ AC_CONFIG_FILES([
> 	tools/nfsdclnts/Makefile
> 	tools/nfsconf/Makefile
> 	tools/nfsdclddb/Makefile
> +	tools/nfsuuid/Makefile
> 	utils/Makefile
> 	utils/blkmapd/Makefile
> 	utils/nfsdcld/Makefile
> diff --git a/tools/Makefile.am b/tools/Makefile.am
> index 9b4b0803db39..a12b0f34f4e7 100644
> --- a/tools/Makefile.am
> +++ b/tools/Makefile.am
> @@ -7,6 +7,7 @@ OPTDIRS += rpcgen
> endif
> 
> OPTDIRS += nfsconf
> +OPTDIRS += nfsuuid
> 
> if CONFIG_NFSDCLD
> OPTDIRS += nfsdclddb
> diff --git a/tools/nfsuuid/Makefile.am b/tools/nfsuuid/Makefile.am
> new file mode 100644
> index 000000000000..7b3a54c30d50
> --- /dev/null
> +++ b/tools/nfsuuid/Makefile.am
> @@ -0,0 +1,8 @@
> +## Process this file with automake to produce Makefile.in
> +
> +man8_MANS	= nfsuuid.man
> +
> +bin_PROGRAMS = nfsuuid
> +
> +nfsuuid_SOURCES = nfsuuid.c
> +nfsuuid_LDADD = $(LIBUUID)
> diff --git a/tools/nfsuuid/nfsuuid.c b/tools/nfsuuid/nfsuuid.c
> new file mode 100644
> index 000000000000..bbdec59f1afe
> --- /dev/null
> +++ b/tools/nfsuuid/nfsuuid.c
> @@ -0,0 +1,203 @@
> +/*
> + * nfsuuid.c -- create and persist uniquifiers for nfs4 clients
> + *
> + * Copyright (C) 2022  Red Hat, Benjamin Coddington <bcodding@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor,
> + * Boston, MA 02110-1301, USA.
> + */
> +
> +#include <stdio.h>
> +#include <stdarg.h>
> +#include <getopt.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <uuid/uuid.h>
> +
> +#define DEFAULT_ID_FILE "/etc/nfsuuid"
> +
> +UUID_DEFINE(nfs4_clientid_uuid_template,
> +	0xa2, 0x25, 0x68, 0xb2, 0x7a, 0x5f, 0x49, 0x90,
> +	0x8f, 0x98, 0xc5, 0xf0, 0x67, 0x78, 0xcc, 0xf1);
> +
> +static char *prog;
> +static char *source = NULL;
> +static char *id_file = DEFAULT_ID_FILE;
> +static char nfs_unique_id[64];
> +static int replace = 0;
> +
> +static void usage(void)
> +{
> +	fprintf(stderr, "usage: %s [-r|--replace] [-f <file> |--file <file> ] [machine]\n", prog);
> +}
> +
> +static void fatal(const char *fmt, ...)
> +{
> +	int err = errno;
> +	va_list args;
> +	char fatal_msg[256] = "fatal: ";
> +
> +	va_start(args, fmt);
> +	vsnprintf(&fatal_msg[7], 255, fmt, args);
> +	if (err)
> +		fprintf(stderr, "%s: %s\n", fatal_msg, strerror(err));
> +	else
> +		fprintf(stderr, "%s\n", fatal_msg);
> +	exit(-1);
> +}
> +
> +static int read_nfs_unique_id(void)
> +{
> +	int fd;
> +	ssize_t len;
> +
> +	if (replace) {
> +		errno = ENOENT;
> +		return -1;
> +	}
> +
> +	fd = open(id_file, O_RDONLY);
> +	if (fd < 0)
> +		return fd;
> +	len = read(fd, nfs_unique_id, 64);
> +	close(fd);
> +	return len;
> +}
> +
> +static void write_nfs_unique_id(void)
> +{
> +	int fd;
> +
> +	fd = open(id_file, O_RDWR|O_TRUNC|O_CREAT, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH);
> +	if (fd < 0)
> +		fatal("could not write id to %s", id_file);
> +	write(fd, nfs_unique_id, 37);
> +}
> +
> +static void print_nfs_unique_id(void)
> +{
> +	fprintf(stdout, "%s", nfs_unique_id);
> +}
> +
> +static void check_or_make_id(void)
> +{
> +	int ret;
> +	uuid_t uuid;
> +
> +	ret = read_nfs_unique_id();
> +	if (ret < 1) {
> +		if (errno != ENOENT )
> +			fatal("reading file %s", id_file);
> +		uuid_generate_random(uuid);
> +		uuid_unparse(uuid, nfs_unique_id);
> +		nfs_unique_id[36] = '\n';
> +		nfs_unique_id[37] = '\0';
> +		write_nfs_unique_id();
> +	}
> +	print_nfs_unique_id();
> +}
> +
> +static void check_or_make_id_from_machine(void)
> +{
> +	int fd, ret;
> +	char machineid[32];
> +	uuid_t uuid;
> +
> +	ret = read_nfs_unique_id();
> +	if (ret < 1) {
> +		if (errno != ENOENT )
> +			fatal("reading file %s", id_file);
> +
> +		fd = open("/etc/machine-id", O_RDONLY);
> +		if (fd < 0)
> +			fatal("unable to read /etc/machine-id");
> +
> +		read(fd, machineid, 32);
> +		close(fd);
> +
> +		uuid_generate_sha1(uuid, nfs4_clientid_uuid_template, machineid, 32);
> +		uuid_unparse(uuid, nfs_unique_id);
> +		nfs_unique_id[36] = '\n';
> +		nfs_unique_id[37] = '\0';
> +		write_nfs_unique_id();
> +	}
> +	print_nfs_unique_id();
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	prog = argv[0];
> +
> +	while (1) {
> +		int opt, prev_ind;
> +		int option_index = 0;
> +		static struct option long_options[] = {
> +			{"replace",	no_argument,		0, 'r' },
> +			{"file",	required_argument,	0, 'f' },
> +			{ 0, 0, 0, 0 }
> +		};
> +
> +		errno = 0;
> +		prev_ind = optind;
> +		opt = getopt_long(argc, argv, ":rf:", long_options, &option_index);
> +		if (opt == -1)
> +			break;
> +
> +		/* Let's detect missing options in the middle of an option list */
> +		if (optind == prev_ind + 2 && *optarg == '-') {
> +			opt = ':';
> +			--optind;
> +		}
> +
> +		switch (opt) {
> +		case 'r':
> +			replace = 1;
> +			break;
> +		case 'f':
> +			id_file = optarg;
> +			break;
> +		case ':':
> +			usage();
> +			fatal("option \"%s\" requires an argument", argv[prev_ind]);
> +			break;
> +		case '?':
> +			usage();
> +			fatal("unexpected arg \"%s\"", argv[optind - 1]);
> +			break;
> +		}
> +	}
> +
> +	argc -= optind;
> +
> +	if (argc > 1) {
> +		usage();
> +		fatal("Too many arguments");
> +	}
> +
> +	if (argc)
> +		source = argv[optind++];
> +
> +	if (!source)
> +		check_or_make_id();
> +	else if (strcmp(source, "machine") == 0)
> +		check_or_make_id_from_machine();
> +	else {
> +		usage();
> +		fatal("unrecognized source %s\n", source);
> +	}
> +}
> diff --git a/tools/nfsuuid/nfsuuid.man b/tools/nfsuuid/nfsuuid.man
> new file mode 100644
> index 000000000000..856d2f383e0f
> --- /dev/null
> +++ b/tools/nfsuuid/nfsuuid.man
> @@ -0,0 +1,33 @@
> +.\"
> +.\" nfsuuid(8)
> +.\"
> +.TH nfsuuid 8 "10 Feb 2022"
> +.SH NAME
> +nfsuuid \- Generate or return nfs client id uniquifiers
> +.SH SYNOPSIS
> +.B nfsuuid [ -r | --replace ] [ -f | --file <file> ] [<source>]
> +
> +.SH DESCRIPTION
> +The
> +.B nfsuuid
> +command provides a simple utility to help NFS Version 4 clients use unique
> +and persistent client id values.  The command checks for the existence of a
> +file /etc/nfsuuid and returns the first 64 chars read from that file.  If
> +the file is not found, a UUID is generated from the specified source and
> +written to the file and returned.
> +.SH OPTIONS
> +.TP
> +.BR \-r,\ \-\-replace
> +Overwrite the <file> with a UUID generated from <source>.
> +.TP
> +.BR \-f,\ \-\-file
> +Use the specified file to lookup or store any generated UUID.  If <file> is
> +not specified, the default file used is /etc/nfsuuid.
> +.SH Sources
> +If <source> is not specified, nfsuuid will generate a new random UUID.
> +
> +If <source> is "machine", nfsuuid will generate a deterministic UUID value
> +derived from a sha1 hash of the contents of /etc/machine-id and a static
> +key.
> +.SH SEE ALSO
> +.BR machine-id (5)
> -- 
> 2.31.1
> 

--
Chuck Lever




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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-10 18:15   ` Chuck Lever III
@ 2022-02-11 13:44     ` Steve Dickson
  2022-02-11 15:48       ` Chuck Lever III
  0 siblings, 1 reply; 36+ messages in thread
From: Steve Dickson @ 2022-02-11 13:44 UTC (permalink / raw)
  To: Chuck Lever III, Benjamin Coddington; +Cc: Linux NFS Mailing List



On 2/10/22 1:15 PM, Chuck Lever III wrote:
> 
> 
>> On Feb 10, 2022, at 1:01 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
>>
>> The nfsuuid program will either create a new UUID from a random source or
>> derive it from /etc/machine-id, else it returns a UUID that has already
>> been written to /etc/nfsuuid or the file specified.  This small,
>> lightweight tool is suitable for execution by systemd-udev in rules to
>> populate the nfs4 client uniquifier.
> 
> I like everything but the name. Without context, even if
> I read NFS protocol specifications, I have no idea what
> "nfsuuid" does.
man nfsuuid :-)
> 
> Possible alternatives:
> 
> nfshostuniquifier
> nfsuniquehost
> nfshostuuid
I'm good with the name... short sweet and easy to type.
The name a command does not have to explain what it does. IMHO.

I think what is more concerning is addressing
Neil's concern as to why we even using udev [1].

steved.

[1] 
https://lore.kernel.org/all/0AB20C82-6200-46E0-A76C-62345DAF8A3A@redhat.com/T/#mdb6af27181ed912ed2220208dea9ca8711937864
> 
> 
>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
>> ---
>> .gitignore                |   1 +
>> aclocal/libuuid.m4        |  17 ++++
>> configure.ac              |   4 +
>> tools/Makefile.am         |   1 +
>> tools/nfsuuid/Makefile.am |   8 ++
>> tools/nfsuuid/nfsuuid.c   | 203 ++++++++++++++++++++++++++++++++++++++
>> tools/nfsuuid/nfsuuid.man |  33 +++++++
>> 7 files changed, 267 insertions(+)
>> create mode 100644 aclocal/libuuid.m4
>> create mode 100644 tools/nfsuuid/Makefile.am
>> create mode 100644 tools/nfsuuid/nfsuuid.c
>> create mode 100644 tools/nfsuuid/nfsuuid.man
>>
>> diff --git a/.gitignore b/.gitignore
>> index c89d1cd2583d..4d63ee93b2dc 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -61,6 +61,7 @@ utils/statd/statd
>> tools/locktest/testlk
>> tools/getiversion/getiversion
>> tools/nfsconf/nfsconf
>> +tools/nfsuuid/nfsuuid
>> support/export/mount.h
>> support/export/mount_clnt.c
>> support/export/mount_xdr.c
>> diff --git a/aclocal/libuuid.m4 b/aclocal/libuuid.m4
>> new file mode 100644
>> index 000000000000..f64085010d1d
>> --- /dev/null
>> +++ b/aclocal/libuuid.m4
>> @@ -0,0 +1,17 @@
>> +AC_DEFUN([AC_LIBUUID], [
>> +        LIBUUID=
>> +
>> +        AC_CHECK_LIB([uuid], [uuid_generate_random], [], [AC_MSG_FAILURE(
>> +                [Missing libuuid uuid_generate_random, needed to build nfs4id])])
>> +
>> +        AC_CHECK_LIB([uuid], [uuid_generate_sha1], [], [AC_MSG_FAILURE(
>> +                [Missing libuuid uuid_generate_sha1, needed to build nfs4id])])
>> +
>> +        AC_CHECK_LIB([uuid], [uuid_unparse], [], [AC_MSG_FAILURE(
>> +                [Missing libuuid uuid_unparse, needed to build nfs4id])])
>> +
>> +        AC_CHECK_HEADER([uuid/uuid.h], [], [AC_MSG_FAILURE(
>> +                [Missing uuid/uuid.h, needed to build nfs4id])])
>> +
>> +        AC_SUBST([LIBUUID], ["-luuid"])
>> +])
>> diff --git a/configure.ac b/configure.ac
>> index 50e9b321dcf3..1342c471f142 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -355,6 +355,9 @@ if test "$enable_nfsv4" = yes; then
>>    dnl check for the keyutils libraries and headers
>>    AC_KEYUTILS
>>
>> +  dnl check for the libuuid library and headers
>> +  AC_LIBUUID
>> +
>>    dnl Check for sqlite3
>>    AC_SQLITE3_VERS
>>
>> @@ -740,6 +743,7 @@ AC_CONFIG_FILES([
>> 	tools/nfsdclnts/Makefile
>> 	tools/nfsconf/Makefile
>> 	tools/nfsdclddb/Makefile
>> +	tools/nfsuuid/Makefile
>> 	utils/Makefile
>> 	utils/blkmapd/Makefile
>> 	utils/nfsdcld/Makefile
>> diff --git a/tools/Makefile.am b/tools/Makefile.am
>> index 9b4b0803db39..a12b0f34f4e7 100644
>> --- a/tools/Makefile.am
>> +++ b/tools/Makefile.am
>> @@ -7,6 +7,7 @@ OPTDIRS += rpcgen
>> endif
>>
>> OPTDIRS += nfsconf
>> +OPTDIRS += nfsuuid
>>
>> if CONFIG_NFSDCLD
>> OPTDIRS += nfsdclddb
>> diff --git a/tools/nfsuuid/Makefile.am b/tools/nfsuuid/Makefile.am
>> new file mode 100644
>> index 000000000000..7b3a54c30d50
>> --- /dev/null
>> +++ b/tools/nfsuuid/Makefile.am
>> @@ -0,0 +1,8 @@
>> +## Process this file with automake to produce Makefile.in
>> +
>> +man8_MANS	= nfsuuid.man
>> +
>> +bin_PROGRAMS = nfsuuid
>> +
>> +nfsuuid_SOURCES = nfsuuid.c
>> +nfsuuid_LDADD = $(LIBUUID)
>> diff --git a/tools/nfsuuid/nfsuuid.c b/tools/nfsuuid/nfsuuid.c
>> new file mode 100644
>> index 000000000000..bbdec59f1afe
>> --- /dev/null
>> +++ b/tools/nfsuuid/nfsuuid.c
>> @@ -0,0 +1,203 @@
>> +/*
>> + * nfsuuid.c -- create and persist uniquifiers for nfs4 clients
>> + *
>> + * Copyright (C) 2022  Red Hat, Benjamin Coddington <bcodding@redhat.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version 2
>> + * of the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor,
>> + * Boston, MA 02110-1301, USA.
>> + */
>> +
>> +#include <stdio.h>
>> +#include <stdarg.h>
>> +#include <getopt.h>
>> +#include <string.h>
>> +#include <errno.h>
>> +#include <stdlib.h>
>> +#include <fcntl.h>
>> +#include <unistd.h>
>> +#include <uuid/uuid.h>
>> +
>> +#define DEFAULT_ID_FILE "/etc/nfsuuid"
>> +
>> +UUID_DEFINE(nfs4_clientid_uuid_template,
>> +	0xa2, 0x25, 0x68, 0xb2, 0x7a, 0x5f, 0x49, 0x90,
>> +	0x8f, 0x98, 0xc5, 0xf0, 0x67, 0x78, 0xcc, 0xf1);
>> +
>> +static char *prog;
>> +static char *source = NULL;
>> +static char *id_file = DEFAULT_ID_FILE;
>> +static char nfs_unique_id[64];
>> +static int replace = 0;
>> +
>> +static void usage(void)
>> +{
>> +	fprintf(stderr, "usage: %s [-r|--replace] [-f <file> |--file <file> ] [machine]\n", prog);
>> +}
>> +
>> +static void fatal(const char *fmt, ...)
>> +{
>> +	int err = errno;
>> +	va_list args;
>> +	char fatal_msg[256] = "fatal: ";
>> +
>> +	va_start(args, fmt);
>> +	vsnprintf(&fatal_msg[7], 255, fmt, args);
>> +	if (err)
>> +		fprintf(stderr, "%s: %s\n", fatal_msg, strerror(err));
>> +	else
>> +		fprintf(stderr, "%s\n", fatal_msg);
>> +	exit(-1);
>> +}
>> +
>> +static int read_nfs_unique_id(void)
>> +{
>> +	int fd;
>> +	ssize_t len;
>> +
>> +	if (replace) {
>> +		errno = ENOENT;
>> +		return -1;
>> +	}
>> +
>> +	fd = open(id_file, O_RDONLY);
>> +	if (fd < 0)
>> +		return fd;
>> +	len = read(fd, nfs_unique_id, 64);
>> +	close(fd);
>> +	return len;
>> +}
>> +
>> +static void write_nfs_unique_id(void)
>> +{
>> +	int fd;
>> +
>> +	fd = open(id_file, O_RDWR|O_TRUNC|O_CREAT, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH);
>> +	if (fd < 0)
>> +		fatal("could not write id to %s", id_file);
>> +	write(fd, nfs_unique_id, 37);
>> +}
>> +
>> +static void print_nfs_unique_id(void)
>> +{
>> +	fprintf(stdout, "%s", nfs_unique_id);
>> +}
>> +
>> +static void check_or_make_id(void)
>> +{
>> +	int ret;
>> +	uuid_t uuid;
>> +
>> +	ret = read_nfs_unique_id();
>> +	if (ret < 1) {
>> +		if (errno != ENOENT )
>> +			fatal("reading file %s", id_file);
>> +		uuid_generate_random(uuid);
>> +		uuid_unparse(uuid, nfs_unique_id);
>> +		nfs_unique_id[36] = '\n';
>> +		nfs_unique_id[37] = '\0';
>> +		write_nfs_unique_id();
>> +	}
>> +	print_nfs_unique_id();
>> +}
>> +
>> +static void check_or_make_id_from_machine(void)
>> +{
>> +	int fd, ret;
>> +	char machineid[32];
>> +	uuid_t uuid;
>> +
>> +	ret = read_nfs_unique_id();
>> +	if (ret < 1) {
>> +		if (errno != ENOENT )
>> +			fatal("reading file %s", id_file);
>> +
>> +		fd = open("/etc/machine-id", O_RDONLY);
>> +		if (fd < 0)
>> +			fatal("unable to read /etc/machine-id");
>> +
>> +		read(fd, machineid, 32);
>> +		close(fd);
>> +
>> +		uuid_generate_sha1(uuid, nfs4_clientid_uuid_template, machineid, 32);
>> +		uuid_unparse(uuid, nfs_unique_id);
>> +		nfs_unique_id[36] = '\n';
>> +		nfs_unique_id[37] = '\0';
>> +		write_nfs_unique_id();
>> +	}
>> +	print_nfs_unique_id();
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> +	prog = argv[0];
>> +
>> +	while (1) {
>> +		int opt, prev_ind;
>> +		int option_index = 0;
>> +		static struct option long_options[] = {
>> +			{"replace",	no_argument,		0, 'r' },
>> +			{"file",	required_argument,	0, 'f' },
>> +			{ 0, 0, 0, 0 }
>> +		};
>> +
>> +		errno = 0;
>> +		prev_ind = optind;
>> +		opt = getopt_long(argc, argv, ":rf:", long_options, &option_index);
>> +		if (opt == -1)
>> +			break;
>> +
>> +		/* Let's detect missing options in the middle of an option list */
>> +		if (optind == prev_ind + 2 && *optarg == '-') {
>> +			opt = ':';
>> +			--optind;
>> +		}
>> +
>> +		switch (opt) {
>> +		case 'r':
>> +			replace = 1;
>> +			break;
>> +		case 'f':
>> +			id_file = optarg;
>> +			break;
>> +		case ':':
>> +			usage();
>> +			fatal("option \"%s\" requires an argument", argv[prev_ind]);
>> +			break;
>> +		case '?':
>> +			usage();
>> +			fatal("unexpected arg \"%s\"", argv[optind - 1]);
>> +			break;
>> +		}
>> +	}
>> +
>> +	argc -= optind;
>> +
>> +	if (argc > 1) {
>> +		usage();
>> +		fatal("Too many arguments");
>> +	}
>> +
>> +	if (argc)
>> +		source = argv[optind++];
>> +
>> +	if (!source)
>> +		check_or_make_id();
>> +	else if (strcmp(source, "machine") == 0)
>> +		check_or_make_id_from_machine();
>> +	else {
>> +		usage();
>> +		fatal("unrecognized source %s\n", source);
>> +	}
>> +}
>> diff --git a/tools/nfsuuid/nfsuuid.man b/tools/nfsuuid/nfsuuid.man
>> new file mode 100644
>> index 000000000000..856d2f383e0f
>> --- /dev/null
>> +++ b/tools/nfsuuid/nfsuuid.man
>> @@ -0,0 +1,33 @@
>> +.\"
>> +.\" nfsuuid(8)
>> +.\"
>> +.TH nfsuuid 8 "10 Feb 2022"
>> +.SH NAME
>> +nfsuuid \- Generate or return nfs client id uniquifiers
>> +.SH SYNOPSIS
>> +.B nfsuuid [ -r | --replace ] [ -f | --file <file> ] [<source>]
>> +
>> +.SH DESCRIPTION
>> +The
>> +.B nfsuuid
>> +command provides a simple utility to help NFS Version 4 clients use unique
>> +and persistent client id values.  The command checks for the existence of a
>> +file /etc/nfsuuid and returns the first 64 chars read from that file.  If
>> +the file is not found, a UUID is generated from the specified source and
>> +written to the file and returned.
>> +.SH OPTIONS
>> +.TP
>> +.BR \-r,\ \-\-replace
>> +Overwrite the <file> with a UUID generated from <source>.
>> +.TP
>> +.BR \-f,\ \-\-file
>> +Use the specified file to lookup or store any generated UUID.  If <file> is
>> +not specified, the default file used is /etc/nfsuuid.
>> +.SH Sources
>> +If <source> is not specified, nfsuuid will generate a new random UUID.
>> +
>> +If <source> is "machine", nfsuuid will generate a deterministic UUID value
>> +derived from a sha1 hash of the contents of /etc/machine-id and a static
>> +key.
>> +.SH SEE ALSO
>> +.BR machine-id (5)
>> -- 
>> 2.31.1
>>
> 
> --
> Chuck Lever
> 
> 
> 


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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-11 13:44     ` Steve Dickson
@ 2022-02-11 15:48       ` Chuck Lever III
  2022-02-11 18:28         ` Benjamin Coddington
  0 siblings, 1 reply; 36+ messages in thread
From: Chuck Lever III @ 2022-02-11 15:48 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Benjamin Coddington, Linux NFS Mailing List


> On Feb 11, 2022, at 8:44 AM, Steve Dickson <SteveD@RedHat.com> wrote:
> 
> On 2/10/22 1:15 PM, Chuck Lever III wrote:
>>> On Feb 10, 2022, at 1:01 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
>>> 
>>> The nfsuuid program will either create a new UUID from a random source or
>>> derive it from /etc/machine-id, else it returns a UUID that has already
>>> been written to /etc/nfsuuid or the file specified.  This small,
>>> lightweight tool is suitable for execution by systemd-udev in rules to
>>> populate the nfs4 client uniquifier.
>> I like everything but the name. Without context, even if
>> I read NFS protocol specifications, I have no idea what
>> "nfsuuid" does.
> man nfsuuid :-)

Any time an administrator has to stop to read documentation
is an unnecessary interruption in their flow of attention.
It wastes their time.


>> Possible alternatives:
>> nfshostuniquifier
>> nfsuniquehost
>> nfshostuuid
> I'm good with the name... short sweet and easy to type.

I'll happily keep it short so it is readable and does not
unnecessarily inflate the length of a line in a bash script.
But almost no-one will ever type this command. Especially
if they don't have to find its man page ;-p

My last serious offers: nfsmachineid nfshostid

I strongly prefer not having uuid in the name. A UUID is
essentially a data type, it does not explain the purpose of
the content.


--
Chuck Lever




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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-11 15:48       ` Chuck Lever III
@ 2022-02-11 18:28         ` Benjamin Coddington
  2022-02-11 19:04           ` Chuck Lever III
  0 siblings, 1 reply; 36+ messages in thread
From: Benjamin Coddington @ 2022-02-11 18:28 UTC (permalink / raw)
  To: Chuck Lever III; +Cc: Steve Dickson, Linux NFS Mailing List

On 11 Feb 2022, at 10:48, Chuck Lever III wrote:

>> On Feb 11, 2022, at 8:44 AM, Steve Dickson <SteveD@RedHat.com> wrote:
>>
>> On 2/10/22 1:15 PM, Chuck Lever III wrote:
>>>> On Feb 10, 2022, at 1:01 PM, Benjamin Coddington 
>>>> <bcodding@redhat.com> wrote:
>>>>
>>>> The nfsuuid program will either create a new UUID from a random 
>>>> source or
>>>> derive it from /etc/machine-id, else it returns a UUID that has 
>>>> already
>>>> been written to /etc/nfsuuid or the file specified.  This small,
>>>> lightweight tool is suitable for execution by systemd-udev in rules 
>>>> to
>>>> populate the nfs4 client uniquifier.
>>> I like everything but the name. Without context, even if
>>> I read NFS protocol specifications, I have no idea what
>>> "nfsuuid" does.
>> man nfsuuid :-)
>
> Any time an administrator has to stop to read documentation
> is an unnecessary interruption in their flow of attention.
> It wastes their time.
>
>
>>> Possible alternatives:
>>> nfshostuniquifier
>>> nfsuniquehost
>>> nfshostuuid
>> I'm good with the name... short sweet and easy to type.
>
> I'll happily keep it short so it is readable and does not
> unnecessarily inflate the length of a line in a bash script.
> But almost no-one will ever type this command. Especially
> if they don't have to find its man page ;-p
>
> My last serious offers: nfsmachineid nfshostid
>
> I strongly prefer not having uuid in the name. A UUID is
> essentially a data type, it does not explain the purpose of
> the content.

How do you feel about these suggestions being misleading since the 
output is
not the nfs client's id?  Should we put that information in the man 
page?
Do sysadmins need to know that the output of this command is (if it is 
even
used at all) merely possibility of being a part of the client's id, the
other parts come from other places in the system: the hostname and 
possibly
the ip address?  That's my worry.  If we name it nfsmachineid or 
nfshostid,
do you feel like we ought to have it do the much more complicated job of
accurately outputting the actual client id?

Right now nfsuuid outputs uuids (or whatever was previously stored, up 
to 64
chars).  It generates uuids, like uuidgen.  Without something external 
to
itself (a udev rule, for example), it doesn't have any relationship to 
an
NFS client's id.  It could plausably be used in the future for other 
parts
of NFS to generate persitent uuids.  The only reason we don't just use
`uuidgen` is that we want to wrap it with some persistence.  A would a 
name
like stickyuuid be better?

Ben


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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-11 18:28         ` Benjamin Coddington
@ 2022-02-11 19:04           ` Chuck Lever III
  2022-02-11 19:30             ` Benjamin Coddington
  0 siblings, 1 reply; 36+ messages in thread
From: Chuck Lever III @ 2022-02-11 19:04 UTC (permalink / raw)
  To: Benjamin Coddington; +Cc: Steve Dickson, Linux NFS Mailing List


> On Feb 11, 2022, at 1:28 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> On 11 Feb 2022, at 10:48, Chuck Lever III wrote:
> 
>>> On Feb 11, 2022, at 8:44 AM, Steve Dickson <SteveD@RedHat.com> wrote:
>>> 
>>> On 2/10/22 1:15 PM, Chuck Lever III wrote:
>>>>> On Feb 10, 2022, at 1:01 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
>>>>> 
>>>>> The nfsuuid program will either create a new UUID from a random source or
>>>>> derive it from /etc/machine-id, else it returns a UUID that has already
>>>>> been written to /etc/nfsuuid or the file specified.  This small,
>>>>> lightweight tool is suitable for execution by systemd-udev in rules to
>>>>> populate the nfs4 client uniquifier.
>>>> I like everything but the name. Without context, even if
>>>> I read NFS protocol specifications, I have no idea what
>>>> "nfsuuid" does.
>>> man nfsuuid :-)
>> 
>> Any time an administrator has to stop to read documentation
>> is an unnecessary interruption in their flow of attention.
>> It wastes their time.
>> 
>> 
>>>> Possible alternatives:
>>>> nfshostuniquifier
>>>> nfsuniquehost
>>>> nfshostuuid
>>> I'm good with the name... short sweet and easy to type.
>> 
>> I'll happily keep it short so it is readable and does not
>> unnecessarily inflate the length of a line in a bash script.
>> But almost no-one will ever type this command. Especially
>> if they don't have to find its man page ;-p
>> 
>> My last serious offers: nfsmachineid nfshostid
>> 
>> I strongly prefer not having uuid in the name. A UUID is
>> essentially a data type, it does not explain the purpose of
>> the content.
> 
> How do you feel about these suggestions being misleading since the output is
> not the nfs client's id?  Should we put that information in the man page?
> Do sysadmins need to know that the output of this command is (if it is even
> used at all) merely possibility of being a part of the client's id, the
> other parts come from other places in the system: the hostname and possibly
> the ip address?  That's my worry.

Yes. We're not using the output of the tool for anything
else. At the very least the man page should explain the
proposed client ID usage as an EXAMPLE with an explanation.

But honestly, I haven't seen any use case that requires
this exact functionality. Can you explain why you believe
there needs to be extra generality? (that's been one of
the main reasons I object to "nfsuuid" -- what else can
this tool be used for?)


> If we name it nfsmachineid or nfshostid,
> do you feel like we ought to have it do the much more complicated job of
> accurately outputting the actual client id?

It could do that, but the part that the kernel struggles
with is the uniquifier part. That is the part that _has_
to be done in user space (because that's the part that
requires persistence). The rest of the nfs_client_id4
argument can be formed in the kernel.


> Right now nfsuuid outputs uuids (or whatever was previously stored, up to 64
> chars).

Right. It can output anything, not just a UUID. It will
output whatever is in the special file. If the content
of that file is a random string, what will nfsuuid output?

If someone runs nfsuuid expecting a UUID and gets the
random crap that was previously stored in the persistence
file, wouldn't that be surprising?

Precisely because it has the ability to output whatever
is in the persistence file, and that file does not have
to contain a UUID, that makes this tool not "nfsuuid".


> It generates uuids, like uuidgen.  Without something external to
> itself (a udev rule, for example), it doesn't have any relationship to an
> NFS client's id.  It could plausably be used in the future for other parts
> of NFS to generate persitent uuids.  The only reason we don't just use
> `uuidgen` is that we want to wrap it with some persistence.  A would a name
> like stickyuuid be better?

No: a UUID is a data type. Would you call the tool
nfsunsignedint if we stored the ino of the net namespace
instead?

Do you have any other specific use cases for an nfsuuid
tool today? If you do, then you have a platform for more
generality. If not, then there really isn't any other
purpose for this tool. It is a single-tasker, and we
shouldn't treat it otherwise.


--
Chuck Lever




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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-11 19:04           ` Chuck Lever III
@ 2022-02-11 19:30             ` Benjamin Coddington
  2022-02-11 20:00               ` Chuck Lever III
  0 siblings, 1 reply; 36+ messages in thread
From: Benjamin Coddington @ 2022-02-11 19:30 UTC (permalink / raw)
  To: Chuck Lever III; +Cc: Steve Dickson, Linux NFS Mailing List

On 11 Feb 2022, at 14:04, Chuck Lever III wrote:
> Yes. We're not using the output of the tool for anything
> else. At the very least the man page should explain the
> proposed client ID usage as an EXAMPLE with an explanation.
>
> But honestly, I haven't seen any use case that requires
> this exact functionality. Can you explain why you believe
> there needs to be extra generality? (that's been one of
> the main reasons I object to "nfsuuid" -- what else can
> this tool be used for?)

Not yet, no.

>> If we name it nfsmachineid or nfshostid,
>> do you feel like we ought to have it do the much more complicated job 
>> of
>> accurately outputting the actual client id?
>
> It could do that, but the part that the kernel struggles
> with is the uniquifier part. That is the part that _has_
> to be done in user space (because that's the part that
> requires persistence). The rest of the nfs_client_id4
> argument can be formed in the kernel.

Sure we could, but what I was trying to point out is that your names are
just as miserable if you apply your exacting logic to them, and we're 
going
to be hard-pressed to meet the bar unless we name it something 
completely
meaningless.

>> Right now nfsuuid outputs uuids (or whatever was previously stored, 
>> up to 64
>> chars).
>
> Right. It can output anything, not just a UUID. It will
> output whatever is in the special file. If the content
> of that file is a random string, what will nfsuuid output?

64 chars of random string.

> If someone runs nfsuuid expecting a UUID and gets the
> random crap that was previously stored in the persistence
> file, wouldn't that be surprising?

Nothing on stdout surprises me.  After years of sysadmining I'm cold as 
ice.

> Precisely because it has the ability to output whatever
> is in the persistence file, and that file does not have
> to contain a UUID, that makes this tool not "nfsuuid".

Alright.

>> It generates uuids, like uuidgen.  Without something external to
>> itself (a udev rule, for example), it doesn't have any relationship 
>> to an
>> NFS client's id.  It could plausably be used in the future for other 
>> parts
>> of NFS to generate persitent uuids.  The only reason we don't just 
>> use
>> `uuidgen` is that we want to wrap it with some persistence.  A would 
>> a name
>> like stickyuuid be better?
>
> No: a UUID is a data type. Would you call the tool
> nfsunsignedint if we stored the ino of the net namespace
> instead?

I really might, sadly, just to get the job done.

> Do you have any other specific use cases for an nfsuuid
> tool today? If you do, then you have a platform for more
> generality. If not, then there really isn't any other
> purpose for this tool. It is a single-tasker, and we
> shouldn't treat it otherwise.

All the arguments for exacting tolerances on how it should be named 
apply
equally well to anything that implies its output will be used for nfs 
client
ids, or host ids.

So, I've forgotten your other suggestions that don't have anything to do
with NFS or clients or machine IDs.  I'll make more suggestions:

persistychars
mostlyuuids
theuniquifier
randomonce
distroschoice

I like these.  These names are more fun.  :)

In good will,
Ben



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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-11 19:30             ` Benjamin Coddington
@ 2022-02-11 20:00               ` Chuck Lever III
  2022-02-11 20:16                 ` Benjamin Coddington
  0 siblings, 1 reply; 36+ messages in thread
From: Chuck Lever III @ 2022-02-11 20:00 UTC (permalink / raw)
  To: Benjamin Coddington; +Cc: Steve Dickson, Linux NFS Mailing List


> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> All the arguments for exacting tolerances on how it should be named apply
> equally well to anything that implies its output will be used for nfs client
> ids, or host ids.

I completely disagree with this assessment. I object strongly
to the name nfsuuid, and you seem to be inflexible. This is
not a hill I want to die on, however I reserve the right to
say "I told you so" when it turns out to be a poor choice.


--
Chuck Lever




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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-11 20:00               ` Chuck Lever III
@ 2022-02-11 20:16                 ` Benjamin Coddington
  2022-02-11 20:51                   ` Chuck Lever III
  0 siblings, 1 reply; 36+ messages in thread
From: Benjamin Coddington @ 2022-02-11 20:16 UTC (permalink / raw)
  To: Chuck Lever III; +Cc: Steve Dickson, Linux NFS Mailing List

On 11 Feb 2022, at 15:00, Chuck Lever III wrote:

>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington 
>> <bcodding@redhat.com> wrote:
>>
>> All the arguments for exacting tolerances on how it should be named 
>> apply
>> equally well to anything that implies its output will be used for nfs 
>> client
>> ids, or host ids.
>
> I completely disagree with this assessment.

But how, and in what way?  The tool just generates uuids, and spits them
out, or it spits out whatever's in the file you specify, up to 64 chars. 
  If
we can't have uuid in the name, how can we have NFS or machine-id or
host-id?

> I object strongly to the name nfsuuid, and you seem to be inflexible. 
> This
> is not a hill I want to die on, however I reserve the right to say "I 
> told
> you so" when it turns out to be a poor choice.

How does agreeing with you multiple times in my last response and making
further suggestions for names seem inflexible to you?  This is the worst
part of working over email - I think you're misreading my good humor in 
the
face of a drudging discussion as sarcasm or ill will.

Let's find the best name.. I'm still going here.  I do really like
`persistychars`, but I expect that's too fun for everyone.  The name at
least gets closer to your bar of describing exactly what the tool does.

I'm really tired of arguing over the name, and I'm trying my best to 
keep
some good humor about it, and find something that meets everyone's 
needs.

Ben


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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-11 20:16                 ` Benjamin Coddington
@ 2022-02-11 20:51                   ` Chuck Lever III
  2022-02-11 21:06                     ` Benjamin Coddington
  0 siblings, 1 reply; 36+ messages in thread
From: Chuck Lever III @ 2022-02-11 20:51 UTC (permalink / raw)
  To: Benjamin Coddington; +Cc: Steve Dickson, Linux NFS Mailing List



> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
> 
>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
>>> 
>>> All the arguments for exacting tolerances on how it should be named apply
>>> equally well to anything that implies its output will be used for nfs client
>>> ids, or host ids.
>> 
>> I completely disagree with this assessment.
> 
> But how, and in what way?  The tool just generates uuids, and spits them
> out, or it spits out whatever's in the file you specify, up to 64 chars.  If
> we can't have uuid in the name, how can we have NFS or machine-id or
> host-id?

We don't have a tool called "string" to get and set the DNS name of
the local host. It's called "hostname".

The purpose of the proposed tool is to persist a unique string to be
used as part of an NFS client ID. I would like to name the tool based
on that purpose, not based on the way the content of the persistent
file happens to be arranged some of the time.

When the tool generates the string, it just happens to be a UUID. It
doesn't have to be. The tool could generate a digest of the boot time
or the current time. In fact, one of those is usually part of certain
types of a UUID anyway. The fact that it is a UUID is totally not
relevant. We happen to use a UUID because it has certain global
uniqueness properties. (By the way, perhaps the man page could mention
that global uniqueness is important for this identifier. Anything with
similar guaranteed global uniqueness could be used).

You keep admitting that the tool can output something that isn't a
UUID. Doesn't that make my argument for me: that the tool doesn't
generate a UUID, it manages a persistent host identifier. Just like
"hostname." Therefore "nfshostid". I really don't see how nfshostid
is just as miserable as nfsuuid -- hence, I completely disagree
that "all arguments ... apply equally well".


In fairness, I'm trying to understand why you want to stick with
"nfsuuid". You originally said you wanted a generic tool. OK, but
now you say you don't have other uses for the tool after all. You
said you don't want it to be associated with an NFS client ID. That
part I still don't grok. Can you help me understand?


>> I object strongly to the name nfsuuid, and you seem to be inflexible. This
>> is not a hill I want to die on, however I reserve the right to say "I told
>> you so" when it turns out to be a poor choice.
> 
> How does agreeing with you multiple times in my last response and making
> further suggestions for names seem inflexible to you?  This is the worst
> part of working over email - I think you're misreading my good humor in the
> face of a drudging discussion as sarcasm or ill will.

Nope, not at all. It wasn't apparent that you agreed, as much
of your reply seemed to be disagreeing with my reply. So maybe
I am overreacting. Though my reply can also be read with humor,
even though it is a bit dry.


--
Chuck Lever




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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-11 20:51                   ` Chuck Lever III
@ 2022-02-11 21:06                     ` Benjamin Coddington
  2022-02-14  0:04                       ` NeilBrown
  0 siblings, 1 reply; 36+ messages in thread
From: Benjamin Coddington @ 2022-02-11 21:06 UTC (permalink / raw)
  To: Chuck Lever III; +Cc: Steve Dickson, Linux NFS Mailing List

On 11 Feb 2022, at 15:51, Chuck Lever III wrote:

>> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington 
>> <bcodding@redhat.com> wrote:
>>
>> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
>>
>>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington 
>>>> <bcodding@redhat.com> wrote:
>>>>
>>>> All the arguments for exacting tolerances on how it should be named 
>>>> apply
>>>> equally well to anything that implies its output will be used for 
>>>> nfs client
>>>> ids, or host ids.
>>>
>>> I completely disagree with this assessment.
>>
>> But how, and in what way?  The tool just generates uuids, and spits 
>> them
>> out, or it spits out whatever's in the file you specify, up to 64 
>> chars.  If
>> we can't have uuid in the name, how can we have NFS or machine-id or
>> host-id?
>
> We don't have a tool called "string" to get and set the DNS name of
> the local host. It's called "hostname".
>
> The purpose of the proposed tool is to persist a unique string to be
> used as part of an NFS client ID. I would like to name the tool based
> on that purpose, not based on the way the content of the persistent
> file happens to be arranged some of the time.
>
> When the tool generates the string, it just happens to be a UUID. It
> doesn't have to be. The tool could generate a digest of the boot time
> or the current time. In fact, one of those is usually part of certain
> types of a UUID anyway. The fact that it is a UUID is totally not
> relevant. We happen to use a UUID because it has certain global
> uniqueness properties. (By the way, perhaps the man page could mention
> that global uniqueness is important for this identifier. Anything with
> similar guaranteed global uniqueness could be used).
>
> You keep admitting that the tool can output something that isn't a
> UUID. Doesn't that make my argument for me: that the tool doesn't
> generate a UUID, it manages a persistent host identifier. Just like
> "hostname." Therefore "nfshostid". I really don't see how nfshostid
> is just as miserable as nfsuuid -- hence, I completely disagree
> that "all arguments ... apply equally well".

Yes - your arguement is a good one.   I wasn't clear enough admitting 
you
were right two emails ago, sorry about that.

However, I still feel the same argument applied to "nfshostid" 
disqualifies
it as well.  It doesn't output the nfshostid.  That, if it even contains 
the
part outputted, is more than what's written out.

In my experience with linux tools, nfshostid sounds like something I can 
use
to set or retrieve the identifier for an NFS host, and this little tool 
does
not do that.

> In fairness, I'm trying to understand why you want to stick with
> "nfsuuid". You originally said you wanted a generic tool. OK, but
> now you say you don't have other uses for the tool after all. You
> said you don't want it to be associated with an NFS client ID. That
> part I still don't grok. Can you help me understand?

I don't want to stick with nfsuuid, I'm just trying to apply everyone's
reasoning to the current list of suggested names and it appears to me
that we've disqualified them all.

>>> I object strongly to the name nfsuuid, and you seem to be 
>>> inflexible. This
>>> is not a hill I want to die on, however I reserve the right to say 
>>> "I told
>>> you so" when it turns out to be a poor choice.
>>
>> How does agreeing with you multiple times in my last response and 
>> making
>> further suggestions for names seem inflexible to you?  This is the 
>> worst
>> part of working over email - I think you're misreading my good humor 
>> in the
>> face of a drudging discussion as sarcasm or ill will.
>
> Nope, not at all. It wasn't apparent that you agreed, as much
> of your reply seemed to be disagreeing with my reply. So maybe
> I am overreacting. Though my reply can also be read with humor,
> even though it is a bit dry.

Sorry about that -- I really just want to move forward.  I can send it 
again
as nfshostid.  I think if we are clear in the man page that it isn't
necessarily part or any of the identifier used by NFS, rather its just
trying to manage the persistent unique portion, it works for me.

Unless we can go with persistychars, because that makes me grin.

Ben


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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-11 21:06                     ` Benjamin Coddington
@ 2022-02-14  0:04                       ` NeilBrown
  2022-02-14 11:15                         ` Benjamin Coddington
  0 siblings, 1 reply; 36+ messages in thread
From: NeilBrown @ 2022-02-14  0:04 UTC (permalink / raw)
  To: Benjamin Coddington
  Cc: Chuck Lever III, Steve Dickson, Linux NFS Mailing List

On Sat, 12 Feb 2022, Benjamin Coddington wrote:
> On 11 Feb 2022, at 15:51, Chuck Lever III wrote:
> 
> >> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington 
> >> <bcodding@redhat.com> wrote:
> >>
> >> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
> >>
> >>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington 
> >>>> <bcodding@redhat.com> wrote:
> >>>>
> >>>> All the arguments for exacting tolerances on how it should be named 
> >>>> apply
> >>>> equally well to anything that implies its output will be used for 
> >>>> nfs client
> >>>> ids, or host ids.
> >>>
> >>> I completely disagree with this assessment.
> >>
> >> But how, and in what way?  The tool just generates uuids, and spits 
> >> them
> >> out, or it spits out whatever's in the file you specify, up to 64 
> >> chars.  If
> >> we can't have uuid in the name, how can we have NFS or machine-id or
> >> host-id?
> >
> > We don't have a tool called "string" to get and set the DNS name of
> > the local host. It's called "hostname".
> >
> > The purpose of the proposed tool is to persist a unique string to be
> > used as part of an NFS client ID. I would like to name the tool based
> > on that purpose, not based on the way the content of the persistent
> > file happens to be arranged some of the time.
> >
> > When the tool generates the string, it just happens to be a UUID. It
> > doesn't have to be. The tool could generate a digest of the boot time
> > or the current time. In fact, one of those is usually part of certain
> > types of a UUID anyway. The fact that it is a UUID is totally not
> > relevant. We happen to use a UUID because it has certain global
> > uniqueness properties. (By the way, perhaps the man page could mention
> > that global uniqueness is important for this identifier. Anything with
> > similar guaranteed global uniqueness could be used).
> >
> > You keep admitting that the tool can output something that isn't a
> > UUID. Doesn't that make my argument for me: that the tool doesn't
> > generate a UUID, it manages a persistent host identifier. Just like
> > "hostname." Therefore "nfshostid". I really don't see how nfshostid
> > is just as miserable as nfsuuid -- hence, I completely disagree
> > that "all arguments ... apply equally well".
> 
> Yes - your arguement is a good one.   I wasn't clear enough admitting 
> you
> were right two emails ago, sorry about that.
> 
> However, I still feel the same argument applied to "nfshostid" 
> disqualifies
> it as well.  It doesn't output the nfshostid.  That, if it even contains 
> the
> part outputted, is more than what's written out.
> 
> In my experience with linux tools, nfshostid sounds like something I can 
> use
> to set or retrieve the identifier for an NFS host, and this little tool 
> does
> not do that.
> 

I agree.  This tool primarily does 1 thing - it sets a string which will
be the uniquifier using the the client_owner4.  So I think the word
"set" should appear in the name.  I also think the name should start "nfs".
I don't much care whether it is
  nfssetid
  nfs-set-uuid
  nfssetowner
  nfssetuniquifier
  nfssetidentity
  nfsidset
though perhaps I'd prefer
  nfs=set=id

If not given any args, it should probably print a usage message rather
than perform a default action, to reduce the number of holes in feet.

.... Naming  - THE hard problem of computer engineering ....

NeilBrown

  

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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-14  0:04                       ` NeilBrown
@ 2022-02-14 11:15                         ` Benjamin Coddington
  2022-02-14 15:39                           ` Chuck Lever III
  2022-02-14 22:40                           ` NeilBrown
  0 siblings, 2 replies; 36+ messages in thread
From: Benjamin Coddington @ 2022-02-14 11:15 UTC (permalink / raw)
  To: NeilBrown; +Cc: Chuck Lever III, Steve Dickson, Linux NFS Mailing List

On 13 Feb 2022, at 19:04, NeilBrown wrote:

> On Sat, 12 Feb 2022, Benjamin Coddington wrote:
>> On 11 Feb 2022, at 15:51, Chuck Lever III wrote:
>>
>>>> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington
>>>> <bcodding@redhat.com> wrote:
>>>>
>>>> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
>>>>
>>>>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington
>>>>>> <bcodding@redhat.com> wrote:
>>>>>>
>>>>>> All the arguments for exacting tolerances on how it should be named
>>>>>> apply
>>>>>> equally well to anything that implies its output will be used for
>>>>>> nfs client
>>>>>> ids, or host ids.
>>>>>
>>>>> I completely disagree with this assessment.
>>>>
>>>> But how, and in what way?  The tool just generates uuids, and spits
>>>> them
>>>> out, or it spits out whatever's in the file you specify, up to 64
>>>> chars.  If
>>>> we can't have uuid in the name, how can we have NFS or machine-id or
>>>> host-id?
>>>
>>> We don't have a tool called "string" to get and set the DNS name of
>>> the local host. It's called "hostname".
>>>
>>> The purpose of the proposed tool is to persist a unique string to be
>>> used as part of an NFS client ID. I would like to name the tool based
>>> on that purpose, not based on the way the content of the persistent
>>> file happens to be arranged some of the time.
>>>
>>> When the tool generates the string, it just happens to be a UUID. It
>>> doesn't have to be. The tool could generate a digest of the boot time
>>> or the current time. In fact, one of those is usually part of certain
>>> types of a UUID anyway. The fact that it is a UUID is totally not
>>> relevant. We happen to use a UUID because it has certain global
>>> uniqueness properties. (By the way, perhaps the man page could mention
>>> that global uniqueness is important for this identifier. Anything with
>>> similar guaranteed global uniqueness could be used).
>>>
>>> You keep admitting that the tool can output something that isn't a
>>> UUID. Doesn't that make my argument for me: that the tool doesn't
>>> generate a UUID, it manages a persistent host identifier. Just like
>>> "hostname." Therefore "nfshostid". I really don't see how nfshostid
>>> is just as miserable as nfsuuid -- hence, I completely disagree
>>> that "all arguments ... apply equally well".
>>
>> Yes - your arguement is a good one.   I wasn't clear enough admitting
>> you
>> were right two emails ago, sorry about that.
>>
>> However, I still feel the same argument applied to "nfshostid"
>> disqualifies
>> it as well.  It doesn't output the nfshostid.  That, if it even contains
>> the
>> part outputted, is more than what's written out.
>>
>> In my experience with linux tools, nfshostid sounds like something I can
>> use
>> to set or retrieve the identifier for an NFS host, and this little tool
>> does
>> not do that.
>>
>
> I agree.  This tool primarily does 1 thing - it sets a string which will
> be the uniquifier using the the client_owner4.  So I think the word
> "set" should appear in the name.  I also think the name should start "nfs".
> I don't much care whether it is
>   nfssetid
>   nfs-set-uuid
>   nfssetowner
>   nfssetuniquifier
>   nfssetidentity
>   nfsidset
> though perhaps I'd prefer
>   nfs=set=id
>
> If not given any args, it should probably print a usage message rather
> than perform a default action, to reduce the number of holes in feet.
>
> .... Naming  - THE hard problem of computer engineering ....

No, it does NOT set the uniquifier string.  It returns it on stdout.  If
you run it without args it just prints the string.  Its completely harmless.

Ben


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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-14 11:15                         ` Benjamin Coddington
@ 2022-02-14 15:39                           ` Chuck Lever III
  2022-02-16 19:01                             ` Benjamin Coddington
  2022-02-14 22:40                           ` NeilBrown
  1 sibling, 1 reply; 36+ messages in thread
From: Chuck Lever III @ 2022-02-14 15:39 UTC (permalink / raw)
  To: Benjamin Coddington, Neil Brown; +Cc: Steve Dickson, Linux NFS Mailing List



> On Feb 14, 2022, at 6:15 AM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> On 13 Feb 2022, at 19:04, NeilBrown wrote:
> 
>> On Sat, 12 Feb 2022, Benjamin Coddington wrote:
>>> On 11 Feb 2022, at 15:51, Chuck Lever III wrote:
>>> 
>>>>> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington
>>>>> <bcodding@redhat.com> wrote:
>>>>> 
>>>>> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
>>>>> 
>>>>>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington
>>>>>>> <bcodding@redhat.com> wrote:
>>>>>>> 
>>>>>>> All the arguments for exacting tolerances on how it should be named
>>>>>>> apply
>>>>>>> equally well to anything that implies its output will be used for
>>>>>>> nfs client
>>>>>>> ids, or host ids.
>>>>>> 
>>>>>> I completely disagree with this assessment.
>>>>> 
>>>>> But how, and in what way?  The tool just generates uuids, and spits
>>>>> them
>>>>> out, or it spits out whatever's in the file you specify, up to 64
>>>>> chars.  If
>>>>> we can't have uuid in the name, how can we have NFS or machine-id or
>>>>> host-id?
>>>> 
>>>> We don't have a tool called "string" to get and set the DNS name of
>>>> the local host. It's called "hostname".
>>>> 
>>>> The purpose of the proposed tool is to persist a unique string to be
>>>> used as part of an NFS client ID. I would like to name the tool based
>>>> on that purpose, not based on the way the content of the persistent
>>>> file happens to be arranged some of the time.
>>>> 
>>>> When the tool generates the string, it just happens to be a UUID. It
>>>> doesn't have to be. The tool could generate a digest of the boot time
>>>> or the current time. In fact, one of those is usually part of certain
>>>> types of a UUID anyway. The fact that it is a UUID is totally not
>>>> relevant. We happen to use a UUID because it has certain global
>>>> uniqueness properties. (By the way, perhaps the man page could mention
>>>> that global uniqueness is important for this identifier. Anything with
>>>> similar guaranteed global uniqueness could be used).
>>>> 
>>>> You keep admitting that the tool can output something that isn't a
>>>> UUID. Doesn't that make my argument for me: that the tool doesn't
>>>> generate a UUID, it manages a persistent host identifier. Just like
>>>> "hostname." Therefore "nfshostid". I really don't see how nfshostid
>>>> is just as miserable as nfsuuid -- hence, I completely disagree
>>>> that "all arguments ... apply equally well".
>>> 
>>> Yes - your arguement is a good one.   I wasn't clear enough admitting
>>> you
>>> were right two emails ago, sorry about that.
>>> 
>>> However, I still feel the same argument applied to "nfshostid"
>>> disqualifies
>>> it as well.  It doesn't output the nfshostid.  That, if it even contains
>>> the
>>> part outputted, is more than what's written out.
>>> 
>>> In my experience with linux tools, nfshostid sounds like something I can
>>> use
>>> to set or retrieve the identifier for an NFS host, and this little tool
>>> does
>>> not do that.
>>> 
>> 
>> I agree.  This tool primarily does 1 thing - it sets a string which will
>> be the uniquifier using the the client_owner4.  So I think the word
>> "set" should appear in the name.  I also think the name should start "nfs".
>> I don't much care whether it is
>>  nfssetid
>>  nfs-set-uuid
>>  nfssetowner
>>  nfssetuniquifier
>>  nfssetidentity
>>  nfsidset
>> though perhaps I'd prefer
>>  nfs=set=id
>> 
>> If not given any args, it should probably print a usage message rather
>> than perform a default action, to reduce the number of holes in feet.
>> 
>> .... Naming  - THE hard problem of computer engineering ....
> 
> No, it does NOT set the uniquifier string.  It returns it on stdout.  If
> you run it without args it just prints the string.  Its completely harmless.

OK, my understanding was that if you run it as root, and the
string isn't already set, it does indeed set a value in the
persistence file.

That should be harmless, though. Once it is set, it is always
the same afterwards, and that's fine. Someone who just types
in the command to see what it does can't damage their
metatarsals; the worst that happens is the persistence file
is never used again.

I agree that's not very "set"-like.

 nfsgetuniquifier
 nfsguestuniquifier
 nfsnsuniquifier
 ns-uniquifier

Use with copious amounts of tab completion.

--
Chuck Lever




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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-14 11:15                         ` Benjamin Coddington
  2022-02-14 15:39                           ` Chuck Lever III
@ 2022-02-14 22:40                           ` NeilBrown
  1 sibling, 0 replies; 36+ messages in thread
From: NeilBrown @ 2022-02-14 22:40 UTC (permalink / raw)
  To: Benjamin Coddington
  Cc: Chuck Lever III, Steve Dickson, Linux NFS Mailing List

On Mon, 14 Feb 2022, Benjamin Coddington wrote:
> On 13 Feb 2022, at 19:04, NeilBrown wrote:
> 
> > On Sat, 12 Feb 2022, Benjamin Coddington wrote:
> >> On 11 Feb 2022, at 15:51, Chuck Lever III wrote:
> >>
> >>>> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington
> >>>> <bcodding@redhat.com> wrote:
> >>>>
> >>>> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
> >>>>
> >>>>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington
> >>>>>> <bcodding@redhat.com> wrote:
> >>>>>>
> >>>>>> All the arguments for exacting tolerances on how it should be named
> >>>>>> apply
> >>>>>> equally well to anything that implies its output will be used for
> >>>>>> nfs client
> >>>>>> ids, or host ids.
> >>>>>
> >>>>> I completely disagree with this assessment.
> >>>>
> >>>> But how, and in what way?  The tool just generates uuids, and spits
> >>>> them
> >>>> out, or it spits out whatever's in the file you specify, up to 64
> >>>> chars.  If
> >>>> we can't have uuid in the name, how can we have NFS or machine-id or
> >>>> host-id?
> >>>
> >>> We don't have a tool called "string" to get and set the DNS name of
> >>> the local host. It's called "hostname".
> >>>
> >>> The purpose of the proposed tool is to persist a unique string to be
> >>> used as part of an NFS client ID. I would like to name the tool based
> >>> on that purpose, not based on the way the content of the persistent
> >>> file happens to be arranged some of the time.
> >>>
> >>> When the tool generates the string, it just happens to be a UUID. It
> >>> doesn't have to be. The tool could generate a digest of the boot time
> >>> or the current time. In fact, one of those is usually part of certain
> >>> types of a UUID anyway. The fact that it is a UUID is totally not
> >>> relevant. We happen to use a UUID because it has certain global
> >>> uniqueness properties. (By the way, perhaps the man page could mention
> >>> that global uniqueness is important for this identifier. Anything with
> >>> similar guaranteed global uniqueness could be used).
> >>>
> >>> You keep admitting that the tool can output something that isn't a
> >>> UUID. Doesn't that make my argument for me: that the tool doesn't
> >>> generate a UUID, it manages a persistent host identifier. Just like
> >>> "hostname." Therefore "nfshostid". I really don't see how nfshostid
> >>> is just as miserable as nfsuuid -- hence, I completely disagree
> >>> that "all arguments ... apply equally well".
> >>
> >> Yes - your arguement is a good one.   I wasn't clear enough admitting
> >> you
> >> were right two emails ago, sorry about that.
> >>
> >> However, I still feel the same argument applied to "nfshostid"
> >> disqualifies
> >> it as well.  It doesn't output the nfshostid.  That, if it even contains
> >> the
> >> part outputted, is more than what's written out.
> >>
> >> In my experience with linux tools, nfshostid sounds like something I can
> >> use
> >> to set or retrieve the identifier for an NFS host, and this little tool
> >> does
> >> not do that.
> >>
> >
> > I agree.  This tool primarily does 1 thing - it sets a string which will
> > be the uniquifier using the the client_owner4.  So I think the word
> > "set" should appear in the name.  I also think the name should start "nfs".
> > I don't much care whether it is
> >   nfssetid
> >   nfs-set-uuid
> >   nfssetowner
> >   nfssetuniquifier
> >   nfssetidentity
> >   nfsidset
> > though perhaps I'd prefer
> >   nfs=set=id
> >
> > If not given any args, it should probably print a usage message rather
> > than perform a default action, to reduce the number of holes in feet.
> >
> > .... Naming  - THE hard problem of computer engineering ....
> 
> No, it does NOT set the uniquifier string.  It returns it on stdout.  If
> you run it without args it just prints the string.  Its completely harmless.

Ahhh... I missed that.
Hence the "%c" magic in the udev rules, which takes the output and
stores it in the attribute.
This is a defensible approach if the tool will always be used with udev
... but you know what I think of using udev here.

Nevertheless, the main purpose when running this program is to set the
identity, even though the current implementation offloads some of the
work to udev.
So I think it should do exactly that and be named accordingly.

NeilBrown

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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-14 15:39                           ` Chuck Lever III
@ 2022-02-16 19:01                             ` Benjamin Coddington
  2022-02-16 19:35                               ` Chuck Lever III
  0 siblings, 1 reply; 36+ messages in thread
From: Benjamin Coddington @ 2022-02-16 19:01 UTC (permalink / raw)
  To: Chuck Lever III; +Cc: Neil Brown, Steve Dickson, Linux NFS Mailing List

On 14 Feb 2022, at 10:39, Chuck Lever III wrote:

>> On Feb 14, 2022, at 6:15 AM, Benjamin Coddington 
>> <bcodding@redhat.com> wrote:
>>
>> On 13 Feb 2022, at 19:04, NeilBrown wrote:
>>
>>> On Sat, 12 Feb 2022, Benjamin Coddington wrote:
>>>> On 11 Feb 2022, at 15:51, Chuck Lever III wrote:
>>>>
>>>>>> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington
>>>>>> <bcodding@redhat.com> wrote:
>>>>>>
>>>>>> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
>>>>>>
>>>>>>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington
>>>>>>>> <bcodding@redhat.com> wrote:
>>>>>>>>
>>>>>>>> All the arguments for exacting tolerances on how it should be 
>>>>>>>> named
>>>>>>>> apply
>>>>>>>> equally well to anything that implies its output will be used 
>>>>>>>> for
>>>>>>>> nfs client
>>>>>>>> ids, or host ids.
>>>>>>>
>>>>>>> I completely disagree with this assessment.
>>>>>>
>>>>>> But how, and in what way?  The tool just generates uuids, and 
>>>>>> spits
>>>>>> them
>>>>>> out, or it spits out whatever's in the file you specify, up to 64
>>>>>> chars.  If
>>>>>> we can't have uuid in the name, how can we have NFS or machine-id 
>>>>>> or
>>>>>> host-id?
>>>>>
>>>>> We don't have a tool called "string" to get and set the DNS name 
>>>>> of
>>>>> the local host. It's called "hostname".
>>>>>
>>>>> The purpose of the proposed tool is to persist a unique string to 
>>>>> be
>>>>> used as part of an NFS client ID. I would like to name the tool 
>>>>> based
>>>>> on that purpose, not based on the way the content of the 
>>>>> persistent
>>>>> file happens to be arranged some of the time.
>>>>>
>>>>> When the tool generates the string, it just happens to be a UUID. 
>>>>> It
>>>>> doesn't have to be. The tool could generate a digest of the boot 
>>>>> time
>>>>> or the current time. In fact, one of those is usually part of 
>>>>> certain
>>>>> types of a UUID anyway. The fact that it is a UUID is totally not
>>>>> relevant. We happen to use a UUID because it has certain global
>>>>> uniqueness properties. (By the way, perhaps the man page could 
>>>>> mention
>>>>> that global uniqueness is important for this identifier. Anything 
>>>>> with
>>>>> similar guaranteed global uniqueness could be used).
>>>>>
>>>>> You keep admitting that the tool can output something that isn't a
>>>>> UUID. Doesn't that make my argument for me: that the tool doesn't
>>>>> generate a UUID, it manages a persistent host identifier. Just 
>>>>> like
>>>>> "hostname." Therefore "nfshostid". I really don't see how 
>>>>> nfshostid
>>>>> is just as miserable as nfsuuid -- hence, I completely disagree
>>>>> that "all arguments ... apply equally well".
>>>>
>>>> Yes - your arguement is a good one.   I wasn't clear enough 
>>>> admitting
>>>> you
>>>> were right two emails ago, sorry about that.
>>>>
>>>> However, I still feel the same argument applied to "nfshostid"
>>>> disqualifies
>>>> it as well.  It doesn't output the nfshostid.  That, if it even 
>>>> contains
>>>> the
>>>> part outputted, is more than what's written out.
>>>>
>>>> In my experience with linux tools, nfshostid sounds like something 
>>>> I can
>>>> use
>>>> to set or retrieve the identifier for an NFS host, and this little 
>>>> tool
>>>> does
>>>> not do that.
>>>>
>>>
>>> I agree.  This tool primarily does 1 thing - it sets a string which 
>>> will
>>> be the uniquifier using the the client_owner4.  So I think the word
>>> "set" should appear in the name.  I also think the name should start 
>>> "nfs".
>>> I don't much care whether it is
>>>  nfssetid
>>>  nfs-set-uuid
>>>  nfssetowner
>>>  nfssetuniquifier
>>>  nfssetidentity
>>>  nfsidset
>>> though perhaps I'd prefer
>>>  nfs=set=id
>>>
>>> If not given any args, it should probably print a usage message 
>>> rather
>>> than perform a default action, to reduce the number of holes in 
>>> feet.
>>>
>>> .... Naming  - THE hard problem of computer engineering ....
>>
>> No, it does NOT set the uniquifier string.  It returns it on stdout.  
>> If
>> you run it without args it just prints the string.  Its completely 
>> harmless.
>
> OK, my understanding was that if you run it as root, and the
> string isn't already set, it does indeed set a value in the
> persistence file.
>
> That should be harmless, though. Once it is set, it is always
> the same afterwards, and that's fine. Someone who just types
> in the command to see what it does can't damage their
> metatarsals; the worst that happens is the persistence file
> is never used again.
>
> I agree that's not very "set"-like.
>
>  nfsgetuniquifier
>  nfsguestuniquifier
>  nfsnsuniquifier
>  ns-uniquifier
>
> Use with copious amounts of tab completion.

Coming back to this.. so it seems at least part of our disagreement 
about
the name had to do with a misunderstanding of what the tool did.

Just to finally clear the air about it: the tool does not write to 
sysfs, it
doesn't try to modify the client in any way.  I'm going to leave it up 
to
the distros.

Considering this, I think your only remaining objection to "nfsuuid" is 
that it
might return data other than a uuid if someone points it at a file that
contains data other than a uuid.

I can fix that.  And its probably not a bad idea either.  The nfsuuid 
tool
can ensure that the persisted data is a uuid.

Maybe I also need to change the man page or description of the patch to 
be
clearer about what the tool does.  Any suggestions there?

Ben


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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-16 19:01                             ` Benjamin Coddington
@ 2022-02-16 19:35                               ` Chuck Lever III
  2022-02-16 20:44                                 ` Benjamin Coddington
  0 siblings, 1 reply; 36+ messages in thread
From: Chuck Lever III @ 2022-02-16 19:35 UTC (permalink / raw)
  To: Benjamin Coddington; +Cc: Neil Brown, Steve Dickson, Linux NFS Mailing List



> On Feb 16, 2022, at 2:01 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> On 14 Feb 2022, at 10:39, Chuck Lever III wrote:
> 
>>> On Feb 14, 2022, at 6:15 AM, Benjamin Coddington <bcodding@redhat.com> wrote:
>>> 
>>> On 13 Feb 2022, at 19:04, NeilBrown wrote:
>>> 
>>>> On Sat, 12 Feb 2022, Benjamin Coddington wrote:
>>>>> On 11 Feb 2022, at 15:51, Chuck Lever III wrote:
>>>>> 
>>>>>>> On Feb 11, 2022, at 3:16 PM, Benjamin Coddington
>>>>>>> <bcodding@redhat.com> wrote:
>>>>>>> 
>>>>>>> On 11 Feb 2022, at 15:00, Chuck Lever III wrote:
>>>>>>> 
>>>>>>>>> On Feb 11, 2022, at 2:30 PM, Benjamin Coddington
>>>>>>>>> <bcodding@redhat.com> wrote:
>>>>>>>>> 
>>>>>>>>> All the arguments for exacting tolerances on how it should be named
>>>>>>>>> apply
>>>>>>>>> equally well to anything that implies its output will be used for
>>>>>>>>> nfs client
>>>>>>>>> ids, or host ids.
>>>>>>>> 
>>>>>>>> I completely disagree with this assessment.
>>>>>>> 
>>>>>>> But how, and in what way?  The tool just generates uuids, and spits
>>>>>>> them
>>>>>>> out, or it spits out whatever's in the file you specify, up to 64
>>>>>>> chars.  If
>>>>>>> we can't have uuid in the name, how can we have NFS or machine-id or
>>>>>>> host-id?
>>>>>> 
>>>>>> We don't have a tool called "string" to get and set the DNS name of
>>>>>> the local host. It's called "hostname".
>>>>>> 
>>>>>> The purpose of the proposed tool is to persist a unique string to be
>>>>>> used as part of an NFS client ID. I would like to name the tool based
>>>>>> on that purpose, not based on the way the content of the persistent
>>>>>> file happens to be arranged some of the time.
>>>>>> 
>>>>>> When the tool generates the string, it just happens to be a UUID. It
>>>>>> doesn't have to be. The tool could generate a digest of the boot time
>>>>>> or the current time. In fact, one of those is usually part of certain
>>>>>> types of a UUID anyway. The fact that it is a UUID is totally not
>>>>>> relevant. We happen to use a UUID because it has certain global
>>>>>> uniqueness properties. (By the way, perhaps the man page could mention
>>>>>> that global uniqueness is important for this identifier. Anything with
>>>>>> similar guaranteed global uniqueness could be used).
>>>>>> 
>>>>>> You keep admitting that the tool can output something that isn't a
>>>>>> UUID. Doesn't that make my argument for me: that the tool doesn't
>>>>>> generate a UUID, it manages a persistent host identifier. Just like
>>>>>> "hostname." Therefore "nfshostid". I really don't see how nfshostid
>>>>>> is just as miserable as nfsuuid -- hence, I completely disagree
>>>>>> that "all arguments ... apply equally well".
>>>>> 
>>>>> Yes - your arguement is a good one.   I wasn't clear enough admitting
>>>>> you
>>>>> were right two emails ago, sorry about that.
>>>>> 
>>>>> However, I still feel the same argument applied to "nfshostid"
>>>>> disqualifies
>>>>> it as well.  It doesn't output the nfshostid.  That, if it even contains
>>>>> the
>>>>> part outputted, is more than what's written out.
>>>>> 
>>>>> In my experience with linux tools, nfshostid sounds like something I can
>>>>> use
>>>>> to set or retrieve the identifier for an NFS host, and this little tool
>>>>> does
>>>>> not do that.
>>>>> 
>>>> 
>>>> I agree.  This tool primarily does 1 thing - it sets a string which will
>>>> be the uniquifier using the the client_owner4.  So I think the word
>>>> "set" should appear in the name.  I also think the name should start "nfs".
>>>> I don't much care whether it is
>>>> nfssetid
>>>> nfs-set-uuid
>>>> nfssetowner
>>>> nfssetuniquifier
>>>> nfssetidentity
>>>> nfsidset
>>>> though perhaps I'd prefer
>>>> nfs=set=id
>>>> 
>>>> If not given any args, it should probably print a usage message rather
>>>> than perform a default action, to reduce the number of holes in feet.
>>>> 
>>>> .... Naming  - THE hard problem of computer engineering ....
>>> 
>>> No, it does NOT set the uniquifier string.  It returns it on stdout.  If
>>> you run it without args it just prints the string.  Its completely harmless.
>> 
>> OK, my understanding was that if you run it as root, and the
>> string isn't already set, it does indeed set a value in the
>> persistence file.
>> 
>> That should be harmless, though. Once it is set, it is always
>> the same afterwards, and that's fine. Someone who just types
>> in the command to see what it does can't damage their
>> metatarsals; the worst that happens is the persistence file
>> is never used again.
>> 
>> I agree that's not very "set"-like.
>> 
>> nfsgetuniquifier
>> nfsguestuniquifier
>> nfsnsuniquifier
>> ns-uniquifier
>> 
>> Use with copious amounts of tab completion.
> 
> Coming back to this.. so it seems at least part of our disagreement about
> the name had to do with a misunderstanding of what the tool did.

I understand what your implementation does. I don't
understand why it works that way.

The disagreement is that I feel like you're trying to
abstract the operation of this little tool in ways that
no-one asked for. It's purpose is very narrow and
completely related to the NFSv4 client ID.


> Just to finally clear the air about it: the tool does not write to sysfs, it
> doesn't try to modify the client in any way.  I'm going to leave it up to
> the distros.

Seems to me that the distros care only about where the
persistence file is located. I can't see a problem with
Neil's suggestion that the tool also write into sysfs.
The location of the sysfs API is invariant; there should
be no need to configure it or expose it.

Can you give a reason why the tool should /not/ write
into sysfs?


> Considering this, I think your only remaining objection to "nfsuuid" is that it
> might return data other than a uuid if someone points it at a file that
> contains data other than a uuid.
> 
> I can fix that.  And its probably not a bad idea either.  The nfsuuid tool
> can ensure that the persisted data is a uuid.

Why is that necessary? I agree that any random string will
do, as long as it is the same after client reboots and is
globally unique. It can be a BLAKE2 hash or anything else.
Is there a hard requirement that the uniquifier is in the
form of an RFC 4122 UUID? (if there is, the requirement
should be explained in the man page).

I have no problem with using a UUID. I just don't believe
there's a hard requirement that it /must/ be a UUID.


> Maybe I also need to change the man page or description of the patch to be
> clearer about what the tool does.  Any suggestions there?

I've made some in previous responses. The rules about
reboot persistence and global uniqueness are paramount.

So I initially agreed with Trond's statement that the
client ID uniquifier is not specific to a particular
mount request, so doesn't belong in mount.nfs.

All still true. But...

If mount.nfs handles it instead, you don't need a
separate tool (naming problem solved), there's no lag
between when the uniquifier is set and the first
SETCLIENTID (race condition solved), no udev rule is
needed (no additional implementation complexity), and
no man page and no new module parameters (no
additional administrative configuration complexity).

What's not to like?


--
Chuck Lever




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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-16 19:35                               ` Chuck Lever III
@ 2022-02-16 20:44                                 ` Benjamin Coddington
  2022-02-16 23:16                                   ` NeilBrown
  2022-02-17 14:43                                   ` [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers Chuck Lever III
  0 siblings, 2 replies; 36+ messages in thread
From: Benjamin Coddington @ 2022-02-16 20:44 UTC (permalink / raw)
  To: Chuck Lever III; +Cc: Neil Brown, Steve Dickson, Linux NFS Mailing List

On 16 Feb 2022, at 14:35, Chuck Lever III wrote:

> On Feb 16, 2022, at 2:01 PM, Benjamin Coddington <bcodding@redhat.com> 
> wrote:
>> Coming back to this.. so it seems at least part of our disagreement 
>> about
>> the name had to do with a misunderstanding of what the tool did.
>
> I understand what your implementation does. I don't
> understand why it works that way.
>
> The disagreement is that I feel like you're trying to
> abstract the operation of this little tool in ways that
> no-one asked for. It's purpose is very narrow and
> completely related to the NFSv4 client ID.
>
>
>> Just to finally clear the air about it: the tool does not write to 
>> sysfs, it
>> doesn't try to modify the client in any way.  I'm going to leave it 
>> up to
>> the distros.
>
> Seems to me that the distros care only about where the
> persistence file is located. I can't see a problem with
> Neil's suggestion that the tool also write into sysfs.
> The location of the sysfs API is invariant; there should
> be no need to configure it or expose it.
>
> Can you give a reason why the tool should /not/ write
> into sysfs?

So that there is a division of work.  Systemd-udevd handles the event 
and
sets the attribute, not the tool.  Systemd-udevd is already set up to 
have
the appropriate selinux contexts and rights to make these changes.
Systemd-udevd's logging can clearly show the action and result instead
of it being hidden away inside this tool.  Systemd-udevd doesn't expect
programs to make the changes, its designed around the idea that programs
provide information and it makes the changes.

You can see how many issues we got into by imagining what would happen 
if
users ran this tool.  If the tool doesn't modify the client, that's one 
less
worry we have upstream, its the distro's problem.

If the tool writes to sysfs, then someone executing it manually can 
change
the client's id with a live mount.  That's bad.  That's less likely to
happen if its all tucked away in a udev rule.  I know the 
counter-arguement
"you can write to sysfs yourself", but I still think its better this 
way.

There's increased flexibility and utility - if an expert sysadmin wants 
to
distinguish groups of clients, they can customize their udev rule to add
custom strings to the uniquifier using the many additional points of 
data
available in udev rules.

>> Considering this, I think your only remaining objection to "nfsuuid" 
>> is that it
>> might return data other than a uuid if someone points it at a file 
>> that
>> contains data other than a uuid.
>>
>> I can fix that.  And its probably not a bad idea either.  The nfsuuid 
>> tool
>> can ensure that the persisted data is a uuid.
>
> Why is that necessary? I agree that any random string will
> do, as long as it is the same after client reboots and is
> globally unique. It can be a BLAKE2 hash or anything else.
> Is there a hard requirement that the uniquifier is in the
> form of an RFC 4122 UUID? (if there is, the requirement
> should be explained in the man page).
>
> I have no problem with using a UUID. I just don't believe
> there's a hard requirement that it /must/ be a UUID.

There's no hard requirement, as you know.  Its just an extremely useful
datatype for this purpose.  I'm sure we could make a `nfsblake2`, but I
can't imagine why.

>> Maybe I also need to change the man page or description of the patch 
>> to be
>> clearer about what the tool does.  Any suggestions there?
>
> I've made some in previous responses. The rules about
> reboot persistence and global uniqueness are paramount.

Ok, I can re-read the threads and find them and include them.

> So I initially agreed with Trond's statement that the
> client ID uniquifier is not specific to a particular
> mount request, so doesn't belong in mount.nfs.
>
> All still true. But...
>
> If mount.nfs handles it instead, you don't need a
> separate tool (naming problem solved), there's no lag
> between when the uniquifier is set and the first
> SETCLIENTID (race condition solved), no udev rule is
> needed (no additional implementation complexity), and
> no man page and no new module parameters (no
> additional administrative configuration complexity).

Yep.  The race is a problem.

> What's not to like?

Not much.  How can we figure out what we want to do as a community 
before
doing it all the ways?

Ben


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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-16 20:44                                 ` Benjamin Coddington
@ 2022-02-16 23:16                                   ` NeilBrown
  2022-03-01  3:43                                     ` [PATCH] nfs.man: document requirements for NFS mounts in a container NeilBrown
  2022-02-17 14:43                                   ` [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers Chuck Lever III
  1 sibling, 1 reply; 36+ messages in thread
From: NeilBrown @ 2022-02-16 23:16 UTC (permalink / raw)
  To: Benjamin Coddington
  Cc: Chuck Lever III, Steve Dickson, Linux NFS Mailing List

On Thu, 17 Feb 2022, Benjamin Coddington wrote:
> How can we figure out what we want to do as a community before doing
> it all the ways?

The evidence suggests that we can't.  Maybe we shouldn't.

Or maybe we can take a completely different approach.  As I like to say:
"When in doubt, document."  (OK, I just made that up...)

I propose the following text to be added to nfs(5) or maybe put in a
separate man page - nfs-container(5).


NFS IN A CONTAINER
  When NFS is used to mount filesystems in a container, and specifically
  in a separate network namespace, these mounts are treated as quite
  separate from any mounts in a different container or not in a
  container (i.e. in a different network namespace).

  In the NFSv4.x protocol, each client must have a unique identifier.
  This is used by the server to determine when a client has restarted,
  so any state from a previous instance can be discarded.  So any two
  concurrent clients that might access the same server MUST have
  different identifiers, and any two consecutive instances of the same
  client SHOULD have the same identifier.

  Linux constructs the identifier (referred to as co_ownerid in the NFS
  specifications) from various pieces of information, three of which can
  be controlled by the sysadmin:

  - the hostname.  This can be different in different containers if they
    have different "UTS" namespaces.  If the container system ensures
    each container sees a unique host name, then this is
    sufficient for a correctly functioning NFS identifier.
    The host name is copied when the first NFS filesystem is mounted in
    a given network namespace.  Any subsequent change in the apparent
    hostname will not change the NFSv4 identifier.

  - the value of the nfs.nfs4_unique_id module parameter.  This is the
    same for all containers on a given host so is not useful to
    differentiate between containers.

  - the value stored in /sys/fs/nfs/client/net/identifier.  This virtual
    file is local to the network namespace that it is accessed in and so
    can provided uniqueness between containers when the hostname is
    uniform among containers.

    This value is empty on namespace creation. though there is a pending
    kernel patch which initialises it to a random value.  This would mean
    that containers which currently rely simply on a unique hostname to
    create unique NFS identifiers, will now *not* have a stable
    identifier from one instance to the next.  This may be a regression,
    so the patch should probably be reconsidered.

    If the value is to be set, that should be done before the first
    mount (much as the hostname is copied before the first mount).
    If the container system has access to some sort of per-container
    identity, then a command like
       uuidgen --sha1 --namespace @url -N "nfs:$CONTAINER_ID" \
                 > /sys/fs/nfs/client/net/identifier 

    might be suitable.  If the containre system provides no stable name,
    but does have stable storage, then something like

      [ -s /etc/nfsv4-uuid ] || uuidgen > /etc/nfsv4-uuid && 
          cat /etc/nfsv4-uuid > /sys/fs/nfs/client/net/identifier 

    would suffice.

    If a container has neither a stable name nor stable (local) storage,
    then it is not possible to provide a stable identifer, so providing
    a random one to ensure uniqueness would be best

        uuidgen > /sys/fs/nfs/client/net/identifier


Comments, revisions, etc most welcome.

Thanks
NeilBrown



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

* Re: [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers
  2022-02-16 20:44                                 ` Benjamin Coddington
  2022-02-16 23:16                                   ` NeilBrown
@ 2022-02-17 14:43                                   ` Chuck Lever III
  1 sibling, 0 replies; 36+ messages in thread
From: Chuck Lever III @ 2022-02-17 14:43 UTC (permalink / raw)
  To: Benjamin Coddington; +Cc: Neil Brown, Steve Dickson, Linux NFS Mailing List



> On Feb 16, 2022, at 3:44 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> On 16 Feb 2022, at 14:35, Chuck Lever III wrote:
> 
>> On Feb 16, 2022, at 2:01 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
>>> Coming back to this.. so it seems at least part of our disagreement about
>>> the name had to do with a misunderstanding of what the tool did.
>> 
>> I understand what your implementation does. I don't
>> understand why it works that way.
>> 
>> The disagreement is that I feel like you're trying to
>> abstract the operation of this little tool in ways that
>> no-one asked for. It's purpose is very narrow and
>> completely related to the NFSv4 client ID.
>> 
>> 
>>> Just to finally clear the air about it: the tool does not write to sysfs, it
>>> doesn't try to modify the client in any way.  I'm going to leave it up to
>>> the distros.
>> 
>> Seems to me that the distros care only about where the
>> persistence file is located. I can't see a problem with
>> Neil's suggestion that the tool also write into sysfs.
>> The location of the sysfs API is invariant; there should
>> be no need to configure it or expose it.
>> 
>> Can you give a reason why the tool should /not/ write
>> into sysfs?
> 
> So that there is a division of work.  Systemd-udevd handles the event and
> sets the attribute, not the tool.  Systemd-udevd is already set up to have
> the appropriate selinux contexts and rights to make these changes.
> Systemd-udevd's logging can clearly show the action and result instead
> of it being hidden away inside this tool.  Systemd-udevd doesn't expect
> programs to make the changes, its designed around the idea that programs
> provide information and it makes the changes.
> 
> You can see how many issues we got into by imagining what would happen if
> users ran this tool.  If the tool doesn't modify the client, that's one less
> worry we have upstream, its the distro's problem.
> 
> If the tool writes to sysfs, then someone executing it manually can change
> the client's id with a live mount.  That's bad.  That's less likely to
> happen if its all tucked away in a udev rule.  I know the counter-arguement
> "you can write to sysfs yourself", but I still think its better this way.
> 
> There's increased flexibility and utility - if an expert sysadmin wants to
> distinguish groups of clients, they can customize their udev rule to add
> custom strings to the uniquifier using the many additional points of data
> available in udev rules.

I was not aware of the SELinux requirement and the particular
interactions the tool would have with systemd. Thanks for
explaining!

I think a "set-like" tool would work fine. We have plenty of
those. But I now understand why you designed the tool this way,
so I don't mind either way. The tool's man page, if we proceed
in that direction, should summarize the systemd expectations
(along with a SEE ALSO citation).


--
Chuck Lever




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

* [PATCH] nfs.man: document requirements for NFS mounts in a container
  2022-02-16 23:16                                   ` NeilBrown
@ 2022-03-01  3:43                                     ` NeilBrown
  2022-03-01 15:08                                       ` Chuck Lever III
  0 siblings, 1 reply; 36+ messages in thread
From: NeilBrown @ 2022-03-01  3:43 UTC (permalink / raw)
  To: Benjamin Coddington, Steve Dickson
  Cc: Chuck Lever III, Linux NFS Mailing List


When mounting NFS filesystems in a network namespace using v4, some care
must be taken to ensure a unique and stable client identity.
Add documentation explaining the requirements for container managers.

Signed-off-by: NeilBrown <neilb@suse.de>
---

NOTE I originally suggested using uuidgen to generate a uuid from a
container name.  I've changed it to use the name as-is because I cannot
see a justification for using a uuid - though I think that was suggested
somewhere in the discussion.
If someone would like to provide that justification, I'm happy to
include it in the document.

Thanks,
NeilBrown


 utils/mount/nfs.man | 63 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/utils/mount/nfs.man b/utils/mount/nfs.man
index d9f34df36b42..4ab76fb2df91 100644
--- a/utils/mount/nfs.man
+++ b/utils/mount/nfs.man
@@ -1844,6 +1844,69 @@ export pathname, but not both, during a remount.  For example,
 merges the mount option
 .B ro
 with the mount options already saved on disk for the NFS server mounted at /mnt.
+.SH "NFS IN A CONTAINER"
+When NFS is used to mount filesystems in a container, and specifically
+in a separate network name-space, these mounts are treated as quite
+separate from any mounts in a different container or not in a
+container (i.e. in a different network name-space).
+.P
+In the NFSv4 protocol, each client must have a unique identifier.
+This is used by the server to determine when a client has restarted,
+allowing any state from a previous instance can be discarded.  So any two
+concurrent clients that might access the same server MUST have
+different identifiers, and any two consecutive instances of the same
+client SHOULD have the same identifier.
+.P
+Linux constructs the identifier (referred to as 
+.B co_ownerid
+in the NFS specifications) from various pieces of information, three of
+which can be controlled by the sysadmin:
+.TP
+Hostname
+The hostname can be different in different containers if they
+have different "UTS" name-spaces.  If the container system ensures
+each container sees a unique host name, then this is
+sufficient for a correctly functioning NFS identifier.
+The host name is copied when the first NFS filesystem is mounted in
+a given network name-space.  Any subsequent change in the apparent
+hostname will not change the NFSv4 identifier.
+.TP
+.B nfs.nfs4_unique_id
+This module parameter is the same for all containers on a given host
+so it is not useful to differentiate between containers.
+.TP
+.B /sys/fs/nfs/client/net/identifier
+This virtual file (available since Linux 5.3) is local to the network
+name-space in which it is accessed and so can provided uniqueness between
+containers when the hostname is uniform among containers.
+.RS
+.PP
+This value is empty on name-space creation.
+If the value is to be set, that should be done before the first
+mount (much as the hostname is copied before the first mount).
+If the container system has access to some sort of per-container
+identity, then a command like
+.RS 4
+echo "$CONTAINER_IDENTITY" \\
+.br
+   > /sys/fs/nfs/client/net/identifier 
+.RE
+might be suitable.  If the container system provides no stable name,
+but does have stable storage, then something like
+.RS 4
+[ -s /etc/nfsv4-uuid ] || uuidgen > /etc/nfsv4-uuid && 
+.br
+cat /etc/nfsv4-uuid > /sys/fs/nfs/client/net/identifier 
+.RE
+would suffice.
+.PP
+If a container has neither a stable name nor stable (local) storage,
+then it is not possible to provide a stable identifier, so providing
+a random one to ensure uniqueness would be best
+.RS 4
+uuidgen > /sys/fs/nfs/client/net/identifier
+.RE
+.RE
 .SH FILES
 .TP 1.5i
 .I /etc/fstab
-- 
2.35.1


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

* Re: [PATCH] nfs.man: document requirements for NFS mounts in a container
  2022-03-01  3:43                                     ` [PATCH] nfs.man: document requirements for NFS mounts in a container NeilBrown
@ 2022-03-01 15:08                                       ` Chuck Lever III
  2022-03-01 15:16                                         ` Chuck Lever III
  2022-03-03  3:26                                         ` NeilBrown
  0 siblings, 2 replies; 36+ messages in thread
From: Chuck Lever III @ 2022-03-01 15:08 UTC (permalink / raw)
  To: Neil Brown; +Cc: Benjamin Coddington, Steve Dickson, Linux NFS Mailing List



> On Feb 28, 2022, at 10:43 PM, NeilBrown <neilb@suse.de> wrote:
> 
> 
> When mounting NFS filesystems in a network namespace using v4, some care
> must be taken to ensure a unique and stable client identity.
> Add documentation explaining the requirements for container managers.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> 
> NOTE I originally suggested using uuidgen to generate a uuid from a
> container name.  I've changed it to use the name as-is because I cannot
> see a justification for using a uuid - though I think that was suggested
> somewhere in the discussion.
> If someone would like to provide that justification, I'm happy to
> include it in the document.
> 
> Thanks,
> NeilBrown
> 
> 
> utils/mount/nfs.man | 63 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 63 insertions(+)
> 
> diff --git a/utils/mount/nfs.man b/utils/mount/nfs.man
> index d9f34df36b42..4ab76fb2df91 100644
> --- a/utils/mount/nfs.man
> +++ b/utils/mount/nfs.man
> @@ -1844,6 +1844,69 @@ export pathname, but not both, during a remount.  For example,
> merges the mount option
> .B ro
> with the mount options already saved on disk for the NFS server mounted at /mnt.
> +.SH "NFS IN A CONTAINER"

To be clear, this explanation is about the operation of the
Linux NFS client in a container environment. The server has
different needs that do not appear to be addressed here.
The section title should be clear that this information
pertains to the client.


> +When NFS is used to mount filesystems in a container, and specifically
> +in a separate network name-space, these mounts are treated as quite
> +separate from any mounts in a different container or not in a
> +container (i.e. in a different network name-space).

It might be helpful to provide an introductory explanation of
how mount works in general in a namespaced environment. There
might already be one somewhere. The above text needs to be
clear that we are not discussing the mount namespace.


> +.P
> +In the NFSv4 protocol, each client must have a unique identifier.

... each client must have a persistent and globally unique
identifier.


> +This is used by the server to determine when a client has restarted,
> +allowing any state from a previous instance can be discarded.

Lots of passive voice here :-)

The server associates a lease with the client's identifier
and a boot instance verifier. The server attaches all of
the client's file open and lock state to that lease, which
it preserves until the client's boot verifier changes.


> So any two
> +concurrent clients that might access the same server MUST have
> +different identifiers, and any two consecutive instances of the same
> +client SHOULD have the same identifier.

Capitalized MUST and SHOULD have specific meanings in IETF
standards that are probably not obvious to average readers
of man pages. To average readers, this looks like shouting.
Can you use something a little friendlier?


> +.P
> +Linux constructs the identifier (referred to as 
> +.B co_ownerid
> +in the NFS specifications) from various pieces of information, three of
> +which can be controlled by the sysadmin:
> +.TP
> +Hostname
> +The hostname can be different in different containers if they
> +have different "UTS" name-spaces.  If the container system ensures
> +each container sees a unique host name,

Actually, it turns out that is a pretty big "if". We've
found that our cloud customers are not careful about
setting unique hostnames. That's exactly why the whole
uniquifier thing is so critical!


> then this is
> +sufficient for a correctly functioning NFS identifier.
> +The host name is copied when the first NFS filesystem is mounted in
> +a given network name-space.  Any subsequent change in the apparent
> +hostname will not change the NFSv4 identifier.

The purpose of using a uuid here is that, given its
definition in RFC 4122, it has very strong global
uniqueness guarantees.

Using a UUID makes hostname uniqueness irrelevant.

Again, I think our goal should be hiding all of this
detail from administrators, because once we get this
mechanism working correctly, there is absolutely no
need for administrators to bother with it.


The remaining part of this text probably should be
part of the man page for Ben's tool, or whatever is
coming next.


> +.TP
> +.B nfs.nfs4_unique_id
> +This module parameter is the same for all containers on a given host
> +so it is not useful to differentiate between containers.
> +.TP
> +.B /sys/fs/nfs/client/net/identifier
> +This virtual file (available since Linux 5.3) is local to the network
> +name-space in which it is accessed and so can provided uniqueness between
> +containers when the hostname is uniform among containers.
> +.RS
> +.PP
> +This value is empty on name-space creation.
> +If the value is to be set, that should be done before the first
> +mount (much as the hostname is copied before the first mount).
> +If the container system has access to some sort of per-container
> +identity, then a command like
> +.RS 4
> +echo "$CONTAINER_IDENTITY" \\
> +.br
> +   > /sys/fs/nfs/client/net/identifier 
> +.RE
> +might be suitable.  If the container system provides no stable name,
> +but does have stable storage, then something like
> +.RS 4
> +[ -s /etc/nfsv4-uuid ] || uuidgen > /etc/nfsv4-uuid && 
> +.br
> +cat /etc/nfsv4-uuid > /sys/fs/nfs/client/net/identifier 
> +.RE
> +would suffice.
> +.PP
> +If a container has neither a stable name nor stable (local) storage,
> +then it is not possible to provide a stable identifier, so providing
> +a random one to ensure uniqueness would be best
> +.RS 4
> +uuidgen > /sys/fs/nfs/client/net/identifier
> +.RE
> +.RE
> .SH FILES
> .TP 1.5i
> .I /etc/fstab
> -- 
> 2.35.1
> 

--
Chuck Lever




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

* Re: [PATCH] nfs.man: document requirements for NFS mounts in a container
  2022-03-01 15:08                                       ` Chuck Lever III
@ 2022-03-01 15:16                                         ` Chuck Lever III
  2022-03-03  3:26                                         ` NeilBrown
  1 sibling, 0 replies; 36+ messages in thread
From: Chuck Lever III @ 2022-03-01 15:16 UTC (permalink / raw)
  To: Neil Brown; +Cc: Benjamin Coddington, Steve Dickson, Linux NFS Mailing List



> On Mar 1, 2022, at 10:08 AM, Chuck Lever III <chuck.lever@oracle.com> wrote:
> 
> 
> 
>> On Feb 28, 2022, at 10:43 PM, NeilBrown <neilb@suse.de> wrote:
>> 
>> 
>> When mounting NFS filesystems in a network namespace using v4, some care
>> must be taken to ensure a unique and stable client identity.
>> Add documentation explaining the requirements for container managers.
>> 
>> Signed-off-by: NeilBrown <neilb@suse.de>
>> ---
>> 
>> NOTE I originally suggested using uuidgen to generate a uuid from a
>> container name.  I've changed it to use the name as-is because I cannot
>> see a justification for using a uuid - though I think that was suggested
>> somewhere in the discussion.
>> If someone would like to provide that justification, I'm happy to
>> include it in the document.
>> 
>> Thanks,
>> NeilBrown
>> 
>> 
>> utils/mount/nfs.man | 63 +++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 63 insertions(+)
>> 
>> diff --git a/utils/mount/nfs.man b/utils/mount/nfs.man
>> index d9f34df36b42..4ab76fb2df91 100644
>> --- a/utils/mount/nfs.man
>> +++ b/utils/mount/nfs.man
>> @@ -1844,6 +1844,69 @@ export pathname, but not both, during a remount.  For example,
>> merges the mount option
>> .B ro
>> with the mount options already saved on disk for the NFS server mounted at /mnt.
>> +.SH "NFS IN A CONTAINER"
> 
> To be clear, this explanation is about the operation of the
> Linux NFS client in a container environment. The server has
> different needs that do not appear to be addressed here.
> The section title should be clear that this information
> pertains to the client.
> 
> 
>> +When NFS is used to mount filesystems in a container, and specifically
>> +in a separate network name-space, these mounts are treated as quite
>> +separate from any mounts in a different container or not in a
>> +container (i.e. in a different network name-space).
> 
> It might be helpful to provide an introductory explanation of
> how mount works in general in a namespaced environment. There
> might already be one somewhere. The above text needs to be
> clear that we are not discussing the mount namespace.
> 
> 
>> +.P
>> +In the NFSv4 protocol, each client must have a unique identifier.
> 
> ... each client must have a persistent and globally unique
> identifier.
> 
> 
>> +This is used by the server to determine when a client has restarted,
>> +allowing any state from a previous instance can be discarded.
> 
> Lots of passive voice here :-)
> 
> The server associates a lease with the client's identifier
> and a boot instance verifier. The server attaches all of
> the client's file open and lock state to that lease, which
> it preserves until the client's boot verifier changes.

Oh and also, this might be a good opportunity to explain
how the server requires that the client use not only the
same identifier string, but also the same principal to
reattach itself to its open and lock state after a server
reboot.

This is why the Linux NFS client attempts to use Kerberos
whenever it can for this purpose. Using AUTH_SYS invites
other another client that happens to have the same identifier
to trigger the server to purge that client's open and lock
state.


>> So any two
>> +concurrent clients that might access the same server MUST have
>> +different identifiers, and any two consecutive instances of the same
>> +client SHOULD have the same identifier.
> 
> Capitalized MUST and SHOULD have specific meanings in IETF
> standards that are probably not obvious to average readers
> of man pages. To average readers, this looks like shouting.
> Can you use something a little friendlier?
> 
> 
>> +.P
>> +Linux constructs the identifier (referred to as 
>> +.B co_ownerid
>> +in the NFS specifications) from various pieces of information, three of
>> +which can be controlled by the sysadmin:
>> +.TP
>> +Hostname
>> +The hostname can be different in different containers if they
>> +have different "UTS" name-spaces.  If the container system ensures
>> +each container sees a unique host name,
> 
> Actually, it turns out that is a pretty big "if". We've
> found that our cloud customers are not careful about
> setting unique hostnames. That's exactly why the whole
> uniquifier thing is so critical!
> 
> 
>> then this is
>> +sufficient for a correctly functioning NFS identifier.
>> +The host name is copied when the first NFS filesystem is mounted in
>> +a given network name-space.  Any subsequent change in the apparent
>> +hostname will not change the NFSv4 identifier.
> 
> The purpose of using a uuid here is that, given its
> definition in RFC 4122, it has very strong global
> uniqueness guarantees.
> 
> Using a UUID makes hostname uniqueness irrelevant.
> 
> Again, I think our goal should be hiding all of this
> detail from administrators, because once we get this
> mechanism working correctly, there is absolutely no
> need for administrators to bother with it.
> 
> 
> The remaining part of this text probably should be
> part of the man page for Ben's tool, or whatever is
> coming next.
> 
> 
>> +.TP
>> +.B nfs.nfs4_unique_id
>> +This module parameter is the same for all containers on a given host
>> +so it is not useful to differentiate between containers.
>> +.TP
>> +.B /sys/fs/nfs/client/net/identifier
>> +This virtual file (available since Linux 5.3) is local to the network
>> +name-space in which it is accessed and so can provided uniqueness between
>> +containers when the hostname is uniform among containers.
>> +.RS
>> +.PP
>> +This value is empty on name-space creation.
>> +If the value is to be set, that should be done before the first
>> +mount (much as the hostname is copied before the first mount).
>> +If the container system has access to some sort of per-container
>> +identity, then a command like
>> +.RS 4
>> +echo "$CONTAINER_IDENTITY" \\
>> +.br
>> +   > /sys/fs/nfs/client/net/identifier 
>> +.RE
>> +might be suitable.  If the container system provides no stable name,
>> +but does have stable storage, then something like
>> +.RS 4
>> +[ -s /etc/nfsv4-uuid ] || uuidgen > /etc/nfsv4-uuid && 
>> +.br
>> +cat /etc/nfsv4-uuid > /sys/fs/nfs/client/net/identifier 
>> +.RE
>> +would suffice.
>> +.PP
>> +If a container has neither a stable name nor stable (local) storage,
>> +then it is not possible to provide a stable identifier, so providing
>> +a random one to ensure uniqueness would be best
>> +.RS 4
>> +uuidgen > /sys/fs/nfs/client/net/identifier
>> +.RE
>> +.RE
>> .SH FILES
>> .TP 1.5i
>> .I /etc/fstab
>> -- 
>> 2.35.1
>> 
> 
> --
> Chuck Lever

--
Chuck Lever




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

* Re: [PATCH] nfs.man: document requirements for NFS mounts in a container
  2022-03-01 15:08                                       ` Chuck Lever III
  2022-03-01 15:16                                         ` Chuck Lever III
@ 2022-03-03  3:26                                         ` NeilBrown
  2022-03-03 14:37                                           ` Trond Myklebust
  2022-03-03 15:53                                           ` Chuck Lever III
  1 sibling, 2 replies; 36+ messages in thread
From: NeilBrown @ 2022-03-03  3:26 UTC (permalink / raw)
  To: Chuck Lever III
  Cc: Benjamin Coddington, Steve Dickson, Linux NFS Mailing List

On Wed, 02 Mar 2022, Chuck Lever III wrote:
> 
> > On Feb 28, 2022, at 10:43 PM, NeilBrown <neilb@suse.de> wrote:
> > 
> > 
> > When mounting NFS filesystems in a network namespace using v4, some care
> > must be taken to ensure a unique and stable client identity.
> > Add documentation explaining the requirements for container managers.
> > 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> > 
> > NOTE I originally suggested using uuidgen to generate a uuid from a
> > container name.  I've changed it to use the name as-is because I cannot
> > see a justification for using a uuid - though I think that was suggested
> > somewhere in the discussion.
> > If someone would like to provide that justification, I'm happy to
> > include it in the document.
> > 
> > Thanks,
> > NeilBrown
> > 
> > 
> > utils/mount/nfs.man | 63 +++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 63 insertions(+)
> > 
> > diff --git a/utils/mount/nfs.man b/utils/mount/nfs.man
> > index d9f34df36b42..4ab76fb2df91 100644
> > --- a/utils/mount/nfs.man
> > +++ b/utils/mount/nfs.man
> > @@ -1844,6 +1844,69 @@ export pathname, but not both, during a remount.  For example,
> > merges the mount option
> > .B ro
> > with the mount options already saved on disk for the NFS server mounted at /mnt.
> > +.SH "NFS IN A CONTAINER"
> 
> To be clear, this explanation is about the operation of the
> Linux NFS client in a container environment. The server has
> different needs that do not appear to be addressed here.
> The section title should be clear that this information
> pertains to the client.

The whole man page is only about the client, but I agree that clarity is
best.  I've changed the section heading to

    NFS MOUNTS IN A CONTAINER

> 
> 
> > +When NFS is used to mount filesystems in a container, and specifically
> > +in a separate network name-space, these mounts are treated as quite
> > +separate from any mounts in a different container or not in a
> > +container (i.e. in a different network name-space).
> 
> It might be helpful to provide an introductory explanation of
> how mount works in general in a namespaced environment. There
> might already be one somewhere. The above text needs to be
> clear that we are not discussing the mount namespace.

Mount namespaces are completely irrelevant for this discussion.
This is "specifically" about "network name-spaces" a I wrote.
Do I need to say more than that?
Maybe a sentence "Mount namespaces are not relevant" ??

> 
> 
> > +.P
> > +In the NFSv4 protocol, each client must have a unique identifier.
> 
> ... each client must have a persistent and globally unique
> identifier.

I dispute "globally".  The id only needs to be unique among clients of
a given NFS server.
I also dispute "persistent" in the context of "must".
Unless I'm missing something, a lack of persistence only matters when a
client stops while still holding state, and then restarts within the
lease period.  It will then be prevented from establishing conflicting
state until the lease period ends.  So persistence is good, but is not a
hard requirement.  Uniqueness IS a hard requirement among concurrent
clients of the one server.

> 
> 
> > +This is used by the server to determine when a client has restarted,
> > +allowing any state from a previous instance can be discarded.
> 
> Lots of passive voice here :-)
> 
> The server associates a lease with the client's identifier
> and a boot instance verifier. The server attaches all of
> the client's file open and lock state to that lease, which
> it preserves until the client's boot verifier changes.

I guess I"m a passivist.  If we are going for that level of detail we
need to mention lease expiry too.

 .... it preserves until the lease time passes without any renewal from
      the client, or the client's boot verifier changes.

In another email you add:

> Oh and also, this might be a good opportunity to explain
> how the server requires that the client use not only the
> same identifier string, but also the same principal to
> reattach itself to its open and lock state after a server
> reboot.
> 
> This is why the Linux NFS client attempts to use Kerberos
> whenever it can for this purpose. Using AUTH_SYS invites
> other another client that happens to have the same identifier
> to trigger the server to purge that client's open and lock
> state.

How relevant is this to the context of a container?
How much extra context would be need to add to make the mention of
credentials coherent?
Maybe we should add another section about credentials, and add it just
before this one??

> 
> 
> > So any two
> > +concurrent clients that might access the same server MUST have
> > +different identifiers, and any two consecutive instances of the same
> > +client SHOULD have the same identifier.
> 
> Capitalized MUST and SHOULD have specific meanings in IETF
> standards that are probably not obvious to average readers
> of man pages. To average readers, this looks like shouting.
> Can you use something a little friendlier?
> 

How about:

   Any two concurrent clients that might access the same server must
   have different identifiers for correct operation, and any two
   consecutive instances of the same client should have the same
   identifier for optimal handling of an unclean restart.

> 
> > +.P
> > +Linux constructs the identifier (referred to as 
> > +.B co_ownerid
> > +in the NFS specifications) from various pieces of information, three of
> > +which can be controlled by the sysadmin:
> > +.TP
> > +Hostname
> > +The hostname can be different in different containers if they
> > +have different "UTS" name-spaces.  If the container system ensures
> > +each container sees a unique host name,
> 
> Actually, it turns out that is a pretty big "if". We've
> found that our cloud customers are not careful about
> setting unique hostnames. That's exactly why the whole
> uniquifier thing is so critical!

:-)  I guess we keep it as "if" though, not "IF" ....

> 
> 
> > then this is
> > +sufficient for a correctly functioning NFS identifier.
> > +The host name is copied when the first NFS filesystem is mounted in
> > +a given network name-space.  Any subsequent change in the apparent
> > +hostname will not change the NFSv4 identifier.
> 
> The purpose of using a uuid here is that, given its
> definition in RFC 4122, it has very strong global
> uniqueness guarantees.

A uuid generated from a given string (uuidgen -N $name ...) has the same
uniqueness as the $name.  Turning it into a uuid doesn't improve the
uniqueness.  It just provides a standard format and obfuscates the
original.  Neither of those seem necessary here.
I think Ben is considering using /etc/mechine-id.  Creating a uuid from
that does make it any better.

> 
> Using a UUID makes hostname uniqueness irrelevant.

Only if the UUID is created appropriately.  If, for example, it is
created with -N from some name that is unique on the host, then it needs
to be combined with the hostname to get sufficient uniqueness.

> 
> Again, I think our goal should be hiding all of this
> detail from administrators, because once we get this
> mechanism working correctly, there is absolutely no
> need for administrators to bother with it.

Except when things break.  Then admins will appreciate having the
details so they can track down the breakage.  My desktop didn't boot
this morning.  Systemd didn't tell me why it was hanging though I
eventually discovered that it was "wicked.service" that wasn't reporting
success.  So I'm currently very focused on the need to provide clarity
to sysadmins, even of "irrelevant" details.

But this documentation isn't just for sysadmins, it is for container
developers too, so they can find out how to make their container work
with NFS.

> 
> 
> The remaining part of this text probably should be
> part of the man page for Ben's tool, or whatever is
> coming next.

My position is that there is no need for any tool.  The total amount of
code needed is a couple of lines as presented in the text below.  Why
provide a wrapper just for that?
We *cannot* automatically decide how to find a name or where to store a
generated uuid, so there is no added value that a tool could provide.

We cannot unilaterally fix container systems.  We can only tell people
who build these systems of the requirements for NFS.

Thanks,
NeilBrown

> 
> 
> > +.TP
> > +.B nfs.nfs4_unique_id
> > +This module parameter is the same for all containers on a given host
> > +so it is not useful to differentiate between containers.
> > +.TP
> > +.B /sys/fs/nfs/client/net/identifier
> > +This virtual file (available since Linux 5.3) is local to the network
> > +name-space in which it is accessed and so can provided uniqueness between
> > +containers when the hostname is uniform among containers.
> > +.RS
> > +.PP
> > +This value is empty on name-space creation.
> > +If the value is to be set, that should be done before the first
> > +mount (much as the hostname is copied before the first mount).
> > +If the container system has access to some sort of per-container
> > +identity, then a command like
> > +.RS 4
> > +echo "$CONTAINER_IDENTITY" \\
> > +.br
> > +   > /sys/fs/nfs/client/net/identifier 
> > +.RE
> > +might be suitable.  If the container system provides no stable name,
> > +but does have stable storage, then something like
> > +.RS 4
> > +[ -s /etc/nfsv4-uuid ] || uuidgen > /etc/nfsv4-uuid && 
> > +.br
> > +cat /etc/nfsv4-uuid > /sys/fs/nfs/client/net/identifier 
> > +.RE
> > +would suffice.
> > +.PP
> > +If a container has neither a stable name nor stable (local) storage,
> > +then it is not possible to provide a stable identifier, so providing
> > +a random one to ensure uniqueness would be best
> > +.RS 4
> > +uuidgen > /sys/fs/nfs/client/net/identifier
> > +.RE
> > +.RE
> > .SH FILES
> > .TP 1.5i
> > .I /etc/fstab
> > -- 
> > 2.35.1
> > 
> 
> --
> Chuck Lever
> 
> 
> 
> 

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

* Re: [PATCH] nfs.man: document requirements for NFS mounts in a container
  2022-03-03  3:26                                         ` NeilBrown
@ 2022-03-03 14:37                                           ` Trond Myklebust
  2022-03-04  1:13                                             ` NeilBrown
  2022-03-03 15:53                                           ` Chuck Lever III
  1 sibling, 1 reply; 36+ messages in thread
From: Trond Myklebust @ 2022-03-03 14:37 UTC (permalink / raw)
  To: neilb, chuck.lever; +Cc: linux-nfs, bcodding, SteveD

On Thu, 2022-03-03 at 14:26 +1100, NeilBrown wrote:
> On Wed, 02 Mar 2022, Chuck Lever III wrote:
> 
> 
> > 
> > 
> > The remaining part of this text probably should be
> > part of the man page for Ben's tool, or whatever is
> > coming next.
> 
> My position is that there is no need for any tool.  The total amount
> of
> code needed is a couple of lines as presented in the text below.  Why
> provide a wrapper just for that?
> We *cannot* automatically decide how to find a name or where to store
> a
> generated uuid, so there is no added value that a tool could provide.
> 
> We cannot unilaterally fix container systems.  We can only tell
> people
> who build these systems of the requirements for NFS.
> 

I disagree with this position. The value of having a standard tool is
that it also creates a standard for how and where the uniquifier is
generated and persisted.

Otherwise you have to deal with the fact that you may have a systemd
script that persists something in one file, a Dockerfile recipe that
generates something at container build time, and then a home-made
script that looks for something in a different location. If you're
trying to debug why your containers are all generating the same
uniquifier, then that can be a problem.

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



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

* Re: [PATCH] nfs.man: document requirements for NFS mounts in a container
  2022-03-03  3:26                                         ` NeilBrown
  2022-03-03 14:37                                           ` Trond Myklebust
@ 2022-03-03 15:53                                           ` Chuck Lever III
  2022-03-08  0:44                                             ` NeilBrown
  1 sibling, 1 reply; 36+ messages in thread
From: Chuck Lever III @ 2022-03-03 15:53 UTC (permalink / raw)
  To: Neil Brown; +Cc: Benjamin Coddington, Steve Dickson, Linux NFS Mailing List



> On Mar 2, 2022, at 10:26 PM, NeilBrown <neilb@suse.de> wrote:
> 
> On Wed, 02 Mar 2022, Chuck Lever III wrote:
>> 
>>> On Feb 28, 2022, at 10:43 PM, NeilBrown <neilb@suse.de> wrote:
>>> 
>>> 
>>> When mounting NFS filesystems in a network namespace using v4, some care
>>> must be taken to ensure a unique and stable client identity.
>>> Add documentation explaining the requirements for container managers.
>>> 
>>> Signed-off-by: NeilBrown <neilb@suse.de>
>>> ---
>>> 
>>> NOTE I originally suggested using uuidgen to generate a uuid from a
>>> container name.  I've changed it to use the name as-is because I cannot
>>> see a justification for using a uuid - though I think that was suggested
>>> somewhere in the discussion.
>>> If someone would like to provide that justification, I'm happy to
>>> include it in the document.
>>> 
>>> Thanks,
>>> NeilBrown
>>> 
>>> 
>>> utils/mount/nfs.man | 63 +++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 63 insertions(+)
>>> 
>>> diff --git a/utils/mount/nfs.man b/utils/mount/nfs.man
>>> index d9f34df36b42..4ab76fb2df91 100644
>>> --- a/utils/mount/nfs.man
>>> +++ b/utils/mount/nfs.man
>>> @@ -1844,6 +1844,69 @@ export pathname, but not both, during a remount.  For example,
>>> merges the mount option
>>> .B ro
>>> with the mount options already saved on disk for the NFS server mounted at /mnt.
>>> +.SH "NFS IN A CONTAINER"
>> 
>> To be clear, this explanation is about the operation of the
>> Linux NFS client in a container environment. The server has
>> different needs that do not appear to be addressed here.
>> The section title should be clear that this information
>> pertains to the client.
> 
> The whole man page is only about the client, but I agree that clarity is
> best.  I've changed the section heading to
> 
>    NFS MOUNTS IN A CONTAINER

Actually I've rethought this.

I think the central point of this text needs to be how
the client uniquifier works. It needs to work this way
for all client deployments, whether containerized or not.

There are some important variations that can be called out:

1. Containers (or virtualized clients)

When multiple NFS clients run on the same physical host.


2. NAT

NAT hasn't been mentioned before, but it is a common
deployment scenario where multiple clients can have the
same hostname and local IP address (a private address such
as 192.168.0.55) but the clients all access the same NFS
server.


3. NFSROOT

Where the uniquifier has to be provided on the boot
command line and can't be persisted locally on the
client.


>>> +When NFS is used to mount filesystems in a container, and specifically
>>> +in a separate network name-space, these mounts are treated as quite
>>> +separate from any mounts in a different container or not in a
>>> +container (i.e. in a different network name-space).
>> 
>> It might be helpful to provide an introductory explanation of
>> how mount works in general in a namespaced environment. There
>> might already be one somewhere. The above text needs to be
>> clear that we are not discussing the mount namespace.
> 
> Mount namespaces are completely irrelevant for this discussion.

Agreed, mount namespaces are irrelevant to this discussion.


> This is "specifically" about "network name-spaces" a I wrote.
> Do I need to say more than that?
> Maybe a sentence "Mount namespaces are not relevant" ??

I would say by way of introduction that "An NFS mount,
unlike a local filesystem mount, exists in both a mount
namespace and a network namespace", then continue with
"this is specifically about network namespaces."


>>> +.P
>>> +In the NFSv4 protocol, each client must have a unique identifier.
>> 
>> ... each client must have a persistent and globally unique
>> identifier.
> 
> I dispute "globally".  The id only needs to be unique among clients of
> a given NFS server.

Practically speaking, that is correct in a limited sense.

However there is no limit on the use of a laptop (ie, a
physically portable client) to access any NFS server that
is local to it. We have no control over how clients are
physically deployed.

A public NFS server is going to see a vast cohort of
clients, all of which need to have unique identifiers.
There's no interaction amongst the clients themselves to
determine whether there are identifier collisions.

Global uniqueness therefore is a requirement to make
that work seamlessly.


> I also dispute "persistent" in the context of "must".
> Unless I'm missing something, a lack of persistence only matters when a
> client stops while still holding state, and then restarts within the
> lease period.  It will then be prevented from establishing conflicting
> state until the lease period ends.

The client's identifier needs to be persistent so that:

1. If the server reboots, it can recognize when clients
   are re-establishing their lock and open state versus
   an unfamiliar creating lock and open state that might
   involve files that an existing client has open.

2. If the client reboots, the server is able to tie the
   rebooted client to an existing lease so that the lease
   and all of the client's previous lock and open state
   are properly purged.

There are moments when a client's identifier can change
without consequences. It's not entirely relevant to the
discussion to go into detail about when those moments
occur.


> So persistence is good, but is not a
> hard requirement.  Uniqueness IS a hard requirement among concurrent
> clients of the one server.

OK, then you were using the colloquial meaning of "must"
and "should", not the RFC 2119 meanings. Capitalizing
them was very confusing. Happily you provided a good
replacement below.


>>> +This is used by the server to determine when a client has restarted,
>>> +allowing any state from a previous instance can be discarded.
>> 
>> Lots of passive voice here :-)
>> 
>> The server associates a lease with the client's identifier
>> and a boot instance verifier. The server attaches all of
>> the client's file open and lock state to that lease, which
>> it preserves until the client's boot verifier changes.
> 
> I guess I"m a passivist.  If we are going for that level of detail we
> need to mention lease expiry too.
> 
> .... it preserves until the lease time passes without any renewal from
>      the client, or the client's boot verifier changes.

This is not entirely true. A server is not required to
dispense with a client's lease state when the lease
period is up. The Linux server does that today, but
soon it won't, instead waiting until a conflicting
open or lock request before it purges the lease of
an unreachable client.

The requirement is actually the converse: the server
must preserve a client's open and lock state during
the lease period. Outside of the lease period,
behavior is an implementation choice.


> In another email you add:
> 
>> Oh and also, this might be a good opportunity to explain
>> how the server requires that the client use not only the
>> same identifier string, but also the same principal to
>> reattach itself to its open and lock state after a server
>> reboot.
>> 
>> This is why the Linux NFS client attempts to use Kerberos
>> whenever it can for this purpose. Using AUTH_SYS invites
>> other another client that happens to have the same identifier
>> to trigger the server to purge that client's open and lock
>> state.
> 
> How relevant is this to the context of a container?

It's relevant because the client's identity consists
of the nfs_client_id4 string and the principal and
authentication flavor used to establish the lease.

If a container is manufactured by duplicating a
template that contains a keytab (and yes, I've seen
this done in practice) the principal and flavor
will be the same in the duplicated container, and
that will be a problem.

If the client is using only AUTH_SYS, as I mention
above, then the only distinction is the nfs_client_id4
string itself (since clients typically use UID 0 as
the principal in this case). There is really no
protection here -- and admins need to be warned
about this because their users will see open and
lock state disappearing for no reason because some
clients happen to choose the same nfs_client_id4 string
and are purging each others' lease.


> How much extra context would be need to add to make the mention of
> credentials coherent?
> Maybe we should add another section about credentials, and add it just
> before this one??

See above. The central discussion needs to be about
client identity IMO.


>>> So any two
>>> +concurrent clients that might access the same server MUST have
>>> +different identifiers, and any two consecutive instances of the same
>>> +client SHOULD have the same identifier.
>> 
>> Capitalized MUST and SHOULD have specific meanings in IETF
>> standards that are probably not obvious to average readers
>> of man pages. To average readers, this looks like shouting.
>> Can you use something a little friendlier?
>> 
> 
> How about:
> 
>   Any two concurrent clients that might access the same server must
>   have different identifiers for correct operation, and any two
>   consecutive instances of the same client should have the same
>   identifier for optimal handling of an unclean restart.

Nice.


>>> +.P
>>> +Linux constructs the identifier (referred to as 
>>> +.B co_ownerid
>>> +in the NFS specifications) from various pieces of information, three of
>>> +which can be controlled by the sysadmin:
>>> +.TP
>>> +Hostname
>>> +The hostname can be different in different containers if they
>>> +have different "UTS" name-spaces.  If the container system ensures
>>> +each container sees a unique host name,
>> 
>> Actually, it turns out that is a pretty big "if". We've
>> found that our cloud customers are not careful about
>> setting unique hostnames. That's exactly why the whole
>> uniquifier thing is so critical!
> 
> :-)  I guess we keep it as "if" though, not "IF" ....

And as mentioned above, it's not possible for them
to select hostnames and IP addresses (in particular
in the private IP address range) that are guaranteed
to be unique enough for a given server. The choices
are completely uncoordinated and have a considerable
risk of collision.


>>> then this is
>>> +sufficient for a correctly functioning NFS identifier.
>>> +The host name is copied when the first NFS filesystem is mounted in
>>> +a given network name-space.  Any subsequent change in the apparent
>>> +hostname will not change the NFSv4 identifier.
>> 
>> The purpose of using a uuid here is that, given its
>> definition in RFC 4122, it has very strong global
>> uniqueness guarantees.
> 
> A uuid generated from a given string (uuidgen -N $name ...) has the same
> uniqueness as the $name.  Turning it into a uuid doesn't improve the
> uniqueness.  It just provides a standard format and obfuscates the
> original.  Neither of those seem necessary here.

If indeed that's what's going on, then that's the
wrong approach. We need to have a globally unique
identifier here. If hashing a hostname has the risk
that the digest will be the same for two clients, then
that version of UUID is not usable for our purpose.

The non-globally unique versions of UUID are hardly
used any more because folks who use UUIDs generally
need a guarantee of global uniqueness without a
central coordinating authority. Time-based and
randomly generated UUIDs are typically the only
style that are used any more.


> I think Ben is considering using /etc/mechine-id.  Creating a uuid from
> that does make it any better.

I assume you mean "does /not/ make it any better".

As long as the machine-id is truly random
and is not, say, a hash of the hostname, then it
should work fine. The only downside of machine-id
is the man page's stipulation that the machine-id
shouldn't be publicly exposed on the network, which
is why it ought be at least hashed before it is used
as part of an nfs_client_id4.

So I guess there's a third requirement, aside from
persistence and global uniqueness: Information about
the sender (client in this case) is not inadvertently
leaked onto the open network.


>> Using a UUID makes hostname uniqueness irrelevant.
> 
> Only if the UUID is created appropriately.  If, for example, it is
> created with -N from some name that is unique on the host, then it needs
> to be combined with the hostname to get sufficient uniqueness.

Then that's the wrong version of UUID to use.


>> Again, I think our goal should be hiding all of this
>> detail from administrators, because once we get this
>> mechanism working correctly, there is absolutely no
>> need for administrators to bother with it.
> 
> Except when things break.  Then admins will appreciate having the
> details so they can track down the breakage.  My desktop didn't boot
> this morning.  Systemd didn't tell me why it was hanging though I
> eventually discovered that it was "wicked.service" that wasn't reporting
> success.  So I'm currently very focused on the need to provide clarity
> to sysadmins, even of "irrelevant" details.
> 
> But this documentation isn't just for sysadmins, it is for container
> developers too, so they can find out how to make their container work
> with NFS.

An alternative location for this detail would be under
Documentation/. A man page is possibly not the right
venue for a detailed explanation of protocol and
implementation; man pages usually are limited to quick
summaries of interfaces.


>> The remaining part of this text probably should be
>> part of the man page for Ben's tool, or whatever is
>> coming next.
> 
> My position is that there is no need for any tool.

Trond's earlier point about having to repeat this
functionality for other ways of mounting NFS
(eg Busybox) suggests we have to have a separate tool,
even though this is only a handful of lines of code.


> The total amount of
> code needed is a couple of lines as presented in the text below.  Why
> provide a wrapper just for that?
> We *cannot* automatically decide how to find a name or where to store a
> generated uuid, so there is no added value that a tool could provide.

I don't think anyone has yet demonstrated (or even
stated) this is impossible. Can you explain why you
believe this?


> We cannot unilaterally fix container systems.  We can only tell people
> who build these systems of the requirements for NFS.


--
Chuck Lever




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

* Re: [PATCH] nfs.man: document requirements for NFS mounts in a container
  2022-03-03 14:37                                           ` Trond Myklebust
@ 2022-03-04  1:13                                             ` NeilBrown
  2022-03-04 15:54                                               ` Steve Dickson
  0 siblings, 1 reply; 36+ messages in thread
From: NeilBrown @ 2022-03-04  1:13 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: chuck.lever, linux-nfs, bcodding, SteveD

On Fri, 04 Mar 2022, Trond Myklebust wrote:
> On Thu, 2022-03-03 at 14:26 +1100, NeilBrown wrote:
> > On Wed, 02 Mar 2022, Chuck Lever III wrote:
> > 
> > 
> > > 
> > > 
> > > The remaining part of this text probably should be
> > > part of the man page for Ben's tool, or whatever is
> > > coming next.
> > 
> > My position is that there is no need for any tool.  The total amount
> > of
> > code needed is a couple of lines as presented in the text below.  Why
> > provide a wrapper just for that?
> > We *cannot* automatically decide how to find a name or where to store
> > a
> > generated uuid, so there is no added value that a tool could provide.
> > 
> > We cannot unilaterally fix container systems.  We can only tell
> > people
> > who build these systems of the requirements for NFS.
> > 
> 
> I disagree with this position. The value of having a standard tool is
> that it also creates a standard for how and where the uniquifier is
> generated and persisted.
> 
> Otherwise you have to deal with the fact that you may have a systemd
> script that persists something in one file, a Dockerfile recipe that
> generates something at container build time, and then a home-made
> script that looks for something in a different location. If you're
> trying to debug why your containers are all generating the same
> uniquifier, then that can be a problem.

I don't see how a tool can provide any consistency.
Is there some standard that say how containers should be built, and
where tools can store persistent data?  If not, the tool needs to be
configured, and that is not importantly different from bash being
configured with a 1-line script to write out the identifier.

I'm not strongly against a tools, I just can't see the benefit.

Thanks,
NeilBrown

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

* Re: [PATCH] nfs.man: document requirements for NFS mounts in a container
  2022-03-04  1:13                                             ` NeilBrown
@ 2022-03-04 15:54                                               ` Steve Dickson
  2022-03-04 16:15                                                 ` Chuck Lever III
  2022-03-05  1:15                                                 ` NeilBrown
  0 siblings, 2 replies; 36+ messages in thread
From: Steve Dickson @ 2022-03-04 15:54 UTC (permalink / raw)
  To: NeilBrown, Trond Myklebust; +Cc: chuck.lever, linux-nfs, bcodding

Hey!

On 3/3/22 8:13 PM, NeilBrown wrote:
> On Fri, 04 Mar 2022, Trond Myklebust wrote:
>> On Thu, 2022-03-03 at 14:26 +1100, NeilBrown wrote:
>>> On Wed, 02 Mar 2022, Chuck Lever III wrote:
>>>
>>>
>>>>
>>>>
>>>> The remaining part of this text probably should be
>>>> part of the man page for Ben's tool, or whatever is
>>>> coming next.
>>>
>>> My position is that there is no need for any tool.  The total amount
>>> of
>>> code needed is a couple of lines as presented in the text below.  Why
>>> provide a wrapper just for that?
>>> We *cannot* automatically decide how to find a name or where to store
>>> a
>>> generated uuid, so there is no added value that a tool could provide.
>>>
>>> We cannot unilaterally fix container systems.  We can only tell
>>> people
>>> who build these systems of the requirements for NFS.
>>>
>>
>> I disagree with this position. The value of having a standard tool is
>> that it also creates a standard for how and where the uniquifier is
>> generated and persisted.
>>
>> Otherwise you have to deal with the fact that you may have a systemd
>> script that persists something in one file, a Dockerfile recipe that
>> generates something at container build time, and then a home-made
>> script that looks for something in a different location. If you're
>> trying to debug why your containers are all generating the same
>> uniquifier, then that can be a problem.
> 
> I don't see how a tool can provide any consistency.
> Is there some standard that say how containers should be built, and
> where tools can store persistent data?  If not, the tool needs to be
> configured, and that is not importantly different from bash being
> configured with a 1-line script to write out the identifier.
> 
> I'm not strongly against a tools, I just can't see the benefit.
I think I agree with this... Thinking about it... having a command that
tries to manipulate different containers in different ways just
seems like a recipe for disaster... I just don't see how a command would
ever get it right... Hell we can't agree on its command's name
much less what it will do. :-)

So I like idea of documenting when needs to happen in the
different types of containers... So I think the man page
is the way to go... and I think it is the safest way to go.

Chuck, if you would like tweak the verbiage... by all means.

Neil, will be a V2 for man page patch from this discussion
or should I just take the one you posted? If you do post
a V2, please start a new thread.

steved.


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

* Re: [PATCH] nfs.man: document requirements for NFS mounts in a container
  2022-03-04 15:54                                               ` Steve Dickson
@ 2022-03-04 16:15                                                 ` Chuck Lever III
  2022-03-04 16:54                                                   ` Steve Dickson
  2022-03-05  1:15                                                 ` NeilBrown
  1 sibling, 1 reply; 36+ messages in thread
From: Chuck Lever III @ 2022-03-04 16:15 UTC (permalink / raw)
  To: Steve Dickson
  Cc: Neil Brown, Trond Myklebust, Linux NFS Mailing List, bcodding



> On Mar 4, 2022, at 10:54 AM, Steve Dickson <steved@redhat.com> wrote:
> 
> Hey!
> 
> On 3/3/22 8:13 PM, NeilBrown wrote:
>> On Fri, 04 Mar 2022, Trond Myklebust wrote:
>>> On Thu, 2022-03-03 at 14:26 +1100, NeilBrown wrote:
>>>> On Wed, 02 Mar 2022, Chuck Lever III wrote:
>>>> 
>>>> 
>>>>> 
>>>>> 
>>>>> The remaining part of this text probably should be
>>>>> part of the man page for Ben's tool, or whatever is
>>>>> coming next.
>>>> 
>>>> My position is that there is no need for any tool.  The total amount
>>>> of
>>>> code needed is a couple of lines as presented in the text below.  Why
>>>> provide a wrapper just for that?
>>>> We *cannot* automatically decide how to find a name or where to store
>>>> a
>>>> generated uuid, so there is no added value that a tool could provide.
>>>> 
>>>> We cannot unilaterally fix container systems.  We can only tell
>>>> people
>>>> who build these systems of the requirements for NFS.
>>>> 
>>> 
>>> I disagree with this position. The value of having a standard tool is
>>> that it also creates a standard for how and where the uniquifier is
>>> generated and persisted.
>>> 
>>> Otherwise you have to deal with the fact that you may have a systemd
>>> script that persists something in one file, a Dockerfile recipe that
>>> generates something at container build time, and then a home-made
>>> script that looks for something in a different location. If you're
>>> trying to debug why your containers are all generating the same
>>> uniquifier, then that can be a problem.
>> I don't see how a tool can provide any consistency.

It seems to me that having a tool with its own man page directed
towards Linux distributors would be the central place for this
kind of configuration and implementation. Otherwise, we will have
to ensure this is done correctly for each implementation of
mount.


>> Is there some standard that say how containers should be built, and
>> where tools can store persistent data?  If not, the tool needs to be
>> configured, and that is not importantly different from bash being
>> configured with a 1-line script to write out the identifier.

IMO six of one, half dozen of another. I don't see this being
any more or less safe than changing each implementation of mount
to deal with an NFS-specific setting.


>> I'm not strongly against a tools, I just can't see the benefit.
> I think I agree with this... Thinking about it... having a command that
> tries to manipulate different containers in different ways just
> seems like a recipe for disaster... I just don't see how a command would
> ever get it right... Hell we can't agree on its command's name
> much less what it will do. :-)

To be clear what you are advocating, each implementation of mount.nfs,
including the ones that are not shipped with nfs-utils (like Busybox
and initramfs) will need to provide a mechanism for setting the client
uniquifier. Just to confirm that is what is behind door number one.

Since it is just a line or two of code, it might be of little
harm just to go with separate implementations for now and stop
talking about it. If it sucks, we can fix the suckage.

Who volunteers to implement this mechanism in mount.nfs ?


> So I like idea of documenting when needs to happen in the
> different types of containers... So I think the man page
> is the way to go... and I think it is the safest way to go.
> 
> Chuck, if you would like tweak the verbiage... by all means.

I stand ready.


> Neil, will be a V2 for man page patch from this discussion
> or should I just take the one you posted? If you do post
> a V2, please start a new thread.
> 
> steved.

--
Chuck Lever




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

* Re: [PATCH] nfs.man: document requirements for NFS mounts in a container
  2022-03-04 16:15                                                 ` Chuck Lever III
@ 2022-03-04 16:54                                                   ` Steve Dickson
  0 siblings, 0 replies; 36+ messages in thread
From: Steve Dickson @ 2022-03-04 16:54 UTC (permalink / raw)
  To: Chuck Lever III
  Cc: Neil Brown, Trond Myklebust, Linux NFS Mailing List, bcodding



On 3/4/22 11:15 AM, Chuck Lever III wrote:
> 
> 
>> On Mar 4, 2022, at 10:54 AM, Steve Dickson <steved@redhat.com> wrote:
>>
>> Hey!
>>
>> On 3/3/22 8:13 PM, NeilBrown wrote:
>>> On Fri, 04 Mar 2022, Trond Myklebust wrote:
>>>> On Thu, 2022-03-03 at 14:26 +1100, NeilBrown wrote:
>>>>> On Wed, 02 Mar 2022, Chuck Lever III wrote:
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>> The remaining part of this text probably should be
>>>>>> part of the man page for Ben's tool, or whatever is
>>>>>> coming next.
>>>>>
>>>>> My position is that there is no need for any tool.  The total amount
>>>>> of
>>>>> code needed is a couple of lines as presented in the text below.  Why
>>>>> provide a wrapper just for that?
>>>>> We *cannot* automatically decide how to find a name or where to store
>>>>> a
>>>>> generated uuid, so there is no added value that a tool could provide.
>>>>>
>>>>> We cannot unilaterally fix container systems.  We can only tell
>>>>> people
>>>>> who build these systems of the requirements for NFS.
>>>>>
>>>>
>>>> I disagree with this position. The value of having a standard tool is
>>>> that it also creates a standard for how and where the uniquifier is
>>>> generated and persisted.
>>>>
>>>> Otherwise you have to deal with the fact that you may have a systemd
>>>> script that persists something in one file, a Dockerfile recipe that
>>>> generates something at container build time, and then a home-made
>>>> script that looks for something in a different location. If you're
>>>> trying to debug why your containers are all generating the same
>>>> uniquifier, then that can be a problem.
>>> I don't see how a tool can provide any consistency.
> 
> It seems to me that having a tool with its own man page directed
> towards Linux distributors would be the central place for this
> kind of configuration and implementation. Otherwise, we will have
> to ensure this is done correctly for each implementation of
> mount.
> 
> 
>>> Is there some standard that say how containers should be built, and
>>> where tools can store persistent data?  If not, the tool needs to be
>>> configured, and that is not importantly different from bash being
>>> configured with a 1-line script to write out the identifier.
> 
> IMO six of one, half dozen of another. I don't see this being
> any more or less safe than changing each implementation of mount
> to deal with an NFS-specific setting.
> 
> 
>>> I'm not strongly against a tools, I just can't see the benefit.
>> I think I agree with this... Thinking about it... having a command that
>> tries to manipulate different containers in different ways just
>> seems like a recipe for disaster... I just don't see how a command would
>> ever get it right... Hell we can't agree on its command's name
>> much less what it will do. :-)
> 
> To be clear what you are advocating, each implementation of mount.nfs,
> including the ones that are not shipped with nfs-utils (like Busybox
> and initramfs) will need to provide a mechanism for setting the client
> uniquifier. Just to confirm that is what is behind door number one.
Well I can't speak for mount.nfs that are not in nfs-utils.
I'm assuming they are going to do... what are going to do...
regardless of what we do. At least we will give them a
guideline of what needs to be done.


> 
> Since it is just a line or two of code, it might be of little
> harm just to go with separate implementations for now and stop
> talking about it. If it sucks, we can fix the suckage.
Right I see documenting what needs to happen is the
first step. Heck don't we even know how accurate that
documentation is... yet! Once we vet the doc to make
sure it is accurate... Maybe then we could come up
with a auto-configuration solution that has been
proposed.

> 
> Who volunteers to implement this mechanism in mount.nfs ?Well, there has been 3 implementations shot down
which tells me, as a community, we need to get
more of an idea of what needs to happen and how.
That's why I think Neil's man page additions
is a good start.

> 
> 
>> So I like idea of documenting when needs to happen in the
>> different types of containers... So I think the man page
>> is the way to go... and I think it is the safest way to go.
>>
>> Chuck, if you would like tweak the verbiage... by all means.
> 
> I stand ready.
That I have confidence in. :-) Thank you!

steved.
> 
> 
>> Neil, will be a V2 for man page patch from this discussion
>> or should I just take the one you posted? If you do post
>> a V2, please start a new thread.
>>
>> steved.
> 
> --
> Chuck Lever
> 
> 
> 


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

* Re: [PATCH] nfs.man: document requirements for NFS mounts in a container
  2022-03-04 15:54                                               ` Steve Dickson
  2022-03-04 16:15                                                 ` Chuck Lever III
@ 2022-03-05  1:15                                                 ` NeilBrown
  1 sibling, 0 replies; 36+ messages in thread
From: NeilBrown @ 2022-03-05  1:15 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Trond Myklebust, chuck.lever, linux-nfs, bcodding

On Sat, 05 Mar 2022, Steve Dickson wrote:
> Hey!
> 
> On 3/3/22 8:13 PM, NeilBrown wrote:
> > On Fri, 04 Mar 2022, Trond Myklebust wrote:
> >> On Thu, 2022-03-03 at 14:26 +1100, NeilBrown wrote:
> >>> On Wed, 02 Mar 2022, Chuck Lever III wrote:
> >>>
> >>>
> >>>>
> >>>>
> >>>> The remaining part of this text probably should be
> >>>> part of the man page for Ben's tool, or whatever is
> >>>> coming next.
> >>>
> >>> My position is that there is no need for any tool.  The total amount
> >>> of
> >>> code needed is a couple of lines as presented in the text below.  Why
> >>> provide a wrapper just for that?
> >>> We *cannot* automatically decide how to find a name or where to store
> >>> a
> >>> generated uuid, so there is no added value that a tool could provide.
> >>>
> >>> We cannot unilaterally fix container systems.  We can only tell
> >>> people
> >>> who build these systems of the requirements for NFS.
> >>>
> >>
> >> I disagree with this position. The value of having a standard tool is
> >> that it also creates a standard for how and where the uniquifier is
> >> generated and persisted.
> >>
> >> Otherwise you have to deal with the fact that you may have a systemd
> >> script that persists something in one file, a Dockerfile recipe that
> >> generates something at container build time, and then a home-made
> >> script that looks for something in a different location. If you're
> >> trying to debug why your containers are all generating the same
> >> uniquifier, then that can be a problem.
> > 
> > I don't see how a tool can provide any consistency.
> > Is there some standard that say how containers should be built, and
> > where tools can store persistent data?  If not, the tool needs to be
> > configured, and that is not importantly different from bash being
> > configured with a 1-line script to write out the identifier.
> > 
> > I'm not strongly against a tools, I just can't see the benefit.
> I think I agree with this... Thinking about it... having a command that
> tries to manipulate different containers in different ways just
> seems like a recipe for disaster... I just don't see how a command would
> ever get it right... Hell we can't agree on its command's name
> much less what it will do. :-)
> 
> So I like idea of documenting when needs to happen in the
> different types of containers... So I think the man page
> is the way to go... and I think it is the safest way to go.
> 
> Chuck, if you would like tweak the verbiage... by all means.
> 
> Neil, will be a V2 for man page patch from this discussion
> or should I just take the one you posted? If you do post
> a V2, please start a new thread.

I'll post a V2.  Chuck made some excellent structural suggestions.

Thanks,
NeilBrown

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

* Re: [PATCH] nfs.man: document requirements for NFS mounts in a container
  2022-03-03 15:53                                           ` Chuck Lever III
@ 2022-03-08  0:44                                             ` NeilBrown
  2022-03-09 16:28                                               ` Chuck Lever III
  0 siblings, 1 reply; 36+ messages in thread
From: NeilBrown @ 2022-03-08  0:44 UTC (permalink / raw)
  To: Chuck Lever III
  Cc: Benjamin Coddington, Steve Dickson, Linux NFS Mailing List

On Fri, 04 Mar 2022, Chuck Lever III wrote:
> 
> 2. NAT
> 
> NAT hasn't been mentioned before, but it is a common
> deployment scenario where multiple clients can have the
> same hostname and local IP address (a private address such
> as 192.168.0.55) but the clients all access the same NFS
> server.

I can't see how NAT is relevant.  Whether or not clients have the same
host name seems to be independent of whether or not they access the
server through NAT.
What am I missing?
> 
> The client's identifier needs to be persistent so that:
> 
> 1. If the server reboots, it can recognize when clients
>    are re-establishing their lock and open state versus
>    an unfamiliar creating lock and open state that might
>    involve files that an existing client has open.

The protocol request clients which are re-establishing state to
explicitly say "I am re-establishing state" (e.g. CLAIM_PREVIOUS).
clients which are creating new state don't make that claim.

IF the server maintainer persistent state, then the reboot server needs
to use the client identifier to find the persistent state, but that is
not importantly different from the more common situation of a server
which hasn't rebooted and needs to find the appropriate state.

Again - what am I missing?


Thanks,
NeilBrown

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

* Re: [PATCH] nfs.man: document requirements for NFS mounts in a container
  2022-03-08  0:44                                             ` NeilBrown
@ 2022-03-09 16:28                                               ` Chuck Lever III
  2022-03-10  0:37                                                 ` NeilBrown
  0 siblings, 1 reply; 36+ messages in thread
From: Chuck Lever III @ 2022-03-09 16:28 UTC (permalink / raw)
  To: Neil Brown; +Cc: Benjamin Coddington, Steve Dickson, Linux NFS Mailing List



> On Mar 7, 2022, at 7:44 PM, NeilBrown <neilb@suse.de> wrote:
> 
> On Fri, 04 Mar 2022, Chuck Lever III wrote:
>> 
>> 2. NAT
>> 
>> NAT hasn't been mentioned before, but it is a common
>> deployment scenario where multiple clients can have the
>> same hostname and local IP address (a private address such
>> as 192.168.0.55) but the clients all access the same NFS
>> server.
> 
> I can't see how NAT is relevant.  Whether or not clients have the same
> host name seems to be independent of whether or not they access the
> server through NAT.
> What am I missing?

The usual construction of Linux's nfs_client_id4 includes
the hostname and client IP address. If two clients behind
two independent NAT boxes happen to use the same private
IP address and the same hostname (for example
"localhost.localdomain" is a common misconfiguration) then
both of these clients present the same nfs_client_id4
string to the NFS server.

Hilarity ensues.


>> The client's identifier needs to be persistent so that:
>> 
>> 1. If the server reboots, it can recognize when clients
>>   are re-establishing their lock and open state versus
>>   an unfamiliar creating lock and open state that might
>>   involve files that an existing client has open.
> 
> The protocol request clients which are re-establishing state to
> explicitly say "I am re-establishing state" (e.g. CLAIM_PREVIOUS).
> clients which are creating new state don't make that claim.
> 
> IF the server maintainer persistent state, then the reboot server needs
> to use the client identifier to find the persistent state, but that is
> not importantly different from the more common situation of a server
> which hasn't rebooted and needs to find the appropriate state.
> 
> Again - what am I missing?

The server records each client's nfs_client_id4 and its
boot verifier.

It's my understanding that the server is required to reject
CLAIM_PREVIOUS opens if it does not recognize either the
nfs_client_id4 string or its boot verifier, since that
means that the client had no previous state during the last
most recent server epoch.


--
Chuck Lever




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

* Re: [PATCH] nfs.man: document requirements for NFS mounts in a container
  2022-03-09 16:28                                               ` Chuck Lever III
@ 2022-03-10  0:37                                                 ` NeilBrown
  0 siblings, 0 replies; 36+ messages in thread
From: NeilBrown @ 2022-03-10  0:37 UTC (permalink / raw)
  To: Chuck Lever III
  Cc: Benjamin Coddington, Steve Dickson, Linux NFS Mailing List

On Thu, 10 Mar 2022, Chuck Lever III wrote:
> 
> > On Mar 7, 2022, at 7:44 PM, NeilBrown <neilb@suse.de> wrote:
> > 
> > On Fri, 04 Mar 2022, Chuck Lever III wrote:
> >> 
> >> 2. NAT
> >> 
> >> NAT hasn't been mentioned before, but it is a common
> >> deployment scenario where multiple clients can have the
> >> same hostname and local IP address (a private address such
> >> as 192.168.0.55) but the clients all access the same NFS
> >> server.
> > 
> > I can't see how NAT is relevant.  Whether or not clients have the same
> > host name seems to be independent of whether or not they access the
> > server through NAT.
> > What am I missing?
> 
> The usual construction of Linux's nfs_client_id4 includes
> the hostname and client IP address. If two clients behind
> two independent NAT boxes happen to use the same private
> IP address and the same hostname (for example
> "localhost.localdomain" is a common misconfiguration) then
> both of these clients present the same nfs_client_id4
> string to the NFS server.
> 
> Hilarity ensues.

This would only apply to NFSv4.0 (and without migration enabled).
NFSv4.1 and later don't include the IP address in the client identity.

So I think the scenario you describe is primarily a problem of the
hostname being misconfigured.  In NFSv4.0 the normal variety of IP
address can hide that problem.  IF NAT Is used in such a way that two
clients are configured with the same IP address, the defeats the hiding.

I don't think the extra complexity of NAT really makes this more
interesting.   The problem is uniform hostnames, and the fix is the same
for any other case of uniform host names.

> 
> 
> >> The client's identifier needs to be persistent so that:
> >> 
> >> 1. If the server reboots, it can recognize when clients
> >>   are re-establishing their lock and open state versus
> >>   an unfamiliar creating lock and open state that might
> >>   involve files that an existing client has open.
> > 
> > The protocol request clients which are re-establishing state to
> > explicitly say "I am re-establishing state" (e.g. CLAIM_PREVIOUS).
> > clients which are creating new state don't make that claim.
> > 
> > IF the server maintainer persistent state, then the reboot server needs
> > to use the client identifier to find the persistent state, but that is
> > not importantly different from the more common situation of a server
> > which hasn't rebooted and needs to find the appropriate state.
> > 
> > Again - what am I missing?
> 
> The server records each client's nfs_client_id4 and its
> boot verifier.
> 
> It's my understanding that the server is required to reject
> CLAIM_PREVIOUS opens if it does not recognize either the
> nfs_client_id4 string or its boot verifier, since that
> means that the client had no previous state during the last
> most recent server epoch.

I think we are saying the same thing with different words.
When you wrote

    If the server reboots, it can recognize when clients
    are re-establishing their lock and open state 

I think that "validate" is more relevant than "recognize".  The server
knows from the request that an attempt is being made to reestablish
state.  The client identity, credential, and boot verifier are used
to validate that request.

But essentially we are on the same page here.

Thanks,
NeilBrown

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

end of thread, other threads:[~2022-03-10  0:37 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-10 18:01 [PATCH v2 0/2] nfsuuid and udev examples Benjamin Coddington
2022-02-10 18:01 ` [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers Benjamin Coddington
2022-02-10 18:15   ` Chuck Lever III
2022-02-11 13:44     ` Steve Dickson
2022-02-11 15:48       ` Chuck Lever III
2022-02-11 18:28         ` Benjamin Coddington
2022-02-11 19:04           ` Chuck Lever III
2022-02-11 19:30             ` Benjamin Coddington
2022-02-11 20:00               ` Chuck Lever III
2022-02-11 20:16                 ` Benjamin Coddington
2022-02-11 20:51                   ` Chuck Lever III
2022-02-11 21:06                     ` Benjamin Coddington
2022-02-14  0:04                       ` NeilBrown
2022-02-14 11:15                         ` Benjamin Coddington
2022-02-14 15:39                           ` Chuck Lever III
2022-02-16 19:01                             ` Benjamin Coddington
2022-02-16 19:35                               ` Chuck Lever III
2022-02-16 20:44                                 ` Benjamin Coddington
2022-02-16 23:16                                   ` NeilBrown
2022-03-01  3:43                                     ` [PATCH] nfs.man: document requirements for NFS mounts in a container NeilBrown
2022-03-01 15:08                                       ` Chuck Lever III
2022-03-01 15:16                                         ` Chuck Lever III
2022-03-03  3:26                                         ` NeilBrown
2022-03-03 14:37                                           ` Trond Myklebust
2022-03-04  1:13                                             ` NeilBrown
2022-03-04 15:54                                               ` Steve Dickson
2022-03-04 16:15                                                 ` Chuck Lever III
2022-03-04 16:54                                                   ` Steve Dickson
2022-03-05  1:15                                                 ` NeilBrown
2022-03-03 15:53                                           ` Chuck Lever III
2022-03-08  0:44                                             ` NeilBrown
2022-03-09 16:28                                               ` Chuck Lever III
2022-03-10  0:37                                                 ` NeilBrown
2022-02-17 14:43                                   ` [PATCH v2 1/2] nfsuuid: a tool to create and persist nfs4 client uniquifiers Chuck Lever III
2022-02-14 22:40                           ` NeilBrown
2022-02-10 18:01 ` [PATCH v2 2/2] nfsuuid: add some example udev rules Benjamin Coddington

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.