GBP: drop and contract counters

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

7 files changed, 162 insertions, 25 deletions

diff --git a/extras/vom/vom/gbp_contract.hpp b/extras/vom/vom/gbp_contract.hpp
index a8e15fb7976..2e8f9d0bc26 100644
--- a/extras/vom/vom/gbp_contract.hpp
+++ b/extras/vom/vom/gbp_contract.hpp
@@ -164,7 +164,7 @@ private:
   /**
    * HW configuration for the result of creating the endpoint
    */
-  HW::item<bool> m_hw;
+  HW::item<uint32_t> m_hw;
 
   /**
    * The source EPG ID
diff --git a/extras/vom/vom/gbp_contract_cmds.cpp b/extras/vom/vom/gbp_contract_cmds.cpp
index b5705347791..6aed9998e1e 100644
--- a/extras/vom/vom/gbp_contract_cmds.cpp
+++ b/extras/vom/vom/gbp_contract_cmds.cpp
@@ -19,7 +19,7 @@
 namespace VOM {
 namespace gbp_contract_cmds {
 
-create_cmd::create_cmd(HW::item<bool>& item,
+create_cmd::create_cmd(HW::item<uint32_t>& item,
                        sclass_t sclass,
                        sclass_t dclass,
                        const handle_t& acl,
@@ -116,7 +116,9 @@ create_cmd::to_string() const
   return (s.str());
 }
 
-delete_cmd::delete_cmd(HW::item<bool>& item, sclass_t sclass, sclass_t dclass)
+delete_cmd::delete_cmd(HW::item<uint32_t>& item,
+                       sclass_t sclass,
+                       sclass_t dclass)
   : rpc_cmd(item)
   , m_sclass(sclass)
   , m_dclass(dclass)
diff --git a/extras/vom/vom/gbp_contract_cmds.hpp b/extras/vom/vom/gbp_contract_cmds.hpp
index 7108c53f77b..3b3fab97a70 100644
--- a/extras/vom/vom/gbp_contract_cmds.hpp
+++ b/extras/vom/vom/gbp_contract_cmds.hpp
@@ -27,13 +27,14 @@ namespace gbp_contract_cmds {
 /**
  * A command class that creates or updates the GBP contract
  */
