Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <78d9e894-7c30-fa5a-58ee-b99b161e475e@redhat.com>
Date:   Sat, 19 Nov 2022 14:28:47 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.1
Subject: Re: [PATCH nfs-utils - v2] nfsd: allow server scope to be set with
 config or command line.
To:     NeilBrown <neilb@suse.de>
Cc:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
References: <166813011417.19313.12216066495338584736@noble.neil.brown.name>
 <166848711530.4614.10805714091203567578@noble.neil.brown.name>
 <166848720192.4614.15106591314080263893@noble.neil.brown.name>
Content-Language: en-US
From:   Steve Dickson <steved@redhat.com>
In-Reply-To: <166848720192.4614.15106591314080263893@noble.neil.brown.name>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hey!

On 11/14/22 11:40 PM, NeilBrown wrote:
> 
> NFSv4.1 and later require the server to report a "scope".  Servers with
> the same scope are expected to understand each other's state ids etc,
> though may not accept them - this ensure there can be no
> misunderstanding.  This is helpful for migration.
> 
> Servers with different scope are known to be different and if a server
> appears to change scope on a restart, lock recovery must not be
> attempted.
> 
> It is important for fail-over configurations to have the same scope for
> all server instances.  Linux NFSD sets scope to host name.  It is common
> for fail-over configurations to use different host names on different
> server nodes.  So the default is not good for these configurations and
> must be over-ridden.
> 
> As discussed in
>    https://github.com/ClusterLabs/resource-agents/issues/1644
> some HA management tools attempt to address this with calls to "unshare"
> and "hostname" before running "rpc.nfsd".  This is unnecessarily
> cumbersome.
> 
> This patch adds a "-S" command-line option and nfsd.scope config value
> so that the scope can be set easily for nfsd.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>   systemd/nfs.conf.man |  1 +
>   utils/nfsd/nfsd.c    | 17 ++++++++++++++++-
>   utils/nfsd/nfsd.man  | 13 ++++++++++++-
>   3 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
> index b95c05a68759..bfd3380ff081 100644
> --- a/systemd/nfs.conf.man
> +++ b/systemd/nfs.conf.man
> @@ -172,6 +172,7 @@ for details.
>   Recognized values:
>   .BR threads ,
>   .BR host ,
> +.BR scope ,
>   .BR port ,
>   .BR grace-time ,
>   .BR lease-time ,
> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> index 4016a761293b..249df00b448d 100644
> --- a/utils/nfsd/nfsd.c
> +++ b/utils/nfsd/nfsd.c
> @@ -23,6 +23,7 @@
>   #include <sys/socket.h>
>   #include <netinet/in.h>
>   #include <arpa/inet.h>
> +#include <sched.h>
>   
>   #include "conffile.h"
>   #include "nfslib.h"
> @@ -39,6 +40,7 @@ static void	usage(const char *);
>   static struct option longopts[] =
>   {
>   	{ "host", 1, 0, 'H' },
> +	{ "scope", 1, 0, 'S'},
>   	{ "help", 0, 0, 'h' },
>   	{ "no-nfs-version", 1, 0, 'N' },
>   	{ "nfs-version", 1, 0, 'V' },
> @@ -69,6 +71,7 @@ main(int argc, char **argv)
>   	int	count = NFSD_NPROC, c, i, error = 0, portnum, fd, found_one;
>   	char *p, *progname, *port, *rdma_port = NULL;
>   	char **haddr = NULL;
> +	char *scope = NULL;
>   	int hcounter = 0;
>   	struct conf_list *hosts;
>   	int	socket_up = 0;
> @@ -168,8 +171,9 @@ main(int argc, char **argv)
>   			hcounter++;
>   		}
>   	}
> +	scope = conf_get_str("nfsd", "scope");
>   
> -	while ((c = getopt_long(argc, argv, "dH:hN:V:p:P:stTuUrG:L:", longopts, NULL)) != EOF) {
> +	while ((c = getopt_long(argc, argv, "dH:S:hN:V:p:P:stTuUrG:L:", longopts, NULL)) != EOF) {
>   		switch(c) {
>   		case 'd':
>   			xlog_config(D_ALL, 1);
> @@ -190,6 +194,9 @@ main(int argc, char **argv)
>   			haddr[hcounter] = optarg;
>   			hcounter++;
>   			break;
> +		case 'S':
> +			scope = optarg;
> +			break;
>   		case 'P':	/* XXX for nfs-server compatibility */
>   		case 'p':
>   			/* only the last -p option has any effect */
> @@ -367,6 +374,14 @@ main(int argc, char **argv)
>   	if (lease  > 0)
>   		nfssvc_set_time("lease", lease);
>   
> +	if (scope) {
> +		if (unshare(soc) < 0 ||
> +		    sethostname(scope, strlen(scope)) < 0) {
> +			xlog(L_ERROR, "Unable to set server scope: %m");
> +			error = -1;
> +			goto out;
> +		}
> +	}
So setting the scope resets the utsname and hostname which
will effect the entire system, possibly negatively. Breaking
DNS... who knows what is going to happen, when the hostname
is changed on the fly.. But with that said..

I understand what you are trying to doing, but I just
don't think it's documented well enough... Saying something
like setting the scope *will* change both utsname and hostname
to given scope or something like... I just want be more explicit
as to what setting the scope is actually going to do.

steved.

>   	i = 0;
>   	do {
>   		error = nfssvc_set_sockets(protobits, haddr[i], port);
> diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
> index bb99fe2b1d89..dc05f3623465 100644
> --- a/utils/nfsd/nfsd.man
> +++ b/utils/nfsd/nfsd.man
> @@ -35,9 +35,17 @@ Note that
>   .B lockd
>   (which performs file locking services for NFS) may still accept
>   request on all known network addresses.  This may change in future
> -releases of the Linux Kernel. This option can be used multiple time
> +releases of the Linux Kernel. This option can be used multiple times
>   to listen to more than one interface.
>   .TP
> +.B \S " or " \-\-scope scope
> +NFSv4.1 and later require the server to report a "scope" which is used
> +by the clients to detect if two connections are to the same server.
> +By default Linux NFSD uses the host name as the scope.
> +.sp
> +It is particularly important for high-availablity configurations to ensure
> +that all potential server nodes report the same server scope.
> +.TP
>   .B \-p " or " \-\-port  port
>   specify a different port to listen on for NFS requests. By default,
>   .B rpc.nfsd
> @@ -134,6 +142,9 @@ will listen on.  Use of the
>   .B --host
>   option replaces all host names listed here.
>   .TP
> +.B scope
> +Set the server scope.
> +.TP
>   .B grace-time
>   The grace time, for both NFSv4 and NLM, in seconds.
>   .TP