[PATCH 1/3] NFSv4: Send implementation id with exchange_id

[PATCH 1/3] NFSv4: Send implementation id with exchange_id

[Weston Andros Adamson]

Send the nfs implementation id in EXCHANGE_ID requests unless the module
parameter nfs.send_implementation_id is 0.

This adds a CONFIG variable for the nii_domain that defaults to "kernel.org".

Signed-off-by: Weston Andros Adamson <dros@netapp.com>
---
 Documentation/kernel-parameters.txt |    9 ++++++++
 fs/nfs/Kconfig                      |    8 +++++++
 fs/nfs/nfs4xdr.c                    |   38 +++++++++++++++++++++++++++++++++-
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 1d369c6..7bae0fd 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1678,6 +1678,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			back to using the idmapper.
 			To turn off this behaviour, set the value to '0'.
 
+	nfs.send_implementation_id =
+			[NFSv4.1] Send client implementation identification
+			information in exchange_id requests.
+			If zero, no implementation identification information
+			will be sent.
+			The default is to send the implementation identification
+			information.
+
+
 	nmi_debug=	[KNL,AVR32,SH] Specify one or more actions to take
 			when a NMI is triggered.
 			Format: [state][,regs][,debounce][,die]
diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
index ee86cfc..09f9e38 100644
--- a/fs/nfs/Kconfig
+++ b/fs/nfs/Kconfig
@@ -99,6 +99,14 @@ config PNFS_OBJLAYOUT
 	depends on NFS_FS && NFS_V4_1 && SCSI_OSD_ULD
 	default m
 
+config NFS_V4_1_IMPLEMENTATION_ID_DOMAIN
+	string "NFSv4.1 Implementation ID Domain"
+	depends on NFS_V4_1
+	default "kernel.org"
+	help
+	  This value will be used in EXCHANGE_ID compounds to help identify
+	  the client implementation.
+
 config ROOT_NFS
 	bool "Root file system on NFS"
 	depends on NFS_FS=y && IP_PNP
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index ae78343..70ded94 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -44,6 +44,8 @@
 #include <linux/pagemap.h>
 #include <linux/proc_fs.h>
 #include <linux/kdev_t.h>
+#include <linux/module.h>
+#include <linux/utsname.h>
 #include <linux/sunrpc/clnt.h>
 #include <linux/sunrpc/msg_prot.h>
 #include <linux/sunrpc/gss_api.h>
@@ -271,7 +273,12 @@ static int nfs4_stat_to_errno(int);
 				1 /* flags */ + \
 				1 /* spa_how */ + \
 				0 /* SP4_NONE (for now) */ + \
-				1 /* zero implemetation id array */)
+				1 /* implemetation id array of size 1 */ + \
+				1 /* nii_domain */ + \
+				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \
+				1 /* nii_name */ + \
+				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \
+				3 /* nii_date */)
 #define decode_exchange_id_maxsz (op_decode_hdr_maxsz + \
 				2 /* eir_clientid */ + \
 				1 /* eir_sequenceid */ + \
@@ -838,6 +845,12 @@ const u32 nfs41_maxread_overhead = ((RPC_MAX_HEADER_WITH_AUTH +
 				    XDR_UNIT);
 #endif /* CONFIG_NFS_V4_1 */
 
+static unsigned short send_implementation_id = 1;
+
+module_param(send_implementation_id, ushort, 0644);
+MODULE_PARM_DESC(send_implementation_id,
+		"Send implementation ID with NFSv4.1 exchange_id");
+
 static const umode_t nfs_type2fmt[] = {
 	[NF4BAD] = 0,
 	[NF4REG] = S_IFREG,
@@ -1766,6 +1779,8 @@ static void encode_exchange_id(struct xdr_stream *xdr,
 			       struct compound_hdr *hdr)
 {
 	__be32 *p;
+	char impl_name[260]; /* 4 strs max 64 each, spaces and null */
+	int len = 0;
 
 	p = reserve_space(xdr, 4 + sizeof(args->verifier->data));
 	*p++ = cpu_to_be32(OP_EXCHANGE_ID);
@@ -1776,7 +1791,26 @@ static void encode_exchange_id(struct xdr_stream *xdr,
 	p = reserve_space(xdr, 12);
 	*p++ = cpu_to_be32(args->flags);
 	*p++ = cpu_to_be32(0);	/* zero length state_protect4_a */
-	*p = cpu_to_be32(0);	/* zero length implementation id array */
+
+	if (send_implementation_id)
+		len = snprintf(impl_name, sizeof(impl_name), "%s %s %s %s",
+			       utsname()->sysname, utsname()->release,
+			       utsname()->version, utsname()->machine);
+
+	if (len > 0) {
+		*p = cpu_to_be32(1);	/* implementation id array length=1 */
+
+		encode_string(xdr,
+			strlen(CONFIG_NFS_V4_1_IMPLEMENTATION_ID_DOMAIN),
+			CONFIG_NFS_V4_1_IMPLEMENTATION_ID_DOMAIN);
+		encode_string(xdr, len, impl_name);
+		/* just send zeros for nii_date - the date is in nii_name */
+		p = reserve_space(xdr, 12);
+		p = xdr_encode_hyper(p, 0);
+		*p = cpu_to_be32(0);
+	} else
+		*p = cpu_to_be32(0);	/* implementation id array length=0 */
+
 	hdr->nops++;
 	hdr->replen += decode_exchange_id_maxsz;
 }
-- 
1.7.4.4

[PATCH 2/3] NFSv4: fix server_scope memory leak

[Weston Andros Adamson]

server_scope would never be freed if nfs4_check_cl_exchange_flags() returned
non-zero

Signed-off-by: Weston Andros Adamson <dros@netapp.com>
---
 fs/nfs/nfs4proc.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 87c584d..20c3bb0 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4945,8 +4945,10 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
 				clp->cl_rpcclient->cl_auth->au_flavor);
 
 	res.server_scope = kzalloc(sizeof(struct server_scope), GFP_KERNEL);
-	if (unlikely(!res.server_scope))
-		return -ENOMEM;
+	if (unlikely(!res.server_scope)) {
+		status = -ENOMEM;
+		goto out;
+	}
 
 	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
 	if (!status)
@@ -4963,12 +4965,13 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
 			clp->server_scope = NULL;
 		}
 
-		if (!clp->server_scope)
+		if (!clp->server_scope) {
 			clp->server_scope = res.server_scope;
-		else
-			kfree(res.server_scope);
+			goto out;
+		}
 	}
-
+	kfree(res.server_scope);
+out:
 	dprintk("<-- %s status= %d\n", __func__, status);
 	return status;
 }
-- 
1.7.4.4

Re: [PATCH 2/3] NFSv4: fix server_scope memory leak

[Myklebust, Trond]