-class create_cmd : public rpc_cmd<HW::item<bool>, vapi::Gbp_contract_add_del>
+class create_cmd
+  : public rpc_cmd<HW::item<uint32_t>, vapi::Gbp_contract_add_del>
 {
 public:
   /**
    * Constructor
    */
-  create_cmd(HW::item<bool>& item,
+  create_cmd(HW::item<uint32_t>& item,
              sclass_t sclass,
              sclass_t dclass,
              const handle_t& acl,
@@ -66,13 +67,14 @@ private:
 /**
  * A cmd class that deletes a GBP contract
  */
-class delete_cmd : public rpc_cmd<HW::item<bool>, vapi::Gbp_contract_add_del>
+class delete_cmd
+  : public rpc_cmd<HW::item<uint32_t>, vapi::Gbp_contract_add_del>
 {
 public:
   /**
    * Constructor
    */
-  delete_cmd(HW::item<bool>& item, sclass_t sclass, sclass_t dclass);
+  delete_cmd(HW::item<uint32_t>& item, sclass_t sclass, sclass_t dclass);
 
   /**
    * Issue the command to VPP/HW
diff --git a/src/plugins/gbp/gbp_policy_dpo.c b/src/plugins/gbp/gbp_policy_dpo.c
index c3a51a46236..a6194df6836 100644
--- a/src/plugins/gbp/gbp_policy_dpo.c
+++ b/src/plugins/gbp/gbp_policy_dpo.c
@@ -217,12 +217,26 @@ gbp_policy_dpo_module_init (vlib_main_t * vm)
 VLIB_INIT_FUNCTION (gbp_policy_dpo_module_init);
 #endif /* CLIB_MARCH_VARIANT */
 
+typedef enum
+{
+#define _(sym,str) GBP_POLICY_DPO_ERROR_##sym,
+  foreach_gbp_policy_error
+#undef _
+    GBP_POLICY_N_ERROR,
+} gbp_policy_dpo_error_t;
+
+static char *gbp_policy_dpo_error_strings[] = {
+#define _(sym,string) string,
+  foreach_gbp_policy_error
+#undef _
+};
+
 typedef struct gbp_policy_dpo_trace_t_
 {
   u32 sclass;
   u32 dclass;
   u32 acl_index;
-  u32 a_bit;
+  u32 flags;
   u32 action;
 } gbp_policy_dpo_trace_t;
 
@@ -255,11 +269,14 @@ gbp_policy_dpo_inline (vlib_main_t * vm,
 		       vlib_frame_t * from_frame, u8 is_ip6)
 {
   gbp_main_t *gm = &gbp_main;
-  u32 n_left_from, next_index, *from, *to_next;
+  u32 n_left_from, next_index, *from, *to_next, thread_index;
+  u32 n_allow_intra, n_allow_a_bit;
   gbp_rule_t *gu;
 
   from = vlib_frame_vector_args (from_frame);
   n_left_from = from_frame->n_vectors;
+  n_allow_intra = n_allow_a_bit = 0;
+  thread_index = vm->thread_index;
 
   next_index = node->cached_next_index;
 
@@ -307,6 +324,7 @@ gbp_policy_dpo_inline (vlib_main_t * vm,
 	    {
 	      next0 = gpd0->gpd_dpo.dpoi_next_node;
 	      key0.as_u32 = ~0;
+	      n_allow_a_bit++;
 	      goto trace;
 	    }
 
@@ -322,6 +340,7 @@ gbp_policy_dpo_inline (vlib_main_t * vm,
 		   */
 		  next0 = gpd0->gpd_dpo.dpoi_next_node;
 		  vnet_buffer2 (b0)->gbp.flags |= VXLAN_GBP_GPFLAGS_A;
+		  n_allow_intra++;
 		  action0 = 0;
 		}
 	      else
@@ -365,13 +384,35 @@ gbp_policy_dpo_inline (vlib_main_t * vm,
 			      next0 = gpd0->gpd_dpo.dpoi_next_node;
 			      break;
 			    case GBP_RULE_DENY:
-			      next0 = 0;
+			      next0 = GBP_POLICY_DROP;
 			      break;
 			    case GBP_RULE_REDIRECT:
 			      next0 = gbp_rule_l3_redirect (gu, b0, is_ip6);
 			      break;
 			    }
 			}
+		      if (next0 == GBP_POLICY_DROP)
+			{
+			  vlib_increment_combined_counter
+			    (&gbp_contract_drop_counters,
+			     thread_index,
+			     gci0, 1, vlib_buffer_length_in_chain (vm, b0));
+			  b0->error =
+			    node->errors[GBP_POLICY_DPO_ERROR_DROP_CONTRACT];
+			}
+		      else
+			{
+			  vlib_increment_combined_counter
+			    (&gbp_contract_permit_counters,
+			     thread_index,
+			     gci0, 1, vlib_buffer_length_in_chain (vm, b0));
+			}
+
+		    }
+		  else
+		    {
+		      b0->error =
+			node->errors[GBP_POLICY_DPO_ERROR_DROP_NO_CONTRACT];
 		    }
 		}
 	    }
@@ -392,7 +433,7 @@ gbp_policy_dpo_inline (vlib_main_t * vm,
 	      tr->sclass = key0.gck_src;
 	      tr->dclass = key0.gck_dst;
 	      tr->acl_index = (gc0 ? gc0->gc_acl_index : ~0);
-	      tr->a_bit = vnet_buffer2 (b0)->gbp.flags & VXLAN_GBP_GPFLAGS_A;
+	      tr->flags = vnet_buffer2 (b0)->gbp.flags;
 	      tr->action = action0;
 	    }
 
@@ -401,6 +442,14 @@ gbp_policy_dpo_inline (vlib_main_t * vm,
 	}
       vlib_put_next_frame (vm, node, next_index, n_left_to_next);
     }
+
+  vlib_node_increment_counter (vm, node->node_index,
+			       GBP_POLICY_DPO_ERROR_ALLOW_INTRA,
+			       n_allow_intra);
+  vlib_node_increment_counter (vm, node->node_index,
+			       GBP_POLICY_DPO_ERROR_ALLOW_A_BIT,
+			       n_allow_a_bit);
+
   return from_frame->n_vectors;
 }
 
