[1/2] nfsd: Allow the caller to turn off NFSv4.0 without turning off NFSv4.x
diff mbox

Message ID 20170224003344.113724-2-trond.myklebust@primarydata.com
State New
Headers show

Commit Message

Trond Myklebust Feb. 24, 2017, 12:33 a.m. UTC
The new semantic is that '-N4' turns off all NFSv4 minor versions, while
'-V4' turns them all on. In order to turn off just minor version x (x >= 0),
use -N4.x, and to turn it back on. '-V4.x'.

Note that on older kernels, attempting to use -N4.0 and -V4.0 is
equivalent to specifying -N4 or -V4.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 support/include/nfs/nfs.h |  7 +++--
 utils/nfsd/nfsd.c         | 39 +++++++++++++++---------
 utils/nfsd/nfsd.man       |  4 +--
 utils/nfsd/nfssvc.c       | 76 ++++++++++++++++++++++++++++++++++++-----------
 utils/nfsd/nfssvc.h       |  1 +
 5 files changed, 92 insertions(+), 35 deletions(-)

Comments

Steve Dickson April 4, 2017, 8:07 p.m. UTC | #1
Hey Trond,

My apologies for taking so long to address this... 

On 02/23/2017 07:33 PM, Trond Myklebust wrote:
> The new semantic is that '-N4' turns off all NFSv4 minor versions, while
> '-V4' turns them all on. In order to turn off just minor version x (x >= 0),
> use -N4.x, and to turn it back on. '-V4.x'.
> 
> Note that on older kernels, attempting to use -N4.0 and -V4.0 is
> equivalent to specifying -N4 or -V4.
doing a 

nfsd -d -N4.0 -V4.1 -V4.2 
nfsd: Writing version string to kernel: -2 +3 -4 +4.1 +4.2

does the right thing but when I do a

nfsd -d -N4.0 
nfsd: Writing version string to kernel: -2 +3 -4

It brings down all of the v4 minor versions, Is that
intentional? It seems to me doing a -N4.0 should only
stop 4.0 from coming up not v4.1 or v4.2

steved.
 
