[PATCH 0/2] mountd: clean up rmtab handling

[PATCH 0/2] mountd: clean up rmtab handling

[Jeff Layton]

We have had a few reports from customers that showmount -a is just plain
broken. The manpage says that it should "List both the client hostname
and mounted directory in  host:dir format". Instead, when used with
recent kernels, the output can look something like this (after mounts
from several hosts):

*:/foo
*:/scratch
*,foo.bar.baz:/scratch
10.10.1.41:*
172.16.33.192:*
172.16.33.192:*,foo.bar.baz

The output here varies somewhat depending on how the export table on the
server is configured, but the key is that we're getting multiple lines
here for every mount (some that don't even contain a path), and the
hostname field is often not a hostname or IP address.

The problem seems to be in how the rmtab is being managed and in how
mountd handles the hostname of the client when checking permissions and
such.

The following two patches remedy this. They both should only affect the
case where new_cache is set.

-- Jeff



-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 0/2] mountd: clean up rmtab handling

[Jeff Layton]

Jeff Layton wrote:
> We have had a few reports from customers that showmount -a is just plain
> broken.

...

> The following two patches remedy this. They both should only affect the
> case where new_cache is set.
> 
> -- Jeff

It's been a little over a month since I originally posted these patches 
and haven't received much comment though Neil and I have traded some 
emails off list concerning it. I'd like to move the discussion back to 
the mailing list to get a wider audience and see if we can come to some 
sort of resolution.

When we last communicated, he mentioned a scheme whereby we could 
generate the showmount -a info on the fly rather than trying to maintain 
the rmtab. That sounds like a good approach, but I don't see how that 
can work without some changes in how the caches are managed.

Neil, would it be reasonable to commit these patches now to fix the 
immediate problem and then work on the new scheme that synthesizes the 
info out of the kernel caches like you proposed?

-- Jeff

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 0/2] mountd: clean up rmtab handling

[Neil Brown]

On Friday December 1, jlayton@poochiereds.net wrote:
> Jeff Layton wrote:
> > We have had a few reports from customers that showmount -a is just plain
> > broken.
> 
> ...
> 
> > The following two patches remedy this. They both should only affect the
> > case where new_cache is set.
> > 
> > -- Jeff
> 
> It's been a little over a month since I originally posted these patches 
> and haven't received much comment though Neil and I have traded some 
> emails off list concerning it. I'd like to move the discussion back to 
> the mailing list to get a wider audience and see if we can come to some 
> sort of resolution.
> 
> When we last communicated, he mentioned a scheme whereby we could 
> generate the showmount -a info on the fly rather than trying to maintain 
> the rmtab. That sounds like a good approach, but I don't see how that 
> can work without some changes in how the caches are managed.
> 
> Neil, would it be reasonable to commit these patches now to fix the 
> immediate problem and then work on the new scheme that synthesizes the 
> info out of the kernel caches like you proposed?

I'm not convinced... I don't like the idea of mapping an IP address to
a hostname and then just working with the hostname.  Because it is
really IP addresses that you trust, not host names (in the case of
multi-homed hosts particularly).

The following patch is a first cut at using the kernel caches for
providing the information for "showmount -a".

Note that this does *not* list precisely those hosts which currently
have filesystems mounted, and precisely which filesystems.  That
information is simply not available reliably.

What it does do is list all the 
  IP-address   exportpoint
combinations that could possibly have been used in the last half hour.

As an example of how this will be less precise than you might like,
suppose you export
       /foo @hostgroup()
       /bar @hostgroup()

and suppose 1.2.3.4 and 5.6.7.8 both map to hosts in @hostgroup.

Support further than 1.2.3.4 has mounted and accessed /foo recently,
and 5.6.7.8 has mounted and accessed /bar recently.

Then the kernel will know:

   1.2.3.4 -> @hostgroup
   5.6.7.8 -> @hostgroup

   @hostgroup can access /foo
   @hostgroup can access /bar

Then this patch will cause "showmount -a" to report

   1.2.3.4:/foo
   1.2.3.4:/bar
   5.6.7.8:/foo
   5.6.7.8:/bar
   @hostgroup:/foo
   @hostgroup:/bar

It would be possible (thought I would rather make it optional) to give
the kernel more fine detail. i.e. not group multiple IP addresses
under the one client name but give each IP address it's own name.
This you would have

    1.2.3.4 -> 1.2.3.4
    5.6.7.8 -> 5.6.7.8

    1.2.3.4 can access /foo
    5.6.7.8 can access /bar

and "showmount -a" would then list

     1.2.3.4:/foo
     5.6.7.8:/bar

but again this would only be if there were accesses in the last half
hour (though that could be configurable too I guess).

Finally we could still leave the authentication the way it is and just
fix up the rmtab entries.
So whenever mountd gives info to the kernel, whether due to a mount
request or an up-call, we write an appropriate IP:/dir to rmtab, and
whenever an unmount request is received, we deleted the corresponding
request.
We could do this ignoring the current count in rmtab, and still have a
fairly reliable system.  We would probably want to erase rmtab at boot
time to stop it from collection cruft.

Actually, that sounds like it could be the best idea.  I'll have a go
at that, but I'll post this patch (That gets info from the in-kernel
cache) anyway.

NeilBrown


diff --git a/support/nfs/cacheio.c b/support/nfs/cacheio.c
index 3e868d8..31e2b75 100644
--- a/support/nfs/cacheio.c
+++ b/support/nfs/cacheio.c
@@ -140,7 +140,7 @@ int qword_get(char **bpp, char *dest, int bufsize)
 		}
 	} else {
 		/* text with \nnn octal quoting */
-		while (*bp != ' ' && *bp != '\n' && *bp && len < bufsize-1) {
+		while (*bp != ' ' && *bp != '\t' && *bp != '\n' && *bp && len < bufsize-1) {
 			if (*bp == '\\' &&
 			    isodigit(bp[1]) && (bp[1] <= '3') &&
 			    isodigit(bp[2]) &&
@@ -158,9 +158,9 @@ int qword_get(char **bpp, char *dest, int bufsize)
 		}
 	}
 
-	if (*bp != ' ' && *bp != '\n' && *bp != '\0')
+	if (*bp != ' ' && *bp != '\t' && *bp != '\n' && *bp != '\0')
 		return -1;
-	while (*bp == ' ') bp++;
+	while (*bp == ' ' || *bp == '\t') bp++;
 	*bpp = bp;
 	*dest = '\0';
 	return len;
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index c57d3b0..d99f22f 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -83,9 +83,6 @@ void auth_unix_ip(FILE *f)
 		qword_print(f, *client?client:"DEFAULT");
 	qword_eol(f);
 
-	if (client && strcmp(ipaddr, client))
-		mountlist_add(ipaddr, *client?client:"DEFAULT");
-
 	if (client) free(client);
 	
 }
@@ -295,7 +292,6 @@ void nfsd_export(FILE *f)
 		qword_printint(f, found->m_export.e_anonuid);
 		qword_printint(f, found->m_export.e_anongid);
 		qword_printint(f, found->m_export.e_fsid);
-		mountlist_add(dom, path);
 	}
 	qword_eol(f);
  out:
@@ -374,7 +370,6 @@ void cache_export_ent(char *domain, struct exportent *exp)
 
 	fclose(f);
 
-	mountlist_add(domain, exp->e_path);
 }
 
 void cache_export(nfs_export *exp)
@@ -393,9 +388,6 @@ void cache_export(nfs_export *exp)
 	
 	fclose(f);
 
-	if (strcmp(inet_ntoa(exp->m_client->m_addrlist[0]), exp->m_client->m_hostname))
-		mountlist_add(inet_ntoa(exp->m_client->m_addrlist[0]), exp->m_client->m_hostname);
-
 	cache_export_ent(exp->m_client->m_hostname, &exp->m_export);
 }
 
@@ -433,3 +425,84 @@ cache_get_filehandle(nfs_export *exp, int len, char *p)
 	return &fh;
 }
 
