[RFC] rpc_ping and looong timeouts

[RFC] rpc_ping and looong timeouts

[Jeff Moyer]

Hello, Ian, list,

You may recall my asking for the rpc_ping function to take TCP only servers
into account.  Well, be careful what you ask for!  Now, if a server is down
in a replicated server configuration, it takes quite a while to timeout
due to the TCP connect.  The mount will eventually succeed, but I've seen
this take up to 10 minutes.

Perhaps we should first test to see if a host is reachable at all before we
begin our rpc pings?

Comments?

Jeff

Re: [RFC] rpc_ping and looong timeouts

[Ian Kent]

On Thu, 3 Jun 2004, Jeff Moyer wrote:

> Hello, Ian, list,
> 
> You may recall my asking for the rpc_ping function to take TCP only servers
> into account.  Well, be careful what you ask for!  Now, if a server is down
> in a replicated server configuration, it takes quite a while to timeout
> due to the TCP connect.  The mount will eventually succeed, but I've seen
> this take up to 10 minutes.

I'm not sure that I see that same behaviour in my test environment.

I'll check and get back.
It's probably best if we understand this a little better before jumping 
in.

> 
> Perhaps we should first test to see if a host is reachable at all before we
> begin our rpc pings?

Sigh!

Will the fall out from the replicated server implementation it never end.

Ian

Re: [RFC] rpc_ping and looong timeouts

[Jeff Moyer]

==> Regarding Re: [autofs] [RFC] rpc_ping and looong timeouts; Ian Kent <raven@themaw.net> adds:

raven> On Thu, 3 Jun 2004, Jeff Moyer wrote:
>> Hello, Ian, list,
>> 
>> You may recall my asking for the rpc_ping function to take TCP only
>> servers into account.  Well, be careful what you ask for!  Now, if a
>> server is down in a replicated server configuration, it takes quite a
>> while to timeout due to the TCP connect.  The mount will eventually
>> succeed, but I've seen this take up to 10 minutes.

raven> I'm not sure that I see that same behaviour in my test environment.

raven> I'll check and get back.  It's probably best if we understand this a
raven> little better before jumping in.

I've strace'd the automount process, and a lot of time is spent in the
clnt_create, for both UDP and the TCP.  I implemented a non-blocking
connect, and it took my ls time down from 8m21s to 2m4s.  I'm still looking
into why the UDP stuff isn't timing out sooner.

>> Perhaps we should first test to see if a host is reachable at all before
>> we begin our rpc pings?

raven> Sigh!

raven> Will the fall out from the replicated server implementation it never
raven> end.

Despair not!  I'm sure we're on the home stretch, here!  ;)  I'll post a
patch for review when it's baked.

-Jeff

Re: [RFC] rpc_ping and looong timeouts

[Jeff Moyer]

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 2173 bytes --]

==> Regarding Re: [autofs] [RFC] rpc_ping and looong timeouts; Jeff Moyer <jmoyer@redhat.com> adds:

==> Regarding Re: [autofs] [RFC] rpc_ping and looong timeouts; Ian Kent <raven@themaw.net> adds:
raven> On Thu, 3 Jun 2004, Jeff Moyer wrote:
>>> Hello, Ian, list,
>>> 
>>> You may recall my asking for the rpc_ping function to take TCP only
>>> servers into account.  Well, be careful what you ask for!  Now, if a
>>> server is down in a replicated server configuration, it takes quite a
>>> while to timeout due to the TCP connect.  The mount will eventually
>>> succeed, but I've seen this take up to 10 minutes.

raven> I'm not sure that I see that same behaviour in my test environment.

raven> I'll check and get back.  It's probably best if we understand this a
raven> little better before jumping in.

jmoyer> I've strace'd the automount process, and a lot of time is spent in
jmoyer> the clnt_create, for both UDP and the TCP.  I implemented a
jmoyer> non-blocking connect, and it took my ls time down from 8m21s to
jmoyer> 2m4s.  I'm still looking into why the UDP stuff isn't timing out
jmoyer> sooner.

The clnt_create call does not take a timeout.  One of the things it does is
to contact the remote portmapper to get the port for the specified program
number.  There is a hard-coded default timeout in that code of 60 seconds,
which explains my 2 minutes.

This patch simply implements an icmp ping (code shamelessly stolen from
clumanager), and does this before even attempting to do the rpc pings.  If
the icmp ping works, then we continue as usual.  This takes care of the
host down case quite nicely:

# time ls /multi/multi1
foo

real    0m0.208s
user    0m0.010s
sys     0m0.010s

Ian, I can also post the nonblocking connect patch, but I'm unsure of its
usefulness vs. complexity ratio.  I say we only cross that bridge if we
need to.

One nit: icmp_ping_host takes a char * for the hostname, whereas rpc_ping
passes a const char *.  As such, we get a 'discards qualifier' warning.  I
didn't think it warrented converting the whole call chain to consts, but if
you're especially concerned about that, Ian, I'd be happy to resubmit.


Comments?

-Jeff