T24gVGh1LCAyMDEyLTAyLTE2IGF0IDExOjE3IC0wNTAwLCBXZXN0b24gQW5kcm9zIEFkYW1zb24g
d3JvdGU6DQo+IHNlcnZlcl9zY29wZSB3b3VsZCBuZXZlciBiZSBmcmVlZCBpZiBuZnM0X2NoZWNr
X2NsX2V4Y2hhbmdlX2ZsYWdzKCkgcmV0dXJuZWQNCj4gbm9uLXplcm8NCj4gDQo+IFNpZ25lZC1v
ZmYtYnk6IFdlc3RvbiBBbmRyb3MgQWRhbXNvbiA8ZHJvc0BuZXRhcHAuY29tPg0KPiAtLS0NCj4g
IGZzL25mcy9uZnM0cHJvYy5jIHwgICAxNSArKysrKysrKystLS0tLS0NCj4gIDEgZmlsZXMgY2hh
bmdlZCwgOSBpbnNlcnRpb25zKCspLCA2IGRlbGV0aW9ucygtKQ0KPiANCj4gZGlmZiAtLWdpdCBh
L2ZzL25mcy9uZnM0cHJvYy5jIGIvZnMvbmZzL25mczRwcm9jLmMNCj4gaW5kZXggODdjNTg0ZC4u
MjBjM2JiMCAxMDA2NDQNCj4gLS0tIGEvZnMvbmZzL25mczRwcm9jLmMNCj4gKysrIGIvZnMvbmZz
L25mczRwcm9jLmMNCj4gQEAgLTQ5NDUsOCArNDk0NSwxMCBAQCBpbnQgbmZzNF9wcm9jX2V4Y2hh
bmdlX2lkKHN0cnVjdCBuZnNfY2xpZW50ICpjbHAsIHN0cnVjdCBycGNfY3JlZCAqY3JlZCkNCj4g
IAkJCQljbHAtPmNsX3JwY2NsaWVudC0+Y2xfYXV0aC0+YXVfZmxhdm9yKTsNCj4gIA0KPiAgCXJl
cy5zZXJ2ZXJfc2NvcGUgPSBremFsbG9jKHNpemVvZihzdHJ1Y3Qgc2VydmVyX3Njb3BlKSwgR0ZQ
X0tFUk5FTCk7DQo+IC0JaWYgKHVubGlrZWx5KCFyZXMuc2VydmVyX3Njb3BlKSkNCj4gLQkJcmV0
dXJuIC1FTk9NRU07DQo+ICsJaWYgKHVubGlrZWx5KCFyZXMuc2VydmVyX3Njb3BlKSkgew0KPiAr
CQlzdGF0dXMgPSAtRU5PTUVNOw0KPiArCQlnb3RvIG91dDsNCj4gKwl9DQo+ICANCj4gIAlzdGF0
dXMgPSBycGNfY2FsbF9zeW5jKGNscC0+Y2xfcnBjY2xpZW50LCAmbXNnLCBSUENfVEFTS19USU1F
T1VUKTsNCj4gIAlpZiAoIXN0YXR1cykNCj4gQEAgLTQ5NjMsMTIgKzQ5NjUsMTMgQEAgaW50IG5m
czRfcHJvY19leGNoYW5nZV9pZChzdHJ1Y3QgbmZzX2NsaWVudCAqY2xwLCBzdHJ1Y3QgcnBjX2Ny
ZWQgKmNyZWQpDQo+ICAJCQljbHAtPnNlcnZlcl9zY29wZSA9IE5VTEw7DQo+ICAJCX0NCj4gIA0K
PiAtCQlpZiAoIWNscC0+c2VydmVyX3Njb3BlKQ0KPiArCQlpZiAoIWNscC0+c2VydmVyX3Njb3Bl
KSB7DQo+ICAJCQljbHAtPnNlcnZlcl9zY29wZSA9IHJlcy5zZXJ2ZXJfc2NvcGU7DQo+IC0JCWVs
c2UNCj4gLQkJCWtmcmVlKHJlcy5zZXJ2ZXJfc2NvcGUpOw0KPiArCQkJZ290byBvdXQ7DQo+ICsJ
CX0NCj4gIAl9DQo+IC0NCj4gKwlrZnJlZShyZXMuc2VydmVyX3Njb3BlKTsNCj4gK291dDoNCj4g
IAlkcHJpbnRrKCI8LS0gJXMgc3RhdHVzPSAlZFxuIiwgX19mdW5jX18sIHN0YXR1cyk7DQo+ICAJ
cmV0dXJuIHN0YXR1czsNCj4gIH0NCg0KVGhpcyBsb29rcyBsaWtlIGl0IGlzIGEgYnVnIGluIGV4
aXN0aW5nIGtlcm5lbHMuIFNob3VsZCBJIHF1ZXVlIGl0IHVwDQpmb3Igc3RhYmxlQHZnZXIua2Vy
bmVsLm9yZz8NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRh
aW5lcg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNv
bQ0KDQo=

Re: [PATCH 2/3] NFSv4: fix server_scope memory leak

[Adamson, Dros]

[-- Attachment #1: Type: text/plain, Size: 1630 bytes --]

On Feb 16, 2012, at 3:43 PM, Myklebust, Trond wrote:

> On Thu, 2012-02-16 at 11:17 -0500, Weston Andros Adamson wrote:
>> server_scope would never be freed if nfs4_check_cl_exchange_flags() returned
>> non-zero
>> 
>> Signed-off-by: Weston Andros Adamson <dros@netapp.com>
>> ---
>> fs/nfs/nfs4proc.c |   15 +++++++++------
>> 1 files changed, 9 insertions(+), 6 deletions(-)
>> 
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 87c584d..20c3bb0 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -4945,8 +4945,10 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
>> 				clp->cl_rpcclient->cl_auth->au_flavor);
>> 
>> 	res.server_scope = kzalloc(sizeof(struct server_scope), GFP_KERNEL);
>> -	if (unlikely(!res.server_scope))
>> -		return -ENOMEM;
>> +	if (unlikely(!res.server_scope)) {
>> +		status = -ENOMEM;
>> +		goto out;
>> +	}
>> 
>> 	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
>> 	if (!status)
>> @@ -4963,12 +4965,13 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
>> 			clp->server_scope = NULL;
>> 		}
>> 
>> -		if (!clp->server_scope)
>> +		if (!clp->server_scope) {
>> 			clp->server_scope = res.server_scope;
>> -		else
>> -			kfree(res.server_scope);
>> +			goto out;
>> +		}
>> 	}
>> -
>> +	kfree(res.server_scope);
>> +out:
>> 	dprintk("<-- %s status= %d\n", __func__, status);
>> 	return status;
>> }
> 
> This looks like it is a bug in existing kernels. Should I queue it up
> for stable@vger.kernel.org?

Yes, I should have mentioned that!

-dros

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 1374 bytes --]

[PATCH 3/3] NFSv4: parse and display server implementation ids

[Weston Andros Adamson]

Shows the implementation ids in /proc/self/mountstats.  This doesn't break
the nfs-utils mountstats tool.

Signed-off-by: Weston Andros Adamson <dros@netapp.com>
---
 fs/nfs/client.c           |    1 +
 fs/nfs/nfs4proc.c         |   21 +++++++++++++++++++++
 fs/nfs/nfs4xdr.c          |   42 +++++++++++++++++++++++++++++++++++++-----
 fs/nfs/super.c            |    8 ++++++++
 include/linux/nfs_fs_sb.h |    2 ++
 include/linux/nfs_xdr.h   |   15 +++++++--------
 6 files changed, 76 insertions(+), 13 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index d0f850f..3fd9a47 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -302,6 +302,7 @@ static void nfs_free_client(struct nfs_client *clp)
 
 	kfree(clp->cl_hostname);
 	kfree(clp->server_scope);
+	kfree(clp->impl_id);
 	kfree(clp);
 
 	dprintk("<-- nfs_free_client()\n");
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 20c3bb0..90a17cc 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4950,11 +4950,24 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
 		goto out;
 	}
 
+	res.impl_id = kzalloc(sizeof(struct nfs41_impl_id), GFP_KERNEL);
+	if (unlikely(!res.impl_id)) {
+		status = -ENOMEM;
+		goto out_server_scope;
+	}
+
 	status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT);
 	if (!status)
 		status = nfs4_check_cl_exchange_flags(clp->cl_exchange_flags);
 
 	if (!status) {
+		/* use the most recent implementation id */
+		kfree(clp->impl_id);
+		clp->impl_id = res.impl_id;
+	} else
+		kfree(res.impl_id);
+
+	if (!status) {
 		if (clp->server_scope &&
 		    !nfs41_same_server_scope(clp->server_scope,
 					     res.server_scope)) {
@@ -4970,8 +4983,16 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred)
 			goto out;
 		}
 	}
+
+out_server_scope:
 	kfree(res.server_scope);
 out:
