[Lcdproc] Calling NetBSD and Darwin developers: Iface fixes

Markus Dolze bsdfan@nurfuerspam.de
Sun Sep 2 21:50:02 2007


This is a multi-part message in MIME format.
--------------070605010604040704040302
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Hello,

while working on the FreeBSD lcdproc client network code, I found a few
small errors, which I also fixed for the other two BSD style
implementations:

1. If an interface priviously been UP goes DOWN, the time when the
interface changed would always show the current time.
2. If an active interface is removed from the system, the server would
still show the old screens and the user will not get any notice.
3. The "first run" detection would not work if more than one interface
is monitored. Now the "last_online" time is used, because this is an
"per interface" value initialized to 0 at startup.
4. The TRUE/FALSE macros are used for the return values as in the other
functions.

I'm asking myself whether the change 3 is really usefull. Without the
change the up / down speed would be shown incorrectly for the first two
seconds. Anyway, we do currently not detect counter wraps and resets
where this would happen, too.

Please test the changes made above!

Regards
Markus

--------------070605010604040704040302
Content-Type: text/plain;
 name="machine_iface.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="machine_iface.diff"

Index: clients/lcdproc/machine_Darwin.c
===================================================================
RCS file: /cvsroot/lcdproc/lcdproc/clients/lcdproc/machine_Darwin.c,v
retrieving revision 1.11
diff -u -r1.11 machine_Darwin.c
--- clients/lcdproc/machine_Darwin.c	7 Apr 2007 17:37:54 -0000	1.11
+++ clients/lcdproc/machine_Darwin.c	2 Sep 2007 21:36:04 -0000
@@ -479,7 +479,6 @@
 /* Get network statistics */
 int machine_get_iface_stats (IfaceInfo *interface)
 {
-	static int      first_time = 1;	/* is it first time we call this function? */
 	int             rows;
 	int             name[6] = {CTL_NET, PF_LINK, NETLINK_GENERIC, IFMIB_SYSTEM, IFMIB_IFCOUNT};
 	size_t          len;
@@ -487,7 +486,6 @@
 	
 	len = sizeof(rows);
 	/* get number of interfaces */
-	//if (sysctlbyname("net.link.generic.system.ifcount", &rows, &len, NULL, 0) == 0) {
 	if (sysctl(name, 5, &rows, &len, 0, 0) == 0) {
 		interface->status = down; /* set status down by default */
 		
@@ -506,31 +504,32 @@
 			}
 			/* check if its interface name matches */
 			if (strcmp(ifmd.ifmd_name, interface->name) == 0) {
-				interface->last_online = time(NULL);	/* save actual time */
-				
-				if ((ifmd.ifmd_flags & IFF_UP) == IFF_UP)
-					interface->status = up;	/* is up */
 
 				interface->rc_byte = ifmd.ifmd_data.ifi_ibytes;
 				interface->tr_byte = ifmd.ifmd_data.ifi_obytes;
 				interface->rc_pkt = ifmd.ifmd_data.ifi_ipackets;
 				interface->tr_pkt = ifmd.ifmd_data.ifi_opackets;
 
-				if (first_time) {
+				if (interface->last_online == 0) {
 					interface->rc_byte_old = interface->rc_byte;
 					interface->tr_byte_old = interface->tr_byte;
 					interface->rc_pkt_old = interface->rc_pkt;
 					interface->tr_pkt_old = interface->tr_pkt;
-					first_time = 0;	/* now it isn't first time */
 				}
-				return 1;
+
+				if ((ifmd.ifmd_flags & IFF_UP) == IFF_UP) {
+					interface->status = up;			/* is up */
+					interface->last_online = time(NULL);	/* save actual time */
+				}
+
+				return (TRUE);
 			}
 		}
 		/* if we are here there is no interface with the given name */
-		return 0;
+		return (TRUE);
 	} else {
 		perror("read sysctl IFMIB_IFCOUNT");
-		return 0;
+		return (FALSE);
 	}
 } /* get_iface_stats() */
 