[-- Attachment #2: autofs-4.1.3-icmp-ping.patch --]
[-- Type: text/plain, Size: 11928 bytes --]

--- autofs-4.1.3/include/ping.h.orig	2004-06-04 17:35:44.000000000 -0400
+++ autofs-4.1.3/include/ping.h	2004-06-04 17:26:08.000000000 -0400
@@ -0,0 +1,60 @@
+/*
+  Copyright Red Hat, Inc. 2003
+
+  This program is free software; you can redistribute it and/or modify it
+  under the terms of the GNU General Public License as published by the
+  Free Software Foundation; either version 2, or (at your option) any
+  later version.
+
+  This program is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; see the file COPYING.  If not, write to the
+  Free Software Foundation, Inc.,  675 Mass Ave, Cambridge,
+  MA 02139, USA.
+ */
+/** @file
+ * Header for ping.c.
+ */
+#ifndef _PING_H
+#define _PING_H
+
+#include <netinet/ip_icmp.h>
+#include <sys/socket.h>
+#include <netdb.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdint.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#define PING_ERRNO		-1
+#define PING_SUCCESS		0
+#define PING_ALIVE		PING_SUCCESS
+#define PING_TIMEOUT		1
+#define PING_HOST_UNREACH	2
+#define PING_HOST_NOT_FOUND	3
+#define PING_INVALID_CHECKSUM	4
+#define PING_INVALID_RESPONSE	5
+#define PING_INVALID_SIZE	6
+#define PING_INVALID_ID		7
+
+int32_t icmp_socket(void);
+int32_t icmp_ping_hostfd(int32_t sock, char *hostname, uint32_t seq,
+			 long seconds, long micros);
+int32_t icmp_ping_host(char *hostname, uint32_t seq, long seconds,long micros);
+
+int32_t icmp_ping_addrfd(int32_t sock, struct sockaddr_in *sin_send, 
+			 uint32_t seq, long seconds, long micros);
+int32_t icmp_ping_addr(struct sockaddr_in *sin_send, uint32_t seq,
+		       long seconds, long micros);
+
+
+#define net_icmp_close(sock) close(sock)
+
+#endif /* _PING_H */
--- autofs-4.1.3/lib/rpc_subs.c.orig	2004-06-04 17:35:32.000000000 -0400
+++ autofs-4.1.3/lib/rpc_subs.c	2004-06-04 17:36:05.000000000 -0400
@@ -5,6 +5,7 @@
 #include <rpc/xdr.h>
  
 #include "automount.h"
+#include "ping.h"
 
 static int rpc_ping_proto(const char *host,
 			  unsigned long nfs_version, const char *proto,
@@ -71,6 +72,10 @@
 {
 	unsigned int status;
 
+	status = icmp_ping_host(host, 0, seconds, micros);
+	if (status)
+		return 0;
+
 	status = rpc_ping_v2(host, seconds, micros);
 	if (status)
 		return status;
@@ -114,4 +119,3 @@
 
 	return status;
 }
-
--- autofs-4.1.3/lib/ping.c.orig	2004-06-04 17:35:38.000000000 -0400
+++ autofs-4.1.3/lib/ping.c	2004-06-04 17:26:08.000000000 -0400
@@ -0,0 +1,384 @@
+/*
+  Copyright Red Hat, Inc. 2003
+
+  This program is free software; you can redistribute it and/or modify it
+  under the terms of the GNU General Public License as published by the
+  Free Software Foundation; either version 2, or (at your option) any
+  later version.
+
+  This program is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; see the file COPYING.  If not, write to the
+  Free Software Foundation, Inc.,  675 Mass Ave, Cambridge,
+  MA 02139, USA.
+*/
+/** @file
+ * Ping functions for Red Hat Cluster Manager (RFC777)
+ *
+ * These functions provide internal ping functionality.  These functions are
+ * used to help determine quorum when a two or four node cluster has an
+ * even split.
+ */
+ 
+#include <ping.h>
+
+/**
+ * From RFC 777:
+ *
+ * [ICMP] Header Checksum
+ * The 16 bit one's complement of the one's complement sum of all 16
+ * bit words in the header.  For computing the checksum, the checksum
+ * field should be zero.  This checksum may be replaced in the
+ * future.
+ *
+ * @param buf		16-but word array
+ * @param buflen	Length (in 8-bit bytes!) of buf
+ * @return		ICMP header checksum of buf.
+ */
+uint16_t
+icmp_checksum(uint16_t *buf, uint32_t buflen)
+{
+	uint32_t remain = buflen, sum = 0;
+	char *data = (char *)buf;
+
+	while (remain > 1) {
+		sum += *((uint16_t *)data);
+		data += 2;
+		remain -= 2;
+	}
+
+	/*
+	 * Clean up the last byte (if there is one)
+	 */
+	if (remain)
+		sum += *data;
+
+	sum = (sum>>16)+(sum&0xffff);
+	sum += (sum>>16);
+	
+	return (uint16_t)((~sum)&0xffff);
+}
+
+
+/**
+ * Set up an ICMP socket (basically, a raw socket).  This requires root 
+ * privileges.
+ *
+ * @return See socket(2).
+ */
+int32_t
+icmp_socket(void)
+{
+	return socket(AF_INET, SOCK_RAW, IPPROTO_ICMP);
+}
+
+
+/**
+ * Call gethostbyname and return an appropriate PING return value based on
+ * the response we get.  Fill in sin_send.
+ *
+ * @param hostname	Hostname to look up.
+ * @param sin_send	IP address structure (pre-allocated).
+ * @return		PING_HOST_NOT_FOUND if the host is not found, 0
+ *			on success, -1 on other error.
+ * @see gethostbyname
+ */
+int32_t
+icmp_ping_getaddr(char *hostname, struct sockaddr_in *sin_send)
+{
+	struct hostent *hp;
+
+	/*
+	 * Grab the hostname
+	 */
+	while ((hp = gethostbyname(hostname)) == NULL) {
+		switch(h_errno) {
+		case TRY_AGAIN:
+			continue;
+		case HOST_NOT_FOUND:
+		case NO_ADDRESS:
+		case NO_RECOVERY:
+			return PING_HOST_NOT_FOUND;
+		}
+		return -1;
+	}
+
+	/*
+	 * Set up send address
+	 */
+	memset(sin_send, 0, sizeof(*sin_send));
+	sin_send->sin_family = AF_INET;
+	sin_send->sin_port = 0;
+	memcpy(&sin_send->sin_addr, hp->h_addr, sizeof(sin_send->sin_addr));
+
+	return 0;
+}
+
+
+/**
+ * Send a ping (ICMP_ECHO) to a given IP address and file descriptor.
+ * This is set up so that a daemon can drop privileges after binding to a raw
+ * socket (perhaps preserving a static ping socket), but still be able to use
+ * ping.  This is a pretty long function.
+ *
+ * @param sock		Socket to send on.
+ * @param sin_send	Address to send to.
+ * @param seq		Sequence number.
+ * @param timeout	Timeout (in seconds)
+ * @return		-1 on syscall error, 0 on success.
+ *			See ping.h for list of return values >0.
+ * @see	icmp_ping_host icmp_ping_hostfd icmp_ping_addr
+ */
+int32_t
+icmp_ping_addrfd(int32_t sock, struct sockaddr_in *sin_send, uint32_t seq,
+		 long seconds, long micros)
+{
+	char buffer[256]; /* XXX */
+	struct icmp *packetp;
+	struct ip *ipp;
+	struct sockaddr_in sin_recv;
+	struct timeval tv;
+	fd_set rfds;
+	uint16_t checksum;
+	uint32_t x, sin_recv_len = sizeof(sin_recv);
+	int timeout = (seconds || micros);
+
+	/*
+	 * Set up ICMP echo packet
+	 */
+	packetp = (struct icmp *)buffer;
+
+	memset(packetp, 0, ICMP_MINLEN);
+	packetp->icmp_type = ICMP_ECHO;
+	packetp->icmp_seq = seq;
+	packetp->icmp_id = getpid();
+	packetp->icmp_cksum = icmp_checksum((uint16_t *)packetp,
+						ICMP_MINLEN);
+
+	/*
+	 * Send the packet
+	 */
+	while ((x = sendto(sock, packetp, ICMP_MINLEN, 0,
+	    		   (struct sockaddr *)sin_send,
+			   sizeof(*sin_send))) < ICMP_MINLEN)
+		if (x < 0)
+			return -1;
+
+	if (timeout) {
+		tv.tv_sec = seconds;
+		tv.tv_usec = micros;
+	}
+
+	/*
+	 * Wait for response
+	 */
+	while (1) {
+		
+		/*
+		 * Set up select call...
+		 */
+		FD_ZERO(&rfds);
+		FD_SET(sock,&rfds);
+		while ((x = select(sock+1, &rfds, NULL, NULL,
+		   		   timeout ? &tv : NULL)) <= 0) {
+			if (!x)
+				return PING_TIMEOUT;
+			return -1;
+		}
+		
+		/*
+		 * Receive response
+		 */
+		if ((x = recvfrom(sock, buffer, sizeof(buffer), 0,
+				  (struct sockaddr *)&sin_recv,
+				  &sin_recv_len)) < 0) {
+			return -1;
+		}
+
+		/*
+		 * Ensure it's the proper size...
+		 * - (ipp->ip_hl << 2) is because IP header length is in
+		 * 32-bit words instead of bytes.
+		 * - ICMP_MINLEN is defined in netinet/ip_icmp.h.
+		 */
+		ipp = (struct ip *)buffer;
+		if (x < ((ipp->ip_hl << 2) + ICMP_MINLEN)) {
+			if (timeout)
+				continue;
+			return PING_INVALID_SIZE;
+		}
+		
+		/*
+		 * Validate the checksum.  The checksum needs to be set to
+		 * 0 for validation purposes.
+		 */
+		packetp = (struct icmp *)((buffer + (ipp->ip_hl << 2)));
+		checksum = packetp->icmp_cksum;
+		packetp->icmp_cksum = 0;
+		if (checksum != icmp_checksum((uint16_t*)packetp,
+					      ICMP_MINLEN)) {
+			if (timeout)
+				continue;
+			return PING_INVALID_CHECKSUM;
+		}
+
+		/*
+		 * Ensure it's the proper id...
+		 */
+		switch (packetp->icmp_type) {
+		case ICMP_ECHOREPLY:
+			if (packetp->icmp_id != getpid()) {
+				if (timeout)
+					continue;
+				return PING_INVALID_ID;
+			}
+			return PING_SUCCESS;
+		case ICMP_DEST_UNREACH:
+			return PING_HOST_UNREACH;
+		}
+
+		/* XXX */
+		return PING_INVALID_RESPONSE;
+	}
+}
+
+
+/**
+ * Send a ping (ICMP_ECHO) to a given IP address.  This is set up so that a
+ * daemon can drop privileges after binding to a raw socket (perhaps
+ * preserving a static ping socket), but still be able to use ping calls.
+ *
+ * @param sock		Socket to send on
+ * @param hostname	Host name to ping
+ * @param seq		Sequence number.
+ * @param timeout	Timeout (in seconds)
+ * @return		-1 on syscall error, 0 on success.
+ *			See ping.h for list of return values >0.
+ * @see	icmp_ping_getaddr icmp_ping_addrfd icmp_ping 
+ */
+int32_t
+icmp_ping_hostfd(int32_t sock, char *hostname, uint32_t seq, 
+		 long seconds, long micros)
+{
+	struct sockaddr_in sin_send;
+	int rv;
+
+	rv = icmp_ping_getaddr(hostname, &sin_send);
+	if (rv)
+		return rv;
+
+	return icmp_ping_addrfd(sock, &sin_send, seq, seconds, micros);
+}
+
+
+/**
+ * Send a ping (ICMP_ECHO) to a given IP address. 
+ *
+ * @param sin_send	Address we want to send to.
+ * @param seq		Sequence number (user defined)
+ * @param timeout	Timeout (in seconds)
+ * @return		-1 on syscall error, 0 on success.
+ *			See ping.h for list of return values >0.
+ * @see	icmp_ping_addrfd icmp_socket icmp_ping_host icmp_ping_hostfd
+ */
+int32_t
+icmp_ping_addr(struct sockaddr_in *sin_send, uint32_t seq, 
+	       long seconds, long micros)
+{
+	int sock, rv, esv;
+
+	sock = icmp_socket();
+	if (sock < 0)
+		return -1;
+
+	rv = icmp_ping_addrfd(sock, sin_send, seq, seconds, micros);
+
+	esv = errno;
+	close(sock);
+	errno = esv;
+
+	return rv;
+}
+
+
+/**
+ * Send a ping (ICMP_ECHO) to a given hostname.
+ *
+ * @param hostname	Target host
+ * @param seq		Sequence number (user defined)
+ * @param timeout	Timeout (in seconds)
+ * @return		-1 on syscall error, 0 on success.
+ *			See ping.h for list of return values >0.
+ * @see	icmp_socket icmp_ping_hostfd icmp_ping_addr icmp_ping_addrfd
+ */
+int32_t
+icmp_ping_host(char *hostname, uint32_t seq, long seconds, long micros)
+{
+	uint32_t rv, sock, esv;
+
+	sock = icmp_socket();
+	if (sock == -1)
+		return -1;
+
+	rv = icmp_ping_hostfd(sock, hostname, seq, seconds, micros);
+
+	esv = errno;
+	close(sock);
+	errno = esv;
+
+	return rv;
+}
+
+
+#ifdef STANDALONE
+int
+main(int argc, char **argv)
+{
+	int timeout = 0, rv;
+
+	if (argc < 2) {
+		printf("usage: %s <host> [timeout]\n", argv[0]);
+		return 2;
+	}
+
+	if (argc == 3)
+		timeout = atoi(argv[2]);
+
+	rv = icmp_ping_host(argv[1], 1, timeout);
+
+	switch(rv) {
+	case PING_ERRNO:
+		perror("icmp_ping_host");
+		break;
+	case PING_SUCCESS:
+		printf("%s is alive\n", argv[1]);
+		break;
+	case PING_TIMEOUT:
+		printf("%s timed out\n", argv[1]);
+		break;
+	case PING_HOST_UNREACH:
+		printf("%s is unreachable\n", argv[1]);
+		break;
+	case PING_HOST_NOT_FOUND:
+		printf("Host %s not found!\n", argv[1]);
+		break;
+	case PING_INVALID_CHECKSUM:
+		printf("Invalid checksum in reply.\n");
+		break;
+	case PING_INVALID_SIZE:
+		printf("Invalid size of reply packet.\n");
+		break;
+	case PING_INVALID_RESPONSE:
+		printf("Invalid response.\n");
+		break;
+	case PING_INVALID_ID:
+		printf("Invalid ID in response.\n");
+		break;
+	}
+	return rv;
+}
+#endif

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

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

Re: [RFC] rpc_ping and looong timeouts

[raven]

On Fri, 4 Jun 2004, Jeff Moyer wrote:

> ==> Regarding Re: [autofs] [RFC] rpc_ping and looong timeouts; Ian Kent <raven@themaw.net> adds:
> 
> raven> On Thu, 3 Jun 2004, Jeff Moyer wrote:
> >> Hello, Ian, list,
> >> 
> >> You may recall my asking for the rpc_ping function to take TCP only
> >> servers into account.  Well, be careful what you ask for!  Now, if a
> >> server is down in a replicated server configuration, it takes quite a
> >> while to timeout due to the TCP connect.  The mount will eventually
> >> succeed, but I've seen this take up to 10 minutes.
> 
> raven> I'm not sure that I see that same behaviour in my test environment.
> 
> raven> I'll check and get back.  It's probably best if we understand this a
> raven> little better before jumping in.
> 
> I've strace'd the automount process, and a lot of time is spent in the
> clnt_create, for both UDP and the TCP.  I implemented a non-blocking
> connect, and it took my ls time down from 8m21s to 2m4s.  I'm still looking
> into why the UDP stuff isn't timing out sooner.
>

Cool. I'm wanting an excuse to look at the sunrpc code anyway.

I'll have a bit of a poke around as well.
 
> >> Perhaps we should first test to see if a host is reachable at all before
> >> we begin our rpc pings?
> 
> raven> Sigh!
> 
> raven> Will the fall out from the replicated server implementation it never
> raven> end.
> 
> Despair not!  I'm sure we're on the home stretch, here!  ;)  I'll post a
> patch for review when it's baked.

Great I'm looking forward to it.

Ian

Re: [RFC] rpc_ping and looong timeouts

[raven]

On Fri, 4 Jun 2004, Jeff Moyer wrote:

> 
> jmoyer> I've strace'd the automount process, and a lot of time is spent in
> jmoyer> the clnt_create, for both UDP and the TCP.  I implemented a
> jmoyer> non-blocking connect, and it took my ls time down from 8m21s to
> jmoyer> 2m4s.  I'm still looking into why the UDP stuff isn't timing out
> jmoyer> sooner.
> 
> The clnt_create call does not take a timeout.  One of the things it does is
> to contact the remote portmapper to get the port for the specified program
> number.  There is a hard-coded default timeout in that code of 60 seconds,
> which explains my 2 minutes.
> 
> This patch simply implements an icmp ping (code shamelessly stolen from
> clumanager), and does this before even attempting to do the rpc pings.  If
> the icmp ping works, then we continue as usual.  This takes care of the
> host down case quite nicely:
> 
> # time ls /multi/multi1
> foo
> 
> real    0m0.208s
> user    0m0.010s
> sys     0m0.010s
> 
> Ian, I can also post the nonblocking connect patch, but I'm unsure of its
> usefulness vs. complexity ratio.  I say we only cross that bridge if we
> need to.

Agreed.

> 
> One nit: icmp_ping_host takes a char * for the hostname, whereas rpc_ping
> passes a const char *.  As such, we get a 'discards qualifier' warning.  I
> didn't think it warrented converting the whole call chain to consts, but if
> you're especially concerned about that, Ian, I'd be happy to resubmit.
>

I'll see if I can get rid of the warning in some simple way. If only as a 
clear indication it's what we intended to do.

In fact I will be able to use this to make the umount smarter later some 
time.
 
> 
> Comments?

I've only had a brief look at the patch but it looks like good clean code.

One small thing though.

I understand you need to attribute Copyright to Redhat but don't you think 
it would be acceptable to add your name their as well?

Ian

Re: [RFC] rpc_ping and looong timeouts

[raven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3695 bytes --]

On Fri, 4 Jun 2004, Jeff Moyer wrote:

A couple of things ...

> 
> jmoyer> I've strace'd the automount process, and a lot of time is spent in
> jmoyer> the clnt_create, for both UDP and the TCP.  I implemented a
> jmoyer> non-blocking connect, and it took my ls time down from 8m21s to
> jmoyer> 2m4s.  I'm still looking into why the UDP stuff isn't timing out
> jmoyer> sooner.
> 
> The clnt_create call does not take a timeout.  One of the things it does is
> to contact the remote portmapper to get the port for the specified program
> number.  There is a hard-coded default timeout in that code of 60 seconds,
> which explains my 2 minutes.
> 
> This patch simply implements an icmp ping (code shamelessly stolen from
> clumanager), and does this before even attempting to do the rpc pings.  If
> the icmp ping works, then we continue as usual.  This takes care of the
> host down case quite nicely:
> 
> One nit: icmp_ping_host takes a char * for the hostname, whereas rpc_ping
> passes a const char *.  As such, we get a 'discards qualifier' warning.  I
> didn't think it warrented converting the whole call chain to consts, but if
> you're especially concerned about that, Ian, I'd be happy to resubmit.

I've put a cast in to eliminate the warning.

I added the Makefile glue and ...

I noticed that, for the case the request is not a replicated server entry 
a mount is still attempted so I added a ping check.

I've attached the complete patch but here is what I added. Can you test 
it in your environment before I commit it please:

diff -Nur autofs-4.1.3.orig/lib/Makefile autofs-4.1.3/lib/Makefile
--- autofs-4.1.3.orig/lib/Makefile	2004-03-07 20:17:54.000000000 +0800
+++ autofs-4.1.3/lib/Makefile	2004-06-05 13:13:05.000000000 +0800
@@ -9,9 +9,9 @@
 RPCGEN = /usr/bin/rpcgen
 RANLIB = /usr/bin/ranlib
 
-SRCS = cache.c listmount.c cat_path.c rpc_subs.c
+SRCS = cache.c listmount.c cat_path.c rpc_subs.c ping.c
 RPCS = mount.h mount_clnt.c mount_xdr.c
-OBJS = cache.o mount_clnt.o mount_xdr.o listmount.o cat_path.o rpc_subs.o
+OBJS = cache.o mount_clnt.o mount_xdr.o listmount.o cat_path.o rpc_subs.o ping.o
 
 LIB = autofs.a
 
diff -Nur autofs-4.1.3.orig/lib/rpc_subs.c autofs-4.1.3/lib/rpc_subs.c
--- autofs-4.1.3.orig/lib/rpc_subs.c	2004-06-05 13:13:43.000000000 +0800
+++ autofs-4.1.3/lib/rpc_subs.c	2004-06-05 13:13:05.000000000 +0800
@@ -72,7 +72,7 @@
 {
 	unsigned int status;
 
-	status = icmp_ping_host(host, 0, seconds, micros);
+	status = icmp_ping_host((char *) host, 0, seconds, micros);
 	if (status)
 		return 0;
 
diff -Nur autofs-4.1.3.orig/modules/mount_nfs.c autofs-4.1.3/modules/mount_nfs.c
--- autofs-4.1.3.orig/modules/mount_nfs.c	2004-05-18 20:20:08.000000000 +0800
+++ autofs-4.1.3/modules/mount_nfs.c	2004-06-05 13:13:05.000000000 +0800
@@ -142,6 +142,7 @@
 	while (p && *p) {
 		char *next;
 		unsigned int ping_stat = 0;
+		unsigned int status;
 
 		p += strspn(p, " \t,");
 		delim = strpbrk(p, "(, \t:");
@@ -216,13 +217,24 @@
 				break;
 		}
 
-		/*
-		 * If it's not local and it's a replicated server map entry
-		 * is it alive
-		 */
-		if (!local && is_replicated && !(ping_stat = rpc_ping(p, sec, micros))) {
-			p = next;
-			continue;
+		/* If it's not local */
+		if (!local) {
+			/* and it's a replicated server map entry, check
+			 * if it's alive and servicing NFS requests */
+			if (is_replicated) {
+				ping_stat = rpc_ping(p, sec, micros);
+				if (!ping_stat) {
+					p = next;
+					continue;
+				}
+			/* otherwise just check if it's alive */
+			} else {
+				status = icmp_ping_host(p, 0, sec, micros);
+				if (status) {
+					*what = '\0';
+					return 0;
+				}
+			}
 		}
 
 		/* see if we have a previous 'winner' */

[-- Attachment #2: icmp patch + my changes --]
[-- Type: TEXT/PLAIN, Size: 14426 bytes --]

diff -Nur autofs-4.1.3.orig/include/ping.h autofs-4.1.3/include/ping.h
--- autofs-4.1.3.orig/include/ping.h	1970-01-01 08:00:00.000000000 +0800
+++ autofs-4.1.3/include/ping.h	2004-06-05 11:00:09.000000000 +0800
@@ -0,0 +1,60 @@
+/*
+  Copyright Red Hat, Inc. 2003
+
+  This program is free software; you can redistribute it and/or modify it
+  under the terms of the GNU General Public License as published by the
+  Free Software Foundation; either version 2, or (at your option) any
+  later version.
+
+  This program is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; see the file COPYING.  If not, write to the
+  Free Software Foundation, Inc.,  675 Mass Ave, Cambridge,
+  MA 02139, USA.
+ */
+/** @file
+ * Header for ping.c.
+ */
+#ifndef _PING_H
+#define _PING_H
+
+#include <netinet/ip_icmp.h>
+#include <sys/socket.h>
+#include <netdb.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdint.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#define PING_ERRNO		-1
+#define PING_SUCCESS		0
+#define PING_ALIVE		PING_SUCCESS
+#define PING_TIMEOUT		1
+#define PING_HOST_UNREACH	2
+#define PING_HOST_NOT_FOUND	3
+#define PING_INVALID_CHECKSUM	4
+#define PING_INVALID_RESPONSE	5
+#define PING_INVALID_SIZE	6
+#define PING_INVALID_ID		7
+
+int32_t icmp_socket(void);
+int32_t icmp_ping_hostfd(int32_t sock, char *hostname, uint32_t seq,
+			 long seconds, long micros);
+int32_t icmp_ping_host(char *hostname, uint32_t seq, long seconds,long micros);
+
+int32_t icmp_ping_addrfd(int32_t sock, struct sockaddr_in *sin_send, 
+			 uint32_t seq, long seconds, long micros);
+int32_t icmp_ping_addr(struct sockaddr_in *sin_send, uint32_t seq,
+		       long seconds, long micros);
+
+
+#define net_icmp_close(sock) close(sock)
+
+#endif /* _PING_H */
diff -Nur autofs-4.1.3.orig/lib/Makefile autofs-4.1.3/lib/Makefile
--- autofs-4.1.3.orig/lib/Makefile	2004-05-30 10:35:40.000000000 +0800
+++ autofs-4.1.3/lib/Makefile	2004-06-05 11:03:56.000000000 +0800
@@ -9,9 +9,9 @@
 RPCGEN = /usr/bin/rpcgen
 RANLIB = /usr/bin/ranlib
 
-SRCS = cache.c listmount.c cat_path.c rpc_subs.c
+SRCS = cache.c listmount.c cat_path.c rpc_subs.c ping.c
 RPCS = mount.h mount_clnt.c mount_xdr.c
-OBJS = cache.o mount_clnt.o mount_xdr.o listmount.o cat_path.o rpc_subs.o
+OBJS = cache.o mount_clnt.o mount_xdr.o listmount.o cat_path.o rpc_subs.o ping.o
 
 LIB = autofs.a
 
diff -Nur autofs-4.1.3.orig/lib/ping.c autofs-4.1.3/lib/ping.c
--- autofs-4.1.3.orig/lib/ping.c	1970-01-01 08:00:00.000000000 +0800
+++ autofs-4.1.3/lib/ping.c	2004-06-05 11:00:09.000000000 +0800
@@ -0,0 +1,384 @@
+/*
+  Copyright Red Hat, Inc. 2003
+
+  This program is free software; you can redistribute it and/or modify it
+  under the terms of the GNU General Public License as published by the
+  Free Software Foundation; either version 2, or (at your option) any
+  later version.
+
+  This program is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; see the file COPYING.  If not, write to the
+  Free Software Foundation, Inc.,  675 Mass Ave, Cambridge,
+  MA 02139, USA.
+*/
+/** @file
+ * Ping functions for Red Hat Cluster Manager (RFC777)
+ *
+ * These functions provide internal ping functionality.  These functions are
+ * used to help determine quorum when a two or four node cluster has an
+ * even split.
+ */
+ 
+#include <ping.h>
+
+/**
+ * From RFC 777:
+ *
+ * [ICMP] Header Checksum
+ * The 16 bit one's complement of the one's complement sum of all 16
+ * bit words in the header.  For computing the checksum, the checksum
+ * field should be zero.  This checksum may be replaced in the
+ * future.
+ *
+ * @param buf		16-but word array
+ * @param buflen	Length (in 8-bit bytes!) of buf
+ * @return		ICMP header checksum of buf.
+ */
+uint16_t
+icmp_checksum(uint16_t *buf, uint32_t buflen)
+{
+	uint32_t remain = buflen, sum = 0;
+	char *data = (char *)buf;
+
+	while (remain > 1) {
+		sum += *((uint16_t *)data);
+		data += 2;
+		remain -= 2;
+	}
+
+	/*
+	 * Clean up the last byte (if there is one)
+	 */
+	if (remain)
+		sum += *data;
+
+	sum = (sum>>16)+(sum&0xffff);
+	sum += (sum>>16);
+	
+	return (uint16_t)((~sum)&0xffff);
+}
+
+
+/**
+ * Set up an ICMP socket (basically, a raw socket).  This requires root 
+ * privileges.
+ *
+ * @return See socket(2).
+ */
+int32_t
+icmp_socket(void)
+{
+	return socket(AF_INET, SOCK_RAW, IPPROTO_ICMP);
+}
+
+
+/**
+ * Call gethostbyname and return an appropriate PING return value based on
+ * the response we get.  Fill in sin_send.
+ *
+ * @param hostname	Hostname to look up.
+ * @param sin_send	IP address structure (pre-allocated).
+ * @return		PING_HOST_NOT_FOUND if the host is not found, 0
+ *			on success, -1 on other error.
+ * @see gethostbyname
+ */
+int32_t
+icmp_ping_getaddr(char *hostname, struct sockaddr_in *sin_send)
+{
+	struct hostent *hp;
+
+	/*
+	 * Grab the hostname
+	 */
+	while ((hp = gethostbyname(hostname)) == NULL) {
+		switch(h_errno) {
+		case TRY_AGAIN:
+			continue;
+		case HOST_NOT_FOUND:
+		case NO_ADDRESS:
+		case NO_RECOVERY:
+			return PING_HOST_NOT_FOUND;
+		}
+		return -1;
+	}
+
+	/*
+	 * Set up send address
+	 */
+	memset(sin_send, 0, sizeof(*sin_send));
+	sin_send->sin_family = AF_INET;
+	sin_send->sin_port = 0;
+	memcpy(&sin_send->sin_addr, hp->h_addr, sizeof(sin_send->sin_addr));
+
+	return 0;
+}
+
+
+/**
+ * Send a ping (ICMP_ECHO) to a given IP address and file descriptor.
+ * This is set up so that a daemon can drop privileges after binding to a raw
+ * socket (perhaps preserving a static ping socket), but still be able to use
+ * ping.  This is a pretty long function.
+ *
+ * @param sock		Socket to send on.
+ * @param sin_send	Address to send to.
+ * @param seq		Sequence number.
+ * @param timeout	Timeout (in seconds)
+ * @return		-1 on syscall error, 0 on success.
+ *			See ping.h for list of return values >0.
+ * @see	icmp_ping_host icmp_ping_hostfd icmp_ping_addr
+ */
+int32_t
+icmp_ping_addrfd(int32_t sock, struct sockaddr_in *sin_send, uint32_t seq,
+		 long seconds, long micros)
+{
+	char buffer[256]; /* XXX */
+	struct icmp *packetp;
+	struct ip *ipp;
+	struct sockaddr_in sin_recv;
+	struct timeval tv;
+	fd_set rfds;
+	uint16_t checksum;
+	uint32_t x, sin_recv_len = sizeof(sin_recv);
+	int timeout = (seconds || micros);
+
+	/*
+	 * Set up ICMP echo packet
+	 */
+	packetp = (struct icmp *)buffer;
+
+	memset(packetp, 0, ICMP_MINLEN);
+	packetp->icmp_type = ICMP_ECHO;
+	packetp->icmp_seq = seq;
+	packetp->icmp_id = getpid();
+	packetp->icmp_cksum = icmp_checksum((uint16_t *)packetp,
+						ICMP_MINLEN);
+
+	/*
+	 * Send the packet
+	 */
+	while ((x = sendto(sock, packetp, ICMP_MINLEN, 0,
+	    		   (struct sockaddr *)sin_send,
+			   sizeof(*sin_send))) < ICMP_MINLEN)
+		if (x < 0)
+			return -1;
+
+	if (timeout) {
+		tv.tv_sec = seconds;
+		tv.tv_usec = micros;
+	}
+
+	/*
+	 * Wait for response
+	 */
+	while (1) {
+		
+		/*
+		 * Set up select call...
+		 */
+		FD_ZERO(&rfds);
+		FD_SET(sock,&rfds);
+		while ((x = select(sock+1, &rfds, NULL, NULL,
+		   		   timeout ? &tv : NULL)) <= 0) {
+			if (!x)
+				return PING_TIMEOUT;
+			return -1;
+		}
+		
+		/*
+		 * Receive response
+		 */
+		if ((x = recvfrom(sock, buffer, sizeof(buffer), 0,
+				  (struct sockaddr *)&sin_recv,
+				  &sin_recv_len)) < 0) {
+			return -1;
+		}
+
+		/*
+		 * Ensure it's the proper size...
+		 * - (ipp->ip_hl << 2) is because IP header length is in
+		 * 32-bit words instead of bytes.
+		 * - ICMP_MINLEN is defined in netinet/ip_icmp.h.
+		 */
+		ipp = (struct ip *)buffer;
+		if (x < ((ipp->ip_hl << 2) + ICMP_MINLEN)) {
+			if (timeout)
+				continue;
+			return PING_INVALID_SIZE;
+		}
+		
+		/*
+		 * Validate the checksum.  The checksum needs to be set to
+		 * 0 for validation purposes.
+		 */
+		packetp = (struct icmp *)((buffer + (ipp->ip_hl << 2)));
+		checksum = packetp->icmp_cksum;
+		packetp->icmp_cksum = 0;
+		if (checksum != icmp_checksum((uint16_t*)packetp,
+					      ICMP_MINLEN)) {
+			if (timeout)
+				continue;
+			return PING_INVALID_CHECKSUM;
+		}
+
+		/*
+		 * Ensure it's the proper id...
+		 */
+		switch (packetp->icmp_type) {
+		case ICMP_ECHOREPLY:
+			if (packetp->icmp_id != getpid()) {
+				if (timeout)
+					continue;
+				return PING_INVALID_ID;
+			}
+			return PING_SUCCESS;
+		case ICMP_DEST_UNREACH:
+			return PING_HOST_UNREACH;
+		}
+
+		/* XXX */
+		return PING_INVALID_RESPONSE;
+	}
+}
+
+
+/**
+ * Send a ping (ICMP_ECHO) to a given IP address.  This is set up so that a
+ * daemon can drop privileges after binding to a raw socket (perhaps
+ * preserving a static ping socket), but still be able to use ping calls.
+ *
+ * @param sock		Socket to send on
+ * @param hostname	Host name to ping
+ * @param seq		Sequence number.
+ * @param timeout	Timeout (in seconds)
+ * @return		-1 on syscall error, 0 on success.
+ *			See ping.h for list of return values >0.
+ * @see	icmp_ping_getaddr icmp_ping_addrfd icmp_ping 
+ */
+int32_t
+icmp_ping_hostfd(int32_t sock, char *hostname, uint32_t seq, 
+		 long seconds, long micros)
+{
+	struct sockaddr_in sin_send;
+	int rv;
+
+	rv = icmp_ping_getaddr(hostname, &sin_send);
+	if (rv)
+		return rv;
+
+	return icmp_ping_addrfd(sock, &sin_send, seq, seconds, micros);
+}
+
+
+/**
+ * Send a ping (ICMP_ECHO) to a given IP address. 
+ *
+ * @param sin_send	Address we want to send to.
+ * @param seq		Sequence number (user defined)
+ * @param timeout	Timeout (in seconds)
+ * @return		-1 on syscall error, 0 on success.
+ *			See ping.h for list of return values >0.
+ * @see	icmp_ping_addrfd icmp_socket icmp_ping_host icmp_ping_hostfd
+ */
+int32_t
+icmp_ping_addr(struct sockaddr_in *sin_send, uint32_t seq, 
+	       long seconds, long micros)
+{
+	int sock, rv, esv;
+
+	sock = icmp_socket();
+	if (sock < 0)
+		return -1;
+
+	rv = icmp_ping_addrfd(sock, sin_send, seq, seconds, micros);
+
+	esv = errno;
+	close(sock);
+	errno = esv;
+
+	return rv;
+}
+
+
+/**
+ * Send a ping (ICMP_ECHO) to a given hostname.
+ *
+ * @param hostname	Target host
+ * @param seq		Sequence number (user defined)
+ * @param timeout	Timeout (in seconds)
+ * @return		-1 on syscall error, 0 on success.
+ *			See ping.h for list of return values >0.
+ * @see	icmp_socket icmp_ping_hostfd icmp_ping_addr icmp_ping_addrfd
+ */
+int32_t
+icmp_ping_host(char *hostname, uint32_t seq, long seconds, long micros)
+{
+	uint32_t rv, sock, esv;
+
+	sock = icmp_socket();
+	if (sock == -1)
+		return -1;
+
+	rv = icmp_ping_hostfd(sock, hostname, seq, seconds, micros);
+
+	esv = errno;
+	close(sock);
+	errno = esv;
+
+	return rv;
+}
+
+
+#ifdef STANDALONE
+int
+main(int argc, char **argv)
+{
+	int timeout = 0, rv;
+
+	if (argc < 2) {
+		printf("usage: %s <host> [timeout]\n", argv[0]);
+		return 2;
+	}
+
+	if (argc == 3)
+		timeout = atoi(argv[2]);
+
+	rv = icmp_ping_host(argv[1], 1, timeout);
+
+	switch(rv) {
+	case PING_ERRNO:
+		perror("icmp_ping_host");
+		break;
+	case PING_SUCCESS:
+		printf("%s is alive\n", argv[1]);
+		break;
+	case PING_TIMEOUT:
+		printf("%s timed out\n", argv[1]);
+		break;
+	case PING_HOST_UNREACH:
+		printf("%s is unreachable\n", argv[1]);
+		break;
+	case PING_HOST_NOT_FOUND:
+		printf("Host %s not found!\n", argv[1]);
+		break;
+	case PING_INVALID_CHECKSUM:
+		printf("Invalid checksum in reply.\n");
+		break;
+	case PING_INVALID_SIZE:
+		printf("Invalid size of reply packet.\n");
+		break;
+	case PING_INVALID_RESPONSE:
+		printf("Invalid response.\n");
+		break;
+	case PING_INVALID_ID:
+		printf("Invalid ID in response.\n");
+		break;
+	}
+	return rv;
+}
+#endif
diff -Nur autofs-4.1.3.orig/lib/rpc_subs.c autofs-4.1.3/lib/rpc_subs.c
--- autofs-4.1.3.orig/lib/rpc_subs.c	2004-05-18 20:20:08.000000000 +0800
+++ autofs-4.1.3/lib/rpc_subs.c	2004-06-05 12:47:13.000000000 +0800
@@ -5,6 +5,7 @@
 #include <rpc/xdr.h>
  
 #include "automount.h"
+#include "ping.h"
 
 static int rpc_ping_proto(const char *host,
 			  unsigned long nfs_version, const char *proto,
@@ -71,6 +72,10 @@
 {
 	unsigned int status;
 
+	status = icmp_ping_host((char *) host, 0, seconds, micros);
+	if (status)
+		return 0;
+
 	status = rpc_ping_v2(host, seconds, micros);
 	if (status)
 		return status;
@@ -114,4 +119,3 @@
 
 	return status;
 }
-
diff -Nur autofs-4.1.3.orig/modules/mount_nfs.c autofs-4.1.3/modules/mount_nfs.c
--- autofs-4.1.3.orig/modules/mount_nfs.c	2004-05-30 11:25:07.000000000 +0800
+++ autofs-4.1.3/modules/mount_nfs.c	2004-06-05 12:48:05.000000000 +0800
@@ -142,6 +142,7 @@
 	while (p && *p) {
 		char *next;
 		unsigned int ping_stat = 0;
+		unsigned int status;
 
 		p += strspn(p, " \t,");
 		delim = strpbrk(p, "(, \t:");
@@ -216,13 +217,24 @@
 				break;
 		}
 
-		/*
-		 * If it's not local and it's a replicated server map entry
-		 * is it alive
-		 */
-		if (!local && is_replicated && !(ping_stat = rpc_ping(p, sec, micros))) {
-			p = next;
-			continue;
+		/* If it's not local */
+		if (!local) {
+			/* and it's a replicated server map entry, check
+			 * if it's alive and servicing NFS requests */
+			if (is_replicated) {
+				ping_stat = rpc_ping(p, sec, micros);
+				if (!ping_stat) {
+					p = next;
+					continue;
+				}
+			/* otherwise just check if it's alive */
+			} else {
+				status = icmp_ping_host(p, 0, sec, micros);
+				if (status) {
+					*what = '\0';
+					return 0;
+				}
+			}
 		}
 
 		/* see if we have a previous 'winner' */

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

_______________________________________________
autofs mailing list
autofs@linux.kernel.org
http://linux.kernel.org/mailman/listinfo/autofs

Re: [RFC] rpc_ping and looong timeouts

[Jeff Moyer]

==> Regarding Re: [autofs] [RFC] rpc_ping and looong timeouts; raven@themaw.net adds:

raven> On Fri, 4 Jun 2004, Jeff Moyer wrote:
>>
[snip]
>> One nit: icmp_ping_host takes a char * for the hostname, whereas
>> rpc_ping passes a const char *.  As such, we get a 'discards qualifier'
>> warning.  I didn't think it warrented converting the whole call chain to
>> consts, but if you're especially concerned about that, Ian, I'd be happy
>> to resubmit.
>> 

raven> I'll see if I can get rid of the warning in some simple way. If only
raven> as a clear indication it's what we intended to do.

raven> In fact I will be able to use this to make the umount smarter later
raven> some time.

Cool.
 
>> Comments?

raven> I've only had a brief look at the patch but it looks like good clean
raven> code.

raven> One small thing though.

raven> I understand you need to attribute Copyright to Redhat but don't you
raven> think it would be acceptable to add your name their as well?

Heh.  Well, I didn't write the ping code (only modified it to make the
timeouts finer granularity).  Lon Hohberger wrote it.  I'll be sure
to pass along your kudos (and suggestion).  ;)

-Jeff

Re: [RFC] rpc_ping and looong timeouts

[Jeff Moyer]

==> Regarding Re: [autofs] [RFC] rpc_ping and looong timeouts; raven@themaw.net adds:

raven> On Fri, 4 Jun 2004, Jeff Moyer wrote: A couple of things ...

>>
jmoyer> I've strace'd the automount process, and a lot of time is spent in
jmoyer> the clnt_create, for both UDP and the TCP.  I implemented a
jmoyer> non-blocking connect, and it took my ls time down from 8m21s to
jmoyer> 2m4s.  I'm still looking into why the UDP stuff isn't timing out
jmoyer> sooner.
>> The clnt_create call does not take a timeout.  One of the things it does
>> is to contact the remote portmapper to get the port for the specified
>> program number.  There is a hard-coded default timeout in that code of
>> 60 seconds, which explains my 2 minutes.
>> 
>> This patch simply implements an icmp ping (code shamelessly stolen from
>> clumanager), and does this before even attempting to do the rpc pings.
>> If the icmp ping works, then we continue as usual.  This takes care of
>> the host down case quite nicely:
>> 
>> One nit: icmp_ping_host takes a char * for the hostname, whereas
>> rpc_ping passes a const char *.  As such, we get a 'discards qualifier'
>> warning.  I didn't think it warrented converting the whole call chain to
>> consts, but if you're especially concerned about that, Ian, I'd be happy
>> to resubmit.

raven> I've put a cast in to eliminate the warning.

raven> I added the Makefile glue and ...

Whoops, I can't believe I left that out!  Sorry!

raven> I noticed that, for the case the request is not a replicated server
raven> entry a mount is still attempted so I added a ping check.

raven> I've attached the complete patch but here is what I added. Can you
raven> test it in your environment before I commit it please:

Yes, that looks correct to me.  I tested it with the same test case I was
using before, and all is well.  One thing:

  mount_nfs.c: In function `get_best_mount':
  mount_nfs.c:232: warning: implicit declaration of function `icmp_ping_host'

You forgot to #include "ping.h"!  Maybe between the two of us we'll get
this right.  ;)

Thanks, Ian!

Jeff

Re: [RFC] rpc_ping and looong timeouts

[raven]

On Sat, 5 Jun 2004, Jeff Moyer wrote:

> 
> Yes, that looks correct to me.  I tested it with the same test case I was
> using before, and all is well.  One thing:
> 
>   mount_nfs.c: In function `get_best_mount':
>   mount_nfs.c:232: warning: implicit declaration of function `icmp_ping_host'
> 
> You forgot to #include "ping.h"!  Maybe between the two of us we'll get
> this right.  ;)

Ha! Missed that.

I'll fix it in mine and commit it.

Ian

Re: [RFC] rpc_ping and looong timeouts

[Jeff Moyer]

==> Regarding Re: [autofs] [RFC] rpc_ping and looong timeouts; raven@themaw.net adds:

raven> On Sat, 5 Jun 2004, Jeff Moyer wrote:
>> Yes, that looks correct to me.  I tested it with the same test case I
>> was using before, and all is well.  One thing:
>> 
mount_nfs.c> In function `get_best_mount': 232: warning: implicit
mount_nfs.c> declaration of function `icmp_ping_host'
>> You forgot to #include "ping.h"!  Maybe between the two of us we'll get
>> this right.  ;)

raven> Ha! Missed that.

raven> I'll fix it in mine and commit it.

For those interested, packages with this patch included are available on my
people site:
       http://people.redhat.com/~jmoyer/

Included is the changelog for 4.1.3 thus far.  Share and enjoy!

-Jeff


* Sat Jun  5 2004 Jeff Moyer <jmoyer@redhat.com> - 1:4.1.3-4 
- Perform an icmp ping request before rpc_pings, since the rpc clnt_create 
  function has a builtin default timeout of 60 seconds.  This could result 
  in a long delay when a server in a replicated mount setup is down. 
- For non-replicated server entries, ping a host before attempting to mount. 
  (Ian Kent) 
- Change to %configure. 
- Put version-release into .version to allow for automount --version to 
  print exact info. 
- Nuke my get-best-mount patch which always uses the long timeout.  This 
  should no longer be needed. 
- Put name into changelog entries to make them consistent.  Add e:n-v-r 
  into Florian's entry. 
- Stop autofs before uninstalling 
 
* Sat Jun 05 2004 Florian La Roche <Florian.LaRoche@redhat.de> - 1:4.1.3-3 
- add a preun script to remove autofs 
 
* Tue Jun  1 2004 Jeff Moyer <jmoyer@redhat.com> - 1:4.1.3-2 
- Incorporate patch from Ian which fixes an infinite loop seen by those 
  running older versions of the kernel patches (triggered by non-strict mounts 
  being the default). 
 
* Tue Jun  1 2004 Jeff Moyer <jmoyer@redhat.com> - 1:4.1.3-1 
- Update to upstream 4.1.3. 

Re: [RFC] rpc_ping and looong timeouts

[Michael Blandford]

Jeff Moyer wrote:

>This patch simply implements an icmp ping (code shamelessly stolen from
>clumanager), and does this before even attempting to do the rpc pings.  If
>the icmp ping works, then we continue as usual.  This takes care of the
>host down case quite nicely:
>
>  
>

Would this patch cause problems for those of us who have icmp blocked?  
If so, could we make it a run time option to enable/disable?

Michael

Disclaimer: The content of this message is my personal opinion only and 
although I am an employee of Intel, the statements I make here in no way 
represent Intel's position on the issue, nor am I authorized to speak on 
behalf of Intel on this matter.

Re: [RFC] rpc_ping and looong timeouts

[Jeff Moyer]

==> Regarding Re: [autofs] [RFC] rpc_ping and looong timeouts; Michael Blandford <mlblandf@sedona.ch.intel.com> adds:

mlblandf> Jeff Moyer wrote:
>> This patch simply implements an icmp ping (code shamelessly stolen from
>> clumanager), and does this before even attempting to do the rpc pings.
>> If the icmp ping works, then we continue as usual.  This takes care of
>> the host down case quite nicely:
>> 
>> 
>> 

mlblandf> Would this patch cause problems for those of us who have icmp
mlblandf> blocked?  If so, could we make it a run time option to
mlblandf> enable/disable?

Wow, you block icmp internally?  This would incur a timeout for every host
listed in your replicated server entry, and with Ian's patch, it would
incur a timeout for non-replicated servers, as well.  In fact, for the
non-replicated server case, I think it would give a false negative, failing
the mount even though the server is up.

Is this really how you have things configured?  NFS clients can't ping
their servers?

-Jeff

Re: [RFC] rpc_ping and looong timeouts

[Ian Kent]

On Mon, 7 Jun 2004, Michael Blandford wrote:

> Jeff Moyer wrote:
> 
> >This patch simply implements an icmp ping (code shamelessly stolen from
> >clumanager), and does this before even attempting to do the rpc pings.  If
> >the icmp ping works, then we continue as usual.  This takes care of the
> >host down case quite nicely:
> >
> >  
> >
> 
> Would this patch cause problems for those of us who have icmp blocked?  
> If so, could we make it a run time option to enable/disable?
> 

So you have ICMP blocked but allow NFS?

Ian

Re: [RFC] rpc_ping and looong timeouts

[Peter C. Norton]

On Tue, Jun 08, 2004 at 08:56:34AM +0800, Ian Kent wrote:
> On Mon, 7 Jun 2004, Michael Blandford wrote:
> 
> > Jeff Moyer wrote:
> > 
> > >This patch simply implements an icmp ping (code shamelessly stolen from
> > >clumanager), and does this before even attempting to do the rpc pings.  If
> > >the icmp ping works, then we continue as usual.  This takes care of the
> > >host down case quite nicely:
> > >
> > >  
> > >
> > 
> > Would this patch cause problems for those of us who have icmp blocked?  
> > If so, could we make it a run time option to enable/disable?
> > 
> 
> So you have ICMP blocked but allow NFS?

Its possible that some sites will have killed ping and pong because of
various viruses in the past, but allow everything else.

-Peter

-- 
The 5 year plan:
In five years we'll make up another plan.
Or just re-use this one.

Re: [RFC] rpc_ping and looong timeouts

[Michael Blandford]

Jeff Moyer wrote:

>mlblandf> Would this patch cause problems for those of us who have icmp
>mlblandf> blocked?  If so, could we make it a run time option to
>mlblandf> enable/disable?
>
>Wow, you block icmp internally?  This would incur a timeout for every host
>listed in your replicated server entry, and with Ian's patch, it would
>incur a timeout for non-replicated servers, as well.  In fact, for the
>non-replicated server case, I think it would give a false negative, failing
>the mount even though the server is up.
>
>Is this really how you have things configured?  NFS clients can't ping
>their servers?
>  
>

In a large environment it wouldn't be uncommon to have NFS mounts that 
span across a WAN.  Using NFS over TCP seems to make the most sense in 
that situation.

As packets traverse the WAN, there may be routers that block icmp.  This 
is the type of situation where replicated server would make the most 
sense - find the fastest server.


Michael

Disclaimer: The content of this message is my personal opinion only and 
although I am an employee of Intel, the statements I make here in no way 
represent Intel's position on the issue, nor am I authorized to speak on 
behalf of Intel on this matter.

Re: [RFC] rpc_ping and looong timeouts

[raven]

On Tue, 8 Jun 2004, Michael Blandford wrote:

> Jeff Moyer wrote:
> 
> >mlblandf> Would this patch cause problems for those of us who have icmp
> >mlblandf> blocked?  If so, could we make it a run time option to
> >mlblandf> enable/disable?
> >
> >Wow, you block icmp internally?  This would incur a timeout for every host
> >listed in your replicated server entry, and with Ian's patch, it would
> >incur a timeout for non-replicated servers, as well.  In fact, for the
> >non-replicated server case, I think it would give a false negative, failing
> >the mount even though the server is up.
> >
> >Is this really how you have things configured?  NFS clients can't ping
> >their servers?
> >  
> >
> 
> In a large environment it wouldn't be uncommon to have NFS mounts that 
> span across a WAN.  Using NFS over TCP seems to make the most sense in 
> that situation.
> 
> As packets traverse the WAN, there may be routers that block icmp.  This 
> is the type of situation where replicated server would make the most 
> sense - find the fastest server.
>

I was thinking that we could make disabling ICMP ping a configure option.
What do you think?

Still, I can't see why an internal WAN would block ICMP. It's needed for 
proper operation of your net.

If your talking about NFS over the public network you really should be 
using a VPN. In which case you don't need to (and probably shouldn't) 
block ICMP.

