[PATCH] closing the loop -- retries and acks

[Chris Friesen <chris_friesen@sympatico.ca>]

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

This patch reduces the already low chances of lost messages.

Once udevsend successfully sends a message, it waits with a timeout for
a response.  If the response is good, then all is well, otherwise it
sends the message again.

In udevd, when we receive a request we store the address of the
requestor, and then once the request has been handled we then send an
ack back so that they know they have been taken care of.

This patch takes care of the basics, but there are still a few things left:
1) tuning timeouts and retry numbers
2) capabilities check on the ack
3) pull out the udevd-specific stuff from struct hotplug_msg so that we
don't waste time/memory.  udevd will have a different struct, one
element of which is hotplug_msg.  udevsend still only knows about
hotplug_msg
4) pull the messaging stuff into a set of headers or something to get
rid of code duplication

Comments?

Chris

[-- Attachment #2: udevack.patch --]
[-- Type: text/plain, Size: 4424 bytes --]

diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet udev/udevd.c myudev/udevd.c
--- udev/udevd.c	Wed Feb 18 00:50:51 2004
+++ myudev/udevd.c	Wed Feb 18 01:16:00 2004
@@ -232,6 +232,7 @@
 	struct iovec iov;
 	struct ucred *cred;
 	char cred_msg[CMSG_SPACE(sizeof(struct ucred))];
+	struct sockaddr_un addr;
 
 	msg = msg_create();
 	if (msg == NULL) {
@@ -247,13 +248,21 @@
 	smsg.msg_iovlen = 1;
 	smsg.msg_control = cred_msg;
 	smsg.msg_controllen = sizeof(cred_msg);
+	smsg.msg_name = &addr;
+	smsg.msg_namelen = sizeof(addr);
 
 	retval = recvmsg(sock, &smsg, 0);
 	if (retval <  0) {
 		if (errno != EINTR)
 			dbg("unable to receive message");
-		return;
+		goto skip;
 	}
+	
+	if (retval != sizeof(struct hotplug_msg)) {
+		dbg("wrong message size");
+		goto skip;
+	}
+	
 	cmsg = CMSG_FIRSTHDR(&smsg);
 	cred = (struct ucred *) CMSG_DATA(cmsg);
 
@@ -271,7 +280,10 @@
 		dbg("message magic '%s' doesn't match, ignore it", msg->magic);
 		goto skip;
 	}
-
+	
+	memcpy(&msg->addr, smsg.msg_name, smsg.msg_namelen);
+	msg->addrlen = smsg.msg_namelen;
+	
 	/* if no seqnum is given, we move straight to exec queue */
 	if (msg->seqnum == -1) {
 		list_add(&msg->list, &exec_list);
@@ -304,7 +316,7 @@
 	}
 }
 
-static void udev_done(int pid)
+static void udev_done(int pid, int sock)
 {
 	/* find msg associated with pid and delete it */
 	struct hotplug_msg *msg;
@@ -312,6 +324,10 @@
 	list_for_each_entry(msg, &running_list, list) {
 		if (msg->pid == pid) {
 			dbg("<== exec seq %d came back", msg->seqnum);
+			
+			/* send ack back to udevsend */
+			sendto(sock, msg, sizeof(struct hotplug_msg), 0, (struct sockaddr*) &msg->addr, msg->addrlen);
+			
 			run_queue_delete(msg);
 			return;
 		}
@@ -384,7 +400,7 @@
 				int pid = waitpid(-1, 0, WNOHANG);
 				if ((pid == -1) || (pid == 0))
 					break;
-				udev_done(pid);
+				udev_done(pid, ssock);
 			}
 		}
 	}
diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet udev/udevd.h myudev/udevd.h
--- udev/udevd.h	Thu Feb  5 23:44:52 2004
+++ myudev/udevd.h	Wed Feb 18 01:16:50 2004
@@ -28,13 +28,16 @@
 #define EVENT_TIMEOUT_SEC		5
 #define UDEVSEND_CONNECT_RETRY		20 /* x 100 millisec */
 #define UDEVD_SOCK_PATH			"udevd"
+#define ACK_TIMEOUT_SEC (EVENT_TIMEOUT_SEC + 1)
 
 struct hotplug_msg {
-	char magic[20];
 	struct list_head list;
 	pid_t pid;
-	int seqnum;
 	time_t queue_time;
+	struct sockaddr_un addr;
+	socklen_t addrlen;
+	char magic[20];
+	int seqnum;
 	char action[8];
 	char devpath[128];
 	char subsystem[16];
diff -Nur --exclude=RCS --exclude=CVS --exclude=SCCS --exclude=BitKeeper --exclude=ChangeSet udev/udevsend.c myudev/udevsend.c
--- udev/udevsend.c	Wed Feb 18 00:50:51 2004
+++ myudev/udevsend.c	Wed Feb 18 00:58:02 2004
@@ -22,6 +22,7 @@
  *
  */
 
+#include <sys/select.h>
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/wait.h>
@@ -180,6 +181,14 @@
 
 	memset(&saddr, 0x00, sizeof(saddr));
 	saddr.sun_family = AF_LOCAL;
+	sprintf(&saddr.sun_path[1], "%d", getpid());
+	addrlen = offsetof(struct sockaddr_un, sun_path) + strlen(saddr.sun_path+1) + 1;
+	
+	if (bind(sock, (struct sockaddr*)&saddr, addrlen) < 0) {
+		dbg("error binding socket");
+		goto exit;
+	}
+		
 	/* use abstract namespace for socket path */
 	strcpy(&saddr.sun_path[1], UDEVD_SOCK_PATH);
 	addrlen = offsetof(struct sockaddr_un, sun_path) + strlen(saddr.sun_path+1) + 1;
@@ -188,11 +197,36 @@
 
 	/* If we can't send, try to start daemon and resend message */
 	loop = UDEVSEND_CONNECT_RETRY;
+send_msg:
 	while (loop--) {
 		retval = sendto(sock, &msg, size, 0, (struct sockaddr *)&saddr, addrlen);
 		if (retval != -1) {
-			retval = 0;
-			goto close_and_exit;
+			
+			/* wait for ack */
+			fd_set rdset;
+			int rc;
+			struct timeval tv = {ACK_TIMEOUT_SEC, 0};
+						
+			while(1) {
+				FD_ZERO(&rdset);
+				FD_SET(sock, &rdset);
+				rc = select(sock+1, &rdset, 0, 0, &tv);
+				
+				if (!rc)
+					goto send_msg;
+				
+				if (FD_ISSET(sock, &rdset)) {
+					struct hotplug_msg ack;
+					
+					retval = recv(sock, &ack, sizeof(ack), MSG_DONTWAIT);
+					if ((retval > 0) && 
+							(strncmp(ack.magic, msg.magic, sizeof(UDEV_MAGIC)) == 0 ) &&
+							(ack.seqnum == msg.seqnum)) {
+						retval = 0;
+						goto close_and_exit;
+					}
+				}
+			}
 		}
 		
 		if (errno != ECONNREFUSED) {
Binary files udev/udevtest and myudev/udevtest differ