Skip to content

Commit

Permalink
zebra: only clear pd_reason on shutdown/sweep
Browse files Browse the repository at this point in the history 
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 <sworley@nvidia.com>
  • Loading branch information
@sworleys
sworleys committed Mar 9, 2022
1 parent 97c7263 commit ab465d2
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 35 deletions.
22 changes: 13 additions & 9 deletions zebra/if_netlink.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand All @@ -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);
}

Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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);
}
Expand Down
45 changes: 29 additions & 16 deletions zebra/interface.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
15 changes: 8 additions & 7 deletions zebra/zebra_dplane.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions zebra/zebra_dplane.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit ab465d2

Please sign in to comment.