+	if (clp->impl_id)
+		dprintk("%s: Server Implementation ID: "
+			"domain: %s, name: %s, date: %llu,%u\n",
+			__func__, clp->impl_id->domain, clp->impl_id->name,
+			clp->impl_id->date.seconds,
+			clp->impl_id->date.nseconds);
 	dprintk("<-- %s status= %d\n", __func__, status);
 	return status;
 }
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 70ded94..6bf3fe9 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -291,7 +291,11 @@ static int nfs4_stat_to_errno(int);
 				/* eir_server_scope<> */ \
 				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + 1 + \
 				1 /* eir_server_impl_id array length */ + \
-				0 /* ignored eir_server_impl_id contents */)
+				1 /* nii_domain */ + \
+				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \
+				1 /* nii_name */ + \
+				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \
+				3 /* nii_date */)
 #define encode_channel_attrs_maxsz  (6 + 1 /* ca_rdma_ird.len (0) */)
 #define decode_channel_attrs_maxsz  (6 + \
 				     1 /* ca_rdma_ird.len */ + \
@@ -5253,6 +5257,7 @@ static int decode_exchange_id(struct xdr_stream *xdr,
 	char *dummy_str;
 	int status;
 	struct nfs_client *clp = res->client;
+	uint32_t impl_id_count;
 
 	status = decode_op_hdr(xdr, OP_EXCHANGE_ID);
 	if (status)
@@ -5294,11 +5299,38 @@ static int decode_exchange_id(struct xdr_stream *xdr,
 	memcpy(res->server_scope->server_scope, dummy_str, dummy);
 	res->server_scope->server_scope_sz = dummy;
 
-	/* Throw away Implementation id array */
-	status = decode_opaque_inline(xdr, &dummy, &dummy_str);
-	if (unlikely(status))
-		return status;
+	/* Implementation Id */
+	p = xdr_inline_decode(xdr, 4);
+	if (unlikely(!p))
+		goto out_overflow;
+	impl_id_count = be32_to_cpup(p++);
 
+	if (impl_id_count) {
+		/* nii_domain */
+		status = decode_opaque_inline(xdr, &dummy, &dummy_str);
+		if (unlikely(status))
+			return status;
+		if (unlikely(dummy > NFS4_OPAQUE_LIMIT))
+			return -EIO;
+		memcpy(res->impl_id->domain, dummy_str, dummy);
+
+		/* nii_name */
+		status = decode_opaque_inline(xdr, &dummy, &dummy_str);
+		if (unlikely(status))
+			return status;
+		if (unlikely(dummy > NFS4_OPAQUE_LIMIT))
+			return -EIO;
+		memcpy(res->impl_id->name, dummy_str, dummy);
+
+		/* nii_date */
+		p = xdr_inline_decode(xdr, 12);
+		if (unlikely(!p))
+			goto out_overflow;
+		p = xdr_decode_hyper(p, &res->impl_id->date.seconds);
+		res->impl_id->date.nseconds = be32_to_cpup(p);
+
+		/* if there's more than one entry, ignore the rest */
+	}
 	return 0;
 out_overflow:
 	print_overflow_msg(__func__, xdr);
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index d05024a..5462225 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -809,6 +809,14 @@ static int nfs_show_stats(struct seq_file *m, struct dentry *root)
 
 	seq_printf(m, "\n\tage:\t%lu", (jiffies - nfss->mount_time) / HZ);
 
+	if (nfss->nfs_client && nfss->nfs_client->impl_id) {
+		struct nfs41_impl_id *impl_id = nfss->nfs_client->impl_id;
+		seq_printf(m, "\n\timpl_id:\tname='%s',domain='%s',"
+			   "date='%llu,%u'",
+			   impl_id->name, impl_id->domain,
+			   impl_id->date.seconds, impl_id->date.nseconds);
+	}
+
 	seq_printf(m, "\n\tcaps:\t");
 	seq_printf(m, "caps=0x%x", nfss->caps);
 	seq_printf(m, ",wtmult=%u", nfss->wtmult);
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 3bf4766..03d0b91 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -18,6 +18,7 @@ struct nfs4_sequence_res;
 struct nfs_server;
 struct nfs4_minor_version_ops;
 struct server_scope;
+struct nfs41_impl_id;
 
 /*
  * The nfs_client identifies our client state to the server.
@@ -86,6 +87,7 @@ struct nfs_client {
 #endif
 
 	struct server_scope	*server_scope;	/* from exchange_id */
+	struct nfs41_impl_id	*impl_id;	/* from exchange_id */
 	struct net		*net;
 };
 
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index adbc84a..046c1bf 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1054,14 +1054,6 @@ struct nfstime4 {
 };
 
 #ifdef CONFIG_NFS_V4_1
-struct nfs_impl_id4 {
-	u32		domain_len;
-	char		*domain;
-	u32		name_len;
-	char		*name;
-	struct nfstime4	date;
-};
-
 #define NFS4_EXCHANGE_ID_LEN	(48)
 struct nfs41_exchange_id_args {
 	struct nfs_client		*client;
@@ -1082,10 +1074,17 @@ struct server_scope {
 	char 				server_scope[NFS4_OPAQUE_LIMIT];
 };
 
+struct nfs41_impl_id {
+	char				domain[NFS4_OPAQUE_LIMIT + 1];
+	char				name[NFS4_OPAQUE_LIMIT + 1];
+	struct nfstime4			date;
+};
+
 struct nfs41_exchange_id_res {
 	struct nfs_client		*client;
 	u32				flags;
 	struct server_scope		*server_scope;
+	struct nfs41_impl_id		*impl_id;
 };
 
 struct nfs41_create_session_args {
-- 
1.7.4.4

Re: [PATCH 1/3] NFSv4: Send implementation id with exchange_id

[Jim Rees]

Weston Andros Adamson wrote:

  Send the nfs implementation id in EXCHANGE_ID requests unless the module
  parameter nfs.send_implementation_id is 0.
  
  This adds a CONFIG variable for the nii_domain that defaults to "kernel.org".
  
  Signed-off-by: Weston Andros Adamson <dros@netapp.com>

  @@ -271,7 +273,12 @@ static int nfs4_stat_to_errno(int);
   				1 /* flags */ + \
   				1 /* spa_how */ + \
   				0 /* SP4_NONE (for now) */ + \
  -				1 /* zero implemetation id array */)
  +				1 /* implemetation id array of size 1 */ + \

implemetation -> implementation
Might as well fix this typo while you're at it.

General comment... If this is going to be configurable, I'd rather make it a
sysctl, and be able to set the id to whatever I want at run time.  But
that's because I like to be able to break things.  I suspect Trond will
disagree.

Re: [PATCH 1/3] NFSv4: Send implementation id with exchange_id

[Myklebust, Trond]

T24gVGh1LCAyMDEyLTAyLTE2IGF0IDEyOjMwIC0wNTAwLCBKaW0gUmVlcyB3cm90ZToNCj4gV2Vz
dG9uIEFuZHJvcyBBZGFtc29uIHdyb3RlOg0KPiANCj4gICBTZW5kIHRoZSBuZnMgaW1wbGVtZW50
YXRpb24gaWQgaW4gRVhDSEFOR0VfSUQgcmVxdWVzdHMgdW5sZXNzIHRoZSBtb2R1bGUNCj4gICBw
YXJhbWV0ZXIgbmZzLnNlbmRfaW1wbGVtZW50YXRpb25faWQgaXMgMC4NCj4gICANCj4gICBUaGlz
IGFkZHMgYSBDT05GSUcgdmFyaWFibGUgZm9yIHRoZSBuaWlfZG9tYWluIHRoYXQgZGVmYXVsdHMg
dG8gImtlcm5lbC5vcmciLg0KPiAgIA0KPiAgIFNpZ25lZC1vZmYtYnk6IFdlc3RvbiBBbmRyb3Mg
QWRhbXNvbiA8ZHJvc0BuZXRhcHAuY29tPg0KPiANCj4gICBAQCAtMjcxLDcgKzI3MywxMiBAQCBz
dGF0aWMgaW50IG5mczRfc3RhdF90b19lcnJubyhpbnQpOw0KPiAgICAJCQkJMSAvKiBmbGFncyAq
LyArIFwNCj4gICAgCQkJCTEgLyogc3BhX2hvdyAqLyArIFwNCj4gICAgCQkJCTAgLyogU1A0X05P
TkUgKGZvciBub3cpICovICsgXA0KPiAgIC0JCQkJMSAvKiB6ZXJvIGltcGxlbWV0YXRpb24gaWQg
YXJyYXkgKi8pDQo+ICAgKwkJCQkxIC8qIGltcGxlbWV0YXRpb24gaWQgYXJyYXkgb2Ygc2l6ZSAx
ICovICsgXA0KPiANCj4gaW1wbGVtZXRhdGlvbiAtPiBpbXBsZW1lbnRhdGlvbg0KPiBNaWdodCBh
cyB3ZWxsIGZpeCB0aGlzIHR5cG8gd2hpbGUgeW91J3JlIGF0IGl0Lg0KPiANCj4gR2VuZXJhbCBj
b21tZW50Li4uIElmIHRoaXMgaXMgZ29pbmcgdG8gYmUgY29uZmlndXJhYmxlLCBJJ2QgcmF0aGVy
IG1ha2UgaXQgYQ0KPiBzeXNjdGwsIGFuZCBiZSBhYmxlIHRvIHNldCB0aGUgaWQgdG8gd2hhdGV2
ZXIgSSB3YW50IGF0IHJ1biB0aW1lLiAgQnV0DQo+IHRoYXQncyBiZWNhdXNlIEkgbGlrZSB0byBi
ZSBhYmxlIHRvIGJyZWFrIHRoaW5ncy4gIEkgc3VzcGVjdCBUcm9uZCB3aWxsDQo+IGRpc2FncmVl
Lg0KDQpJIGRvLiBXZSBkb24ndCBtYWtlIHRoZSAndW5hbWUnIGNvbmZpZ3VyYWJsZSBhcyBhIHN5
c2N0bC4gSG93IGlzIHRoaXMNCmFueSBkaWZmZXJlbnQ/IFdoeSB3b3VsZCB5b3Ugd2FudCB0byBk
ZWxpYmVyYXRlbHkgbWlzcmVwcmVzZW50IHdoaWNoIE5GUw0KY2xpZW50IHRoaXMgaXMgdG8gdGhl
IHNlcnZlcj8gVGhlIGltcGxlbWVudGF0aW9uIGlkIGlzIHRoZXJlIGZvciBlcnJvcg0KbG9nZ2lu
ZyBwdXJwb3Nlcywgbm90aGluZyBlbHNlLi4uDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51
eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBw
LmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K

Re: [PATCH 1/3] NFSv4: Send implementation id with exchange_id

[Adamson, Dros]

[-- Attachment #1: Type: text/plain, Size: 1067 bytes --]


On Feb 16, 2012, at 12:30 PM, Jim Rees wrote:

> Weston Andros Adamson wrote:
> 
>  Send the nfs implementation id in EXCHANGE_ID requests unless the module
>  parameter nfs.send_implementation_id is 0.
> 
>  This adds a CONFIG variable for the nii_domain that defaults to "kernel.org".
> 
>  Signed-off-by: Weston Andros Adamson <dros@netapp.com>
> 
>  @@ -271,7 +273,12 @@ static int nfs4_stat_to_errno(int);
>   				1 /* flags */ + \
>   				1 /* spa_how */ + \
>   				0 /* SP4_NONE (for now) */ + \
>  -				1 /* zero implemetation id array */)
>  +				1 /* implemetation id array of size 1 */ + \
> 
> implemetation -> implementation
> Might as well fix this typo while you're at it.
> 
> General comment... If this is going to be configurable, I'd rather make it a
> sysctl, and be able to set the id to whatever I want at run time.  But
> that's because I like to be able to break things.  I suspect Trond will
> disagree.

Hrm my ispell pass missed this.

Yes, Trond was against using a sysctl.  This is how he wants it.

-dros

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 1374 bytes --]

Re: [PATCH 1/3] NFSv4: Send implementation id with exchange_id

[Adamson, Dros]

[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]


On Feb 16, 2012, at 12:37 PM, Adamson, Dros wrote:

> 
> On Feb 16, 2012, at 12:30 PM, Jim Rees wrote:
> 
>> Weston Andros Adamson wrote:
>> 
>> Send the nfs implementation id in EXCHANGE_ID requests unless the module
>> parameter nfs.send_implementation_id is 0.
>> 
>> This adds a CONFIG variable for the nii_domain that defaults to "kernel.org".
>> 
>> Signed-off-by: Weston Andros Adamson <dros@netapp.com>
>> 
>> @@ -271,7 +273,12 @@ static int nfs4_stat_to_errno(int);
>>  				1 /* flags */ + \
>>  				1 /* spa_how */ + \
>>  				0 /* SP4_NONE (for now) */ + \
>> -				1 /* zero implemetation id array */)
>> +				1 /* implemetation id array of size 1 */ + \
>> 
>> implemetation -> implementation
>> Might as well fix this typo while you're at it.
>> 
>> General comment... If this is going to be configurable, I'd rather make it a
>> sysctl, and be able to set the id to whatever I want at run time.  But
>> that's because I like to be able to break things.  I suspect Trond will
>> disagree.
> 
> Hrm my ispell pass missed this.

