2012-03-06 23:18:07

by Weston Andros Adamson

[permalink] [raw]
Subject: [PATCH] NFS: add crc hash to nfs_display_fhandle

Match wireshark's CRC-32 hash for easier debugging.

Signed-off-by: Weston Andros Adamson <[email protected]>
---
Requested by Chuck and others.

Nothing in trond/nfs-for-next uses nfs_display_fhandle() yet.
I tested by calling it in nfs4xdr.c:encode_putfh().

fs/nfs/inode.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 99a4f52..c1f44e9 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -39,6 +39,7 @@
#include <linux/slab.h>
#include <linux/compat.h>
#include <linux/freezer.h>
+#include <linux/crc32.h>

#include <asm/system.h>
#include <asm/uaccess.h>
@@ -1057,13 +1058,18 @@ struct nfs_fh *nfs_alloc_fhandle(void)
void _nfs_display_fhandle(const struct nfs_fh *fh, const char *caption)
{
unsigned short i;
+ u32 crc;

if (fh->size == 0 || fh == NULL) {
printk(KERN_DEFAULT "%s at %p is empty\n", caption, fh);
return;
}

- printk(KERN_DEFAULT "%s at %p is %u bytes:\n", caption, fh, fh->size);
+ /* match wireshark's CRC-32 hash */
+ crc = ~crc32(0xFFFFFFFF, &fh->data[0], fh->size);
+
+ printk(KERN_DEFAULT "%s at %p is %u bytes, crc: 0x%08x:\n",
+ caption, fh, fh->size, crc);
for (i = 0; i < fh->size; i += 16) {
__be32 *pos = (__be32 *)&fh->data[i];

--
1.7.4.4



2012-03-06 23:24:26

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH] NFS: add crc hash to nfs_display_fhandle


On Mar 6, 2012, at 6:17 PM, Weston Andros Adamson wrote:

> Match wireshark's CRC-32 hash for easier debugging.
>
> Signed-off-by: Weston Andros Adamson <[email protected]>
> ---
> Requested by Chuck and others.
>
> Nothing in trond/nfs-for-next uses nfs_display_fhandle() yet.
> I tested by calling it in nfs4xdr.c:encode_putfh().

The one patch in my series that used it was dropped. So this is for debugging only. If we don't add any permanent call sites for it, enterprise distributions could wrap nfs_display_fhandle() in RPC_DEBUG.

> fs/nfs/inode.c | 8 +++++++-
> 1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> index 99a4f52..c1f44e9 100644
> --- a/fs/nfs/inode.c
> +++ b/fs/nfs/inode.c
> @@ -39,6 +39,7 @@
> #include <linux/slab.h>
> #include <linux/compat.h>
> #include <linux/freezer.h>
> +#include <linux/crc32.h>
>
> #include <asm/system.h>
> #include <asm/uaccess.h>
> @@ -1057,13 +1058,18 @@ struct nfs_fh *nfs_alloc_fhandle(void)
> void _nfs_display_fhandle(const struct nfs_fh *fh, const char *caption)
> {
> unsigned short i;
> + u32 crc;
>
> if (fh->size == 0 || fh == NULL) {
> printk(KERN_DEFAULT "%s at %p is empty\n", caption, fh);
> return;
> }
>
> - printk(KERN_DEFAULT "%s at %p is %u bytes:\n", caption, fh, fh->size);
> + /* match wireshark's CRC-32 hash */
> + crc = ~crc32(0xFFFFFFFF, &fh->data[0], fh->size);
> +
> + printk(KERN_DEFAULT "%s at %p is %u bytes, crc: 0x%08x:\n",
> + caption, fh, fh->size, crc);
> for (i = 0; i < fh->size; i += 16) {
> __be32 *pos = (__be32 *)&fh->data[i];
>
> --
> 1.7.4.4
>

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





2012-03-07 00:00:47

by Adamson, Dros

[permalink] [raw]
Subject: Re: [PATCH] NFS: add crc hash to nfs_display_fhandle


On Mar 6, 2012, at 6:29 PM, Myklebust, Trond wrote:

> On Tue, 2012-03-06 at 18:17 -0500, Weston Andros Adamson wrote:
>> Match wireshark's CRC-32 hash for easier debugging.
>>
>> Signed-off-by: Weston Andros Adamson <[email protected]>
>> ---
>> Requested by Chuck and others.
>>
>> Nothing in trond/nfs-for-next uses nfs_display_fhandle() yet.
>> I tested by calling it in nfs4xdr.c:encode_putfh().
>>
>> fs/nfs/inode.c | 8 +++++++-
>> 1 files changed, 7 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
>> index 99a4f52..c1f44e9 100644
>> --- a/fs/nfs/inode.c
>> +++ b/fs/nfs/inode.c
>> @@ -39,6 +39,7 @@
>> #include <linux/slab.h>
>> #include <linux/compat.h>
>> #include <linux/freezer.h>
>> +#include <linux/crc32.h>
>>
>> #include <asm/system.h>
>> #include <asm/uaccess.h>
>> @@ -1057,13 +1058,18 @@ struct nfs_fh *nfs_alloc_fhandle(void)
>> void _nfs_display_fhandle(const struct nfs_fh *fh, const char *caption)
>> {
>> unsigned short i;
>> + u32 crc;
>>
>> if (fh->size == 0 || fh == NULL) {
>> printk(KERN_DEFAULT "%s at %p is empty\n", caption, fh);
>> return;
>> }
>>
>> - printk(KERN_DEFAULT "%s at %p is %u bytes:\n", caption, fh, fh->size);
>> + /* match wireshark's CRC-32 hash */
>> + crc = ~crc32(0xFFFFFFFF, &fh->data[0], fh->size);
>> +
>> + printk(KERN_DEFAULT "%s at %p is %u bytes, crc: 0x%08x:\n",
>> + caption, fh, fh->size, crc);
>> for (i = 0; i < fh->size; i += 16) {
>> __be32 *pos = (__be32 *)&fh->data[i];
> Could we split this into 2 functions:
>
> nfs_display_fhandle_hash() and nfs_display_fhandle(), with the latter
> calling the former?
>
> That will allow us to select both a short form and a full form. I can
> see the former being used in something like nfs_fhget(), or the
> revalidation functions, while the latter would be useful in
> *_proc_lookup() or nfs_instantiate()...

No problem.

-dros


Attachments:
smime.p7s (1.34 kB)

2012-03-06 23:29:54

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] NFS: add crc hash to nfs_display_fhandle

T24gVHVlLCAyMDEyLTAzLTA2IGF0IDE4OjE3IC0wNTAwLCBXZXN0b24gQW5kcm9zIEFkYW1zb24g
d3JvdGU6DQo+IE1hdGNoIHdpcmVzaGFyaydzIENSQy0zMiBoYXNoIGZvciBlYXNpZXIgZGVidWdn
aW5nLg0KPiANCj4gU2lnbmVkLW9mZi1ieTogV2VzdG9uIEFuZHJvcyBBZGFtc29uIDxkcm9zQG5l
dGFwcC5jb20+DQo+IC0tLQ0KPiBSZXF1ZXN0ZWQgYnkgQ2h1Y2sgYW5kIG90aGVycy4NCj4gDQo+
IE5vdGhpbmcgaW4gdHJvbmQvbmZzLWZvci1uZXh0IHVzZXMgbmZzX2Rpc3BsYXlfZmhhbmRsZSgp
IHlldC4NCj4gSSB0ZXN0ZWQgYnkgY2FsbGluZyBpdCBpbiBuZnM0eGRyLmM6ZW5jb2RlX3B1dGZo
KCkuDQo+IA0KPiAgZnMvbmZzL2lub2RlLmMgfCAgICA4ICsrKysrKystDQo+ICAxIGZpbGVzIGNo
YW5nZWQsIDcgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbnMoLSkNCj4gDQo+IGRpZmYgLS1naXQg
YS9mcy9uZnMvaW5vZGUuYyBiL2ZzL25mcy9pbm9kZS5jDQo+IGluZGV4IDk5YTRmNTIuLmMxZjQ0
ZTkgMTAwNjQ0DQo+IC0tLSBhL2ZzL25mcy9pbm9kZS5jDQo+ICsrKyBiL2ZzL25mcy9pbm9kZS5j
DQo+IEBAIC0zOSw2ICszOSw3IEBADQo+ICAjaW5jbHVkZSA8bGludXgvc2xhYi5oPg0KPiAgI2lu
Y2x1ZGUgPGxpbnV4L2NvbXBhdC5oPg0KPiAgI2luY2x1ZGUgPGxpbnV4L2ZyZWV6ZXIuaD4NCj4g
KyNpbmNsdWRlIDxsaW51eC9jcmMzMi5oPg0KPiAgDQo+ICAjaW5jbHVkZSA8YXNtL3N5c3RlbS5o
Pg0KPiAgI2luY2x1ZGUgPGFzbS91YWNjZXNzLmg+DQo+IEBAIC0xMDU3LDEzICsxMDU4LDE4IEBA
IHN0cnVjdCBuZnNfZmggKm5mc19hbGxvY19maGFuZGxlKHZvaWQpDQo+ICB2b2lkIF9uZnNfZGlz
cGxheV9maGFuZGxlKGNvbnN0IHN0cnVjdCBuZnNfZmggKmZoLCBjb25zdCBjaGFyICpjYXB0aW9u
KQ0KPiAgew0KPiAgCXVuc2lnbmVkIHNob3J0IGk7DQo+ICsJdTMyIGNyYzsNCj4gIA0KPiAgCWlm
IChmaC0+c2l6ZSA9PSAwIHx8IGZoID09IE5VTEwpIHsNCj4gIAkJcHJpbnRrKEtFUk5fREVGQVVM
VCAiJXMgYXQgJXAgaXMgZW1wdHlcbiIsIGNhcHRpb24sIGZoKTsNCj4gIAkJcmV0dXJuOw0KPiAg
CX0NCj4gIA0KPiAtCXByaW50ayhLRVJOX0RFRkFVTFQgIiVzIGF0ICVwIGlzICV1IGJ5dGVzOlxu
IiwgY2FwdGlvbiwgZmgsIGZoLT5zaXplKTsNCj4gKwkvKiBtYXRjaCB3aXJlc2hhcmsncyBDUkMt
MzIgaGFzaCAqLw0KPiArCWNyYyA9IH5jcmMzMigweEZGRkZGRkZGLCAmZmgtPmRhdGFbMF0sIGZo
LT5zaXplKTsNCj4gKw0KPiArCXByaW50ayhLRVJOX0RFRkFVTFQgIiVzIGF0ICVwIGlzICV1IGJ5
dGVzLCBjcmM6IDB4JTA4eDpcbiIsDQo+ICsJICAgICAgICAgICAgICAgICAgICBjYXB0aW9uLCBm
aCwgZmgtPnNpemUsIGNyYyk7DQo+ICAJZm9yIChpID0gMDsgaSA8IGZoLT5zaXplOyBpICs9IDE2
KSB7DQo+ICAJCV9fYmUzMiAqcG9zID0gKF9fYmUzMiAqKSZmaC0+ZGF0YVtpXTsNCkNvdWxkIHdl
IHNwbGl0IHRoaXMgaW50byAyIGZ1bmN0aW9uczoNCg0KbmZzX2Rpc3BsYXlfZmhhbmRsZV9oYXNo
KCkgYW5kIG5mc19kaXNwbGF5X2ZoYW5kbGUoKSwgd2l0aCB0aGUgbGF0dGVyDQpjYWxsaW5nIHRo
ZSBmb3JtZXI/DQoNClRoYXQgd2lsbCBhbGxvdyB1cyB0byBzZWxlY3QgYm90aCBhIHNob3J0IGZv
cm0gYW5kIGEgZnVsbCBmb3JtLiBJIGNhbg0Kc2VlIHRoZSBmb3JtZXIgYmVpbmcgdXNlZCBpbiBz
b21ldGhpbmcgbGlrZSBuZnNfZmhnZXQoKSwgb3IgdGhlDQpyZXZhbGlkYXRpb24gZnVuY3Rpb25z
LCB3aGlsZSB0aGUgbGF0dGVyIHdvdWxkIGJlIHVzZWZ1bCBpbg0KKl9wcm9jX2xvb2t1cCgpIG9y
IG5mc19pbnN0YW50aWF0ZSgpLi4uDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMg
Y2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0K
d3d3Lm5ldGFwcC5jb20NCg0K