NFS: Add support for AF_INET6 addresses in nfs_compare_super()

PATCH In review Chuck Lever 2 participants 3 messages Nov 30, 2007 2 messages not in the archive

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

Chuck Lever

author

2007-11-30 19:13 UTC #0 lore raw

PATCH 1/1 NFS: Add support for AF_INET6 addresses in nfs_compare_super()

Refactor nfs_compare_super() and add AF_INET6 support.

Replace the generic memcmp() to document explicitly what parts of the addresses must match in this check, and make the comparison independent of the lengths of both addresses.

A side benefit is both tests are more computationally efficient than a memcmp().

Note the "void *" casts are temporary.

Signed-off-by Chuck Lever <[email protected]>

---

 fs/nfs/super.c |   41 ++++++++++++++++++++++++++++++++++++++---
 1 files changed, 38 insertions(+), 3 deletions(-)

fs/nfs/super.c +38 -3

index 6b004d8..e5646a9 100644
@@ -45,6 +45,8 @@
 #include <linux/nfs_idmap.h>
 #include <linux/vfs.h>
 #include <linux/inet.h>
+#include <linux/in6.h>
+#include <net/ipv6.h>
 #include <linux/nfs_xdr.h>
 #include <linux/magic.h>
 #include <linux/parser.h>

@@ -1302,15 +1304,48 @@ static int nfs_set_super(struct super_block *s, void *data)
 	return ret;
 }
 
+static int nfs_compare_super_address(struct nfs_server *server1,
+				     struct nfs_server *server2)
+{
+	struct sockaddr_storage *ssp1 = (void *)&server1->nfs_client->cl_addr;
+	struct sockaddr_storage *ssp2 = (void *)&server2->nfs_client->cl_addr;
+
+	if (ssp1->ss_family != ssp2->ss_family)
+		return 0;
+
+	switch (ssp1->ss_family) {
+	case AF_INET: {
+		struct sockaddr_in *sin1 = (struct sockaddr_in *)ssp1;
+		struct sockaddr_in *sin2 = (struct sockaddr_in *)ssp2;
+		if (sin1->sin_addr.s_addr != sin2->sin_addr.s_addr)
+			return 0;
+		if (sin1->sin_port != sin2->sin_port)
+			return 0;
+		break;
+	}
+	case AF_INET6: {
+		struct sockaddr_in6 *sin1 = (struct sockaddr_in6 *)ssp1;
+		struct sockaddr_in6 *sin2 = (struct sockaddr_in6 *)ssp2;
+		if (!ipv6_addr_equal(&sin1->sin6_addr, &sin2->sin6_addr))
+			return 0;
+		if (sin1->sin6_port != sin2->sin6_port)
+			return 0;
+		break;
+	}
+	default:
+		return 0;
+	}
+
+	return 1;
+}
+
 static int nfs_compare_super(struct super_block *sb, void *data)
 {
 	struct nfs_sb_mountdata *sb_mntdata = data;
 	struct nfs_server *server = sb_mntdata->server, *old = NFS_SB(sb);
 	int mntflags = sb_mntdata->mntflags;
 
-	if (memcmp(&old->nfs_client->cl_addr,
-				&server->nfs_client->cl_addr,
-				sizeof(old->nfs_client->cl_addr)) != 0)
+	if (!nfs_compare_super_address(old, server))
 		return 0;
 	/* Note: NFS_MOUNT_UNSHARED == NFS4_MOUNT_UNSHARED */
 	if (old->flags & NFS_MOUNT_UNSHARED)

-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper from Novell. From the desktop to the data center, Linux is going mainstream. Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
_______________________________________________
Please note that [email protected] is being discontinued. Please subscribe to [email protected] instead.
http://vger.kernel.org/vger-lists.html#linux-nfs

A message that is not in the archive (sent off-list or not yet received): 20071130191237.3431.12598.stgit-uiCyzNpeAnB9HhUboXbp9zCvJB+x5qRC@public.gmane.org

Trond Myklebust

2007-11-30 19:18 UTC #2 lore raw

Quoted 40 lines · Chuck Lever

Refactor nfs_compare_super() and add AF_INET6 support.
Replace the generic memcmp() to document explicitly what parts of the addresses must match in this check, and make the comparison independent of the lengths of both addresses.
A side benefit is both tests are more computationally efficient than a memcmp().
Note the "void *" casts are temporary.
Signed-off-by Chuck Lever <[email protected]>
---
 fs/nfs/super.c |   41 ++++++++++++++++++++++++++++++++++++++---
 1 files changed, 38 insertions(+), 3 deletions(-)
fs/nfs/super.c +8 -0
index 6b004d8..e5646a9 100644
@@ -45,6 +45,8 @@
 #include <linux/nfs_idmap.h>
 #include <linux/vfs.h>
 #include <linux/inet.h>
+#include <linux/in6.h>
+#include <net/ipv6.h>
 #include <linux/nfs_xdr.h>
 #include <linux/magic.h>
 #include <linux/parser.h>
@@ -1302,15 +1304,48 @@ static int nfs_set_super(struct super_block *s, void *data)
 	return ret;
 }
 