+mountlist
+cache_mountlist_list(void)
+{
+	/* Read /proc/net/rpc/auth.unix.ip/content and
+	 *      /proc/net/rpc/nfsd.export/content
+	 * and construct a mountlist from that.
+	 * Every entry in nfsd.export creates a mountlist entry.
+	 * Then every entry in auth.unix.ip which matches an already
+	 * created entry creates another entry.
+	 */
+	static mountlist mlist = NULL;
+	mountlist mlist_base;
+	char buf[8192];
+	FILE *f;
+
+	while (mlist) {
+		mountlist m = mlist;
+		mlist = mlist->ml_next;
+		xfree(m->ml_hostname);
+		xfree(m->ml_directory);
+		xfree(m);
+	}
+
+	f = fopen("/proc/net/rpc/nfsd.export/content", "r");
+	if (!f)
+		return mlist;
+
+	while (fgets(buf, sizeof(buf), f) != NULL) {
+		char *cp;
+		char path[1024], host[1024];
+		mountlist m;
+
+		if (buf[0] == '#' || buf[0] == '\n' || buf[0] == '\0')
+			continue;
+		cp = buf;
+		qword_get(&cp, path, 1024);
+		qword_get(&cp, host, 1024);
+
+		cp = strchr(host, '(');
+		if (cp)
+			*cp = 0;
+		m = (mountlist) xmalloc(sizeof(*m));
+		m->ml_hostname = xstrdup(host);
+		m->ml_directory = xstrdup(path);
+		m->ml_next = mlist;
+		mlist = m;
+	}
+	fclose(f);
+
+
+	f = fopen("/proc/net/rpc/auth.unix.ip/content", "r");
+	if (!f)
+		return mlist;
+
+	mlist_base = mlist;
+	while (fgets(buf, sizeof(buf), f) != NULL) {
+		char *cp;
+		char ip[1024], host[1024];
+		mountlist m, mp;
+
+		if (buf[0] == '#' || buf[0] == '\n' || buf[0] == '\0')
+			continue;
+		if (strncmp(buf, "nfsd ", 5) != 0)
+			continue;
+		cp = buf+5;
+		qword_get(&cp, ip, 1024);
+		qword_get(&cp, host, 1024);
+
+		for (mp = mlist_base; mp; mp = mp->ml_next)
+			if (strcmp(mp->ml_hostname, host) == 0) {
+				m = (mountlist) xmalloc(sizeof(*m));
+				m->ml_hostname = xstrdup(ip);
+				m->ml_directory = xstrdup(mp->ml_directory);
+				m->ml_next = mlist;
+				mlist = m;
+				break;
+			}
+	}
+	fclose(f);
+	return mlist;
+}
diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
index f40d367..5aa3c83 100644
--- a/utils/mountd/mountd.c
+++ b/utils/mountd/mountd.c
@@ -198,7 +198,11 @@ mount_dump_1_svc(struct svc_req *rqstp, void *argp, mountlist *res)
 	struct sockaddr_in *addr =
 		(struct sockaddr_in *) svc_getcaller(rqstp->rq_xprt);
 
-	if ((*res = mountlist_list()) == NULL)
+	if (new_cache)
+		*res = cache_mountlist_list();
+	else
+		*res = mountlist_list();
+	if (*res == NULL)
 		xlog(L_WARNING, "dump request from %s failed.",
 			inet_ntoa(addr->sin_addr));
 
diff --git a/utils/mountd/mountd.h b/utils/mountd/mountd.h
index d64c171..ecaaff9 100644
--- a/utils/mountd/mountd.h
+++ b/utils/mountd/mountd.h
@@ -49,6 +49,7 @@ void		mountlist_add(char *host, const char *path);
 void		mountlist_del(char *host, const char *path);
 void		mountlist_del_all(struct sockaddr_in *sin);
 mountlist	mountlist_list(void);
+mountlist	cache_mountlist_list(void);
 
 
 #endif /* MOUNTD_H */

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 0/2] mountd: clean up rmtab handling

[Neil Brown]

On Monday December 4, neilb@suse.de wrote:
> 
> Finally we could still leave the authentication the way it is and just
> fix up the rmtab entries.
> So whenever mountd gives info to the kernel, whether due to a mount
> request or an up-call, we write an appropriate IP:/dir to rmtab, and
> whenever an unmount request is received, we deleted the corresponding
> request.
> We could do this ignoring the current count in rmtab, and still have a
> fairly reliable system.  We would probably want to erase rmtab at boot
> time to stop it from collection cruft.
> 
> Actually, that sounds like it could be the best idea.  I'll have a go
> at that, but I'll post this patch (That gets info from the in-kernel
> cache) anyway.

I couldn't quite do that the way I had hoped as the info isn't
available at the right places.

However here is another patch for consideration.

It tries a bit harder to keep vaguely sensible information in rmtab
and if you run mounted with "-I", then "showmount -a" will list IP and
export-point of all current mounts ... as best as possible.
e.g. if you mount the same filesystem twice on the one client, then it
will appear once in "showmount -a".  If you then unmount one of them,
it will disappear.  But as soon as you access it again it will
re-appear. 

I think this is about the best I can do.

Does it meet the needs?

NeilBrown


diff --git a/support/export/client.c b/support/export/client.c
index 33dfdb0..19b6f1c 100644
--- a/support/export/client.c
+++ b/support/export/client.c
@@ -241,6 +241,7 @@ client_find(struct hostent *hp)
  *
  */
 static char *add_name(char *old, char *add);
+extern int use_ip_address;
 
 char *
 client_compose(struct in_addr addr)
@@ -249,6 +250,9 @@ client_compose(struct in_addr addr)
 	char *name = NULL;
 	int i;
 
+	if (use_ip_address)
+		return strdup(inet_ntoa(addr));
+
 	if (clientlist[MCL_WILDCARD] || clientlist[MCL_NETGROUP])
 		he = get_reliable_hostbyaddr((const char*)&addr, sizeof(addr), AF_INET);
 	if (he == NULL)
@@ -284,6 +288,25 @@ client_member(char *client, char *name)
 	return 0;
 }
 
+int client_matches(char *client, nfs_client *clp)
+{
+	struct hostent host;
+	struct in_addr *alist[2];
+	struct in_addr a;
+	if (! use_ip_address)
+		return client_member(client, clp->m_hostname);
+
+	host.h_addrtype = AF_INET;
+	host.h_length = 4;
+	host.h_name = client;
+	host.h_addr_list = (char*)alist;
+	alist[0] = &a;
+	alist[1] = NULL;
+	inet_aton(client, &a);
+	
+	return client_check(clp, &host);
+}
+
 
 int
 name_cmp(char *a, char *b)
diff --git a/support/include/exportfs.h b/support/include/exportfs.h
index 10f38c7..1d05357 100644
--- a/support/include/exportfs.h
+++ b/support/include/exportfs.h
@@ -57,6 +57,7 @@ void				client_release(nfs_client *);
 void				client_freeall(void);
 char *				client_compose(struct in_addr addr);
 int 				client_member(char *client, char *name);
+int 				client_matches(char *client, nfs_client *clp);
 
 int				export_read(char *fname);
 void				export_add(nfs_export *);
diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
index cd49a3b..5a5e8fe 100644
--- a/utils/exportfs/exportfs.c
+++ b/utils/exportfs/exportfs.c
@@ -33,6 +33,7 @@ static void	dump(int verbose);
 static void	error(nfs_export *exp, int err);
 static void	usage(void);
 
+int use_ip_address; /* Should not get used... */
 
 int
 main(int argc, char **argv)
