When an adjacency changes from incomplete to complete (arp to rewrite) and vice-versa, that update must produce valid behavour w.r.t. the packets that continue to encounter it.
Change-Id: Icc2709aff9807c5d24f91ef7649268991ad9a45d
Signed-off-by: Neale Ranns <nranns@cisco.com>
adj_get_midchain_node(adj->ia_link),
adj->sub_type.midchain.tx_function_node,
rewrite);
-
- /*
- * time for walkies fido.
- */
- fib_node_back_walk_ctx_t bw_ctx = {
- .fnbw_reason = FIB_NODE_BW_REASON_FLAG_ADJ_UPDATE,
- };
-
- fib_walk_sync(FIB_NODE_TYPE_ADJ, adj_get_index(adj), &bw_ctx);
}
/**
u8 *rewrite)
{
ip_adjacency_t *adj;
- u32 old_next;
ASSERT(ADJ_INDEX_INVALID != adj_index);
adj = adj_get(adj_index);
- old_next = adj->lookup_next_index;
if (flags & ADJ_NBR_REWRITE_FLAG_COMPLETE)
{
adj->rewrite_header.sw_if_index),
rewrite);
}
-
- if (old_next != adj->lookup_next_index)
- {
- /*
- * time for walkies fido.
- * The link type MPLS Adj never has children. So if it is this adj
- * that is updated, we need to walk from its IP sibling.
- */
- if (VNET_LINK_MPLS == adj->ia_link)
- {
- adj_index = adj_nbr_find(adj->ia_nh_proto,
- fib_proto_to_link(adj->ia_nh_proto),
- &adj->sub_type.nbr.next_hop,
- adj->rewrite_header.sw_if_index);
-
- ASSERT(ADJ_INDEX_INVALID != adj_index);
- }
-
- fib_node_back_walk_ctx_t bw_ctx = {
- .fnbw_reason = FIB_NODE_BW_REASON_FLAG_ADJ_UPDATE,
- /*
- * This walk only needs to go back one level, but there is no control
- * here. the first receiving fib_entry_t will quash the walk
- */
- };
-
- fib_walk_sync(FIB_NODE_TYPE_ADJ, adj_index, &bw_ctx);
- }
}
/**
u32 next_node,
u8 *rewrite)
{
- vlib_main_t * vm = vlib_get_main();
+ adj_index_t walk_ai;
+ vlib_main_t * vm;
+ u32 old_next;
+
+ vm = vlib_get_main();
+ old_next = adj->lookup_next_index;
+
+ walk_ai = adj_get_index(adj);
+ if (VNET_LINK_MPLS == adj->ia_link)
+ {
+ /*
+ * The link type MPLS has no children in the control plane graph, it only
+ * has children in the data-palne graph. The backwalk is up the former.
+ * So we need to walk from its IP cousin.
+ */
+ walk_ai = adj_nbr_find(adj->ia_nh_proto,
+ fib_proto_to_link(adj->ia_nh_proto),
+ &adj->sub_type.nbr.next_hop,
+ adj->rewrite_header.sw_if_index);
+ }
/*
* Updating a rewrite string is not atomic;
* - the rewrite string is too long to write in one instruction
* - when swapping from incomplete to complete, we also need to update
- * the VLIB graph next-index.
+ * the VLIB graph next-index of the adj.
* ideally we would only want to suspend forwarding via this adj whilst we
* do this, but we do not have that level of granularity - it's suspend all
* worker threads or nothing.
* from the set.
* - update the next_node index of this adj to point to error-drop
* both of which will mean for MAC change we will drop for this adj
- * which is not acceptable.
- * So the pause all threads is preferable. We don't update MAC addresses often
- * so it's no big deal.
+ * which is not acceptable. However, when the adj changes type (from
+ * complete to incomplete and vice-versa) the child DPOs, which have the
+ * VLIB graph next node index, will be sending packets to the wrong graph
+ * node. So from the options above, updating the next_node of the adj to
+ * be drop will work, but it relies on each graph node v4/v6/mpls, rewrite/
+ * arp/midchain always be valid w.r.t. a mis-match of adj type and node type
+ * (i.e. a rewrite adj in the arp node). This is not enforcable. Getting it
+ * wrong will lead to hard to find bugs since its a race condition. So we
+ * choose the more reliable method of updating the children to use the drop,
+ * then switching adj's type, then updating the children again. Did I mention
+ * that this doesn't happen often...
+ * So we need to distinguish between the two cases:
+ * 1 - mac change
+ * 2 - adj type change
+ */
+ if (old_next != adj_next_index &&
+ ADJ_INDEX_INVALID != walk_ai)
+ {
+ /*
+ * the adj is changing type. we need to fix all children so that they
+ * stack momentarily on a drop, while the adj changes. If we don't do
+ * this the children will send packets to a VLIB graph node that does
+ * not correspond to the adj's type - and it goes downhill from there.
+ */
+ fib_node_back_walk_ctx_t bw_ctx = {
+ .fnbw_reason = FIB_NODE_BW_REASON_FLAG_ADJ_DOWN,
+ /*
+ * force this walk to be synchrous. if we don't and a node in the graph
+ * (a heavily shared path-list) chooses to back-ground the walk (make it
+ * async) then it will pause and we will do the adj update below, before
+ * all the children are updated. not good.
+ */
+ .fnbw_flags = FIB_NODE_BW_FLAG_FORCE_SYNC,
+ };
+
+ fib_walk_sync(FIB_NODE_TYPE_ADJ, walk_ai, &bw_ctx);
+ }
+
+ /*
+ * If we are just updating the MAC string of the adj (which we also can't
+ * do atomically), then we need to stop packets switching through the adj.
+ * We can't do that on a per-adj basis, so it's all the packets.
+ * If we are updating the type, and we walked back to the children above,
+ * then this barrier serves to flush the queues/frames.
*/
vlib_worker_thread_barrier_sync(vm);
sizeof(adj->rewrite_data),
rewrite,
vec_len(rewrite));
-
- adj->rewrite_header.node_index = this_node;
- adj->rewrite_header.next_index = vlib_node_add_next (vlib_get_main(),
- this_node,
- next_node);
-
vec_free(rewrite);
}
else
vnet_rewrite_clear_data_internal(&adj->rewrite_header,
sizeof(adj->rewrite_data));
}
+ adj->rewrite_header.node_index = this_node;
+ adj->rewrite_header.next_index = vlib_node_add_next(vlib_get_main(),
+ this_node,
+ next_node);
/*
* done with the rewirte update - let the workers loose.
*/
vlib_worker_thread_barrier_release(vm);
+
+ if (old_next != adj->lookup_next_index &&
+ ADJ_INDEX_INVALID != walk_ai)
+ {
+ /*
+ * backwalk to the children so they can stack on the now updated
+ * adjacency
+ */
+ fib_node_back_walk_ctx_t bw_ctx = {
+ .fnbw_reason = FIB_NODE_BW_REASON_FLAG_ADJ_UPDATE,
+ };
+
+ fib_walk_sync(FIB_NODE_TYPE_ADJ, walk_ai, &bw_ctx);
+ }
}
typedef struct adj_db_count_ctx_t_ {
*/
static dpo_type_t dpo_dynamic = DPO_LAST;
+dpo_proto_t
+vnet_link_to_dpo_proto (vnet_link_t linkt)
+{
+ switch (linkt)
+ {
+ case VNET_LINK_IP6:
+ return (DPO_PROTO_IP6);
+ case VNET_LINK_IP4:
+ return (DPO_PROTO_IP4);
+ case VNET_LINK_MPLS:
+ return (DPO_PROTO_MPLS);
+ case VNET_LINK_ETHERNET:
+ return (DPO_PROTO_ETHERNET);
+ case VNET_LINK_ARP:
+ break;
+ }
+ ASSERT(0);
+ return (0);
+}
+
u8 *
format_dpo_type (u8 * s, va_list * args)
{
void
dpo_reset (dpo_id_t *dpo)
{
- dpo_set(dpo, DPO_FIRST, DPO_PROTO_NONE, INDEX_INVALID);
+ dpo_id_t tmp = DPO_INVALID;
+
+ /*
+ * use the atomic copy operation.
+ */
+ dpo_copy(dpo, &tmp);
}
/**
"atomic updates need to be revisited");
/**
- * @brief An initialiser for DPos declared on the stack.
+ * @brief An initialiser for DPOs declared on the stack.
+ * Thenext node is set to 0 since VLIB graph nodes should set 0 index to drop.
*/
-#define DPO_INVALID {0}
+#define DPO_INVALID \
+{ \
+ .dpoi_type = DPO_FIRST, \
+ .dpoi_proto = DPO_PROTO_NONE, \
+ .dpoi_index = INDEX_INVALID, \
+ .dpoi_next_node = 0, \
+}
/**
* @brief Return true if the DPO object is valid, i.e. has been initialised.
dpoi->dpoi_index != INDEX_INVALID);
}
+extern dpo_proto_t vnet_link_to_dpo_proto(vnet_link_t linkt);
+
/**
* @brief
* Take a reference counting lock on the DPO
if (FIB_NODE_BW_REASON_FLAG_EVALUATE & ctx->fnbw_reason ||
FIB_NODE_BW_REASON_FLAG_ADJ_UPDATE & ctx->fnbw_reason ||
+ FIB_NODE_BW_REASON_FLAG_ADJ_DOWN & ctx->fnbw_reason ||
FIB_NODE_BW_REASON_FLAG_INTERFACE_UP & ctx->fnbw_reason ||
FIB_NODE_BW_REASON_FLAG_INTERFACE_DOWN & ctx->fnbw_reason ||
FIB_NODE_BW_REASON_FLAG_INTERFACE_DELETE & ctx->fnbw_reason)
*/
ctx->fnbw_reason = FIB_NODE_BW_REASON_FLAG_EVALUATE;
+ /*
+ * ... and nothing is forced sync from now on.
+ */
+ ctx->fnbw_flags &= ~FIB_NODE_BW_FLAG_FORCE_SYNC;
+
/*
* propagate the backwalk further if we haven't already reached the
* maximum depth.
* a unipath adjacency changes
*/
FIB_NODE_BW_REASON_ADJ_UPDATE,
+ /**
+ * Walk to update children to inform them the adjacency is now down.
+ */
+ FIB_NODE_BW_REASON_ADJ_DOWN,
/**
* Marker. Add new before and update
*/
- FIB_NODE_BW_REASON_LAST = FIB_NODE_BW_REASON_EVALUATE,
+ FIB_NODE_BW_REASON_LAST = FIB_NODE_BW_REASON_ADJ_DOWN,
} fib_node_back_walk_reason_t;
-#define FIB_NODE_BW_REASONS { \
- [FIB_NODE_BW_REASON_RESOLVE] = "resolve" \
- [FIB_NODE_BW_REASON_EVALUATE] = "evaluate" \
- [FIB_NODE_BW_REASON_INTERFACE_UP] = "if-up" \
- [FIB_NODE_BW_REASON_INTERFACE_DOWN] = "if-down" \
- [FIB_NODE_BW_REASON_INTERFACE_DELETE] = "if-delete" \
- [FIB_NODE_BW_REASON_ADJ_UPDATE] = "adj-update" \
+#define FIB_NODE_BW_REASONS { \
+ [FIB_NODE_BW_REASON_RESOLVE] = "resolve", \
+ [FIB_NODE_BW_REASON_EVALUATE] = "evaluate", \
+ [FIB_NODE_BW_REASON_INTERFACE_UP] = "if-up", \
+ [FIB_NODE_BW_REASON_INTERFACE_DOWN] = "if-down", \
+ [FIB_NODE_BW_REASON_INTERFACE_DELETE] = "if-delete", \
+ [FIB_NODE_BW_REASON_ADJ_UPDATE] = "adj-update", \
+ [FIB_NODE_BW_REASON_ADJ_DOWN] = "adj-down", \
}
+#define FOR_EACH_FIB_NODE_BW_REASON(_item) \
+ for (_item = FIB_NODE_BW_REASON_FIRST; \
+ _item <= FIB_NODE_BW_REASON_LAST; \
+ _item++)
+
/**
* Flags enum constructed from the reaons
*/
FIB_NODE_BW_REASON_FLAG_INTERFACE_DOWN = (1 << FIB_NODE_BW_REASON_INTERFACE_DOWN),
FIB_NODE_BW_REASON_FLAG_INTERFACE_DELETE = (1 << FIB_NODE_BW_REASON_INTERFACE_DELETE),
FIB_NODE_BW_REASON_FLAG_ADJ_UPDATE = (1 << FIB_NODE_BW_REASON_ADJ_UPDATE),
+ FIB_NODE_BW_REASON_FLAG_ADJ_DOWN = (1 << FIB_NODE_BW_REASON_ADJ_DOWN),
} __attribute__ ((packed)) fib_node_bw_reason_flag_t;
STATIC_ASSERT(sizeof(fib_node_bw_reason_flag_t) < 2,
"BW Reason enum < 2 byte. Consequences for cover_upd_res_t");
+/**
+ * Flags on the walk
+ */
+typedef enum fib_node_bw_flags_t_
+{
+ /**
+ * Force the walk to be synchronous
+ */
+ FIB_NODE_BW_FLAG_FORCE_SYNC = (1 << 0),
+} fib_node_bw_flags_t;
+
/**
* Forward eclarations
*/
*/
fib_node_bw_reason_flag_t fnbw_reason;
+ /**
+ * additional flags for the walk
+ */
+ fib_node_bw_flags_t fnbw_flags;
+
/**
* the number of levels the walk has already traversed.
* this value is maintained by the walk infra, tp limit the depth of
fib_path_proto_to_chain_type(path->fp_nh_proto),
&path->fp_dpo);
}
- if (FIB_NODE_BW_REASON_FLAG_ADJ_UPDATE & ctx->fnbw_reason)
+ if ((FIB_NODE_BW_REASON_FLAG_ADJ_UPDATE & ctx->fnbw_reason) ||
+ (FIB_NODE_BW_REASON_FLAG_ADJ_DOWN & ctx->fnbw_reason))
{
/*
* ADJ updates (complete<->incomplete) do not need to propagate to
*/
adj_index_t ai;
+ if (vnet_sw_interface_is_admin_up(vnet_get_main(),
+ path->attached_next_hop.fp_interface))
+ {
+ path->fp_oper_flags |= FIB_PATH_OPER_FLAG_RESOLVED;
+ }
+
ai = fib_path_attached_next_hop_get_adj(
path,
fib_proto_to_link(path->fp_nh_proto));
ai);
adj_unlock(ai);
}
+ if (FIB_NODE_BW_REASON_FLAG_ADJ_DOWN & ctx->fnbw_reason)
+ {
+ /*
+ * the adj has gone down. the path is no longer resolved.
+ */
+ path->fp_oper_flags &= ~FIB_PATH_OPER_FLAG_RESOLVED;
+ }
break;
case FIB_PATH_TYPE_ATTACHED:
/*
return (0);
}
-dpo_proto_t
-vnet_link_to_dpo_proto (vnet_link_t linkt)
-{
- switch (linkt)
- {
- case VNET_LINK_IP6:
- return (DPO_PROTO_IP6);
- case VNET_LINK_IP4:
- return (DPO_PROTO_IP4);
- case VNET_LINK_MPLS:
- return (DPO_PROTO_MPLS);
- case VNET_LINK_ETHERNET:
- return (DPO_PROTO_ETHERNET);
- case VNET_LINK_ARP:
- break;
- }
- ASSERT(0);
- return (0);
-}
-
fib_protocol_t
dpo_proto_to_fib (dpo_proto_t dpo_proto)
{
extern u8 * format_fib_forw_chain_type(u8 * s, va_list * args);
extern dpo_proto_t fib_proto_to_dpo(fib_protocol_t fib_proto);
-extern dpo_proto_t vnet_link_to_dpo_proto(vnet_link_t linkt);
extern fib_protocol_t dpo_proto_to_fib(dpo_proto_t dpo_proto);
/**
* The names of the walk stats
*/
static const char * const fib_walk_queue_stats_names[] = FIB_WALK_QUEUE_STATS;
+/**
+ * The names of the walk reasons
+ */
+static const char * const fib_node_bw_reason_names[] = FIB_NODE_BW_REASONS;
/**
* A represenation of one queue of walk
u32 fwh_n_visits;
f64 fwh_duration;
fib_node_ptr_t fwh_parent;
+ fib_walk_flags_t fwh_flags;
+ fib_node_bw_reason_flag_t fwh_reason;
} fib_walk_history_t;
static fib_walk_history_t fib_walk_history[HISTORY_N_WALKS];
static void
fib_walk_destroy (fib_walk_t *fwalk)
{
+ fib_node_back_walk_ctx_t *ctx;
u32 bucket;
if (FIB_NODE_INDEX_INVALID != fwalk->fw_prio_sibling)
vlib_time_now(vlib_get_main()) - fwalk->fw_start_time;
fib_walk_history[history_last_walk_pos].fwh_parent =
fwalk->fw_parent;
+ fib_walk_history[history_last_walk_pos].fwh_flags =
+ fwalk->fw_flags;
+
+ fib_walk_history[history_last_walk_pos].fwh_reason = 0;
+ vec_foreach(ctx, fwalk->fw_ctx)
+ {
+ fib_walk_history[history_last_walk_pos].fwh_reason |=
+ ctx->fnbw_reason;
+ }
history_last_walk_pos = (history_last_walk_pos + 1) % HISTORY_N_WALKS;
*/
return;
}
+ if (ctx->fnbw_flags & FIB_NODE_BW_FLAG_FORCE_SYNC)
+ {
+ /*
+ * the originator of the walk wanted it to be synchronous, but the
+ * parent object chose async - denied.
+ */
+ return (fib_walk_sync(parent_type, parent_index, ctx));
+ }
+
fwalk = fib_walk_alloc(parent_type,
parent_index,
vlib_cli_output(vm, "Brief History (last %d walks):", HISTORY_N_WALKS);
- ii = history_last_walk_pos;
- do
+ ii = history_last_walk_pos - 1;
+ if (ii < 0)
+ ii = HISTORY_N_WALKS - 1;
+
+ while (ii != history_last_walk_pos)
{
if (0 != fib_walk_history[ii].fwh_n_visits)
{
- vlib_cli_output(
- vm, " %s:%d visits:%d duration:%.2f ",
- fib_node_type_get_name(fib_walk_history[ii].fwh_parent.fnp_type),
- fib_walk_history[ii].fwh_parent.fnp_index,
- fib_walk_history[ii].fwh_n_visits,
- fib_walk_history[ii].fwh_duration);
+ fib_node_back_walk_reason_t reason;
+ u8 *s = NULL;
+
+ s = format(s, " %s:%d visits:%d duration:%.2f ",
+ fib_node_type_get_name(fib_walk_history[ii].fwh_parent.fnp_type),
+ fib_walk_history[ii].fwh_parent.fnp_index,
+ fib_walk_history[ii].fwh_n_visits,
+ fib_walk_history[ii].fwh_duration);
+ if (FIB_WALK_FLAG_SYNC & fib_walk_history[ii].fwh_flags)
+ s = format(s, "sync, ");
+ if (FIB_WALK_FLAG_ASYNC & fib_walk_history[ii].fwh_flags)
+ s = format(s, "async, ");
+
+ s = format(s, "reason:");
+ FOR_EACH_FIB_NODE_BW_REASON(reason) {
+ if ((1<<reason) & fib_walk_history[ii].fwh_reason) {
+ s = format (s, "%s,", fib_node_bw_reason_names[reason]);
+ }
+ }
+ vlib_cli_output(vm, "%v", s);
}
- ii = (ii + 1) % HISTORY_N_WALKS;
- } while (ii != history_last_walk_pos);
-
+ ii--;
+ if (ii < 0)
+ ii = HISTORY_N_WALKS - 1;
+ }
return (NULL);
}
Code Review / vpp.git / commitdiff