@@ -411,8 +460,9 @@ format_gbp_policy_dpo_trace (u8 * s, va_list * args)
   CLIB_UNUSED (vlib_node_t * node) = va_arg (*args, vlib_node_t *);
   gbp_policy_dpo_trace_t *t = va_arg (*args, gbp_policy_dpo_trace_t *);
 
-  s = format (s, " sclass:%d dclass:%d acl-index:%d a-bit:%d action:%d",
-	      t->sclass, t->dclass, t->acl_index, t->a_bit, t->action);
+  s = format (s, " sclass:%d dclass:%d acl-index:%d flags:%U action:%d",
+	      t->sclass, t->dclass, t->acl_index,
+	      format_vxlan_gbp_header_gpflags, t->flags, t->action);
 
   return s;
 }
@@ -436,6 +486,10 @@ VLIB_REGISTER_NODE (ip4_gbp_policy_dpo_node) = {
     .name = "ip4-gbp-policy-dpo",
     .vector_size = sizeof (u32),
     .format_trace = format_gbp_policy_dpo_trace,
+
+    .n_errors = ARRAY_LEN(gbp_policy_dpo_error_strings),
+    .error_strings = gbp_policy_dpo_error_strings,
+
     .n_next_nodes = GBP_POLICY_N_NEXT,
     .next_nodes =
     {
@@ -446,6 +500,10 @@ VLIB_REGISTER_NODE (ip6_gbp_policy_dpo_node) = {
     .name = "ip6-gbp-policy-dpo",
     .vector_size = sizeof (u32),
     .format_trace = format_gbp_policy_dpo_trace,
+
+    .n_errors = ARRAY_LEN(gbp_policy_dpo_error_strings),
+    .error_strings = gbp_policy_dpo_error_strings,
+
     .n_next_nodes = GBP_POLICY_N_NEXT,
     .next_nodes =
     {
diff --git a/src/plugins/gbp/gbp_policy_node.c b/src/plugins/gbp/gbp_policy_node.c
index 1f2ac4310e0..8fe1d7f6c0f 100644
--- a/src/plugins/gbp/gbp_policy_node.c
+++ b/src/plugins/gbp/gbp_policy_node.c
@@ -25,15 +25,15 @@
 
 typedef enum
 {
-#define _(sym,str) GBP_ERROR_##sym,
-  foreach_gbp_policy
+#define _(sym,str) GBP_POLICY_ERROR_##sym,
+  foreach_gbp_policy_error
 #undef _
     GBP_POLICY_N_ERROR,
 } gbp_policy_error_t;
 
 static char *gbp_policy_error_strings[] = {
 #define _(sym,string) string,
-  foreach_gbp_policy
+  foreach_gbp_policy_error
 #undef _
 };
 
@@ -115,11 +115,14 @@ gbp_policy_inline (vlib_main_t * vm,
   gbp_main_t *gm = &gbp_main;
   gbp_policy_main_t *gpm = &gbp_policy_main;
   u32 n_left_from, *from, *to_next;
-  u32 next_index;
+  u32 next_index, thread_index;
+  u32 n_allow_intra, n_allow_a_bit;
 
   next_index = 0;
   n_left_from = frame->n_vectors;
   from = vlib_frame_vector_args (frame);
+  thread_index = vm->thread_index;
+  n_allow_intra = n_allow_a_bit = 0;
 
   while (n_left_from > 0)
     {
@@ -172,6 +175,7 @@ gbp_policy_inline (vlib_main_t * vm,
 					    (is_port_based ?
 					     L2OUTPUT_FEAT_GBP_POLICY_PORT :
 					     L2OUTPUT_FEAT_GBP_POLICY_MAC));
+	      n_allow_a_bit++;
 	      key0.as_u32 = ~0;
 	      goto trace;
 	    }
@@ -188,9 +192,11 @@ gbp_policy_inline (vlib_main_t * vm,
 	  if (NULL != ge0)
 	    key0.gck_dst = ge0->ge_fwd.gef_sclass;
 	  else
-	    /* If you cannot determine the destination EP then drop */
-	    goto trace;
-
+	    {
+	      /* If you cannot determine the destination EP then drop */
+	      b0->error = node->errors[GBP_POLICY_ERROR_DROP_NO_DCLASS];
+	      goto trace;
+	    }
 	  key0.gck_src = vnet_buffer2 (b0)->gbp.sclass;
 
 	  if (SCLASS_INVALID != key0.gck_src)
@@ -208,6 +214,7 @@ gbp_policy_inline (vlib_main_t * vm,
 					   L2OUTPUT_FEAT_GBP_POLICY_PORT :
 					   L2OUTPUT_FEAT_GBP_POLICY_MAC));
 		  vnet_buffer2 (b0)->gbp.flags |= VXLAN_GBP_GPFLAGS_A;
+		  n_allow_intra++;
 		}
 	      else
 		{
@@ -223,6 +230,11 @@ gbp_policy_inline (vlib_main_t * vm,
 		      u16 ether_type0;
 		      const u8 *h0;
 
+		      vlib_prefetch_combined_counter
+			(&gbp_contract_drop_counters, thread_index, gci0);
+		      vlib_prefetch_combined_counter
+			(&gbp_contract_permit_counters, thread_index, gci0);
+
 		      action0 = 0;
 		      gc0 = gbp_contract_get (gci0);
 		      l2_len0 = vnet_buffer (b0)->l2.l2_len;
@@ -235,6 +247,14 @@ gbp_policy_inline (vlib_main_t * vm,
 			  /*
 			   * black list model so drop
 			   */
+			  b0->error =
+			    node->errors[GBP_POLICY_ERROR_DROP_ETHER_TYPE];
+
+			  vlib_increment_combined_counter
+			    (&gbp_contract_drop_counters,
+			     thread_index,
+			     gci0, 1, vlib_buffer_length_in_chain (vm, b0));
+
 			  goto trace;
 			}
 
@@ -286,7 +306,7 @@ gbp_policy_inline (vlib_main_t * vm,
 				      L2OUTPUT_FEAT_GBP_POLICY_MAC));
 				  break;
 				case GBP_RULE_DENY:
-				  next0 = 0;
+				  next0 = GBP_POLICY_NEXT_DROP;
 				  break;
 				case GBP_RULE_REDIRECT:
 				  next0 = gbp_rule_l2_redirect (gu, b0);
@@ -294,6 +314,27 @@ gbp_policy_inline (vlib_main_t * vm,
 				}
 			    }
 			}
+		      if (next0 == GBP_POLICY_NEXT_DROP)
+			{
+			  vlib_increment_combined_counter
+			    (&gbp_contract_drop_counters,
+			     thread_index,
+			     gci0, 1, vlib_buffer_length_in_chain (vm, b0));
+			  b0->error =
+			    node->errors[GBP_POLICY_ERROR_DROP_CONTRACT];
+			}
+		      else
+			{
+			  vlib_increment_combined_counter
+			    (&gbp_contract_permit_counters,
+			     thread_index,
+			     gci0, 1, vlib_buffer_length_in_chain (vm, b0));
+			}
+		    }
+		  else
+		    {
+		      b0->error =
+			node->errors[GBP_POLICY_ERROR_DROP_NO_CONTRACT];
 		    }
 		}
 	    }
