From 2dd192b76774beb9c7960527fb3f397a2848f679 Mon Sep 17 00:00:00 2001 From: Dave Barach Date: Mon, 19 Nov 2018 09:31:48 -0500 Subject: [PATCH] Improve feature arc order constraint specification Add the VNET_FEATURE_ARC_ORDER macro, which allows specification of bulk order constraints. Here's an example: VNET_FEATURE_ARC_ORDER(ip4_unicast_arc_order, static) = { .arc_name = "ip4-unicast", .node_names = VNET_FEATURES ("ip4-flow-classify", "ip4-inacl", "ip4-source-check-via-rx", "ip4-source-check-via-any", "ip4-source-and-port-range-check-rx", "ip4-policer-classify", "ipsec4-input", "vpath-input-ip4", "ip4-vxlan-bypass", "ip4-not-enabled", "ip4-lookup"), }; Simply list feature nodes in the desired order, and you're done. Multiple macro instances per are are fine / expected / tested. Under the covers: generate "a before b" tuples by chain-dragging across the ordered list. No need to touch existing per-feature constraints. Fixed a long-broken "you lose!" error message. Change-Id: I259282e426fd305e22c8d65886787c41a1d348d3 Signed-off-by: Dave Barach --- src/vnet/feature/feature.c | 37 +++++++++++++++++++++---- src/vnet/feature/feature.h | 61 +++++++++++++++++++++++++++++++++++++---- src/vnet/feature/registration.c | 50 +++++++++++++++++++++++++++++++-- 3 files changed, 135 insertions(+), 13 deletions(-) diff --git a/src/vnet/feature/feature.c b/src/vnet/feature/feature.c index 6525bcbdce5..2cdbcff88c8 100644 --- a/src/vnet/feature/feature.c +++ b/src/vnet/feature/feature.c @@ -24,6 +24,7 @@ vnet_feature_init (vlib_main_t * vm) vnet_feature_main_t *fm = &feature_main; vnet_feature_registration_t *freg; vnet_feature_arc_registration_t *areg; + vnet_feature_constraint_registration_t *creg; u32 arc_index = 0; fm->arc_index_by_name = hash_create_string (0, sizeof (uword)); @@ -58,6 +59,7 @@ vnet_feature_init (vlib_main_t * vm) vec_validate (fm->next_feature_by_name, arc_index - 1); vec_validate (fm->sw_if_index_has_features, arc_index - 1); vec_validate (fm->feature_count_by_sw_if_index, arc_index - 1); + vec_validate (fm->next_constraint_by_arc, arc_index - 1); freg = fm->next_feature; while (freg) @@ -82,6 +84,31 @@ vnet_feature_init (vlib_main_t * vm) freg = next; } + /* Move bulk constraints to the constraint by arc lists */ + creg = fm->next_constraint; + while (creg) + { + vnet_feature_constraint_registration_t *next; + uword *p = hash_get_mem (fm->arc_index_by_name, creg->arc_name); + if (p == 0) + { + /* Don't start vpp with broken features arcs */ + clib_warning ("Unknown feature arc '%s'", creg->arc_name); + os_exit (1); + } + + areg = uword_to_pointer (p[0], vnet_feature_arc_registration_t *); + arc_index = areg->feature_arc_index; + + next = creg->next; + creg->next_in_arc = fm->next_constraint_by_arc[arc_index]; + fm->next_constraint_by_arc[arc_index] = creg; + + /* next */ + creg = next; + } + + areg = fm->next_arc; while (areg) { @@ -92,11 +119,11 @@ vnet_feature_init (vlib_main_t * vm) arc_index = areg->feature_arc_index; cm = &fm->feature_config_mains[arc_index]; vcm = &cm->config_main; - if ((error = vnet_feature_arc_init (vm, vcm, - areg->start_nodes, - areg->n_start_nodes, - fm->next_feature_by_arc[arc_index], - &fm->feature_nodes[arc_index]))) + if ((error = vnet_feature_arc_init + (vm, vcm, areg->start_nodes, areg->n_start_nodes, + fm->next_feature_by_arc[arc_index], + fm->next_constraint_by_arc[arc_index], + &fm->feature_nodes[arc_index]))) { clib_error_report (error); os_exit (1); diff --git a/src/vnet/feature/feature.h b/src/vnet/feature/feature.h index eb9b7b06243..5c202dda274 100644 --- a/src/vnet/feature/feature.h +++ b/src/vnet/feature/feature.h @@ -60,6 +60,21 @@ typedef struct _vnet_feature_registration vnet_feature_enable_disable_function_t *enable_disable_cb; } vnet_feature_registration_t; +/** constraint registration object */ +typedef struct _vnet_feature_constraint_registration +{ + /** next constraint set in list of all registrations*/ + struct _vnet_feature_constraint_registration *next, *next_in_arc; + /** Feature arc name */ + char *arc_name; + + /** Feature arc index, assigned by init function */ + u8 feature_arc_index; + + /** Node names, to run in the specified order */ + char **node_names; +} vnet_feature_constraint_registration_t; + typedef struct vnet_feature_config_main_t_ { vnet_config_main_t config_main; @@ -75,6 +90,8 @@ typedef struct /** feature path configuration lists */ vnet_feature_registration_t *next_feature; vnet_feature_registration_t **next_feature_by_arc; + vnet_feature_constraint_registration_t *next_constraint; + vnet_feature_constraint_registration_t **next_constraint_by_arc; uword **next_feature_by_name; /** feature config main objects */ @@ -139,6 +156,28 @@ static void __vnet_rm_feature_registration_##x (void) \ VLIB_REMOVE_FROM_LINKED_LIST (fm->next_feature, r, next); \ } \ __VA_ARGS__ vnet_feature_registration_t vnet_feat_##x + +#define VNET_FEATURE_ARC_ORDER(x,...) \ + __VA_ARGS__ vnet_feature_constraint_registration_t \ +vnet_feature_constraint_##x; \ +static void __vnet_add_constraint_registration_##x (void) \ + __attribute__((__constructor__)) ; \ +static void __vnet_add_constraint_registration_##x (void) \ +{ \ + vnet_feature_main_t * fm = &feature_main; \ + vnet_feature_constraint_##x.next = fm->next_constraint; \ + fm->next_constraint = & vnet_feature_constraint_##x; \ +} \ +static void __vnet_rm_constraint_registration_##x (void) \ + __attribute__((__destructor__)) ; \ +static void __vnet_rm_constraint_registration_##x (void) \ +{ \ + vnet_feature_main_t * fm = &feature_main; \ + vnet_feature_constraint_registration_t *r = &vnet_feature_constraint_##x; \ + VLIB_REMOVE_FROM_LINKED_LIST (fm->next_constraint, r, next); \ +} \ +__VA_ARGS__ vnet_feature_constraint_registration_t vnet_feature_constraint_##x + #else #define VNET_FEATURE_ARC_INIT(x,...) \ extern vnet_feature_arc_registration_t __clib_unused vnet_feat_arc_##x; \ @@ -146,6 +185,14 @@ static vnet_feature_arc_registration_t __clib_unused __clib_unused_vnet_feat_arc #define VNET_FEATURE_INIT(x,...) \ extern vnet_feature_registration_t __clib_unused vnet_feat_##x; \ static vnet_feature_registration_t __clib_unused __clib_unused_vnet_feat_##x + +#define VNET_FEATURE_ARC_ORDER(x,...) \ +extern vnet_feature_constraint_registration_t \ +__clib_unused vnet_feature_constraint_##x; \ +static vnet_feature_constraint_registration_t __clib_unused \ +__clib_unused_vnet_feature_constraint_##x + + #endif void @@ -385,12 +432,14 @@ vnet_feature_start_device_input_x4 (u32 sw_if_index, #define VNET_FEATURES(...) (char*[]) { __VA_ARGS__, 0} -clib_error_t *vnet_feature_arc_init (vlib_main_t * vm, - vnet_config_main_t * vcm, - char **feature_start_nodes, - int num_feature_start_nodes, - vnet_feature_registration_t * - first_reg, char ***feature_nodes); +clib_error_t *vnet_feature_arc_init + (vlib_main_t * vm, + vnet_config_main_t * vcm, + char **feature_start_nodes, + int num_feature_start_nodes, + vnet_feature_registration_t * first_reg, + vnet_feature_constraint_registration_t * first_const_set, + char ***in_feature_nodes); void vnet_interface_features_show (vlib_main_t * vm, u32 sw_if_index, int verbose); diff --git a/src/vnet/feature/registration.c b/src/vnet/feature/registration.c index 61024ca08cf..fb10fbbe0db 100644 --- a/src/vnet/feature/registration.c +++ b/src/vnet/feature/registration.c @@ -107,6 +107,9 @@ comma_split (u8 * s, u8 ** a, u8 ** b) * @param first_reg first element in * [an __attribute__((constructor)) function built, or * otherwise created] singly-linked list of feature registrations + * @param first_const first element in + * [an __attribute__((constructor)) function built, or + * otherwise created] singly-linked list of bulk order constraints * @param [out] in_feature_nodes returned vector of * topologically-sorted feature node names, for use in * show commands @@ -120,12 +123,14 @@ vnet_feature_arc_init (vlib_main_t * vm, char **feature_start_nodes, int num_feature_start_nodes, vnet_feature_registration_t * first_reg, - char ***in_feature_nodes) + vnet_feature_constraint_registration_t * + first_const_set, char ***in_feature_nodes) { uword *index_by_name; uword *reg_by_index; u8 **node_names = 0; u8 *node_name; + char *prev_name; char **these_constraints; char *this_constraint_c; u8 **constraints = 0; @@ -139,6 +144,7 @@ vnet_feature_arc_init (vlib_main_t * vm, int n_features; u32 *result = 0; vnet_feature_registration_t *this_reg = 0; + vnet_feature_constraint_registration_t *this_const_set = 0; char **feature_nodes = 0; hash_pair_t *hp; u8 **keys_to_delete = 0; @@ -183,6 +189,44 @@ vnet_feature_arc_init (vlib_main_t * vm, this_reg = this_reg->next_in_arc; } + /* pass 2, collect bulk "a then b then c then d" constraints */ + this_const_set = first_const_set; + while (this_const_set) + { + these_constraints = this_const_set->node_names; + + prev_name = 0; + /* Across the list of constraints */ + while (these_constraints && these_constraints[0]) + { + this_constraint_c = these_constraints[0]; + p = hash_get_mem (index_by_name, this_constraint_c); + if (p == 0) + { + clib_warning + ("bulk constraint feature node '%s' not found for arc '%s'", + this_constraint_c); + these_constraints++; + continue; + } + + if (prev_name == 0) + { + prev_name = this_constraint_c; + these_constraints++; + continue; + } + + constraint_tuple = format (0, "%s,%s%c", prev_name, + this_constraint_c, 0); + vec_add1 (constraints, constraint_tuple); + prev_name = this_constraint_c; + these_constraints++; + } + + this_const_set = this_const_set->next_in_arc; + } + n_features = vec_len (node_names); orig = clib_ptclosure_alloc (n_features); @@ -252,7 +296,9 @@ again: /* see if we got a partial order... */ if (vec_len (result) != n_features) - return clib_error_return (0, "%d feature_init_cast no partial order!"); + return clib_error_return + (0, "Arc '%s': failed to find a suitable feature order!", + first_reg->arc_name); /* * We win. -- 2.16.6