From 5d4141383344bf244c572f9d23c0175d9573f41d Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Wed, 26 Jan 2022 00:07:57 -0500 Subject: 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/if_netlink.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 129 insertions(+), 3 deletions(-) (limited to 'zebra/if_netlink.c') 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); } -- cgit v1.2.3 From c40e1b1cfbbf8db7b03f9faa51afbea8cda722c3 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Wed, 19 Jan 2022 14:36:10 -0500 Subject: 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(+) (limited to 'zebra/if_netlink.c') 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 -- cgit v1.2.3 From 0dcd8506f2dac65bcac06f79a1660d809b329340 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 25 Jan 2022 13:49:05 -0500 Subject: 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(-) (limited to 'zebra/if_netlink.c') 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 -- cgit v1.2.3 From d89b300829ff85cc4e6b51384170f67cf5f3f6a1 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 25 Jan 2022 14:44:25 -0500 Subject: 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(-) (limited to 'zebra/if_netlink.c') 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)); -- cgit v1.2.3 From e4d87b58943f3d293bb578d605bafb6d1123f847 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 25 Jan 2022 14:49:01 -0500 Subject: 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(-) (limited to 'zebra/if_netlink.c') 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"); -- cgit v1.2.3 From ab465d24bd864c7ab9f5841af89c108e03e768ac Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Mon, 31 Jan 2022 16:12:01 -0500 Subject: 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(-) (limited to 'zebra/if_netlink.c') 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); -- cgit v1.2.3 From 4b82b9548879ad3b888fd7d8b9cea99a55260c55 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Feb 2022 12:36:18 -0500 Subject: 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(+) (limited to 'zebra/if_netlink.c') 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", -- cgit v1.2.3 From ed7a1622c347bf5cb0929da07f9072e04f0730c1 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Feb 2022 17:56:50 -0500 Subject: 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(-) (limited to 'zebra/if_netlink.c') 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; -- cgit v1.2.3 From 7140b00cb0c0f9e9f007f092890f04166a9a66f8 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Feb 2022 18:21:18 -0500 Subject: 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(-) (limited to 'zebra/if_netlink.c') 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); } -- cgit v1.2.3 From 47c1d76a6c79fb0460a7079f9e24228bc9052877 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Tue, 15 Feb 2022 18:53:01 -0500 Subject: 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(-) (limited to 'zebra/if_netlink.c') 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); } -- cgit v1.2.3