Ian

Re: [RFC] rpc_ping and looong timeouts

[Jeff Moyer]

==> Regarding Re: [autofs] [RFC] rpc_ping and looong timeouts; raven@themaw.net adds:

raven> On Tue, 8 Jun 2004, Michael Blandford wrote:
>> Jeff Moyer wrote:
>> 
>> >mlblandf> Would this patch cause problems for those of us who have icmp
>> >mlblandf> blocked?  If so, could we make it a run time option to
>> >mlblandf> enable/disable?
>> >
>> >Wow, you block icmp internally?  This would incur a timeout for every
>> host >listed in your replicated server entry, and with Ian's patch, it
>> would >incur a timeout for non-replicated servers, as well.  In fact,
>> for the >non-replicated server case, I think it would give a false
>> negative, failing >the mount even though the server is up.
>> >
>> >Is this really how you have things configured?  NFS clients can't ping
>> >their servers?
>> >  
>> >
>> 
>> In a large environment it wouldn't be uncommon to have NFS mounts that
>> span across a WAN.  Using NFS over TCP seems to make the most sense in
>> that situation.
>> 
>> As packets traverse the WAN, there may be routers that block icmp.  This
>> is the type of situation where replicated server would make the most
>> sense - find the fastest server.
>> 

raven> I was thinking that we could make disabling ICMP ping a configure
raven> option.  What do you think?

