Skip to contentSkip to conversation

idmapd.c: deactivate the ASCII characters check

PATCH In review Gregor Gruener 6 participants 9 messages Feb 23, 2012 – May 18, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 idmapd.c: deactivate the ASCII characters check

Customers are using Unicode characters umlauts (ö,ä,ü) in group names and this
creates problems with NFS ID Mapping. Groups with umlauts will be redirected to
the group NFS "nobody".
This patch deactivate the ASCII characters check. It's maybe more like a temporary fix and I think it would be nicer to adjust the check to support Unicode characters instead to deactivate the check.

========================================================================================================

utils/idmapd/idmapd.c +0 -3
diff -Nurp nfs-utils-1.2.2/utils/idmapd/idmapd.c nfs-utils-1.2.2-fag/utils/idmapd/idmapd.c
@@ -850,9 +850,6 @@ validateascii(char *string, u_int32_t le
 	for (i = 0; i<  len; i++) {
 		if (string[i] == '\0')
 			break;
-
-		if (string[i]&  0x80)
-			return (-1);
 	}
 
 	if ((i>= len) || string[i] != '\0')

=========================================================================================================

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or

(b) The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or

(c) The contribution was provided directly to me by some other person who certified (a), (b) or (c) and I have not modified it.

(d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved.

#1 lore raw

Gregor Gruener wrote:

Customers are using Unicode characters umlauts (ö,ä,ü) in group names and this
creates problems with NFS ID Mapping. Groups with umlauts will be redirected to
the group NFS "nobody".
This patch deactivate the ASCII characters check. It's maybe more like a temporary fix and I think it would be nicer to adjust the check to support Unicode characters instead to deactivate the
check.

Maybe the name of the routine should also be changed to something other than validateascii(). But I'm curious why that check was put in there.

I found this in rfc5661 section 22.1, which seems to be a bug in the spec:

1. A US-ASCII string name that is the actual name of the attribute. This name must be unique. This string name can be 1 to 128 UTF-8 characters long.

#2 lore raw
Quoted 11 lines · Jim Rees

Gregor Gruener wrote:

Customers are using Unicode characters umlauts (ö,ä,ü) in group names and this
creates problems with NFS ID Mapping. Groups with umlauts will be redirected to
the group NFS "nobody".
This patch deactivate the ASCII characters check. It's maybe more like a temporary fix and I think it would be nicer to adjust the check to support Unicode characters instead to deactivate the
check.

Maybe the name of the routine should also be changed to something other than validateascii(). But I'm curious why that check was put in there.

Sounds totally wrong.

By the nfs rfc's of course, if we're going to check anything we should be checking for utf8.

Though even then, it's hard to see how failing and mapping to nobody really helps anyone here once they already have a non-utf8 name.

Best might be to allow a non-utf8 mapping and print a one-time warning. And work on fixing any account-creation tools to enforce utf8.

Quoted 6 lines

I found this in rfc5661 section 22.1, which seems to be a bug in the spec:

1. A US-ASCII string name that is the actual name of the attribute. This name must be unique. This string name can be 1 to 128 UTF-8 characters long.

Oops.

--b.

#3 lore raw
In reply to Gregor Gruener
Quoted 49 lines · Gregor Gruener

Customers are using Unicode characters umlauts (ö,ä,ü) in group names and this
creates problems with NFS ID Mapping. Groups with umlauts will be redirected to
the group NFS "nobody".
This patch deactivate the ASCII characters check. It's maybe more like a temporary fix and I think it would be nicer to adjust the check to support Unicode characters instead to deactivate the check.

========================================================================================================

diff -Nurp nfs-utils-1.2.2/utils/idmapd/idmapd.c nfs-utils-1.2.2-fag/utils/idmapd/idmapd.c

utils/idmapd/idmapd.c 2011-10-05 09:09:05.815103100 +0200 renamed from utils/idmapd/idmapd.c 2010-02-18 13:35:00.000000000 +0100 +0 -3
@@ -850,9 +850,6 @@ validateascii(char *string, u_int32_t le
     for (i = 0; i<  len; i++) {
         if (string[i] == '\0')
             break;
-
-        if (string[i]&  0x80)
-            return (-1);
     }
 
     if ((i>= len) || string[i] != '\0')

=========================================================================================================

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or

(b) The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or

(c) The contribution was provided directly to me by some other person who certified (a), (b) or (c) and I have not modified it.

Quoted 7 lines

(d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved.

Committed...

steved.

Quoted 5 lines
Signature · 4 lines
-- 
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
#4 lore raw

Steve Dickson wrote:

Quoted 6 lines · Gregor Gruener

Customers are using Unicode characters umlauts (ö,ä,ü) in group names and this
creates problems with NFS ID Mapping. Groups with umlauts will be redirected to
the group NFS "nobody".
This patch deactivate the ASCII characters check. It's maybe more like a temporary fix and I think it would be nicer to adjust the check to support Unicode characters instead to deactivate the check.

...
Committed...

I still think the name should be changed. The only thing validateascii() is doing now is verifying that the string is null terminated.

#5 lore raw

Hey,

Quoted 9 lines · Jim Rees

Steve Dickson wrote:

Quoted 6 lines · Gregor Gruener

Customers are using Unicode characters umlauts (ö,ä,ü) in group names and this
creates problems with NFS ID Mapping. Groups with umlauts will be redirected to
the group NFS "nobody".
This patch deactivate the ASCII characters check. It's maybe more like a temporary fix and I think it would be nicer to adjust the check to support Unicode characters instead to deactivate the check.

Quoted 5 lines

...
Committed...

I still think the name should be changed. The only thing validateascii() is doing now is verifying that the string is null terminated.

Sorry Jim... I kinda overlooked that part of the discussion... I guess validateascii() is not a published interface so we could change it... any suggestions? (Patches always welcome! ;-) )

steved.

#6 lore raw
In reply to Steve Dickson

Team,

These are some preliminary results with pNFS block. They look very promising both performance and scalability. They uncovered some instability but mainly due to HW issues; no SW issues. These results are our prove that the latest pNFS block code is stable and performing reasonably well.

Single client pNFS performance was 86MB/sec write performance for reference. Thank you very much for your patience and support

We’ve finished the testing and analysis of pNFS vs. MPFS. It took much longer than expected due to several stability issues uncovered on both the client and the server. Some of the issues occurred during read testing, which had never been a focus of previous testing efforts. Anyway, please find the results below. To help with the below interpretation, please reference the following key:

SW8xN-MPFS              Sequential Write 8 clients by N threads using MPFS (special build using latest kernel on Fedora 15)
SR8xN-MPFS              Sequential Read 8 clients by N threads using MPFS (special build on Fedora 15)
SW8xN-pNFS              Sequential Write 8 clients by N threads using pNFS (on Fedora 15)
SR8xN-pNFS              Sequential Read 8 clients by N threads using pNFS (on Fedora 15)
SW8xN-pNFS2             Sequential Write 8 clients by N threads using pNFS (on Fedora 15)
SR8xN-pNFS2             Sequential Read 8 clients by N threads using pNFS (on Fedora 15)
SR8xN-pNFS3             Sequential Read 8 clients by N threads using pNFS (on Fedora 15)
SW8xN-MPFS2             Sequential Write 8 clients by N threads using MPFS
SR8xN-MPFS2             Sequential Read 8 clients by N threads using MPFS

I think/hope the rest is self-explanatory based on labels, etc. To summarize:

pNFS2 pNFS4vs.MPFS
SW8x1 653121.85 -8%
SW8x2 835101.05 -10%
SW8x3 846188.63 -8%
SW8x4 852617.66 -6%
SR8x1 674129.88 43%
SR8x2 634800.33 34%
SR8x3 629843.20 28%
SR8x4 577197.70 19%

Let me know what you think or if you have any questions. I’m moving on to other projects that I’ve committed to and am currently behind on.

Signature · 12 lines
--
Best Regards

Sorin Faibish
Corporate Distinguished Engineer
Fast Data Group - Office of CTO
                          EMC2
where information lives

Phone: 508-249-5745
Mobile: 617-510-0422
email: [email protected]
#7 lore raw

pNFS block performance evaluation (sorry for the wrong subject title)

In reply to Steve Dickson

Team,

These are some preliminary results with pNFS block. They look very promising both performance and scalability. They uncovered some instability but mainly due to HW issues; no SW issues. These results are our prove that the latest pNFS block code is stable and performing reasonably well.

Single client pNFS performance was 86MB/sec write performance for reference. Thank you very much for your patience and support

We’ve finished the testing and analysis of pNFS vs. MPFS. It took much longer than expected due to several stability issues uncovered on both the client and the server. Some of the issues occurred during read testing, which had never been a focus of previous testing efforts. Anyway, please find the results below. To help with the below interpretation, please reference the following key:

SW8xN-MPFS Sequential Write 8 clients by N threads using MPFS
(special build using latest kernel on Fedora 15)
SR8xN-MPFS Sequential Read 8 clients by N threads using MPFS
(special build on Fedora 15)
SW8xN-pNFS Sequential Write 8 clients by N threads using pNFS
(on Fedora 15)
SR8xN-pNFS Sequential Read 8 clients by N threads using pNFS
(on Fedora 15)
SW8xN-pNFS2 Sequential Write 8 clients by N threads using pNFS
(on Fedora 15)
SR8xN-pNFS2 Sequential Read 8 clients by N threads using pNFS
(on Fedora 15)
SR8xN-pNFS3 Sequential Read 8 clients by N threads using pNFS
(on Fedora 15)
SW8xN-MPFS2 Sequential Write 8 clients by N threads using MPFS
SR8xN-MPFS2 Sequential Read 8 clients by N threads using MPFS

I think/hope the rest is self-explanatory based on labels, etc. To summarize:

pNFS2 pNFS4vs.MPFS
SW8x1 653121.85 -8%
SW8x2 835101.05 -10%
SW8x3 846188.63 -8%
SW8x4 852617.66 -6%
SR8x1 674129.88 43%
SR8x2 634800.33 34%
SR8x3 629843.20 28%
SR8x4 577197.70 19%

Let me know what you think or if you have any questions. I’m moving on to other projects that I’ve committed to and am currently behind on.

Signature · 12 lines
--
Best Regards

Sorin Faibish
Corporate Distinguished Engineer
Fast Data Group - Office of CTO
                            EMC2
where information lives

Phone: 508-249-5745
Mobile: 617-510-0422
email: [email protected]
#8 lore raw

RE: pNFS block performance evaluation (sorry for the wrong subject title)

Quoted 7 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Sorin Faibish
Sent: Friday, May 18, 2012 1:43 AM
To: [email protected]
Subject: pNFS block performance evaluation (sorry for the wrong subject title)

Quoted 8 lines

Team,

These are some preliminary results with pNFS block. They look very promising both performance and scalability. They uncovered some instability but mainly due to HW issues; no SW issues. These results are our prove that the latest pNFS block code is stable and performing reasonably well.

To be specific, the performance test was run with some additional patches that make client send real IO size in layoutget requests. I attached the patches in the email for your reference. These patches were sent out before and got some objections. I will rework them to address comments and send again later.

Thanks,
Tao

Quoted 56 lines

Single client pNFS performance was 86MB/sec write performance for reference. Thank you very much for your patience and support

We’ve finished the testing and analysis of pNFS vs. MPFS. It took much longer than expected due to several stability issues uncovered on both the client and the server. Some of the issues occurred during read testing, which had never been a focus of previous testing efforts. Anyway, please find the results below. To help with the below interpretation, please reference the following key:

SW8xN-MPFS Sequential Write 8 clients by N threads using MPFS
(special build using latest kernel on Fedora 15)
SR8xN-MPFS Sequential Read 8 clients by N threads using MPFS
(special build on Fedora 15)
SW8xN-pNFS Sequential Write 8 clients by N threads using pNFS
(on Fedora 15)
SR8xN-pNFS Sequential Read 8 clients by N threads using pNFS
(on Fedora 15)
SW8xN-pNFS2 Sequential Write 8 clients by N threads using pNFS
(on Fedora 15)
SR8xN-pNFS2 Sequential Read 8 clients by N threads using pNFS
(on Fedora 15)
SR8xN-pNFS3 Sequential Read 8 clients by N threads using pNFS
(on Fedora 15)
SW8xN-MPFS2 Sequential Write 8 clients by N threads using MPFS
SR8xN-MPFS2 Sequential Read 8 clients by N threads using MPFS

I think/hope the rest is self-explanatory based on labels, etc. To summarize:

pNFS2 pNFS4vs.MPFS
SW8x1 653121.85 -8%
SW8x2 835101.05 -10%
SW8x3 846188.63 -8%
SW8x4 852617.66 -6%
SR8x1 674129.88 43%
SR8x2 634800.33 34%
SR8x3 629843.20 28%
SR8x4 577197.70 19%

Let me know what you think or if you have any questions. I’m moving on to other projects that I’ve committed to and am currently behind on.

--
--
Best Regards

Sorin Faibish
Corporate Distinguished Engineer
Fast Data Group - Office of CTO
EMC2
where information lives

Quoted 7 lines

Phone: 508-249-5745
Mobile: 617-510-0422
email: [email protected]
--
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

From 55a7c6a10189d04645a84e9b2eb3d9b5d2f7e44d Mon Sep 17 00:00:00 2001
From: Peng Tao <[email protected]>
Date: Sat, 19 Nov 2011 04:23:26 -0500
Subject: [PATCH 1/4] nfsv41: export pnfs_find_alloc_layout

So that layout driver can access layout header when there is none.

---
 fs/nfs/pnfs.c |    3 ++-
 fs/nfs/pnfs.h |    4 ++++
 2 files changed, 6 insertions(+), 1 deletions(-)
fs/nfs/pnfs.c +2 -1
index baf7353..3be29c7 100644
@@ -848,7 +848,7 @@ alloc_init_layout_hdr(struct inode *ino,
 	return lo;
 }
 
-static struct pnfs_layout_hdr *
+struct pnfs_layout_hdr *
 pnfs_find_alloc_layout(struct inode *ino,
 		       struct nfs_open_context *ctx,
 		       gfp_t gfp_flags)
@@ -875,6 +875,7 @@ pnfs_find_alloc_layout(struct inode *ino,
 		pnfs_free_layout_hdr(new);
 	return nfsi->layout;
 }
+EXPORT_SYMBOL_GPL(pnfs_find_alloc_layout);
 
 /*
  * iomode matching rules:
fs/nfs/pnfs.h +4 -0
index 1509530..9614ac9 100644
@@ -209,6 +209,10 @@ struct pnfs_layout_segment *pnfs_update_layout(struct inode *ino,
 					       u64 count,
 					       enum pnfs_iomode iomode,
 					       gfp_t gfp_flags);
+struct pnfs_layout_hdr *
+pnfs_find_alloc_layout(struct inode *ino,
+		       struct nfs_open_context *ctx,
+		       gfp_t gfp_flags);
 
 void nfs4_deviceid_mark_client_invalid(struct nfs_client *clp);
 
Signature · 1 line
1.6.6

From 904e0e42d12b86f8b03791a325b90c1a415b9dcb Mon Sep 17 00:00:00 2001
From: Peng Tao <[email protected]>
Date: Sat, 19 Nov 2011 04:27:00 -0500
Subject: [PATCH 2/4] nfsv41: add and export pnfs_find_get_layout_locked

It tries to find the lseg from local cache but not retrive layout from server.

---
 fs/nfs/pnfs.c |   25 +++++++++++++++++++++++++
 fs/nfs/pnfs.h |    5 +++++
 2 files changed, 30 insertions(+), 0 deletions(-)
fs/nfs/pnfs.c +25 -0
index 3be29c7..734e670 100644
@@ -933,6 +933,31 @@ pnfs_find_lseg(struct pnfs_layout_hdr *lo,
 }
 
 /*
+ * Find and reference lseg with ino->i_lock held.
+ */
+struct pnfs_layout_segment *
+pnfs_find_get_layout_locked(struct inode *ino,
+			loff_t pos,
+			u64 count,
+			enum pnfs_iomode iomode)
+{
+	struct pnfs_layout_segment *lseg = NULL;
+	struct pnfs_layout_range range = {
+		.iomode = iomode,
+		.offset = pos,
+		.length = count,
+	};
+
+	if (NFS_I(ino)->layout == NULL)
+		goto out;
+
+	lseg = pnfs_find_lseg(NFS_I(ino)->layout, &range);
+out:
+	return lseg;
+}
+EXPORT_SYMBOL_GPL(pnfs_find_get_layout_locked);
+
+/*
  * Layout segment is retreived from the server if not cached.
  * The appropriate layout segment is referenced and returned to the caller.
  */
fs/nfs/pnfs.h +5 -0
index 9614ac9..0c55fc1 100644
@@ -213,6 +213,11 @@ struct pnfs_layout_hdr *
 pnfs_find_alloc_layout(struct inode *ino,
 		       struct nfs_open_context *ctx,
 		       gfp_t gfp_flags);
+struct pnfs_layout_segment *
+pnfs_find_get_layout_locked(struct inode *ino,
+			loff_t pos,
+			u64 count,
+			enum pnfs_iomode iomode);
 
 void nfs4_deviceid_mark_client_invalid(struct nfs_client *clp);
 
Signature · 1 line
1.6.6

From d3d1626dd2d362aee6c971591c37a49a5c5e1031 Mon Sep 17 00:00:00 2001
From: Peng Tao <[email protected]>
Date: Sat, 19 Nov 2011 04:33:39 -0500
Subject: [PATCH 3/4] nfsv41: get lseg before issue LD IO if pgio doesn't carry one

This gives LD option not to ask for layout in pg_init.

---
 fs/nfs/pnfs.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 46 insertions(+), 0 deletions(-)
fs/nfs/pnfs.c +46 -0

72 lines not on this page · raw message

Signature · 1 line
1.6.6

From 9813911e4763a36d3340a0ec9bdc118dc8d253b0 Mon Sep 17 00:00:00 2001
From: Peng Tao <[email protected]>
Date: Sat, 19 Nov 2011 04:46:49 -0500
Subject: [PATCH 4/4] pnfsblock: do not ask for layout in pg_init

Asking for layout in pg_init will always make client ask for only 4KB layout in every layoutget. This way, client drops the IO size information that is meaningful for MDS in handing out layout.

In stead, if layout is not find in cache, do not send layoutget at once. Wait until before issuing IO in pnfs_do_multiple_reads/writes because that is where we know the real size of current IO. By telling the real IO size to MDS, MDS will have a better chance to give proper layout.

---
 fs/nfs/blocklayout/blocklayout.c |   54 ++++++++++++++++++++++++++++++++++++-
 1 files changed, 52 insertions(+), 2 deletions(-)
fs/nfs/blocklayout/blocklayout.c +52 -2

72 lines not on this page · raw message

Signature · 1 line
1.6.6