2013-07-28 18:36:41

by Steve Dickson

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

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 <[email protected]>
---
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



2013-07-30 15:51:40

by J. Bruce Fields

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

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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2013-07-30 15:59:27

by J. Bruce Fields

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

On Sun, Jul 28, 2013 at 02:36:38PM -0400, Steve Dickson wrote:
> Signed-off-by: Steve Dickson <[email protected]>

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.

2013-07-30 16:09:08

by Myklebust, Trond

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

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=

2013-07-28 18:36:42

by Steve Dickson

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

Signed-off-by: Steve Dickson <[email protected]>
---
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


2013-07-30 20:30:02

by J. Bruce Fields

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

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 <[email protected]>
> >
> > 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.

2013-08-19 18:26:02

by Steve Dickson

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



On 28/07/13 14:36, Steve Dickson wrote:
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> 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);
>