@@ -332,6 +373,11 @@ gbp_policy_inline (vlib_main_t * vm,
       vlib_put_next_frame (vm, node, next_index, n_left_to_next);
     }
 
+  vlib_node_increment_counter (vm, node->node_index,
+			       GBP_POLICY_ERROR_ALLOW_INTRA, n_allow_intra);
+  vlib_node_increment_counter (vm, node->node_index,
+			       GBP_POLICY_ERROR_ALLOW_A_BIT, n_allow_a_bit);
+
   return frame->n_vectors;
 }
 
@@ -376,7 +422,6 @@ VLIB_REGISTER_NODE (gbp_policy_port_node) = {
   .error_strings = gbp_policy_error_strings,
 
   .n_next_nodes = GBP_POLICY_N_NEXT,
-
   .next_nodes = {
     [GBP_POLICY_NEXT_DROP] = "error-drop",
   },
@@ -387,7 +432,14 @@ VLIB_REGISTER_NODE (gbp_policy_mac_node) = {
   .vector_size = sizeof (u32),
   .format_trace = format_gbp_policy_trace,
   .type = VLIB_NODE_TYPE_INTERNAL,
-  .sibling_of = "gbp-policy-port",
+
+  .n_errors = ARRAY_LEN(gbp_policy_error_strings),
+  .error_strings = gbp_policy_error_strings,
+
+  .n_next_nodes = GBP_POLICY_N_NEXT,
+  .next_nodes = {
+    [GBP_POLICY_NEXT_DROP] = "error-drop",
+  },
 };
 
 /* *INDENT-ON* */
diff --git a/src/vpp-api/client/stat_client.c b/src/vpp-api/client/stat_client.c
index 761c73f0fb7..3959c92167f 100644
--- a/src/vpp-api/client/stat_client.c
+++ b/src/vpp-api/client/stat_client.c
@@ -198,7 +198,7 @@ stat_segment_heartbeat (void)
   return stat_segment_heartbeat_r (sm);
 }
 
