On Fri, 18 Oct 2013 08:32:53 +1100 NeilBrown <neilb@xxxxxxx> wrote:
> On Thu, 17 Oct 2013 09:14:02 -0400 Chuck Lever <chuck.lever@xxxxxxxxxx> 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:
signature.asc
Description: PGP signature