+static int nfs_compare_super_address(struct nfs_server *server1,
+				     struct nfs_server *server2)
+{
+	struct sockaddr_storage *ssp1 = (void *)&server1->nfs_client->cl_addr;
+	struct sockaddr_storage *ssp2 = (void *)&server2->nfs_client->cl_addr;
+

fs/nfs/super.c static int nfs_set_super(struct super_block *s, void *data) · line 1322

Quoted 10 lines

fs/nfs/super.c +10 -0

+	if (ssp1->ss_family != ssp2->ss_family)
+		return 0;
+
+	switch (ssp1->ss_family) {
+	case AF_INET: {
+		struct sockaddr_in *sin1 = (struct sockaddr_in *)ssp1;
+		struct sockaddr_in *sin2 = (struct sockaddr_in *)ssp2;
+		if (sin1->sin_addr.s_addr != sin2->sin_addr.s_addr)
+			return 0;
+		if (sin1->sin_port != sin2->sin_port)

What if sin2->sin_port == 0?

Quoted 23 lines

fs/nfs/super.c +19 -0

+			return 0;
+		break;
+	}
+	case AF_INET6: {
+		struct sockaddr_in6 *sin1 = (struct sockaddr_in6 *)ssp1;
+		struct sockaddr_in6 *sin2 = (struct sockaddr_in6 *)ssp2;
+		if (!ipv6_addr_equal(&sin1->sin6_addr, &sin2->sin6_addr))
+			return 0;
+		if (sin1->sin6_port != sin2->sin6_port)
+			return 0;
+		break;
+	}
+	default:
+		return 0;
+	}
+
+	return 1;
+}
+
 static int nfs_compare_super(struct super_block *sb, void *data)
 {
 	struct nfs_sb_mountdata *sb_mntdata = data;

fs/nfs/super.c static int nfs_set_super(struct super_block *s, void *data) · line 1351

Quoted 10 lines

fs/nfs/super.c +1 -3

 	struct nfs_server *server = sb_mntdata->server, *old = NFS_SB(sb);
 	int mntflags = sb_mntdata->mntflags;
 
-	if (memcmp(&old->nfs_client->cl_addr,
-				&server->nfs_client->cl_addr,
-				sizeof(old->nfs_client->cl_addr)) != 0)
+	if (!nfs_compare_super_address(old, server))
 		return 0;
 	/* Note: NFS_MOUNT_UNSHARED == NFS4_MOUNT_UNSHARED */
 	if (old->flags & NFS_MOUNT_UNSHARED)

A message that is not in the archive (sent off-list or not yet received): 1196450263.17561.0.camel-rJ7iovZKK19ZJLDQqaL3InhyD016LWXt@public.gmane.org

Chuck Lever

author

2007-11-30 20:12 UTC #4 lore raw

Quoted 46 lines · Trond Myklebust

Quoted 44 lines · Chuck Lever
Refactor nfs_compare_super() and add AF_INET6 support.
Replace the generic memcmp() to document explicitly what parts of the addresses must match in this check, and make the comparison independent
of the lengths of both addresses.
A side benefit is both tests are more computationally efficient than a
memcmp().
Note the "void *" casts are temporary.
Signed-off-by Chuck Lever <[email protected]>
---
 fs/nfs/super.c |   41 ++++++++++++++++++++++++++++++++++++++---
 1 files changed, 38 insertions(+), 3 deletions(-)
fs/nfs/super.c +7 -0
index 6b004d8..e5646a9 100644
@@ -45,6 +45,8 @@
 #include <linux/nfs_idmap.h>
 #include <linux/vfs.h>
 #include <linux/inet.h>
+#include <linux/in6.h>
+#include <net/ipv6.h>
 #include <linux/nfs_xdr.h>
 #include <linux/magic.h>
 #include <linux/parser.h>
@@ -1302,15 +1304,48 @@ static int nfs_set_super(struct  
 super_block *s, void *data)
 	return ret;
 }
 