-stat_segment_data_t
+static stat_segment_data_t
 copy_data (stat_segment_directory_entry_t * ep, stat_client_main_t * sm)
 {
   stat_segment_data_t result = { 0 };
diff --git a/test/test_gbp.py b/test/test_gbp.py
index ee842b33052..df529c6c880 100644
--- a/test/test_gbp.py
+++ b/test/test_gbp.py
@@ -447,13 +447,14 @@ class VppGbpContract(VppObject):
         rules = []
         for r in self.rules:
             rules.append(r.encode())
-        self._test.vapi.gbp_contract_add_del(
+        r = self._test.vapi.gbp_contract_add_del(
             1,
             self.sclass,
             self.dclass,
             self.acl_index,
             rules,
             self.allowed_ethertypes)
+        self.stats_index = r.stats_index
         self._test.registry.register(self, self._test.logger)
 
     def remove_vpp_config(self):
@@ -478,6 +479,14 @@ class VppGbpContract(VppObject):
                 return True
         return False
 
+    def get_drop_stats(self):
+        c = self._test.statistics.get_counter("/net/gbp/contract/drop")
+        return c[0][self.stats_index]
+
+    def get_permit_stats(self):
+        c = self._test.statistics.get_counter("/net/gbp/contract/permit")
+        return c[0][self.stats_index]
+
 
 class VppGbpVxlanTunnel(VppInterface):
     """
@@ -1154,6 +1163,11 @@ class TestGBP(VppTestCase):
                                      pkt_inter_epg_221_to_220 * 65,
                                      eps[0].itf)
 
+        ds = c2.get_drop_stats()
+        self.assertEqual(ds['packets'], 0)
+        ps = c2.get_permit_stats()
+        self.assertEqual(ps['packets'], 65)
+
         #
         # the contract does not allow non-IP
         #
@@ -1425,6 +1439,8 @@ class TestGBP(VppTestCase):
     def test_gbp_learn_l2(self):
         """ GBP L2 Endpoint Learning """
 
+        self.vapi.cli("clear errors")
+
         ep_flags = VppEnum.vl_api_gbp_endpoint_flags_t
         learnt = [{'mac': '00:00:11:11:11:01',
                    'ip': '10.0.0.1',
@@ -1533,6 +1549,10 @@ class TestGBP(VppTestCase):
 
         self.send_and_assert_no_replies(self.pg2, p)
 
+        self.logger.info(self.vapi.cli("sh error"))
+        # self.assert_packet_counter_equal(
+        #    '/err/gbp-policy-port/drop-no-contract', 1)
+
         #
         # we should not have learnt a new tunnel endpoint, since
         # the EPG was not learnt.
@@ -1584,6 +1604,9 @@ class TestGBP(VppTestCase):
                                               vx_tun_l2_1.sw_if_index,
                                               ip=l['ip']))
 
+        # self.assert_packet_counter_equal(
+        #    '/err/gbp-policy-port/allow-intra-sclass', 2)
+
         self.logger.info(self.vapi.cli("show gbp endpoint"))
         self.logger.info(self.vapi.cli("show gbp vxlan"))
         self.logger.info(self.vapi.cli("show ip mfib"))