From 8e23507a638e4371618913d776e1e69c418e4995 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Wed, 26 Jan 2022 00:01:08 -0500 Subject: [PATCH 01/24] include: bump if_netlink.h version for protodown Bump if_netlink.h UAPI files for protodown netlink processing. Signed-off-by: Stephen Worley --- include/linux/if_link.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/include/linux/if_link.h b/include/linux/if_link.h index 22a45914a2..e5cea27829 100644 --- a/include/linux/if_link.h +++ b/include/linux/if_link.h @@ -167,12 +167,25 @@ enum { IFLA_NEW_IFINDEX, IFLA_MIN_MTU, IFLA_MAX_MTU, + IFLA_PROP_LIST, + IFLA_ALT_IFNAME, /* Alternative ifname */ + IFLA_PERM_ADDRESS, + IFLA_PROTO_DOWN_REASON, __IFLA_MAX }; #define IFLA_MAX (__IFLA_MAX - 1) +enum { + IFLA_PROTO_DOWN_REASON_UNSPEC, + IFLA_PROTO_DOWN_REASON_MASK, /* u32, mask for reason bits */ + IFLA_PROTO_DOWN_REASON_VALUE, /* u32, reason bit value */ + + __IFLA_PROTO_DOWN_REASON_CNT, + IFLA_PROTO_DOWN_REASON_MAX = __IFLA_PROTO_DOWN_REASON_CNT - 1 +}; + /* backwards compatibility for userspace */ #ifndef __KERNEL__ #define IFLA_RTA(r) ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct ifinfomsg)))) From 5d4141383344bf244c572f9d23c0175d9573f41d Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Wed, 26 Jan 2022 00:07:57 -0500 Subject: [PATCH 02/24] zebra: add support for protodown reason code Add support for setting the protodown reason code. https://github.com/torvalds/linux/commit/829eb208e80d6db95c0201cb8fa00c2f9ad87faf These patches handle all our netlink code for setting the reason. For protodown reason we only set `frr` as the reason externally but internally we have more descriptive reasoning available via `show interface IFNAME`. The kernel only provides a bitwidth of 32 that all userspace programs have to share so this makes the most sense. Since this is new functionality, it needs to be added to the dplane pthread instead. So these patches, also move the protodown setting we were doing before into the dplane pthread. For this, we abstract it a bit more to make it a general interface LINK update dplane API. This API can be expanded to support gernal link creation/updating when/if someone ever adds that code. We also move a more common entrypoint for evpn-mh and from zapi clients like vrrpd. They both call common code now to set our internal flags for protodown and protodown reason. Also add debugging code for dumping netlink packets with protodown/protodown_reason. Signed-off-by: Stephen Worley --- zebra/debug_nl.c | 110 +++++++++++++++-- zebra/dplane_fpm_nl.c | 3 + zebra/if_netlink.c | 132 +++++++++++++++++++- zebra/if_netlink.h | 13 +- zebra/interface.c | 273 ++++++++++++++++++++++++++++++++--------- zebra/interface.h | 12 +- zebra/kernel_netlink.c | 6 + zebra/rt_netlink.h | 2 + zebra/zapi_msg.c | 22 +++- zebra/zebra_dplane.c | 189 ++++++++++++++++++++++++++++ zebra/zebra_dplane.h | 20 +++ zebra/zebra_errors.c | 9 ++ zebra/zebra_errors.h | 1 + zebra/zebra_evpn_mh.c | 46 +++---- zebra/zebra_evpn_mh.h | 2 +- zebra/zebra_nhg.c | 3 + zebra/zebra_rib.c | 8 +- zebra/zebra_router.h | 13 +- 18 files changed, 744 insertions(+), 120 deletions(-) diff --git a/zebra/debug_nl.c b/zebra/debug_nl.c index 260ba30b3c..b7d12bf537 100644 --- a/zebra/debug_nl.c +++ b/zebra/debug_nl.c @@ -255,6 +255,40 @@ const char *ifi_type2str(int type) } } +const char *ifla_pdr_type2str(int type) +{ + switch (type) { + case IFLA_PROTO_DOWN_REASON_UNSPEC: + return "UNSPEC"; + case IFLA_PROTO_DOWN_REASON_MASK: + return "MASK"; + case IFLA_PROTO_DOWN_REASON_VALUE: + return "VALUE"; + default: + return "UNKNOWN"; + } +} + +const char *ifla_info_type2str(int type) +{ + switch (type) { + case IFLA_INFO_UNSPEC: + return "UNSPEC"; + case IFLA_INFO_KIND: + return "KIND"; + case IFLA_INFO_DATA: + return "DATA"; + case IFLA_INFO_XSTATS: + return "XSTATS"; + case IFLA_INFO_SLAVE_KIND: + return "SLAVE_KIND"; + case IFLA_INFO_SLAVE_DATA: + return "SLAVE_DATA"; + default: + return "UNKNOWN"; + } +} + const char *rta_type2str(int type) { switch (type) { @@ -358,6 +392,8 @@ const char *rta_type2str(int type) case IFLA_EVENT: return "EVENT"; #endif /* IFLA_EVENT */ + case IFLA_PROTO_DOWN_REASON: + return "PROTO_DOWN_REASON"; default: return "UNKNOWN"; } @@ -838,6 +874,42 @@ const char *nh_flags2str(uint32_t flags, char *buf, size_t buflen) /* * Netlink abstractions. */ +static void nllink_pdr_dump(struct rtattr *rta, size_t msglen) +{ + size_t plen; + uint32_t u32v; + +next_rta: + /* Check the header for valid length and for outbound access. */ + if (RTA_OK(rta, msglen) == 0) + return; + + plen = RTA_PAYLOAD(rta); + zlog_debug(" linkinfo [len=%d (payload=%zu) type=(%d) %s]", + rta->rta_len, plen, rta->rta_type, + ifla_pdr_type2str(rta->rta_type)); + switch (rta->rta_type) { + case IFLA_PROTO_DOWN_REASON_MASK: + case IFLA_PROTO_DOWN_REASON_VALUE: + if (plen < sizeof(uint32_t)) { + zlog_debug(" invalid length"); + break; + } + + u32v = *(uint32_t *)RTA_DATA(rta); + zlog_debug(" %u", u32v); + break; + + default: + /* NOTHING: unhandled. */ + break; + } + + /* Get next pointer and start iteration again. */ + rta = RTA_NEXT(rta, msglen); + goto next_rta; +} + static void nllink_linkinfo_dump(struct rtattr *rta, size_t msglen) { size_t plen; @@ -851,7 +923,7 @@ next_rta: plen = RTA_PAYLOAD(rta); zlog_debug(" linkinfo [len=%d (payload=%zu) type=(%d) %s]", rta->rta_len, plen, rta->rta_type, - rta_type2str(rta->rta_type)); + ifla_info_type2str(rta->rta_type)); switch (rta->rta_type) { case IFLA_INFO_KIND: if (plen == 0) { @@ -888,8 +960,10 @@ static void nllink_dump(struct ifinfomsg *ifi, size_t msglen) struct rtattr *rta; size_t plen, it; uint32_t u32v; + uint8_t u8v; char bytestr[16]; char dbuf[128]; + unsigned short rta_type; /* Get the first attribute and go from there. */ rta = IFLA_RTA(ifi); @@ -899,10 +973,10 @@ next_rta: return; plen = RTA_PAYLOAD(rta); + rta_type = rta->rta_type & ~NLA_F_NESTED; zlog_debug(" rta [len=%d (payload=%zu) type=(%d) %s]", rta->rta_len, - plen, rta->rta_type, rta_type2str(rta->rta_type)); - switch (rta->rta_type) { - case IFLA_IFNAME: + plen, rta_type, rta_type2str(rta_type)); + switch (rta_type) { case IFLA_IFALIAS: if (plen == 0) { zlog_debug(" invalid length"); @@ -927,6 +1001,7 @@ next_rta: #endif /* IFLA_GSO_MAX_SIZE */ case IFLA_CARRIER_CHANGES: case IFLA_MASTER: + case IFLA_LINK: if (plen < sizeof(uint32_t)) { zlog_debug(" invalid length"); break; @@ -936,6 +1011,15 @@ next_rta: zlog_debug(" %u", u32v); break; + case IFLA_PROTO_DOWN: + if (plen < sizeof(uint8_t)) { + zlog_debug(" invalid length"); + break; + } + + u8v = *(uint8_t *)RTA_DATA(rta); + zlog_debug(" %u", u8v); + break; case IFLA_ADDRESS: datap = RTA_DATA(rta); dbuf[0] = 0; @@ -952,7 +1036,11 @@ next_rta: break; case IFLA_LINKINFO: - nllink_linkinfo_dump(RTA_DATA(rta), msglen); + nllink_linkinfo_dump(RTA_DATA(rta), plen); + break; + + case IFLA_PROTO_DOWN_REASON: + nllink_pdr_dump(RTA_DATA(rta), plen); break; default: @@ -1027,6 +1115,7 @@ static void nlneigh_dump(struct ndmsg *ndm, size_t msglen) uint16_t vid; char bytestr[16]; char dbuf[128]; + unsigned short rta_type; #ifndef NDA_RTA #define NDA_RTA(ndm) \ @@ -1043,9 +1132,10 @@ next_rta: return; plen = RTA_PAYLOAD(rta); + rta_type = rta->rta_type & ~NLA_F_NESTED; zlog_debug(" rta [len=%d (payload=%zu) type=(%d) %s]", rta->rta_len, - plen, rta->rta_type, neigh_rta2str(rta->rta_type)); - switch (rta->rta_type & ~ NLA_F_NESTED) { + plen, rta->rta_type, neigh_rta2str(rta_type)); + switch (rta_type) { case NDA_LLADDR: datap = RTA_DATA(rta); dbuf[0] = 0; @@ -1153,6 +1243,7 @@ static void nlnh_dump(struct nhmsg *nhm, size_t msglen) uint32_t u32v; unsigned long count, i; struct nexthop_grp *nhgrp; + unsigned short rta_type; rta = RTM_NHA(nhm); @@ -1162,9 +1253,10 @@ next_rta: return; plen = RTA_PAYLOAD(rta); + rta_type = rta->rta_type & ~NLA_F_NESTED; zlog_debug(" rta [len=%d (payload=%zu) type=(%d) %s]", rta->rta_len, - plen, rta->rta_type, nhm_rta2str(rta->rta_type)); - switch (rta->rta_type & ~NLA_F_NESTED) { + plen, rta->rta_type, nhm_rta2str(rta_type)); + switch (rta_type) { case NHA_ID: u32v = *(uint32_t *)RTA_DATA(rta); zlog_debug(" %u", u32v); diff --git a/zebra/dplane_fpm_nl.c b/zebra/dplane_fpm_nl.c index 8c8004190b..ae5ce25a70 100644 --- a/zebra/dplane_fpm_nl.c +++ b/zebra/dplane_fpm_nl.c @@ -812,6 +812,9 @@ static int fpm_nl_enqueue(struct fpm_nl_ctx *fnc, struct zebra_dplane_ctx *ctx) case DPLANE_OP_INTF_ADDR_ADD: case DPLANE_OP_INTF_ADDR_DEL: case DPLANE_OP_INTF_NETCONFIG: + case DPLANE_OP_INTF_INSTALL: + case DPLANE_OP_INTF_UPDATE: + case DPLANE_OP_INTF_DELETE: case DPLANE_OP_NONE: break; diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index a75b165270..748f239db9 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -77,6 +77,7 @@ #include "zebra/netconf_netlink.h" extern struct zebra_privs_t zserv_privs; +uint8_t frr_protodown_r_bit = FRR_PROTODOWN_REASON_DEFAULT_BIT; /* Note: on netlink systems, there should be a 1-to-1 mapping between interface names and ifindex values. */ @@ -822,6 +823,12 @@ static void netlink_proc_dplane_if_protodown(struct zebra_if *zif, { bool zif_protodown; + /* Set our reason code to note it wasn't us */ + if (protodown) + zif->protodown_rc |= ZEBRA_PROTODOWN_EXTERNAL; + else + zif->protodown_rc &= ~ZEBRA_PROTODOWN_EXTERNAL; + zif_protodown = !!(zif->flags & ZIF_FLAG_PROTODOWN); if (protodown == zif_protodown) return; @@ -835,7 +842,7 @@ static void netlink_proc_dplane_if_protodown(struct zebra_if *zif, zlog_debug( "bond mbr %s re-instate protdown %s in the dplane", zif->ifp->name, zif_protodown ? "on" : "off"); - netlink_protodown(zif->ifp, zif_protodown); + netlink_protodown(zif->ifp, zif_protodown, zif->protodown_rc); } else { if (protodown) zif->flags |= ZIF_FLAG_PROTODOWN; @@ -1244,6 +1251,41 @@ netlink_put_address_update_msg(struct nl_batch *bth, false); } +static ssize_t netlink_intf_msg_encoder(struct zebra_dplane_ctx *ctx, void *buf, + size_t buflen) +{ + enum dplane_op_e op; + int cmd = 0; + + op = dplane_ctx_get_op(ctx); + + switch (op) { + case DPLANE_OP_INTF_UPDATE: + cmd = RTM_SETLINK; + break; + case DPLANE_OP_INTF_INSTALL: + cmd = RTM_NEWLINK; + break; + case DPLANE_OP_INTF_DELETE: + cmd = RTM_DELLINK; + break; + default: + flog_err( + EC_ZEBRA_NHG_FIB_UPDATE, + "Context received for kernel interface update with incorrect OP code (%u)", + op); + return -1; + } + + return netlink_intf_msg_encode(cmd, ctx, buf, buflen); +} + +enum netlink_msg_status +netlink_put_intf_update_msg(struct nl_batch *bth, struct zebra_dplane_ctx *ctx) +{ + return netlink_batch_add_msg(bth, ctx, netlink_intf_msg_encoder, false); +} + int netlink_interface_addr(struct nlmsghdr *h, ns_id_t ns_id, int startup) { int len; @@ -2049,9 +2091,78 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) return 0; } -int netlink_protodown(struct interface *ifp, bool down) +/** + * Interface encoding helper function. + * + * \param[in] cmd netlink command. + * \param[in] ctx dataplane context (information snapshot). + * \param[out] buf buffer to hold the packet. + * \param[in] buflen amount of buffer bytes. + */ + +ssize_t netlink_intf_msg_encode(uint16_t cmd, + const struct zebra_dplane_ctx *ctx, void *buf, + size_t buflen) +{ + struct { + struct nlmsghdr n; + struct ifinfomsg ifa; + char buf[]; + } *req = buf; + + struct rtattr *nest_protodown_reason; + ifindex_t ifindex = dplane_ctx_get_ifindex(ctx); + uint32_t r_bitfield = dplane_ctx_get_intf_r_bitfield(ctx); + bool down = dplane_ctx_intf_is_protodown(ctx); + struct nlsock *nl = + kernel_netlink_nlsock_lookup(dplane_ctx_get_ns_sock(ctx)); + + if (buflen < sizeof(*req)) + return 0; + + memset(req, 0, sizeof(*req)); + + if (cmd != RTM_SETLINK) + flog_err( + EC_ZEBRA_INTF_UPDATE_FAILURE, + "Only RTM_SETLINK message type currently supported in dplane pthread"); + + req->n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)); + req->n.nlmsg_flags = NLM_F_REQUEST; + req->n.nlmsg_type = cmd; + req->n.nlmsg_pid = nl->snl.nl_pid; + + req->ifa.ifi_index = ifindex; + + nl_attr_put8(&req->n, buflen, IFLA_PROTO_DOWN, down); + nl_attr_put32(&req->n, buflen, IFLA_LINK, ifindex); + + if (r_bitfield) { + nest_protodown_reason = + nl_attr_nest(&req->n, buflen, IFLA_PROTO_DOWN_REASON); + + if (!nest_protodown_reason) + return -1; + + nl_attr_put32(&req->n, buflen, IFLA_PROTO_DOWN_REASON_MASK, + (1 << frr_protodown_r_bit)); + nl_attr_put32(&req->n, buflen, IFLA_PROTO_DOWN_REASON_VALUE, + ((int)down) << frr_protodown_r_bit); + + nl_attr_nest_end(&req->n, nest_protodown_reason); + } + + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug("%s: %s, protodown=%d ifindex=%u", __func__, + nl_msg_type_to_str(cmd), down, ifindex); + + return NLMSG_ALIGN(req->n.nlmsg_len); +} + +int netlink_protodown(struct interface *ifp, bool down, uint32_t r_bitfield) { struct zebra_ns *zns = zebra_ns_lookup(NS_DEFAULT); + struct rtattr *nest_protodown_reason; struct { struct nlmsghdr n; @@ -2068,9 +2179,24 @@ int netlink_protodown(struct interface *ifp, bool down) req.ifa.ifi_index = ifp->ifindex; - nl_attr_put(&req.n, sizeof(req), IFLA_PROTO_DOWN, &down, sizeof(down)); + nl_attr_put8(&req.n, sizeof(req), IFLA_PROTO_DOWN, down); nl_attr_put32(&req.n, sizeof(req), IFLA_LINK, ifp->ifindex); + if (r_bitfield) { + nest_protodown_reason = nl_attr_nest(&req.n, sizeof(req), + IFLA_PROTO_DOWN_REASON); + + if (!nest_protodown_reason) + return -1; + + nl_attr_put32(&req.n, sizeof(req), IFLA_PROTO_DOWN_REASON_MASK, + (1 << frr_protodown_r_bit)); + nl_attr_put32(&req.n, sizeof(req), IFLA_PROTO_DOWN_REASON_VALUE, + ((int)down) << frr_protodown_r_bit); + + nl_attr_nest_end(&req.n, nest_protodown_reason); + } + return netlink_talk(netlink_talk_filter, &req.n, &zns->netlink_cmd, zns, false); } diff --git a/zebra/if_netlink.h b/zebra/if_netlink.h index a1ce7af8c7..d5a73bb467 100644 --- a/zebra/if_netlink.h +++ b/zebra/if_netlink.h @@ -40,6 +40,9 @@ int netlink_interface_addr_dplane(struct nlmsghdr *h, ns_id_t ns_id, extern int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup); extern int interface_lookup_netlink(struct zebra_ns *zns); +extern ssize_t netlink_intf_msg_encode(uint16_t cmd, + const struct zebra_dplane_ctx *ctx, + void *buf, size_t buflen); extern enum netlink_msg_status netlink_put_gre_set_msg(struct nl_batch *bth, struct zebra_dplane_ctx *ctx); @@ -47,6 +50,11 @@ extern enum netlink_msg_status netlink_put_address_update_msg(struct nl_batch *bth, struct zebra_dplane_ctx *ctx); +extern enum netlink_msg_status +netlink_put_intf_update_msg(struct nl_batch *bth, struct zebra_dplane_ctx *ctx); + +#define FRR_PROTODOWN_REASON_DEFAULT_BIT 7 +#define PROTODOWN_REASON_NUM_BITS 32 /* * Set protodown status of interface. * @@ -56,10 +64,13 @@ netlink_put_address_update_msg(struct nl_batch *bth, * down * If true, set protodown on. If false, set protodown off. * + * reason + * bitfield representing reason codes + * * Returns: * 0 */ -int netlink_protodown(struct interface *ifp, bool down); +int netlink_protodown(struct interface *ifp, bool down, uint32_t r_bitfield); #ifdef __cplusplus } diff --git a/zebra/interface.c b/zebra/interface.c index a76f8741e0..d326109c19 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -1229,58 +1229,73 @@ void zebra_if_update_all_links(struct zebra_ns *zns) } } -void zebra_if_set_protodown(struct interface *ifp, bool down) +int zebra_if_set_protodown(struct interface *ifp, bool new_down, + enum protodown_reasons new_reason) { + struct zebra_if *zif; + bool old_down; + uint32_t new_protodown_rc; + + zif = ifp->info; + + old_down = !!(zif->flags & ZIF_FLAG_PROTODOWN); + + if (new_down) + new_protodown_rc = zif->protodown_rc | new_reason; + else + new_protodown_rc = zif->protodown_rc & ~new_reason; + + /* Early return if already set down & reason bitfield matches */ + if ((new_down == old_down) && (new_protodown_rc == zif->protodown_rc)) { + + if (IS_ZEBRA_DEBUG_KERNEL) { + zlog_debug( + "Ignoring %s (%u): protodown %s already set reason: old 0x%x new 0x%x", + ifp->name, ifp->ifindex, + new_down ? "on" : "off", zif->protodown_rc, + new_protodown_rc); + return 1; + } + } + + zlog_info( + "Setting interface %s (%u): protodown %s reason: old 0x%x new 0x%x", + ifp->name, ifp->ifindex, new_down ? "on" : "off", + zif->protodown_rc, new_protodown_rc); + + zif->protodown_rc = new_protodown_rc; + #ifdef HAVE_NETLINK - netlink_protodown(ifp, down); + // TODO: remove this as separate commit + // netlink_protodown(ifp, new_down, zif->protodown_rc); + dplane_intf_update(ifp); #else zlog_warn("Protodown is not supported on this platform"); #endif + return 0; } /* - * Handle an interface addr event based on info in a dplane context object. + * Handle an interface events based on info in a dplane context object. * This runs in the main pthread, using the info in the context object to * modify an interface. */ -void zebra_if_addr_update_ctx(struct zebra_dplane_ctx *ctx) +static void zebra_if_addr_update_ctx(struct zebra_dplane_ctx *ctx, + struct interface *ifp) { - struct interface *ifp; uint8_t flags = 0; const char *label = NULL; - ns_id_t ns_id; - struct zebra_ns *zns; uint32_t metric = METRIC_MAX; - ifindex_t ifindex; const struct prefix *addr, *dest = NULL; enum dplane_op_e op; op = dplane_ctx_get_op(ctx); - ns_id = dplane_ctx_get_ns_id(ctx); - - zns = zebra_ns_lookup(ns_id); - if (zns == NULL) { - /* No ns - deleted maybe? */ - if (IS_ZEBRA_DEBUG_KERNEL) - zlog_debug("%s: can't find zns id %u", __func__, ns_id); - goto done; - } - - ifindex = dplane_ctx_get_ifindex(ctx); - - ifp = if_lookup_by_index_per_ns(zns, ifindex); - if (ifp == NULL) { - if (IS_ZEBRA_DEBUG_KERNEL) - zlog_debug("%s: can't find ifp at nsid %u index %d", - __func__, ns_id, ifindex); - goto done; - } - addr = dplane_ctx_get_intf_addr(ctx); if (IS_ZEBRA_DEBUG_KERNEL) - zlog_debug("%s: %s: ifindex %u, addr %pFX", __func__, - dplane_op2str(op), ifindex, addr); + zlog_debug("%s: %s: ifindex %s(%u), addr %pFX", __func__, + dplane_op2str(dplane_ctx_get_op(ctx)), ifp->name, + ifp->ifindex, addr); /* Is there a peer or broadcast address? */ dest = dplane_ctx_get_intf_dest(ctx); @@ -1335,41 +1350,64 @@ void zebra_if_addr_update_ctx(struct zebra_dplane_ctx *ctx) */ if (op != DPLANE_OP_INTF_ADDR_ADD) rib_update(RIB_UPDATE_KERNEL); +} -done: - /* We're responsible for the ctx object */ - dplane_ctx_fini(&ctx); +static void zebra_if_update_ctx(struct zebra_dplane_ctx *ctx, + struct interface *ifp) +{ + enum zebra_dplane_result dp_res; + struct zebra_if *zif; + uint32_t r_bitfield; + bool down; + + dp_res = dplane_ctx_get_status(ctx); + r_bitfield = dplane_ctx_get_intf_r_bitfield(ctx); + down = dplane_ctx_intf_is_protodown(ctx); + + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug("%s: %s: if %s(%u) ctx-protodown %s ctx-reason 0x%x", + __func__, dplane_op2str(dplane_ctx_get_op(ctx)), + ifp->name, ifp->ifindex, down ? "on" : "off", + r_bitfield); + + zif = ifp->info; + if (!zif) { + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug("%s: if %s(%u) zebra info pointer is NULL", + __func__, ifp->name, ifp->ifindex); + return; + } + + if (dp_res != ZEBRA_DPLANE_REQUEST_SUCCESS) { + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug("%s: if %s(%u) dplane update failed", + __func__, ifp->name, ifp->ifindex); + return; + } + + /* Update our info */ + if (down) + zif->flags |= ZIF_FLAG_PROTODOWN; + else + zif->flags &= ~ZIF_FLAG_PROTODOWN; } /* * Handle netconf change from a dplane context object; runs in the main * pthread so it can update zebra data structs. */ -int zebra_if_netconf_update_ctx(struct zebra_dplane_ctx *ctx) +static void zebra_if_netconf_update_ctx(struct zebra_dplane_ctx *ctx, + struct interface *ifp) { - struct zebra_ns *zns; - struct interface *ifp; struct zebra_if *zif; enum dplane_netconf_status_e mpls; - int ret = 0; - - zns = zebra_ns_lookup(dplane_ctx_get_netconf_ns_id(ctx)); - if (zns == NULL) { - ret = -1; - goto done; - } - - ifp = if_lookup_by_index_per_ns(zns, - dplane_ctx_get_netconf_ifindex(ctx)); - if (ifp == NULL) { - ret = -1; - goto done; - } zif = ifp->info; - if (zif == NULL) { - ret = -1; - goto done; + if (!zif) { + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug("%s: if %s(%u) zebra info pointer is NULL", + __func__, ifp->name, ifp->ifindex); + return; } mpls = dplane_ctx_get_netconf_mpls(ctx); @@ -1383,12 +1421,105 @@ int zebra_if_netconf_update_ctx(struct zebra_dplane_ctx *ctx) zlog_debug("%s: if %s, ifindex %d, mpls %s", __func__, ifp->name, ifp->ifindex, (zif->mpls ? "ON" : "OFF")); +} +void zebra_if_dplane_result(struct zebra_dplane_ctx *ctx) +{ + struct zebra_ns *zns; + struct interface *ifp; + ns_id_t ns_id; + enum dplane_op_e op; + enum zebra_dplane_result dp_res; + ifindex_t ifindex; + + ns_id = dplane_ctx_get_ns_id(ctx); + dp_res = dplane_ctx_get_status(ctx); + op = dplane_ctx_get_op(ctx); + ifindex = dplane_ctx_get_ifindex(ctx); + + if (IS_ZEBRA_DEBUG_DPLANE_DETAIL || IS_ZEBRA_DEBUG_KERNEL) + zlog_debug("Intf dplane ctx %p, op %s, ifindex (%u), result %s", + ctx, dplane_op2str(op), ifindex, + dplane_res2str(dp_res)); + + zns = zebra_ns_lookup(ns_id); + if (zns == NULL) { + /* No ns - deleted maybe? */ + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug("%s: can't find zns id %u", __func__, ns_id); + + goto done; + } + + ifp = if_lookup_by_index_per_ns(zns, ifindex); + if (ifp == NULL) { + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug("%s: can't find ifp at nsid %u index %d", + __func__, ns_id, ifindex); + + goto done; + } + + switch (op) { + case DPLANE_OP_INTF_ADDR_ADD: + case DPLANE_OP_INTF_ADDR_DEL: + zebra_if_addr_update_ctx(ctx, ifp); + break; + + case DPLANE_OP_INTF_INSTALL: + case DPLANE_OP_INTF_UPDATE: + case DPLANE_OP_INTF_DELETE: + zebra_if_update_ctx(ctx, ifp); + break; + + case DPLANE_OP_INTF_NETCONFIG: + zebra_if_netconf_update_ctx(ctx, ifp); + break; + + case DPLANE_OP_ROUTE_INSTALL: + case DPLANE_OP_ROUTE_UPDATE: + case DPLANE_OP_ROUTE_DELETE: + case DPLANE_OP_NH_DELETE: + case DPLANE_OP_NH_INSTALL: + case DPLANE_OP_NH_UPDATE: + case DPLANE_OP_ROUTE_NOTIFY: + case DPLANE_OP_LSP_INSTALL: + case DPLANE_OP_LSP_UPDATE: + case DPLANE_OP_LSP_DELETE: + case DPLANE_OP_LSP_NOTIFY: + case DPLANE_OP_PW_INSTALL: + case DPLANE_OP_PW_UNINSTALL: + case DPLANE_OP_SYS_ROUTE_ADD: + case DPLANE_OP_SYS_ROUTE_DELETE: + case DPLANE_OP_ADDR_INSTALL: + case DPLANE_OP_ADDR_UNINSTALL: + case DPLANE_OP_MAC_INSTALL: + case DPLANE_OP_MAC_DELETE: + case DPLANE_OP_NEIGH_INSTALL: + case DPLANE_OP_NEIGH_UPDATE: + case DPLANE_OP_NEIGH_DELETE: + case DPLANE_OP_NEIGH_IP_INSTALL: + case DPLANE_OP_NEIGH_IP_DELETE: + case DPLANE_OP_VTEP_ADD: + case DPLANE_OP_VTEP_DELETE: + case DPLANE_OP_RULE_ADD: + case DPLANE_OP_RULE_DELETE: + case DPLANE_OP_RULE_UPDATE: + case DPLANE_OP_NEIGH_DISCOVER: + case DPLANE_OP_BR_PORT_UPDATE: + case DPLANE_OP_NONE: + case DPLANE_OP_IPTABLE_ADD: + case DPLANE_OP_IPTABLE_DELETE: + case DPLANE_OP_IPSET_ADD: + case DPLANE_OP_IPSET_DELETE: + case DPLANE_OP_IPSET_ENTRY_ADD: + case DPLANE_OP_IPSET_ENTRY_DELETE: + case DPLANE_OP_NEIGH_TABLE_UPDATE: + case DPLANE_OP_GRE_SET: + break; /* should never hit here */ + } done: - /* Free the context */ dplane_ctx_fini(&ctx); - - return ret; } /* Dump if address information to vty. */ @@ -1651,8 +1782,8 @@ static void ifs_dump_brief_vty_json(json_object *json, struct vrf *vrf) } } -const char *zebra_protodown_rc_str(enum protodown_reasons protodown_rc, - char *pd_buf, uint32_t pd_buf_len) +const char *zebra_protodown_rc_str(uint32_t protodown_rc, char *pd_buf, + uint32_t pd_buf_len) { bool first = true; @@ -1660,6 +1791,14 @@ const char *zebra_protodown_rc_str(enum protodown_reasons protodown_rc, strlcat(pd_buf, "(", pd_buf_len); + if (protodown_rc & ZEBRA_PROTODOWN_EXTERNAL) { + if (first) + first = false; + else + strlcat(pd_buf, ",", pd_buf_len); + strlcat(pd_buf, "external", pd_buf_len); + } + if (protodown_rc & ZEBRA_PROTODOWN_EVPN_STARTUP_DELAY) { if (first) first = false; @@ -1669,11 +1808,27 @@ const char *zebra_protodown_rc_str(enum protodown_reasons protodown_rc, } if (protodown_rc & ZEBRA_PROTODOWN_EVPN_UPLINK_DOWN) { - if (!first) + if (first) + first = false; + else strlcat(pd_buf, ",", pd_buf_len); strlcat(pd_buf, "uplinks-down", pd_buf_len); } + if (protodown_rc & ZEBRA_PROTODOWN_VRRP) { + if (first) + first = false; + else + strlcat(pd_buf, ",", pd_buf_len); + strlcat(pd_buf, "vrrp", pd_buf_len); + } + + if (protodown_rc & ZEBRA_PROTODOWN_SHARP) { + if (!first) + strlcat(pd_buf, ",", pd_buf_len); + strlcat(pd_buf, "sharp", pd_buf_len); + } + strlcat(pd_buf, ")", pd_buf_len); return pd_buf; diff --git a/zebra/interface.h b/zebra/interface.h index 315a3170d8..2c3784ee15 100644 --- a/zebra/interface.h +++ b/zebra/interface.h @@ -403,7 +403,7 @@ struct zebra_if { * in the dataplane. This results in a carrier/L1 down on the * physical device. */ - enum protodown_reasons protodown_rc; + uint32_t protodown_rc; /* list of zebra_mac entries using this interface as destination */ struct list *mac_list; @@ -497,7 +497,8 @@ extern void if_handle_vrf_change(struct interface *ifp, vrf_id_t vrf_id); extern void zebra_if_update_link(struct interface *ifp, ifindex_t link_ifindex, ns_id_t ns_id); extern void zebra_if_update_all_links(struct zebra_ns *zns); -extern void zebra_if_set_protodown(struct interface *ifp, bool down); +extern int zebra_if_set_protodown(struct interface *ifp, bool down, + enum protodown_reasons new_reason); extern int if_ip_address_install(struct interface *ifp, struct prefix *prefix, const char *label, struct prefix *pp); extern int if_ipv6_address_install(struct interface *ifp, struct prefix *prefix, @@ -521,10 +522,9 @@ extern bool if_nhg_dependents_is_empty(const struct interface *ifp); extern void vrf_add_update(struct vrf *vrfp); extern void zebra_l2_map_slave_to_bond(struct zebra_if *zif, vrf_id_t vrf); extern void zebra_l2_unmap_slave_from_bond(struct zebra_if *zif); -extern const char *zebra_protodown_rc_str(enum protodown_reasons protodown_rc, - char *pd_buf, uint32_t pd_buf_len); -void zebra_if_addr_update_ctx(struct zebra_dplane_ctx *ctx); -int zebra_if_netconf_update_ctx(struct zebra_dplane_ctx *ctx); +extern const char *zebra_protodown_rc_str(uint32_t protodown_rc, char *pd_buf, + uint32_t pd_buf_len); +void zebra_if_dplane_result(struct zebra_dplane_ctx *ctx); #ifdef HAVE_PROC_NET_DEV extern void ifstat_update_proc(void); diff --git a/zebra/kernel_netlink.c b/zebra/kernel_netlink.c index d84b0c1325..60bce6e03f 100644 --- a/zebra/kernel_netlink.c +++ b/zebra/kernel_netlink.c @@ -94,6 +94,7 @@ static const struct message nlmsg_str[] = {{RTM_NEWROUTE, "RTM_NEWROUTE"}, {RTM_DELROUTE, "RTM_DELROUTE"}, {RTM_GETROUTE, "RTM_GETROUTE"}, {RTM_NEWLINK, "RTM_NEWLINK"}, + {RTM_SETLINK, "RTM_SETLINK"}, {RTM_DELLINK, "RTM_DELLINK"}, {RTM_GETLINK, "RTM_GETLINK"}, {RTM_NEWADDR, "RTM_NEWADDR"}, @@ -1491,6 +1492,11 @@ static enum netlink_msg_status nl_put_msg(struct nl_batch *bth, case DPLANE_OP_INTF_NETCONFIG: case DPLANE_OP_NONE: return FRR_NETLINK_ERROR; + + case DPLANE_OP_INTF_INSTALL: + case DPLANE_OP_INTF_UPDATE: + case DPLANE_OP_INTF_DELETE: + return netlink_put_intf_update_msg(bth, ctx); } return FRR_NETLINK_ERROR; diff --git a/zebra/rt_netlink.h b/zebra/rt_netlink.h index 0a79771708..b1af4b20e1 100644 --- a/zebra/rt_netlink.h +++ b/zebra/rt_netlink.h @@ -128,6 +128,8 @@ const char *af_type2str(int type); const char *ifi_type2str(int type); const char *rta_type2str(int type); const char *rtm_type2str(int type); +const char *ifla_pdr_type2str(int type); +const char *ifla_info_type2str(int type); const char *rtm_protocol2str(int type); const char *rtm_scope2str(int type); const char *rtm_rta2str(int type); diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index 1b6f37ec6a..e94aee5c1a 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -1487,6 +1487,7 @@ static void zread_interface_set_protodown(ZAPI_HANDLER_ARGS) ifindex_t ifindex; struct interface *ifp; char down; + enum protodown_reasons reason; STREAM_GETL(msg, ifindex); STREAM_GETC(msg, down); @@ -1494,16 +1495,27 @@ static void zread_interface_set_protodown(ZAPI_HANDLER_ARGS) /* set ifdown */ ifp = if_lookup_by_index_per_ns(zebra_ns_lookup(NS_DEFAULT), ifindex); - if (ifp) { - zlog_info("Setting interface %s (%u): protodown %s", ifp->name, - ifindex, down ? "on" : "off"); - zebra_if_set_protodown(ifp, down); - } else { + if (!ifp) { zlog_warn( "Cannot set protodown %s for interface %u; does not exist", down ? "on" : "off", ifindex); + + return; } + switch (client->proto) { + case ZEBRA_ROUTE_VRRP: + reason = ZEBRA_PROTODOWN_VRRP; + break; + case ZEBRA_ROUTE_SHARP: + reason = ZEBRA_PROTODOWN_SHARP; + break; + default: + reason = 0; + break; + } + + zebra_if_set_protodown(ifp, down, reason); stream_failure: return; diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c index 6de2be3ab8..abafa6f8fe 100644 --- a/zebra/zebra_dplane.c +++ b/zebra/zebra_dplane.c @@ -192,6 +192,9 @@ struct dplane_intf_info { uint32_t metric; uint32_t flags; + uint32_t r_bitfield; + + bool protodown; #define DPLANE_INTF_CONNECTED (1 << 0) /* Connected peer, p2p */ #define DPLANE_INTF_SECONDARY (1 << 1) @@ -526,6 +529,9 @@ static struct zebra_dplane_globals { _Atomic uint32_t dg_gre_set_in; _Atomic uint32_t dg_gre_set_errors; + _Atomic uint32_t dg_intfs_in; + _Atomic uint32_t dg_intf_errors; + /* Dataplane pthread */ struct frr_pthread *dg_pthread; @@ -760,6 +766,9 @@ static void dplane_ctx_free_internal(struct zebra_dplane_ctx *ctx) case DPLANE_OP_NONE: case DPLANE_OP_IPSET_ADD: case DPLANE_OP_IPSET_DELETE: + case DPLANE_OP_INTF_INSTALL: + case DPLANE_OP_INTF_UPDATE: + case DPLANE_OP_INTF_DELETE: break; case DPLANE_OP_IPSET_ENTRY_ADD: @@ -1073,6 +1082,16 @@ const char *dplane_op2str(enum dplane_op_e op) case DPLANE_OP_INTF_NETCONFIG: return "INTF_NETCONFIG"; + + case DPLANE_OP_INTF_INSTALL: + ret = "INTF_INSTALL"; + break; + case DPLANE_OP_INTF_UPDATE: + ret = "INTF_UPDATE"; + break; + case DPLANE_OP_INTF_DELETE: + ret = "INTF_DELETE"; + break; } return ret; @@ -1771,6 +1790,28 @@ void dplane_ctx_set_intf_metric(struct zebra_dplane_ctx *ctx, uint32_t metric) ctx->u.intf.metric = metric; } +uint32_t dplane_ctx_get_intf_r_bitfield(const struct zebra_dplane_ctx *ctx) +{ + DPLANE_CTX_VALID(ctx); + + return ctx->u.intf.r_bitfield; +} + +void dplane_ctx_set_intf_r_bitfield(struct zebra_dplane_ctx *ctx, + uint32_t r_bitfield) +{ + DPLANE_CTX_VALID(ctx); + + ctx->u.intf.r_bitfield = r_bitfield; +} + +bool dplane_ctx_intf_is_protodown(const struct zebra_dplane_ctx *ctx) +{ + DPLANE_CTX_VALID(ctx); + + return ctx->u.intf.protodown; +} + /* Is interface addr p2p? */ bool dplane_ctx_intf_is_connected(const struct zebra_dplane_ctx *ctx) { @@ -2638,6 +2679,57 @@ done: return ret; } +/** + * dplane_ctx_intf_init() - Initialize a context block for a inteface update + * + * @ctx: Dataplane context to init + * @op: Operation being performed + * @ifp: Interface + * + * Return: Result status + */ +int dplane_ctx_intf_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op, + const struct interface *ifp) +{ + struct zebra_ns *zns = NULL; + struct zebra_if *zif = NULL; + int ret = EINVAL; + + if (!ctx || !ifp) + goto done; + + ctx->zd_op = op; + ctx->zd_status = ZEBRA_DPLANE_REQUEST_SUCCESS; + ctx->zd_vrf_id = ifp->vrf->vrf_id; + + strlcpy(ctx->zd_ifname, ifp->name, sizeof(ctx->zd_ifname)); + ctx->zd_ifindex = ifp->ifindex; + + zns = zebra_ns_lookup(ifp->vrf->vrf_id); + dplane_ctx_ns_init(ctx, zns, false); + + + /* Copy over ifp info */ + ctx->u.intf.metric = ifp->metric; + ctx->u.intf.flags = ifp->flags; + + /* Copy over extra zebra info, if available */ + zif = (struct zebra_if *)ifp->info; + + if (zif) { + ctx->u.intf.r_bitfield = zif->protodown_rc; + ctx->u.intf.protodown = !!(zif->flags & ZIF_FLAG_PROTODOWN); + } + + dplane_ctx_ns_init(ctx, zns, (op == DPLANE_OP_INTF_UPDATE)); + ctx->zd_is_update = (op == DPLANE_OP_INTF_UPDATE); + + ret = AOK; + +done: + return ret; +} + /* * Capture information for an LSP update in a dplane context. */ @@ -3824,6 +3916,85 @@ static enum zebra_dplane_result intf_addr_update_internal( return result; } +/** + * dplane_intf_update_internal() - Helper for enqueuing interface changes + * + * @ifp: Interface where the change occured + * @op: The operation to be enqued + * + * Return: Result of the change + */ +static enum zebra_dplane_result +dplane_intf_update_internal(const struct interface *ifp, enum dplane_op_e op) +{ + enum zebra_dplane_result result = ZEBRA_DPLANE_REQUEST_FAILURE; + int ret = EINVAL; + struct zebra_dplane_ctx *ctx = NULL; + + /* Obtain context block */ + ctx = dplane_ctx_alloc(); + if (!ctx) { + ret = ENOMEM; + goto done; + } + + ret = dplane_ctx_intf_init(ctx, op, ifp); + if (ret == AOK) + ret = dplane_update_enqueue(ctx); + +done: + /* Update counter */ + atomic_fetch_add_explicit(&zdplane_info.dg_intfs_in, 1, + memory_order_relaxed); + + if (ret == AOK) + result = ZEBRA_DPLANE_REQUEST_QUEUED; + else { + atomic_fetch_add_explicit(&zdplane_info.dg_intf_errors, 1, + memory_order_relaxed); + if (ctx) + dplane_ctx_free(&ctx); + } + + return result; +} + +/* + * Enqueue a interface add for the dataplane. + */ +enum zebra_dplane_result dplane_intf_add(const struct interface *ifp) +{ + enum zebra_dplane_result ret = ZEBRA_DPLANE_REQUEST_FAILURE; + + if (ifp) + ret = dplane_intf_update_internal(ifp, DPLANE_OP_INTF_INSTALL); + return ret; +} + +/* + * Enqueue a interface update for the dataplane. + */ +enum zebra_dplane_result dplane_intf_update(const struct interface *ifp) +{ + enum zebra_dplane_result ret = ZEBRA_DPLANE_REQUEST_FAILURE; + + if (ifp) + ret = dplane_intf_update_internal(ifp, DPLANE_OP_INTF_UPDATE); + return ret; +} + +/* + * Enqueue a interface delete for the dataplane. + */ +enum zebra_dplane_result dplane_intf_delete(const struct interface *ifp) +{ + enum zebra_dplane_result ret = ZEBRA_DPLANE_REQUEST_FAILURE; + + if (ifp) + ret = dplane_intf_update_internal(ifp, DPLANE_OP_INTF_DELETE); + return ret; +} + /* * Enqueue vxlan/evpn mac add (or update). */ @@ -5241,6 +5412,15 @@ static void kernel_dplane_log_detail(struct zebra_dplane_ctx *ctx) dplane_ctx_get_netconf_mpls(ctx), dplane_ctx_get_netconf_mcast(ctx)); break; + + case DPLANE_OP_INTF_INSTALL: + case DPLANE_OP_INTF_UPDATE: + case DPLANE_OP_INTF_DELETE: + zlog_debug("Dplane intf %s, idx %u, protodown %d", + dplane_op2str(dplane_ctx_get_op(ctx)), + dplane_ctx_get_ifindex(ctx), + dplane_ctx_intf_is_protodown(ctx)); + break; } } @@ -5375,6 +5555,15 @@ static void kernel_dplane_handle_result(struct zebra_dplane_ctx *ctx) &zdplane_info.dg_gre_set_errors, 1, memory_order_relaxed); break; + + case DPLANE_OP_INTF_INSTALL: + case DPLANE_OP_INTF_UPDATE: + case DPLANE_OP_INTF_DELETE: + if (res != ZEBRA_DPLANE_REQUEST_SUCCESS) + atomic_fetch_add_explicit(&zdplane_info.dg_intf_errors, + 1, memory_order_relaxed); + break; + /* Ignore 'notifications' - no-op */ case DPLANE_OP_SYS_ROUTE_ADD: case DPLANE_OP_SYS_ROUTE_DELETE: diff --git a/zebra/zebra_dplane.h b/zebra/zebra_dplane.h index 29555d5b56..73acf03411 100644 --- a/zebra/zebra_dplane.h +++ b/zebra/zebra_dplane.h @@ -188,6 +188,11 @@ enum dplane_op_e { /* Incoming interface config events */ DPLANE_OP_INTF_NETCONFIG, + + /* Interface update */ + DPLANE_OP_INTF_INSTALL, + DPLANE_OP_INTF_UPDATE, + DPLANE_OP_INTF_DELETE, }; /* @@ -480,6 +485,10 @@ dplane_ctx_get_pw_backup_nhg(const struct zebra_dplane_ctx *ctx); /* Accessors for interface information */ uint32_t dplane_ctx_get_intf_metric(const struct zebra_dplane_ctx *ctx); void dplane_ctx_set_intf_metric(struct zebra_dplane_ctx *ctx, uint32_t metric); +uint32_t dplane_ctx_get_intf_r_bitfield(const struct zebra_dplane_ctx *ctx); +void dplane_ctx_set_intf_r_bitfield(struct zebra_dplane_ctx *ctx, + uint32_t r_bitfield); +bool dplane_ctx_intf_is_protodown(const struct zebra_dplane_ctx *ctx); /* Is interface addr p2p? */ bool dplane_ctx_intf_is_connected(const struct zebra_dplane_ctx *ctx); void dplane_ctx_intf_set_connected(struct zebra_dplane_ctx *ctx); @@ -676,6 +685,13 @@ enum zebra_dplane_result dplane_intf_addr_set(const struct interface *ifp, enum zebra_dplane_result dplane_intf_addr_unset(const struct interface *ifp, const struct connected *ifc); +/* + * Enqueue interface link changes for the dataplane. + */ +enum zebra_dplane_result dplane_intf_add(const struct interface *ifp); +enum zebra_dplane_result dplane_intf_update(const struct interface *ifp); +enum zebra_dplane_result dplane_intf_delete(const struct interface *ifp); + /* * Link layer operations for the dataplane. */ @@ -814,6 +830,10 @@ int dplane_ctx_route_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op, int dplane_ctx_nexthop_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op, struct nhg_hash_entry *nhe); +/* Encode interface information into data plane context. */ +int dplane_ctx_intf_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op, + const struct interface *ifp); + /* Retrieve the limit on the number of pending, unprocessed updates. */ uint32_t dplane_get_in_queue_limit(void); diff --git a/zebra/zebra_errors.c b/zebra/zebra_errors.c index c3890f7220..7549a3d5c0 100644 --- a/zebra/zebra_errors.c +++ b/zebra/zebra_errors.c @@ -791,6 +791,15 @@ static struct log_ref ferr_zebra_err[] = { .description = "Zebra's srv6-locator chunk cleanup procedure ran, but no srv6 locator chunks were released.", .suggestion = "Ignore this error.", }, + { + .code = EC_ZEBRA_INTF_UPDATE_FAILURE, + .title = + "Zebra failed to update interface in the kernel", + .description = + "Zebra made an attempt to update an interfce in the kernel, but it was not successful.", + .suggestion = + "Wait for Zebra to reattempt update.", + }, { .code = END_FERR, } diff --git a/zebra/zebra_errors.h b/zebra/zebra_errors.h index 540c6dd7d0..5164de09d6 100644 --- a/zebra/zebra_errors.h +++ b/zebra/zebra_errors.h @@ -136,6 +136,7 @@ enum zebra_log_refs { EC_ZEBRA_ES_CREATE, EC_ZEBRA_GRE_SET_UPDATE, EC_ZEBRA_SRV6M_UNRELEASED_LOCATOR_CHUNK, + EC_ZEBRA_INTF_UPDATE_FAILURE, }; void zebra_error_init(void); diff --git a/zebra/zebra_evpn_mh.c b/zebra/zebra_evpn_mh.c index 50eecd31d5..463ede158c 100644 --- a/zebra/zebra_evpn_mh.c +++ b/zebra/zebra_evpn_mh.c @@ -3623,10 +3623,10 @@ bool zebra_evpn_is_es_bond_member(struct interface *ifp) void zebra_evpn_mh_update_protodown_bond_mbr(struct zebra_if *zif, bool clear, const char *caller) { - bool old_protodown; bool new_protodown; - enum protodown_reasons old_protodown_rc = 0; - enum protodown_reasons protodown_rc = 0; + uint32_t old_protodown_rc = 0; + uint32_t new_protodown_rc = 0; + uint32_t protodown_rc = 0; if (!clear) { struct zebra_if *bond_zif; @@ -3635,32 +3635,23 @@ void zebra_evpn_mh_update_protodown_bond_mbr(struct zebra_if *zif, bool clear, protodown_rc = bond_zif->protodown_rc; } - old_protodown = !!(zif->flags & ZIF_FLAG_PROTODOWN); old_protodown_rc = zif->protodown_rc; - zif->protodown_rc &= ~ZEBRA_PROTODOWN_EVPN_ALL; - zif->protodown_rc |= (protodown_rc & ZEBRA_PROTODOWN_EVPN_ALL); - new_protodown = !!zif->protodown_rc; + new_protodown_rc &= ~ZEBRA_PROTODOWN_EVPN_ALL; + new_protodown_rc |= (protodown_rc & ZEBRA_PROTODOWN_EVPN_ALL); + new_protodown = !!new_protodown_rc; - if (IS_ZEBRA_DEBUG_EVPN_MH_ES - && (zif->protodown_rc != old_protodown_rc)) + if (IS_ZEBRA_DEBUG_EVPN_MH_ES && (new_protodown_rc != old_protodown_rc)) zlog_debug( "%s bond mbr %s protodown_rc changed; old 0x%x new 0x%x", caller, zif->ifp->name, old_protodown_rc, - zif->protodown_rc); + new_protodown_rc); - if (old_protodown == new_protodown) - return; - - if (new_protodown) - zif->flags |= ZIF_FLAG_PROTODOWN; - else - zif->flags &= ~ZIF_FLAG_PROTODOWN; - - if (IS_ZEBRA_DEBUG_EVPN_MH_ES) - zlog_debug("%s protodown %s", zif->ifp->name, - new_protodown ? "on" : "off"); - - zebra_if_set_protodown(zif->ifp, new_protodown); + if (zebra_if_set_protodown(zif->ifp, new_protodown, new_protodown_rc) == + 0) { + if (IS_ZEBRA_DEBUG_EVPN_MH_ES) + zlog_debug("%s protodown %s", zif->ifp->name, + new_protodown ? "on" : "off"); + } } /* The bond members inherit the protodown reason code from the bond */ @@ -3683,7 +3674,7 @@ static void zebra_evpn_mh_update_protodown_es(struct zebra_evpn_es *es, bool resync_dplane) { struct zebra_if *zif; - enum protodown_reasons old_protodown_rc; + uint32_t old_protodown_rc; zif = es->zif; /* if the reason code is the same bail unless it is a new @@ -3714,7 +3705,7 @@ static void zebra_evpn_mh_update_protodown_es(struct zebra_evpn_es *es, static void zebra_evpn_mh_clear_protodown_es(struct zebra_evpn_es *es) { struct zebra_if *zif; - enum protodown_reasons old_protodown_rc; + uint32_t old_protodown_rc; zif = es->zif; if (!(zif->protodown_rc & ZEBRA_PROTODOWN_EVPN_ALL)) @@ -3742,10 +3733,9 @@ static void zebra_evpn_mh_update_protodown_es_all(void) zebra_evpn_mh_update_protodown_es(es, false /*resync_dplane*/); } -static void zebra_evpn_mh_update_protodown(enum protodown_reasons protodown_rc, - bool set) +static void zebra_evpn_mh_update_protodown(uint32_t protodown_rc, bool set) { - enum protodown_reasons old_protodown_rc = zmh_info->protodown_rc; + uint32_t old_protodown_rc = zmh_info->protodown_rc; if (set) { if ((protodown_rc & zmh_info->protodown_rc) == protodown_rc) diff --git a/zebra/zebra_evpn_mh.h b/zebra/zebra_evpn_mh.h index af6832092b..ce7b920de1 100644 --- a/zebra/zebra_evpn_mh.h +++ b/zebra/zebra_evpn_mh.h @@ -263,7 +263,7 @@ struct zebra_evpn_mh_info { uint32_t uplink_oper_up_cnt; /* These protodown bits are inherited by all ES bonds */ - enum protodown_reasons protodown_rc; + uint32_t protodown_rc; }; /* returns TRUE if the EVPN is ready to be sent to BGP */ diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index 469a94a65b..1b926dba5f 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -2993,6 +2993,9 @@ void zebra_nhg_dplane_result(struct zebra_dplane_ctx *ctx) case DPLANE_OP_INTF_ADDR_ADD: case DPLANE_OP_INTF_ADDR_DEL: case DPLANE_OP_INTF_NETCONFIG: + case DPLANE_OP_INTF_INSTALL: + case DPLANE_OP_INTF_UPDATE: + case DPLANE_OP_INTF_DELETE: break; } diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index e376d4b2af..c6840a503c 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -4318,11 +4318,11 @@ static void rib_process_dplane_results(struct thread *thread) case DPLANE_OP_INTF_ADDR_ADD: case DPLANE_OP_INTF_ADDR_DEL: - zebra_if_addr_update_ctx(ctx); - break; - + case DPLANE_OP_INTF_INSTALL: + case DPLANE_OP_INTF_UPDATE: + case DPLANE_OP_INTF_DELETE: case DPLANE_OP_INTF_NETCONFIG: - zebra_if_netconf_update_ctx(ctx); + zebra_if_dplane_result(ctx); break; /* Some op codes not handled here */ diff --git a/zebra/zebra_router.h b/zebra/zebra_router.h index 63a61d5293..40f7d87980 100644 --- a/zebra/zebra_router.h +++ b/zebra/zebra_router.h @@ -68,19 +68,24 @@ enum multicast_mode { * physical device. */ enum protodown_reasons { + /* A process outside of FRR's control protodowned the interface */ + ZEBRA_PROTODOWN_EXTERNAL = (1 << 0), /* On startup local ESs are held down for some time to * allow the underlay to converge and EVPN routes to * get learnt */ - ZEBRA_PROTODOWN_EVPN_STARTUP_DELAY = (1 << 0), + ZEBRA_PROTODOWN_EVPN_STARTUP_DELAY = (1 << 1), /* If all the uplinks are down the switch has lost access * to the VxLAN overlay and must shut down the access * ports to allow servers to re-direct their traffic to * other switches on the Ethernet Segment */ - ZEBRA_PROTODOWN_EVPN_UPLINK_DOWN = (1 << 1), - ZEBRA_PROTODOWN_EVPN_ALL = (ZEBRA_PROTODOWN_EVPN_UPLINK_DOWN - | ZEBRA_PROTODOWN_EVPN_STARTUP_DELAY) + ZEBRA_PROTODOWN_EVPN_UPLINK_DOWN = (1 << 2), + ZEBRA_PROTODOWN_EVPN_ALL = (ZEBRA_PROTODOWN_EVPN_UPLINK_DOWN | + ZEBRA_PROTODOWN_EVPN_STARTUP_DELAY), + ZEBRA_PROTODOWN_VRRP = (1 << 3), + /* This reason used exclusively for testing */ + ZEBRA_PROTODOWN_SHARP = (1 << 4) }; #define ZEBRA_PROTODOWN_RC_STR_LEN 80 From f9a1140c9a512972bd9d2ebe1d650f29e93b7328 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Wed, 26 Jan 2022 00:23:07 -0500 Subject: [PATCH 03/24] sharpd: add support for setting protodown Add support for setting protodown via sharpd. This is just useful for testing. We can make use of this in topotests. Signed-off-by: Stephen Worley --- sharpd/sharp_vty.c | 64 ++++++++++++++++++++++++++++++++++++++++++++ sharpd/sharp_zebra.c | 12 +++++++++ sharpd/sharp_zebra.h | 2 ++ 3 files changed, 78 insertions(+) diff --git a/sharpd/sharp_vty.c b/sharpd/sharp_vty.c index 78cc57cc44..889643f65e 100644 --- a/sharpd/sharp_vty.c +++ b/sharpd/sharp_vty.c @@ -1258,6 +1258,67 @@ DEFPY (show_sharp_cspf, return CMD_SUCCESS; } +static struct interface *if_lookup_vrf_all(const char *ifname) +{ + struct interface *ifp; + struct vrf *vrf; + + RB_FOREACH(vrf, vrf_name_head, &vrfs_by_name) { + ifp = if_lookup_by_name(ifname, vrf->vrf_id); + if (ifp) + return ifp; + } + + return NULL; +} + +DEFPY (sharp_interface_protodown, + sharp_interface_protodown_cmd, + "sharp interface IFNAME$ifname protodown", + SHARP_STR + INTERFACE_STR + IFNAME_STR + "Set interface protodown\n") +{ + struct interface *ifp; + + ifp = if_lookup_vrf_all(ifname); + + if (!ifp) { + vty_out(vty, "%% Can't find interface %s\n", ifname); + return CMD_WARNING; + } + + if (sharp_zebra_send_interface_protodown(ifp, true) != 0) + return CMD_WARNING; + + return CMD_SUCCESS; +} + +DEFPY (no_sharp_interface_protodown, + no_sharp_interface_protodown_cmd, + "no sharp interface IFNAME$ifname protodown", + NO_STR + SHARP_STR + INTERFACE_STR + IFNAME_STR + "Set interface protodown\n") +{ + struct interface *ifp; + + ifp = if_lookup_vrf_all(ifname); + + if (!ifp) { + vty_out(vty, "%% Can't find interface %s\n", ifname); + return CMD_WARNING; + } + + if (sharp_zebra_send_interface_protodown(ifp, false) != 0) + return CMD_WARNING; + + return CMD_SUCCESS; +} + void sharp_vty_init(void) { install_element(ENABLE_NODE, &install_routes_data_dump_cmd); @@ -1290,5 +1351,8 @@ void sharp_vty_init(void) &sharp_srv6_manager_release_locator_chunk_cmd); install_element(ENABLE_NODE, &show_sharp_segment_routing_srv6_cmd); + install_element(ENABLE_NODE, &sharp_interface_protodown_cmd); + install_element(ENABLE_NODE, &no_sharp_interface_protodown_cmd); + return; } diff --git a/sharpd/sharp_zebra.c b/sharpd/sharp_zebra.c index 313febd9bb..e33dbeb97c 100644 --- a/sharpd/sharp_zebra.c +++ b/sharpd/sharp_zebra.c @@ -968,6 +968,18 @@ static int sharp_zebra_process_srv6_locator_chunk(ZAPI_CALLBACK_ARGS) return 0; } +int sharp_zebra_send_interface_protodown(struct interface *ifp, bool down) +{ + zlog_debug("Sending zebra to set %s protodown %s", ifp->name, + down ? "on" : "off"); + + if (zclient_send_interface_protodown(zclient, ifp->vrf->vrf_id, ifp, + down) == ZCLIENT_SEND_FAILURE) + return -1; + + return 0; +} + static zclient_handler *const sharp_handlers[] = { [ZEBRA_INTERFACE_ADDRESS_ADD] = interface_address_add, [ZEBRA_INTERFACE_ADDRESS_DELETE] = interface_address_delete, diff --git a/sharpd/sharp_zebra.h b/sharpd/sharp_zebra.h index 49f11a67e8..d8ea679797 100644 --- a/sharpd/sharp_zebra.h +++ b/sharpd/sharp_zebra.h @@ -73,4 +73,6 @@ extern void sharp_install_seg6local_route_helper(struct prefix *p, enum seg6local_action_t act, struct seg6local_context *ctx); +extern int sharp_zebra_send_interface_protodown(struct interface *ifp, + bool down); #endif From c40e1b1cfbbf8db7b03f9faa51afbea8cda722c3 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Wed, 19 Jan 2022 14:36:10 -0500 Subject: [PATCH 04/24] zebra: add command for setting protodown bit Add command for use to set protodown via frr.conf in the case our default conflicts with another application they are using. Signed-off-by: Stephen Worley --- doc/user/zebra.rst | 11 +++++++++++ zebra/if_netlink.c | 30 ++++++++++++++++++++++++++++++ zebra/if_netlink.h | 10 ++++++++++ zebra/kernel_netlink.c | 4 ++++ zebra/zebra_vty.c | 28 ++++++++++++++++++++++++++++ 5 files changed, 83 insertions(+) diff --git a/doc/user/zebra.rst b/doc/user/zebra.rst index 221e9c6fe2..0244f7c583 100644 --- a/doc/user/zebra.rst +++ b/doc/user/zebra.rst @@ -255,6 +255,17 @@ Link Parameters Commands for InterASv2 link in OSPF (RFC5392). Note that this option is not yet supported for ISIS (RFC5316). +Global Commands +------------------------ + +.. clicmd:: zebra protodown reason-bit (0-31) + + This command is only supported for linux and a kernel > 5.1. + Change reason-bit frr uses for setting protodown. We default to 7, but + if another userspace app ever conflicts with this, you can change it here. + The descriptor for this bit should exist in :file:`/etc/iproute2/protodown_reasons.d/` + to display with :clicmd:`ip -d link show`. + Nexthop Tracking ================ diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index 748f239db9..71a26c8d57 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -2214,4 +2214,34 @@ void interface_list(struct zebra_ns *zns) interface_addr_lookup_netlink(zns); } +void if_netlink_set_frr_protodown_r_bit(uint8_t bit) +{ + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug("FRR protodown reason bit change %u -> %u", + frr_protodown_r_bit, bit); + + frr_protodown_r_bit = bit; +} + +void if_netlink_unset_frr_protodown_r_bit(void) +{ + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug("FRR protodown reason bit change %u -> %u", + frr_protodown_r_bit, + FRR_PROTODOWN_REASON_DEFAULT_BIT); + + frr_protodown_r_bit = FRR_PROTODOWN_REASON_DEFAULT_BIT; +} + + +bool if_netlink_frr_protodown_r_bit_is_set(void) +{ + return (frr_protodown_r_bit != FRR_PROTODOWN_REASON_DEFAULT_BIT); +} + +uint8_t if_netlink_get_frr_protodown_r_bit(void) +{ + return frr_protodown_r_bit; +} + #endif /* GNU_LINUX */ diff --git a/zebra/if_netlink.h b/zebra/if_netlink.h index d5a73bb467..3d9e934fb0 100644 --- a/zebra/if_netlink.h +++ b/zebra/if_netlink.h @@ -72,6 +72,16 @@ netlink_put_intf_update_msg(struct nl_batch *bth, struct zebra_dplane_ctx *ctx); */ int netlink_protodown(struct interface *ifp, bool down, uint32_t r_bitfield); +/* Protodown bit setter/getter + * + * Allow users to change the bit if it conflicts with another + * on their system. + */ +extern void if_netlink_set_frr_protodown_r_bit(uint8_t bit); +extern void if_netlink_unset_frr_protodown_r_bit(void); +extern bool if_netlink_frr_protodown_r_bit_is_set(void); +extern uint8_t if_netlink_get_frr_protodown_r_bit(void); + #ifdef __cplusplus } #endif diff --git a/zebra/kernel_netlink.c b/zebra/kernel_netlink.c index 60bce6e03f..0dd76e3253 100644 --- a/zebra/kernel_netlink.c +++ b/zebra/kernel_netlink.c @@ -210,6 +210,10 @@ int netlink_config_write_helper(struct vty *vty) vty_out(vty, "zebra kernel netlink batch-tx-buf %u %u\n", size, threshold); + if (if_netlink_frr_protodown_r_bit_is_set()) + vty_out(vty, "zebra protodown reason-bit %u\n", + if_netlink_get_frr_protodown_r_bit()); + return 0; } diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c index bb16232118..6b155b8812 100644 --- a/zebra/zebra_vty.c +++ b/zebra/zebra_vty.c @@ -60,6 +60,7 @@ #include "northbound_cli.h" #include "zebra/zebra_nb.h" #include "zebra/kernel_netlink.h" +#include "zebra/if_netlink.h" #include "zebra/table_manager.h" #include "zebra/zebra_script.h" #include "zebra/rtadv.h" @@ -4356,6 +4357,31 @@ DEFUN_HIDDEN(no_zebra_kernel_netlink_batch_tx_buf, return CMD_SUCCESS; } +DEFPY (zebra_protodown_bit, + zebra_protodown_bit_cmd, + "zebra protodown reason-bit (0-31)$bit", + ZEBRA_STR + "Protodown Configuration\n" + "Reason Bit used in the kernel for application\n" + "Reason Bit range\n") +{ + if_netlink_set_frr_protodown_r_bit(bit); + return CMD_SUCCESS; +} + +DEFPY (no_zebra_protodown_bit, + no_zebra_protodown_bit_cmd, + "no zebra protodown reason-bit [(0-31)$bit]", + NO_STR + ZEBRA_STR + "Protodown Configuration\n" + "Reason Bit used in the kernel for setting protodown\n" + "Reason Bit Range\n") +{ + if_netlink_unset_frr_protodown_r_bit(); + return CMD_SUCCESS; +} + #endif /* HAVE_NETLINK */ DEFUN(ip_table_range, ip_table_range_cmd, @@ -4561,6 +4587,8 @@ void zebra_vty_init(void) #ifdef HAVE_NETLINK install_element(CONFIG_NODE, &zebra_kernel_netlink_batch_tx_buf_cmd); install_element(CONFIG_NODE, &no_zebra_kernel_netlink_batch_tx_buf_cmd); + install_element(CONFIG_NODE, &zebra_protodown_bit_cmd); + install_element(CONFIG_NODE, &no_zebra_protodown_bit_cmd); #endif /* HAVE_NETLINK */ #ifdef HAVE_SCRIPTING From 71ef5cbb9563e09a76996448a7f34cec37ed3c15 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Fri, 21 Jan 2022 04:03:01 -0500 Subject: [PATCH 05/24] zebra: add enum set/unset states for queing Add enums for set/unset of prodown state to handle the mainthread knowing an update is already queued without actually marking it as complete. This is to make the logic confirm a bit more with other parts of the code where we queue dplane updates and not update our internal structs until success callback is received. Signed-off-by: Stephen Worley --- zebra/interface.c | 86 +++++++++++++++++++++++++++++++++++--------- zebra/interface.h | 6 +++- zebra/zebra_dplane.c | 17 ++++++++- 3 files changed, 90 insertions(+), 19 deletions(-) diff --git a/zebra/interface.c b/zebra/interface.c index d326109c19..23ff91faa3 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -1229,34 +1229,78 @@ void zebra_if_update_all_links(struct zebra_ns *zns) } } +static bool if_ignore_set_protodown(const struct interface *ifp, bool new_down, + uint32_t new_protodown_rc) +{ + struct zebra_if *zif; + bool old_down, old_set_down, old_unset_down; + + zif = ifp->info; + + /* Current state as we know it */ + old_down = !!(zif->flags & ZIF_FLAG_PROTODOWN); + old_set_down = !!(zif->flags & ZIF_FLAG_SET_PROTODOWN); + old_unset_down = !!(zif->flags & ZIF_FLAG_UNSET_PROTODOWN); + + if (new_protodown_rc == zif->protodown_rc) { + /* Early return if already down & reason bitfield matches */ + if (new_down == old_down) { + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug( + "Ignoring %s (%u): protodown %s already set reason: old 0x%x new 0x%x", + ifp->name, ifp->ifindex, + new_down ? "on" : "off", + zif->protodown_rc, new_protodown_rc); + + return true; + } + + /* Early return if already set queued & reason bitfield matches + */ + if (new_down && old_set_down) { + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug( + "Ignoring %s (%u): protodown %s queued to dplane already reason: old 0x%x new 0x%x", + ifp->name, ifp->ifindex, + new_down ? "on" : "off", + zif->protodown_rc, new_protodown_rc); + + return true; + } + + /* Early return if already unset queued & reason bitfield + * matches */ + if (!new_down && old_unset_down) { + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug( + "Ignoring %s (%u): protodown %s queued to dplane already reason: old 0x%x new 0x%x", + ifp->name, ifp->ifindex, + new_down ? "on" : "off", + zif->protodown_rc, new_protodown_rc); + + return true; + } + } + + return false; +} + int zebra_if_set_protodown(struct interface *ifp, bool new_down, enum protodown_reasons new_reason) { struct zebra_if *zif; - bool old_down; uint32_t new_protodown_rc; zif = ifp->info; - old_down = !!(zif->flags & ZIF_FLAG_PROTODOWN); - if (new_down) new_protodown_rc = zif->protodown_rc | new_reason; else new_protodown_rc = zif->protodown_rc & ~new_reason; - /* Early return if already set down & reason bitfield matches */ - if ((new_down == old_down) && (new_protodown_rc == zif->protodown_rc)) { - - if (IS_ZEBRA_DEBUG_KERNEL) { - zlog_debug( - "Ignoring %s (%u): protodown %s already set reason: old 0x%x new 0x%x", - ifp->name, ifp->ifindex, - new_down ? "on" : "off", zif->protodown_rc, - new_protodown_rc); - return 1; - } - } + /* Check if we already have this state or it's queued */ + if (if_ignore_set_protodown(ifp, new_down, new_protodown_rc)) + return 1; zlog_info( "Setting interface %s (%u): protodown %s reason: old 0x%x new 0x%x", @@ -1265,6 +1309,11 @@ int zebra_if_set_protodown(struct interface *ifp, bool new_down, zif->protodown_rc = new_protodown_rc; + if (new_down) + zif->flags |= ZIF_FLAG_SET_PROTODOWN; + else + zif->flags |= ZIF_FLAG_UNSET_PROTODOWN; + #ifdef HAVE_NETLINK // TODO: remove this as separate commit // netlink_protodown(ifp, new_down, zif->protodown_rc); @@ -1386,10 +1435,13 @@ static void zebra_if_update_ctx(struct zebra_dplane_ctx *ctx, } /* Update our info */ - if (down) + if (down) { zif->flags |= ZIF_FLAG_PROTODOWN; - else + zif->flags &= ~ZIF_FLAG_SET_PROTODOWN; + } else { zif->flags &= ~ZIF_FLAG_PROTODOWN; + zif->flags &= ~ZIF_FLAG_UNSET_PROTODOWN; + } } /* diff --git a/zebra/interface.h b/zebra/interface.h index 2c3784ee15..70a5581a88 100644 --- a/zebra/interface.h +++ b/zebra/interface.h @@ -308,12 +308,16 @@ enum zebra_if_flags { /* Dataplane protodown-on */ ZIF_FLAG_PROTODOWN = (1 << 2), + /* Dataplane protodown-on Queued to the dplane */ + ZIF_FLAG_SET_PROTODOWN = (1 << 3), + /* Dataplane protodown-off Queued to the dplane */ + ZIF_FLAG_UNSET_PROTODOWN = (1 << 4), /* LACP bypass state is set by the dataplane on a bond member * and inherited by the bond (if one or more bond members are in * a bypass state the bond is placed in a bypass state) */ - ZIF_FLAG_LACP_BYPASS = (1 << 3) + ZIF_FLAG_LACP_BYPASS = (1 << 5) }; /* `zebra' daemon local interface structure. */ diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c index abafa6f8fe..f60951cac4 100644 --- a/zebra/zebra_dplane.c +++ b/zebra/zebra_dplane.c @@ -2694,6 +2694,7 @@ int dplane_ctx_intf_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op, struct zebra_ns *zns = NULL; struct zebra_if *zif = NULL; int ret = EINVAL; + bool set_pdown, unset_pdown; if (!ctx || !ifp) goto done; @@ -2717,8 +2718,22 @@ int dplane_ctx_intf_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op, zif = (struct zebra_if *)ifp->info; if (zif) { + set_pdown = !!(zif->flags & ZIF_FLAG_SET_PROTODOWN); + unset_pdown = !!(zif->flags & ZIF_FLAG_UNSET_PROTODOWN); + ctx->u.intf.r_bitfield = zif->protodown_rc; - ctx->u.intf.protodown = !!(zif->flags & ZIF_FLAG_PROTODOWN); + + /* + * See if we have new protodown state to set, otherwise keep + * current state + */ + if (set_pdown) + ctx->u.intf.protodown = true; + else if (unset_pdown) + ctx->u.intf.protodown = false; + else + ctx->u.intf.protodown = + !!(zif->flags & ZIF_FLAG_PROTODOWN); } dplane_ctx_ns_init(ctx, zns, (op == DPLANE_OP_INTF_UPDATE)); From 0dcd8506f2dac65bcac06f79a1660d809b329340 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 25 Jan 2022 13:49:05 -0500 Subject: [PATCH 06/24] zebra: clear protodown_rc on shutdown and sweep Add functionality to clear any reason code set on shutdown of zebra when we are freeing the interface, in case a bad client didn't tell us to clear it when the shutdown. Also, in case of a crash or failure to do the above, clear reason on startup if it is set. Signed-off-by: Stephen Worley --- zebra/if_netlink.c | 137 ++++++++++++++++++++++++++++--------------- zebra/interface.c | 6 ++ zebra/interface.h | 4 ++ zebra/zebra_router.h | 5 +- 4 files changed, 104 insertions(+), 48 deletions(-) diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index 71a26c8d57..f26d52332b 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -815,39 +815,73 @@ static int netlink_bridge_interface(struct nlmsghdr *h, int len, ns_id_t ns_id, return 0; } -/* If the interface is an es bond member then it must follow EVPN's - * protodown setting +static bool is_if_protodown_r_only_frr(uint32_t rc_bitfield) +{ + /* This shouldn't be possible */ + assert(frr_protodown_r_bit < 32); + return (rc_bitfield == (((uint32_t)1) << frr_protodown_r_bit)); +} + +/* + * Process interface protodown dplane update. + * + * If the interface is an es bond member then it must follow EVPN's + * protodown setting. */ static void netlink_proc_dplane_if_protodown(struct zebra_if *zif, - bool protodown) + struct rtattr **tb) { - bool zif_protodown; + bool protodown; + bool old_protodown; + uint32_t rc_bitfield = 0; + struct rtattr *pd_reason_info[IFLA_MAX + 1]; - /* Set our reason code to note it wasn't us */ - if (protodown) + protodown = !!*(uint8_t *)RTA_DATA(tb[IFLA_PROTO_DOWN]); + + if (tb[IFLA_PROTO_DOWN_REASON]) { + netlink_parse_rtattr_nested(pd_reason_info, IFLA_INFO_MAX, + tb[IFLA_PROTO_DOWN_REASON]); + + if (pd_reason_info[IFLA_PROTO_DOWN_REASON_VALUE]) + rc_bitfield = *(uint32_t *)RTA_DATA( + pd_reason_info[IFLA_PROTO_DOWN_REASON_VALUE]); + } + + /* + * Set our reason code to note it wasn't us. + * If the reason we got from the kernel is ONLY frr though, don't + * set it. + */ + if (protodown && is_if_protodown_r_only_frr(rc_bitfield) == false) zif->protodown_rc |= ZEBRA_PROTODOWN_EXTERNAL; else zif->protodown_rc &= ~ZEBRA_PROTODOWN_EXTERNAL; - zif_protodown = !!(zif->flags & ZIF_FLAG_PROTODOWN); - if (protodown == zif_protodown) + old_protodown = !!(zif->flags & ZIF_FLAG_PROTODOWN); + if (protodown == old_protodown) return; if (IS_ZEBRA_DEBUG_EVPN_MH_ES || IS_ZEBRA_DEBUG_KERNEL) zlog_debug("interface %s dplane change, protdown %s", zif->ifp->name, protodown ? "on" : "off"); + if (protodown) + zif->flags |= ZIF_FLAG_PROTODOWN; + else + zif->flags &= ~ZIF_FLAG_PROTODOWN; + if (zebra_evpn_is_es_bond_member(zif->ifp)) { if (IS_ZEBRA_DEBUG_EVPN_MH_ES || IS_ZEBRA_DEBUG_KERNEL) zlog_debug( "bond mbr %s re-instate protdown %s in the dplane", - zif->ifp->name, zif_protodown ? "on" : "off"); - netlink_protodown(zif->ifp, zif_protodown, zif->protodown_rc); - } else { - if (protodown) - zif->flags |= ZIF_FLAG_PROTODOWN; + zif->ifp->name, old_protodown ? "on" : "off"); + + if (old_protodown) + zif->flags |= ZIF_FLAG_SET_PROTODOWN; else - zif->flags &= ~ZIF_FLAG_PROTODOWN; + zif->flags |= ZIF_FLAG_UNSET_PROTODOWN; + + dplane_intf_update(zif->ifp); } } @@ -865,6 +899,28 @@ static uint8_t netlink_parse_lacp_bypass(struct rtattr **linkinfo) return bypass; } +/* + * Only called at startup to cleanup leftover protodown we may have not cleanup + */ +static void if_sweep_protodown(struct zebra_if *zif) +{ + bool protodown; + + protodown = !!(zif->flags & ZIF_FLAG_PROTODOWN); + + if (!protodown) + return; + + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug("interface %s sweeping protdown %s", zif->ifp->name, + protodown ? "on" : "off"); + + /* Only clear our reason codes, leave external if it was set */ + zif->protodown_rc |= ~ZEBRA_PROTODOWN_ALL; + zif->flags |= ZIF_FLAG_UNSET_PROTODOWN; + dplane_intf_update(zif->ifp); +} + /* * Called from interface_lookup_netlink(). This function is only used * during bootstrap. @@ -912,7 +968,8 @@ static int netlink_interface(struct nlmsghdr *h, ns_id_t ns_id, int startup) /* Looking up interface name. */ memset(linkinfo, 0, sizeof(linkinfo)); - netlink_parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len); + netlink_parse_rtattr_flags(tb, IFLA_MAX, IFLA_RTA(ifi), len, + NLA_F_NESTED); /* check for wireless messages to ignore */ if ((tb[IFLA_WIRELESS] != NULL) && (ifi->ifi_change == 0)) { @@ -1027,10 +1084,8 @@ static int netlink_interface(struct nlmsghdr *h, ns_id_t ns_id, int startup) zebra_l2if_update_bond_slave(ifp, bond_ifindex, !!bypass); if (tb[IFLA_PROTO_DOWN]) { - uint8_t protodown; - - protodown = *(uint8_t *)RTA_DATA(tb[IFLA_PROTO_DOWN]); - netlink_proc_dplane_if_protodown(zif, !!protodown); + netlink_proc_dplane_if_protodown(zif, tb); + if_sweep_protodown(zif); } return 0; @@ -1758,7 +1813,8 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) /* Looking up interface name. */ memset(linkinfo, 0, sizeof(linkinfo)); - netlink_parse_rtattr(tb, IFLA_MAX, IFLA_RTA(ifi), len); + netlink_parse_rtattr_flags(tb, IFLA_MAX, IFLA_RTA(ifi), len, + NLA_F_NESTED); /* check for wireless messages to ignore */ if ((tb[IFLA_WIRELESS] != NULL) && (ifi->ifi_change == 0)) { @@ -1898,14 +1954,9 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) zebra_l2if_update_bond_slave(ifp, bond_ifindex, !!bypass); - if (tb[IFLA_PROTO_DOWN]) { - uint8_t protodown; + if (tb[IFLA_PROTO_DOWN]) + netlink_proc_dplane_if_protodown(ifp->info, tb); - protodown = *(uint8_t *)RTA_DATA( - tb[IFLA_PROTO_DOWN]); - netlink_proc_dplane_if_protodown(ifp->info, - !!protodown); - } } else if (ifp->vrf->vrf_id != vrf_id) { /* VRF change for an interface. */ if (IS_ZEBRA_DEBUG_KERNEL) @@ -1952,14 +2003,8 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) netlink_to_zebra_link_type(ifi->ifi_type); netlink_interface_update_hw_addr(tb, ifp); - if (tb[IFLA_PROTO_DOWN]) { - uint8_t protodown; - - protodown = *(uint8_t *)RTA_DATA( - tb[IFLA_PROTO_DOWN]); - netlink_proc_dplane_if_protodown(zif, - !!protodown); - } + if (tb[IFLA_PROTO_DOWN]) + netlink_proc_dplane_if_protodown(ifp->info, tb); if (if_is_no_ptm_operative(ifp)) { bool is_up = if_is_operative(ifp); @@ -2112,7 +2157,6 @@ ssize_t netlink_intf_msg_encode(uint16_t cmd, struct rtattr *nest_protodown_reason; ifindex_t ifindex = dplane_ctx_get_ifindex(ctx); - uint32_t r_bitfield = dplane_ctx_get_intf_r_bitfield(ctx); bool down = dplane_ctx_intf_is_protodown(ctx); struct nlsock *nl = kernel_netlink_nlsock_lookup(dplane_ctx_get_ns_sock(ctx)); @@ -2137,20 +2181,19 @@ ssize_t netlink_intf_msg_encode(uint16_t cmd, nl_attr_put8(&req->n, buflen, IFLA_PROTO_DOWN, down); nl_attr_put32(&req->n, buflen, IFLA_LINK, ifindex); - if (r_bitfield) { - nest_protodown_reason = - nl_attr_nest(&req->n, buflen, IFLA_PROTO_DOWN_REASON); + /* Reason info nest */ + nest_protodown_reason = + nl_attr_nest(&req->n, buflen, IFLA_PROTO_DOWN_REASON); - if (!nest_protodown_reason) - return -1; + if (!nest_protodown_reason) + return -1; - nl_attr_put32(&req->n, buflen, IFLA_PROTO_DOWN_REASON_MASK, - (1 << frr_protodown_r_bit)); - nl_attr_put32(&req->n, buflen, IFLA_PROTO_DOWN_REASON_VALUE, - ((int)down) << frr_protodown_r_bit); + nl_attr_put32(&req->n, buflen, IFLA_PROTO_DOWN_REASON_MASK, + (1 << frr_protodown_r_bit)); + nl_attr_put32(&req->n, buflen, IFLA_PROTO_DOWN_REASON_VALUE, + ((int)down) << frr_protodown_r_bit); - nl_attr_nest_end(&req->n, nest_protodown_reason); - } + nl_attr_nest_end(&req->n, nest_protodown_reason); if (IS_ZEBRA_DEBUG_KERNEL) zlog_debug("%s: %s, protodown=%d ifindex=%u", __func__, diff --git a/zebra/interface.c b/zebra/interface.c index 23ff91faa3..15f8339c08 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -261,6 +261,12 @@ static int if_zebra_delete_hook(struct interface *ifp) if (ifp->info) { zebra_if = ifp->info; + /* If we set protodown, clear it now from the kernel */ + if (ZEBRA_IF_IS_PROTODOWN(zebra_if) && + !ZEBRA_IF_IS_PROTODOWN_ONLY_EXTERNAL(zebra_if)) + zebra_if_set_protodown(ifp, false, ZEBRA_PROTODOWN_ALL); + + /* Free installed address chains tree. */ if (zebra_if->ipv4_subnets) route_table_finish(zebra_if->ipv4_subnets); diff --git a/zebra/interface.h b/zebra/interface.h index 70a5581a88..7429f5eade 100644 --- a/zebra/interface.h +++ b/zebra/interface.h @@ -320,6 +320,10 @@ enum zebra_if_flags { ZIF_FLAG_LACP_BYPASS = (1 << 5) }; +#define ZEBRA_IF_IS_PROTODOWN(zif) (zif->flags & ZIF_FLAG_PROTODOWN) +#define ZEBRA_IF_IS_PROTODOWN_ONLY_EXTERNAL(zif) \ + (zif->protodown_rc == ZEBRA_PROTODOWN_EXTERNAL) + /* `zebra' daemon local interface structure. */ struct zebra_if { /* back pointer to the interface */ diff --git a/zebra/zebra_router.h b/zebra/zebra_router.h index 40f7d87980..7aca91959c 100644 --- a/zebra/zebra_router.h +++ b/zebra/zebra_router.h @@ -85,7 +85,10 @@ enum protodown_reasons { ZEBRA_PROTODOWN_EVPN_STARTUP_DELAY), ZEBRA_PROTODOWN_VRRP = (1 << 3), /* This reason used exclusively for testing */ - ZEBRA_PROTODOWN_SHARP = (1 << 4) + ZEBRA_PROTODOWN_SHARP = (1 << 4), + /* Just used to clear our fields on shutdown, externel not included */ + ZEBRA_PROTODOWN_ALL = (ZEBRA_PROTODOWN_EVPN_ALL | ZEBRA_PROTODOWN_VRRP | + ZEBRA_PROTODOWN_SHARP) }; #define ZEBRA_PROTODOWN_RC_STR_LEN 80 From d89b300829ff85cc4e6b51384170f67cf5f3f6a1 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 25 Jan 2022 14:44:25 -0500 Subject: [PATCH 07/24] zebra: use a macro for check protodown Add a helper macro for checking if interface is protodown, typing this out is annoying. Signed-off-by: Stephen Worley --- zebra/if_netlink.c | 4 ++-- zebra/interface.c | 6 +++--- zebra/zebra_dplane.c | 3 +-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index f26d52332b..4d6439652b 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -857,7 +857,7 @@ static void netlink_proc_dplane_if_protodown(struct zebra_if *zif, else zif->protodown_rc &= ~ZEBRA_PROTODOWN_EXTERNAL; - old_protodown = !!(zif->flags & ZIF_FLAG_PROTODOWN); + old_protodown = !!ZEBRA_IF_IS_PROTODOWN(zif); if (protodown == old_protodown) return; @@ -906,7 +906,7 @@ static void if_sweep_protodown(struct zebra_if *zif) { bool protodown; - protodown = !!(zif->flags & ZIF_FLAG_PROTODOWN); + protodown = !!ZEBRA_IF_IS_PROTODOWN(zif); if (!protodown) return; diff --git a/zebra/interface.c b/zebra/interface.c index 15f8339c08..35a2d3e83d 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -1244,7 +1244,7 @@ static bool if_ignore_set_protodown(const struct interface *ifp, bool new_down, zif = ifp->info; /* Current state as we know it */ - old_down = !!(zif->flags & ZIF_FLAG_PROTODOWN); + old_down = !!(ZEBRA_IF_IS_PROTODOWN(zif)); old_set_down = !!(zif->flags & ZIF_FLAG_SET_PROTODOWN); old_unset_down = !!(zif->flags & ZIF_FLAG_UNSET_PROTODOWN); @@ -2091,7 +2091,7 @@ static void if_dump_vty(struct vty *vty, struct interface *ifp) zebra_evpn_if_es_print(vty, NULL, zebra_if); vty_out(vty, " protodown: %s %s\n", - (zebra_if->flags & ZIF_FLAG_PROTODOWN) ? "on" : "off", + (ZEBRA_IF_IS_PROTODOWN(zebra_if)) ? "on" : "off", if_is_protodown_applicable(ifp) ? "" : "(n/a)"); if (zebra_if->protodown_rc) vty_out(vty, " protodown reasons: %s\n", @@ -2442,7 +2442,7 @@ static void if_dump_vty_json(struct vty *vty, struct interface *ifp, if (if_is_protodown_applicable(ifp)) { json_object_string_add( json_if, "protodown", - (zebra_if->flags & ZIF_FLAG_PROTODOWN) ? "on" : "off"); + (ZEBRA_IF_IS_PROTODOWN(zebra_if)) ? "on" : "off"); if (zebra_if->protodown_rc) json_object_string_add( json_if, "protodownReason", diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c index f60951cac4..2d13b9b54b 100644 --- a/zebra/zebra_dplane.c +++ b/zebra/zebra_dplane.c @@ -2732,8 +2732,7 @@ int dplane_ctx_intf_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op, else if (unset_pdown) ctx->u.intf.protodown = false; else - ctx->u.intf.protodown = - !!(zif->flags & ZIF_FLAG_PROTODOWN); + ctx->u.intf.protodown = !!ZEBRA_IF_IS_PROTODOWN(zif); } dplane_ctx_ns_init(ctx, zns, (op == DPLANE_OP_INTF_UPDATE)); From e4d87b58943f3d293bb578d605bafb6d1123f847 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 25 Jan 2022 14:49:01 -0500 Subject: [PATCH 08/24] zebra: remove old protodown dplane path Remove the old protodown dplane path install on the main thread. This is now dead code. Signed-off-by: Stephen Worley --- zebra/if_netlink.c | 42 ------------------------------------------ zebra/if_netlink.h | 18 ------------------ zebra/interface.c | 2 -- 3 files changed, 62 deletions(-) diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index 4d6439652b..2467e837b8 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -2202,48 +2202,6 @@ ssize_t netlink_intf_msg_encode(uint16_t cmd, return NLMSG_ALIGN(req->n.nlmsg_len); } -int netlink_protodown(struct interface *ifp, bool down, uint32_t r_bitfield) -{ - struct zebra_ns *zns = zebra_ns_lookup(NS_DEFAULT); - struct rtattr *nest_protodown_reason; - - struct { - struct nlmsghdr n; - struct ifinfomsg ifa; - char buf[NL_PKT_BUF_SIZE]; - } req; - - memset(&req, 0, sizeof(req)); - - req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg)); - req.n.nlmsg_flags = NLM_F_REQUEST; - req.n.nlmsg_type = RTM_SETLINK; - req.n.nlmsg_pid = zns->netlink_cmd.snl.nl_pid; - - req.ifa.ifi_index = ifp->ifindex; - - nl_attr_put8(&req.n, sizeof(req), IFLA_PROTO_DOWN, down); - nl_attr_put32(&req.n, sizeof(req), IFLA_LINK, ifp->ifindex); - - if (r_bitfield) { - nest_protodown_reason = nl_attr_nest(&req.n, sizeof(req), - IFLA_PROTO_DOWN_REASON); - - if (!nest_protodown_reason) - return -1; - - nl_attr_put32(&req.n, sizeof(req), IFLA_PROTO_DOWN_REASON_MASK, - (1 << frr_protodown_r_bit)); - nl_attr_put32(&req.n, sizeof(req), IFLA_PROTO_DOWN_REASON_VALUE, - ((int)down) << frr_protodown_r_bit); - - nl_attr_nest_end(&req.n, nest_protodown_reason); - } - - return netlink_talk(netlink_talk_filter, &req.n, &zns->netlink_cmd, zns, - false); -} - /* Interface information read by netlink. */ void interface_list(struct zebra_ns *zns) { diff --git a/zebra/if_netlink.h b/zebra/if_netlink.h index 3d9e934fb0..46eac25377 100644 --- a/zebra/if_netlink.h +++ b/zebra/if_netlink.h @@ -54,24 +54,6 @@ extern enum netlink_msg_status netlink_put_intf_update_msg(struct nl_batch *bth, struct zebra_dplane_ctx *ctx); #define FRR_PROTODOWN_REASON_DEFAULT_BIT 7 -#define PROTODOWN_REASON_NUM_BITS 32 -/* - * Set protodown status of interface. - * - * ifp - * Interface to set protodown on. - * - * down - * If true, set protodown on. If false, set protodown off. - * - * reason - * bitfield representing reason codes - * - * Returns: - * 0 - */ -int netlink_protodown(struct interface *ifp, bool down, uint32_t r_bitfield); - /* Protodown bit setter/getter * * Allow users to change the bit if it conflicts with another diff --git a/zebra/interface.c b/zebra/interface.c index 35a2d3e83d..6fb34d59ac 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -1321,8 +1321,6 @@ int zebra_if_set_protodown(struct interface *ifp, bool new_down, zif->flags |= ZIF_FLAG_UNSET_PROTODOWN; #ifdef HAVE_NETLINK - // TODO: remove this as separate commit - // netlink_protodown(ifp, new_down, zif->protodown_rc); dplane_intf_update(ifp); #else zlog_warn("Protodown is not supported on this platform"); From 0605d4b8462fe56cc8843e14d0b5fa02d2b3b2e1 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 25 Jan 2022 22:57:03 -0500 Subject: [PATCH 09/24] tests: add protodown topotest using sharpd Add a protodown topotest using sharpd. Signed-off-by: Stephen Worley --- tests/topotests/zebra_rib/test_zebra_rib.py | 57 +++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tests/topotests/zebra_rib/test_zebra_rib.py b/tests/topotests/zebra_rib/test_zebra_rib.py index ae891d9067..b72691b71e 100644 --- a/tests/topotests/zebra_rib/test_zebra_rib.py +++ b/tests/topotests/zebra_rib/test_zebra_rib.py @@ -31,6 +31,7 @@ import sys from functools import partial import pytest import json +import platform # Save the Current Working Directory to find configuration files. CWD = os.path.dirname(os.path.realpath(__file__)) @@ -45,6 +46,20 @@ from time import sleep pytestmark = [pytest.mark.sharpd] +krel = platform.release() + + +def config_macvlan(tgen, r_str, device, macvlan): + "Creates specified macvlan interace on physical device" + + if topotest.version_cmp(krel, "5.1") < 0: + return + + router = tgen.gears[r_str] + router.run( + "ip link add {} link {} type macvlan mode bridge".format(macvlan, device) + ) + router.run("ip link set {} up".format(macvlan)) def setup_module(mod): @@ -62,6 +77,8 @@ def setup_module(mod): TopoRouter.RD_SHARP, os.path.join(CWD, "{}/sharpd.conf".format(rname)) ) + # Macvlan interface for protodown func test */ + config_macvlan(tgen, "r1", "r1-eth0", "r1-eth0-macvlan") # Initialize all routers. tgen.start_router() @@ -269,6 +286,46 @@ def test_route_map_usage(): assert ok, result +def test_protodown(): + "Run protodown basic functionality test and report results." + pdown = False + count = 0 + tgen = get_topogen() + if topotest.version_cmp(krel, "5.1") < 0: + tgen.errors = "kernel 5.1 needed for protodown tests" + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + + # Set interface protodown on + r1.vtysh_cmd("sharp interface r1-eth0-macvlan protodown") + + # Timeout to wait for dplane to handle it + while count < 10: + count += 1 + output = r1.vtysh_cmd("show interface r1-eth0-macvlan") + if re.search(r"protodown reasons:.*sharp", output): + pdown = True + break + sleep(1) + + assert pdown is True, "Interface r1-eth0-macvlan not set protodown" + + # Set interface protodown off + r1.vtysh_cmd("no sharp interface r1-eth0-macvlan protodown") + + # Timeout to wait for dplane to handle it + while count < 10: + count += 1 + output = r1.vtysh_cmd("show interface r1-eth0-macvlan") + if not re.search(r"protodown reasons:.*sharp", output): + pdown = False + break + sleep(1) + + assert pdown is False, "Interface r1-eth0-macvlan not set protodown off" + + def test_memory_leak(): "Run the memory leak test and report results." tgen = get_topogen() From 3393afc9903d60771584bbde4c57c72386ec7bec Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 25 Jan 2022 23:41:11 -0500 Subject: [PATCH 10/24] doc: add user doc for sharp protodown Add user doc for setting protodown on an interface with sharpd. Signed-off-by: Stephen Worley --- doc/user/sharp.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/user/sharp.rst b/doc/user/sharp.rst index e9d4e2763f..8d201a3c06 100644 --- a/doc/user/sharp.rst +++ b/doc/user/sharp.rst @@ -296,3 +296,7 @@ keyword. At present, no sharp commands will be preserved in the config. router# show sharp segment-routing srv6 (nothing) + +.. clicmd:: sharp interface IFNAME protodown + + Set an interface protodown. From 97c726337389b7084ad970cf631a1126e5d5329c Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Wed, 26 Jan 2022 01:10:43 -0500 Subject: [PATCH 11/24] zebra: add boilerplate protodown updates for *bsd Add boilerplate for someone to come and add protdown updates for bsd platforms if it ever exists. Signed-off-by: Stephen Worley --- zebra/if_socket.c | 41 +++++++++++++++++++++++++++++++++++++++++ zebra/kernel_socket.c | 6 ++++++ zebra/rt.h | 3 +++ zebra/subdir.am | 1 + 4 files changed, 51 insertions(+) create mode 100644 zebra/if_socket.c diff --git a/zebra/if_socket.c b/zebra/if_socket.c new file mode 100644 index 0000000000..309d5a3f3e --- /dev/null +++ b/zebra/if_socket.c @@ -0,0 +1,41 @@ +/* + * Zebra Interface interaction with the kernel using socket. + * Copyright (C) 2022 NVIDIA CORPORATION & AFFILIATES + * Stephen Worley + * + * This file is part of FRR. + * + * FRR is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2, or (at your option) any + * later version. + * + * FRR is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with FRR; see the file COPYING. If not, write to the Free + * Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA + * 02111-1307, USA. + */ + +#include + +#ifndef HAVE_NETLINK + +#include "lib_errors.h" + +#include "zebra/rt.h" +#include "zebra/zebra_dplane.h" +#include "zebra/zebra_errors.h" + +enum zebra_dplane_result kernel_intf_update(struct zebra_dplane_ctx *ctx) +{ + flog_err(EC_LIB_UNAVAILABLE, "%s not Implemented for this platform", + __func__); + return ZEBRA_DPLANE_REQUEST_FAILURE; +} + +#endif diff --git a/zebra/kernel_socket.c b/zebra/kernel_socket.c index ce1f17111b..6583af0a54 100644 --- a/zebra/kernel_socket.c +++ b/zebra/kernel_socket.c @@ -1577,6 +1577,12 @@ void kernel_update_multi(struct dplane_ctx_q *ctx_list) res = kernel_pbr_rule_update(ctx); break; + case DPLANE_OP_INTF_INSTALL: + case DPLANE_OP_INTF_UPDATE: + case DPLANE_OP_INTF_DELETE: + res = kernel_intf_update(ctx); + break; + /* Ignore 'notifications' - no-op */ case DPLANE_OP_SYS_ROUTE_ADD: case DPLANE_OP_SYS_ROUTE_DELETE: diff --git a/zebra/rt.h b/zebra/rt.h index 5e626928d9..4952c3eb1a 100644 --- a/zebra/rt.h +++ b/zebra/rt.h @@ -66,6 +66,9 @@ enum zebra_dplane_result kernel_neigh_update_ctx(struct zebra_dplane_ctx *ctx); extern enum zebra_dplane_result kernel_pbr_rule_update(struct zebra_dplane_ctx *ctx); +extern enum zebra_dplane_result +kernel_intf_update(struct zebra_dplane_ctx *ctx); + #endif /* !HAVE_NETLINK */ extern int kernel_neigh_update(int cmd, int ifindex, void *addr, char *lla, diff --git a/zebra/subdir.am b/zebra/subdir.am index 77e0898d81..8cb1237c22 100644 --- a/zebra/subdir.am +++ b/zebra/subdir.am @@ -60,6 +60,7 @@ zebra_zebra_SOURCES = \ zebra/debug.c \ zebra/if_ioctl.c \ zebra/if_netlink.c \ + zebra/if_socket.c \ zebra/if_sysctl.c \ zebra/interface.c \ zebra/ioctl.c \ From ab465d24bd864c7ab9f5841af89c108e03e768ac Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Mon, 31 Jan 2022 16:12:01 -0500 Subject: [PATCH 12/24] zebra: only clear pd_reason on shutdown/sweep Only clear protodown reason on shutdown/sweep, retain protodown state. This is to retain traditional and expected behavior with daemons like vrrpd setting protodown. They expet it to be set on shutdown and retained on bring up to prevent traffic from being dropped. We must cleanup our reason code though to prevent us from blocking others. Signed-off-by: Stephen Worley --- zebra/if_netlink.c | 22 +++++++++++++--------- zebra/interface.c | 45 ++++++++++++++++++++++++++++---------------- zebra/zebra_dplane.c | 15 ++++++++------- zebra/zebra_dplane.h | 5 ++--- 4 files changed, 52 insertions(+), 35 deletions(-) diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index 2467e837b8..3591106fed 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -852,7 +852,8 @@ static void netlink_proc_dplane_if_protodown(struct zebra_if *zif, * If the reason we got from the kernel is ONLY frr though, don't * set it. */ - if (protodown && is_if_protodown_r_only_frr(rc_bitfield) == false) + if (protodown && rc_bitfield && + is_if_protodown_r_only_frr(rc_bitfield) == false) zif->protodown_rc |= ZEBRA_PROTODOWN_EXTERNAL; else zif->protodown_rc &= ~ZEBRA_PROTODOWN_EXTERNAL; @@ -900,7 +901,8 @@ static uint8_t netlink_parse_lacp_bypass(struct rtattr **linkinfo) } /* - * Only called at startup to cleanup leftover protodown we may have not cleanup + * Only called at startup to cleanup leftover protodown reasons we may + * have not cleaned up. We leave protodown set though. */ static void if_sweep_protodown(struct zebra_if *zif) { @@ -912,12 +914,12 @@ static void if_sweep_protodown(struct zebra_if *zif) return; if (IS_ZEBRA_DEBUG_KERNEL) - zlog_debug("interface %s sweeping protdown %s", zif->ifp->name, - protodown ? "on" : "off"); + zlog_debug("interface %s sweeping protodown %s reason 0x%x", + zif->ifp->name, protodown ? "on" : "off", + zif->protodown_rc); /* Only clear our reason codes, leave external if it was set */ - zif->protodown_rc |= ~ZEBRA_PROTODOWN_ALL; - zif->flags |= ZIF_FLAG_UNSET_PROTODOWN; + zif->protodown_rc &= ~ZEBRA_PROTODOWN_ALL; dplane_intf_update(zif->ifp); } @@ -2158,6 +2160,7 @@ ssize_t netlink_intf_msg_encode(uint16_t cmd, struct rtattr *nest_protodown_reason; ifindex_t ifindex = dplane_ctx_get_ifindex(ctx); bool down = dplane_ctx_intf_is_protodown(ctx); + bool pd_reason_val = dplane_ctx_get_intf_pd_reason_val(ctx); struct nlsock *nl = kernel_netlink_nlsock_lookup(dplane_ctx_get_ns_sock(ctx)); @@ -2191,13 +2194,14 @@ ssize_t netlink_intf_msg_encode(uint16_t cmd, nl_attr_put32(&req->n, buflen, IFLA_PROTO_DOWN_REASON_MASK, (1 << frr_protodown_r_bit)); nl_attr_put32(&req->n, buflen, IFLA_PROTO_DOWN_REASON_VALUE, - ((int)down) << frr_protodown_r_bit); + ((int)pd_reason_val) << frr_protodown_r_bit); nl_attr_nest_end(&req->n, nest_protodown_reason); if (IS_ZEBRA_DEBUG_KERNEL) - zlog_debug("%s: %s, protodown=%d ifindex=%u", __func__, - nl_msg_type_to_str(cmd), down, ifindex); + zlog_debug("%s: %s, protodown=%d reason_val=%d ifindex=%u", + __func__, nl_msg_type_to_str(cmd), down, + pd_reason_val, ifindex); return NLMSG_ALIGN(req->n.nlmsg_len); } diff --git a/zebra/interface.c b/zebra/interface.c index 6fb34d59ac..167f1b4587 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -63,6 +63,8 @@ DEFINE_HOOK(zebra_if_config_wr, (struct vty * vty, struct interface *ifp), static void if_down_del_nbr_connected(struct interface *ifp); +static int zebra_if_update_protodown(struct interface *ifp, bool new_down, + uint32_t new_protodown_rc); static void if_zebra_speed_update(struct thread *thread) { @@ -261,11 +263,12 @@ static int if_zebra_delete_hook(struct interface *ifp) if (ifp->info) { zebra_if = ifp->info; - /* If we set protodown, clear it now from the kernel */ - if (ZEBRA_IF_IS_PROTODOWN(zebra_if) && + /* If we set protodown, clear our reason now from the kernel */ + if (ZEBRA_IF_IS_PROTODOWN(zebra_if) && zebra_if->protodown_rc && !ZEBRA_IF_IS_PROTODOWN_ONLY_EXTERNAL(zebra_if)) - zebra_if_set_protodown(ifp, false, ZEBRA_PROTODOWN_ALL); - + zebra_if_update_protodown(ifp, true, + (zebra_if->protodown_rc & + ~ZEBRA_PROTODOWN_ALL)); /* Free installed address chains tree. */ if (zebra_if->ipv4_subnets) @@ -1291,19 +1294,13 @@ static bool if_ignore_set_protodown(const struct interface *ifp, bool new_down, return false; } -int zebra_if_set_protodown(struct interface *ifp, bool new_down, - enum protodown_reasons new_reason) +static int zebra_if_update_protodown(struct interface *ifp, bool new_down, + uint32_t new_protodown_rc) { struct zebra_if *zif; - uint32_t new_protodown_rc; zif = ifp->info; - if (new_down) - new_protodown_rc = zif->protodown_rc | new_reason; - else - new_protodown_rc = zif->protodown_rc & ~new_reason; - /* Check if we already have this state or it's queued */ if (if_ignore_set_protodown(ifp, new_down, new_protodown_rc)) return 1; @@ -1328,6 +1325,22 @@ int zebra_if_set_protodown(struct interface *ifp, bool new_down, return 0; } +int zebra_if_set_protodown(struct interface *ifp, bool new_down, + enum protodown_reasons new_reason) +{ + struct zebra_if *zif; + uint32_t new_protodown_rc; + + zif = ifp->info; + + if (new_down) + new_protodown_rc = zif->protodown_rc | new_reason; + else + new_protodown_rc = zif->protodown_rc & ~new_reason; + + return zebra_if_update_protodown(ifp, new_down, new_protodown_rc); +} + /* * Handle an interface events based on info in a dplane context object. * This runs in the main pthread, using the info in the context object to @@ -1410,18 +1423,18 @@ static void zebra_if_update_ctx(struct zebra_dplane_ctx *ctx, { enum zebra_dplane_result dp_res; struct zebra_if *zif; - uint32_t r_bitfield; + bool pd_reason_val; bool down; dp_res = dplane_ctx_get_status(ctx); - r_bitfield = dplane_ctx_get_intf_r_bitfield(ctx); + pd_reason_val = dplane_ctx_get_intf_pd_reason_val(ctx); down = dplane_ctx_intf_is_protodown(ctx); if (IS_ZEBRA_DEBUG_KERNEL) - zlog_debug("%s: %s: if %s(%u) ctx-protodown %s ctx-reason 0x%x", + zlog_debug("%s: %s: if %s(%u) ctx-protodown %s ctx-reason %d", __func__, dplane_op2str(dplane_ctx_get_op(ctx)), ifp->name, ifp->ifindex, down ? "on" : "off", - r_bitfield); + pd_reason_val); zif = ifp->info; if (!zif) { diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c index 2d13b9b54b..1ad5d93ae2 100644 --- a/zebra/zebra_dplane.c +++ b/zebra/zebra_dplane.c @@ -192,9 +192,9 @@ struct dplane_intf_info { uint32_t metric; uint32_t flags; - uint32_t r_bitfield; bool protodown; + bool pd_reason_val; #define DPLANE_INTF_CONNECTED (1 << 0) /* Connected peer, p2p */ #define DPLANE_INTF_SECONDARY (1 << 1) @@ -1790,19 +1790,18 @@ void dplane_ctx_set_intf_metric(struct zebra_dplane_ctx *ctx, uint32_t metric) ctx->u.intf.metric = metric; } -uint32_t dplane_ctx_get_intf_r_bitfield(const struct zebra_dplane_ctx *ctx) +uint32_t dplane_ctx_get_intf_pd_reason_val(const struct zebra_dplane_ctx *ctx) { DPLANE_CTX_VALID(ctx); - return ctx->u.intf.r_bitfield; + return ctx->u.intf.pd_reason_val; } -void dplane_ctx_set_intf_r_bitfield(struct zebra_dplane_ctx *ctx, - uint32_t r_bitfield) +void dplane_ctx_set_intf_pd_reason_val(struct zebra_dplane_ctx *ctx, bool val) { DPLANE_CTX_VALID(ctx); - ctx->u.intf.r_bitfield = r_bitfield; + ctx->u.intf.pd_reason_val = val; } bool dplane_ctx_intf_is_protodown(const struct zebra_dplane_ctx *ctx) @@ -2721,7 +2720,9 @@ int dplane_ctx_intf_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op, set_pdown = !!(zif->flags & ZIF_FLAG_SET_PROTODOWN); unset_pdown = !!(zif->flags & ZIF_FLAG_UNSET_PROTODOWN); - ctx->u.intf.r_bitfield = zif->protodown_rc; + if (zif->protodown_rc && + ZEBRA_IF_IS_PROTODOWN_ONLY_EXTERNAL(zif) == false) + ctx->u.intf.pd_reason_val = true; /* * See if we have new protodown state to set, otherwise keep diff --git a/zebra/zebra_dplane.h b/zebra/zebra_dplane.h index 73acf03411..334d440a2f 100644 --- a/zebra/zebra_dplane.h +++ b/zebra/zebra_dplane.h @@ -485,9 +485,8 @@ dplane_ctx_get_pw_backup_nhg(const struct zebra_dplane_ctx *ctx); /* Accessors for interface information */ uint32_t dplane_ctx_get_intf_metric(const struct zebra_dplane_ctx *ctx); void dplane_ctx_set_intf_metric(struct zebra_dplane_ctx *ctx, uint32_t metric); -uint32_t dplane_ctx_get_intf_r_bitfield(const struct zebra_dplane_ctx *ctx); -void dplane_ctx_set_intf_r_bitfield(struct zebra_dplane_ctx *ctx, - uint32_t r_bitfield); +uint32_t dplane_ctx_get_intf_pd_reason_val(const struct zebra_dplane_ctx *ctx); +void dplane_ctx_set_intf_pd_reason_val(struct zebra_dplane_ctx *ctx, bool val); bool dplane_ctx_intf_is_protodown(const struct zebra_dplane_ctx *ctx); /* Is interface addr p2p? */ bool dplane_ctx_intf_is_connected(const struct zebra_dplane_ctx *ctx); From 321c80d6b2142a769059df5b9fed0f251409ed92 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Feb 2022 12:33:06 -0500 Subject: [PATCH 13/24] zebra: extern setting protodown reason directly Extern the api for setting the protodown reason code bitfield directly. Some places may want to completely update the bitfield with more than one reason at a time. Signed-off-by: Stephen Worley --- zebra/interface.c | 14 ++++++-------- zebra/interface.h | 8 ++++++++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/zebra/interface.c b/zebra/interface.c index 167f1b4587..a547230d37 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -63,8 +63,6 @@ DEFINE_HOOK(zebra_if_config_wr, (struct vty * vty, struct interface *ifp), static void if_down_del_nbr_connected(struct interface *ifp); -static int zebra_if_update_protodown(struct interface *ifp, bool new_down, - uint32_t new_protodown_rc); static void if_zebra_speed_update(struct thread *thread) { @@ -266,9 +264,9 @@ static int if_zebra_delete_hook(struct interface *ifp) /* If we set protodown, clear our reason now from the kernel */ if (ZEBRA_IF_IS_PROTODOWN(zebra_if) && zebra_if->protodown_rc && !ZEBRA_IF_IS_PROTODOWN_ONLY_EXTERNAL(zebra_if)) - zebra_if_update_protodown(ifp, true, - (zebra_if->protodown_rc & - ~ZEBRA_PROTODOWN_ALL)); + zebra_if_update_protodown_rc(ifp, true, + (zebra_if->protodown_rc & + ~ZEBRA_PROTODOWN_ALL)); /* Free installed address chains tree. */ if (zebra_if->ipv4_subnets) @@ -1294,8 +1292,8 @@ static bool if_ignore_set_protodown(const struct interface *ifp, bool new_down, return false; } -static int zebra_if_update_protodown(struct interface *ifp, bool new_down, - uint32_t new_protodown_rc) +int zebra_if_update_protodown_rc(struct interface *ifp, bool new_down, + uint32_t new_protodown_rc) { struct zebra_if *zif; @@ -1338,7 +1336,7 @@ int zebra_if_set_protodown(struct interface *ifp, bool new_down, else new_protodown_rc = zif->protodown_rc & ~new_reason; - return zebra_if_update_protodown(ifp, new_down, new_protodown_rc); + return zebra_if_update_protodown_rc(ifp, new_down, new_protodown_rc); } /* diff --git a/zebra/interface.h b/zebra/interface.h index 7429f5eade..d4eaf389f7 100644 --- a/zebra/interface.h +++ b/zebra/interface.h @@ -505,6 +505,14 @@ extern void if_handle_vrf_change(struct interface *ifp, vrf_id_t vrf_id); extern void zebra_if_update_link(struct interface *ifp, ifindex_t link_ifindex, ns_id_t ns_id); extern void zebra_if_update_all_links(struct zebra_ns *zns); +/** + * Directly update entire protodown & reason code bitfield. + */ +extern int zebra_if_update_protodown_rc(struct interface *ifp, bool new_down, + uint32_t new_protodown_rc); +/** + * Set protodown with single reason. + */ extern int zebra_if_set_protodown(struct interface *ifp, bool down, enum protodown_reasons new_reason); extern int if_ip_address_install(struct interface *ifp, struct prefix *prefix, From cb5b31f5d011d3dcb2df5d876447fb3c7f758e6a Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Feb 2022 12:35:02 -0500 Subject: [PATCH 14/24] zebra: evpn-mh use protodown update reason api When setting the protodown reason use the update api where we can directly update the entire reason bitfield since we have to set more than one. Signed-off-by: Stephen Worley --- zebra/zebra_evpn_mh.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zebra/zebra_evpn_mh.c b/zebra/zebra_evpn_mh.c index 463ede158c..bc050671c7 100644 --- a/zebra/zebra_evpn_mh.c +++ b/zebra/zebra_evpn_mh.c @@ -3646,8 +3646,8 @@ void zebra_evpn_mh_update_protodown_bond_mbr(struct zebra_if *zif, bool clear, caller, zif->ifp->name, old_protodown_rc, new_protodown_rc); - if (zebra_if_set_protodown(zif->ifp, new_protodown, new_protodown_rc) == - 0) { + if (zebra_if_update_protodown_rc(zif->ifp, new_protodown, + new_protodown_rc) == 0) { if (IS_ZEBRA_DEBUG_EVPN_MH_ES) zlog_debug("%s protodown %s", zif->ifp->name, new_protodown ? "on" : "off"); From 4b82b9548879ad3b888fd7d8b9cea99a55260c55 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Feb 2022 12:36:18 -0500 Subject: [PATCH 15/24] zebra: evpn-mh bonds protodown check for set When we are processing a bond member's protodown we get from the dataplane, check to make sure we haven't already queued up a set. If we have, it's likely this is just a notification we get from the kernel after we set protodown and before we have processed the result in our dplane pthread. This change is needed now that we set protodown via the dplane pthread. Signed-off-by: Stephen Worley --- zebra/if_netlink.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index 3591106fed..47e0140256 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -872,6 +872,13 @@ static void netlink_proc_dplane_if_protodown(struct zebra_if *zif, zif->flags &= ~ZIF_FLAG_PROTODOWN; if (zebra_evpn_is_es_bond_member(zif->ifp)) { + /* Check it's not already being sent to the dplane first */ + if (protodown && (zif->flags & ZIF_FLAG_SET_PROTODOWN)) + return; + + if (!protodown && (zif->flags & ZIF_FLAG_UNSET_PROTODOWN)) + return; + if (IS_ZEBRA_DEBUG_EVPN_MH_ES || IS_ZEBRA_DEBUG_KERNEL) zlog_debug( "bond mbr %s re-instate protdown %s in the dplane", From b1da028e451b5b84ad1a6662fa70e4cc9123dbe4 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Feb 2022 14:05:10 -0500 Subject: [PATCH 16/24] zebra: avoid initialization in ctx_intf_init Avoid initialization in dplane_ctx_intf_init() so the compiler can warn us about using unintialized data. Signed-off-by: Stephen Worley --- zebra/zebra_dplane.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c index 1ad5d93ae2..d034c8f306 100644 --- a/zebra/zebra_dplane.c +++ b/zebra/zebra_dplane.c @@ -2690,8 +2690,8 @@ done: int dplane_ctx_intf_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op, const struct interface *ifp) { - struct zebra_ns *zns = NULL; - struct zebra_if *zif = NULL; + struct zebra_ns *zns; + struct zebra_if *zif; int ret = EINVAL; bool set_pdown, unset_pdown; From ddfea8a23393da835e009188d205002560e94ec3 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Feb 2022 14:08:06 -0500 Subject: [PATCH 17/24] zebra: wrap macro zif argument in parans Missing parenthesis for macro ZIF argument. Signed-off-by: Stephen Worley --- zebra/interface.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zebra/interface.h b/zebra/interface.h index d4eaf389f7..4b06603e7f 100644 --- a/zebra/interface.h +++ b/zebra/interface.h @@ -320,9 +320,9 @@ enum zebra_if_flags { ZIF_FLAG_LACP_BYPASS = (1 << 5) }; -#define ZEBRA_IF_IS_PROTODOWN(zif) (zif->flags & ZIF_FLAG_PROTODOWN) +#define ZEBRA_IF_IS_PROTODOWN(zif) ((zif)->flags & ZIF_FLAG_PROTODOWN) #define ZEBRA_IF_IS_PROTODOWN_ONLY_EXTERNAL(zif) \ - (zif->protodown_rc == ZEBRA_PROTODOWN_EXTERNAL) + ((zif)->protodown_rc == ZEBRA_PROTODOWN_EXTERNAL) /* `zebra' daemon local interface structure. */ struct zebra_if { From 3db9f2db2bb44a459cd4bca93f4fc6b53b88adec Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Feb 2022 14:22:50 -0500 Subject: [PATCH 18/24] zebra: cleanup protodown api logs Cleanup the logs in the api for setting protodown on/off that zapi and others use. Make them more useful to a user parsing them after an issue. Signed-off-by: Stephen Worley --- zebra/interface.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/zebra/interface.c b/zebra/interface.c index a547230d37..708e7b5485 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -1254,9 +1254,9 @@ static bool if_ignore_set_protodown(const struct interface *ifp, bool new_down, if (new_down == old_down) { if (IS_ZEBRA_DEBUG_KERNEL) zlog_debug( - "Ignoring %s (%u): protodown %s already set reason: old 0x%x new 0x%x", - ifp->name, ifp->ifindex, - new_down ? "on" : "off", + "Ignoring request to set protodown %s for interface %s (%u): protodown %s is already set (reason bitfield: old 0x%x new 0x%x)", + new_down ? "on" : "off", ifp->name, + ifp->ifindex, new_down ? "on" : "off", zif->protodown_rc, new_protodown_rc); return true; @@ -1267,9 +1267,9 @@ static bool if_ignore_set_protodown(const struct interface *ifp, bool new_down, if (new_down && old_set_down) { if (IS_ZEBRA_DEBUG_KERNEL) zlog_debug( - "Ignoring %s (%u): protodown %s queued to dplane already reason: old 0x%x new 0x%x", - ifp->name, ifp->ifindex, - new_down ? "on" : "off", + "Ignoring request to set protodown %s for interface %s (%u): protodown %s is already queued to dplane (reason bitfield: old 0x%x new 0x%x)", + new_down ? "on" : "off", ifp->name, + ifp->ifindex, new_down ? "on" : "off", zif->protodown_rc, new_protodown_rc); return true; @@ -1280,9 +1280,9 @@ static bool if_ignore_set_protodown(const struct interface *ifp, bool new_down, if (!new_down && old_unset_down) { if (IS_ZEBRA_DEBUG_KERNEL) zlog_debug( - "Ignoring %s (%u): protodown %s queued to dplane already reason: old 0x%x new 0x%x", - ifp->name, ifp->ifindex, - new_down ? "on" : "off", + "Ignoring request to set protodown %s for interface %s (%u): protodown %s is already queued to dplane (reason bitfield: old 0x%x new 0x%x)", + new_down ? "on" : "off", ifp->name, + ifp->ifindex, new_down ? "on" : "off", zif->protodown_rc, new_protodown_rc); return true; @@ -1304,8 +1304,8 @@ int zebra_if_update_protodown_rc(struct interface *ifp, bool new_down, return 1; zlog_info( - "Setting interface %s (%u): protodown %s reason: old 0x%x new 0x%x", - ifp->name, ifp->ifindex, new_down ? "on" : "off", + "Setting protodown %s - interface %s (%u): reason bitfield change from 0x%x --> 0x%x", + new_down ? "on" : "off", ifp->name, ifp->ifindex, zif->protodown_rc, new_protodown_rc); zif->protodown_rc = new_protodown_rc; From a3ea8493a8f173f96b84cc928c1ea25b1197ee52 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Feb 2022 17:46:05 -0500 Subject: [PATCH 19/24] zebra: simplify reason code printing in show Simplify the code for printing the reason codes via show command. Just remove the trailing comma last before printing. Signed-off-by: Stephen Worley --- zebra/interface.c | 52 ++++++++++++++++------------------------------- 1 file changed, 17 insertions(+), 35 deletions(-) diff --git a/zebra/interface.c b/zebra/interface.c index 708e7b5485..97741bd404 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -1852,49 +1852,31 @@ static void ifs_dump_brief_vty_json(json_object *json, struct vrf *vrf) const char *zebra_protodown_rc_str(uint32_t protodown_rc, char *pd_buf, uint32_t pd_buf_len) { - bool first = true; - pd_buf[0] = '\0'; + size_t len; strlcat(pd_buf, "(", pd_buf_len); - if (protodown_rc & ZEBRA_PROTODOWN_EXTERNAL) { - if (first) - first = false; - else - strlcat(pd_buf, ",", pd_buf_len); - strlcat(pd_buf, "external", pd_buf_len); - } + if (protodown_rc & ZEBRA_PROTODOWN_EXTERNAL) + strlcat(pd_buf, "external,", pd_buf_len); - if (protodown_rc & ZEBRA_PROTODOWN_EVPN_STARTUP_DELAY) { - if (first) - first = false; - else - strlcat(pd_buf, ",", pd_buf_len); - strlcat(pd_buf, "startup-delay", pd_buf_len); - } + if (protodown_rc & ZEBRA_PROTODOWN_EVPN_STARTUP_DELAY) + strlcat(pd_buf, "startup-delay,", pd_buf_len); - if (protodown_rc & ZEBRA_PROTODOWN_EVPN_UPLINK_DOWN) { - if (first) - first = false; - else - strlcat(pd_buf, ",", pd_buf_len); - strlcat(pd_buf, "uplinks-down", pd_buf_len); - } + if (protodown_rc & ZEBRA_PROTODOWN_EVPN_UPLINK_DOWN) + strlcat(pd_buf, "uplinks-down,", pd_buf_len); - if (protodown_rc & ZEBRA_PROTODOWN_VRRP) { - if (first) - first = false; - else - strlcat(pd_buf, ",", pd_buf_len); - strlcat(pd_buf, "vrrp", pd_buf_len); - } + if (protodown_rc & ZEBRA_PROTODOWN_VRRP) + strlcat(pd_buf, "vrrp,", pd_buf_len); - if (protodown_rc & ZEBRA_PROTODOWN_SHARP) { - if (!first) - strlcat(pd_buf, ",", pd_buf_len); - strlcat(pd_buf, "sharp", pd_buf_len); - } + if (protodown_rc & ZEBRA_PROTODOWN_SHARP) + strlcat(pd_buf, "sharp,", pd_buf_len); + + len = strnlen(pd_buf, pd_buf_len); + + /* Remove trailing comma */ + if (pd_buf[len - 1] == ',') + pd_buf[len - 1] = '\0'; strlcat(pd_buf, ")", pd_buf_len); From 00d57e6dd546c4d901d64d4efd8d48fda0f36316 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Feb 2022 17:51:36 -0500 Subject: [PATCH 20/24] zebra: clear dplane flags on failure for protodown Make sure we clear our dplane flags for SET/UNSET on failure so that we try again. Signed-off-by: Stephen Worley --- zebra/interface.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/zebra/interface.c b/zebra/interface.c index 97741bd404..10cc665752 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -1446,17 +1446,19 @@ static void zebra_if_update_ctx(struct zebra_dplane_ctx *ctx, if (IS_ZEBRA_DEBUG_KERNEL) zlog_debug("%s: if %s(%u) dplane update failed", __func__, ifp->name, ifp->ifindex); - return; + goto done; } /* Update our info */ - if (down) { + if (down) zif->flags |= ZIF_FLAG_PROTODOWN; - zif->flags &= ~ZIF_FLAG_SET_PROTODOWN; - } else { + else zif->flags &= ~ZIF_FLAG_PROTODOWN; - zif->flags &= ~ZIF_FLAG_UNSET_PROTODOWN; - } + +done: + /* Clear our dplane flags */ + zif->flags &= ~ZIF_FLAG_SET_PROTODOWN; + zif->flags &= ~ZIF_FLAG_UNSET_PROTODOWN; } /* From ed7a1622c347bf5cb0929da07f9072e04f0730c1 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Feb 2022 17:56:50 -0500 Subject: [PATCH 21/24] zebra: make netlink protodown func more readable Make the netlink protodown static function for checking if the only bit set for protodown reason is FRR's more easily readable to someone not familiar with the code. Signed-off-by: Stephen Worley --- zebra/if_netlink.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index 47e0140256..0ed03ae750 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -815,7 +815,7 @@ static int netlink_bridge_interface(struct nlmsghdr *h, int len, ns_id_t ns_id, return 0; } -static bool is_if_protodown_r_only_frr(uint32_t rc_bitfield) +static bool is_if_protodown_reason_only_frr(uint32_t rc_bitfield) { /* This shouldn't be possible */ assert(frr_protodown_r_bit < 32); @@ -853,7 +853,7 @@ static void netlink_proc_dplane_if_protodown(struct zebra_if *zif, * set it. */ if (protodown && rc_bitfield && - is_if_protodown_r_only_frr(rc_bitfield) == false) + is_if_protodown_reason_only_frr(rc_bitfield) == false) zif->protodown_rc |= ZEBRA_PROTODOWN_EXTERNAL; else zif->protodown_rc &= ~ZEBRA_PROTODOWN_EXTERNAL; From 26a64ff9ca7b8afdd5ab9b9bc8a59f3ea9dbc53b Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Feb 2022 18:42:41 -0500 Subject: [PATCH 22/24] zebra: include old reason in evpn-mh bond update Ensure we include the old reason when we are updating the reason code for a evpn-mh bond member. Now that this is a common API it could include things external to EVPN in this reason code bitfield (ex: vrrp). Signed-off-by: Stephen Worley --- zebra/zebra_evpn_mh.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra/zebra_evpn_mh.c b/zebra/zebra_evpn_mh.c index bc050671c7..45eb51ae14 100644 --- a/zebra/zebra_evpn_mh.c +++ b/zebra/zebra_evpn_mh.c @@ -3636,7 +3636,7 @@ void zebra_evpn_mh_update_protodown_bond_mbr(struct zebra_if *zif, bool clear, } old_protodown_rc = zif->protodown_rc; - new_protodown_rc &= ~ZEBRA_PROTODOWN_EVPN_ALL; + new_protodown_rc = (old_protodown_rc & ~ZEBRA_PROTODOWN_EVPN_ALL); new_protodown_rc |= (protodown_rc & ZEBRA_PROTODOWN_EVPN_ALL); new_protodown = !!new_protodown_rc; From 7140b00cb0c0f9e9f007f092890f04166a9a66f8 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Feb 2022 18:21:18 -0500 Subject: [PATCH 23/24] zebra: use SET/UNSET/CHECK/COND in protodown code Use the SET/UNSET/CHECK/COND macros for flag bifields where appropriate throught the protodown code base. Signed-off-by: Stephen Worley --- zebra/if_netlink.c | 26 ++++++++++++-------------- zebra/interface.c | 27 ++++++++++++--------------- zebra/zebra_evpn_mh.c | 9 +++++---- 3 files changed, 29 insertions(+), 33 deletions(-) diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index 0ed03ae750..e21ff533ec 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -852,11 +852,10 @@ static void netlink_proc_dplane_if_protodown(struct zebra_if *zif, * If the reason we got from the kernel is ONLY frr though, don't * set it. */ - if (protodown && rc_bitfield && - is_if_protodown_reason_only_frr(rc_bitfield) == false) - zif->protodown_rc |= ZEBRA_PROTODOWN_EXTERNAL; - else - zif->protodown_rc &= ~ZEBRA_PROTODOWN_EXTERNAL; + COND_FLAG(zif->protodown_rc, ZEBRA_PROTODOWN_EXTERNAL, + protodown && rc_bitfield && + !is_if_protodown_reason_only_frr(rc_bitfield)); + old_protodown = !!ZEBRA_IF_IS_PROTODOWN(zif); if (protodown == old_protodown) @@ -866,17 +865,16 @@ static void netlink_proc_dplane_if_protodown(struct zebra_if *zif, zlog_debug("interface %s dplane change, protdown %s", zif->ifp->name, protodown ? "on" : "off"); - if (protodown) - zif->flags |= ZIF_FLAG_PROTODOWN; - else - zif->flags &= ~ZIF_FLAG_PROTODOWN; + /* Set protodown, respectively */ + COND_FLAG(zif->flags, ZIF_FLAG_PROTODOWN, protodown); if (zebra_evpn_is_es_bond_member(zif->ifp)) { /* Check it's not already being sent to the dplane first */ - if (protodown && (zif->flags & ZIF_FLAG_SET_PROTODOWN)) + if (protodown && CHECK_FLAG(zif->flags, ZIF_FLAG_SET_PROTODOWN)) return; - if (!protodown && (zif->flags & ZIF_FLAG_UNSET_PROTODOWN)) + if (!protodown + && CHECK_FLAG(zif->flags, ZIF_FLAG_UNSET_PROTODOWN)) return; if (IS_ZEBRA_DEBUG_EVPN_MH_ES || IS_ZEBRA_DEBUG_KERNEL) @@ -885,9 +883,9 @@ static void netlink_proc_dplane_if_protodown(struct zebra_if *zif, zif->ifp->name, old_protodown ? "on" : "off"); if (old_protodown) - zif->flags |= ZIF_FLAG_SET_PROTODOWN; + SET_FLAG(zif->flags, ZIF_FLAG_SET_PROTODOWN); else - zif->flags |= ZIF_FLAG_UNSET_PROTODOWN; + SET_FLAG(zif->flags, ZIF_FLAG_UNSET_PROTODOWN); dplane_intf_update(zif->ifp); } @@ -926,7 +924,7 @@ static void if_sweep_protodown(struct zebra_if *zif) zif->protodown_rc); /* Only clear our reason codes, leave external if it was set */ - zif->protodown_rc &= ~ZEBRA_PROTODOWN_ALL; + UNSET_FLAG(zif->protodown_rc, ZEBRA_PROTODOWN_ALL); dplane_intf_update(zif->ifp); } diff --git a/zebra/interface.c b/zebra/interface.c index 10cc665752..69d611e583 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -1246,8 +1246,8 @@ static bool if_ignore_set_protodown(const struct interface *ifp, bool new_down, /* Current state as we know it */ old_down = !!(ZEBRA_IF_IS_PROTODOWN(zif)); - old_set_down = !!(zif->flags & ZIF_FLAG_SET_PROTODOWN); - old_unset_down = !!(zif->flags & ZIF_FLAG_UNSET_PROTODOWN); + old_set_down = !!CHECK_FLAG(zif->flags, ZIF_FLAG_SET_PROTODOWN); + old_unset_down = !!CHECK_FLAG(zif->flags, ZIF_FLAG_UNSET_PROTODOWN); if (new_protodown_rc == zif->protodown_rc) { /* Early return if already down & reason bitfield matches */ @@ -1311,9 +1311,9 @@ int zebra_if_update_protodown_rc(struct interface *ifp, bool new_down, zif->protodown_rc = new_protodown_rc; if (new_down) - zif->flags |= ZIF_FLAG_SET_PROTODOWN; + SET_FLAG(zif->flags, ZIF_FLAG_SET_PROTODOWN); else - zif->flags |= ZIF_FLAG_UNSET_PROTODOWN; + SET_FLAG(zif->flags, ZIF_FLAG_UNSET_PROTODOWN); #ifdef HAVE_NETLINK dplane_intf_update(ifp); @@ -1450,15 +1450,12 @@ static void zebra_if_update_ctx(struct zebra_dplane_ctx *ctx, } /* Update our info */ - if (down) - zif->flags |= ZIF_FLAG_PROTODOWN; - else - zif->flags &= ~ZIF_FLAG_PROTODOWN; + COND_FLAG(zif->flags, ZIF_FLAG_PROTODOWN, down); done: /* Clear our dplane flags */ - zif->flags &= ~ZIF_FLAG_SET_PROTODOWN; - zif->flags &= ~ZIF_FLAG_UNSET_PROTODOWN; + UNSET_FLAG(zif->flags, ZIF_FLAG_SET_PROTODOWN); + UNSET_FLAG(zif->flags, ZIF_FLAG_UNSET_PROTODOWN); } /* @@ -1859,19 +1856,19 @@ const char *zebra_protodown_rc_str(uint32_t protodown_rc, char *pd_buf, strlcat(pd_buf, "(", pd_buf_len); - if (protodown_rc & ZEBRA_PROTODOWN_EXTERNAL) + if (CHECK_FLAG(protodown_rc, ZEBRA_PROTODOWN_EXTERNAL)) strlcat(pd_buf, "external,", pd_buf_len); - if (protodown_rc & ZEBRA_PROTODOWN_EVPN_STARTUP_DELAY) + if (CHECK_FLAG(protodown_rc, ZEBRA_PROTODOWN_EVPN_STARTUP_DELAY)) strlcat(pd_buf, "startup-delay,", pd_buf_len); - if (protodown_rc & ZEBRA_PROTODOWN_EVPN_UPLINK_DOWN) + if (CHECK_FLAG(protodown_rc, ZEBRA_PROTODOWN_EVPN_UPLINK_DOWN)) strlcat(pd_buf, "uplinks-down,", pd_buf_len); - if (protodown_rc & ZEBRA_PROTODOWN_VRRP) + if (CHECK_FLAG(protodown_rc, ZEBRA_PROTODOWN_VRRP)) strlcat(pd_buf, "vrrp,", pd_buf_len); - if (protodown_rc & ZEBRA_PROTODOWN_SHARP) + if (CHECK_FLAG(protodown_rc, ZEBRA_PROTODOWN_SHARP)) strlcat(pd_buf, "sharp,", pd_buf_len); len = strnlen(pd_buf, pd_buf_len); diff --git a/zebra/zebra_evpn_mh.c b/zebra/zebra_evpn_mh.c index 45eb51ae14..43eef69be2 100644 --- a/zebra/zebra_evpn_mh.c +++ b/zebra/zebra_evpn_mh.c @@ -3463,13 +3463,14 @@ void zebra_evpn_mh_json(json_object *json) if (zmh_info->protodown_rc) { json_array = json_object_new_array(); - if (zmh_info->protodown_rc & ZEBRA_PROTODOWN_EVPN_STARTUP_DELAY) + if (CHECK_FLAG(zmh_info->protodown_rc, + ZEBRA_PROTODOWN_EVPN_STARTUP_DELAY)) json_object_array_add( json_array, json_object_new_string("startupDelay")); - if (zmh_info->protodown_rc & ZEBRA_PROTODOWN_EVPN_UPLINK_DOWN) - json_object_array_add( - json_array, + if (CHECK_FLAG(zmh_info->protodown_rc, + ZEBRA_PROTODOWN_EVPN_UPLINK_DOWN)) + json_object_array_add(json_array, json_object_new_string("uplinkDown")); json_object_object_add(json, "protodownReasons", json_array); } From 47c1d76a6c79fb0460a7079f9e24228bc9052877 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Feb 2022 18:53:01 -0500 Subject: [PATCH 24/24] zebra: cleanup protodown netlink logs Cleanup the logs in the netlink code for setting protodown on/off to be more useful to a user parsing them after an issue. Signed-off-by: Stephen Worley --- zebra/if_netlink.c | 30 +++++++++++++++++++++--------- zebra/zebra_evpn_mh.c | 3 ++- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index e21ff533ec..f26f798364 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -870,16 +870,27 @@ static void netlink_proc_dplane_if_protodown(struct zebra_if *zif, if (zebra_evpn_is_es_bond_member(zif->ifp)) { /* Check it's not already being sent to the dplane first */ - if (protodown && CHECK_FLAG(zif->flags, ZIF_FLAG_SET_PROTODOWN)) + if (protodown && + CHECK_FLAG(zif->flags, ZIF_FLAG_SET_PROTODOWN)) { + if (IS_ZEBRA_DEBUG_EVPN_MH_ES || IS_ZEBRA_DEBUG_KERNEL) + zlog_debug( + "bond mbr %s protodown on recv'd but already sent protodown on to the dplane", + zif->ifp->name); return; + } - if (!protodown - && CHECK_FLAG(zif->flags, ZIF_FLAG_UNSET_PROTODOWN)) + if (!protodown && + CHECK_FLAG(zif->flags, ZIF_FLAG_UNSET_PROTODOWN)) { + if (IS_ZEBRA_DEBUG_EVPN_MH_ES || IS_ZEBRA_DEBUG_KERNEL) + zlog_debug( + "bond mbr %s protodown off recv'd but already sent protodown off to the dplane", + zif->ifp->name); return; + } if (IS_ZEBRA_DEBUG_EVPN_MH_ES || IS_ZEBRA_DEBUG_KERNEL) zlog_debug( - "bond mbr %s re-instate protdown %s in the dplane", + "bond mbr %s reinstate protodown %s in the dplane", zif->ifp->name, old_protodown ? "on" : "off"); if (old_protodown) @@ -2227,8 +2238,9 @@ void interface_list(struct zebra_ns *zns) void if_netlink_set_frr_protodown_r_bit(uint8_t bit) { if (IS_ZEBRA_DEBUG_KERNEL) - zlog_debug("FRR protodown reason bit change %u -> %u", - frr_protodown_r_bit, bit); + zlog_debug( + "Protodown reason bit index changed: bit-index %u -> bit-index %u", + frr_protodown_r_bit, bit); frr_protodown_r_bit = bit; } @@ -2236,9 +2248,9 @@ void if_netlink_set_frr_protodown_r_bit(uint8_t bit) void if_netlink_unset_frr_protodown_r_bit(void) { if (IS_ZEBRA_DEBUG_KERNEL) - zlog_debug("FRR protodown reason bit change %u -> %u", - frr_protodown_r_bit, - FRR_PROTODOWN_REASON_DEFAULT_BIT); + zlog_debug( + "Protodown reason bit index changed: bit-index %u -> bit-index %u", + frr_protodown_r_bit, FRR_PROTODOWN_REASON_DEFAULT_BIT); frr_protodown_r_bit = FRR_PROTODOWN_REASON_DEFAULT_BIT; } diff --git a/zebra/zebra_evpn_mh.c b/zebra/zebra_evpn_mh.c index 43eef69be2..9099c066b1 100644 --- a/zebra/zebra_evpn_mh.c +++ b/zebra/zebra_evpn_mh.c @@ -3470,7 +3470,8 @@ void zebra_evpn_mh_json(json_object *json) json_object_new_string("startupDelay")); if (CHECK_FLAG(zmh_info->protodown_rc, ZEBRA_PROTODOWN_EVPN_UPLINK_DOWN)) - json_object_array_add(json_array, + json_object_array_add( + json_array, json_object_new_string("uplinkDown")); json_object_object_add(json, "protodownReasons", json_array); }