… I'll wait for additional comments before reposting.

-dros

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 1374 bytes --]

Re: [PATCH 1/3] NFSv4: Send implementation id with exchange_id

[Jim Rees]

Adamson, Dros wrote:

  >> General comment... If this is going to be configurable, I'd rather make it a
  >> sysctl, and be able to set the id to whatever I want at run time.  But
  >> that's because I like to be able to break things.  I suspect Trond will
  >> disagree.
  > 
  > Hrm my ispell pass missed this.
  
  … I'll wait for additional comments before reposting.

I guess my point was, why make it configurable at all?

Re: [PATCH 1/3] NFSv4: Send implementation id with exchange_id

[Myklebust, Trond]

T24gVGh1LCAyMDEyLTAyLTE2IGF0IDE1OjEwIC0wNTAwLCBKaW0gUmVlcyB3cm90ZToNCj4gQWRh
bXNvbiwgRHJvcyB3cm90ZToNCj4gDQo+ICAgPj4gR2VuZXJhbCBjb21tZW50Li4uIElmIHRoaXMg
aXMgZ29pbmcgdG8gYmUgY29uZmlndXJhYmxlLCBJJ2QgcmF0aGVyIG1ha2UgaXQgYQ0KPiAgID4+
IHN5c2N0bCwgYW5kIGJlIGFibGUgdG8gc2V0IHRoZSBpZCB0byB3aGF0ZXZlciBJIHdhbnQgYXQg
cnVuIHRpbWUuICBCdXQNCj4gICA+PiB0aGF0J3MgYmVjYXVzZSBJIGxpa2UgdG8gYmUgYWJsZSB0
byBicmVhayB0aGluZ3MuICBJIHN1c3BlY3QgVHJvbmQgd2lsbA0KPiAgID4+IGRpc2FncmVlLg0K
PiAgID4gDQo+ICAgPiBIcm0gbXkgaXNwZWxsIHBhc3MgbWlzc2VkIHRoaXMuDQo+ICAgDQo+ICAg
4oCmIEknbGwgd2FpdCBmb3IgYWRkaXRpb25hbCBjb21tZW50cyBiZWZvcmUgcmVwb3N0aW5nLg0K
PiANCj4gSSBndWVzcyBteSBwb2ludCB3YXMsIHdoeSBtYWtlIGl0IGNvbmZpZ3VyYWJsZSBhdCBh
bGw/DQoNClVuZm9ydHVuYXRlbHksIHRoZSBzcGVjIGhhcyBzb21lIHdpbGQgaWRlYXMgYWJvdXQg
aG93IHRvIGlkZW50aWZ5IGFuIE5GUw0KY2xpZW50LiBJbiBwYXJ0aWN1bGFyIGl0IHRoaW5rcyB0
aGF0IHRoZXJlIGlzIGFuICdpbXBsZW1lbnRvcicgd2l0aCBhDQpETlMgYWRkcmVzcy4uLg0KDQot
LSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFw
cA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQoNCg==