Well, if it's not hard to do...

raven> Still, I can't see why an internal WAN would block ICMP. It's needed
raven> for proper operation of your net.

I don't think that's for us to dictate.  ;)  There's no reason not to be
flexible on this issue.

-Jeff

Re: [RFC] rpc_ping and looong timeouts

[raven]

On Tue, 8 Jun 2004, Jeff Moyer wrote:

> ==> Regarding Re: [autofs] [RFC] rpc_ping and looong timeouts; raven@themaw.net adds:
> 
> raven> On Tue, 8 Jun 2004, Michael Blandford wrote:
> >> Jeff Moyer wrote:
> >> 
> >> >mlblandf> Would this patch cause problems for those of us who have icmp
> >> >mlblandf> blocked?  If so, could we make it a run time option to
> >> >mlblandf> enable/disable?
> >> >
> >> >Wow, you block icmp internally?  This would incur a timeout for every
> >> host >listed in your replicated server entry, and with Ian's patch, it
> >> would >incur a timeout for non-replicated servers, as well.  In fact,
> >> for the >non-replicated server case, I think it would give a false
> >> negative, failing >the mount even though the server is up.
> >> >
> >> >Is this really how you have things configured?  NFS clients can't ping
> >> >their servers?
> >> >  
> >> >
> >> 
> >> In a large environment it wouldn't be uncommon to have NFS mounts that
> >> span across a WAN.  Using NFS over TCP seems to make the most sense in
> >> that situation.
> >> 
> >> As packets traverse the WAN, there may be routers that block icmp.  This
> >> is the type of situation where replicated server would make the most
> >> sense - find the fastest server.
> >> 
> 
> raven> I was thinking that we could make disabling ICMP ping a configure
> raven> option.  What do you think?
> 
> Well, if it's not hard to do...

