From 9f2379bdd2d832370588d3ace1dde591b68cdcaf Mon Sep 17 00:00:00 2001 From: Siger Yang Date: Fri, 25 Nov 2022 13:52:46 +0800 Subject: [PATCH] vrrpd: add IPv4 pseudoheader option for VRRPv3 This commit adds a new option to control whether a VRRPv3 group accepts / computes its checksum with a prepended IPv4 pseudoheader. This should improve interoperability with other devices. Signed-off-by: Siger Yang --- vrrpd/vrrp.c | 19 ++++++++++++++++--- vrrpd/vrrp.h | 10 ++++++++++ vrrpd/vrrp_northbound.c | 27 +++++++++++++++++++++++++++ vrrpd/vrrp_packet.c | 31 +++++++++++++++++-------------- vrrpd/vrrp_packet.h | 10 +++++----- vrrpd/vrrp_vty.c | 40 +++++++++++++++++++++++++++++++++++++++- vrrpd/vrrp_vty.h | 3 +++ yang/frr-vrrpd.yang | 7 +++++++ 8 files changed, 124 insertions(+), 23 deletions(-) diff --git a/vrrpd/vrrp.c b/vrrpd/vrrp.c index 4a0356411f..e1bb40c28d 100644 --- a/vrrpd/vrrp.c +++ b/vrrpd/vrrp.c @@ -647,6 +647,8 @@ struct vrrp_vrouter *vrrp_vrouter_create(struct interface *ifp, uint8_t vrid, vr->priority = vd.priority; vr->preempt_mode = vd.preempt_mode; vr->accept_mode = vd.accept_mode; + vr->checksum_with_ipv4_pseudoheader = + vd.checksum_with_ipv4_pseudoheader; vr->shutdown = vd.shutdown; vr->v4 = vrrp_router_create(vr, AF_INET); @@ -789,7 +791,8 @@ static void vrrp_send_advertisement(struct vrrp_router *r) pktsz = vrrp_pkt_adver_build(&pkt, &r->src, r->vr->version, r->vr->vrid, r->priority, r->vr->advertisement_interval, - r->addrs->count, (struct ipaddr **)&addrs); + r->addrs->count, (struct ipaddr **)&addrs, + r->vr->checksum_with_ipv4_pseudoheader); if (DEBUG_MODE_CHECK(&vrrp_dbg_pkt, DEBUG_MODE_ALL)) zlog_hexdump(pkt, (size_t)pktsz); @@ -1027,8 +1030,10 @@ static void vrrp_read(struct thread *thread) zlog_hexdump(r->ibuf, nbytes); } - pktsize = vrrp_pkt_parse_datagram(r->family, r->vr->version, &m, nbytes, - &src, &pkt, errbuf, sizeof(errbuf)); + pktsize = vrrp_pkt_parse_datagram( + r->family, r->vr->version, + r->vr->checksum_with_ipv4_pseudoheader, &m, nbytes, &src, &pkt, + errbuf, sizeof(errbuf)); if (pktsize < 0) DEBUGD(&vrrp_dbg_pkt, @@ -2347,6 +2352,12 @@ int vrrp_config_write_global(struct vty *vty) vty_out(vty, "%svrrp default accept\n", !vd.accept_mode ? "no " : ""); + if (vd.checksum_with_ipv4_pseudoheader != + VRRP_DEFAULT_CHECKSUM_WITH_IPV4_PSEUDOHEADER && + ++writes) + vty_out(vty, "%svrrp default checksum-with-ipv4-pseudoheader\n", + !vd.checksum_with_ipv4_pseudoheader ? "no " : ""); + if (vd.shutdown != VRRP_DEFAULT_SHUTDOWN && ++writes) vty_out(vty, "%svrrp default shutdown\n", !vd.shutdown ? "no " : ""); @@ -2387,6 +2398,8 @@ void vrrp_init(void) vd.preempt_mode = yang_get_default_bool("%s/preempt", VRRP_XPATH_FULL); vd.accept_mode = yang_get_default_bool("%s/accept-mode", VRRP_XPATH_FULL); + vd.checksum_with_ipv4_pseudoheader = yang_get_default_bool( + "%s/checksum-with-ipv4-pseudoheader", VRRP_XPATH_FULL); vd.shutdown = VRRP_DEFAULT_SHUTDOWN; vrrp_autoconfig_version = 3; diff --git a/vrrpd/vrrp.h b/vrrpd/vrrp.h index c181c2159b..b3141ef318 100644 --- a/vrrpd/vrrp.h +++ b/vrrpd/vrrp.h @@ -53,6 +53,7 @@ #define VRRP_DEFAULT_ADVINT 100 #define VRRP_DEFAULT_PREEMPT true #define VRRP_DEFAULT_ACCEPT true +#define VRRP_DEFAULT_CHECKSUM_WITH_IPV4_PSEUDOHEADER true #define VRRP_DEFAULT_SHUTDOWN false /* User compatibility constant */ @@ -70,6 +71,7 @@ struct vrrp_defaults { uint16_t advertisement_interval; bool preempt_mode; bool accept_mode; + bool checksum_with_ipv4_pseudoheader; bool shutdown; }; @@ -266,6 +268,14 @@ struct vrrp_vrouter { */ bool accept_mode; + /* + * Indicates whether this router computes and accepts VRRPv3 checksums + * without pseudoheader, for device interoperability. + * + * This option should only affect IPv4 virtual routers. + */ + bool checksum_with_ipv4_pseudoheader; + struct vrrp_router *v4; struct vrrp_router *v6; }; diff --git a/vrrpd/vrrp_northbound.c b/vrrpd/vrrp_northbound.c index d25dc0a197..76d0ad3b1b 100644 --- a/vrrpd/vrrp_northbound.c +++ b/vrrpd/vrrp_northbound.c @@ -602,6 +602,26 @@ lib_interface_vrrp_vrrp_group_shutdown_modify(struct nb_cb_modify_args *args) return NB_OK; } +/* + * XPath: /frr-interface:lib/interface/frr-vrrpd:vrrp/vrrp-group/checksum-with- + * ipv4-pseudoheader + */ +static int lib_interface_vrrp_vrrp_group_checksum_with_ipv4_pseudoheader_modify( + struct nb_cb_modify_args *args) +{ + if (args->event != NB_EV_APPLY) + return NB_OK; + + struct vrrp_vrouter *vr; + bool checksum_with_ipv4_ph; + + vr = nb_running_get_entry(args->dnode, NULL, true); + checksum_with_ipv4_ph = yang_dnode_get_bool(args->dnode, NULL); + vr->checksum_with_ipv4_pseudoheader = checksum_with_ipv4_ph; + + return NB_OK; +} + /* clang-format off */ const struct frr_yang_module_info frr_vrrpd_info = { .name = "frr-vrrpd", @@ -643,6 +663,13 @@ const struct frr_yang_module_info frr_vrrpd_info = { .modify = lib_interface_vrrp_vrrp_group_accept_mode_modify, } }, + { + .xpath = "/frr-interface:lib/interface/frr-vrrpd:vrrp/vrrp-group/checksum-with-ipv4-pseudoheader", + .cbs = { + .modify = lib_interface_vrrp_vrrp_group_checksum_with_ipv4_pseudoheader_modify, + .cli_show = cli_show_checksum_with_ipv4_pseudoheader, + } + }, { .xpath = "/frr-interface:lib/interface/frr-vrrpd:vrrp/vrrp-group/advertisement-interval", .cbs = { diff --git a/vrrpd/vrrp_packet.c b/vrrpd/vrrp_packet.c index 991c030196..96a19da88c 100644 --- a/vrrpd/vrrp_packet.c +++ b/vrrpd/vrrp_packet.c @@ -71,7 +71,7 @@ static const char *const vrrp_packet_names[16] = { * VRRP checksum in network byte order. */ static uint16_t vrrp_pkt_checksum(struct vrrp_pkt *pkt, size_t pktsize, - struct ipaddr *src) + struct ipaddr *src, bool ipv4_ph) { uint16_t chksum; bool v6 = (src->ipa_type == IPADDR_V6); @@ -89,13 +89,16 @@ static uint16_t vrrp_pkt_checksum(struct vrrp_pkt *pkt, size_t pktsize, ph.next_hdr = IPPROTO_VRRP; chksum = in_cksum_with_ph6(&ph, pkt, pktsize); } else if (!v6 && ((pkt->hdr.vertype >> 4) == 3)) { - struct ipv4_ph ph = {}; + if (ipv4_ph) { + struct ipv4_ph ph = {}; - ph.src = src->ipaddr_v4; - inet_pton(AF_INET, VRRP_MCASTV4_GROUP_STR, &ph.dst); - ph.proto = IPPROTO_VRRP; - ph.len = htons(pktsize); - chksum = in_cksum_with_ph4(&ph, pkt, pktsize); + ph.src = src->ipaddr_v4; + inet_pton(AF_INET, VRRP_MCASTV4_GROUP_STR, &ph.dst); + ph.proto = IPPROTO_VRRP; + ph.len = htons(pktsize); + chksum = in_cksum_with_ph4(&ph, pkt, pktsize); + } else + chksum = in_cksum(pkt, pktsize); } else if (!v6 && ((pkt->hdr.vertype >> 4) == 2)) { chksum = in_cksum(pkt, pktsize); } else { @@ -110,7 +113,7 @@ static uint16_t vrrp_pkt_checksum(struct vrrp_pkt *pkt, size_t pktsize, 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) + struct ipaddr **ips, bool ipv4_ph) { bool v6 = false; size_t addrsz = 0; @@ -147,7 +150,7 @@ ssize_t vrrp_pkt_adver_build(struct vrrp_pkt **pkt, struct ipaddr *src, aptr += addrsz; } - (*pkt)->hdr.chksum = vrrp_pkt_checksum(*pkt, pktsize, src); + (*pkt)->hdr.chksum = vrrp_pkt_checksum(*pkt, pktsize, src, ipv4_ph); return pktsize; } @@ -188,10 +191,10 @@ size_t vrrp_pkt_adver_dump(char *buf, size_t buflen, struct vrrp_pkt *pkt) return rs; } -ssize_t vrrp_pkt_parse_datagram(int family, int version, struct msghdr *m, - size_t read, struct ipaddr *src, - struct vrrp_pkt **pkt, char *errmsg, - size_t errmsg_len) +ssize_t vrrp_pkt_parse_datagram(int family, int version, bool ipv4_ph, + struct msghdr *m, size_t read, + struct ipaddr *src, 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) @@ -289,7 +292,7 @@ ssize_t vrrp_pkt_parse_datagram(int family, int version, struct msghdr *m, VRRP_PKT_VCHECK(pktver == version, "Bad version %u", pktver); /* Checksum check */ - uint16_t chksum = vrrp_pkt_checksum(*pkt, pktsize, src); + uint16_t chksum = vrrp_pkt_checksum(*pkt, pktsize, src, ipv4_ph); VRRP_PKT_VCHECK((*pkt)->hdr.chksum == chksum, "Bad VRRP checksum %hx; should be %hx", diff --git a/vrrpd/vrrp_packet.h b/vrrpd/vrrp_packet.h index 082935f080..eec709593e 100644 --- a/vrrpd/vrrp_packet.h +++ b/vrrpd/vrrp_packet.h @@ -131,7 +131,7 @@ struct vrrp_pkt { 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); + struct ipaddr **ips, bool ipv4_ph); /* free memory allocated by vrrp_pkt_adver_build's pkt arg */ void vrrp_pkt_free(struct vrrp_pkt *pkt); @@ -195,9 +195,9 @@ size_t vrrp_pkt_adver_dump(char *buf, size_t buflen, struct vrrp_pkt *pkt); * Returns: * Size of VRRP packet, or -1 upon error */ -ssize_t vrrp_pkt_parse_datagram(int family, int version, struct msghdr *m, - size_t read, struct ipaddr *src, - struct vrrp_pkt **pkt, char *errmsg, - size_t errmsg_len); +ssize_t vrrp_pkt_parse_datagram(int family, int version, bool ipv4_ph, + struct msghdr *m, size_t read, + struct ipaddr *src, struct vrrp_pkt **pkt, + char *errmsg, size_t errmsg_len); #endif /* __VRRP_PACKET_H__ */ diff --git a/vrrpd/vrrp_vty.c b/vrrpd/vrrp_vty.c index f822b89854..1e1edb8212 100644 --- a/vrrpd/vrrp_vty.c +++ b/vrrpd/vrrp_vty.c @@ -281,6 +281,35 @@ void cli_show_preempt(struct vty *vty, const struct lyd_node *dnode, vty_out(vty, " %svrrp %s preempt\n", pre ? "" : "no ", vrid); } +/* + * XPath: /frr-interface:lib/interface/frr-vrrpd:vrrp/vrrp-group/checksum-with- + * ipv4-pseudoheader + */ +DEFPY_YANG(vrrp_checksum_with_ipv4_pseudoheader, + vrrp_checksum_with_ipv4_pseudoheader_cmd, + "[no] vrrp (1-255)$vrid checksum-with-ipv4-pseudoheader", + NO_STR + VRRP_STR + VRRP_VRID_STR + "Checksum mode in VRRPv3\n") +{ + nb_cli_enqueue_change(vty, "./checksum-with-ipv4-pseudoheader", + NB_OP_MODIFY, no ? "false" : "true"); + + return nb_cli_apply_changes(vty, VRRP_XPATH_ENTRY, vrid); +} + +void cli_show_checksum_with_ipv4_pseudoheader(struct vty *vty, + const struct lyd_node *dnode, + bool show_defaults) +{ + const char *vrid = yang_dnode_get_string(dnode, "../virtual-router-id"); + const bool pre = yang_dnode_get_bool(dnode, NULL); + + vty_out(vty, " %svrrp %s checksum-with-ipv4-pseudoheader\n", + pre ? "" : "no ", vrid); +} + /* XXX: yang conversion */ DEFPY_YANG(vrrp_autoconfigure, vrrp_autoconfigure_cmd, @@ -304,7 +333,7 @@ DEFPY_YANG(vrrp_autoconfigure, /* XXX: yang conversion */ DEFPY_YANG(vrrp_default, vrrp_default_cmd, - "[no] vrrp default ", + "[no] vrrp default ", NO_STR VRRP_STR "Configure defaults for new VRRP instances\n" @@ -313,6 +342,7 @@ DEFPY_YANG(vrrp_default, "Preempt mode\n" VRRP_PRIORITY_STR "Priority value\n" + "Checksum mode in VRRPv3\n" "Force VRRP router into administrative shutdown\n") { if (adv) { @@ -329,6 +359,8 @@ DEFPY_YANG(vrrp_default, vd.preempt_mode = !no; if (prio) vd.priority = no ? VRRP_DEFAULT_PRIORITY : prioval; + if (ipv4ph) + vd.checksum_with_ipv4_pseudoheader = !no; if (s) vd.shutdown = !no; @@ -374,6 +406,8 @@ static struct json_object *vrrp_build_json(struct vrrp_vrouter *vr) json_object_boolean_add(j, "shutdown", vr->shutdown); json_object_boolean_add(j, "preemptMode", vr->preempt_mode); json_object_boolean_add(j, "acceptMode", vr->accept_mode); + json_object_boolean_add(j, "checksumWithIpv4Pseudoheader", + vr->checksum_with_ipv4_pseudoheader); json_object_string_add(j, "interface", vr->ifp->name); json_object_int_add(j, "advertisementInterval", vr->advertisement_interval * CS2MS); @@ -499,6 +533,8 @@ static void vrrp_show(struct vty *vty, struct vrrp_vrouter *vr) vr->preempt_mode ? "Yes" : "No"); ttable_add_row(tt, "%s|%s", "Accept Mode", vr->accept_mode ? "Yes" : "No"); + ttable_add_row(tt, "%s|%s", "Checksum with IPv4 Pseudoheader", + vr->checksum_with_ipv4_pseudoheader ? "Yes" : "No"); ttable_add_row(tt, "%s|%d ms", "Advertisement Interval", vr->advertisement_interval * CS2MS); ttable_add_row(tt, "%s|%d ms (stale)", @@ -752,4 +788,6 @@ void vrrp_vty_init(void) install_element(INTERFACE_NODE, &vrrp_ip_cmd); install_element(INTERFACE_NODE, &vrrp_ip6_cmd); install_element(INTERFACE_NODE, &vrrp_preempt_cmd); + install_element(INTERFACE_NODE, + &vrrp_checksum_with_ipv4_pseudoheader_cmd); } diff --git a/vrrpd/vrrp_vty.h b/vrrpd/vrrp_vty.h index 587537a6f3..be88349e78 100644 --- a/vrrpd/vrrp_vty.h +++ b/vrrpd/vrrp_vty.h @@ -40,5 +40,8 @@ void cli_show_ipv6(struct vty *vty, const struct lyd_node *dnode, bool show_defaults); void cli_show_preempt(struct vty *vty, const struct lyd_node *dnode, bool show_defaults); +void cli_show_checksum_with_ipv4_pseudoheader(struct vty *vty, + const struct lyd_node *dnode, + bool show_defaults); #endif /* __VRRP_VTY_H__ */ diff --git a/yang/frr-vrrpd.yang b/yang/frr-vrrpd.yang index 200eaeb0b5..cd04df2670 100644 --- a/yang/frr-vrrpd.yang +++ b/yang/frr-vrrpd.yang @@ -110,6 +110,13 @@ module frr-vrrpd { address is not owned by the router interface"; } + leaf checksum-with-ipv4-pseudoheader { + type boolean; + default "true"; + description + "Enabled if VRRPv3 checksum for IPv4 involves pseudoheader"; + } + leaf advertisement-interval { type uint16 { range "1..4095";