Re: [PATCH 1/3] NFSv4: Send implementation id with exchange_id

[Myklebust, Trond]

T24gVGh1LCAyMDEyLTAyLTE2IGF0IDExOjE3IC0wNTAwLCBXZXN0b24gQW5kcm9zIEFkYW1zb24g
d3JvdGU6DQo+IFNlbmQgdGhlIG5mcyBpbXBsZW1lbnRhdGlvbiBpZCBpbiBFWENIQU5HRV9JRCBy
ZXF1ZXN0cyB1bmxlc3MgdGhlIG1vZHVsZQ0KPiBwYXJhbWV0ZXIgbmZzLnNlbmRfaW1wbGVtZW50
YXRpb25faWQgaXMgMC4NCj4gDQo+IFRoaXMgYWRkcyBhIENPTkZJRyB2YXJpYWJsZSBmb3IgdGhl
IG5paV9kb21haW4gdGhhdCBkZWZhdWx0cyB0byAia2VybmVsLm9yZyIuDQo+IA0KPiBTaWduZWQt
b2ZmLWJ5OiBXZXN0b24gQW5kcm9zIEFkYW1zb24gPGRyb3NAbmV0YXBwLmNvbT4NCj4gLS0tDQo+
ICBEb2N1bWVudGF0aW9uL2tlcm5lbC1wYXJhbWV0ZXJzLnR4dCB8ICAgIDkgKysrKysrKysNCj4g
IGZzL25mcy9LY29uZmlnICAgICAgICAgICAgICAgICAgICAgIHwgICAgOCArKysrKysrDQo+ICBm
cy9uZnMvbmZzNHhkci5jICAgICAgICAgICAgICAgICAgICB8ICAgMzggKysrKysrKysrKysrKysr
KysrKysrKysrKysrKysrKysrLQ0KPiAgMyBmaWxlcyBjaGFuZ2VkLCA1MyBpbnNlcnRpb25zKCsp
LCAyIGRlbGV0aW9ucygtKQ0KPiANCj4gZGlmZiAtLWdpdCBhL0RvY3VtZW50YXRpb24va2VybmVs
LXBhcmFtZXRlcnMudHh0IGIvRG9jdW1lbnRhdGlvbi9rZXJuZWwtcGFyYW1ldGVycy50eHQNCj4g
aW5kZXggMWQzNjljNi4uN2JhZTBmZCAxMDA2NDQNCj4gLS0tIGEvRG9jdW1lbnRhdGlvbi9rZXJu
ZWwtcGFyYW1ldGVycy50eHQNCj4gKysrIGIvRG9jdW1lbnRhdGlvbi9rZXJuZWwtcGFyYW1ldGVy
cy50eHQNCj4gQEAgLTE2NzgsNiArMTY3OCwxNSBAQCBieXRlcyByZXNwZWN0aXZlbHkuIFN1Y2gg
bGV0dGVyIHN1ZmZpeGVzIGNhbiBhbHNvIGJlIGVudGlyZWx5IG9taXR0ZWQuDQo+ICAJCQliYWNr
IHRvIHVzaW5nIHRoZSBpZG1hcHBlci4NCj4gIAkJCVRvIHR1cm4gb2ZmIHRoaXMgYmVoYXZpb3Vy
LCBzZXQgdGhlIHZhbHVlIHRvICcwJy4NCj4gIA0KPiArCW5mcy5zZW5kX2ltcGxlbWVudGF0aW9u
X2lkID0NCj4gKwkJCVtORlN2NC4xXSBTZW5kIGNsaWVudCBpbXBsZW1lbnRhdGlvbiBpZGVudGlm
aWNhdGlvbg0KPiArCQkJaW5mb3JtYXRpb24gaW4gZXhjaGFuZ2VfaWQgcmVxdWVzdHMuDQo+ICsJ
CQlJZiB6ZXJvLCBubyBpbXBsZW1lbnRhdGlvbiBpZGVudGlmaWNhdGlvbiBpbmZvcm1hdGlvbg0K
PiArCQkJd2lsbCBiZSBzZW50Lg0KPiArCQkJVGhlIGRlZmF1bHQgaXMgdG8gc2VuZCB0aGUgaW1w
bGVtZW50YXRpb24gaWRlbnRpZmljYXRpb24NCj4gKwkJCWluZm9ybWF0aW9uLg0KPiArDQo+ICsN
Cj4gIAlubWlfZGVidWc9CVtLTkwsQVZSMzIsU0hdIFNwZWNpZnkgb25lIG9yIG1vcmUgYWN0aW9u
cyB0byB0YWtlDQo+ICAJCQl3aGVuIGEgTk1JIGlzIHRyaWdnZXJlZC4NCj4gIAkJCUZvcm1hdDog
W3N0YXRlXVsscmVnc11bLGRlYm91bmNlXVssZGllXQ0KPiBkaWZmIC0tZ2l0IGEvZnMvbmZzL0tj
b25maWcgYi9mcy9uZnMvS2NvbmZpZw0KPiBpbmRleCBlZTg2Y2ZjLi4wOWY5ZTM4IDEwMDY0NA0K
PiAtLS0gYS9mcy9uZnMvS2NvbmZpZw0KPiArKysgYi9mcy9uZnMvS2NvbmZpZw0KPiBAQCAtOTks
NiArOTksMTQgQEAgY29uZmlnIFBORlNfT0JKTEFZT1VUDQo+ICAJZGVwZW5kcyBvbiBORlNfRlMg
JiYgTkZTX1Y0XzEgJiYgU0NTSV9PU0RfVUxEDQo+ICAJZGVmYXVsdCBtDQo+ICANCj4gK2NvbmZp
ZyBORlNfVjRfMV9JTVBMRU1FTlRBVElPTl9JRF9ET01BSU4NCj4gKwlzdHJpbmcgIk5GU3Y0LjEg
SW1wbGVtZW50YXRpb24gSUQgRG9tYWluIg0KPiArCWRlcGVuZHMgb24gTkZTX1Y0XzENCj4gKwlk
ZWZhdWx0ICJrZXJuZWwub3JnIg0KPiArCWhlbHANCj4gKwkgIFRoaXMgdmFsdWUgd2lsbCBiZSB1
c2VkIGluIEVYQ0hBTkdFX0lEIGNvbXBvdW5kcyB0byBoZWxwIGlkZW50aWZ5DQo+ICsJICB0aGUg
Y2xpZW50IGltcGxlbWVudGF0aW9uLg0KDQpUaGlzIG5lZWRzIG1vcmUgZmxlc2hpbmcgb3V0IGlu
IG9yZGVyIHRvIGV4cGxhaW4gd2hhdCB0aGUgcHVycG9zZSBpcw0KKGlkZW50aWZ5IHRoZSBETlMg
ZG9tYWlubmFtZSBvZiB0aGUgZGlzdHJpYnV0aW9uKSwgYW5kIHdoYXQgZm9ybWF0IHRoZQ0Kc3Ry
aW5nIG5lZWRzIHRvIHRha2UgKEROUyBkb21haW5uYW1lKS4gWW91IG1pZ2h0IGFsc28gZm9sbG93
IHVwIHdpdGggYQ0KcmVjb21tZW5kYXRpb24gdGhhdCBpZiB0aGUgTkZTIGNsaWVudCBpcyB1bmNo
YW5nZWQgZnJvbSB0aGUgdXBzdHJlYW0NCmtlcm5lbCwgdGhlbiBwZW9wbGUgc2hvdWxkIHVzZSB0
aGUgZGVmYXVsdCAna2VybmVsLm9yZycuDQoNCj4gKw0KPiAgY29uZmlnIFJPT1RfTkZTDQo+ICAJ
Ym9vbCAiUm9vdCBmaWxlIHN5c3RlbSBvbiBORlMiDQo+ICAJZGVwZW5kcyBvbiBORlNfRlM9eSAm
JiBJUF9QTlANCj4gZGlmZiAtLWdpdCBhL2ZzL25mcy9uZnM0eGRyLmMgYi9mcy9uZnMvbmZzNHhk
ci5jDQo+IGluZGV4IGFlNzgzNDMuLjcwZGVkOTQgMTAwNjQ0DQo+IC0tLSBhL2ZzL25mcy9uZnM0
eGRyLmMNCj4gKysrIGIvZnMvbmZzL25mczR4ZHIuYw0KPiBAQCAtNDQsNiArNDQsOCBAQA0KPiAg
I2luY2x1ZGUgPGxpbnV4L3BhZ2VtYXAuaD4NCj4gICNpbmNsdWRlIDxsaW51eC9wcm9jX2ZzLmg+
DQo+ICAjaW5jbHVkZSA8bGludXgva2Rldl90Lmg+DQo+ICsjaW5jbHVkZSA8bGludXgvbW9kdWxl
Lmg+DQo+ICsjaW5jbHVkZSA8bGludXgvdXRzbmFtZS5oPg0KPiAgI2luY2x1ZGUgPGxpbnV4L3N1
bnJwYy9jbG50Lmg+DQo+ICAjaW5jbHVkZSA8bGludXgvc3VucnBjL21zZ19wcm90Lmg+DQo+ICAj
aW5jbHVkZSA8bGludXgvc3VucnBjL2dzc19hcGkuaD4NCj4gQEAgLTI3MSw3ICsyNzMsMTIgQEAg
c3RhdGljIGludCBuZnM0X3N0YXRfdG9fZXJybm8oaW50KTsNCj4gIAkJCQkxIC8qIGZsYWdzICov
ICsgXA0KPiAgCQkJCTEgLyogc3BhX2hvdyAqLyArIFwNCj4gIAkJCQkwIC8qIFNQNF9OT05FIChm
b3Igbm93KSAqLyArIFwNCj4gLQkJCQkxIC8qIHplcm8gaW1wbGVtZXRhdGlvbiBpZCBhcnJheSAq
LykNCj4gKwkJCQkxIC8qIGltcGxlbWV0YXRpb24gaWQgYXJyYXkgb2Ygc2l6ZSAxICovICsgXA0K
PiArCQkJCTEgLyogbmlpX2RvbWFpbiAqLyArIFwNCj4gKwkJCQlYRFJfUVVBRExFTihORlM0X09Q
QVFVRV9MSU1JVCkgKyBcDQo+ICsJCQkJMSAvKiBuaWlfbmFtZSAqLyArIFwNCj4gKwkJCQlYRFJf
UVVBRExFTihORlM0X09QQVFVRV9MSU1JVCkgKyBcDQo+ICsJCQkJMyAvKiBuaWlfZGF0ZSAqLykN
Cj4gICNkZWZpbmUgZGVjb2RlX2V4Y2hhbmdlX2lkX21heHN6IChvcF9kZWNvZGVfaGRyX21heHN6
ICsgXA0KPiAgCQkJCTIgLyogZWlyX2NsaWVudGlkICovICsgXA0KPiAgCQkJCTEgLyogZWlyX3Nl
cXVlbmNlaWQgKi8gKyBcDQo+IEBAIC04MzgsNiArODQ1LDEyIEBAIGNvbnN0IHUzMiBuZnM0MV9t
YXhyZWFkX292ZXJoZWFkID0gKChSUENfTUFYX0hFQURFUl9XSVRIX0FVVEggKw0KPiAgCQkJCSAg
ICBYRFJfVU5JVCk7DQo+ICAjZW5kaWYgLyogQ09ORklHX05GU19WNF8xICovDQo+ICANCj4gK3N0
YXRpYyB1bnNpZ25lZCBzaG9ydCBzZW5kX2ltcGxlbWVudGF0aW9uX2lkID0gMTsNCj4gKw0KPiAr
bW9kdWxlX3BhcmFtKHNlbmRfaW1wbGVtZW50YXRpb25faWQsIHVzaG9ydCwgMDY0NCk7DQo+ICtN
T0RVTEVfUEFSTV9ERVNDKHNlbmRfaW1wbGVtZW50YXRpb25faWQsDQo+ICsJCSJTZW5kIGltcGxl
bWVudGF0aW9uIElEIHdpdGggTkZTdjQuMSBleGNoYW5nZV9pZCIpOw0KPiArDQo+ICBzdGF0aWMg
Y29uc3QgdW1vZGVfdCBuZnNfdHlwZTJmbXRbXSA9IHsNCj4gIAlbTkY0QkFEXSA9IDAsDQo+ICAJ
W05GNFJFR10gPSBTX0lGUkVHLA0KPiBAQCAtMTc2Niw2ICsxNzc5LDggQEAgc3RhdGljIHZvaWQg
ZW5jb2RlX2V4Y2hhbmdlX2lkKHN0cnVjdCB4ZHJfc3RyZWFtICp4ZHIsDQo+ICAJCQkgICAgICAg
c3RydWN0IGNvbXBvdW5kX2hkciAqaGRyKQ0KPiAgew0KPiAgCV9fYmUzMiAqcDsNCj4gKwljaGFy
IGltcGxfbmFtZVsyNjBdOyAvKiA0IHN0cnMgbWF4IDY0IGVhY2gsIHNwYWNlcyBhbmQgbnVsbCAq
Lw0KICAgICAgICAgICAgICAgICAgICAgICAgXl5eXiBTaG91bGRuJ3QgeW91IGJlIHB1dHRpbmcg
dGhpcyBpbiB0ZXJtcyBvZg0KTkZTNF9PUEFRVUVfTElNSVQgZXRjIGxpa2UgeW91IGRpZCBhYm92
ZT8NCg0KPiArCWludCBsZW4gPSAwOw0KPiAgDQo+ICAJcCA9IHJlc2VydmVfc3BhY2UoeGRyLCA0
ICsgc2l6ZW9mKGFyZ3MtPnZlcmlmaWVyLT5kYXRhKSk7DQo+ICAJKnArKyA9IGNwdV90b19iZTMy
KE9QX0VYQ0hBTkdFX0lEKTsNCj4gQEAgLTE3NzYsNyArMTc5MSwyNiBAQCBzdGF0aWMgdm9pZCBl
bmNvZGVfZXhjaGFuZ2VfaWQoc3RydWN0IHhkcl9zdHJlYW0gKnhkciwNCj4gIAlwID0gcmVzZXJ2
ZV9zcGFjZSh4ZHIsIDEyKTsNCj4gIAkqcCsrID0gY3B1X3RvX2JlMzIoYXJncy0+ZmxhZ3MpOw0K
PiAgCSpwKysgPSBjcHVfdG9fYmUzMigwKTsJLyogemVybyBsZW5ndGggc3RhdGVfcHJvdGVjdDRf
YSAqLw0KPiAtCSpwID0gY3B1X3RvX2JlMzIoMCk7CS8qIHplcm8gbGVuZ3RoIGltcGxlbWVudGF0
aW9uIGlkIGFycmF5ICovDQo+ICsNCj4gKwlpZiAoc2VuZF9pbXBsZW1lbnRhdGlvbl9pZCkNCj4g
KwkJbGVuID0gc25wcmludGYoaW1wbF9uYW1lLCBzaXplb2YoaW1wbF9uYW1lKSwgIiVzICVzICVz
ICVzIiwNCj4gKwkJCSAgICAgICB1dHNuYW1lKCktPnN5c25hbWUsIHV0c25hbWUoKS0+cmVsZWFz
ZSwNCj4gKwkJCSAgICAgICB1dHNuYW1lKCktPnZlcnNpb24sIHV0c25hbWUoKS0+bWFjaGluZSk7
DQo+ICsNCg0KQ2FuIHdlIHB1dCBpbiBhIGNvbXBpbGUtdGltZSBjaGVjayBvbg0KQ09ORklHX05G
U19WNF8xX0lNUExFTUVOVEFUSU9OX0lEX0RPTUFJTiBoZXJlIHRvIG1ha2Ugc3VyZSB0aGF0IHRo
ZQ0KbGVuZ3RoIGlzIGJvdW5kZWQ/DQoNCj4gKwlpZiAobGVuID4gMCkgew0KPiArCQkqcCA9IGNw
dV90b19iZTMyKDEpOwkvKiBpbXBsZW1lbnRhdGlvbiBpZCBhcnJheSBsZW5ndGg9MSAqLw0KPiAr
DQo+ICsJCWVuY29kZV9zdHJpbmcoeGRyLA0KPiArCQkJc3RybGVuKENPTkZJR19ORlNfVjRfMV9J
TVBMRU1FTlRBVElPTl9JRF9ET01BSU4pLA0KPiArCQkJQ09ORklHX05GU19WNF8xX0lNUExFTUVO
VEFUSU9OX0lEX0RPTUFJTik7DQo+ICsJCWVuY29kZV9zdHJpbmcoeGRyLCBsZW4sIGltcGxfbmFt
ZSk7DQo+ICsJCS8qIGp1c3Qgc2VuZCB6ZXJvcyBmb3IgbmlpX2RhdGUgLSB0aGUgZGF0ZSBpcyBp
biBuaWlfbmFtZSAqLw0KPiArCQlwID0gcmVzZXJ2ZV9zcGFjZSh4ZHIsIDEyKTsNCj4gKwkJcCA9
IHhkcl9lbmNvZGVfaHlwZXIocCwgMCk7DQo+ICsJCSpwID0gY3B1X3RvX2JlMzIoMCk7DQo+ICsJ
fSBlbHNlDQo+ICsJCSpwID0gY3B1X3RvX2JlMzIoMCk7CS8qIGltcGxlbWVudGF0aW9uIGlkIGFy
cmF5IGxlbmd0aD0wICovDQo+ICsNCj4gIAloZHItPm5vcHMrKzsNCj4gIAloZHItPnJlcGxlbiAr
PSBkZWNvZGVfZXhjaGFuZ2VfaWRfbWF4c3o7DQo+ICB9DQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0
DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RA
bmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K