diff --git a/utils/mountd/auth.c b/utils/mountd/auth.c
index 44d9980..02a26e1 100644
--- a/utils/mountd/auth.c
+++ b/utils/mountd/auth.c
@@ -96,7 +96,7 @@ auth_authenticate_internal(char *what, struct sockaddr_in *caller,
 		exp = NULL;
 		for (i = 0; !exp && i < MCL_MAXTYPES; i++) 
 			for (exp = exportlist[i]; exp; exp = exp->m_next) {
-				if (!client_member(my_client.m_hostname, exp->m_client->m_hostname))
+				if (!client_matches(my_client.m_hostname, exp->m_client))
 					continue;
 				if (strcmp(path, exp->m_export.e_path))
 					continue;
diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
index c57d3b0..16d0ccd 100644
--- a/utils/mountd/cache.c
+++ b/utils/mountd/cache.c
@@ -83,9 +83,6 @@ void auth_unix_ip(FILE *f)
 		qword_print(f, *client?client:"DEFAULT");
 	qword_eol(f);
 
-	if (client && strcmp(ipaddr, client))
-		mountlist_add(ipaddr, *client?client:"DEFAULT");
-
 	if (client) free(client);
 	
 }
@@ -174,7 +171,7 @@ void nfsd_fh(FILE *f)
 		for (exp = exportlist[i]; exp; exp = exp->m_next) {
 			struct stat stb;
 
-			if (!client_member(dom, exp->m_client->m_hostname))
+			if (!client_matches(dom, exp->m_client))
 				continue;
 			if (exp->m_export.e_mountpoint &&
 			    !is_mountpoint(exp->m_export.e_mountpoint[0]?
@@ -273,7 +270,7 @@ void nfsd_export(FILE *f)
 	/* now find flags for this export point in this domain */
 	for (i=0 ; i < MCL_MAXTYPES; i++) {
 		for (exp = exportlist[i]; exp; exp = exp->m_next) {
-			if (!client_member(dom, exp->m_client->m_hostname))
+			if (!client_matches(dom, exp->m_client))
 				continue;
 			if (strcmp(path, exp->m_export.e_path))
 				continue;
@@ -377,6 +374,23 @@ void cache_export_ent(char *domain, struct exportent *exp)
 	mountlist_add(domain, exp->e_path);
 }
 
+void cache_export_flush(char *domain, struct exportent *exp)
+{
+
+	FILE *f = fopen("/proc/net/rpc/nfsd.export/channel", "w");
+	if (!f)
+		return;
+
+	qword_print(f, domain);
+	qword_print(f, exp->e_path);
+	qword_printint(f, time(0)-60);
+	qword_eol(f);
+
+	fclose(f);
+
+	mountlist_del(domain, exp->e_path, 1);
+}
+
 void cache_export(nfs_export *exp)
 {
 	FILE *f;
@@ -393,9 +407,6 @@ void cache_export(nfs_export *exp)
 	
 	fclose(f);
 
-	if (strcmp(inet_ntoa(exp->m_client->m_addrlist[0]), exp->m_client->m_hostname))
-		mountlist_add(inet_ntoa(exp->m_client->m_addrlist[0]), exp->m_client->m_hostname);
-
 	cache_export_ent(exp->m_client->m_hostname, &exp->m_export);
 }
 
diff --git a/utils/mountd/mountd.c b/utils/mountd/mountd.c
index f40d367..97cd1a0 100644
--- a/utils/mountd/mountd.c
+++ b/utils/mountd/mountd.c
@@ -30,6 +30,7 @@
 extern void	cache_open(void);
 extern struct nfs_fh_len *cache_get_filehandle(nfs_export *exp, int len, char *p);
 extern void cache_export(nfs_export *exp);
+extern void cache_export_flush(char *domain, struct exportent *exp);
 
 extern void my_svc_run(void);
 
@@ -38,6 +39,7 @@ static exports		get_exportlist(void);
 static struct nfs_fh_len *get_rootfh(struct svc_req *, dirpath *, mountstat3 *, int v3);
 
 int new_cache = 0;
+int use_ip_address = 0;
 
 /* PRC: a high-availability callout program can be specified with -H
  * When this is done, the program will receive callouts whenever clients
@@ -66,6 +68,7 @@ static struct option longopts[] =
 	{ "ha-callout", 1, 0, 'H' },
 	{ "state-directory-path", 1, 0, 's' },
 	{ "num-threads", 1, 0, 't' },
+	{ "use-ip-addresses", 0, 0, 'I'},
 	{ NULL, 0, 0, 0 }
 };
 
@@ -226,11 +229,10 @@ mount_umnt_1_svc(struct svc_req *rqstp, dirpath *argp, void *resp)
 		return 1;
 	}
 	if (new_cache) {
-		if (strcmp(inet_ntoa(exp->m_client->m_addrlist[0]), exp->m_client->m_hostname))
-			mountlist_del(inet_ntoa(exp->m_client->m_addrlist[0]), exp->m_client->m_hostname);
-		mountlist_del(exp->m_client->m_hostname, p);
+		cache_export_flush(exp->m_client->m_hostname,
+				   &exp->m_export);
 	} else {
-		mountlist_del(exp->m_client->m_hostname, p);
+		mountlist_del(exp->m_client->m_hostname, p, 0);
 		export_reset (exp);
 	}
 	return 1;
@@ -558,8 +560,11 @@ main(int argc, char **argv)
 
 	/* Parse the command line options and arguments. */
 	opterr = 0;
-	while ((c = getopt_long(argc, argv, "o:nFd:f:p:P:hH:N:V:vs:t:", longopts, NULL)) != EOF)
+	while ((c = getopt_long(argc, argv, "Io:nFd:f:p:P:hH:N:V:vs:t:", longopts, NULL)) != EOF)
 		switch (c) {
+		case 'I':
+			use_ip_address = 1;
+			break;
 		case 'o':
 			descriptors = atoi(optarg);
 			if (descriptors <= 0) {
diff --git a/utils/mountd/mountd.h b/utils/mountd/mountd.h
index d64c171..d62623c 100644
--- a/utils/mountd/mountd.h
+++ b/utils/mountd/mountd.h
@@ -46,7 +46,7 @@ nfs_export *	auth_authenticate(char *what, struct sockaddr_in *sin,
 void		auth_export(nfs_export *exp);
 
 void		mountlist_add(char *host, const char *path);
-void		mountlist_del(char *host, const char *path);
+void		mountlist_del(char *host, const char *path, int force);
 void		mountlist_del_all(struct sockaddr_in *sin);
 mountlist	mountlist_list(void);
 
diff --git a/utils/mountd/rmtab.c b/utils/mountd/rmtab.c
index f641a96..ab4c200 100644
--- a/utils/mountd/rmtab.c
+++ b/utils/mountd/rmtab.c
@@ -11,6 +11,7 @@
 #endif
 
 #include <sys/types.h>
+#include <time.h>
 #include <sys/stat.h>
 #include <unistd.h>
 #include <netinet/in.h>
@@ -89,7 +90,7 @@ mountlist_add(char *host, const char *path)
 }
 
 void
-mountlist_del(char *hname, const char *path)
+mountlist_del(char *hname, const char *path, int force)
 {
 	struct rmtabent	*rep;
 	FILE		*fp;
@@ -115,7 +116,7 @@ mountlist_del(char *hname, const char *path)
 			/* PRC: do the HA callout: */
 			ha_callout("unmount", rep->r_client, rep->r_path, rep->r_count);
 		}
-		if (!match || rep->r_count)
+		if (!match || (!force && rep->r_count))
 			fputrmtabent(fp, rep, NULL);
 	}
 	if (slink_safe_rename(_PATH_RMTABTMP, _PATH_RMTAB) < 0) {
@@ -200,6 +201,8 @@ mountlist_list(void)
 			xfree(m);
 		}
 		last_mtime = stb.st_mtime;
+		if (last_mtime == time(0))
+			last_mtime--;
 
 		setrmtabent("r");
 		while ((rep = getrmtabent(1, NULL)) != NULL) {

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 0/2] mountd: clean up rmtab handling

[Jeff Layton]

Neil Brown wrote:
> 
> I'm not convinced... I don't like the idea of mapping an IP address to
> a hostname and then just working with the hostname.  Because it is
> really IP addresses that you trust, not host names (in the case of
> multi-homed hosts particularly).
> 

The patch I posted doesn't do that though. Perhaps I didn't explain it 
well enough...

All that patch does is get rid of the caching of the hostname list in 
my_client. Currently, what happens is that we build a comma separated 
list of "hostnames" and stuff that into my_client.m_hostname. We build 
this comma-separated list via client_compose, which just calls 
client_check repeatedly to see what hostnames this address matches.

The patch I posted still uses the exact same decision making process to 
see if an address matches an nfs_client entry (client_check). It just no 
longer does this in two stages via the comma-separated list.

The patch I posted should make no change in behavior of whether a host 
is allowed or denied, aside from the particular case that we already 
discussed where a cached my_client hostname list is no longer correct.

All that said, I'm OK with pulling this info out of the kernel caches 
instead. I'll have a look over the latest patch that you sent and see if 
it does what we need.

Thanks,
Jeff

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 0/2] mountd: clean up rmtab handling

[Jeff Layton]

Neil Brown wrote:
 > It tries a bit harder to keep vaguely sensible information in rmtab
 > and if you run mounted with "-I", then "showmount -a" will list IP and
 > export-point of all current mounts ... as best as possible.
 > e.g. if you mount the same filesystem twice on the one client, then it
 > will appear once in "showmount -a".  If you then unmount one of them,
 > it will disappear.  But as soon as you access it again it will
 > re-appear.
 >
 > I think this is about the best I can do.
 >
 > Does it meet the needs?
 >

I think we need to fix the default case here instead of adding a 
workaround flag. We should also strive to make it report hostnames 
rather than ip addresses if possible. I don't see a need for the -I flag 
here.

How about this patch instead? It combines some of my earlier patches 
with the one you just proposed.  I did some very cursory testing and it 
seems to work correctly, populating rmtab with hostname:/path instead of 
ipaddr:/path.

Signed-off-by: Jeff Layton <jlayton@redhat.com>

--- nfs-utils-1.0.10/support/export/client.c.rmtab	2006-08-07 
02:40:50.000000000 -0400
+++ nfs-utils-1.0.10/support/export/client.c	2006-12-04 
20:33:51.000000000 -0500
@@ -234,102 +234,15 @@
  	return NULL;
  }

-/*
- * Find client name given an IP address
- * This is found by gathering all known names that match that IP address,
- * sorting them and joining them with '+'
- *
- */
-static char *add_name(char *old, char *add);
-
-char *
-client_compose(struct in_addr addr)
+struct hostent *
+client_resolve(struct in_addr addr)
  {
  	struct hostent *he = NULL;
-	char *name = NULL;
-	int i;

-	if (clientlist[MCL_WILDCARD] || clientlist[MCL_NETGROUP])
-		he = get_reliable_hostbyaddr((const char*)&addr, sizeof(addr), AF_INET);
+	he = get_reliable_hostbyaddr((const char*)&addr, sizeof(addr), AF_INET);
  	if (he == NULL)
  		he = get_hostent((const char*)&addr, sizeof(addr), AF_INET);
-
-	for (i = 0 ; i < MCL_MAXTYPES; i++) {
-		nfs_client	*clp;
-		for (clp = clientlist[i]; clp ; clp = clp->m_next) {
-			if (!client_check(clp, he))
-				continue;
-			name = add_name(name, clp->m_hostname);
-		}
-	}
-	return name;
-}
-
-int
-client_member(char *client, char *name)
-{
-	/* check if "client" (a ',' separated list of names)
-	 * contains 'name' as a member
-	 */
-	int l = strlen(name);
-	while (*client) {
-		if (strncmp(client, name, l) == 0 &&
-		    (client[l] == ',' || client[l] == '\0'))
-			return 1;
-		client = strchr(client, ',');
-		if (client == NULL)
-			return 0;
-		client++;
-	}
-	return 0;
-}
-
-
-int
-name_cmp(char *a, char *b)
-{
-	/* compare strings a and b, but only upto ',' in a */
-	while (*a && *b && *a != ',' && *a == *b)
-		a++, b++;
-	if (!*b && (!*a || !a == ',') )
-		return 0;
-	if (!*b) return 1;
-	if (!*a || *a == ',') return -1;
-	return *a - *b;
-}
-
-static char *
-add_name(char *old, char *add)
-{
-	int len = strlen(add)+2;
-	char *new;
-	char *cp;
-	if (old) len += strlen(old);
-	
-	new = malloc(len);
-	if (!new) {
-		free(old);
-		return NULL;
-	}
-	cp = old;
-	while (cp && *cp && name_cmp(cp, add) < 0) {
-		/* step cp forward over a name */
-		char *e = strchr(cp, ',');
-		if (e)
-			cp = e+1;
-		else
-			cp = cp + strlen(cp);
-	}
-	strncpy(new, old, cp-old);
-	new[cp-old] = 0;
-	if (cp != old && !*cp)
-		strcat(new, ",");
-	strcat(new, add);
-	if (cp && *cp) {
-		strcat(new, ",");
-		strcat(new, cp);
-	}
-	return new;
+	return he;
  }

  /*
--- nfs-utils-1.0.10/support/include/exportfs.h.rmtab	2006-08-07 
02:40:50.000000000 -0400
+++ nfs-utils-1.0.10/support/include/exportfs.h	2006-12-04 
20:38:46.000000000 -0500
@@ -55,8 +55,7 @@
  int				client_match(nfs_client *, char *hname);
  void				client_release(nfs_client *);
  void				client_freeall(void);
-char *				client_compose(struct in_addr addr);
-int 				client_member(char *client, char *name);
+struct hostent *		client_resolve(struct in_addr addr);

  int				export_read(char *fname);
  void				export_add(nfs_export *);
--- nfs-utils-1.0.10/utils/mountd/auth.c.rmtab	2006-08-07 
02:40:50.000000000 -0400
+++ nfs-utils-1.0.10/utils/mountd/auth.c	2006-12-04 20:33:51.000000000 -0500
@@ -76,27 +76,16 @@
  	if (new_cache) {
  		int i;
  		/* return static nfs_export with details filled in */
-		if (my_client.m_naddr != 1 ||
-		    my_client.m_addrlist[0].s_addr != caller->sin_addr.s_addr) {
-			/* different client to last time, so do a lookup */
-			char *n;
-			my_client.m_naddr = 0;
-			my_client.m_addrlist[0] = caller->sin_addr;
-			n = client_compose(caller->sin_addr);
-			*error = unknown_host;
-			if (!n)
-				return NULL;
-			strcpy(my_client.m_hostname, *n?n:"DEFAULT");
-			free(n);
-			my_client.m_naddr = 1;
-		}
-
+		my_client.m_addrlist[0] = caller->sin_addr;
+		my_client.m_naddr = 1;
+		strncpy(my_client.m_hostname, hp->h_name,
+			sizeof(my_client.m_hostname));
  		my_exp.m_client = &my_client;

  		exp = NULL;
  		for (i = 0; !exp && i < MCL_MAXTYPES; i++)
  			for (exp = exportlist[i]; exp; exp = exp->m_next) {
-				if (!client_member(my_client.m_hostname, exp->m_client->m_hostname))
+				if (!client_check(exp->m_client, hp))
  					continue;
  				if (strcmp(path, exp->m_export.e_path))
  					continue;
--- nfs-utils-1.0.10/utils/mountd/cache.c.rmtab	2006-08-07 
02:40:50.000000000 -0400
+++ nfs-utils-1.0.10/utils/mountd/cache.c	2006-12-04 20:37:24.000000000 
-0500
@@ -53,8 +53,8 @@
  	char *cp;
  	char class[20];
  	char ipaddr[20];
-	char *client;
  	struct in_addr addr;
+	struct hostent *he;
  	if (readline(fileno(f), &lbuf, &lbuflen) != 1)
  		return;

@@ -73,21 +73,16 @@
  	auth_reload();

  	/* addr is a valid, interesting address, find the domain name... */
-	client = client_compose(addr);
+	he = client_resolve(addr);

-	
  	qword_print(f, "nfsd");
  	qword_print(f, ipaddr);
  	qword_printint(f, time(0)+30*60);
-	if (client)
-		qword_print(f, *client?client:"DEFAULT");
+	if (he) {
+		qword_print(f, *he->h_name?he->h_name:"DEFAULT");
+		free(he);
+	}
  	qword_eol(f);
-
-	if (client && strcmp(ipaddr, client))
-		mountlist_add(ipaddr, *client?client:"DEFAULT");
-
-	if (client) free(client);
-	
  }

  void nfsd_fh(FILE *f)
@@ -109,6 +104,7 @@
  	nfs_export *exp;
  	int i;
  	int dev_missing = 0;
+	struct hostent *hp = NULL;

  	if (readline(fileno(f), &lbuf, &lbuflen) != 1)
  		return;
@@ -170,11 +166,12 @@
  	auth_reload();

  	/* Now determine export point for this fsid/domain */
+	hp = gethostbyname(dom);
  	for (i=0 ; i < MCL_MAXTYPES; i++) {
  		for (exp = exportlist[i]; exp; exp = exp->m_next) {
  			struct stat stb;

-			if (!client_member(dom, exp->m_client->m_hostname))
+			if (!client_check(exp->m_client, hp))
  				continue;
  			if (exp->m_export.e_mountpoint &&
  			    !is_mountpoint(exp->m_export.e_mountpoint[0]?
@@ -251,6 +248,7 @@
  	int i;
  	char *dom, *path;
  	nfs_export *exp, *found = NULL;
+	struct hostent *hp;


  	if (readline(fileno(f), &lbuf, &lbuflen) != 1)
@@ -271,9 +269,10 @@
  	auth_reload();

  	/* now find flags for this export point in this domain */
+	hp = gethostbyname(dom);
  	for (i=0 ; i < MCL_MAXTYPES; i++) {
  		for (exp = exportlist[i]; exp; exp = exp->m_next) {
-			if (!client_member(dom, exp->m_client->m_hostname))
+			if (!client_check(exp->m_client, hp))
  				continue;
  			if (strcmp(path, exp->m_export.e_path))
  				continue;
@@ -377,6 +376,23 @@
  	mountlist_add(domain, exp->e_path);
  }

+void cache_export_flush(char *domain, struct exportent *exp)
+{
+
+	FILE *f = fopen("/proc/net/rpc/nfsd.export/channel", "w");
+	if (!f)
+		return;
+
+	qword_print(f, domain);
+	qword_print(f, exp->e_path);
+	qword_printint(f, time(0)-60);
+	qword_eol(f);
+
+	fclose(f);
+
+	mountlist_del(domain, exp->e_path, 1);
+}
+
  void cache_export(nfs_export *exp)
  {
  	FILE *f;
@@ -393,9 +409,6 @@
  	
  	fclose(f);

-	if (strcmp(inet_ntoa(exp->m_client->m_addrlist[0]), 
exp->m_client->m_hostname))
-		mountlist_add(inet_ntoa(exp->m_client->m_addrlist[0]), 
exp->m_client->m_hostname);
-
  	cache_export_ent(exp->m_client->m_hostname, &exp->m_export);
  }

--- nfs-utils-1.0.10/utils/mountd/mountd.c.rmtab	2006-08-07 
02:40:50.000000000 -0400
+++ nfs-utils-1.0.10/utils/mountd/mountd.c	2006-12-04 20:33:51.000000000 
-0500
@@ -30,6 +30,7 @@
  extern void	cache_open(void);
  extern struct nfs_fh_len *cache_get_filehandle(nfs_export *exp, int 
len, char *p);
  extern void cache_export(nfs_export *exp);
+extern void cache_export_flush(char *domain, struct exportent *exp);

  extern void my_svc_run(void);

@@ -226,11 +227,10 @@
  		return 1;
  	}
  	if (new_cache) {
-		if (strcmp(inet_ntoa(exp->m_client->m_addrlist[0]), 
exp->m_client->m_hostname))
-			mountlist_del(inet_ntoa(exp->m_client->m_addrlist[0]), 
exp->m_client->m_hostname);
-		mountlist_del(exp->m_client->m_hostname, p);
+		cache_export_flush(exp->m_client->m_hostname,
+				   &exp->m_export);
  	} else {
-		mountlist_del(exp->m_client->m_hostname, p);
+		mountlist_del(exp->m_client->m_hostname, p, 0);
  		export_reset (exp);
  	}
  	return 1;
--- nfs-utils-1.0.10/utils/mountd/mountd.h.rmtab	2006-08-07 
02:40:50.000000000 -0400
+++ nfs-utils-1.0.10/utils/mountd/mountd.h	2006-12-04 20:33:51.000000000 
-0500
@@ -46,7 +46,7 @@
  void		auth_export(nfs_export *exp);

  void		mountlist_add(char *host, const char *path);
-void		mountlist_del(char *host, const char *path);
+void		mountlist_del(char *host, const char *path, int force);
  void		mountlist_del_all(struct sockaddr_in *sin);
  mountlist	mountlist_list(void);

--- nfs-utils-1.0.10/utils/mountd/rmtab.c.rmtab	2006-08-07 
02:40:50.000000000 -0400
+++ nfs-utils-1.0.10/utils/mountd/rmtab.c	2006-12-04 20:33:51.000000000 
-0500
@@ -11,6 +11,7 @@
  #endif

  #include <sys/types.h>
+#include <time.h>
  #include <sys/stat.h>
  #include <unistd.h>
  #include <netinet/in.h>
@@ -89,7 +90,7 @@
  }

  void
-mountlist_del(char *hname, const char *path)
+mountlist_del(char *hname, const char *path, int force)
  {
  	struct rmtabent	*rep;
  	FILE		*fp;
@@ -115,7 +116,7 @@
  			/* PRC: do the HA callout: */
  			ha_callout("unmount", rep->r_client, rep->r_path, rep->r_count);
  		}
-		if (!match || rep->r_count)
+		if (!match || (!force && rep->r_count))
  			fputrmtabent(fp, rep, NULL);
  	}
  	if (slink_safe_rename(_PATH_RMTABTMP, _PATH_RMTAB) < 0) {
@@ -200,6 +201,8 @@
  			xfree(m);
  		}
  		last_mtime = stb.st_mtime;
+		if (last_mtime == time(0))
+			last_mtime--;

  		setrmtabent("r");
  		while ((rep = getrmtabent(1, NULL)) != NULL) {

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 0/2] mountd: clean up rmtab handling

[Jeff Layton]

Jeff Layton wrote:
> Neil Brown wrote:
>  > It tries a bit harder to keep vaguely sensible information in rmtab
>  > and if you run mounted with "-I", then "showmount -a" will list IP and
>  > export-point of all current mounts ... as best as possible.
>  > e.g. if you mount the same filesystem twice on the one client, then it
>  > will appear once in "showmount -a".  If you then unmount one of them,
>  > it will disappear.  But as soon as you access it again it will
>  > re-appear.
>  >
>  > I think this is about the best I can do.
>  >
>  > Does it meet the needs?
>  >
>
> I think we need to fix the default case here instead of adding a
> workaround flag. We should also strive to make it report hostnames
> rather than ip addresses if possible. I don't see a need for the -I flag
> here.
>
> How about this patch instead? It combines some of my earlier patches
> with the one you just proposed.  I did some very cursory testing and it
> seems to work correctly, populating rmtab with hostname:/path instead of
> ipaddr:/path.
>

Apologies, my mailer munged that last patch. Here it is again...

Signed-off-by: Jeff Layton <jlayton@redhat.com>

--- nfs-utils-1.0.10/support/export/client.c.rmtab	2006-08-07 02:40:50.000000000 -0400
+++ nfs-utils-1.0.10/support/export/client.c	2006-12-04 20:33:51.000000000 -0500
@@ -234,102 +234,15 @@
  	return NULL;
  }

-/*
- * Find client name given an IP address
- * This is found by gathering all known names that match that IP address,
- * sorting them and joining them with '+'
- *
- */
-static char *add_name(char *old, char *add);
-
-char *
-client_compose(struct in_addr addr)
+struct hostent *
+client_resolve(struct in_addr addr)
  {
  	struct hostent *he = NULL;
-	char *name = NULL;
-	int i;

-	if (clientlist[MCL_WILDCARD] || clientlist[MCL_NETGROUP])
-		he = get_reliable_hostbyaddr((const char*)&addr, sizeof(addr), AF_INET);
+	he = get_reliable_hostbyaddr((const char*)&addr, sizeof(addr), AF_INET);
  	if (he == NULL)
  		he = get_hostent((const char*)&addr, sizeof(addr), AF_INET);
-
-	for (i = 0 ; i < MCL_MAXTYPES; i++) {
-		nfs_client	*clp;
-		for (clp = clientlist[i]; clp ; clp = clp->m_next) {
-			if (!client_check(clp, he))
-				continue;
-			name = add_name(name, clp->m_hostname);
-		}
-	}
-	return name;
-}
-
-int
-client_member(char *client, char *name)
-{
-	/* check if "client" (a ',' separated list of names)
-	 * contains 'name' as a member
-	 */
-	int l = strlen(name);
-	while (*client) {
-		if (strncmp(client, name, l) == 0 &&
-		    (client[l] == ',' || client[l] == '\0'))
-			return 1;
-		client = strchr(client, ',');
-		if (client == NULL)
-			return 0;
-		client++;
-	}
-	return 0;
-}
-
-
-int
-name_cmp(char *a, char *b)
-{
-	/* compare strings a and b, but only upto ',' in a */
-	while (*a && *b && *a != ',' && *a == *b)
-		a++, b++;
-	if (!*b && (!*a || !a == ',') )
-		return 0;
-	if (!*b) return 1;
-	if (!*a || *a == ',') return -1;
-	return *a - *b;
-}
-
-static char *
-add_name(char *old, char *add)
-{
-	int len = strlen(add)+2;
-	char *new;
-	char *cp;
-	if (old) len += strlen(old);
-	
-	new = malloc(len);
-	if (!new) {
-		free(old);
-		return NULL;
-	}
-	cp = old;
-	while (cp && *cp && name_cmp(cp, add) < 0) {
-		/* step cp forward over a name */
-		char *e = strchr(cp, ',');
-		if (e)
-			cp = e+1;
-		else
-			cp = cp + strlen(cp);
-	}
-	strncpy(new, old, cp-old);
-	new[cp-old] = 0;
-	if (cp != old && !*cp)
-		strcat(new, ",");
-	strcat(new, add);
-	if (cp && *cp) {
-		strcat(new, ",");
-		strcat(new, cp);
-	}
-	return new;
+	return he;
  }

  /*
--- nfs-utils-1.0.10/support/include/exportfs.h.rmtab	2006-08-07 02:40:50.000000000 -0400
+++ nfs-utils-1.0.10/support/include/exportfs.h	2006-12-04 20:38:46.000000000 -0500
@@ -55,8 +55,7 @@
  int				client_match(nfs_client *, char *hname);
  void				client_release(nfs_client *);
  void				client_freeall(void);
-char *				client_compose(struct in_addr addr);
-int 				client_member(char *client, char *name);
+struct hostent *		client_resolve(struct in_addr addr);

  int				export_read(char *fname);
  void				export_add(nfs_export *);
--- nfs-utils-1.0.10/utils/mountd/auth.c.rmtab	2006-08-07 02:40:50.000000000 -0400
+++ nfs-utils-1.0.10/utils/mountd/auth.c	2006-12-04 20:33:51.000000000 -0500
@@ -76,27 +76,16 @@
  	if (new_cache) {
  		int i;
  		/* return static nfs_export with details filled in */
-		if (my_client.m_naddr != 1 ||
-		    my_client.m_addrlist[0].s_addr != caller->sin_addr.s_addr) {
-			/* different client to last time, so do a lookup */
-			char *n;
-			my_client.m_naddr = 0;
-			my_client.m_addrlist[0] = caller->sin_addr;
-			n = client_compose(caller->sin_addr);
-			*error = unknown_host;
-			if (!n)
-				return NULL;
-			strcpy(my_client.m_hostname, *n?n:"DEFAULT");
-			free(n);
-			my_client.m_naddr = 1;
-		}
-
+		my_client.m_addrlist[0] = caller->sin_addr;
+		my_client.m_naddr = 1;
+		strncpy(my_client.m_hostname, hp->h_name,
+			sizeof(my_client.m_hostname));
  		my_exp.m_client = &my_client;

  		exp = NULL;
  		for (i = 0; !exp && i < MCL_MAXTYPES; i++)
  			for (exp = exportlist[i]; exp; exp = exp->m_next) {
-				if (!client_member(my_client.m_hostname, exp->m_client->m_hostname))
+				if (!client_check(exp->m_client, hp))
  					continue;
  				if (strcmp(path, exp->m_export.e_path))
  					continue;
--- nfs-utils-1.0.10/utils/mountd/cache.c.rmtab	2006-08-07 02:40:50.000000000 -0400
+++ nfs-utils-1.0.10/utils/mountd/cache.c	2006-12-04 20:37:24.000000000 -0500
@@ -53,8 +53,8 @@
  	char *cp;
  	char class[20];
  	char ipaddr[20];
-	char *client;
  	struct in_addr addr;
+	struct hostent *he;
  	if (readline(fileno(f), &lbuf, &lbuflen) != 1)
  		return;

@@ -73,21 +73,16 @@
  	auth_reload();

  	/* addr is a valid, interesting address, find the domain name... */
-	client = client_compose(addr);
+	he = client_resolve(addr);

-	
  	qword_print(f, "nfsd");
  	qword_print(f, ipaddr);
  	qword_printint(f, time(0)+30*60);
-	if (client)
-		qword_print(f, *client?client:"DEFAULT");
+	if (he) {
+		qword_print(f, *he->h_name?he->h_name:"DEFAULT");
+		free(he);
+	}
  	qword_eol(f);
-
-	if (client && strcmp(ipaddr, client))
-		mountlist_add(ipaddr, *client?client:"DEFAULT");
-
-	if (client) free(client);
-	
  }

  void nfsd_fh(FILE *f)
@@ -109,6 +104,7 @@
  	nfs_export *exp;
  	int i;
  	int dev_missing = 0;
+	struct hostent *hp = NULL;

  	if (readline(fileno(f), &lbuf, &lbuflen) != 1)
  		return;
@@ -170,11 +166,12 @@
  	auth_reload();

  	/* Now determine export point for this fsid/domain */
+	hp = gethostbyname(dom);
  	for (i=0 ; i < MCL_MAXTYPES; i++) {
  		for (exp = exportlist[i]; exp; exp = exp->m_next) {
  			struct stat stb;

-			if (!client_member(dom, exp->m_client->m_hostname))
+			if (!client_check(exp->m_client, hp))
  				continue;
  			if (exp->m_export.e_mountpoint &&
  			    !is_mountpoint(exp->m_export.e_mountpoint[0]?
@@ -251,6 +248,7 @@
  	int i;
  	char *dom, *path;
  	nfs_export *exp, *found = NULL;
+	struct hostent *hp;


  	if (readline(fileno(f), &lbuf, &lbuflen) != 1)
@@ -271,9 +269,10 @@
  	auth_reload();

  	/* now find flags for this export point in this domain */
+	hp = gethostbyname(dom);
  	for (i=0 ; i < MCL_MAXTYPES; i++) {
  		for (exp = exportlist[i]; exp; exp = exp->m_next) {
-			if (!client_member(dom, exp->m_client->m_hostname))
+			if (!client_check(exp->m_client, hp))
  				continue;
  			if (strcmp(path, exp->m_export.e_path))
  				continue;
@@ -377,6 +376,23 @@
  	mountlist_add(domain, exp->e_path);
  }

+void cache_export_flush(char *domain, struct exportent *exp)
+{
+
+	FILE *f = fopen("/proc/net/rpc/nfsd.export/channel", "w");
+	if (!f)
+		return;
+
+	qword_print(f, domain);
+	qword_print(f, exp->e_path);
+	qword_printint(f, time(0)-60);
+	qword_eol(f);
+
+	fclose(f);
+
+	mountlist_del(domain, exp->e_path, 1);
+}
+
  void cache_export(nfs_export *exp)
  {
  	FILE *f;
@@ -393,9 +409,6 @@
  	
  	fclose(f);

-	if (strcmp(inet_ntoa(exp->m_client->m_addrlist[0]), exp->m_client->m_hostname))
-		mountlist_add(inet_ntoa(exp->m_client->m_addrlist[0]), exp->m_client->m_hostname);
-
  	cache_export_ent(exp->m_client->m_hostname, &exp->m_export);
  }

--- nfs-utils-1.0.10/utils/mountd/mountd.c.rmtab	2006-08-07 02:40:50.000000000 -0400
+++ nfs-utils-1.0.10/utils/mountd/mountd.c	2006-12-04 20:33:51.000000000 -0500
@@ -30,6 +30,7 @@
  extern void	cache_open(void);
  extern struct nfs_fh_len *cache_get_filehandle(nfs_export *exp, int len, char *p);
  extern void cache_export(nfs_export *exp);
+extern void cache_export_flush(char *domain, struct exportent *exp);

  extern void my_svc_run(void);

@@ -226,11 +227,10 @@
  		return 1;
  	}
  	if (new_cache) {
-		if (strcmp(inet_ntoa(exp->m_client->m_addrlist[0]), exp->m_client->m_hostname))
-			mountlist_del(inet_ntoa(exp->m_client->m_addrlist[0]), exp->m_client->m_hostname);
-		mountlist_del(exp->m_client->m_hostname, p);
+		cache_export_flush(exp->m_client->m_hostname,
+				   &exp->m_export);
  	} else {
-		mountlist_del(exp->m_client->m_hostname, p);
+		mountlist_del(exp->m_client->m_hostname, p, 0);
  		export_reset (exp);
  	}
  	return 1;
--- nfs-utils-1.0.10/utils/mountd/mountd.h.rmtab	2006-08-07 02:40:50.000000000 -0400
+++ nfs-utils-1.0.10/utils/mountd/mountd.h	2006-12-04 20:33:51.000000000 -0500
@@ -46,7 +46,7 @@
  void		auth_export(nfs_export *exp);

  void		mountlist_add(char *host, const char *path);
-void		mountlist_del(char *host, const char *path);
+void		mountlist_del(char *host, const char *path, int force);
  void		mountlist_del_all(struct sockaddr_in *sin);
  mountlist	mountlist_list(void);

--- nfs-utils-1.0.10/utils/mountd/rmtab.c.rmtab	2006-08-07 02:40:50.000000000 -0400
+++ nfs-utils-1.0.10/utils/mountd/rmtab.c	2006-12-04 20:33:51.000000000 -0500
@@ -11,6 +11,7 @@
  #endif

  #include <sys/types.h>
+#include <time.h>
  #include <sys/stat.h>
  #include <unistd.h>
  #include <netinet/in.h>
@@ -89,7 +90,7 @@
  }

  void
-mountlist_del(char *hname, const char *path)
+mountlist_del(char *hname, const char *path, int force)
  {
  	struct rmtabent	*rep;
  	FILE		*fp;
@@ -115,7 +116,7 @@
  			/* PRC: do the HA callout: */
  			ha_callout("unmount", rep->r_client, rep->r_path, rep->r_count);
  		}
-		if (!match || rep->r_count)
+		if (!match || (!force && rep->r_count))
  			fputrmtabent(fp, rep, NULL);
  	}
  	if (slink_safe_rename(_PATH_RMTABTMP, _PATH_RMTAB) < 0) {
@@ -200,6 +201,8 @@
  			xfree(m);
  		}
  		last_mtime = stb.st_mtime;
+		if (last_mtime == time(0))
+			last_mtime--;

  		setrmtabent("r");
  		while ((rep = getrmtabent(1, NULL)) != NULL) {



-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 0/2] mountd: clean up rmtab handling

[Jeff Layton]

Jeff Layton wrote:
>> How about this patch instead? It combines some of my earlier patches
>> with the one you just proposed.  I did some very cursory testing and it
>> seems to work correctly, populating rmtab with hostname:/path instead of
>> ipaddr:/path.
>>
> 
> Apologies, my mailer munged that last patch. Here it is again...
> 

Hi Neil. Any thoughts on the patch I sent out on Dec 4th?

Thanks,
Jeff

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 0/2] mountd: clean up rmtab handling

[Neil Brown]

On Monday December 4, jlayton@poochiereds.net wrote:
> Neil Brown wrote:
> > 
> > I'm not convinced... I don't like the idea of mapping an IP address to
> > a hostname and then just working with the hostname.  Because it is
> > really IP addresses that you trust, not host names (in the case of
> > multi-homed hosts particularly).
> > 
> 
> The patch I posted doesn't do that though. Perhaps I didn't explain it 
> well enough...
> 
> All that patch does is get rid of the caching of the hostname list in 
> my_client. Currently, what happens is that we build a comma separated 
> list of "hostnames" and stuff that into my_client.m_hostname. We build 
> this comma-separated list via client_compose, which just calls 
> client_check repeatedly to see what hostnames this address matches.
> 
> The patch I posted still uses the exact same decision making process to 
> see if an address matches an nfs_client entry (client_check). It just no 
> longer does this in two stages via the comma-separated list.
> 
> The patch I posted should make no change in behavior of whether a host 
> is allowed or denied, aside from the particular case that we already 
> discussed where a cached my_client hostname list is no longer correct.

I don't agree that it makes no change.
At one level this is clear from the fact that it loses information. 
auth_unix_ip (which gets called when the kernel finds an unknown IP
address and wants a name for it) always provides a mapping from the IP
address to a hostname (As returned by the DNS) - if such a hostname is
available.  If the host is multi-homed (multiple IP addresses for the
one host), this might lose information.

Consider a situation where we have one subnet that is physically very
secure, and we export some filesystem (rw,no_root_squash) to that
subnet, and another subnet that is physically less secure and we
export the filesystem (ro,root_squash) to that subnet.

Suppose further there is some host - foo - with an interface on both
subnets.

Now attacker Alice sets up a machine on the less secure subnet and
injects packets from foo's IP address (on that subnet).  They get
routed to the fileserver which maps the IP address to "foo".
When you then call client_check, both of the exports could match, and
you might end up giving internal access to this external packets.  Not
good.

So I'm not happy with any patch that removes client_compose and
client_member in favour of client_resolve.

To the big-picture issue - you want "showmount -a" to return "the
right thing".  I don't believe there is a universal "the right thing"
as various issues are fairly poorly defined.  That is part of why I
was think of adding flags to mountd so you could choose your poison.

The two main areas of uncertainty in "showmount" output are:
 - how to identify the host - IP address or host name.  The former is
   more precise in the case of multi-homed hosts.   The latter is 
   possibly more common.
 - How current should the information be?  rmtab cannot be kept
   reliably uptodate and tends to grow - there can easily be hosts
   listed there that do not exist any more.  Conversely the kernel
   cache information is relatively short term, and there could easily 
   be clients that have the filesystem mounted but that are not 
   listed in the cache.

So: what exactly do you - or your customers - want?

My impression is that most people don't want anything as "showmount -a"
will have been returned the "interesting" information that you mention
for some years and this is the first time it has even been mentioned.

Some people might want IP addresses for precision.  Others might want
host names so that it works with current tools.
Some people might want "who has mounted this since I last manually
flushed the rmtab file".  Others might want "who has accessed this in
the last X minutes".

And if you want precise info from the kernel cache, you need to make
them bigger than they currently need to be (The 'exports' list only
need to be about the size of your /etc/exports file, but to get
precise information, it will need to be roughly the size of the number
of active clients).

So I really think that any solution to this issue needs to involve
configuration options.

I would like to see mountd work internally with IP addresses, and map
them to hostnames only as they are being returned to 'showmount' if
that is what is wanted.
I would like to see mountd usually using a minimal cache, and only
adding the extra information when it is actually requested.
I can imagine that you might even want to added timestamps to the
rmtab file which are updated when the kernel cache is updated, and the
relevant lines are ignored if older than some set time.

But we really need a precise answer to "what are the actual
requirements that someone has on 'showmount -a'??"

NeilBrown

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 0/2] mountd: clean up rmtab handling

[Jeff Layton]

Neil Brown wrote:
 > I don't agree that it makes no change.
 > At one level this is clear from the fact that it loses information.
 > auth_unix_ip (which gets called when the kernel finds an unknown IP
 > address and wants a name for it) always provides a mapping from the IP
 > address to a hostname (As returned by the DNS) - if such a hostname is
 > available.  If the host is multi-homed (multiple IP addresses for the
 > one host), this might lose information.
 >
 > Consider a situation where we have one subnet that is physically very
 > secure, and we export some filesystem (rw,no_root_squash) to that
 > subnet, and another subnet that is physically less secure and we
 > export the filesystem (ro,root_squash) to that subnet.
 >
 > Suppose further there is some host - foo - with an interface on both
 > subnets.
 >
 > Now attacker Alice sets up a machine on the less secure subnet and
 > injects packets from foo's IP address (on that subnet).  They get
 > routed to the fileserver which maps the IP address to "foo".
 > When you then call client_check, both of the exports could match, and
 > you might end up giving internal access to this external packets.  Not
 > good.
 >
 > So I'm not happy with any patch that removes client_compose and
 > client_member in favour of client_resolve.
 >

Here's where I'm confused. I don't see how the patch I proposed loses any
information:

In the old method, we get an IP address and we build a list of client
hostnames (in no particular order), using repeated calls to the client_check
function. We then roll through the list of exports and make repeated
calls to client_member to compare the m_client of the export against the
list. If it doesn't match, we go to the next export.

With my patch, we don't bother building a list, and simply call client_check
repeatedly within the loop to do the comparison. This seems to me to be
functionally equivalent. Since the list is built solely on the result of
client_check calls, and the order there doesn't seem to matter, then I don't
see what information that it could carry that we don't get by just calling
client_check directly.

So I don't see how the patch I proposed would leave us any more vulnerable to
the situation you describe than what's already in place. The patch I proposed
*does* remove one vulnerability as well. The situation where my_client has
a list of hostnames that are no longer valid due to DNS or netgroup changes.

 > To the big-picture issue - you want "showmount -a" to return "the
 > right thing".  I don't believe there is a universal "the right thing"
 > as various issues are fairly poorly defined.  That is part of why I
 > was think of adding flags to mountd so you could choose your poison.
 >
 > The two main areas of uncertainty in "showmount" output are:
 >  - how to identify the host - IP address or host name.  The former is
 >    more precise in the case of multi-homed hosts.   The latter is
 >    possibly more common.

Very true. Perhaps IP addresses would be better overall. It's certainly
preferable to the current situation where "hostnames" can look like:

*,foo.example.com,192.168.10.0/255.255.255.0

Probably having it command line switchable is reasonable as well, but it
seems like if people *don't* choose to have it report IP addresses, then it
should give actual, resolvable, hostnames and not the comma-separated lists
that we get now.

 >  - How current should the information be?  rmtab cannot be kept
 >    reliably uptodate and tends to grow - there can easily be hosts
 >    listed there that do not exist any more.  Conversely the kernel
 >    cache information is relatively short term, and there could easily
 >    be clients that have the filesystem mounted but that are not
 >    listed in the cache.
 >
 > So: what exactly do you - or your customers - want?
 >
 > My impression is that most people don't want anything as "showmount -a"
 > will have been returned the "interesting" information that you mention
 > for some years and this is the first time it has even been mentioned.
 >

The problem reports I have so far have really to do with the fact that
the info returned by showmount -a doesn't really reflect anything
comprehensible. The lack of reports is probably more reflective of the fact
that rather few people bother to run showmount -a since it is traditionally
unreliable (and not just on Linux). I'm sure, though, that once we get over
the hurdle of having output that matches what's in the manpage, we'll start
getting problem reports about what the info actually represents.

In my opinion, it would be ideal to have it list all active mounts. Now comes
the question -- what constitutes an "active" mount? We really don't have a
reliable way to track that, since a host can go idle and not send out any
packets for a long time, and clients can spontaneously reboot and not remount
the filesystem. The design of NFS makes this very difficult to reconcile.

As you said, neither the rmtab or the kernel cache is an ideal place to pull
this info. So, my inclincation is to stick with the rmtab, and simply have it
track what we know is trackable -- mount and unmount calls into mountd. I
think having false positives is preferable to possibly having some mounts
that are not reflected at all because they have gone idle.

Either way, a manpage update is probably also in order to outline the folly
of depending on showmount -a :-).

Thanks,
Jeff


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 0/2] mountd: clean up rmtab handling

[Neil Brown]

On Sunday December 10, jlayton@redhat.com wrote:
> Neil Brown wrote:
>  > So I'm not happy with any patch that removes client_compose and
>  > client_member in favour of client_resolve.
>  >
> 
> Here's where I'm confused. I don't see how the patch I proposed loses any
> information:
> 
> In the old method, we get an IP address and we build a list of client
> hostnames (in no particular order), using repeated calls to the client_check
> function. We then roll through the list of exports and make repeated
> calls to client_member to compare the m_client of the export against the
> list. If it doesn't match, we go to the next export.
> 
> With my patch, we don't bother building a list, and simply call client_check
> repeatedly within the loop to do the comparison. This seems to me to be
> functionally equivalent. Since the list is built solely on the result of
> client_check calls, and the order there doesn't seem to matter, then I don't
> see what information that it could carry that we don't get by just calling
> client_check directly.

Suppose your /etc/exports contains:
 
/baz   192.168.1.0/24(rw,no_root_squash)
/baz   192.168.2.0/24(ro,root_squash)

and your host 'foo' has two IP addresses: 192.168.1.10 and
192.168.2.10.

Suppose a request comes from 192.168.2.10 to access a file in /baz.

Old code:
  Kernel asks for "192.168.2.10".
  This goes to auth_unix_ip (in cache.c) which calls client_compose on
  that address.  client_compose will return 192.168.2.0/24 so kernel
  is told
          192.168.2.10 -> 192.168.2.0/24

  Then kernel asks "how can 192.168.2.0/24 access /baz".
  This goes to nfsd_export which checks each export and the first (and
  only) one that matches is 192.168.2.0/24 so it answers
  "ro,root_squash" as you would hope.

New code:
  Kernel asks for "192.168.2.10"
  This goes to auth_unix_ip which calls client_resolve which calls
  get_reliable_hostbyaddr which returns "foo".  So kernel is told
     192.168.2.10 -> foo

  Then kernel asks "how can foo access /baz".
  This goes to nfsd_export which calls gethostbyname on 'foo' getting
  { "foo", 192.168.1.10, 192.168.2.10} and then calls client_check
  against each export.  If the order is unchanged from the
  /etc/exports file, the first match will be
          /baz 192.168.1.0/24(rw,no_root_squash)

  so that will be returned to the kernel.  Not what you would hope.

Does that make sense?


> 
> So I don't see how the patch I proposed would leave us any more vulnerable to
> the situation you describe than what's already in place. The patch I proposed
> *does* remove one vulnerability as well. The situation where my_client has
> a list of hostnames that are no longer valid due to DNS or netgroup changes.

I agree that the my_client caching needs to be fixed.

> 
> As you said, neither the rmtab or the kernel cache is an ideal place to pull
> this info. So, my inclincation is to stick with the rmtab, and simply have it
> track what we know is trackable -- mount and unmount calls into mountd. I
> think having false positives is preferable to possibly having some mounts
> that are not reflected at all because they have gone idle.

Ok, how about this as a way forward.

 1/ remove that 'my_client' caching.
 2/ Remove the updates of rmtab on client upcalls and simply do rmtab
    updates with hostname (from get_reliable_hostbyaddr) and path name
    from MOUNT or UMOUNT requests
 3/ Add -I flag which:
      passes IP address rather than client_compose name to kernel and
      adds rmtab entries on kernel upcalls.  It also hard-removes
      rmtab entries on 'UMOUNT' and flush the kernel cache so that the
      next access for that host/path causes an upcall.
 4/ Add a -? flag (haven't chosen a letter yet) which:
      implies -I, but when asked for a 'DUMP', it calls gethosytbyname
      on each IP address to ge a hostname.

Then the default would be vaguely usable, and more precise information
would be available (at a cost).
  
> 
> Either way, a manpage update is probably also in order to outline the folly
> of depending on showmount -a :-).

Yes, I would happily accept a patch making such a change.

NeilBrown

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 0/2] mountd: clean up rmtab handling

[Warren Beldad]

[-- Attachment #1.1: Type: text/plain, Size: 5245 bytes --]

thanks for opening up this mail.
this is really my old problem, I used showmount -a to get who is mounted but
its not reporting correctly. It includes the previous mounts until
everything is messed up and you can no longer trust anything. I hope this
can be improved at least. :-)
I would prefer IP address than hostnames.

warren

On 12/12/06, Neil Brown <neilb@suse.de> wrote:
>
> On Sunday December 10, jlayton@redhat.com wrote:
> > Neil Brown wrote:
> >  > So I'm not happy with any patch that removes client_compose and
> >  > client_member in favour of client_resolve.
> >  >
> >
> > Here's where I'm confused. I don't see how the patch I proposed loses
> any
> > information:
> >
> > In the old method, we get an IP address and we build a list of client
> > hostnames (in no particular order), using repeated calls to the
> client_check
> > function. We then roll through the list of exports and make repeated
> > calls to client_member to compare the m_client of the export against the
> > list. If it doesn't match, we go to the next export.
> >
> > With my patch, we don't bother building a list, and simply call
> client_check
> > repeatedly within the loop to do the comparison. This seems to me to be
> > functionally equivalent. Since the list is built solely on the result of
> > client_check calls, and the order there doesn't seem to matter, then I
> don't
> > see what information that it could carry that we don't get by just
> calling
> > client_check directly.
>
> Suppose your /etc/exports contains:
>
> /baz   192.168.1.0/24(rw,no_root_squash)
> /baz   192.168.2.0/24(ro,root_squash)
>
> and your host 'foo' has two IP addresses: 192.168.1.10 and
> 192.168.2.10.
>
> Suppose a request comes from 192.168.2.10 to access a file in /baz.
>
> Old code:
>   Kernel asks for "192.168.2.10".
>   This goes to auth_unix_ip (in cache.c) which calls client_compose on
>   that address.  client_compose will return 192.168.2.0/24 so kernel
>   is told
>           192.168.2.10 -> 192.168.2.0/24
>
>   Then kernel asks "how can 192.168.2.0/24 access /baz".
>   This goes to nfsd_export which checks each export and the first (and
>   only) one that matches is 192.168.2.0/24 so it answers
>   "ro,root_squash" as you would hope.
>
> New code:
>   Kernel asks for "192.168.2.10"
>   This goes to auth_unix_ip which calls client_resolve which calls
>   get_reliable_hostbyaddr which returns "foo".  So kernel is told
>      192.168.2.10 -> foo
>
>   Then kernel asks "how can foo access /baz".
>   This goes to nfsd_export which calls gethostbyname on 'foo' getting
>   { "foo", 192.168.1.10, 192.168.2.10} and then calls client_check
>   against each export.  If the order is unchanged from the
>   /etc/exports file, the first match will be
>           /baz 192.168.1.0/24(rw,no_root_squash)
>
>   so that will be returned to the kernel.  Not what you would hope.
>
> Does that make sense?
>
>
> >
> > So I don't see how the patch I proposed would leave us any more
> vulnerable to
> > the situation you describe than what's already in place. The patch I
> proposed
> > *does* remove one vulnerability as well. The situation where my_client
> has
> > a list of hostnames that are no longer valid due to DNS or netgroup
> changes.
>
> I agree that the my_client caching needs to be fixed.
>
> >
> > As you said, neither the rmtab or the kernel cache is an ideal place to
> pull
> > this info. So, my inclincation is to stick with the rmtab, and simply
> have it
> > track what we know is trackable -- mount and unmount calls into mountd.
> I
> > think having false positives is preferable to possibly having some
> mounts
> > that are not reflected at all because they have gone idle.
>
> Ok, how about this as a way forward.
>
> 1/ remove that 'my_client' caching.
> 2/ Remove the updates of rmtab on client upcalls and simply do rmtab
>     updates with hostname (from get_reliable_hostbyaddr) and path name
>     from MOUNT or UMOUNT requests
> 3/ Add -I flag which:
>       passes IP address rather than client_compose name to kernel and
>       adds rmtab entries on kernel upcalls.  It also hard-removes
>       rmtab entries on 'UMOUNT' and flush the kernel cache so that the
>       next access for that host/path causes an upcall.
> 4/ Add a -? flag (haven't chosen a letter yet) which:
>       implies -I, but when asked for a 'DUMP', it calls gethosytbyname
>       on each IP address to ge a hostname.
>
> Then the default would be vaguely usable, and more precise information
> would be available (at a cost).
>
> >
> > Either way, a manpage update is probably also in order to outline the
> folly
> > of depending on showmount -a :-).
>
> Yes, I would happily accept a patch making such a change.
>
> NeilBrown
>
> -------------------------------------------------------------------------
> Take Surveys. Earn Cash. Influence the Future of IT
> Join SourceForge.net's Techsay panel and you'll get the chance to share
> your
> opinions on IT & business topics through brief surveys - and earn cash
> http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
> _______________________________________________
> NFS maillist  -  NFS@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nfs
>

[-- Attachment #1.2: Type: text/html, Size: 7342 bytes --]

[-- Attachment #2: Type: text/plain, Size: 347 bytes --]

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

[-- Attachment #3: Type: text/plain, Size: 140 bytes --]

_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

Re: [PATCH 0/2] mountd: clean up rmtab handling

[Jeff Layton]

Neil Brown wrote:
 >
 > Does that make sense?

Yes. Thanks for explaining that -- it makes total sense now. I was focusing
on auth_authenticate, which I don't think would behave differently, but you're
clearly correct that we'd need to ensure the same for the kernel caches.

 >> As you said, neither the rmtab or the kernel cache is an ideal place to pull
 >> this info. So, my inclincation is to stick with the rmtab, and simply have it
 >> track what we know is trackable -- mount and unmount calls into mountd. I
 >> think having false positives is preferable to possibly having some mounts
 >> that are not reflected at all because they have gone idle.
 >
 > Ok, how about this as a way forward.
 >
 >  1/ remove that 'my_client' caching.
 >  2/ Remove the updates of rmtab on client upcalls and simply do rmtab
 >     updates with hostname (from get_reliable_hostbyaddr) and path name
 >     from MOUNT or UMOUNT requests
 >  3/ Add -I flag which:
 >       passes IP address rather than client_compose name to kernel and
 >       adds rmtab entries on kernel upcalls.  It also hard-removes
 >       rmtab entries on 'UMOUNT' and flush the kernel cache so that the
 >       next access for that host/path causes an upcall.
 >  4/ Add a -? flag (haven't chosen a letter yet) which:
 >       implies -I, but when asked for a 'DUMP', it calls gethosytbyname
 >       on each IP address to ge a hostname.
 >
 > Then the default would be vaguely usable, and more precise information
 > would be available (at a cost).
 >

At first glance, that sounds reasonable. I'll have a closer look at the code
and your idea and see if I can come up with a patch.

 >> Either way, a manpage update is probably also in order to outline the folly
 >> of depending on showmount -a :-).
 >
 > Yes, I would happily accept a patch making such a change.
 >

Once we come to consensus on a code patch, I'll have a look at what manpage
changes need to be made.

Thanks,
Jeff


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs