Most of the following patches are bugfixes:
There are a couple of spin locking fixes to follow up the BKL removal.
There are some attribute update fixups, mainly to make the spin
locking more efficient
A fix for the problem of autobind on cloned RPC clients
The one feature that has been added is the lookup revalidation mount
option to allow clients to specify that they might not want to cache
negative dentries, and that they might want strict dentry revalidation.
--
Trond Myklebust
Linux NFS client maintainer
NetApp
[email protected]
http://www.netapp.com
On Tue, Oct 07, 2008 at 06:19:52PM -0400, Trond Myklebust wrote:
> Most of the following patches are bugfixes:
> There are a couple of spin locking fixes to follow up the BKL removal.
> There are some attribute update fixups, mainly to make the spin
> locking more efficient
> A fix for the problem of autobind on cloned RPC clients
>
> The one feature that has been added is the lookup revalidation mount
> option to allow clients to specify that they might not want to cache
> negative dentries, and that they might want strict dentry revalidation.
The version of the fs_locations fixes are slightly older than my most
recent. Looking back through the archive, I think it may have been my
mistake. Sorry!
There's a missing patch from Chuck that makes the ip address parsing a
bit more paranoid. I'll resend that, followed by the newer versions of
the other patches.
Aside from the missing patch from Chuck, the only difference between the
versions you have and my latest are summarized below. it's just:
- a cosmetic change to set_port to address your complaints about
blocks used just to define local variables, and
- changes to stop bothering to check for scope id's in v6
addresses.
So--could you just take the following patches, and throw out the 4
corresponding patches from me in your current tree?
--b.
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 625abae..14199c2 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -277,24 +277,21 @@ unsigned int nfs_page_array_len(unsigned int base, size_t len)
PAGE_SIZE - 1) >> PAGE_SHIFT;
}
-#define IPV6_SCOPE_DELIMITER '%'
-
/*
* Set the port number in an address. Be agnostic about the address
* family.
*/
static inline void nfs_set_port(struct sockaddr *sap, unsigned short port)
{
+ struct sockaddr_in *ap = (struct sockaddr_in *)sap;
+ struct sockaddr_in6 *ap6 = (struct sockaddr_in6 *)sap;
+
switch (sap->sa_family) {
- case AF_INET: {
- struct sockaddr_in *ap = (struct sockaddr_in *)sap;
- ap->sin_port = htons(port);
- break;
- }
- case AF_INET6: {
- struct sockaddr_in6 *ap = (struct sockaddr_in6 *)sap;
- ap->sin6_port = htons(port);
- break;
- }
+ case AF_INET:
+ ap->sin_port = htons(port);
+ break;
+ case AF_INET6:
+ ap6->sin6_port = htons(port);
+ break;
}
}
diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index 30befc3..13c37de 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -118,8 +118,6 @@ static struct vfsmount *try_location(struct nfs_clone_mount *mountdata,
mountdata->addr = (struct sockaddr *)&addr;
- if (memchr(buf->data, IPV6_SCOPE_DELIMITER, buf->len))
- continue;
nfs_parse_ip_address(buf->data, buf->len,
mountdata->addr, &mountdata->addrlen);
if (mountdata->addr->sa_family == AF_UNSPEC)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index b173653..78c0dd9 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -705,6 +705,8 @@ static void nfs_parse_ipv4_address(char *string, size_t str_len,
*addr_len = 0;
}
+#define IPV6_SCOPE_DELIMITER '%'
+
#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
static void nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
const char *delim,
This function is a little longer and more deeply nested than necessary.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfs/nfs4namespace.c | 84 ++++++++++++++++++++++++++---------------------
1 files changed, 46 insertions(+), 38 deletions(-)
diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index b112857..956cbbc 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -110,6 +110,48 @@ static inline int valid_ipaddr4(const char *buf)
return 0;
}
+static struct vfsmount *try_location(struct nfs_clone_mount *mountdata,
+ char *page, char *page2,
+ const struct nfs4_fs_location *location)
+{
+ struct vfsmount *mnt = ERR_PTR(-ENOENT);
+ char *mnt_path;
+ unsigned int s = 0;
+
+ mnt_path = nfs4_pathname_string(&location->rootpath, page2, PAGE_SIZE);
+ if (IS_ERR(mnt_path))
+ return mnt;
+ mountdata->mnt_path = mnt_path;
+
+ while (s < location->nservers) {
+ struct sockaddr_in addr = {
+ .sin_family = AF_INET,
+ .sin_port = htons(NFS_PORT),
+ };
+
+ if (location->servers[s].len <= 0 ||
+ valid_ipaddr4(location->servers[s].data) < 0) {
+ s++;
+ continue;
+ }
+
+ mountdata->hostname = location->servers[s].data;
+ addr.sin_addr.s_addr = in_aton(mountdata->hostname),
+ mountdata->addr = (struct sockaddr *)&addr;
+ mountdata->addrlen = sizeof(addr);
+
+ snprintf(page, PAGE_SIZE, "%s:%s",
+ mountdata->hostname,
+ mountdata->mnt_path);
+
+ mnt = vfs_kern_mount(&nfs4_referral_fs_type, 0, page, mountdata);
+ if (!IS_ERR(mnt))
+ break;
+ s++;
+ }
+ return mnt;
+}
+
/**
* nfs_follow_referral - set up mountpoint when hitting a referral on moved error
* @mnt_parent - mountpoint of parent directory
@@ -128,7 +170,6 @@ static struct vfsmount *nfs_follow_referral(const struct vfsmount *mnt_parent,
.authflavor = NFS_SB(mnt_parent->mnt_sb)->client->cl_auth->au_flavor,
};
char *page = NULL, *page2 = NULL;
- unsigned int s;
int loc, error;
if (locations == NULL || locations->nlocations <= 0)
@@ -153,9 +194,8 @@ static struct vfsmount *nfs_follow_referral(const struct vfsmount *mnt_parent,
}
loc = 0;
- while (loc < locations->nlocations && IS_ERR(mnt)) {
+ while (loc < locations->nlocations) {
const struct nfs4_fs_location *location = &locations->locations[loc];
- char *mnt_path;
if (location == NULL || location->nservers <= 0 ||
location->rootpath.ncomponents == 0) {
@@ -163,41 +203,9 @@ static struct vfsmount *nfs_follow_referral(const struct vfsmount *mnt_parent,
continue;
}
- mnt_path = nfs4_pathname_string(&location->rootpath, page2, PAGE_SIZE);
- if (IS_ERR(mnt_path)) {
- loc++;
- continue;
- }
- mountdata.mnt_path = mnt_path;
-
- s = 0;
- while (s < location->nservers) {
- struct sockaddr_in addr = {
- .sin_family = AF_INET,
- .sin_port = htons(NFS_PORT),
- };
-
- if (location->servers[s].len <= 0 ||
- valid_ipaddr4(location->servers[s].data) < 0) {
- s++;
- continue;
- }
-
- mountdata.hostname = location->servers[s].data;
- addr.sin_addr.s_addr = in_aton(mountdata.hostname),
- mountdata.addr = (struct sockaddr *)&addr;
- mountdata.addrlen = sizeof(addr);
-
- snprintf(page, PAGE_SIZE, "%s:%s",
- mountdata.hostname,
- mountdata.mnt_path);
-
- mnt = vfs_kern_mount(&nfs4_referral_fs_type, 0, page, &mountdata);
- if (!IS_ERR(mnt)) {
- break;
- }
- s++;
- }
+ mnt = try_location(&mountdata, page, page2, location);
+ if (!IS_ERR(mnt))
+ break;
loc++;
}
--
1.5.5.rc1
We plan to use this function elsewhere.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfs/internal.h | 18 ++++++++++++++++++
fs/nfs/super.c | 19 -------------------
2 files changed, 18 insertions(+), 19 deletions(-)
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 24241fc..926d8db 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -276,3 +276,21 @@ unsigned int nfs_page_array_len(unsigned int base, size_t len)
PAGE_SIZE - 1) >> PAGE_SHIFT;
}
+/*
+ * Set the port number in an address. Be agnostic about the address
+ * family.
+ */
+static inline void nfs_set_port(struct sockaddr *sap, unsigned short port)
+{
+ struct sockaddr_in *ap = (struct sockaddr_in *)sap;
+ struct sockaddr_in6 *ap6 = (struct sockaddr_in6 *)sap;
+
+ switch (sap->sa_family) {
+ case AF_INET:
+ ap->sin_port = htons(port);
+ break;
+ case AF_INET6:
+ ap6->sin6_port = htons(port);
+ break;
+ }
+}
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index c989e71..9608797 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -664,25 +664,6 @@ static void nfs_umount_begin(struct super_block *sb)
}
/*
- * Set the port number in an address. Be agnostic about the address family.
- */
-static void nfs_set_port(struct sockaddr *sap, unsigned short port)
-{
- switch (sap->sa_family) {
- case AF_INET: {
- struct sockaddr_in *ap = (struct sockaddr_in *)sap;
- ap->sin_port = htons(port);
- break;
- }
- case AF_INET6: {
- struct sockaddr_in6 *ap = (struct sockaddr_in6 *)sap;
- ap->sin6_port = htons(port);
- break;
- }
- }
-}
-
-/*
* Sanity-check a server address provided by the mount command.
*
* Address family must be initialized, and address must not be
--
1.5.5.rc1
The code incorrectly assumes here that the server name (or ip address)
is null-terminated. This can cause referrals to fail in some cases.
Also support ipv6 addresses.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfs/internal.h | 1 +
fs/nfs/nfs4namespace.c | 42 ++++++++++++++++--------------------------
fs/nfs/super.c | 2 +-
3 files changed, 18 insertions(+), 27 deletions(-)
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 926d8db..14199c2 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -153,6 +153,7 @@ extern void nfs4_clear_inode(struct inode *);
void nfs_zap_acl_cache(struct inode *inode);
/* super.c */
+void nfs_parse_ip_address(char *, size_t, struct sockaddr *, size_t *);
extern struct file_system_type nfs_xdev_fs_type;
#ifdef CONFIG_NFS_V4
extern struct file_system_type nfs4_xdev_fs_type;
diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index 6bcc569..13c37de 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -93,50 +93,40 @@ static int nfs4_validate_fspath(const struct vfsmount *mnt_parent,
return 0;
}
-/*
- * Check if the string represents a "valid" IPv4 address
- */
-static inline int valid_ipaddr4(const char *buf)
-{
- int rc, count, in[4];
-
- rc = sscanf(buf, "%d.%d.%d.%d", &in[0], &in[1], &in[2], &in[3]);
- if (rc != 4)
- return -EINVAL;
- for (count = 0; count < 4; count++) {
- if (in[count] > 255)
- return -EINVAL;
- }
- return 0;
-}
-
static struct vfsmount *try_location(struct nfs_clone_mount *mountdata,
char *page, char *page2,
const struct nfs4_fs_location *location)
{
struct vfsmount *mnt = ERR_PTR(-ENOENT);
char *mnt_path;
+ int page2len;
unsigned int s;
mnt_path = nfs4_pathname_string(&location->rootpath, page2, PAGE_SIZE);
if (IS_ERR(mnt_path))
return mnt;
mountdata->mnt_path = mnt_path;
+ page2 += strlen(mnt_path) + 1;
+ page2len = PAGE_SIZE - strlen(mnt_path) - 1;
for (s = 0; s < location->nservers; s++) {
- struct sockaddr_in addr = {
- .sin_family = AF_INET,
- .sin_port = htons(NFS_PORT),
- };
+ const struct nfs4_string *buf = &location->servers[s];
+ struct sockaddr_storage addr;
- if (location->servers[s].len <= 0 ||
- valid_ipaddr4(location->servers[s].data) < 0)
+ if (buf->len <= 0 || buf->len >= PAGE_SIZE)
continue;
- mountdata->hostname = location->servers[s].data;
- addr.sin_addr.s_addr = in_aton(mountdata->hostname),
mountdata->addr = (struct sockaddr *)&addr;
- mountdata->addrlen = sizeof(addr);
+
+ nfs_parse_ip_address(buf->data, buf->len,
+ mountdata->addr, &mountdata->addrlen);
+ if (mountdata->addr->sa_family == AF_UNSPEC)
+ continue;
+ nfs_set_port(mountdata->addr, NFS_PORT);
+
+ strncpy(page2, buf->data, page2len);
+ page2[page2len] = '\0';
+ mountdata->hostname = page2;
snprintf(page, PAGE_SIZE, "%s:%s",
mountdata->hostname,
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 9608797..713c75a 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -791,7 +791,7 @@ static void nfs_parse_ipv6_address(char *string, size_t str_len,
* If there is a problem constructing the new sockaddr, set the address
* family to AF_UNSPEC.
*/
-static void nfs_parse_ip_address(char *string, size_t str_len,
+void nfs_parse_ip_address(char *string, size_t str_len,
struct sockaddr *sap, size_t *addr_len)
{
unsigned int i, colons;
--
1.5.5.rc1
Whoever wrote this had a bizarre allergy to for loops.
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfs/nfs4namespace.c | 17 +++++------------
1 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index 956cbbc..6bcc569 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -116,24 +116,22 @@ static struct vfsmount *try_location(struct nfs_clone_mount *mountdata,
{
struct vfsmount *mnt = ERR_PTR(-ENOENT);
char *mnt_path;
- unsigned int s = 0;
+ unsigned int s;
mnt_path = nfs4_pathname_string(&location->rootpath, page2, PAGE_SIZE);
if (IS_ERR(mnt_path))
return mnt;
mountdata->mnt_path = mnt_path;
- while (s < location->nservers) {
+ for (s = 0; s < location->nservers; s++) {
struct sockaddr_in addr = {
.sin_family = AF_INET,
.sin_port = htons(NFS_PORT),
};
if (location->servers[s].len <= 0 ||
- valid_ipaddr4(location->servers[s].data) < 0) {
- s++;
+ valid_ipaddr4(location->servers[s].data) < 0)
continue;
- }
mountdata->hostname = location->servers[s].data;
addr.sin_addr.s_addr = in_aton(mountdata->hostname),
@@ -147,7 +145,6 @@ static struct vfsmount *try_location(struct nfs_clone_mount *mountdata,
mnt = vfs_kern_mount(&nfs4_referral_fs_type, 0, page, mountdata);
if (!IS_ERR(mnt))
break;
- s++;
}
return mnt;
}
@@ -193,20 +190,16 @@ static struct vfsmount *nfs_follow_referral(const struct vfsmount *mnt_parent,
goto out;
}
- loc = 0;
- while (loc < locations->nlocations) {
+ for (loc = 0; loc < locations->nlocations; loc++) {
const struct nfs4_fs_location *location = &locations->locations[loc];
if (location == NULL || location->nservers <= 0 ||
- location->rootpath.ncomponents == 0) {
- loc++;
+ location->rootpath.ncomponents == 0)
continue;
- }
mnt = try_location(&mountdata, page, page2, location);
if (!IS_ERR(mnt))
break;
- loc++;
}
out:
--
1.5.5.rc1
From: Chuck Lever <[email protected]>
Bruce observed that nfs_parse_ip_address() will successfully parse an
IPv6 address that looks like this:
"::1%"
A scope delimiter is present, but there is no scope ID following it.
This is harmless, as it would simply set the scope ID to zero. However,
in some cases we would like to flag this as an improperly formed
address.
We are now also careful to reject addresses where garbage follows the
address (up to the length of the string), instead of ignoring the
non-address characters; and where the scope ID is nonsense (not a valid
device name, but also not numeric). Before, both of these cases would
result in a harmless zero scope ID.
Signed-off-by: Chuck Lever <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfs/super.c | 34 +++++++++++++++++++++++-----------
1 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 9abcd2b..c989e71 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -727,17 +727,21 @@ static void nfs_parse_ipv4_address(char *string, size_t str_len,
#define IPV6_SCOPE_DELIMITER '%'
#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
-static void nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
- const char *delim,
- struct sockaddr_in6 *sin6)
+static int nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
+ const char *delim,
+ struct sockaddr_in6 *sin6)
{
char *p;
size_t len;
- if (!(ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL))
- return ;
+ if ((string + str_len) == delim)
+ return 1;
+
if (*delim != IPV6_SCOPE_DELIMITER)
- return;
+ return 0;
+
+ if (!(ipv6_addr_type(&sin6->sin6_addr) & IPV6_ADDR_LINKLOCAL))
+ return 0;
len = (string + str_len) - delim - 1;
p = kstrndup(delim + 1, len, GFP_KERNEL);
@@ -750,14 +754,20 @@ static void nfs_parse_ipv6_scope_id(const char *string, const size_t str_len,
scope_id = dev->ifindex;
dev_put(dev);
} else {
- /* scope_id is set to zero on error */
- strict_strtoul(p, 10, &scope_id);
+ if (strict_strtoul(p, 10, &scope_id) == 0) {
+ kfree(p);
+ return 0;
+ }
}
kfree(p);
+
sin6->sin6_scope_id = scope_id;
dfprintk(MOUNT, "NFS: IPv6 scope ID = %lu\n", scope_id);
+ return 1;
}
+
+ return 0;
}
static void nfs_parse_ipv6_address(char *string, size_t str_len,
@@ -773,9 +783,11 @@ static void nfs_parse_ipv6_address(char *string, size_t str_len,
sin6->sin6_family = AF_INET6;
*addr_len = sizeof(*sin6);
- if (in6_pton(string, str_len, addr, IPV6_SCOPE_DELIMITER, &delim)) {
- nfs_parse_ipv6_scope_id(string, str_len, delim, sin6);
- return;
+ if (in6_pton(string, str_len, addr,
+ IPV6_SCOPE_DELIMITER, &delim) != 0) {
+ if (nfs_parse_ipv6_scope_id(string, str_len,
+ delim, sin6) != 0)
+ return;
}
}
--
1.5.5.rc1
On Wed, 2008-10-08 at 15:31 -0400, J. Bruce Fields wrote:
> On Tue, Oct 07, 2008 at 06:19:52PM -0400, Trond Myklebust wrote:
> > Most of the following patches are bugfixes:
> > There are a couple of spin locking fixes to follow up the BKL removal.
> > There are some attribute update fixups, mainly to make the spin
> > locking more efficient
> > A fix for the problem of autobind on cloned RPC clients
> >
> > The one feature that has been added is the lookup revalidation mount
> > option to allow clients to specify that they might not want to cache
> > negative dentries, and that they might want strict dentry revalidation.
>
> The version of the fs_locations fixes are slightly older than my most
> recent. Looking back through the archive, I think it may have been my
> mistake. Sorry!
>
> There's a missing patch from Chuck that makes the ip address parsing a
> bit more paranoid. I'll resend that, followed by the newer versions of
> the other patches.
>
> Aside from the missing patch from Chuck, the only difference between the
> versions you have and my latest are summarized below. it's just:
>
> - a cosmetic change to set_port to address your complaints about
> blocks used just to define local variables, and
> - changes to stop bothering to check for scope id's in v6
> addresses.
>
> So--could you just take the following patches, and throw out the 4
> corresponding patches from me in your current tree?
I'll split your patch into 2 and apply on top the existing tree.
Trond
--
Trond Myklebust
Linux NFS client maintainer
NetApp
[email protected]
http://www.netapp.com
On Wed, Oct 08, 2008 at 03:37:50PM -0400, Trond Myklebust wrote:
> On Wed, 2008-10-08 at 15:31 -0400, J. Bruce Fields wrote:
> > On Tue, Oct 07, 2008 at 06:19:52PM -0400, Trond Myklebust wrote:
> > > Most of the following patches are bugfixes:
> > > There are a couple of spin locking fixes to follow up the BKL removal.
> > > There are some attribute update fixups, mainly to make the spin
> > > locking more efficient
> > > A fix for the problem of autobind on cloned RPC clients
> > >
> > > The one feature that has been added is the lookup revalidation mount
> > > option to allow clients to specify that they might not want to cache
> > > negative dentries, and that they might want strict dentry revalidation.
> >
> > The version of the fs_locations fixes are slightly older than my most
> > recent. Looking back through the archive, I think it may have been my
> > mistake. Sorry!
> >
> > There's a missing patch from Chuck that makes the ip address parsing a
> > bit more paranoid. I'll resend that, followed by the newer versions of
> > the other patches.
> >
> > Aside from the missing patch from Chuck, the only difference between the
> > versions you have and my latest are summarized below. it's just:
> >
> > - a cosmetic change to set_port to address your complaints about
> > blocks used just to define local variables, and
> > - changes to stop bothering to check for scope id's in v6
> > addresses.
> >
> > So--could you just take the following patches, and throw out the 4
> > corresponding patches from me in your current tree?
>
> I'll split your patch into 2 and apply on top the existing tree.
That'll work too, thanks; let me know if you'd like me to do that for
you.
(And note also that patch #1, from Chuck, also needs to be applied.)
--b.