Re: [PATCH 1/3] NFSv4: Send implementation id with exchange_id

[Adamson, Dros]

[-- Attachment #1: Type: text/plain, Size: 6145 bytes --]


On Feb 16, 2012, at 3:40 PM, Myklebust, Trond wrote:

> On Thu, 2012-02-16 at 11:17 -0500, Weston Andros Adamson wrote:
>> Send the nfs implementation id in EXCHANGE_ID requests unless the module
>> parameter nfs.send_implementation_id is 0.
>> 
>> This adds a CONFIG variable for the nii_domain that defaults to "kernel.org".
>> 
>> Signed-off-by: Weston Andros Adamson <dros@netapp.com>
>> ---
>> Documentation/kernel-parameters.txt |    9 ++++++++
>> fs/nfs/Kconfig                      |    8 +++++++
>> fs/nfs/nfs4xdr.c                    |   38 +++++++++++++++++++++++++++++++++-
>> 3 files changed, 53 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index 1d369c6..7bae0fd 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -1678,6 +1678,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>> 			back to using the idmapper.
>> 			To turn off this behaviour, set the value to '0'.
>> 
>> +	nfs.send_implementation_id =
>> +			[NFSv4.1] Send client implementation identification
>> +			information in exchange_id requests.
>> +			If zero, no implementation identification information
>> +			will be sent.
>> +			The default is to send the implementation identification
>> +			information.
>> +
>> +
>> 	nmi_debug=	[KNL,AVR32,SH] Specify one or more actions to take
>> 			when a NMI is triggered.
>> 			Format: [state][,regs][,debounce][,die]
>> diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
>> index ee86cfc..09f9e38 100644
>> --- a/fs/nfs/Kconfig
>> +++ b/fs/nfs/Kconfig
>> @@ -99,6 +99,14 @@ config PNFS_OBJLAYOUT
>> 	depends on NFS_FS && NFS_V4_1 && SCSI_OSD_ULD
>> 	default m
>> 
>> +config NFS_V4_1_IMPLEMENTATION_ID_DOMAIN
>> +	string "NFSv4.1 Implementation ID Domain"
>> +	depends on NFS_V4_1
>> +	default "kernel.org"
>> +	help
>> +	  This value will be used in EXCHANGE_ID compounds to help identify
>> +	  the client implementation.
> 
> This needs more fleshing out in order to explain what the purpose is
> (identify the DNS domainname of the distribution), and what format the
> string needs to take (DNS domainname). You might also follow up with a
> recommendation that if the NFS client is unchanged from the upstream
> kernel, then people should use the default 'kernel.org'.

Understood.

> 
>> +
>> config ROOT_NFS
>> 	bool "Root file system on NFS"
>> 	depends on NFS_FS=y && IP_PNP
>> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
>> index ae78343..70ded94 100644
>> --- a/fs/nfs/nfs4xdr.c
>> +++ b/fs/nfs/nfs4xdr.c
>> @@ -44,6 +44,8 @@
>> #include <linux/pagemap.h>
>> #include <linux/proc_fs.h>
>> #include <linux/kdev_t.h>
>> +#include <linux/module.h>
>> +#include <linux/utsname.h>
>> #include <linux/sunrpc/clnt.h>
>> #include <linux/sunrpc/msg_prot.h>
>> #include <linux/sunrpc/gss_api.h>
>> @@ -271,7 +273,12 @@ static int nfs4_stat_to_errno(int);
>> 				1 /* flags */ + \
>> 				1 /* spa_how */ + \
>> 				0 /* SP4_NONE (for now) */ + \
>> -				1 /* zero implemetation id array */)
>> +				1 /* implemetation id array of size 1 */ + \
>> +				1 /* nii_domain */ + \
>> +				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \
>> +				1 /* nii_name */ + \
>> +				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \
>> +				3 /* nii_date */)
>> #define decode_exchange_id_maxsz (op_decode_hdr_maxsz + \
>> 				2 /* eir_clientid */ + \
>> 				1 /* eir_sequenceid */ + \
>> @@ -838,6 +845,12 @@ const u32 nfs41_maxread_overhead = ((RPC_MAX_HEADER_WITH_AUTH +
>> 				    XDR_UNIT);
>> #endif /* CONFIG_NFS_V4_1 */
>> 
>> +static unsigned short send_implementation_id = 1;
>> +
>> +module_param(send_implementation_id, ushort, 0644);
>> +MODULE_PARM_DESC(send_implementation_id,
>> +		"Send implementation ID with NFSv4.1 exchange_id");
>> +
>> static const umode_t nfs_type2fmt[] = {
>> 	[NF4BAD] = 0,
>> 	[NF4REG] = S_IFREG,
>> @@ -1766,6 +1779,8 @@ static void encode_exchange_id(struct xdr_stream *xdr,
>> 			       struct compound_hdr *hdr)
>> {
>> 	__be32 *p;
>> +	char impl_name[260]; /* 4 strs max 64 each, spaces and null */
>                        ^^^^ Shouldn't you be putting this in terms of
> NFS4_OPAQUE_LIMIT etc like you did above?

The NFS4_OPAQUE_LIMIT is from the spec - that is, a server could make an nii_name string that big.  
Creating the client's nii_name is bound by utsname() size limits (__NEW_UTS_LEN = 64).

If you'd prefer me to just use NFS4_OPAQUE_LIMIT, I have no problem with it.

> 
>> +	int len = 0;
>> 
>> 	p = reserve_space(xdr, 4 + sizeof(args->verifier->data));
>> 	*p++ = cpu_to_be32(OP_EXCHANGE_ID);
>> @@ -1776,7 +1791,26 @@ static void encode_exchange_id(struct xdr_stream *xdr,
>> 	p = reserve_space(xdr, 12);
>> 	*p++ = cpu_to_be32(args->flags);
>> 	*p++ = cpu_to_be32(0);	/* zero length state_protect4_a */
>> -	*p = cpu_to_be32(0);	/* zero length implementation id array */
>> +
>> +	if (send_implementation_id)
>> +		len = snprintf(impl_name, sizeof(impl_name), "%s %s %s %s",
>> +			       utsname()->sysname, utsname()->release,
>> +			       utsname()->version, utsname()->machine);
>> +
> 
> Can we put in a compile-time check on
> CONFIG_NFS_V4_1_IMPLEMENTATION_ID_DOMAIN here to make sure that the
> length is bounded?

Yes, good call.  It should also check to make sure its not an empty string -- some servers (pynfs at least) blow up when you send a string of size 0 here.

> 
>> +	if (len > 0) {
>> +		*p = cpu_to_be32(1);	/* implementation id array length=1 */
>> +
>> +		encode_string(xdr,
>> +			strlen(CONFIG_NFS_V4_1_IMPLEMENTATION_ID_DOMAIN),
>> +			CONFIG_NFS_V4_1_IMPLEMENTATION_ID_DOMAIN);
>> +		encode_string(xdr, len, impl_name);
>> +		/* just send zeros for nii_date - the date is in nii_name */
>> +		p = reserve_space(xdr, 12);
>> +		p = xdr_encode_hyper(p, 0);
>> +		*p = cpu_to_be32(0);
>> +	} else
>> +		*p = cpu_to_be32(0);	/* implementation id array length=0 */
>> +
>> 	hdr->nops++;
>> 	hdr->replen += decode_exchange_id_maxsz;
>> }

Thanks,
-dros

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 1374 bytes --]

[PATCH 1/3] NFSv4: Send implementation id with exchange_id

[Weston Andros Adamson]

Send the nfs implementation id in EXCHANGE_ID requests unless the module
parameter nfs.send_implementation_id is 0.

This adds a CONFIG variable for the nii_domain that defaults to "kernel.org".

Signed-off-by: Weston Andros Adamson <dros@netapp.com>
---
Update: Applied Trond's comments

 Documentation/kernel-parameters.txt |    9 +++++++
 fs/nfs/Kconfig                      |   12 ++++++++++
 fs/nfs/nfs4xdr.c                    |   41 +++++++++++++++++++++++++++++++++-
 3 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 1d369c6..7bae0fd 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1678,6 +1678,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			back to using the idmapper.
 			To turn off this behaviour, set the value to '0'.
 
+	nfs.send_implementation_id =
+			[NFSv4.1] Send client implementation identification
+			information in exchange_id requests.
+			If zero, no implementation identification information
+			will be sent.
+			The default is to send the implementation identification
+			information.
+
+
 	nmi_debug=	[KNL,AVR32,SH] Specify one or more actions to take
 			when a NMI is triggered.
 			Format: [state][,regs][,debounce][,die]
diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
index ee86cfc..7bce64c 100644
--- a/fs/nfs/Kconfig
+++ b/fs/nfs/Kconfig
@@ -99,6 +99,18 @@ config PNFS_OBJLAYOUT
 	depends on NFS_FS && NFS_V4_1 && SCSI_OSD_ULD
 	default m
 
+config NFS_V4_1_IMPLEMENTATION_ID_DOMAIN
+	string "NFSv4.1 Implementation ID Domain"
+	depends on NFS_V4_1
+	default "kernel.org"
+	help
+	  This option defines the domain portion of the implementation ID that
+	  may be sent in the NFS exchange_id operation.  The value must be in
+	  the format of a DNS domain name and should be set to the DNS domain
+	  name of the distribution.
+	  If the NFS client is unchanged from the upstream kernel, this
+	  option should be set to the default "kernel.org".
+
 config ROOT_NFS
 	bool "Root file system on NFS"
 	depends on NFS_FS=y && IP_PNP
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index ae78343..d824aed 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -44,6 +44,8 @@
 #include <linux/pagemap.h>
 #include <linux/proc_fs.h>
 #include <linux/kdev_t.h>
+#include <linux/module.h>
+#include <linux/utsname.h>
 #include <linux/sunrpc/clnt.h>
 #include <linux/sunrpc/msg_prot.h>
 #include <linux/sunrpc/gss_api.h>
@@ -271,7 +273,12 @@ static int nfs4_stat_to_errno(int);
 				1 /* flags */ + \
 				1 /* spa_how */ + \
 				0 /* SP4_NONE (for now) */ + \
-				1 /* zero implemetation id array */)
+				1 /* implementation id array of size 1 */ + \
+				1 /* nii_domain */ + \
+				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \
+				1 /* nii_name */ + \
+				XDR_QUADLEN(NFS4_OPAQUE_LIMIT) + \
+				3 /* nii_date */)
 #define decode_exchange_id_maxsz (op_decode_hdr_maxsz + \
 				2 /* eir_clientid */ + \
 				1 /* eir_sequenceid */ + \
