From: Vincent Bernat <vincent@bernat.im>
Date: Sat, 6 Dec 2014 23:05:53 +0000 (+0100)
Subject: lldpd: don't rely on a checksum to detect a port change
X-Git-Tag: 0.7.13~4
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=17d341157926e8cc2287d3587f4836fb1b28d6a4;p=thirdparty%2Flldpd.git

lldpd: don't rely on a checksum to detect a port change

Instead, just compare the serialized copies. This is more reliable than
a checksum and also (a bit) faster.
---

diff --git a/src/daemon/lldpd.c b/src/daemon/lldpd.c
index 437efc2b..c4b923df 100644
--- a/src/daemon/lldpd.c
+++ b/src/daemon/lldpd.c
@@ -207,6 +207,7 @@ lldpd_hardware_cleanup(struct lldpd *cfg, struct lldpd_hardware *hardware)
 {
 	log_debug("alloc", "cleanup hardware port %s", hardware->h_ifname);
 
+	free(hardware->h_lport_previous);
 	lldpd_port_cleanup(&hardware->h_lport, 1);
 	if (hardware->h_ops && hardware->h_ops->cleanup)
 		hardware->h_ops->cleanup(cfg, hardware);
@@ -292,11 +293,10 @@ lldpd_reset_timer(struct lldpd *cfg)
 	/* Reset timer for ports that have been changed. */
 	struct lldpd_hardware *hardware;
 	TAILQ_FOREACH(hardware, &cfg->g_hardware, h_entries) {
-		/* We need to compute a checksum of the local port. To do this,
-		 * we zero out fields that are not significant, marshal the
-		 * port, compute the checksum, then restore. */
+		/* We keep a flat copy of the local port to see if there is any
+		 * change. To do this, we zero out fields that are not
+		 * significant, marshal the port, then restore. */
 		struct lldpd_port *port = &hardware->h_lport;
-		u_int32_t cksum;
 		u_int8_t *output = NULL;
 		ssize_t output_len;
 		char save[LLDPD_PORT_START_MARKER];
@@ -312,24 +312,25 @@ lldpd_reset_timer(struct lldpd *cfg)
 			    hardware->h_ifname);
 			continue;
 		}
-		/* Port change is detected by computing a checksum. 0 means the
-		 * checksum never was computed (new interface). */
-		cksum  = frame_checksum(output, output_len/2, 0) << 16;
-		cksum += frame_checksum(output + output_len/2,
-		    output_len - output_len/2, 0);
-		cksum  = cksum?cksum:1;
-		free(output);
-		if (cksum != hardware->h_lport_cksum) {
+
+		/* Compare with the previous value */
+		if (hardware->h_lport_previous &&
+		    output_len == hardware->h_lport_previous_len &&
+		    !memcmp(output, hardware->h_lport_previous, output_len)) {
 			log_debug("localchassis",
-			    "change detected for port %s, resetting its timer",
+			    "no change detected for port %s",
 			    hardware->h_ifname);
-			hardware->h_lport_cksum = cksum;
-			levent_schedule_pdu(hardware);
 		} else {
 			log_debug("localchassis",
-			    "no change detected for port %s",
+			    "change detected for port %s, resetting its timer",
 			    hardware->h_ifname);
+			levent_schedule_pdu(hardware);
 		}
+
+		/* Update the value */
+		free(hardware->h_lport_previous);
+		hardware->h_lport_previous = output;
+		hardware->h_lport_previous_len = output_len;
 	}
 }
 
diff --git a/src/lldpd-structs.h b/src/lldpd-structs.h
index e9727681..b0828d2f 100644
--- a/src/lldpd-structs.h
+++ b/src/lldpd-structs.h
@@ -398,7 +398,8 @@ struct lldpd_hardware {
 	u_int64_t		 h_delete_cnt;
 	u_int64_t		 h_drop_cnt;
 
-	u_int32_t		 h_lport_cksum; /* Checksum on local port to see if there is a change */
+	void			*h_lport_previous; /* Backup of last value for localport */
+	ssize_t			 h_lport_previous_len;
 	struct lldpd_port	 h_lport;  /* Port attached to this hardware port */
 	TAILQ_HEAD(, lldpd_port) h_rports; /* Remote ports */