Re: Different sequence of "exportfs" produce different effects on nfs client mounts

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 3510 bytes --]

On Fri, 18 Oct 2013 08:32:53 +1100 NeilBrown <neilb@suse.de> wrote:

> On Thu, 17 Oct 2013 09:14:02 -0400 Chuck Lever <chuck.lever@oracle.com> wrote:
> 
> > Hi-
> > 
> > 281         if (ident[0] == '\0')
> > 282                 return MCL_ANONYMOUS;
> > 283         if (ident[0] == '@') {
> > 284 #ifndef HAVE_INNETGR
> > 285                 xlog(L_WARNING, "netgroup support not compiled in");
> > 286 #endif
> > 287                 return MCL_NETGROUP;
> > 288         }
> > 289         for (sp = ident; *sp; sp++) {
> > 290                 if (*sp == '*' || *sp == '?' || *sp == '[')
> > 291                         return MCL_WILDCARD;
> > 292                 if (*sp == '/')
> > 293                         return MCL_SUBNETWORK;
> > 294                 if (*sp == '\\' && sp[1])
> > 295                         sp++;
> > 296         }
> > 
> > is still in play today.  The "host_pton()" code you posted was added by commit 502edf1d just after this paragraph.  But here is what that commit replaced.
> > 
> > -       /* check for N.N.N.N */
> > -       sp = ident;
> > -       if(!isdigit(*sp) || strtoul(sp, &sp, 10) > 255 || *sp != '.') return MCL_FQDN;
> > -       sp++; if(!isdigit(*sp) || strtoul(sp, &sp, 10) > 255 || *sp != '.') return MCL_F
> > -       sp++; if(!isdigit(*sp) || strtoul(sp, &sp, 10) > 255 || *sp != '.') return MCL_F
> > -       sp++; if(!isdigit(*sp) || strtoul(sp, &sp, 10) > 255 || *sp != '\0') return MCL_
> > -       /* we lie here a bit. but technically N.N.N.N == N.N.N.N/32 :) */
> > -       return MCL_SUBNETWORK;
> > +
> > +       /*
> > +        * Treat unadorned IP addresses as MCL_SUBNETWORK.
> > +        * Everything else is MCL_FQDN.
> > +        */
> > +       ai = host_pton(ident);
> > +       if (ai != NULL) {
> > +               freeaddrinfo(ai);
> > +               return MCL_SUBNETWORK;
> > +       }
> > +
> > +       return MCL_FQDN;
> >  }
> > 
> > The replaced logic also treats IP addresses as MCL_SUBNETWORK.  That's from commit 54669c98 in 2005.  Neil, do you remember why this semantic change was made?
> > 
> 
> See this thread:
> 
>   http://marc.info/?t=110993941000003&r=1&w=2
> 
> It was a simple (though possibly flawed) solution to clearly differentiate
> those addresses where DNS looked might be needed, and those where it was not.
> 
> I may have more to say later but I have to rush off now, so thought I'd just
> post this anyway.
> 

Unfortunately I cannot see how that change ever made any important
difference, and the email exchanges ends without resolving anything.

It could only make a difference to the number of DNS lookups if there was
somewhere a test for whether clientlist[MCL_FQDN] was NULL, but there isn't
and never was.

So it seems very likely that:

diff --git a/support/export/client.c b/support/export/client.c
index ba2db8f..adbeed8 100644
--- a/support/export/client.c
+++ b/support/export/client.c
@@ -767,15 +767,5 @@ client_gettype(char *ident)
 			sp++;
 	}
 
-	/*
-	 * Treat unadorned IP addresses as MCL_SUBNETWORK.
-	 * Everything else is MCL_FQDN.
-	 */
-	ai = host_pton(ident);
-	if (ai != NULL) {
-		freeaddrinfo(ai);
-		return MCL_SUBNETWORK;
-	}
-
 	return MCL_FQDN;
 }

is appropriate and may well fix the current issue.

It would be good to test how many DNS looks (hopefully none) are performed
when using a exports file that contains only IP addresses, both before and
after the patch.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]