@@ -838,6 +845,12 @@ const u32 nfs41_maxread_overhead = ((RPC_MAX_HEADER_WITH_AUTH +
 				    XDR_UNIT);
 #endif /* CONFIG_NFS_V4_1 */
 
+static unsigned short send_implementation_id = 1;
+
+module_param(send_implementation_id, ushort, 0644);
+MODULE_PARM_DESC(send_implementation_id,
+		"Send implementation ID with NFSv4.1 exchange_id");
+
 static const umode_t nfs_type2fmt[] = {
 	[NF4BAD] = 0,
 	[NF4REG] = S_IFREG,
@@ -1766,6 +1779,8 @@ static void encode_exchange_id(struct xdr_stream *xdr,
 			       struct compound_hdr *hdr)
 {
 	__be32 *p;
+	char impl_name[NFS4_OPAQUE_LIMIT];
+	int len = 0;
 
 	p = reserve_space(xdr, 4 + sizeof(args->verifier->data));
 	*p++ = cpu_to_be32(OP_EXCHANGE_ID);
@@ -1776,7 +1791,29 @@ static void encode_exchange_id(struct xdr_stream *xdr,
 	p = reserve_space(xdr, 12);
 	*p++ = cpu_to_be32(args->flags);
 	*p++ = cpu_to_be32(0);	/* zero length state_protect4_a */
-	*p = cpu_to_be32(0);	/* zero length implementation id array */
+
+	if (send_implementation_id &&
+	    sizeof(CONFIG_NFS_V4_1_IMPLEMENTATION_ID_DOMAIN) > 1 &&
+	    sizeof(CONFIG_NFS_V4_1_IMPLEMENTATION_ID_DOMAIN)
+		<= NFS4_OPAQUE_LIMIT + 1)
+		len = snprintf(impl_name, sizeof(impl_name), "%s %s %s %s",
+			       utsname()->sysname, utsname()->release,
+			       utsname()->version, utsname()->machine);
+
+	if (len > 0) {
+		*p = cpu_to_be32(1);	/* implementation id array length=1 */
+
+		encode_string(xdr,
+			sizeof(CONFIG_NFS_V4_1_IMPLEMENTATION_ID_DOMAIN) - 1,
+			CONFIG_NFS_V4_1_IMPLEMENTATION_ID_DOMAIN);
+		encode_string(xdr, len, impl_name);
+		/* just send zeros for nii_date - the date is in nii_name */
+		p = reserve_space(xdr, 12);
+		p = xdr_encode_hyper(p, 0);
+		*p = cpu_to_be32(0);
+	} else
+		*p = cpu_to_be32(0);	/* implementation id array length=0 */
+
 	hdr->nops++;
 	hdr->replen += decode_exchange_id_maxsz;
 }
-- 
1.7.4.4