Re: [PATCH] STATD - SM_NOTIFY have wrong ID_NAME on multihost servers.

[Steve Dickson <SteveD@redhat.com>]

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

Ragnar Kjørstad wrote:

>On Tue, Nov 23, 2004 at 07:46:04PM -0500, Steve Dickson wrote:
>  
>
>>Ragnar Kjørstad wrote:
>>    
>>
>>>1. What happens when you run statd with the "-n" option?
>>> Does this patch override the name the user gave?
>>>
>>>      
>>>
>>hmm.... this is a problem, since the global SM_stat_chge.mon_name
>>that the -n sets is now ignored... I guess I was thinking it would be better
>>for statd to dynamically set the name that sent the notify message.
>>But it would probably be a smart to maintain the same functionality.
>>
>>Question: do people actually run multiple statds using the -n to define
>>the interface they monitor?  That's a scenario  I guess  I didn't think 
>>of but
>>it sound a bit awkward....
>>    
>>
>
>Let me just explain why -n was added in the first place:
>  
>
thanks... that gave me a bit more insight on how the -n is used.... 

>>> This may or may not match the hostname that the server find when
>>> running gethostbyaddr on the interface's IP, so one can easily find
>>> scenarios where this patch will cause statd to stop working.
>>>      
>>>
>>I don't think this is an issue... as long as the hostname and all of its 
>>aliases
>>resolve to the same ip address, things should work since its the ip address
>>the kernel needs to find the lock that has to be reclaimed... 
>>    
>>
>OK, so the client maps the name back to the IP?
>and compares it to gethostbyname() on the hostname that it used to
>mount?
>  
>
close, the client statd does the gethostbyname() and send the ip address
to the kernel via loopback... The kernel then uses that ip address along
with protocol and version to locate the lock.

>Is this in the specification, or only the linux-implementation?
>
>If that's the case, are we allowed to just send the IP instead of the
>name?
>  
>
No, since the spec says a the id_mon needs to be character string....

>  
>
>>Now the 
>>problem
>>arises when the hostname resolves to a different ip address.... 
>>something this patch
>>is trying to address...
>>    
>>
>
>I didn't know the client actually compared IPs - I thought it compared
>the actual names.
>  
>
The kernel searches the list of nlm_hosts look for the correct 
ipaddr,proto,vers
threesome...

>Then I guess it's less of a problem than I thought.
>
>A couple of cases where I think it would fail though:
>- If you have inconsistant name-resolution (/etc/hosts) in
>  your system.
>  
>
Well I don't think it possible to program for a screwy DNS servers,
or should we try.... imho...

>- If you have aliases (eth0, eth0:0) with different IPs but on the
>  same network. The current patch (if I read it correctly) will then
>  use the name from the last alias, which is kind of random.
>  
>
Well it would not be random, it would be the first interface....

>The second case may be very relevant to the HA-problem which "-n" was
>originally added for.
>  
>
Yes... the -n flag wold be the answer here...

> <>
>
>There are two reasons to keep -n:
>
I see your point.... especially since all the HA code is now in
the updated FC3 nfs-utils... Believe me the last thing I want to do
is break  that code....  ;-) so attached is an updated patch that should
not affect the HA code at all....

SteveD.

[-- Attachment #2: nfs-utils-1.0.6-statd-notify-hostname2.patch --]
[-- Type: text/plain, Size: 3428 bytes --]

--- nfs-utils-1.0.6/utils/statd/rmtcall.c.orig	2004-11-24 05:42:13.494353000 -0500
+++ nfs-utils-1.0.6/utils/statd/rmtcall.c	2004-11-24 06:02:37.935145000 -0500
@@ -26,6 +26,7 @@
 #include <sys/socket.h>
 #include <sys/time.h>
 #include <netinet/in.h>
+#include <net/if.h>
 #include <arpa/inet.h>
 #include <rpc/rpc.h>
 #include <rpc/pmap_prot.h>
@@ -34,6 +35,7 @@
 #include <netdb.h>
 #include <string.h>
 #include <unistd.h>
+#include <ifaddrs.h>
 #include "sm_inter.h"
 #include "statd.h"
 #include "notlist.h"
@@ -73,7 +75,44 @@ statd_get_socket(int port)
 
 	return sockfd;
 }
