On Wed, 2009-02-04 at 13:47 -0500, Trond Myklebust wrote:
> On Wed, 2009-02-04 at 13:44 -0500, Chuck Lever wrote:
> > Right. So, then, can nfs_match_client() use the same address
> > comparison method as nfs_find_client()? I'm inclined to think
> > nfs_match_client() can use a more straightforward address comparison
> > method.
>
> It could. We should certainly be checking that the sa_family field
> matches...
Based on this, here is a new version of the patch. I've dropped the
contentious clean up of the conditional compilation. The match is done
in a new function nfs_sockaddr_match(). Even though this function
compares IPv4 and IPv6 addresses it compiles and runs OK even when IPV6
is not configured, because the necessary data structures and macros are
still defined.
When IPV6 is not configured a few bytes (of executable) could be saved
by making two versions of nfs_sockaddr_match() conditional on
CONFIG_IPV6 but personally I don't think it is worth it.
Log:
======================
sockaddr structures can't be reliably compared using memcmp()
because there are padding bytes in the structure which can't be
guaranteed to be the same even when the sockaddr structures refer to
the same socket.
Signed-off-by: Ian Dall <ian@xxxxxxxxxxxxxxxxxxxxx>
======================
diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 9b728f3..04b11d9 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -272,6 +272,42 @@ static int nfs_sockaddr_match_ipaddr(const struct sockaddr *sa1,
}
#endif
+/* Test if two socket addresses are the same family, ipaddr and port */
+static int nfs_sockaddr_match(const struct sockaddr *sa1,
+ const struct sockaddr *sa2)
+{
+ if (sa1->sa_family != sa2->sa_family)
+ return 0;
+
+ switch (sa1->sa_family) {
+ case AF_INET:
+ {
+ const struct sockaddr_in * saddr1 =
+ (const struct sockaddr_in *) sa1;
+ const struct sockaddr_in * saddr2 =
+ (const struct sockaddr_in *) sa2;
+ if (likely(saddr1->sin_addr.s_addr !=
+ saddr2->sin_addr.s_addr)) {
+ return 0;
+ }
+ return (saddr1->sin_port == saddr2->sin_port);
+ }
+ case AF_INET6:
+ {
+ const struct sockaddr_in6 * saddr1 =
+ (const struct sockaddr_in6 *) sa1;
+ const struct sockaddr_in6 * saddr2 =
+ (const struct sockaddr_in6 *) sa2;
+ if (likely(!ipv6_addr_equal(&saddr1->sin6_addr,
+ &saddr1->sin6_addr))) {
+ return 0;
+ }
+ return (saddr1->sin6_port == saddr2->sin6_port);
+ }
+ }
+ return 0;
+}
+
/*
* Find a client by IP address and protocol version
* - returns NULL if no such client
@@ -344,8 +380,10 @@ struct nfs_client *nfs_find_client_next(struct nfs_client *clp)
static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *data)
{
struct nfs_client *clp;
+ const struct sockaddr *sap = data->addr;
list_for_each_entry(clp, &nfs_client_list, cl_share_link) {
+ const struct sockaddr *clap = (struct sockaddr *)&clp->cl_addr;
/* Don't match clients that failed to initialise properly */
if (clp->cl_cons_state < 0)
continue;
@@ -358,7 +396,7 @@ static struct nfs_client *nfs_match_client(const struct nfs_client_initdata *dat
continue;
/* Match the full socket address */
- if (memcmp(&clp->cl_addr, data->addr, sizeof(clp->cl_addr)) != 0)
+ if (!nfs_sockaddr_match(sap, clap))
continue;
atomic_inc(&clp->cl_count);
--
Ian Dall <ian@xxxxxxxxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