> 
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
> ---
>  support/include/nfs/nfs.h |  7 +++--
>  utils/nfsd/nfsd.c         | 39 +++++++++++++++---------
>  utils/nfsd/nfsd.man       |  4 +--
>  utils/nfsd/nfssvc.c       | 76 ++++++++++++++++++++++++++++++++++++-----------
>  utils/nfsd/nfssvc.h       |  1 +
>  5 files changed, 92 insertions(+), 35 deletions(-)
> 
> diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
> index 15ecc6bfc485..5860343f78b7 100644
> --- a/support/include/nfs/nfs.h
> +++ b/support/include/nfs/nfs.h
> @@ -16,8 +16,8 @@
>  #define NFSD_MINVERS 2
>  #define NFSD_MAXVERS 4
>  
> -#define NFS4_MINMINOR 1
> -#define NFS4_MAXMINOR WORD_BIT
> +#define NFS4_MINMINOR 0
> +#define NFS4_MAXMINOR (WORD_BIT-1)
>  
>  struct nfs_fh_len {
>  	int		fh_size;
> @@ -29,15 +29,18 @@ struct nfs_fh_len {
>  #define NFSCTL_TCPBIT		      (1 << (18 - 1))
>  
>  #define NFSCTL_VERUNSET(_cltbits, _v) ((_cltbits) &= ~(1 << ((_v) - 1))) 
> +#define NFSCTL_MINORUNSET(_cltbits, _v) ((_cltbits) &= ~(1 << (_v)))
>  #define NFSCTL_UDPUNSET(_cltbits)     ((_cltbits) &= ~NFSCTL_UDPBIT) 
>  #define NFSCTL_TCPUNSET(_cltbits)     ((_cltbits) &= ~NFSCTL_TCPBIT) 
>  
>  #define NFSCTL_VERISSET(_cltbits, _v) ((_cltbits) & (1 << ((_v) - 1))) 
> +#define NFSCTL_MINORISSET(_cltbits, _v) ((_cltbits) & (1 << (_v)))
>  #define NFSCTL_UDPISSET(_cltbits)     ((_cltbits) & NFSCTL_UDPBIT) 
>  #define NFSCTL_TCPISSET(_cltbits)     ((_cltbits) & NFSCTL_TCPBIT) 
>  
>  #define NFSCTL_VERDEFAULT (0xc)       /* versions 3 and 4 */
>  #define NFSCTL_VERSET(_cltbits, _v)   ((_cltbits) |= (1 << ((_v) - 1))) 
> +#define NFSCTL_MINORSET(_cltbits, _v)   ((_cltbits) |= (1 << (_v)))
>  #define NFSCTL_UDPSET(_cltbits)       ((_cltbits) |= NFSCTL_UDPBIT)
>  #define NFSCTL_TCPSET(_cltbits)       ((_cltbits) |= NFSCTL_TCPBIT)
>  
> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> index 20f4b7952203..1708521ab286 100644
> --- a/utils/nfsd/nfsd.c
> +++ b/utils/nfsd/nfsd.c
> @@ -67,6 +67,7 @@ main(int argc, char **argv)
>  	int	socket_up = 0;
>  	unsigned int minorvers = 0;
>  	unsigned int minorversset = 0;
> +	unsigned int minormask = 0;
>  	unsigned int versbits = NFSCTL_VERDEFAULT;
>  	unsigned int protobits = NFSCTL_ALLBITS;
>  	int grace = -1;
> @@ -104,10 +105,16 @@ main(int argc, char **argv)
>  		else
>  			NFSCTL_VERUNSET(versbits, i);
>  	}
> +
> +	nfssvc_get_minormask(&minormask);
>  	/* We assume the kernel will default all minor versions to 'on',
>  	 * and allow the config file to disable some.
>  	 */
> -	for (i = NFS4_MINMINOR; i <= NFS4_MAXMINOR; i++) {
> +	if (NFSCTL_VERISSET(versbits, 4)) {
> +		NFSCTL_MINORSET(minorversset, 0);
> +		NFSCTL_MINORSET(minorvers, 0);
> +	}
> +	for (i = 1; i <= NFS4_MAXMINOR; i++) {
>  		char tag[20];
>  		sprintf(tag, "vers4.%d", i);
>  		/* The default for minor version support is to let the
> @@ -119,12 +126,12 @@ main(int argc, char **argv)
>  		 * (i.e. don't set the bit in minorversset).
>  		 */
>  		if (!conf_get_bool("nfsd", tag, 1)) {
> -			NFSCTL_VERSET(minorversset, i);
> -			NFSCTL_VERUNSET(minorvers, i);
> +			NFSCTL_MINORSET(minorversset, i);
> +			NFSCTL_MINORUNSET(minorvers, i);
>  		}
>  		if (conf_get_bool("nfsd", tag, 0)) {
> -			NFSCTL_VERSET(minorversset, i);
> -			NFSCTL_VERSET(minorvers, i);
> +			NFSCTL_MINORSET(minorversset, i);
> +			NFSCTL_MINORSET(minorvers, i);
>  		}
>  	}
>  
> @@ -179,13 +186,17 @@ main(int argc, char **argv)
>  			case 4:
>  				if (*p == '.') {
>  					int i = atoi(p+1);
> -					if (i < NFS4_MINMINOR || i > NFS4_MAXMINOR) {
> +					if (i < 0 || i > NFS4_MAXMINOR) {
>  						fprintf(stderr, "%s: unsupported minor version\n", optarg);
>  						exit(1);
>  					}
> -					NFSCTL_VERSET(minorversset, i);
> -					NFSCTL_VERUNSET(minorvers, i);
> -					break;
> +					NFSCTL_MINORSET(minorversset, i);
> +					NFSCTL_MINORUNSET(minorvers, i);
> +					if (minorvers != 0)
> +						break;
> +				} else {
> +					minorvers = 0;
> +					minorversset = minormask;
>  				}
>  			case 3:
>  			case 2:
> @@ -201,14 +212,14 @@ main(int argc, char **argv)
>  			case 4:
>  				if (*p == '.') {
>  					int i = atoi(p+1);
> -					if (i < NFS4_MINMINOR || i > NFS4_MAXMINOR) {
> +					if (i < 0 || i > NFS4_MAXMINOR) {
>  						fprintf(stderr, "%s: unsupported minor version\n", optarg);
>  						exit(1);
>  					}
> -					NFSCTL_VERSET(minorversset, i);
> -					NFSCTL_VERSET(minorvers, i);
> -					break;
> -				}
> +					NFSCTL_MINORSET(minorversset, i);
> +					NFSCTL_MINORSET(minorvers, i);
> +				} else
> +					minorvers = minorversset = minormask;
>  			case 3:
>  			case 2:
>  				NFSCTL_VERSET(versbits, c);
> diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
> index 8901fb6c6872..0d797fd2ec8d 100644
> --- a/utils/nfsd/nfsd.man
> +++ b/utils/nfsd/nfsd.man
> @@ -57,7 +57,7 @@ This option can be used to request that
>  .B rpc.nfsd
>  does not offer certain versions of NFS. The current version of
>  .B rpc.nfsd
> -can support major NFS versions 2,3,4 and the minor versions 4.1 and 4.2.
> +can support major NFS versions 2,3,4 and the minor versions 4.0, 4.1 and 4.2.
>  .TP
>  .B \-s " or " \-\-syslog
>  By default,
> @@ -82,7 +82,7 @@ This option can be used to request that
>  .B rpc.nfsd
>  offer certain versions of NFS. The current version of
>  .B rpc.nfsd
> -can support major NFS versions 2,3,4 and the minor versions 4.1 and 4.2.
> +can support major NFS versions 2,3,4 and the minor versions 4.0, 4.1 and 4.2.
>  .TP
>  .B \-L " or " \-\-lease-time seconds
>  Set the lease-time used for NFSv4.  This corresponds to how often
> diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
> index 07f6ff1204d1..e8609c15b8e5 100644
> --- a/utils/nfsd/nfssvc.c
> +++ b/utils/nfsd/nfssvc.c
> @@ -330,36 +330,78 @@ nfssvc_set_time(const char *type, const int seconds)
>  }
>  
>  void
> +nfssvc_get_minormask(unsigned int *mask)
> +{
> +	int fd;
> +	char *ptr = buf;
> +	ssize_t size;
> +
> +	fd = open(NFSD_VERS_FILE, O_RDONLY);
> +	if (fd < 0)
> +		return;
> +
> +	size = read(fd, buf, sizeof(buf));
> +	if (size < 0) {
> +		xlog(L_ERROR, "Getting versions failed: errno %d (%m)", errno);
> +		goto out;
> +	}
> +	ptr[size] = '\0';
> +	for (;;) {
> +		unsigned vers, minor = 0;
> +		char *token = strtok(ptr, " ");
> +
> +		if (!token)
> +			break;
> +		ptr = NULL;
> +		if (*token != '+' && *token != '-')
> +			continue;
> +		if (sscanf(++token, "%u.%u", &vers, &minor) > 0 &&
> +		    vers == 4 && minor <= NFS4_MAXMINOR)
> +			NFSCTL_MINORSET(*mask, minor);
> +	}
> +out:
> +	close(fd);
> +	return;
> +}
> +
> +static int
> +nfssvc_print_vers(char *ptr, unsigned size, unsigned vers, unsigned minorvers,
> +		int isset)
> +{
> +	char sign = isset ? '+' : '-';
> +	if (minorvers == 0)
> +		return snprintf(ptr, size, "%c%u ", sign, vers);
> +	return snprintf(ptr, size, "%c%u.%u ", sign, vers, minorvers);
> +}
> +
> +void
>  nfssvc_setvers(unsigned int ctlbits, unsigned int minorvers, unsigned int minorversset)
>  {
>  	int fd, n, off;
> -	char *ptr;
>  
> -	ptr = buf;
>  	off = 0;
>  	fd = open(NFSD_VERS_FILE, O_WRONLY);
>  	if (fd < 0)
>  		return;
>  
> -	for (n = NFS4_MINMINOR; n <= NFS4_MAXMINOR; n++) {
> -		if (NFSCTL_VERISSET(minorversset, n)) {
> -			if (NFSCTL_VERISSET(minorvers, n))
> -				off += snprintf(ptr+off, sizeof(buf) - off, "+4.%d ", n);
> -			else
> -				off += snprintf(ptr+off, sizeof(buf) - off, "-4.%d ", n);
> -		}
> -	}
> -	for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) {
> -		if (NFSCTL_VERISSET(ctlbits, n))
> -		    off += snprintf(ptr+off, sizeof(buf) - off, "+%d ", n);
> -		else
> -		    off += snprintf(ptr+off, sizeof(buf) - off, "-%d ", n);
> +	for (n = NFSD_MINVERS; n <= ((NFSD_MAXVERS < 3) ? NFSD_MAXVERS : 3); n++)
> +		off += nfssvc_print_vers(&buf[off], sizeof(buf) - off,
> +				n, 0, NFSCTL_VERISSET(ctlbits, n));
> +
> +	for (n = 0; n <= NFS4_MAXMINOR; n++) {
> +		if (!NFSCTL_MINORISSET(minorversset, n))
> +			continue;
> +		off += nfssvc_print_vers(&buf[off], sizeof(buf) - off,
> +				4, n, NFSCTL_MINORISSET(minorvers, n));
>  	}
> +	if (!off--)
> +		goto out;
> +	buf[off] = '\0';
>  	xlog(D_GENERAL, "Writing version string to kernel: %s", buf);
> -	snprintf(ptr+off, sizeof(buf) - off, "\n");
> +	snprintf(&buf[off], sizeof(buf) - off, "\n");
>  	if (write(fd, buf, strlen(buf)) != (ssize_t)strlen(buf))
>  		xlog(L_ERROR, "Setting version failed: errno %d (%m)", errno);
> -
> +out:
>  	close(fd);
>  
>  	return;
> diff --git a/utils/nfsd/nfssvc.h b/utils/nfsd/nfssvc.h
> index cd5a7e84dc7e..39ebf37a90fe 100644
> --- a/utils/nfsd/nfssvc.h
> +++ b/utils/nfsd/nfssvc.h
> @@ -28,3 +28,4 @@ void	nfssvc_set_time(const char *type, const int seconds);
>  int	nfssvc_set_rdmaport(const char *port);
>  void	nfssvc_setvers(unsigned int ctlbits, unsigned int minorvers4, unsigned int minorvers4set);
>  int	nfssvc_threads(int nrservs);
> +void	nfssvc_get_minormask(unsigned int *mask);
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Trond Myklebust April 4, 2017, 8:26 p.m. UTC | #2
On Tue, 2017-04-04 at 16:07 -0400, Steve Dickson wrote:
> Hey Trond,

> 

> My apologies for taking so long to address this... 

> 

> On 02/23/2017 07:33 PM, Trond Myklebust wrote:

> > The new semantic is that '-N4' turns off all NFSv4 minor versions,

> > while

> > '-V4' turns them all on. In order to turn off just minor version x

> > (x >= 0),

> > use -N4.x, and to turn it back on. '-V4.x'.

> > 

> > Note that on older kernels, attempting to use -N4.0 and -V4.0 is

> > equivalent to specifying -N4 or -V4.

> 

> doing a 

> 

> nfsd -d -N4.0 -V4.1 -V4.2 

> nfsd: Writing version string to kernel: -2 +3 -4 +4.1 +4.2

> 

> does the right thing but when I do a

> 

> nfsd -d -N4.0 

> nfsd: Writing version string to kernel: -2 +3 -4

> 

> It brings down all of the v4 minor versions, Is that

> intentional? It seems to me doing a -N4.0 should only

> stop 4.0 from coming up not v4.1 or v4.2


That is unfortunately not possible for older kernels. They lack the
kernel API to turn off NFSv4.0 only. We could perhaps try to return an
error if you were to specify these flags for those kernels, but that
would require us to hard-code a "minimal" kernel version in nfs-utils.

> 

> steved.

>  

> > 

> > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>

> > ---

> >  support/include/nfs/nfs.h |  7 +++--

> >  utils/nfsd/nfsd.c         | 39 +++++++++++++++---------

> >  utils/nfsd/nfsd.man       |  4 +--

> >  utils/nfsd/nfssvc.c       | 76

> > ++++++++++++++++++++++++++++++++++++-----------

> >  utils/nfsd/nfssvc.h       |  1 +

> >  5 files changed, 92 insertions(+), 35 deletions(-)

> > 

> > diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h

> > index 15ecc6bfc485..5860343f78b7 100644

> > --- a/support/include/nfs/nfs.h

> > +++ b/support/include/nfs/nfs.h

> > @@ -16,8 +16,8 @@

> >  #define NFSD_MINVERS 2

> >  #define NFSD_MAXVERS 4

> >  

> > -#define NFS4_MINMINOR 1

> > -#define NFS4_MAXMINOR WORD_BIT

> > +#define NFS4_MINMINOR 0

> > +#define NFS4_MAXMINOR (WORD_BIT-1)

> >  

> >  struct nfs_fh_len {

> >  	int		fh_size;

> > @@ -29,15 +29,18 @@ struct nfs_fh_len {

> >  #define NFSCTL_TCPBIT		      (1 << (18 - 1))

> >  

> >  #define NFSCTL_VERUNSET(_cltbits, _v) ((_cltbits) &= ~(1 << ((_v)

> > - 1))) 

> > +#define NFSCTL_MINORUNSET(_cltbits, _v) ((_cltbits) &= ~(1 <<

> > (_v)))

