Leak locks and tables in the Classifier

Change-Id: Iae04c57bba87ab3665388eadd0805f75171636a5
Signed-off-by: Neale Ranns <nranns@cisco.com>

diff --git a/src/vnet/classify/vnet_classify.c b/src/vnet/classify/vnet_classify.c
index 418e3da..99b10dc 100644 (file)
--- a/src/vnet/classify/vnet_classify.c
+++ b/src/vnet/classify/vnet_classify.c
@@ -362,6 +362,34 @@ split_and_rehash_linear (vnet_classify_table_t * t,
   return new_values;
 }
 
+static void
+vnet_classify_entry_claim_resource (vnet_classify_entry_t *e)
+{
+    switch (e->action)
+    {
+    case CLASSIFY_ACTION_SET_IP4_FIB_INDEX:
+        fib_table_lock (e->metadata, FIB_PROTOCOL_IP4);
+        break;
+    case CLASSIFY_ACTION_SET_IP6_FIB_INDEX:
+        fib_table_lock (e->metadata, FIB_PROTOCOL_IP6);
+        break;
+    }
+}
+
+static void
+vnet_classify_entry_release_resource (vnet_classify_entry_t *e)
+{
+    switch (e->action)
+    {
+    case CLASSIFY_ACTION_SET_IP4_FIB_INDEX:
+        fib_table_unlock (e->metadata, FIB_PROTOCOL_IP4);
+        break;
+    case CLASSIFY_ACTION_SET_IP6_FIB_INDEX:
+        fib_table_unlock (e->metadata, FIB_PROTOCOL_IP6);
+        break;
+    }
+}
+
 int vnet_classify_add_del (vnet_classify_table_t * t, 
                            vnet_classify_entry_t * add_v,
                            int is_add)
