vrrpd: cleanup vrrp packet crafting code

* Prefix all packet functions with 'vrrp_pkt'
* Break out checksum computation into separate function
* Accept version field when building advertisements
* Update doc comments

Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>

vrrpd/vrrp.c

@@ -424,18 +424,18 @@ static int vrrp_master_down_timer_expire(struct thread *thread);
 static void vrrp_send_advertisement(struct vrrp_router *r)
 {
 	struct vrrp_pkt *pkt;
-	ssize_t pktlen;
+	ssize_t pktsz;
 	struct ipaddr *addrs[r->addrs->count];
 	union sockunion dest;
 
 	list_to_array(r->addrs, (void **)addrs, r->addrs->count);
 
-	pktlen = vrrp_pkt_build(&pkt, &r->src, r->vr->vrid, r->priority,
+	pktsz = vrrp_pkt_adver_build(&pkt, &r->src, 3, r->vr->vrid, r->priority,
-				r->vr->advertisement_interval, r->addrs->count,
+				     r->vr->advertisement_interval,
-				(struct ipaddr **)&addrs);
+				     r->addrs->count, (struct ipaddr **)&addrs);
 
-	if (pktlen > 0)
+	if (pktsz > 0)
-		zlog_hexdump(pkt, (size_t) pktlen);
+		zlog_hexdump(pkt, (size_t) pktsz);
 	else
 		zlog_warn("Could not build VRRP packet");
 
@@ -443,7 +443,7 @@ static void vrrp_send_advertisement(struct vrrp_router *r)
 		r->family == AF_INET ? VRRP_MCASTV4_GROUP_STR : VRRP_MCASTV6_GROUP_STR;
 	str2sockunion(group, &dest);
 
-	ssize_t sent = sendto(r->sock_tx, pkt, (size_t)pktlen, 0, &dest.sa,
+	ssize_t sent = sendto(r->sock_tx, pkt, (size_t)pktsz, 0, &dest.sa,
 			      sockunion_sizeof(&dest));
 
 	XFREE(MTYPE_VRRP_PKT, pkt);
@@ -469,7 +469,7 @@ static int vrrp_recv_advertisement(struct vrrp_router *r, struct vrrp_pkt *pkt,
 				    size_t pktsize)
 {
 	char dumpbuf[BUFSIZ];
-	vrrp_pkt_dump(dumpbuf, sizeof(dumpbuf), pkt);
+	vrrp_pkt_adver_dump(dumpbuf, sizeof(dumpbuf), pkt);
 	zlog_debug("Received VRRP Advertisement:\n%s", dumpbuf);
 
 	/* Check that VRID matches our configured VRID */
@@ -595,8 +595,8 @@ static int vrrp_read(struct thread *thread)
 		   r->vr->vrid, family2str(r->family));
 	zlog_hexdump(r->ibuf, nbytes);
 
-	pktsize = vrrp_parse_datagram(r->family, &m, nbytes, &pkt, errbuf,
+	pktsize = vrrp_pkt_parse_datagram(r->family, &m, nbytes, &pkt, errbuf,
-				      sizeof(errbuf));
+					  sizeof(errbuf));
 
 	if (pktsize < 0) {
 		zlog_warn(VRRP_LOGPFX VRRP_LOGPFX_VRID

vrrpd/vrrp_packet.c

@@ -51,17 +51,67 @@ const char *vrrp_packet_names[16] = {
 };
 /* clang-format on */
 
-ssize_t vrrp_pkt_build(struct vrrp_pkt **pkt, struct ipaddr *src, uint8_t vrid,
+/*
-		       uint8_t prio, uint16_t max_adver_int, uint8_t numip,
+ * Compute the VRRP checksum.
-		       struct ipaddr **ips)
+ *
+ * Checksum is not set in the packet, just computed.
+ *
+ * pkt
+ *    VRRP packet, fully filled out except for checksum field.
+ *
+ * pktsize
+ *    sizeof(*pkt)
+ *
+ * src
+ *    IP address that pkt will be transmitted from.
+ *
+ * Returns:
+ *    VRRP checksum in network byte order.
+ */
+static uint16_t vrrp_pkt_checksum(struct vrrp_pkt *pkt, size_t pktsize,
+				  struct ipaddr *src)
 {
+	uint16_t chksum;
+	bool v6 = (src->ipa_type == IPADDR_V6);
+
+	uint16_t chksum_pre = pkt->hdr.chksum;
+	pkt->hdr.chksum = 0;
+
+	if (v6) {
+		struct ipv6_ph ph = {};
+		ph.src = src->ipaddr_v6;
+		inet_pton(AF_INET6, VRRP_MCASTV6_GROUP_STR, &ph.dst);
+		ph.ulpl = htons(pktsize);
+		ph.next_hdr = 112;
+		chksum = in_cksum_with_ph6(&ph, pkt, pktsize);
+	} else {
+		struct ipv4_ph ph = {};
+		ph.src = src->ipaddr_v4;
+		inet_pton(AF_INET, VRRP_MCASTV4_GROUP_STR, &ph.dst);
+		ph.proto = 112;
+		ph.len = htons(pktsize);
+		chksum = in_cksum_with_ph4(&ph, pkt, pktsize);
+	}
+
+	pkt->hdr.chksum = chksum_pre;
+
+	return chksum;
+}
+
+ssize_t vrrp_pkt_adver_build(struct vrrp_pkt **pkt, struct ipaddr *src,
+			     uint8_t version, uint8_t vrid, uint8_t prio,
+			     uint16_t max_adver_int, uint8_t numip,
+			     struct ipaddr **ips)
+{
+	assert(version >= 2 && version <= 3);
+
 	bool v6 = IS_IPADDR_V6(ips[0]);
 
 	size_t addrsz = v6 ? sizeof(struct in6_addr) : sizeof(struct in_addr);
 	size_t pktsize = VRRP_PKT_SIZE(v6 ? AF_INET6 : AF_INET, numip);
 	*pkt = XCALLOC(MTYPE_VRRP_PKT, pktsize);
 
-	(*pkt)->hdr.vertype |= VRRP_VERSION << 4;
+	(*pkt)->hdr.vertype |= version << 4;
 	(*pkt)->hdr.vertype |= VRRP_TYPE_ADVERTISEMENT;
 	(*pkt)->hdr.vrid = vrid;
 	(*pkt)->hdr.priority = prio;
@@ -75,28 +125,12 @@ ssize_t vrrp_pkt_build(struct vrrp_pkt **pkt, struct ipaddr *src, uint8_t vrid,
 		aptr += addrsz;
 	}
 
-	(*pkt)->hdr.chksum = 0;
+	(*pkt)->hdr.chksum = vrrp_pkt_checksum(*pkt, pktsize, src);
-
-	if (v6) {
-		struct ipv6_ph ph = {};
-		ph.src = src->ipaddr_v6;
-		inet_pton(AF_INET6, VRRP_MCASTV6_GROUP_STR, &ph.dst);
-		ph.ulpl = htons(pktsize);
-		ph.next_hdr = 112;
-		(*pkt)->hdr.chksum = in_cksum_with_ph6(&ph, *pkt, pktsize);
-	} else {
-		struct ipv4_ph ph = {};
-		ph.src = src->ipaddr_v4;
-		inet_pton(AF_INET, VRRP_MCASTV4_GROUP_STR, &ph.dst);
-		ph.proto = 112;
-		ph.len = htons(pktsize);
-		(*pkt)->hdr.chksum = in_cksum_with_ph4(&ph, *pkt, pktsize);
-	}
 
 	return pktsize;
 }
 
-size_t vrrp_pkt_dump(char *buf, size_t buflen, struct vrrp_pkt *pkt)
+size_t vrrp_pkt_adver_dump(char *buf, size_t buflen, struct vrrp_pkt *pkt)
 {
 	if (buflen < 1)
 		return 0;
@@ -127,9 +161,9 @@ size_t vrrp_pkt_dump(char *buf, size_t buflen, struct vrrp_pkt *pkt)
 	return rs;
 }
 
-ssize_t vrrp_parse_datagram(int family, struct msghdr *m, size_t read,
+ssize_t vrrp_pkt_parse_datagram(int family, struct msghdr *m, size_t read,
-			    struct vrrp_pkt **pkt, char *errmsg,
+				struct vrrp_pkt **pkt, char *errmsg,
-			    size_t errmsg_len)
+				size_t errmsg_len)
 {
 	/* Source (MAC & IP), Dest (MAC & IP) TTL validation done by kernel */
 	size_t addrsz = (family == AF_INET) ? sizeof(struct in_addr)

vrrpd/vrrp_packet.h

@@ -26,7 +26,6 @@
 #include "lib/memory.h"
 #include "lib/prefix.h"
 
-#define VRRP_VERSION 3
 #define VRRP_TYPE_ADVERTISEMENT 1
 
 extern const char *vrrp_packet_names[16];
@@ -94,11 +93,19 @@ struct vrrp_pkt {
 #define VRRP_MAX_PKT_SIZE VRRP_MAX_PKT_SIZE_V6
 
 /*
- * Builds a VRRP packet.
+ * Builds a VRRP ADVERTISEMENT packet.
  *
  * pkt
  *    Pointer to store pointer to result buffer in
  *
+ * src
+ *    Source address packet will be transmitted from. This is needed to compute
+ *    the VRRP checksum. The returned packet must be sent in an IP datagram with
+ *    the source address equal to this field, or the checksum will be invalid.
+ *
+ * version
+ *    VRRP version; must be 2 or 3
+ *
  * vrid
  *    Virtual Router Identifier
  *
@@ -118,12 +125,13 @@ struct vrrp_pkt {
  *    array of pointer to either struct in_addr (v6 = false) or struct in6_addr
  *    (v6 = true)
  */
-ssize_t vrrp_pkt_build(struct vrrp_pkt **pkt, struct ipaddr *src, uint8_t vrid,
+ssize_t vrrp_pkt_adver_build(struct vrrp_pkt **pkt, struct ipaddr *src,
-		       uint8_t prio, uint16_t max_adver_int, uint8_t numip,
+			     uint8_t version, uint8_t vrid, uint8_t prio,
-		       struct ipaddr **ips);
+			     uint16_t max_adver_int, uint8_t numip,
+			     struct ipaddr **ips);
 
 /*
- * Dumps a VRRP packet to a string.
+ * Dumps a VRRP ADVERTISEMENT packet to a string.
  *
  * Currently only dumps the header.
  *
@@ -139,7 +147,7 @@ ssize_t vrrp_pkt_build(struct vrrp_pkt **pkt, struct ipaddr *src, uint8_t vrid,
  * Returns:
  *    # bytes written to buf
  */
-size_t vrrp_pkt_dump(char *buf, size_t buflen, struct vrrp_pkt *pkt);
+size_t vrrp_pkt_adver_dump(char *buf, size_t buflen, struct vrrp_pkt *pkt);
 
 
 /*
@@ -170,8 +178,8 @@ size_t vrrp_pkt_dump(char *buf, size_t buflen, struct vrrp_pkt *pkt);
  * Returns:
  *    Size of VRRP packet, or -1 upon error
  */
-ssize_t vrrp_parse_datagram(int family, struct msghdr *m, size_t read,
+ssize_t vrrp_pkt_parse_datagram(int family, struct msghdr *m, size_t read,
-			    struct vrrp_pkt **pkt, char *errmsg,
+				struct vrrp_pkt **pkt, char *errmsg,
-			    size_t errmsg_len);
+				size_t errmsg_len);
 
 #endif /* __VRRP_PACKET_H__ */