> >  #define NFSCTL_UDPUNSET(_cltbits)     ((_cltbits) &=

> > ~NFSCTL_UDPBIT) 

> >  #define NFSCTL_TCPUNSET(_cltbits)     ((_cltbits) &=

> > ~NFSCTL_TCPBIT) 

> >  

> >  #define NFSCTL_VERISSET(_cltbits, _v) ((_cltbits) & (1 << ((_v) -

> > 1))) 

> > +#define NFSCTL_MINORISSET(_cltbits, _v) ((_cltbits) & (1 << (_v)))

> >  #define NFSCTL_UDPISSET(_cltbits)     ((_cltbits) &

> > NFSCTL_UDPBIT) 

> >  #define NFSCTL_TCPISSET(_cltbits)     ((_cltbits) &

> > NFSCTL_TCPBIT) 

> >  

> >  #define NFSCTL_VERDEFAULT (0xc)       /* versions 3 and 4 */

> >  #define NFSCTL_VERSET(_cltbits, _v)   ((_cltbits) |= (1 << ((_v) -

> > 1))) 

> > +#define NFSCTL_MINORSET(_cltbits, _v)   ((_cltbits) |= (1 <<

> > (_v)))

> >  #define NFSCTL_UDPSET(_cltbits)       ((_cltbits) |=

> > NFSCTL_UDPBIT)

> >  #define NFSCTL_TCPSET(_cltbits)       ((_cltbits) |=

> > NFSCTL_TCPBIT)

> >  

> > diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c

> > index 20f4b7952203..1708521ab286 100644

> > --- a/utils/nfsd/nfsd.c

> > +++ b/utils/nfsd/nfsd.c

> > @@ -67,6 +67,7 @@ main(int argc, char **argv)

> >  	int	socket_up = 0;

> >  	unsigned int minorvers = 0;

> >  	unsigned int minorversset = 0;

> > +	unsigned int minormask = 0;

> >  	unsigned int versbits = NFSCTL_VERDEFAULT;

> >  	unsigned int protobits = NFSCTL_ALLBITS;

> >  	int grace = -1;

> > @@ -104,10 +105,16 @@ main(int argc, char **argv)

> >  		else

> >  			NFSCTL_VERUNSET(versbits, i);

> >  	}

> > +

> > +	nfssvc_get_minormask(&minormask);

> >  	/* We assume the kernel will default all minor versions to

> > 'on',

> >  	 * and allow the config file to disable some.

> >  	 */

