| Message ID | 20121031121601.5de16a5b@notabene.brown (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Thanks, Neil! On Oct 30, 2012, at 9:16 PM, NeilBrown <neilb@suse.de> wrote: > > The DNS resolver's use of the sunrpc cache involves a 'ttl' number > (relative) rather that a timeout (absolute). This confused me when > I wrote > commit c5b29f885afe890f953f7f23424045cdad31d3e4 > "sunrpc: use seconds since boot in expiry cache" > > and I managed to break it. The effect is that any TTL is interpreted > as 0, and nothing useful gets into the cache. > > This patch removes the use of get_expiry() - which really expects an > expiry time - and uses get_uint() instead, treating the int correctly > as a ttl. Nit: The description says "get_uint()" but the patch uses "get_int()." Should we change the patch? > This fixes a regression that has been present since 2.6.37, causing > certain NFS accesses in certain environments to incorrectly fail. In particular, when the legacy DNS resolver is in use, this regression prevents the NFS client from following any NFSv4 referral which contains a DNS hostname. > Reported-by: Chuck Lever <chuck.lever@oracle.com> > Tested-by: Chuck Lever <chuck.lever@oracle.com> > Cc: stable@vger.kernel.org > Signed-off-by: NeilBrown <neilb@suse.de> > > diff --git a/fs/nfs/dns_resolve.c b/fs/nfs/dns_resolve.c > index 31c26c4..d9415a2 100644 > --- a/fs/nfs/dns_resolve.c > +++ b/fs/nfs/dns_resolve.c > @@ -217,7 +217,7 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen) > { > char buf1[NFS_DNS_HOSTNAME_MAXLEN+1]; > struct nfs_dns_ent key, *item; > - unsigned long ttl; > + unsigned int ttl; > ssize_t len; > int ret = -EINVAL; > > @@ -240,7 +240,8 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen) > key.namelen = len; > memset(&key.h, 0, sizeof(key.h)); > > - ttl = get_expiry(&buf); > + if (get_int(&buf, &ttl) < 0) > + goto out; > if (ttl == 0) > goto out; > key.h.expiry_time = ttl + seconds_since_boot();
diff --git a/fs/nfs/dns_resolve.c b/fs/nfs/dns_resolve.c index 31c26c4..d9415a2 100644 --- a/fs/nfs/dns_resolve.c +++ b/fs/nfs/dns_resolve.c @@ -217,7 +217,7 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen) { char buf1[NFS_DNS_HOSTNAME_MAXLEN+1]; struct nfs_dns_ent key, *item; - unsigned long ttl; + unsigned int ttl; ssize_t len; int ret = -EINVAL; @@ -240,7 +240,8 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen) key.namelen = len; memset(&key.h, 0, sizeof(key.h)); - ttl = get_expiry(&buf); + if (get_int(&buf, &ttl) < 0) + goto out; if (ttl == 0) goto out; key.h.expiry_time = ttl + seconds_since_boot();
The DNS resolver's use of the sunrpc cache involves a 'ttl' number (relative) rather that a timeout (absolute). This confused me when I wrote commit c5b29f885afe890f953f7f23424045cdad31d3e4 "sunrpc: use seconds since boot in expiry cache" and I managed to break it. The effect is that any TTL is interpreted as 0, and nothing useful gets into the cache. This patch removes the use of get_expiry() - which really expects an expiry time - and uses get_uint() instead, treating the int correctly as a ttl. This fixes a regression that has been present since 2.6.37, causing certain NFS accesses in certain environments to incorrectly fail. Reported-by: Chuck Lever <chuck.lever@oracle.com> Tested-by: Chuck Lever <chuck.lever@oracle.com> Cc: stable@vger.kernel.org Signed-off-by: NeilBrown <neilb@suse.de>