-
+/*
+ * Using the NL_ADDR(lp), reset (if needed) the hostname
+ * that will be put in the SM_NOTIFY to the hostname
+ * that is associated with the network interface 
+ * that was monitored
+ */
+static void
+reset_my_name(notify_list *lp)
+{
+	struct ifaddrs *ifa = NULL, *ifap;
+	struct in_addr netaddr, tmp;
+	struct sockaddr_in *sin, *nsin;
+	struct hostent *hp;
+
+	netaddr.s_addr = inet_netof(NL_ADDR(lp));
+	if (getifaddrs(&ifa) >= 0) {
+		for (ifap = ifa; ifap != NULL; ifap = ifap->ifa_next) {
+			if (ifap->ifa_addr->sa_family != AF_INET)
+				continue;
+			if (!(ifap->ifa_flags & IFF_UP))
+				continue;
+			sin = (struct sockaddr_in *)ifap->ifa_addr;
+			nsin = (struct sockaddr_in *)ifap->ifa_netmask;
+			tmp.s_addr = sin->sin_addr.s_addr & nsin->sin_addr.s_addr;
+			if (memcmp(&tmp.s_addr, &netaddr.s_addr, sizeof(netaddr.s_addr)))
+				continue;
+			hp = gethostbyaddr((char *)&sin->sin_addr, 
+				sizeof(sin->sin_addr), AF_INET);
+			if (hp == NULL)
+				continue;
+			if (strcmp(NL_MY_NAME(lp), hp->h_name)) {
+				free(NL_MY_NAME(lp));
+				NL_MY_NAME(lp)= strdup(hp->h_name);
+			}
+		}
+	}
+	return;
+}
 /*
  * Try to resolve host name for notify/callback request
  *
@@ -283,6 +322,7 @@ process_entry(int sockfd, notify_list *l
 {
 	struct sockaddr_in	sin;
 	struct status		new_status;
+	stat_chge           new_stat;
 	xdrproc_t		func;
 	void			*objp;
 	u_int32_t		proc, vers, prog;
@@ -309,9 +349,19 @@ process_entry(int sockfd, notify_list *l
 
 		/* Use source address for notify replies */
 		sin.sin_addr   = lp->addr;
+		/* 
+		 * Unless a static hostname has been defined
+		 * set the NL_MY_NAME(lp) hostname to the 
+		 * one associated with the network interface
+		 */
+		if (!(run_mode & STATIC_HOSTNAME))
+			reset_my_name(lp);
 
 		func = (xdrproc_t) xdr_stat_chge;
-		objp = &SM_stat_chge;
+		new_stat.state = MY_STATE;
+		new_stat.mon_name = NL_MY_NAME(lp);
+
+		objp = &new_stat;
 		break;
 	case NOTIFY_CALLBACK:
 		prog = NL_MY_PROG(lp);
--- nfs-utils-1.0.6/utils/statd/statd.c.orig	2004-11-24 05:42:15.099369000 -0500
+++ nfs-utils-1.0.6/utils/statd/statd.c	2004-11-24 05:52:27.570265000 -0500
@@ -288,6 +288,7 @@ int main (int argc, char **argv)
 			}
 			break;
 		case 'n':	/* Specify local hostname */
+			run_mode |= STATIC_HOSTNAME;
 			MY_NAME = xstrdup(optarg);
 			break;
 		case 'P':
--- nfs-utils-1.0.6/utils/statd/statd.h.orig	2004-11-24 05:42:14.899371000 -0500
+++ nfs-utils-1.0.6/utils/statd/statd.h	2004-11-24 05:53:50.516929000 -0500
@@ -79,6 +79,7 @@ extern int run_mode;
 /* LH - notify_only mode would be for notifying hosts on an IP alias
  * that just came back up, for ex, when failing over a HA service to
  * another host.... */
+#define STATIC_HOSTNAME 8	/* Always use the hostname set by -n */
 
 /*
  * Program name and version pointers -- See statd.c for the reasoning