Should be fairly straight forward.
The code sections are small and easily identified.

Of course it would be neccessary to make an SRPM from the tarball, install 
it and edit the configure statement, then rebuild.

> 
> raven> Still, I can't see why an internal WAN would block ICMP. It's needed
> raven> for proper operation of your net.
> 
> I don't think that's for us to dictate.  ;)  There's no reason not to be
> flexible on this issue.
> 

Right you are Jeff.
My bad.

Ian

Re: [RFC] rpc_ping and looong timeouts

[Jeff Moyer]

==> Regarding Re: [autofs] [RFC] rpc_ping and looong timeouts; raven@themaw.net adds:

raven> On Tue, 8 Jun 2004, Jeff Moyer wrote:
>> ==> Regarding Re: [autofs] [RFC] rpc_ping and looong timeouts;
>> raven@themaw.net adds:
>> 
raven> On Tue, 8 Jun 2004, Michael Blandford wrote:
>> >> Jeff Moyer wrote:
>> >> 
>> >> >mlblandf> Would this patch cause problems for those of us who have
>> icmp >> >mlblandf> blocked?  If so, could we make it a run time option
>> to >> >mlblandf> enable/disable?
>> >> >
>> >> >Wow, you block icmp internally?  This would incur a timeout for
>> every >> host >listed in your replicated server entry, and with Ian's
>> patch, it >> would >incur a timeout for non-replicated servers, as well.
>> In fact, >> for the >non-replicated server case, I think it would give a
>> false >> negative, failing >the mount even though the server is up.
>> >> >
>> >> >Is this really how you have things configured?  NFS clients can't
>> ping >> >their servers?
>> >> >  
>> >> >
>> >> 
>> >> In a large environment it wouldn't be uncommon to have NFS mounts
>> that >> span across a WAN.  Using NFS over TCP seems to make the most
>> sense in >> that situation.
>> >> 
>> >> As packets traverse the WAN, there may be routers that block icmp.
>> This >> is the type of situation where replicated server would make the
>> most >> sense - find the fastest server.
>> >> 
>> 
raven> I was thinking that we could make disabling ICMP ping a configure
raven> option.  What do you think?
>> Well, if it's not hard to do...