Index: clients/lcdproc/machine_FreeBSD.c
===================================================================
RCS file: /cvsroot/lcdproc/lcdproc/clients/lcdproc/machine_FreeBSD.c,v
retrieving revision 1.12
diff -u -r1.12 machine_FreeBSD.c
--- clients/lcdproc/machine_FreeBSD.c	26 Aug 2007 21:07:42 -0000	1.12
+++ clients/lcdproc/machine_FreeBSD.c	2 Sep 2007 21:36:05 -0000
@@ -439,7 +439,6 @@
  */
 int machine_get_iface_stats (IfaceInfo *interface)
 {
-	static int      first_time = 1;	/* is it first time we call this function? */
 	int             rows;
 	int             name[6] = {CTL_NET, PF_LINK, NETLINK_GENERIC, IFMIB_IFDATA, 0, IFDATA_GENERAL};
 	size_t          len;
@@ -461,31 +460,32 @@
 			}
 			/* check if its interface name matches */
 			if (strcmp(ifmd.ifmd_name, interface->name) == 0) {
-				interface->last_online = time(NULL);	/* save actual time */
-
-				if ((ifmd.ifmd_flags & IFF_UP) == IFF_UP)
-					interface->status = up;	/* is up */
 
 				interface->rc_byte = ifmd.ifmd_data.ifi_ibytes;
 				interface->tr_byte = ifmd.ifmd_data.ifi_obytes;
 				interface->rc_pkt = ifmd.ifmd_data.ifi_ipackets;
 				interface->tr_pkt = ifmd.ifmd_data.ifi_opackets;
 
-				if (first_time) {
+				if (interface->last_online == 0) {
 					interface->rc_byte_old = interface->rc_byte;
 					interface->tr_byte_old = interface->tr_byte;
 					interface->rc_pkt_old = interface->rc_pkt;
 					interface->tr_pkt_old = interface->tr_pkt;
-					first_time = 0;	/* now it isn't first time */
 				}
-				return 1;
+
+				if ((ifmd.ifmd_flags & IFF_UP) == IFF_UP) {
+					interface->status = up;			/* is up */
+					interface->last_online = time(NULL);	/* save actual time */
+				}
+
+				return (TRUE);
 			}
 		}
 		/* if we are here there is no interface with the given name */
-		return 0;
+		return (TRUE);
 	} else {
 		perror("read sysctlbyname");
-		return 0;
+		return (FALSE);
 	}
 } /* get_iface_stats() */
 
Index: clients/lcdproc/machine_NetBSD.c
===================================================================
RCS file: /cvsroot/lcdproc/lcdproc/clients/lcdproc/machine_NetBSD.c,v
retrieving revision 1.10
diff -u -r1.10 machine_NetBSD.c
--- clients/lcdproc/machine_NetBSD.c	7 Apr 2007 17:37:54 -0000	1.10
+++ clients/lcdproc/machine_NetBSD.c	2 Sep 2007 21:36:05 -0000
@@ -380,7 +380,6 @@
 /* Get network statistics */
 int machine_get_iface_stats (IfaceInfo *interface)
 {
-	static int     first_time = 1;	/* is it first time we call this function? */
 	struct ifaddrs *ifa, *ifa_ptr;
 	struct if_data *ifd;
 
@@ -397,30 +396,31 @@
 			(ifa_ptr->ifa_addr->sa_family == AF_LINK)) {
 
 			ifd = (struct if_data *)ifa_ptr->ifa_data;
-			interface->last_online = time(NULL);	/* save actual time */
-
-			if ((ifa_ptr->ifa_flags & IFF_UP) == IFF_UP)
-				interface->status = up;	/* is up */
 
 			interface->rc_byte = ifd->ifi_ibytes;
 			interface->tr_byte = ifd->ifi_obytes;
 			interface->rc_pkt = ifd->ifi_ipackets;
 			interface->tr_pkt = ifd->ifi_opackets;
 
-			if (first_time) {
+			if (interface->last_online == 0) {
 				interface->rc_byte_old = interface->rc_byte;
 				interface->tr_byte_old = interface->tr_byte;
 				interface->rc_pkt_old = interface->rc_pkt;
 				interface->tr_pkt_old = interface->tr_pkt;
-				first_time = 0;	/* now it isn't first time */
 			}
-			return 1;
+
+			if ((ifa_ptr->ifa_flags & IFF_UP) == IFF_UP) {
+				interface->status = up;			/* is up */
+				interface->last_online = time(NULL);	/* save actual time */
+			}
+
+			return (TRUE);
 		}
 	}
 	freeifaddrs(ifa);
 
 	/* if we are here there is no interface with the given name */
-	return 0;
+	return (TRUE);
 }
 
 #endif /* __NetBSD__ */

--------------070605010604040704040302--