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 <dave@barachs.net>

diff --git a/src/vnet/feature/feature.c b/src/vnet/feature/feature.c
index 6525bcb..2cdbcff 100644 (file)
--- 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 eb9b7b0..5c202dd 100644 (file)
--- 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 61024ca..fb10fbb 100644 (file)
--- 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.