Fix failure during enable/disable of features

vnet_feature_enable_disable_with_index() checks the
return status of vnet_config_{add,del}_feature().
If the config string heap index returned is the same
index that was in use prior to the add/delete, it is
concluded that a failure occurred and processing of
the feature stops.

Sometimes the config index that is returned
can legitimately be the same index that was in used
before the add/delete. The old list of features can
have its heap entry deallocated before a new entry for
the new list is allocated. The heap entry for the new
list can be the entry that was deallocated while
deleting the old one.

Make vnet_config_{add,del}_feature() return ~0 on
failure. Look for that return value as an indication
that an error occurred in
vnet_enable_disable_feature_by_index().

Change-Id: I88bb3ff88a76971c1b5e5ece74784ce8ba78373c
Signed-off-by: Matthew Smith <mgsmith@netgate.com>

diff --git a/src/vnet/config.c b/src/vnet/config.c
index 03189d7..26b0cad 100644 (file)
--- a/src/vnet/config.c
+++ b/src/vnet/config.c
@@ -247,7 +247,7 @@ vnet_config_add_feature (vlib_main_t * vm,
   u32 node_index = vec_elt (cm->node_index_by_feature_index, feature_index);
 
   if (node_index == ~0)                // feature node does not exist
-    return config_string_heap_index;   // return original config index
+    return ~0;
 
   if (config_string_heap_index == ~0)
     {
@@ -330,7 +330,7 @@ vnet_config_del_feature (vlib_main_t * vm,
 
   /* Feature not found. */
   if (f >= vec_end (old->features))
-    return config_string_heap_index;   // return original config index
+    return ~0;
 
   new_features = duplicate_feature_vector (old->features);
   f = new_features + (f - old->features);

diff --git a/src/vnet/feature/feature.c b/src/vnet/feature/feature.c
index 9710004..714e20e 100644 (file)
--- a/src/vnet/feature/feature.c
+++ b/src/vnet/feature/feature.c
@@ -186,7 +186,7 @@ vnet_feature_enable_disable_with_index (u8 arc_index, u32 feature_index,
   vnet_feature_main_t *fm = &feature_main;
   vnet_feature_config_main_t *cm;
   i16 feature_count;
-  u32 old_ci, ci;
+  u32 ci;
 
   if (arc_index == (u8) ~ 0)
     return VNET_API_ERROR_INVALID_VALUE;
@@ -196,7 +196,7 @@ vnet_feature_enable_disable_with_index (u8 arc_index, u32 feature_index,
 
   cm = &fm->feature_config_mains[arc_index];
   vec_validate_init_empty (cm->config_index_by_sw_if_index, sw_if_index, ~0);
-  old_ci = ci = cm->config_index_by_sw_if_index[sw_if_index];
+  ci = cm->config_index_by_sw_if_index[sw_if_index];
 
   vec_validate (fm->feature_count_by_sw_if_index[arc_index], sw_if_index);
   feature_count = fm->feature_count_by_sw_if_index[arc_index][sw_if_index];
@@ -209,7 +209,7 @@ vnet_feature_enable_disable_with_index (u8 arc_index, u32 feature_index,
        : vnet_config_del_feature)
     (vlib_get_main (), &cm->config_main, ci, feature_index, feature_config,
      n_feature_config_bytes);
-  if (old_ci == ci)
+  if (ci == ~0)
     {
       return 0;
     }