raven> Should be fairly straight forward.  The code sections are small and
raven> easily identified.

raven> Of course it would be neccessary to make an SRPM from the tarball,
raven> install it and edit the configure statement, then rebuild.

Wow, I misread that initially.  I was thinking of a run-time configuration.
I'm not sure I like the idea of a configure option, as it just makes
support that much more difficult.

I guess we could come full circle on this issue, and try to solve it at the
rpc layer.

There are 2 issues here.  First, for UDP, the portmapper lookup has a
static timeout of 60 seconds, and that is not tunable (afaict).  For TCP,
it takes a long time to timout a connect call.  I've already written code
to address the TCP connect issue.

As it happens, the UDP issue is not insurmountable.  We can simply write
code which queries the portmapper ourselves.  Here is the excerpt from
pmap_getport, edited to show only the UDP stuff:

static const struct timeval tottimeout =  {60, 0};

u_short
pmap_getport (address, program, version, protocol)
     struct sockaddr_in *address;
     u_long program;
     u_long version;
     u_int protocol;
{
  address->sin_port = htons (PMAPPORT);
  client = INTUSE(clntudp_bufcreate) (address, PMAPPROG, PMAPVERS, timeout,
					&socket, RPCSMALLMSGSIZE,
					RPCSMALLMSGSIZE);
  if (client != (CLIENT *) NULL)
    {
      struct rpc_createerr *ce = &get_rpc_createerr ();
      parms.pm_prog = program;
      parms.pm_vers = version;
      parms.pm_prot = protocol;
      parms.pm_port = 0;	/* not needed or used */
      if (CLNT_CALL (client, PMAPPROC_GETPORT, (xdrproc_t)INTUSE(xdr_pmap),
		     (caddr_t)&parms, (xdrproc_t)INTUSE(xdr_u_short),
		     (caddr_t)&port, tottimeout) != RPC_SUCCESS)

clntudp_bufcreate is simply clnt_create.  In this case, we fill in
address->sin_port, so the clnt_create call will not do a pmap_lookup.

Next we do a clnt_call, passing in a timeout.  Note the 60 second timeout
above.  There, you're done.  So, if we add the TCP case back into the code,
we do the nonblocking connect, and then do that clnt_create with this
socket.

Ian, I think this is do-able, and not entirely disgusting.  What's more is
that we can get rid of the ping.c file.  ;)  I was never really happy with
using a separate protocol anyway.  The more I think about this, the more I
like it.

Comments?  If you like it, I'll set aside some time to implement it.

-Jeff

Re: [RFC] rpc_ping and looong timeouts

[Michael Blandford]

Jeff Moyer wrote:

>I guess we could come full circle on this issue, and try to solve it at the
>rpc layer.
>
I like this solution.  It removes the dependency on icmp.

Michael

Disclaimer: The content of this message is my personal opinion only and 
although I am an employee of Intel, the statements I make here in no way 
represent Intel's position on the issue, nor am I authorized to speak on 
behalf of Intel on this matter.

Re: [RFC] rpc_ping and looong timeouts

[Ian Kent]

On Tue, 8 Jun 2004, Jeff Moyer wrote:

> 
> I guess we could come full circle on this issue, and try to solve it at the
> rpc layer.
> 
> There are 2 issues here.  First, for UDP, the portmapper lookup has a
> static timeout of 60 seconds, and that is not tunable (afaict).  For TCP,
> it takes a long time to timout a connect call.  I've already written code
> to address the TCP connect issue.
> 
> As it happens, the UDP issue is not insurmountable.  We can simply write
> code which queries the portmapper ourselves.  Here is the excerpt from
> pmap_getport, edited to show only the UDP stuff:
> 
> static const struct timeval tottimeout =  {60, 0};
> 
> u_short
> pmap_getport (address, program, version, protocol)
>      struct sockaddr_in *address;
>      u_long program;
>      u_long version;
>      u_int protocol;
> {
>   address->sin_port = htons (PMAPPORT);
>   client = INTUSE(clntudp_bufcreate) (address, PMAPPROG, PMAPVERS, timeout,
> 					&socket, RPCSMALLMSGSIZE,
> 					RPCSMALLMSGSIZE);
>   if (client != (CLIENT *) NULL)
>     {
>       struct rpc_createerr *ce = &get_rpc_createerr ();
>       parms.pm_prog = program;
>       parms.pm_vers = version;
>       parms.pm_prot = protocol;
>       parms.pm_port = 0;	/* not needed or used */
>       if (CLNT_CALL (client, PMAPPROC_GETPORT, (xdrproc_t)INTUSE(xdr_pmap),
> 		     (caddr_t)&parms, (xdrproc_t)INTUSE(xdr_u_short),
> 		     (caddr_t)&port, tottimeout) != RPC_SUCCESS)
> 
> clntudp_bufcreate is simply clnt_create.  In this case, we fill in
> address->sin_port, so the clnt_create call will not do a pmap_lookup.
> 
> Next we do a clnt_call, passing in a timeout.  Note the 60 second timeout
> above.  There, you're done.  So, if we add the TCP case back into the code,
> we do the nonblocking connect, and then do that clnt_create with this
> socket.
> 
> Ian, I think this is do-able, and not entirely disgusting.  What's more is
> that we can get rid of the ping.c file.  ;)  I was never really happy with
> using a separate protocol anyway.  The more I think about this, the more I
> like it.

I'm in.

Doing everything using the method as we are going to use for NFS is 
definutely the way to go if we can.

What have we got to work with in terms of patches/code examples (besides 
above). What would you like/have time to do and what can I do. We don't 
want to duplicate efforts here.

> 
> Comments?  If you like it, I'll set aside some time to implement it.

Jeff's hogging all the fun.

Seriously though, whatever you do is a be a big help.
Thanks Jeff.

Ian

Re: [RFC] rpc_ping and looong timeouts

[Jeff Moyer]

==> Regarding Re: [autofs] [RFC] rpc_ping and looong timeouts; Ian Kent <raven@themaw.net> adds:

raven> On Tue, 8 Jun 2004, Jeff Moyer wrote:
>> I guess we could come full circle on this issue, and try to solve it at
>> the rpc layer.
>> 
>> There are 2 issues here.  First, for UDP, the portmapper lookup has a
>> static timeout of 60 seconds, and that is not tunable (afaict).  For
>> TCP, it takes a long time to timout a connect call.  I've already
>> written code to address the TCP connect issue.
>> 
>> As it happens, the UDP issue is not insurmountable.  We can simply write
>> code which queries the portmapper ourselves.  Here is the excerpt from
>> pmap_getport, edited to show only the UDP stuff:
>> 
[snip]
>> clntudp_bufcreate is simply clnt_create.  In this case, we fill in
address-> sin_port, so the clnt_create call will not do a pmap_lookup.
>> Next we do a clnt_call, passing in a timeout.  Note the 60 second
>> timeout above.  There, you're done.  So, if we add the TCP case back
>> into the code, we do the nonblocking connect, and then do that
>> clnt_create with this socket.
>> 
>> Ian, I think this is do-able, and not entirely disgusting.  What's more
>> is that we can get rid of the ping.c file.  ;) I was never really happy
>> with using a separate protocol anyway.  The more I think about this, the
>> more I like it.

raven> I'm in.

raven> Doing everything using the method as we are going to use for NFS is
raven> definutely the way to go if we can.

raven> What have we got to work with in terms of patches/code examples
raven> (besides above). What would you like/have time to do and what can I
raven> do. We don't want to duplicate efforts here.

Okay, I sent you some mail with a non-blocking connect example (not posted
here b/c of immaturity of code).  Apart from that, I have nothing but what
I've written above.

>> Comments?  If you like it, I'll set aside some time to implement it.

raven> Jeff's hogging all the fun.

raven> Seriously though, whatever you do is a be a big help.  Thanks Jeff.

Heh.  I was merely trying to keep your load down to a sustainable volume.
;)  I'm more than happy to review your patches!

