[PATCH 1/2] rpc.nfsd: No longer advertise NFS v2 support.

[PATCH 1/2] rpc.nfsd: No longer advertise NFS v2 support.

[Steve Dickson]

Changed the default protocol versions that rpc.nfsd
register with rpcbind to just 3 and 4. Version 2
can still be enabled with the '-V' flag, but it
will not be on by default.

Signed-off-by: Steve Dickson <steved@redhat.com>
---
 support/include/nfs/nfs.h | 1 +
 utils/nfsd/nfsd.c         | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
index 174c2dd..5fd86f6 100644
--- a/support/include/nfs/nfs.h
+++ b/support/include/nfs/nfs.h
@@ -52,6 +52,7 @@ struct nfs_fh_old {
 #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_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 e87c0a9..df48392 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -100,7 +100,7 @@ main(int argc, char **argv)
 	char *haddr = NULL;
 	int	socket_up = 0;
 	int minorvers41 = 0;	/* nfsv4 minor version */
-	unsigned int versbits = NFSCTL_ALLBITS;
+	unsigned int versbits = NFSCTL_VERDEFAULT;
 	unsigned int protobits = NFSCTL_ALLBITS;
 	unsigned int proto4 = 0;
 	unsigned int proto6 = 0;
-- 
1.8.1.4

[PATCH 2/2] rpc.nfsd: Allow v4.2 server support with the -V option

[Steve Dickson]

Signed-off-by: Steve Dickson <steved@redhat.com>
---
 support/include/nfs/nfs.h |  4 ++++
 utils/nfsd/nfsd.c         | 12 ++++++------
 utils/nfsd/nfssvc.c       | 11 +++++++----
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
index 5fd86f6..38db5b5 100644
--- a/support/include/nfs/nfs.h
+++ b/support/include/nfs/nfs.h
@@ -15,6 +15,10 @@
 #define NFSD_MINVERS 2
 #define NFSD_MAXVERS 4
 
+#define NFS4_MINMINOR 1
+#define NFS4_MAXMINOR 2
+#define NFS4_VERDEFAULT  0x1  /* minor verion 1 */
+
 struct nfs_fh_len {
 	int		fh_size;
 	u_int8_t	fh_handle[NFS3_FHSIZE];
diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index df48392..6db92f0 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -99,7 +99,7 @@ main(int argc, char **argv)
 	char *p, *progname, *port;
 	char *haddr = NULL;
 	int	socket_up = 0;
-	int minorvers41 = 0;	/* nfsv4 minor version */
+	int minorvers = NFS4_VERDEFAULT;	/* nfsv4 minor version */
 	unsigned int versbits = NFSCTL_VERDEFAULT;
 	unsigned int protobits = NFSCTL_ALLBITS;
 	unsigned int proto4 = 0;
@@ -160,11 +160,11 @@ main(int argc, char **argv)
 			case 4:
 				if (*p == '.') {
 					int i = atoi(p+1);
-					if (i != 1) {
+					if (i > 2) {
 						fprintf(stderr, "%s: unsupported minor version\n", optarg);
 						exit(1);
 					}
-					minorvers41 = -1;
+					NFSCTL_VERUNSET(minorvers, i);
 					break;
 				}
 			case 3:
@@ -181,11 +181,11 @@ main(int argc, char **argv)
 			case 4:
 				if (*p == '.') {
 					int i = atoi(p+1);
-					if (i != 1) {
+					if (i > 2) {
 						fprintf(stderr, "%s: unsupported minor version\n", optarg);
 						exit(1);
 					}
-					minorvers41 = 1;
+					NFSCTL_VERSET(minorvers, i);
 					break;
 				}
 			case 3:
@@ -282,7 +282,7 @@ main(int argc, char **argv)
 	 * registered with rpcbind. Note that on older kernels w/o the right
 	 * interfaces, these are a no-op.
 	 */
-	nfssvc_setvers(versbits, minorvers41);
+	nfssvc_setvers(versbits, minorvers);
  
 	error = nfssvc_set_sockets(AF_INET, proto4, haddr, port);
 	if (!error)
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 683008e..8b85846 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -269,7 +269,7 @@ nfssvc_set_sockets(const int family, const unsigned int protobits,
 }
 
 void
-nfssvc_setvers(unsigned int ctlbits, int minorvers41)
+nfssvc_setvers(unsigned int ctlbits, int minorvers)
 {
 	int fd, n, off;
 	char *ptr;
@@ -280,9 +280,12 @@ nfssvc_setvers(unsigned int ctlbits, int minorvers41)
 	if (fd < 0)
 		return;
 
-	if (minorvers41)
-		off += snprintf(ptr+off, sizeof(buf) - off, "%c4.1",
-				minorvers41 > 0 ? '+' : '-');
+	for (n = NFS4_MINMINOR; n <= NFS4_MAXMINOR; 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);
-- 
1.8.1.4

Re: [PATCH 1/2] rpc.nfsd: No longer advertise NFS v2 support.

[J. Bruce Fields]

On Sun, Jul 28, 2013 at 02:36:37PM -0400, Steve Dickson wrote:
> Changed the default protocol versions that rpc.nfsd
> register with rpcbind to just 3 and 4. Version 2
> can still be enabled with the '-V' flag, but it
> will not be on by default.

OK, I think that's a first reasonable step--ACK.

--b.

> 
> Signed-off-by: Steve Dickson <steved@redhat.com>
> ---
>  support/include/nfs/nfs.h | 1 +
>  utils/nfsd/nfsd.c         | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
> index 174c2dd..5fd86f6 100644
> --- a/support/include/nfs/nfs.h
> +++ b/support/include/nfs/nfs.h
> @@ -52,6 +52,7 @@ struct nfs_fh_old {
>  #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_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 e87c0a9..df48392 100644
> --- a/utils/nfsd/nfsd.c
> +++ b/utils/nfsd/nfsd.c
> @@ -100,7 +100,7 @@ main(int argc, char **argv)
>  	char *haddr = NULL;
>  	int	socket_up = 0;
>  	int minorvers41 = 0;	/* nfsv4 minor version */
> -	unsigned int versbits = NFSCTL_ALLBITS;
> +	unsigned int versbits = NFSCTL_VERDEFAULT;
>  	unsigned int protobits = NFSCTL_ALLBITS;
>  	unsigned int proto4 = 0;
>  	unsigned int proto6 = 0;
> -- 
> 1.8.1.4
> 
> --
> 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

Re: [PATCH 2/2] rpc.nfsd: Allow v4.2 server support with the -V option

[J. Bruce Fields]

On Sun, Jul 28, 2013 at 02:36:38PM -0400, Steve Dickson wrote:
> Signed-off-by: Steve Dickson <steved@redhat.com>

I agree that we want to be able to turn on 4.2 support with -V, but this
does more than that:

> -	if (minorvers41)
> -		off += snprintf(ptr+off, sizeof(buf) - off, "%c4.1",
> -				minorvers41 > 0 ? '+' : '-');
> +	for (n = NFS4_MINMINOR; n <= NFS4_MAXMINOR; 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);

Previously minorvers41 could be set to:

	 1: turn on 4.1
	-1: turn off 4.1
	 0: keep kernel default (user didn't specify)

This patch removes the "0" case, which I liked.

--b.

Re: [PATCH 2/2] rpc.nfsd: Allow v4.2 server support with the -V option

[Myklebust, Trond]

T24gVHVlLCAyMDEzLTA3LTMwIGF0IDExOjU5IC0wNDAwLCBKLiBCcnVjZSBGaWVsZHMgd3JvdGU6
DQo+IE9uIFN1biwgSnVsIDI4LCAyMDEzIGF0IDAyOjM2OjM4UE0gLTA0MDAsIFN0ZXZlIERpY2tz
b24gd3JvdGU6DQo+ID4gU2lnbmVkLW9mZi1ieTogU3RldmUgRGlja3NvbiA8c3RldmVkQHJlZGhh
dC5jb20+DQo+IA0KPiBJIGFncmVlIHRoYXQgd2Ugd2FudCB0byBiZSBhYmxlIHRvIHR1cm4gb24g
NC4yIHN1cHBvcnQgd2l0aCAtViwgYnV0IHRoaXMNCj4gZG9lcyBtb3JlIHRoYW4gdGhhdDoNCj4g
DQo+ID4gLQlpZiAobWlub3J2ZXJzNDEpDQo+ID4gLQkJb2ZmICs9IHNucHJpbnRmKHB0citvZmYs
IHNpemVvZihidWYpIC0gb2ZmLCAiJWM0LjEiLA0KPiA+IC0JCQkJbWlub3J2ZXJzNDEgPiAwID8g
JysnIDogJy0nKTsNCj4gPiArCWZvciAobiA9IE5GUzRfTUlOTUlOT1I7IG4gPD0gTkZTNF9NQVhN
SU5PUjsgbisrKSB7DQo+ID4gKwkJaWYgKE5GU0NUTF9WRVJJU1NFVChtaW5vcnZlcnMsIG4pKSAN
Cj4gPiArCQkJb2ZmICs9IHNucHJpbnRmKHB0citvZmYsIHNpemVvZihidWYpIC0gb2ZmLCAiKzQu
JWQgIiwgbik7DQo+ID4gKwkJZWxzZQkJCQ0KPiA+ICsJCQlvZmYgKz0gc25wcmludGYocHRyK29m
Ziwgc2l6ZW9mKGJ1ZikgLSBvZmYsICItNC4lZCAiLCBuKTsNCj4gPiArCX0NCj4gPiAgCWZvciAo
biA9IE5GU0RfTUlOVkVSUzsgbiA8PSBORlNEX01BWFZFUlM7IG4rKykgew0KPiA+ICAJCWlmIChO
RlNDVExfVkVSSVNTRVQoY3RsYml0cywgbikpDQo+ID4gIAkJICAgIG9mZiArPSBzbnByaW50Zihw
dHIrb2ZmLCBzaXplb2YoYnVmKSAtIG9mZiwgIislZCAiLCBuKTsNCj4gDQo+IFByZXZpb3VzbHkg
bWlub3J2ZXJzNDEgY291bGQgYmUgc2V0IHRvOg0KPiANCj4gCSAxOiB0dXJuIG9uIDQuMQ0KPiAJ
LTE6IHR1cm4gb2ZmIDQuMQ0KPiAJIDA6IGtlZXAga2VybmVsIGRlZmF1bHQgKHVzZXIgZGlkbid0
IHNwZWNpZnkpDQo+IA0KPiBUaGlzIHBhdGNoIHJlbW92ZXMgdGhlICIwIiBjYXNlLCB3aGljaCBJ
IGxpa2VkLg0KDQpXaHkgc2hvdWxkIE5GU3Y0IG1pbm9yIHZlcnNpb25zIGJlIHRyZWF0ZWQgYW55
IGRpZmZlcmVudGx5IGZyb20gbWFqb3INCnZlcnNpb25zIGhlcmU/IFdlIGRvbid0IGhhdmUgYSAn
a2VybmVsIGRlZmF1bHQnIGZvciBORlN2MiBvciB2MyBvciB2NC4wLg0KDQotLSANClRyb25kIE15
a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlr
bGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQo=

Re: [PATCH 2/2] rpc.nfsd: Allow v4.2 server support with the -V option

[J. Bruce Fields]

On Tue, Jul 30, 2013 at 04:09:06PM +0000, Myklebust, Trond wrote:
> On Tue, 2013-07-30 at 11:59 -0400, J. Bruce Fields wrote:
> > On Sun, Jul 28, 2013 at 02:36:38PM -0400, Steve Dickson wrote:
> > > Signed-off-by: Steve Dickson <steved@redhat.com>
> > 
> > I agree that we want to be able to turn on 4.2 support with -V, but this
> > does more than that:
> > 
> > > -	if (minorvers41)
> > > -		off += snprintf(ptr+off, sizeof(buf) - off, "%c4.1",
> > > -				minorvers41 > 0 ? '+' : '-');
> > > +	for (n = NFS4_MINMINOR; n <= NFS4_MAXMINOR; 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);
> > 
> > Previously minorvers41 could be set to:
> > 
> > 	 1: turn on 4.1
> > 	-1: turn off 4.1
> > 	 0: keep kernel default (user didn't specify)
> > 
> > This patch removes the "0" case, which I liked.
> 
> Why should NFSv4 minor versions be treated any differently from major
> versions here? We don't have a 'kernel default' for NFSv2 or v3 or v4.0.

I think "run the highest minorversion currently considered sufficiently
mature" is a useful option.

I don't know why that wasn't done for earlier versions.

--b.

Re: [PATCH 2/2] rpc.nfsd: Allow v4.2 server support with the -V option

[Steve Dickson]

On 28/07/13 14:36, Steve Dickson wrote:
> Signed-off-by: Steve Dickson <steved@redhat.com>
> ---
>  support/include/nfs/nfs.h |  4 ++++
>  utils/nfsd/nfsd.c         | 12 ++++++------
>  utils/nfsd/nfssvc.c       | 11 +++++++----
>  3 files changed, 17 insertions(+), 10 deletions(-)
Committed...

steved.

> 
> diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
> index 5fd86f6..38db5b5 100644
> --- a/support/include/nfs/nfs.h
> +++ b/support/include/nfs/nfs.h
> @@ -15,6 +15,10 @@
>  #define NFSD_MINVERS 2
>  #define NFSD_MAXVERS 4
>  
> +#define NFS4_MINMINOR 1
> +#define NFS4_MAXMINOR 2
> +#define NFS4_VERDEFAULT  0x1  /* minor verion 1 */
> +
>  struct nfs_fh_len {
>  	int		fh_size;
>  	u_int8_t	fh_handle[NFS3_FHSIZE];
> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> index df48392..6db92f0 100644
> --- a/utils/nfsd/nfsd.c
> +++ b/utils/nfsd/nfsd.c
> @@ -99,7 +99,7 @@ main(int argc, char **argv)
>  	char *p, *progname, *port;
>  	char *haddr = NULL;
>  	int	socket_up = 0;
> -	int minorvers41 = 0;	/* nfsv4 minor version */
> +	int minorvers = NFS4_VERDEFAULT;	/* nfsv4 minor version */
>  	unsigned int versbits = NFSCTL_VERDEFAULT;
>  	unsigned int protobits = NFSCTL_ALLBITS;
>  	unsigned int proto4 = 0;
> @@ -160,11 +160,11 @@ main(int argc, char **argv)
>  			case 4:
>  				if (*p == '.') {
>  					int i = atoi(p+1);
> -					if (i != 1) {
> +					if (i > 2) {
>  						fprintf(stderr, "%s: unsupported minor version\n", optarg);
>  						exit(1);
>  					}
> -					minorvers41 = -1;
> +					NFSCTL_VERUNSET(minorvers, i);
>  					break;
>  				}
>  			case 3:
> @@ -181,11 +181,11 @@ main(int argc, char **argv)
>  			case 4:
>  				if (*p == '.') {
>  					int i = atoi(p+1);
> -					if (i != 1) {
> +					if (i > 2) {
>  						fprintf(stderr, "%s: unsupported minor version\n", optarg);
>  						exit(1);
>  					}
> -					minorvers41 = 1;
> +					NFSCTL_VERSET(minorvers, i);
>  					break;
>  				}
>  			case 3:
> @@ -282,7 +282,7 @@ main(int argc, char **argv)
>  	 * registered with rpcbind. Note that on older kernels w/o the right
>  	 * interfaces, these are a no-op.
>  	 */
> -	nfssvc_setvers(versbits, minorvers41);
> +	nfssvc_setvers(versbits, minorvers);
>   
>  	error = nfssvc_set_sockets(AF_INET, proto4, haddr, port);
>  	if (!error)
> diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
> index 683008e..8b85846 100644
> --- a/utils/nfsd/nfssvc.c
> +++ b/utils/nfsd/nfssvc.c
> @@ -269,7 +269,7 @@ nfssvc_set_sockets(const int family, const unsigned int protobits,
>  }
>  
>  void
> -nfssvc_setvers(unsigned int ctlbits, int minorvers41)
> +nfssvc_setvers(unsigned int ctlbits, int minorvers)
>  {
>  	int fd, n, off;
>  	char *ptr;
> @@ -280,9 +280,12 @@ nfssvc_setvers(unsigned int ctlbits, int minorvers41)
>  	if (fd < 0)
>  		return;
>  
> -	if (minorvers41)
> -		off += snprintf(ptr+off, sizeof(buf) - off, "%c4.1",
> -				minorvers41 > 0 ? '+' : '-');
> +	for (n = NFS4_MINMINOR; n <= NFS4_MAXMINOR; 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);
>