+static int nfs_compare_super_address(struct nfs_server *server1,
+				     struct nfs_server *server2)
+{
+	struct sockaddr_storage *ssp1 = (void *)&server1->nfs_client- 
 >cl_addr;
+	struct sockaddr_storage *ssp2 = (void *)&server2->nfs_client- 
 >cl_addr;

Quoted 13 lines

fs/nfs/super.c static int nfs_set_super(struct super_block *s, void *data) · line 1322

Quoted 11 lines

fs/nfs/super.c +11 -0

+
+	if (ssp1->ss_family != ssp2->ss_family)
+		return 0;
+
+	switch (ssp1->ss_family) {
+	case AF_INET: {
+		struct sockaddr_in *sin1 = (struct sockaddr_in *)ssp1;
+		struct sockaddr_in *sin2 = (struct sockaddr_in *)ssp2;
+		if (sin1->sin_addr.s_addr != sin2->sin_addr.s_addr)
+			return 0;
+		if (sin1->sin_port != sin2->sin_port)

What if sin2->sin_port == 0?

The original comparison in nfs_compare_super() uses a memcmp() to compare sizeof(struct sockaddr_in) bytes, which would include the sin_port field of both addresses, right? Thus the original comparison would find a match when sin2->sin_port is zero only if sin1->sin_port is also zero.

AFAICT, the comparisons in nfs_compare_super_address() provide precisely the same check as the memcmp() in the original.

Quoted 25 lines

fs/nfs/super.c +19 -0

+			return 0;
+		break;
+	}
+	case AF_INET6: {
+		struct sockaddr_in6 *sin1 = (struct sockaddr_in6 *)ssp1;
+		struct sockaddr_in6 *sin2 = (struct sockaddr_in6 *)ssp2;
+		if (!ipv6_addr_equal(&sin1->sin6_addr, &sin2->sin6_addr))
+			return 0;
+		if (sin1->sin6_port != sin2->sin6_port)
+			return 0;
+		break;
+	}
+	default:
+		return 0;
+	}
+
+	return 1;
+}
+
 static int nfs_compare_super(struct super_block *sb, void *data)
 {
 	struct nfs_sb_mountdata *sb_mntdata = data;
 	struct nfs_server *server = sb_mntdata->server, *old = NFS_SB(sb);
 	int mntflags = sb_mntdata->mntflags;

Quoted 9 lines

fs/nfs/super.c static int nfs_set_super(struct super_block *s, void *data) · line 1351

Quoted 8 lines

fs/nfs/super.c +1 -3

-	if (memcmp(&old->nfs_client->cl_addr,
-				&server->nfs_client->cl_addr,
-				sizeof(old->nfs_client->cl_addr)) != 0)
+	if (!nfs_compare_super_address(old, server))

fs/nfs/super.c +0 -0

 		return 0;
 	/* Note: NFS_MOUNT_UNSHARED == NFS4_MOUNT_UNSHARED */
 	if (old->flags & NFS_MOUNT_UNSHARED)

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