So, if you're totally into doing this, just let me know.  I'm definitely
quite busy enough with other things to share the fun!

-Jeff

Re: [RFC] rpc_ping and looong timeouts

[Ian Kent]

On Tue, 8 Jun 2004, Jeff Moyer wrote:

> ==> Regarding Re: [autofs] [RFC] rpc_ping and looong timeouts; Ian Kent <raven@themaw.net> adds:
> 
> raven> On Tue, 8 Jun 2004, Jeff Moyer wrote:
> >> I guess we could come full circle on this issue, and try to solve it at
> >> the rpc layer.
> >> 
> >> There are 2 issues here.  First, for UDP, the portmapper lookup has a
> >> static timeout of 60 seconds, and that is not tunable (afaict).  For
> >> TCP, it takes a long time to timout a connect call.  I've already
> >> written code to address the TCP connect issue.
> >> 
> >> As it happens, the UDP issue is not insurmountable.  We can simply write
> >> code which queries the portmapper ourselves.  Here is the excerpt from
> >> pmap_getport, edited to show only the UDP stuff:
> >> 
> [snip]
> >> clntudp_bufcreate is simply clnt_create.  In this case, we fill in
> address-> sin_port, so the clnt_create call will not do a pmap_lookup.
> >> Next we do a clnt_call, passing in a timeout.  Note the 60 second
> >> timeout above.  There, you're done.  So, if we add the TCP case back
> >> into the code, we do the nonblocking connect, and then do that
> >> clnt_create with this socket.
> >> 
> >> Ian, I think this is do-able, and not entirely disgusting.  What's more
> >> is that we can get rid of the ping.c file.  ;) I was never really happy
> >> with using a separate protocol anyway.  The more I think about this, the
> >> more I like it.
> 
> raven> I'm in.
> 
> raven> Doing everything using the method as we are going to use for NFS is
> raven> definutely the way to go if we can.
> 
> raven> What have we got to work with in terms of patches/code examples
> raven> (besides above). What would you like/have time to do and what can I
> raven> do. We don't want to duplicate efforts here.
> 
> Okay, I sent you some mail with a non-blocking connect example (not posted
> here b/c of immaturity of code).  Apart from that, I have nothing but what
> I've written above.
> 
> >> Comments?  If you like it, I'll set aside some time to implement it.
> 
> raven> Jeff's hogging all the fun.
> 
> raven> Seriously though, whatever you do is a be a big help.  Thanks Jeff.
> 
> Heh.  I was merely trying to keep your load down to a sustainable volume.
> ;)  I'm more than happy to review your patches!
> 
> So, if you're totally into doing this, just let me know.  I'm definitely
> quite busy enough with other things to share the fun!

I'd like to do it but not without patch contributions. They are really 
usefull in cutting down development time.

I read the sunrpc code in its entirity a while ago but still haven't got 
enough to work well with it.

So delving into it to solve an actual problem is probably the best teacher 
I could get.

Ian