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>

diff --git a/vrrpd/vrrp.c b/vrrpd/vrrp.c
index 37846d625301f327c0f84cd2d55c704ce0cc16ce..edac0f66cbbca9941d16a7afce975bb2f8e514e4 100644 (file)
--- a/vrrpd/vrrp.c
+++ b/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,
-                               r->vr->advertisement_interval, r->addrs->count,
-                               (struct ipaddr **)&addrs);
+       pktsz = vrrp_pkt_adver_build(&pkt, &r->src, 3, r->vr->vrid, r->priority,
+                                    r->vr->advertisement_interval,
+                                    r->addrs->count, (struct ipaddr **)&addrs);
 
-       if (pktlen > 0)
-               zlog_hexdump(pkt, (size_t) pktlen);
+       if (pktsz > 0)
+               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,
-                                     sizeof(errbuf));
+       pktsize = vrrp_pkt_parse_datagram(r->family, &m, nbytes, &pkt, errbuf,
+                                         sizeof(errbuf));
 
        if (pktsize < 0) {
                zlog_warn(VRRP_LOGPFX VRRP_LOGPFX_VRID

diff --git a/vrrpd/vrrp_packet.c b/vrrpd/vrrp_packet.c
index eca3af0437b6dd04072cdab80b711c943a7825cd..2d5129b285eae43a3a5d010c368709cea5dd1aaf 100644 (file)
--- a/vrrpd/vrrp_packet.c
+++ b/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,
-                      struct ipaddr **ips)
+/*
+ * Compute the VRRP checksum.
+ *
+ * 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;
-
-       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);
-       }
+       (*pkt)->hdr.chksum = vrrp_pkt_checksum(*pkt, pktsize, src);
 
        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,
-                           struct vrrp_pkt **pkt, char *errmsg,
-                           size_t errmsg_len)
+ssize_t vrrp_pkt_parse_datagram(int family, struct msghdr *m, size_t read,
+                               struct vrrp_pkt **pkt, char *errmsg,
+                               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)

diff --git a/vrrpd/vrrp_packet.h b/vrrpd/vrrp_packet.h
index df43ee76045e5416b058796fd2e3b1b8aafe6073..ed577ddcfa3aa739808c6b567b371001c9401806 100644 (file)
--- a/vrrpd/vrrp_packet.h
+++ b/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,
-                      uint8_t prio, uint16_t max_adver_int, uint8_t numip,
-                      struct ipaddr **ips);
+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);
 
 /*
- * 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,
-                           struct vrrp_pkt **pkt, char *errmsg,
-                           size_t errmsg_len);
+ssize_t vrrp_pkt_parse_datagram(int family, struct msghdr *m, size_t read,
+                               struct vrrp_pkt **pkt, char *errmsg,
+                               size_t errmsg_len);
 
 #endif /* __VRRP_PACKET_H__ */