From 71ef5cbb9563e09a76996448a7f34cec37ed3c15 Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Fri, 21 Jan 2022 04:03:01 -0500 Subject: [PATCH] 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));