@@ -408,6 +436,7 @@ int vnet_classify_add_del (vnet_classify_table_t * t,
       clib_memcpy (v, add_v, sizeof (vnet_classify_entry_t) +
               t->match_n_vectors * sizeof (u32x4));
       v->flags &= ~(VNET_CLASSIFY_ENTRY_FREE);
+      vnet_classify_entry_claim_resource (v);
 
       tmp_b.as_u64 = 0;
       tmp_b.offset = vnet_classify_get_offset (t, v);
@@ -445,6 +474,7 @@ int vnet_classify_add_del (vnet_classify_table_t * t,
               clib_memcpy (v, add_v, sizeof (vnet_classify_entry_t) +
                       t->match_n_vectors * sizeof(u32x4));
               v->flags &= ~(VNET_CLASSIFY_ENTRY_FREE);
+              vnet_classify_entry_claim_resource (v);
 
               CLIB_MEMORY_BARRIER();
               /* Restore the previous (k,v) pairs */
@@ -461,6 +491,8 @@ int vnet_classify_add_del (vnet_classify_table_t * t,
               clib_memcpy (v, add_v, sizeof (vnet_classify_entry_t) +
                       t->match_n_vectors * sizeof(u32x4));
               v->flags &= ~(VNET_CLASSIFY_ENTRY_FREE);
+              vnet_classify_entry_claim_resource (v);
+
               CLIB_MEMORY_BARRIER();
               b->as_u64 = t->saved_bucket.as_u64;
               t->active_elements ++;
@@ -477,9 +509,11 @@ int vnet_classify_add_del (vnet_classify_table_t * t,
 
           if (!memcmp (v->key, add_v->key, t->match_n_vectors * sizeof (u32x4)))
             {
+              vnet_classify_entry_release_resource (v);
               memset (v, 0xff, sizeof (vnet_classify_entry_t) +
                       t->match_n_vectors * sizeof(u32x4));
               v->flags |= VNET_CLASSIFY_ENTRY_FREE;
+
               CLIB_MEMORY_BARRIER();
               b->as_u64 = t->saved_bucket.as_u64;
               t->active_elements --;
@@ -552,6 +586,8 @@ int vnet_classify_add_del (vnet_classify_table_t * t,
           clib_memcpy (new_v, add_v, sizeof (vnet_classify_entry_t) +
                   t->match_n_vectors * sizeof(u32x4));
           new_v->flags &= ~(VNET_CLASSIFY_ENTRY_FREE);
+          vnet_classify_entry_claim_resource (new_v);
+
           goto expand_ok;
         }
     }
@@ -2075,6 +2111,9 @@ int vnet_classify_add_del_session (vnet_classify_main_t * cm,
     e->key[i] &= t->mask[i];
 
   rv = vnet_classify_add_del (t, e, is_add);
+
+  vnet_classify_entry_release_resource(e);
+
   if (rv)
     return VNET_API_ERROR_NO_SUCH_ENTRY;
   return 0;

diff --git a/src/vnet/classify/vnet_classify.h b/src/vnet/classify/vnet_classify.h
index ffe3dff..1eb5b14 100644 (file)
--- a/src/vnet/classify/vnet_classify.h
+++ b/src/vnet/classify/vnet_classify.h
@@ -62,8 +62,11 @@ extern vlib_node_registration_t ip6_classify_node;
  *   - Classified IP packets will be looked up
  *     from the specified ipv6 fib table
  */
-#define CLASSIFY_ACTION_SET_IP4_FIB_INDEX       1
-#define CLASSIFY_ACTION_SET_IP6_FIB_INDEX       2
+typedef enum vnet_classify_action_t_
+{
+  CLASSIFY_ACTION_SET_IP4_FIB_INDEX = 1,
+  CLASSIFY_ACTION_SET_IP6_FIB_INDEX = 2,
+} __attribute__ ((packed)) vnet_classify_action_t;
 
 struct _vnet_classify_main;
 typedef struct _vnet_classify_main vnet_classify_main_t;
@@ -93,7 +96,7 @@ typedef CLIB_PACKED(struct _vnet_classify_entry {
   u8 flags;
 #define VNET_CLASSIFY_ENTRY_FREE       (1<<0)
 
-  u8 action;
+  vnet_classify_action_t action;
   u16 metadata;
 
   /* Hit counter, last heard time */

diff --git a/src/vnet/fib/ip4_fib.c b/src/vnet/fib/ip4_fib.c
index 878b4db..d563baf 100644 (file)
--- a/src/vnet/fib/ip4_fib.c
+++ b/src/vnet/fib/ip4_fib.c
@@ -531,10 +531,11 @@ ip4_show_fib (vlib_main_t * vm,
        if (fib_index != ~0 && fib_index != (int)fib->index)
            continue;
 
-       vlib_cli_output (vm, "%U, fib_index %d, flow hash: %U", 
+       vlib_cli_output (vm, "%U, fib_index:%d, flow hash:[%U] locks:%d", 
                         format_fib_table_name, fib->index, FIB_PROTOCOL_IP4,
                         fib->index,
-                        format_ip_flow_hash_config, fib_table->ft_flow_hash_config);
+                        format_ip_flow_hash_config, fib_table->ft_flow_hash_config,
+                         fib_table->ft_locks);
 
        /* Show summary? */
        if (! verbose)

diff --git a/src/vnet/fib/ip6_fib.c b/src/vnet/fib/ip6_fib.c
index e046b34..4a24c21 100644 (file)
--- a/src/vnet/fib/ip6_fib.c
+++ b/src/vnet/fib/ip6_fib.c
@@ -633,9 +633,10 @@ ip6_show_fib (vlib_main_t * vm,
        if (fib_index != ~0 && fib_index != (int)fib->index)
            continue;
 
-       vlib_cli_output (vm, "%s, fib_index %d, flow hash: %U", 
+       vlib_cli_output (vm, "%s, fib_index:%d, flow hash:[%U] locks:%d", 
                         fib_table->ft_desc, fib->index,
-                        format_ip_flow_hash_config, fib_table->ft_flow_hash_config);
+                        format_ip_flow_hash_config, fib_table->ft_flow_hash_config,
+                         fib_table->ft_locks);
 
        /* Show summary? */
        if (! verbose)

diff --git a/test/test_classifier.py b/test/test_classifier.py
index faa107d..a43f7a3 100644 (file)
--- a/test/test_classifier.py
+++ b/test/test_classifier.py
@@ -64,7 +64,7 @@ class TestClassifier(VppTestCase):
             self.logger.info(self.vapi.cli("show classify table verbose"))
             self.logger.info(self.vapi.cli("show ip fib"))
 
-    def config_pbr_fib_entry(self, intf):
+    def config_pbr_fib_entry(self, intf, is_add=1):
         """Configure fib entry to route traffic toward PBR VRF table
 
         :param VppInterface intf: destination interface to be routed for PBR.
@@ -74,7 +74,8 @@ class TestClassifier(VppTestCase):
         self.vapi.ip_add_del_route(intf.local_ip4n,
                                    addr_len,
                                    intf.remote_ip4n,
-                                   table_id=self.pbr_vrfid)
+                                   table_id=self.pbr_vrfid,
+                                   is_add=is_add)
 
     def create_stream(self, src_if, dst_if, packet_sizes):
         """Create input packet stream for defined interfaces.
@@ -139,6 +140,25 @@ class TestClassifier(VppTestCase):
                             "Interface %s: Packet expected from interface %s "
                             "didn't arrive" % (dst_if.name, i.name))
 
+    def verify_vrf(self, vrf_id):
+        """
+        Check if the FIB table / VRF ID is configured.
+
+        :param int vrf_id: The FIB table / VRF ID to be verified.
+        :return: 1 if the FIB table / VRF ID is configured, otherwise return 0.
+        """
+        ip_fib_dump = self.vapi.ip_fib_dump()
+        vrf_count = 0
+        for ip_fib_details in ip_fib_dump:
+            if ip_fib_details[2] == vrf_id:
+                vrf_count += 1
+        if vrf_count == 0:
+            self.logger.info("IPv4 VRF ID %d is not configured" % vrf_id)
+            return 0
+        else:
+            self.logger.info("IPv4 VRF ID %d is configured" % vrf_id)
+            return 1
+
     @staticmethod
     def build_ip_mask(proto='', src_ip='', dst_ip='',
                       src_port='', dst_port=''):
@@ -332,10 +352,12 @@ class TestClassifier(VppTestCase):
             'pbr', self.build_ip_mask(
                 src_ip='ffffffff'))
         pbr_option = 1
+        # this will create the VRF/table in which we will insert the route
         self.create_classify_session(
             self.pg0, self.acl_tbl_idx.get('pbr'),
             self.build_ip_match(src_ip=self.pg0.remote_ip4),
             pbr_option, self.pbr_vrfid)
+        self.assertTrue(self.verify_vrf(self.pbr_vrfid))
         self.config_pbr_fib_entry(self.pg3)
         self.input_acl_set_interface(self.pg0, self.acl_tbl_idx.get('pbr'))
 
@@ -349,6 +371,15 @@ class TestClassifier(VppTestCase):
         self.pg1.assert_nothing_captured(remark="packets forwarded")
         self.pg2.assert_nothing_captured(remark="packets forwarded")
 
+        # remove the classify session and the route
+        self.config_pbr_fib_entry(self.pg3, is_add=0)
+        self.create_classify_session(
+            self.pg0, self.acl_tbl_idx.get('pbr'),
+            self.build_ip_match(src_ip=self.pg0.remote_ip4),
+            pbr_option, self.pbr_vrfid, is_add=0)
+
+        # and the table should be gone.
+        self.assertFalse(self.verify_vrf(self.pbr_vrfid))
 
 if __name__ == '__main__':
     unittest.main(testRunner=VppTestRunner)