reassembly: bug fixes 95/11395/7
authorKlement Sekera <ksekera@cisco.com>
Tue, 27 Mar 2018 08:34:43 +0000 (10:34 +0200)
committerDamjan Marion <dmarion.lists@gmail.com>
Tue, 3 Apr 2018 19:56:37 +0000 (19:56 +0000)
This change fixes a bug which would corrupt features infra by making
feature infra resistant to double-removal. It also fixes 'out of memory'
issue by properly initializing the bihash tables.

Change-Id: I78ac03139234a9a0e0b48e7bdfac1c38a0069e82
Signed-off-by: Klement Sekera <ksekera@cisco.com>
src/vnet/feature/feature.c
src/vnet/ip/ip4_reassembly.c
src/vnet/ip/ip6_reassembly.c
test/test_reassembly.py

index 6756d0d..89a1951 100644 (file)
@@ -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 ci;
+  u32 old_ci, 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);
-  ci = cm->config_index_by_sw_if_index[sw_if_index];
+  old_ci = 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,6 +209,10 @@ 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)
+    {
+      return 0;
+    }
   cm->config_index_by_sw_if_index[sw_if_index] = ci;
 
   /* update feature count */
index b94a258..33cdbd5 100644 (file)
@@ -1110,8 +1110,7 @@ ip4_reass_set (u32 timeout_ms, u32 max_reassemblies,
                             ip4_reass_main.ip4_reass_expire_node_idx,
                             IP4_EVENT_CONFIG_CHANGED, 0);
   u32 new_nbuckets = ip4_reass_get_nbuckets ();
-  if (ip4_reass_main.max_reass_n > 0 && new_nbuckets > 1 &&
-      new_nbuckets != old_nbuckets)
+  if (ip4_reass_main.max_reass_n > 0 && new_nbuckets > old_nbuckets)
     {
       clib_bihash_24_8_t new_hash;
       memset (&new_hash, 0, sizeof (new_hash));
@@ -1170,6 +1169,10 @@ ip4_reass_init_function (vlib_main_t * vm)
   ASSERT (node);
   rm->ip4_reass_expire_node_idx = node->index;
 
+  ip4_reass_set_params (IP4_REASS_TIMEOUT_DEFAULT_MS,
+                       IP4_REASS_MAX_REASSEMBLIES_DEFAULT,
+                       IP4_REASS_EXPIRE_WALK_INTERVAL_DEFAULT_MS);
+
   nbuckets = ip4_reass_get_nbuckets ();
   clib_bihash_init_24_8 (&rm->hash, "ip4-reass", nbuckets, nbuckets * 1024);
 
@@ -1177,10 +1180,6 @@ ip4_reass_init_function (vlib_main_t * vm)
   ASSERT (node);
   rm->ip4_drop_idx = node->index;
 
-  ip4_reass_set_params (IP4_REASS_TIMEOUT_DEFAULT_MS,
-                       IP4_REASS_MAX_REASSEMBLIES_DEFAULT,
-                       IP4_REASS_EXPIRE_WALK_INTERVAL_DEFAULT_MS);
-
   return error;
 }
 
index 1be5b5e..8b199f3 100644 (file)
@@ -926,10 +926,13 @@ ip6_reassembly_inline (vlib_main_t * vm,
          b0 = vlib_get_buffer (vm, bi0);
 
          ip6_header_t *ip0 = vlib_buffer_get_current (b0);
-         ip6_frag_hdr_t *frag_hdr;
+         ip6_frag_hdr_t *frag_hdr = NULL;
          ip6_ext_header_t *prev_hdr;
-         ip6_ext_header_find_t (ip0, prev_hdr, frag_hdr,
-                                IP_PROTOCOL_IPV6_FRAGMENTATION);
+         if (ip6_ext_hdr (ip0->protocol))
+           {
+             ip6_ext_header_find_t (ip0, prev_hdr, frag_hdr,
+                                    IP_PROTOCOL_IPV6_FRAGMENTATION);
+           }
          if (!frag_hdr)
            {
              // this is a regular packet - no fragmentation
@@ -1146,8 +1149,7 @@ ip6_reass_set (u32 timeout_ms, u32 max_reassemblies,
                             ip6_reass_main.ip6_reass_expire_node_idx,
                             IP6_EVENT_CONFIG_CHANGED, 0);
   u32 new_nbuckets = ip6_reass_get_nbuckets ();
-  if (ip6_reass_main.max_reass_n > 0 && new_nbuckets > 1 &&
-      new_nbuckets != old_nbuckets)
+  if (ip6_reass_main.max_reass_n > 0 && new_nbuckets > old_nbuckets)
     {
       clib_bihash_48_8_t new_hash;
       memset (&new_hash, 0, sizeof (new_hash));
@@ -1206,6 +1208,10 @@ ip6_reass_init_function (vlib_main_t * vm)
   ASSERT (node);
   rm->ip6_reass_expire_node_idx = node->index;
 
+  ip6_reass_set_params (IP6_REASS_TIMEOUT_DEFAULT_MS,
+                       IP6_REASS_MAX_REASSEMBLIES_DEFAULT,
+                       IP6_REASS_EXPIRE_WALK_INTERVAL_DEFAULT_MS);
+
   nbuckets = ip6_reass_get_nbuckets ();
   clib_bihash_init_48_8 (&rm->hash, "ip6-reass", nbuckets, nbuckets * 1024);
 
@@ -1221,10 +1227,6 @@ ip6_reass_init_function (vlib_main_t * vm)
   ip6_register_protocol (IP_PROTOCOL_IPV6_FRAGMENTATION,
                         ip6_reass_node.index);
 
-  ip6_reass_set_params (IP6_REASS_TIMEOUT_DEFAULT_MS,
-                       IP6_REASS_MAX_REASSEMBLIES_DEFAULT,
-                       IP6_REASS_EXPIRE_WALK_INTERVAL_DEFAULT_MS);
-
   return error;
 }
 
index 531caa4..76aabcb 100644 (file)
@@ -749,7 +749,6 @@ class TestIPv6Reassembly(VppTestCase):
         self.assert_equal(icmp[ICMPv6ParamProblem].code, 0, "ICMP code")
 
 
-@unittest.skip("removing GRE tunnels broken, need fix")
 class TestFIFReassembly(VppTestCase):
     """ Fragments in fragments reassembly """
 
@@ -965,7 +964,7 @@ class TestFIFReassembly(VppTestCase):
 
         # TODO remove gre vpp config by hand until VppIpRoute gets fixed
         # so that it's query_vpp_config() works as it should
-        self.gre6.remove_vpp_config()
+        self.gre6.remove_vpp_config()
 
 
 if __name__ == '__main__':