> > -	for (i = NFS4_MINMINOR; i <= NFS4_MAXMINOR; i++) {

> > +	if (NFSCTL_VERISSET(versbits, 4)) {

> > +		NFSCTL_MINORSET(minorversset, 0);

> > +		NFSCTL_MINORSET(minorvers, 0);

> > +	}

> > +	for (i = 1; i <= NFS4_MAXMINOR; i++) {

> >  		char tag[20];

> >  		sprintf(tag, "vers4.%d", i);

> >  		/* The default for minor version support is to let

> > the

> > @@ -119,12 +126,12 @@ main(int argc, char **argv)

> >  		 * (i.e. don't set the bit in minorversset).

> >  		 */

> >  		if (!conf_get_bool("nfsd", tag, 1)) {

> > -			NFSCTL_VERSET(minorversset, i);

> > -			NFSCTL_VERUNSET(minorvers, i);

> > +			NFSCTL_MINORSET(minorversset, i);

> > +			NFSCTL_MINORUNSET(minorvers, i);

> >  		}

> >  		if (conf_get_bool("nfsd", tag, 0)) {

> > -			NFSCTL_VERSET(minorversset, i);

> > -			NFSCTL_VERSET(minorvers, i);

> > +			NFSCTL_MINORSET(minorversset, i);

> > +			NFSCTL_MINORSET(minorvers, i);

> >  		}

> >  	}

> >  

> > @@ -179,13 +186,17 @@ main(int argc, char **argv)

> >  			case 4:

> >  				if (*p == '.') {

> >  					int i = atoi(p+1);

> > -					if (i < NFS4_MINMINOR || i

> > > NFS4_MAXMINOR) {

> > +					if (i < 0 || i >

> > NFS4_MAXMINOR) {

> >  						fprintf(stderr,

> > "%s: unsupported minor version\n", optarg);

> >  						exit(1);

> >  					}

> > -					NFSCTL_VERSET(minorversset

> > , i);

> > -					NFSCTL_VERUNSET(minorvers,

> > i);

> > -					break;

> > +					NFSCTL_MINORSET(minorverss

> > et, i);

> > +					NFSCTL_MINORUNSET(minorver

> > s, i);

> > +					if (minorvers != 0)

> > +						break;

> > +				} else {

> > +					minorvers = 0;

> > +					minorversset = minormask;

> >  				}

> >  			case 3:

> >  			case 2:

> > @@ -201,14 +212,14 @@ main(int argc, char **argv)

> >  			case 4:

> >  				if (*p == '.') {

> >  					int i = atoi(p+1);

> > -					if (i < NFS4_MINMINOR || i

> > > NFS4_MAXMINOR) {

> > +					if (i < 0 || i >

> > NFS4_MAXMINOR) {

> >  						fprintf(stderr,

> > "%s: unsupported minor version\n", optarg);

> >  						exit(1);

> >  					}

> > -					NFSCTL_VERSET(minorversset

> > , i);

> > -					NFSCTL_VERSET(minorvers,

> > i);

> > -					break;

> > -				}

> > +					NFSCTL_MINORSET(minorverss

> > et, i);

> > +					NFSCTL_MINORSET(minorvers,

> > i);

> > +				} else

> > +					minorvers = minorversset =

> > minormask;

> >  			case 3:

> >  			case 2:

> >  				NFSCTL_VERSET(versbits, c);

> > diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man

> > index 8901fb6c6872..0d797fd2ec8d 100644

> > --- a/utils/nfsd/nfsd.man

> > +++ b/utils/nfsd/nfsd.man

> > @@ -57,7 +57,7 @@ This option can be used to request that

> >  .B rpc.nfsd

> >  does not offer certain versions of NFS. The current version of

> >  .B rpc.nfsd

> > -can support major NFS versions 2,3,4 and the minor versions 4.1

> > and 4.2.

> > +can support major NFS versions 2,3,4 and the minor versions 4.0,

> > 4.1 and 4.2.

> >  .TP

> >  .B \-s " or " \-\-syslog

> >  By default,

> > @@ -82,7 +82,7 @@ This option can be used to request that

> >  .B rpc.nfsd

> >  offer certain versions of NFS. The current version of

> >  .B rpc.nfsd

> > -can support major NFS versions 2,3,4 and the minor versions 4.1

> > and 4.2.

> > +can support major NFS versions 2,3,4 and the minor versions 4.0,

> > 4.1 and 4.2.

> >  .TP

> >  .B \-L " or " \-\-lease-time seconds

> >  Set the lease-time used for NFSv4.  This corresponds to how often

> > diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c

> > index 07f6ff1204d1..e8609c15b8e5 100644

> > --- a/utils/nfsd/nfssvc.c

> > +++ b/utils/nfsd/nfssvc.c

> > @@ -330,36 +330,78 @@ nfssvc_set_time(const char *type, const int

> > seconds)

> >  }

> >  

> >  void

> > +nfssvc_get_minormask(unsigned int *mask)

> > +{

> > +	int fd;

> > +	char *ptr = buf;

> > +	ssize_t size;

> > +

> > +	fd = open(NFSD_VERS_FILE, O_RDONLY);

> > +	if (fd < 0)

> > +		return;

> > +

> > +	size = read(fd, buf, sizeof(buf));

> > +	if (size < 0) {

> > +		xlog(L_ERROR, "Getting versions failed: errno %d

> > (%m)", errno);

> > +		goto out;

> > +	}

> > +	ptr[size] = '\0';

> > +	for (;;) {

> > +		unsigned vers, minor = 0;

> > +		char *token = strtok(ptr, " ");

> > +

> > +		if (!token)

> > +			break;

> > +		ptr = NULL;

> > +		if (*token != '+' && *token != '-')

> > +			continue;

> > +		if (sscanf(++token, "%u.%u", &vers, &minor) > 0 &&

> > +		    vers == 4 && minor <= NFS4_MAXMINOR)

> > +			NFSCTL_MINORSET(*mask, minor);

> > +	}

> > +out:

> > +	close(fd);

> > +	return;

> > +}

> > +

> > +static int

> > +nfssvc_print_vers(char *ptr, unsigned size, unsigned vers,

> > unsigned minorvers,

> > +		int isset)

> > +{

> > +	char sign = isset ? '+' : '-';

> > +	if (minorvers == 0)

> > +		return snprintf(ptr, size, "%c%u ", sign, vers);

> > +	return snprintf(ptr, size, "%c%u.%u ", sign, vers,

> > minorvers);

> > +}

> > +

> > +void

> >  nfssvc_setvers(unsigned int ctlbits, unsigned int minorvers,

> > unsigned int minorversset)

> >  {

> >  	int fd, n, off;

> > -	char *ptr;

> >  

> > -	ptr = buf;

> >  	off = 0;

> >  	fd = open(NFSD_VERS_FILE, O_WRONLY);

> >  	if (fd < 0)

> >  		return;

> >  

> > -	for (n = NFS4_MINMINOR; n <= NFS4_MAXMINOR; n++) {

> > -		if (NFSCTL_VERISSET(minorversset, n)) {

> > -			if (NFSCTL_VERISSET(minorvers, n))

> > -				off += snprintf(ptr+off,

> > sizeof(buf) - off, "+4.%d ", n);

> > -			else

> > -				off += snprintf(ptr+off,

> > sizeof(buf) - off, "-4.%d ", n);

> > -		}

> > -	}

> > -	for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) {

> > -		if (NFSCTL_VERISSET(ctlbits, n))

> > -		    off += snprintf(ptr+off, sizeof(buf) - off,

> > "+%d ", n);

> > -		else

> > -		    off += snprintf(ptr+off, sizeof(buf) - off, "-

> > %d ", n);

> > +	for (n = NFSD_MINVERS; n <= ((NFSD_MAXVERS < 3) ?

> > NFSD_MAXVERS : 3); n++)

> > +		off += nfssvc_print_vers(&buf[off], sizeof(buf) -

> > off,

> > +				n, 0, NFSCTL_VERISSET(ctlbits,

> > n));

> > +

> > +	for (n = 0; n <= NFS4_MAXMINOR; n++) {

> > +		if (!NFSCTL_MINORISSET(minorversset, n))

> > +			continue;

> > +		off += nfssvc_print_vers(&buf[off], sizeof(buf) -

> > off,

> > +				4, n, NFSCTL_MINORISSET(minorvers,

> > n));

> >  	}

> > +	if (!off--)

> > +		goto out;

> > +	buf[off] = '\0';

> >  	xlog(D_GENERAL, "Writing version string to kernel: %s",

> > buf);

> > -	snprintf(ptr+off, sizeof(buf) - off, "\n");

> > +	snprintf(&buf[off], sizeof(buf) - off, "\n");

> >  	if (write(fd, buf, strlen(buf)) != (ssize_t)strlen(buf))

> >  		xlog(L_ERROR, "Setting version failed: errno %d

> > (%m)", errno);

> > -

> > +out:

> >  	close(fd);

> >  

> >  	return;

> > diff --git a/utils/nfsd/nfssvc.h b/utils/nfsd/nfssvc.h

> > index cd5a7e84dc7e..39ebf37a90fe 100644

> > --- a/utils/nfsd/nfssvc.h

> > +++ b/utils/nfsd/nfssvc.h

> > @@ -28,3 +28,4 @@ void	nfssvc_set_time(const char *type,

> > const int seconds);

> >  int	nfssvc_set_rdmaport(const char *port);

> >  void	nfssvc_setvers(unsigned int ctlbits, unsigned int

> > minorvers4, unsigned int minorvers4set);

> >  int	nfssvc_threads(int nrservs);

> > +void	nfssvc_get_minormask(unsigned int *mask);

> > 

> 

> 

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com
Steve Dickson April 4, 2017, 9:07 p.m. UTC | #3
On 04/04/2017 04:26 PM, Trond Myklebust wrote:
> On Tue, 2017-04-04 at 16:07 -0400, Steve Dickson wrote:
>> Hey Trond,
>>
>> My apologies for taking so long to address this... 
>>
>> On 02/23/2017 07:33 PM, Trond Myklebust wrote:
>>> The new semantic is that '-N4' turns off all NFSv4 minor versions,
>>> while
>>> '-V4' turns them all on. In order to turn off just minor version x
>>> (x >= 0),
>>> use -N4.x, and to turn it back on. '-V4.x'.
>>>
>>> Note that on older kernels, attempting to use -N4.0 and -V4.0 is
>>> equivalent to specifying -N4 or -V4.
>>
>> doing a 
>>
>> nfsd -d -N4.0 -V4.1 -V4.2 
>> nfsd: Writing version string to kernel: -2 +3 -4 +4.1 +4.2
>>
>> does the right thing but when I do a
>>
>> nfsd -d -N4.0 
>> nfsd: Writing version string to kernel: -2 +3 -4
>>
>> It brings down all of the v4 minor versions, Is that
>> intentional? It seems to me doing a -N4.0 should only
>> stop 4.0 from coming up not v4.1 or v4.2
> 
> That is unfortunately not possible for older kernels. They lack the
> kernel API to turn off NFSv4.0 only. We could perhaps try to return an
> error if you were to specify these flags for those kernels, but that
> would require us to hard-code a "minimal" kernel version in nfs-utils.
Well we already do that in the legacy mounting code to figure out
what mount version to use, but looking at that code it would be pain
to pull it out. 

But I'm thinking the expectation of nfsd -N4.0 is to only 
turn off v4.0 and not work the same as -N4 which turns all of
v4 off which is expected. 

When you say old kernel... How old?

steved.

> 
>>
>> steved.
>>  
>>>
>>> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
>>> ---
>>>  support/include/nfs/nfs.h |  7 +++--
>>>  utils/nfsd/nfsd.c         | 39 +++++++++++++++---------
>>>  utils/nfsd/nfsd.man       |  4 +--
>>>  utils/nfsd/nfssvc.c       | 76
>>> ++++++++++++++++++++++++++++++++++++-----------
>>>  utils/nfsd/nfssvc.h       |  1 +
>>>  5 files changed, 92 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
>>> index 15ecc6bfc485..5860343f78b7 100644
>>> --- a/support/include/nfs/nfs.h
>>> +++ b/support/include/nfs/nfs.h
>>> @@ -16,8 +16,8 @@
>>>  #define NFSD_MINVERS 2
>>>  #define NFSD_MAXVERS 4
>>>  
>>> -#define NFS4_MINMINOR 1
>>> -#define NFS4_MAXMINOR WORD_BIT
>>> +#define NFS4_MINMINOR 0
>>> +#define NFS4_MAXMINOR (WORD_BIT-1)
>>>  
>>>  struct nfs_fh_len {
>>>  	int		fh_size;
>>> @@ -29,15 +29,18 @@ struct nfs_fh_len {
>>>  #define NFSCTL_TCPBIT		      (1 << (18 - 1))
>>>  
>>>  #define NFSCTL_VERUNSET(_cltbits, _v) ((_cltbits) &= ~(1 << ((_v)
>>> - 1))) 
>>> +#define NFSCTL_MINORUNSET(_cltbits, _v) ((_cltbits) &= ~(1 <<
>>> (_v)))
>>>  #define NFSCTL_UDPUNSET(_cltbits)     ((_cltbits) &=
>>> ~NFSCTL_UDPBIT) 
>>>  #define NFSCTL_TCPUNSET(_cltbits)     ((_cltbits) &=
>>> ~NFSCTL_TCPBIT) 
>>>  
>>>  #define NFSCTL_VERISSET(_cltbits, _v) ((_cltbits) & (1 << ((_v) -
>>> 1))) 
>>> +#define NFSCTL_MINORISSET(_cltbits, _v) ((_cltbits) & (1 << (_v)))
>>>  #define NFSCTL_UDPISSET(_cltbits)     ((_cltbits) &
>>> NFSCTL_UDPBIT) 
>>>  #define NFSCTL_TCPISSET(_cltbits)     ((_cltbits) &
>>> NFSCTL_TCPBIT) 
>>>  
>>>  #define NFSCTL_VERDEFAULT (0xc)       /* versions 3 and 4 */
>>>  #define NFSCTL_VERSET(_cltbits, _v)   ((_cltbits) |= (1 << ((_v) -
>>> 1))) 
>>> +#define NFSCTL_MINORSET(_cltbits, _v)   ((_cltbits) |= (1 <<
>>> (_v)))
>>>  #define NFSCTL_UDPSET(_cltbits)       ((_cltbits) |=
>>> NFSCTL_UDPBIT)
>>>  #define NFSCTL_TCPSET(_cltbits)       ((_cltbits) |=
>>> NFSCTL_TCPBIT)
>>>  
>>> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
>>> index 20f4b7952203..1708521ab286 100644
>>> --- a/utils/nfsd/nfsd.c
>>> +++ b/utils/nfsd/nfsd.c
>>> @@ -67,6 +67,7 @@ main(int argc, char **argv)
>>>  	int	socket_up = 0;
>>>  	unsigned int minorvers = 0;
>>>  	unsigned int minorversset = 0;
>>> +	unsigned int minormask = 0;
>>>  	unsigned int versbits = NFSCTL_VERDEFAULT;
>>>  	unsigned int protobits = NFSCTL_ALLBITS;
>>>  	int grace = -1;
>>> @@ -104,10 +105,16 @@ main(int argc, char **argv)
>>>  		else
>>>  			NFSCTL_VERUNSET(versbits, i);
>>>  	}
>>> +
>>> +	nfssvc_get_minormask(&minormask);
>>>  	/* We assume the kernel will default all minor versions to
>>> 'on',
>>>  	 * and allow the config file to disable some.
>>>  	 */
>>> -	for (i = NFS4_MINMINOR; i <= NFS4_MAXMINOR; i++) {
>>> +	if (NFSCTL_VERISSET(versbits, 4)) {
>>> +		NFSCTL_MINORSET(minorversset, 0);
>>> +		NFSCTL_MINORSET(minorvers, 0);
>>> +	}
>>> +	for (i = 1; i <= NFS4_MAXMINOR; i++) {
>>>  		char tag[20];
>>>  		sprintf(tag, "vers4.%d", i);
>>>  		/* The default for minor version support is to let
>>> the
>>> @@ -119,12 +126,12 @@ main(int argc, char **argv)
>>>  		 * (i.e. don't set the bit in minorversset).
>>>  		 */
>>>  		if (!conf_get_bool("nfsd", tag, 1)) {
>>> -			NFSCTL_VERSET(minorversset, i);
>>> -			NFSCTL_VERUNSET(minorvers, i);
>>> +			NFSCTL_MINORSET(minorversset, i);
>>> +			NFSCTL_MINORUNSET(minorvers, i);
>>>  		}
>>>  		if (conf_get_bool("nfsd", tag, 0)) {
>>> -			NFSCTL_VERSET(minorversset, i);
>>> -			NFSCTL_VERSET(minorvers, i);
>>> +			NFSCTL_MINORSET(minorversset, i);
>>> +			NFSCTL_MINORSET(minorvers, i);
>>>  		}
>>>  	}
>>>  
>>> @@ -179,13 +186,17 @@ main(int argc, char **argv)
>>>  			case 4:
>>>  				if (*p == '.') {
>>>  					int i = atoi(p+1);
>>> -					if (i < NFS4_MINMINOR || i
>>>> NFS4_MAXMINOR) {
>>> +					if (i < 0 || i >
>>> NFS4_MAXMINOR) {
>>>  						fprintf(stderr,
>>> "%s: unsupported minor version\n", optarg);
>>>  						exit(1);
>>>  					}
>>> -					NFSCTL_VERSET(minorversset
>>> , i);
>>> -					NFSCTL_VERUNSET(minorvers,
>>> i);
>>> -					break;
>>> +					NFSCTL_MINORSET(minorverss
>>> et, i);
>>> +					NFSCTL_MINORUNSET(minorver
>>> s, i);
>>> +					if (minorvers != 0)
>>> +						break;
>>> +				} else {
>>> +					minorvers = 0;
>>> +					minorversset = minormask;
>>>  				}
>>>  			case 3:
>>>  			case 2:
>>> @@ -201,14 +212,14 @@ main(int argc, char **argv)
>>>  			case 4:
>>>  				if (*p == '.') {
>>>  					int i = atoi(p+1);
>>> -					if (i < NFS4_MINMINOR || i
>>>> NFS4_MAXMINOR) {
>>> +					if (i < 0 || i >
>>> NFS4_MAXMINOR) {
>>>  						fprintf(stderr,
>>> "%s: unsupported minor version\n", optarg);
>>>  						exit(1);
>>>  					}
>>> -					NFSCTL_VERSET(minorversset
>>> , i);
>>> -					NFSCTL_VERSET(minorvers,
>>> i);
>>> -					break;
>>> -				}
>>> +					NFSCTL_MINORSET(minorverss
>>> et, i);
>>> +					NFSCTL_MINORSET(minorvers,
>>> i);
>>> +				} else
>>> +					minorvers = minorversset =
>>> minormask;
>>>  			case 3:
>>>  			case 2:
>>>  				NFSCTL_VERSET(versbits, c);
>>> diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
>>> index 8901fb6c6872..0d797fd2ec8d 100644
>>> --- a/utils/nfsd/nfsd.man
>>> +++ b/utils/nfsd/nfsd.man
>>> @@ -57,7 +57,7 @@ This option can be used to request that
>>>  .B rpc.nfsd
>>>  does not offer certain versions of NFS. The current version of
>>>  .B rpc.nfsd
>>> -can support major NFS versions 2,3,4 and the minor versions 4.1
>>> and 4.2.
>>> +can support major NFS versions 2,3,4 and the minor versions 4.0,
>>> 4.1 and 4.2.
>>>  .TP
>>>  .B \-s " or " \-\-syslog
>>>  By default,
>>> @@ -82,7 +82,7 @@ This option can be used to request that
>>>  .B rpc.nfsd
>>>  offer certain versions of NFS. The current version of
>>>  .B rpc.nfsd
>>> -can support major NFS versions 2,3,4 and the minor versions 4.1
>>> and 4.2.
>>> +can support major NFS versions 2,3,4 and the minor versions 4.0,
>>> 4.1 and 4.2.
>>>  .TP
>>>  .B \-L " or " \-\-lease-time seconds
>>>  Set the lease-time used for NFSv4.  This corresponds to how often
>>> diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
>>> index 07f6ff1204d1..e8609c15b8e5 100644
>>> --- a/utils/nfsd/nfssvc.c
>>> +++ b/utils/nfsd/nfssvc.c
>>> @@ -330,36 +330,78 @@ nfssvc_set_time(const char *type, const int
>>> seconds)
>>>  }
>>>  
>>>  void
>>> +nfssvc_get_minormask(unsigned int *mask)
>>> +{
>>> +	int fd;
>>> +	char *ptr = buf;
>>> +	ssize_t size;
>>> +
>>> +	fd = open(NFSD_VERS_FILE, O_RDONLY);
>>> +	if (fd < 0)
>>> +		return;
>>> +
>>> +	size = read(fd, buf, sizeof(buf));
>>> +	if (size < 0) {
>>> +		xlog(L_ERROR, "Getting versions failed: errno %d
>>> (%m)", errno);
>>> +		goto out;
>>> +	}
>>> +	ptr[size] = '\0';
>>> +	for (;;) {
>>> +		unsigned vers, minor = 0;
>>> +		char *token = strtok(ptr, " ");
>>> +
>>> +		if (!token)
>>> +			break;
>>> +		ptr = NULL;
>>> +		if (*token != '+' && *token != '-')
>>> +			continue;
>>> +		if (sscanf(++token, "%u.%u", &vers, &minor) > 0 &&
>>> +		    vers == 4 && minor <= NFS4_MAXMINOR)
>>> +			NFSCTL_MINORSET(*mask, minor);
>>> +	}
>>> +out:
>>> +	close(fd);
>>> +	return;
>>> +}
>>> +
>>> +static int
>>> +nfssvc_print_vers(char *ptr, unsigned size, unsigned vers,
>>> unsigned minorvers,
>>> +		int isset)
>>> +{
>>> +	char sign = isset ? '+' : '-';
>>> +	if (minorvers == 0)
>>> +		return snprintf(ptr, size, "%c%u ", sign, vers);
>>> +	return snprintf(ptr, size, "%c%u.%u ", sign, vers,
>>> minorvers);
>>> +}
>>> +
>>> +void
>>>  nfssvc_setvers(unsigned int ctlbits, unsigned int minorvers,
>>> unsigned int minorversset)
>>>  {
>>>  	int fd, n, off;
>>> -	char *ptr;
>>>  
>>> -	ptr = buf;
>>>  	off = 0;
>>>  	fd = open(NFSD_VERS_FILE, O_WRONLY);
>>>  	if (fd < 0)
>>>  		return;
>>>  
>>> -	for (n = NFS4_MINMINOR; n <= NFS4_MAXMINOR; n++) {
>>> -		if (NFSCTL_VERISSET(minorversset, n)) {
>>> -			if (NFSCTL_VERISSET(minorvers, n))
>>> -				off += snprintf(ptr+off,
>>> sizeof(buf) - off, "+4.%d ", n);
>>> -			else
>>> -				off += snprintf(ptr+off,
>>> sizeof(buf) - off, "-4.%d ", n);
>>> -		}
>>> -	}
>>> -	for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) {
>>> -		if (NFSCTL_VERISSET(ctlbits, n))
>>> -		    off += snprintf(ptr+off, sizeof(buf) - off,
>>> "+%d ", n);
>>> -		else
>>> -		    off += snprintf(ptr+off, sizeof(buf) - off, "-
>>> %d ", n);
>>> +	for (n = NFSD_MINVERS; n <= ((NFSD_MAXVERS < 3) ?
>>> NFSD_MAXVERS : 3); n++)
>>> +		off += nfssvc_print_vers(&buf[off], sizeof(buf) -
>>> off,
>>> +				n, 0, NFSCTL_VERISSET(ctlbits,
>>> n));
>>> +
>>> +	for (n = 0; n <= NFS4_MAXMINOR; n++) {
>>> +		if (!NFSCTL_MINORISSET(minorversset, n))
>>> +			continue;
>>> +		off += nfssvc_print_vers(&buf[off], sizeof(buf) -
>>> off,
>>> +				4, n, NFSCTL_MINORISSET(minorvers,
>>> n));
>>>  	}
>>> +	if (!off--)
>>> +		goto out;
>>> +	buf[off] = '\0';
>>>  	xlog(D_GENERAL, "Writing version string to kernel: %s",
>>> buf);
>>> -	snprintf(ptr+off, sizeof(buf) - off, "\n");
>>> +	snprintf(&buf[off], sizeof(buf) - off, "\n");
>>>  	if (write(fd, buf, strlen(buf)) != (ssize_t)strlen(buf))
>>>  		xlog(L_ERROR, "Setting version failed: errno %d
>>> (%m)", errno);
>>> -
>>> +out:
>>>  	close(fd);
>>>  
>>>  	return;
>>> diff --git a/utils/nfsd/nfssvc.h b/utils/nfsd/nfssvc.h
>>> index cd5a7e84dc7e..39ebf37a90fe 100644
>>> --- a/utils/nfsd/nfssvc.h
>>> +++ b/utils/nfsd/nfssvc.h
>>> @@ -28,3 +28,4 @@ void	nfssvc_set_time(const char *type,
>>> const int seconds);
>>>  int	nfssvc_set_rdmaport(const char *port);
>>>  void	nfssvc_setvers(unsigned int ctlbits, unsigned int
>>> minorvers4, unsigned int minorvers4set);
>>>  int	nfssvc_threads(int nrservs);
>>> +void	nfssvc_get_minormask(unsigned int *mask);
>>>
>>
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve Dickson April 4, 2017, 9:31 p.m. UTC | #4
On 04/04/2017 05:11 PM, Trond Myklebust wrote:
> 
>> On Apr 4, 2017, at 17:07, Steve Dickson <SteveD@RedHat.com <mailto:SteveD@RedHat.com>> wrote:
>>
>>
>>
>> On 04/04/2017 04:26 PM, Trond Myklebust wrote:
>>> On Tue, 2017-04-04 at 16:07 -0400, Steve Dickson wrote:
>>>> Hey Trond,
>>>>
>>>> My apologies for taking so long to address this... 
>>>>
>>>> On 02/23/2017 07:33 PM, Trond Myklebust wrote:
>>>>> The new semantic is that '-N4' turns off all NFSv4 minor versions,
>>>>> while
>>>>> '-V4' turns them all on. In order to turn off just minor version x
>>>>> (x >= 0),
>>>>> use -N4.x, and to turn it back on. '-V4.x'.
>>>>>
>>>>> Note that on older kernels, attempting to use -N4.0 and -V4.0 is
>>>>> equivalent to specifying -N4 or -V4.
>>>>
>>>> doing a 
>>>>
>>>> nfsd -d -N4.0 -V4.1 -V4.2 
>>>> nfsd: Writing version string to kernel: -2 +3 -4 +4.1 +4.2
>>>>
>>>> does the right thing but when I do a
>>>>
>>>> nfsd -d -N4.0 
>>>> nfsd: Writing version string to kernel: -2 +3 -4
>>>>
>>>> It brings down all of the v4 minor versions, Is that
>>>> intentional? It seems to me doing a -N4.0 should only
>>>> stop 4.0 from coming up not v4.1 or v4.2
>>>
>>> That is unfortunately not possible for older kernels. They lack the
>>> kernel API to turn off NFSv4.0 only. We could perhaps try to return an
>>> error if you were to specify these flags for those kernels, but that
>>> would require us to hard-code a "minimal" kernel version in nfs-utils.
>> Well we already do that in the legacy mounting code to figure out
>> what mount version to use, but looking at that code it would be pain
>> to pull it out. 
>>
>> But I'm thinking the expectation of nfsd -N4.0 is to only 
>> turn off v4.0 and not work the same as -N4 which turns all of
>> v4 off which is expected. 
>>
>> When you say old kernel... How old?
> 
> Linux 4.10 or older.
So what will happen when this is done
    nfsd -d -N4.0 -V4.1 -V4.2
on an older kernel?

Also looking at the code when -N4.0 is used
it zero out the entire minorversset which means 
the -4.1 and -4.2 is not written out to the versions file

nfsd -d -N4
nfsd: Writing version string to kernel: -2 +3 -4 -4.1 -4.2

nfsd -d -N4.0
nfsd: Writing version string to kernel: -2 +3 -4

I'm not sure if this matters or not... but it is different.

steved.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve Dickson April 5, 2017, 5:29 p.m. UTC | #5
On 04/04/2017 05:31 PM, Steve Dickson wrote:
>>> But I'm thinking the expectation of nfsd -N4.0 is to only 
>>> turn off v4.0 and not work the same as -N4 which turns all of
>>> v4 off which is expected. 
>>>
>>> When you say old kernel... How old?
>> Linux 4.10 or older.
> So what will happen when this is done
>     nfsd -d -N4.0 -V4.1 -V4.2
> on an older kernel?
On an older kernel this turns off all v4 minor version.
Plus on older kernels you can turn v4.1 and v4.2 on
and off but as soon as v4 or v4.0 is turned off
all versions are off...

With newer kernels things work as expected. 

Committed... 

steved.

> 
> Also looking at the code when -N4.0 is used
> it zero out the entire minorversset which means 
> the -4.1 and -4.2 is not written out to the versions file
> 
> nfsd -d -N4
> nfsd: Writing version string to kernel: -2 +3 -4 -4.1 -4.2
> 
> nfsd -d -N4.0
> nfsd: Writing version string to kernel: -2 +3 -4
> 
> I'm not sure if this matters or not... but it is different.
> 
> steved.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
index 15ecc6bfc485..5860343f78b7 100644
--- a/support/include/nfs/nfs.h
+++ b/support/include/nfs/nfs.h
@@ -16,8 +16,8 @@ 
 #define NFSD_MINVERS 2
 #define NFSD_MAXVERS 4
 
-#define NFS4_MINMINOR 1
-#define NFS4_MAXMINOR WORD_BIT
+#define NFS4_MINMINOR 0
+#define NFS4_MAXMINOR (WORD_BIT-1)
 
 struct nfs_fh_len {
 	int		fh_size;
@@ -29,15 +29,18 @@  struct nfs_fh_len {
 #define NFSCTL_TCPBIT		      (1 << (18 - 1))
 
 #define NFSCTL_VERUNSET(_cltbits, _v) ((_cltbits) &= ~(1 << ((_v) - 1))) 
+#define NFSCTL_MINORUNSET(_cltbits, _v) ((_cltbits) &= ~(1 << (_v)))
 #define NFSCTL_UDPUNSET(_cltbits)     ((_cltbits) &= ~NFSCTL_UDPBIT) 
 #define NFSCTL_TCPUNSET(_cltbits)     ((_cltbits) &= ~NFSCTL_TCPBIT) 
 
 #define NFSCTL_VERISSET(_cltbits, _v) ((_cltbits) & (1 << ((_v) - 1))) 
+#define NFSCTL_MINORISSET(_cltbits, _v) ((_cltbits) & (1 << (_v)))
 #define NFSCTL_UDPISSET(_cltbits)     ((_cltbits) & NFSCTL_UDPBIT) 
 #define NFSCTL_TCPISSET(_cltbits)     ((_cltbits) & NFSCTL_TCPBIT) 
 
 #define NFSCTL_VERDEFAULT (0xc)       /* versions 3 and 4 */
 #define NFSCTL_VERSET(_cltbits, _v)   ((_cltbits) |= (1 << ((_v) - 1))) 
+#define NFSCTL_MINORSET(_cltbits, _v)   ((_cltbits) |= (1 << (_v)))
 #define NFSCTL_UDPSET(_cltbits)       ((_cltbits) |= NFSCTL_UDPBIT)
 #define NFSCTL_TCPSET(_cltbits)       ((_cltbits) |= NFSCTL_TCPBIT)
 
diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index 20f4b7952203..1708521ab286 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -67,6 +67,7 @@  main(int argc, char **argv)
 	int	socket_up = 0;
 	unsigned int minorvers = 0;
 	unsigned int minorversset = 0;
+	unsigned int minormask = 0;
 	unsigned int versbits = NFSCTL_VERDEFAULT;
 	unsigned int protobits = NFSCTL_ALLBITS;
 	int grace = -1;
@@ -104,10 +105,16 @@  main(int argc, char **argv)
 		else
 			NFSCTL_VERUNSET(versbits, i);
 	}
+
+	nfssvc_get_minormask(&minormask);
 	/* We assume the kernel will default all minor versions to 'on',
 	 * and allow the config file to disable some.
 	 */
-	for (i = NFS4_MINMINOR; i <= NFS4_MAXMINOR; i++) {
+	if (NFSCTL_VERISSET(versbits, 4)) {
+		NFSCTL_MINORSET(minorversset, 0);
+		NFSCTL_MINORSET(minorvers, 0);
+	}
+	for (i = 1; i <= NFS4_MAXMINOR; i++) {
 		char tag[20];
 		sprintf(tag, "vers4.%d", i);
 		/* The default for minor version support is to let the
@@ -119,12 +126,12 @@  main(int argc, char **argv)
 		 * (i.e. don't set the bit in minorversset).
 		 */
 		if (!conf_get_bool("nfsd", tag, 1)) {
-			NFSCTL_VERSET(minorversset, i);
-			NFSCTL_VERUNSET(minorvers, i);
+			NFSCTL_MINORSET(minorversset, i);
+			NFSCTL_MINORUNSET(minorvers, i);
 		}
 		if (conf_get_bool("nfsd", tag, 0)) {
-			NFSCTL_VERSET(minorversset, i);
-			NFSCTL_VERSET(minorvers, i);
+			NFSCTL_MINORSET(minorversset, i);
+			NFSCTL_MINORSET(minorvers, i);
 		}
 	}
 
@@ -179,13 +186,17 @@  main(int argc, char **argv)
 			case 4:
 				if (*p == '.') {
 					int i = atoi(p+1);
-					if (i < NFS4_MINMINOR || i > NFS4_MAXMINOR) {
+					if (i < 0 || i > NFS4_MAXMINOR) {
 						fprintf(stderr, "%s: unsupported minor version\n", optarg);
 						exit(1);
 					}
-					NFSCTL_VERSET(minorversset, i);
-					NFSCTL_VERUNSET(minorvers, i);
-					break;
+					NFSCTL_MINORSET(minorversset, i);
+					NFSCTL_MINORUNSET(minorvers, i);
+					if (minorvers != 0)
+						break;
+				} else {
+					minorvers = 0;
+					minorversset = minormask;
 				}
 			case 3:
 			case 2:
@@ -201,14 +212,14 @@  main(int argc, char **argv)
 			case 4:
 				if (*p == '.') {
 					int i = atoi(p+1);
-					if (i < NFS4_MINMINOR || i > NFS4_MAXMINOR) {
+					if (i < 0 || i > NFS4_MAXMINOR) {
 						fprintf(stderr, "%s: unsupported minor version\n", optarg);
 						exit(1);
 					}
-					NFSCTL_VERSET(minorversset, i);
-					NFSCTL_VERSET(minorvers, i);
-					break;
-				}
+					NFSCTL_MINORSET(minorversset, i);
+					NFSCTL_MINORSET(minorvers, i);
+				} else
+					minorvers = minorversset = minormask;
 			case 3:
 			case 2:
 				NFSCTL_VERSET(versbits, c);
diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
index 8901fb6c6872..0d797fd2ec8d 100644
--- a/utils/nfsd/nfsd.man
+++ b/utils/nfsd/nfsd.man
@@ -57,7 +57,7 @@  This option can be used to request that
 .B rpc.nfsd
 does not offer certain versions of NFS. The current version of
 .B rpc.nfsd
-can support major NFS versions 2,3,4 and the minor versions 4.1 and 4.2.
+can support major NFS versions 2,3,4 and the minor versions 4.0, 4.1 and 4.2.
 .TP
 .B \-s " or " \-\-syslog
 By default,
@@ -82,7 +82,7 @@  This option can be used to request that
 .B rpc.nfsd
 offer certain versions of NFS. The current version of
 .B rpc.nfsd
-can support major NFS versions 2,3,4 and the minor versions 4.1 and 4.2.
+can support major NFS versions 2,3,4 and the minor versions 4.0, 4.1 and 4.2.
 .TP
 .B \-L " or " \-\-lease-time seconds
 Set the lease-time used for NFSv4.  This corresponds to how often
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 07f6ff1204d1..e8609c15b8e5 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -330,36 +330,78 @@  nfssvc_set_time(const char *type, const int seconds)
 }
 
 void
+nfssvc_get_minormask(unsigned int *mask)
+{
+	int fd;
+	char *ptr = buf;
+	ssize_t size;
+
+	fd = open(NFSD_VERS_FILE, O_RDONLY);
+	if (fd < 0)
+		return;
+
+	size = read(fd, buf, sizeof(buf));
+	if (size < 0) {
+		xlog(L_ERROR, "Getting versions failed: errno %d (%m)", errno);
+		goto out;
+	}
+	ptr[size] = '\0';
+	for (;;) {
+		unsigned vers, minor = 0;
+		char *token = strtok(ptr, " ");
+
+		if (!token)
+			break;
+		ptr = NULL;
+		if (*token != '+' && *token != '-')
+			continue;
+		if (sscanf(++token, "%u.%u", &vers, &minor) > 0 &&
+		    vers == 4 && minor <= NFS4_MAXMINOR)
+			NFSCTL_MINORSET(*mask, minor);
+	}
+out:
+	close(fd);
+	return;
+}
+
+static int
+nfssvc_print_vers(char *ptr, unsigned size, unsigned vers, unsigned minorvers,
+		int isset)
+{
+	char sign = isset ? '+' : '-';
+	if (minorvers == 0)
+		return snprintf(ptr, size, "%c%u ", sign, vers);
+	return snprintf(ptr, size, "%c%u.%u ", sign, vers, minorvers);
+}
+
+void
 nfssvc_setvers(unsigned int ctlbits, unsigned int minorvers, unsigned int minorversset)
 {
 	int fd, n, off;
-	char *ptr;
 
-	ptr = buf;
 	off = 0;
 	fd = open(NFSD_VERS_FILE, O_WRONLY);
 	if (fd < 0)
 		return;
 
-	for (n = NFS4_MINMINOR; n <= NFS4_MAXMINOR; n++) {
-		if (NFSCTL_VERISSET(minorversset, n)) {
-			if (NFSCTL_VERISSET(minorvers, n))
-				off += snprintf(ptr+off, sizeof(buf) - off, "+4.%d ", n);
-			else
-				off += snprintf(ptr+off, sizeof(buf) - off, "-4.%d ", n);
-		}
-	}
-	for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) {
-		if (NFSCTL_VERISSET(ctlbits, n))
-		    off += snprintf(ptr+off, sizeof(buf) - off, "+%d ", n);
-		else
-		    off += snprintf(ptr+off, sizeof(buf) - off, "-%d ", n);
+	for (n = NFSD_MINVERS; n <= ((NFSD_MAXVERS < 3) ? NFSD_MAXVERS : 3); n++)
+		off += nfssvc_print_vers(&buf[off], sizeof(buf) - off,
+				n, 0, NFSCTL_VERISSET(ctlbits, n));
+
+	for (n = 0; n <= NFS4_MAXMINOR; n++) {
+		if (!NFSCTL_MINORISSET(minorversset, n))
+			continue;
+		off += nfssvc_print_vers(&buf[off], sizeof(buf) - off,
+				4, n, NFSCTL_MINORISSET(minorvers, n));
 	}
+	if (!off--)
+		goto out;
+	buf[off] = '\0';
 	xlog(D_GENERAL, "Writing version string to kernel: %s", buf);
-	snprintf(ptr+off, sizeof(buf) - off, "\n");
+	snprintf(&buf[off], sizeof(buf) - off, "\n");
 	if (write(fd, buf, strlen(buf)) != (ssize_t)strlen(buf))
 		xlog(L_ERROR, "Setting version failed: errno %d (%m)", errno);
-
+out:
 	close(fd);
 
 	return;
diff --git a/utils/nfsd/nfssvc.h b/utils/nfsd/nfssvc.h
index cd5a7e84dc7e..39ebf37a90fe 100644
--- a/utils/nfsd/nfssvc.h
+++ b/utils/nfsd/nfssvc.h
@@ -28,3 +28,4 @@  void	nfssvc_set_time(const char *type, const int seconds);
 int	nfssvc_set_rdmaport(const char *port);
 void	nfssvc_setvers(unsigned int ctlbits, unsigned int minorvers4, unsigned int minorvers4set);
 int	nfssvc_threads(int nrservs);
+void	nfssvc_get_minormask(unsigned int *mask);