From f183e380fae61b7c1f89fed6e32ed5a9d1ede8a8 Mon Sep 17 00:00:00 2001 From: Mark Stapp Date: Fri, 17 Aug 2018 16:53:24 -0400 Subject: [PATCH] zebra: add handy res2str utility Add a 2str utility for dplane result codes; use it in a debug or two. Signed-off-by: Mark Stapp --- zebra/rt_netlink.c | 21 ++++++++++++ zebra/rt_socket.c | 79 +++++++++++++++++++++++--------------------- zebra/zebra_dplane.c | 29 +++++++++++++++- zebra/zebra_dplane.h | 1 + zebra/zebra_rib.c | 48 ++++++++++++++++++++++----- zebra/zebra_rnh.c | 17 +++++++++- 6 files changed, 146 insertions(+), 49 deletions(-) diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c index ab1d80f90..909a155ee 100644 --- a/zebra/rt_netlink.c +++ b/zebra/rt_netlink.c @@ -1965,6 +1965,7 @@ static int netlink_route_multipath_ctx(int cmd, dplane_ctx_h ctx) setsrc = 1; } } + continue; } if ((cmd == RTM_NEWROUTE @@ -2205,6 +2206,7 @@ enum zebra_dplane_result kernel_route_update(dplane_ctx_h ctx) { int cmd, ret; const struct prefix *p = dplane_ctx_get_dest(ctx); + struct nexthop *nexthop; if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_DELETE) { cmd = RTM_DELROUTE; @@ -2237,6 +2239,25 @@ enum zebra_dplane_result kernel_route_update(dplane_ctx_h ctx) } ret = netlink_route_multipath_ctx(cmd, ctx); + if ((cmd == RTM_NEWROUTE) && (ret == 0)) { + /* Update installed nexthops to signal which have been + * installed. + */ + for (ALL_NEXTHOPS_PTR(dplane_ctx_get_ng(ctx), nexthop)) { + if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE)) + continue; + + if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE)) { + SET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB); + + /* If we're only allowed a single nh, don't + * continue. + */ + if (multipath_num == 1) + break; + } + } + } return (ret == 0 ? ZEBRA_DPLANE_REQUEST_SUCCESS : ZEBRA_DPLANE_REQUEST_FAILURE); diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c index 99c4474e6..d5146c643 100644 --- a/zebra/rt_socket.c +++ b/zebra/rt_socket.c @@ -182,7 +182,7 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, : NULL, smplsp, ifindex, bh_type, metric); - if (IS_ZEBRA_DEBUG_RIB) { + if (IS_ZEBRA_DEBUG_KERNEL) { if (!gate) { zlog_debug( "%s: %s: attention! gate not found for re", @@ -197,10 +197,15 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, * did its work. */ case ZEBRA_ERR_NOERROR: nexthop_num++; - if (IS_ZEBRA_DEBUG_RIB) + if (IS_ZEBRA_DEBUG_KERNEL) zlog_debug( "%s: %s: successfully did NH %s", __func__, prefix_buf, gate_buf); + + if (cmd == RTM_ADD) + SET_FLAG(nexthop->flags, + NEXTHOP_FLAG_FIB); + break; /* The only valid case for this error is kernel's @@ -216,14 +221,8 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, "%s: rtm_write() returned %d for command %d", __func__, error, cmd); continue; - break; - /* Given that our NEXTHOP_FLAG_FIB matches real kernel - * FIB, it isn't - * normal to get any other messages in ANY case. - */ - case ZEBRA_ERR_RTNOEXIST: - case ZEBRA_ERR_RTUNREACH: + /* Note any unexpected status returns */ default: flog_err( EC_LIB_SYSTEM_CALL, @@ -236,7 +235,7 @@ static int kernel_rtm_ipv4(int cmd, const struct prefix *p, break; } } /* if (cmd and flags make sense) */ - else if (IS_ZEBRA_DEBUG_RIB) + else if (IS_ZEBRA_DEBUG_KERNEL) zlog_debug("%s: odd command %s for flags %d", __func__, lookup_msg(rtm_type_str, cmd, NULL), nexthop->flags); @@ -367,7 +366,10 @@ static int kernel_rtm_ipv6(int cmd, const struct prefix *p, (union sockunion *)mask, gate ? (union sockunion *)&sin_gate : NULL, smplsp, ifindex, bh_type, metric); - (void)error; + + /* Update installed nexthop info on success */ + if ((cmd == RTM_ADD) && (error == ZEBRA_ERR_NOERROR)) + SET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB); nexthop_num++; } @@ -408,33 +410,34 @@ enum zebra_dplane_result kernel_route_update(dplane_ctx_h ctx) goto done; } - if (zserv_privs.change(ZPRIVS_RAISE)) - zlog_err("Can't raise privileges"); - - if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_DELETE) - kernel_rtm(RTM_DELETE, dplane_ctx_get_dest(ctx), - dplane_ctx_get_ng(ctx), dplane_ctx_get_metric(ctx)); - else if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_INSTALL) - kernel_rtm(RTM_ADD, dplane_ctx_get_dest(ctx), - dplane_ctx_get_ng(ctx), dplane_ctx_get_metric(ctx)); - else if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_UPDATE) { - /* - * Must do delete and add separately - no update available - */ - kernel_rtm(RTM_DELETE, dplane_ctx_get_dest(ctx), - dplane_ctx_get_old_ng(ctx), - dplane_ctx_get_old_metric(ctx)); - - kernel_rtm(RTM_ADD, dplane_ctx_get_dest(ctx), - dplane_ctx_get_ng(ctx), dplane_ctx_get_metric(ctx)); - } else { - zlog_err("Invalid routing socket update op %u", - dplane_ctx_get_op(ctx)); - res = ZEBRA_DPLANE_REQUEST_FAILURE; - } - - if (zserv_privs.change(ZPRIVS_LOWER)) - zlog_err("Can't lower privileges"); + frr_elevate_privs(ZPRIVS_RAISE) { + + if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_DELETE) + kernel_rtm(RTM_DELETE, dplane_ctx_get_dest(ctx), + dplane_ctx_get_ng(ctx), + dplane_ctx_get_metric(ctx)); + else if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_INSTALL) + kernel_rtm(RTM_ADD, dplane_ctx_get_dest(ctx), + dplane_ctx_get_ng(ctx), + dplane_ctx_get_metric(ctx)); + else if (dplane_ctx_get_op(ctx) == DPLANE_OP_ROUTE_UPDATE) { + /* Must do delete and add separately - + * no update available + */ + kernel_rtm(RTM_DELETE, dplane_ctx_get_dest(ctx), + dplane_ctx_get_old_ng(ctx), + dplane_ctx_get_old_metric(ctx)); + + kernel_rtm(RTM_ADD, dplane_ctx_get_dest(ctx), + dplane_ctx_get_ng(ctx), + dplane_ctx_get_metric(ctx)); + } else { + zlog_err("Invalid routing socket update op %s (%u)", + dplane_op2str(dplane_ctx_get_op(ctx)), + dplane_ctx_get_op(ctx)); + res = ZEBRA_DPLANE_REQUEST_FAILURE; + } + } /* Elevated privs */ done: diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c index 468611e6f..2bc8e45eb 100644 --- a/zebra/zebra_dplane.c +++ b/zebra/zebra_dplane.c @@ -308,6 +308,25 @@ const char *dplane_op2str(enum dplane_op_e op) return ret; } +const char *dplane_res2str(enum zebra_dplane_result res) +{ + const char *ret = ""; + + switch (res) { + case ZEBRA_DPLANE_REQUEST_FAILURE: + ret = "FAILURE"; + break; + case ZEBRA_DPLANE_REQUEST_QUEUED: + ret = "QUEUED"; + break; + case ZEBRA_DPLANE_REQUEST_SUCCESS: + ret = "SUCCESS"; + break; + }; + + return ret; +} + const struct prefix *dplane_ctx_get_dest(const dplane_ctx_h ctx) { DPLANE_CTX_VALID(ctx); @@ -500,6 +519,7 @@ static int dplane_ctx_route_init(dplane_ctx_h ctx, const struct prefix *p, *src_p; struct zebra_ns *zns; struct zebra_vrf *zvrf; + struct nexthop *nexthop; if (!ctx || !rn || !re) goto done; @@ -558,6 +578,11 @@ static int dplane_ctx_route_init(dplane_ctx_h ctx, /* TODO -- maybe use array of nexthops to avoid allocs? */ + /* Ensure that the dplane's nexthop flag is clear. */ + for (ALL_NEXTHOPS(ctx->zd_ng, nexthop)) { + UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB); + } + /* Trying out the sequence number idea, so we can try to detect * when a result is stale. */ @@ -654,7 +679,9 @@ dplane_route_update_internal(struct route_node *rn, ctx->zd_old_metric = old_re->metric; #ifndef HAVE_NETLINK - /* Capture previous re's nexthops too for bsd, sigh */ + /* For bsd, capture previous re's nexthops too, sigh. + * We'll need these to do per-nexthop deletes. + */ copy_nexthops(&(ctx->zd_old_ng.nexthop), old_re->ng.nexthop, NULL); #endif /* !HAVE_NETLINK */ diff --git a/zebra/zebra_dplane.h b/zebra/zebra_dplane.h index aa7443cf8..37cd2ca7f 100644 --- a/zebra/zebra_dplane.h +++ b/zebra/zebra_dplane.h @@ -134,6 +134,7 @@ void dplane_ctx_dequeue(struct dplane_ctx_q_s *q, dplane_ctx_h *ctxp); * Accessors for information from the context object */ enum zebra_dplane_result dplane_ctx_get_status(const dplane_ctx_h ctx); +const char *dplane_res2str(enum zebra_dplane_result res); enum dplane_op_e dplane_ctx_get_op(const dplane_ctx_h ctx); const char *dplane_op2str(enum dplane_op_e op); diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 964885f47..7a3b902b1 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -1951,7 +1951,7 @@ static void rib_process_after(dplane_ctx_h ctx) struct route_node *rn = NULL; struct route_entry *re = NULL, *old_re = NULL, *rib; bool is_update = false; - struct nexthop *nexthop; + struct nexthop *nexthop, *ctx_nexthop; char dest_str[PREFIX_STRLEN] = ""; enum dplane_op_e op; enum zebra_dplane_result status; @@ -2000,9 +2000,9 @@ static void rib_process_after(dplane_ctx_h ctx) status = dplane_ctx_get_status(ctx); if (IS_ZEBRA_DEBUG_DPLANE_DETAIL) { - zlog_debug("%u:%s Processing dplane ctx %p, op %s result %d", + zlog_debug("%u:%s Processing dplane ctx %p, op %s result %s", dplane_ctx_get_vrf(ctx), dest_str, ctx, - dplane_op2str(op), status); + dplane_op2str(op), dplane_res2str(status)); } if (op == DPLANE_OP_ROUTE_DELETE) { @@ -2090,19 +2090,34 @@ static void rib_process_after(dplane_ctx_h ctx) } if (status == ZEBRA_DPLANE_REQUEST_SUCCESS) { - /* Set nexthop FIB flags */ - for (ALL_NEXTHOPS(re->ng, nexthop)) { + /* Update zebra nexthop FIB flag for each + * nexthop that was installed. + */ + for (ALL_NEXTHOPS_PTR(dplane_ctx_get_ng(ctx), ctx_nexthop)) { + + for (ALL_NEXTHOPS(re->ng, nexthop)) { + if (nexthop_same(ctx_nexthop, nexthop)) + break; + } + + if (nexthop == NULL) + continue; + if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE)) continue; - if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE)) + if (CHECK_FLAG(ctx_nexthop->flags, + NEXTHOP_FLAG_FIB)) SET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB); else UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB); } - if (zvrf) + if (zvrf) { zvrf->installs++; + /* Set flag for nexthop tracking processing */ + zvrf->flags |= ZEBRA_VRF_RIB_SCHEDULED; + } /* Redistribute */ /* TODO -- still calling the redist api using the route_entries, @@ -2179,9 +2194,9 @@ static unsigned int process_subq(struct list *subq, uint8_t qindex) } /* - * All meta queues have been processed. Trigger next-hop evaluation. + * Perform next-hop tracking processing after RIB updates. */ -static void meta_queue_process_complete(struct work_queue *dummy) +static void do_nht_processing(void) { struct vrf *vrf; struct zebra_vrf *zvrf; @@ -2196,6 +2211,10 @@ static void meta_queue_process_complete(struct work_queue *dummy) if (zvrf == NULL || !(zvrf->flags & ZEBRA_VRF_RIB_SCHEDULED)) continue; + if (IS_ZEBRA_DEBUG_RIB_DETAILED || IS_ZEBRA_DEBUG_NHT) + zlog_debug("NHT processing check for zvrf %s", + zvrf_name(zvrf)); + zvrf->flags &= ~ZEBRA_VRF_RIB_SCHEDULED; zebra_evaluate_rnh(zvrf, AF_INET, 0, RNH_NEXTHOP_TYPE, NULL); zebra_evaluate_rnh(zvrf, AF_INET, 0, RNH_IMPORT_CHECK_TYPE, @@ -2217,6 +2236,14 @@ static void meta_queue_process_complete(struct work_queue *dummy) } } +/* + * All meta queues have been processed. Trigger next-hop evaluation. + */ +static void meta_queue_process_complete(struct work_queue *dummy) +{ + do_nht_processing(); +} + /* Dispatch the meta queue by picking, processing and unlocking the next RN from * a non-empty sub-queue with lowest priority. wq is equal to zebra->ribq and * data @@ -3300,6 +3327,9 @@ static int rib_process_dplane_results(struct thread *thread) } while (1); + /* Check for nexthop tracking processing after finishing with results */ + do_nht_processing(); + return 0; } diff --git a/zebra/zebra_rnh.c b/zebra/zebra_rnh.c index 0a531990d..838a623ef 100644 --- a/zebra/zebra_rnh.c +++ b/zebra/zebra_rnh.c @@ -530,6 +530,7 @@ zebra_rnh_resolve_nexthop_entry(struct zebra_vrf *zvrf, int family, struct route_table *route_table; struct route_node *rn; struct route_entry *re; + struct nexthop *nexthop; *prn = NULL; @@ -562,12 +563,26 @@ zebra_rnh_resolve_nexthop_entry(struct zebra_vrf *zvrf, int family, if (!CHECK_FLAG(re->flags, ZEBRA_FLAG_SELECTED)) continue; + /* Just being SELECTED isn't quite enough - must + * have an installed nexthop to be useful. + */ + for (nexthop = re->ng.nexthop; nexthop; + nexthop = nexthop->next) + if ((CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB) + || CHECK_FLAG(nexthop->flags, + NEXTHOP_FLAG_RECURSIVE)) + && CHECK_FLAG(nexthop->flags, + NEXTHOP_FLAG_ACTIVE)) + break; + + if (nexthop == NULL) + continue; + if (CHECK_FLAG(rnh->flags, ZEBRA_NHT_CONNECTED)) { if ((re->type == ZEBRA_ROUTE_CONNECT) || (re->type == ZEBRA_ROUTE_STATIC)) break; if (re->type == ZEBRA_ROUTE_NHRP) { - struct nexthop *nexthop; for (nexthop = re->ng.nexthop; nexthop; nexthop = nexthop->next) -- 2.39.5