bpf_trace_filter: support bpf filter optimization and dump

BPF filter w/o optimization can take x2 - x3 more instructions,
causing significant slow down in fast path.

Enable pcap optimization by default via cli and introduce api v2
with pcap optimization control, keep v1 for a while as it exists
in previous release already.
Intriduce bpf filter cli dump, similar to tcpdump -d.

Also fix memleak, function name typo, cli pcap format hint and
add related tests.

Type: improvement
Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru>
Change-Id: I92b2b519e92326f1b8e1a4dda6a3e3edc52f87ad

6 files changed, 127 insertions, 15 deletions

diff --git a/src/plugins/bpf_trace_filter/api.c b/src/plugins/bpf_trace_filter/api.c
index 9e15c61ef07..30beaddd201 100644
--- a/src/plugins/bpf_trace_filter/api.c
+++ b/src/plugins/bpf_trace_filter/api.c
@@ -34,12 +34,11 @@ vl_api_bpf_trace_filter_set_t_handler (vl_api_bpf_trace_filter_set_t *mp)
   vl_api_bpf_trace_filter_set_reply_t *rmp;
   clib_error_t *err = 0;
   int rv = 0;
-  u8 is_del;
+  u8 is_del = !mp->is_add;
   char *bpf_expr;
-  is_del = !mp->is_add;
 
   bpf_expr = vl_api_from_api_to_new_c_string (&mp->filter);
-  err = bpf_trace_filter_set_unset (bpf_expr, is_del);
+  err = bpf_trace_filter_set_unset (bpf_expr, is_del, 0);
 
   if (err)
     {
@@ -51,6 +50,30 @@ vl_api_bpf_trace_filter_set_t_handler (vl_api_bpf_trace_filter_set_t *mp)
   REPLY_MACRO (VL_API_BPF_TRACE_FILTER_SET_REPLY);
 }
 
+static void
+vl_api_bpf_trace_filter_set_v2_t_handler (vl_api_bpf_trace_filter_set_v2_t *mp)
+{
+  bpf_trace_filter_main_t *bm = &bpf_trace_filter_main;
+  vl_api_bpf_trace_filter_set_v2_reply_t *rmp;
+  clib_error_t *err = 0;
+  int rv = 0;
+  u8 is_del = !mp->is_add;
+  u8 optimize = !!mp->optimize;
+  char *bpf_expr;
+
+  bpf_expr = vl_api_from_api_to_new_c_string (&mp->filter);
+  err = bpf_trace_filter_set_unset (bpf_expr, is_del, optimize);
+
+  if (err)
+    {
+      rv = -1;
+      clib_error_report (err);
+    }
+  vec_free (bpf_expr);
+
+  REPLY_MACRO (VL_API_BPF_TRACE_FILTER_SET_V2_REPLY);
+}
+
 #include <bpf_trace_filter/bpf_trace_filter.api.c>
 
 static clib_error_t *
diff --git a/src/plugins/bpf_trace_filter/bpf_trace_filter.api b/src/plugins/bpf_trace_filter/bpf_trace_filter.api
index 53f1b176a21..c2d47c8b3bf 100644
--- a/src/plugins/bpf_trace_filter/bpf_trace_filter.api
+++ b/src/plugins/bpf_trace_filter/bpf_trace_filter.api
@@ -23,4 +23,13 @@
   u32 context;
   bool is_add [default = true];
   string filter[];
+ };
+
+ autoreply define bpf_trace_filter_set_v2
+ {
+  u32 client_index;
+  u32 context;
+  bool is_add [default = true];
+  bool optimize [default = true];
+  string filter[];
  };
\ No newline at end of file
diff --git a/src/plugins/bpf_trace_filter/bpf_trace_filter.c b/src/plugins/bpf_trace_filter/bpf_trace_filter.c
index 01d5b1e96a7..9d86c8483a6 100644
--- a/src/plugins/bpf_trace_filter/bpf_trace_filter.c
+++ b/src/plugins/bpf_trace_filter/bpf_trace_filter.c
@@ -30,8 +30,24 @@ bpf_trace_filter_init (vlib_main_t *vm)
 int vnet_is_packet_traced (vlib_buffer_t *b, u32 classify_table_index,
 			   int func);
 
+u8 *
+format_bpf_trace_filter (u8 *s, va_list *a)
+{
+  bpf_trace_filter_main_t *btm = va_arg (*a, bpf_trace_filter_main_t *);
+  struct bpf_insn *insn;
+
+  if (!btm->prog_set)
+    return format (s, "bpf trace filter is not set");
+
+  insn = btm->prog.bf_insns;
+  for (int i = 0; i < btm->prog.bf_len; insn++, i++)
+    s = format (s, "%s\n", bpf_image (insn, i));
+
+  return s;
+}
+
 clib_error_t *
-bpf_trace_filter_set_unset (const char *bpf_expr, u8 is_del)
+bpf_trace_filter_set_unset (const char *bpf_expr, u8 is_del, u8 optimize)
 {
   bpf_trace_filter_main_t *btm = &bpf_trace_filter_main;
   if (is_del)
@@ -47,7 +63,7 @@ bpf_trace_filter_set_unset (const char *bpf_expr, u8 is_del)
       if (btm->prog_set)
 	pcap_freecode (&btm->prog);
       btm->prog_set = 0;
-      if (pcap_compile (btm->pcap, &btm->prog, (char *) bpf_expr, 0,
+      if (pcap_compile (btm->pcap, &btm->prog, (char *) bpf_expr, optimize,
 			PCAP_NETMASK_UNKNOWN))
 	{
 	  return clib_error_return (0, "Failed pcap_compile of %s", bpf_expr);
@@ -58,7 +74,7 @@ bpf_trace_filter_set_unset (const char *bpf_expr, u8 is_del)
 };
 
 int
-bpf_is_packed_traced (vlib_buffer_t *b, u32 classify_table_index, int func)
+bpf_is_packet_traced (vlib_buffer_t *b, u32 classify_table_index, int func)
 {
   bpf_trace_filter_main_t *bfm = &bpf_trace_filter_main;
   struct pcap_pkthdr phdr = { 0 };
@@ -82,7 +98,7 @@ VLIB_REGISTER_TRACE_FILTER_FUNCTION (bpf_trace_filter_fn, static) = {
   .name = "bpf_trace_filter",
   .description = "bpf based trace filter",
   .priority = 10,
-  .function = bpf_is_packed_traced
+  .function = bpf_is_packet_traced
 };
 
 VLIB_INIT_FUNCTION (bpf_trace_filter_init);
diff --git a/src/plugins/bpf_trace_filter/bpf_trace_filter.h b/src/plugins/bpf_trace_filter/bpf_trace_filter.h
index a031f98cbdb..52413ebe0ad 100644
--- a/src/plugins/bpf_trace_filter/bpf_trace_filter.h
+++ b/src/plugins/bpf_trace_filter/bpf_trace_filter.h
@@ -28,7 +28,9 @@ typedef struct
 } bpf_trace_filter_main_t;
 
 extern bpf_trace_filter_main_t bpf_trace_filter_main;
-clib_error_t *bpf_trace_filter_set_unset (const char *bpf_expr, u8 is_del);
+clib_error_t *bpf_trace_filter_set_unset (const char *bpf_expr, u8 is_del,
+					  u8 optimize);
+u8 *format_bpf_trace_filter (u8 *s, va_list *a);
 #endif /* _BPF_TRACE_FILTER_H_ */
 
 /*
diff --git a/src/plugins/bpf_trace_filter/cli.c b/src/plugins/bpf_trace_filter/cli.c
index 906ca71d5f3..f340b1667e1 100644
--- a/src/plugins/bpf_trace_filter/cli.c
+++ b/src/plugins/bpf_trace_filter/cli.c
@@ -21,7 +21,6 @@
 
 #include <vlib/vlib.h>
 #include <bpf_trace_filter/bpf_trace_filter.h>
-#include <pcap.h>
 
 static clib_error_t *
 set_bpf_trace_filter_command_fn (vlib_main_t *vm, unformat_input_t *input,
@@ -30,6 +29,7 @@ set_bpf_trace_filter_command_fn (vlib_main_t *vm, unformat_input_t *input,
   unformat_input_t _line_input, *line_input = &_line_input;
   u8 *bpf_expr = 0;
   u8 is_del = 0;
+  u8 optimize = 1;
   clib_error_t *err = 0;
 
   /* Get a line of input. */
@@ -40,25 +40,56 @@ set_bpf_trace_filter_command_fn (vlib_main_t *vm, unformat_input_t *input,
     {
       if (unformat (line_input, "del"))
 	is_del = 1;
+      else if (unformat (line_input, "no-optimize"))
+	optimize = 0;
       else if (unformat (line_input, "%s", &bpf_expr))
 	;
       else
-	return clib_error_return (0, "unknown input `%U'",
-				  format_unformat_error, input);
+	{
+	  err = clib_error_return (0, "unknown input `%U'",
+				   format_unformat_error, input);
+	  break;
+	}
     }
-
-  err = bpf_trace_filter_set_unset ((char *) bpf_expr, is_del);
   unformat_free (line_input);
 
+  if (err != 0)
+    return err;
+
+  err = bpf_trace_filter_set_unset ((char *) bpf_expr, is_del, optimize);
+
   return err;
 }
 
 VLIB_CLI_COMMAND (set_bpf_trace_filter, static) = {
   .path = "set bpf trace filter",
-  .short_help = "set bpf trace filter {<pcap string>}",
+  .short_help = "set bpf trace filter [del] [no-optimize] {<pcap string>}",
   .function = set_bpf_trace_filter_command_fn,
 };
 
+static clib_error_t *
+show_bpf_trace_filter_command_fn (vlib_main_t *vm, unformat_input_t *input,
+				  vlib_cli_command_t *cmd)
+{
+  bpf_trace_filter_main_t *btm = &bpf_trace_filter_main;
+
+  if (unformat_check_input (input) != UNFORMAT_END_OF_INPUT)
+    {
+      return (clib_error_return (0, "unknown input '%U'",
+				 format_unformat_error, input));
+    }
+
+  vlib_cli_output (vm, "%U", format_bpf_trace_filter, btm);
+
+  return 0;
+}
+
+VLIB_CLI_COMMAND (show_bpf_trace_filter, static) = {
+  .path = "show bpf trace filter",
+  .short_help = "show bpf trace filter",
+  .function = show_bpf_trace_filter_command_fn,
+};
+
 /*
  * fd.io coding-style-patch-verification: ON
  *
diff --git a/test/test_bpf_trace_filter.py b/test/test_bpf_trace_filter.py
index 9ee74db681e..6958caa6b37 100644
--- a/test/test_bpf_trace_filter.py
+++ b/test/test_bpf_trace_filter.py
@@ -48,7 +48,7 @@ class TestBpfTraceFilter(VppTestCase):
             p = (
                 Ether(dst=src_if.local_mac, src=src_if.remote_mac)
                 / IP(src=src_if.remote_ip4, dst=dst_if.remote_ip4)
-                / UDP(sport=randint(49152, 65535), dport=5678)
+                / UDP(sport=randint(49152, 65535), dport=5678 + i)
             )
             info.data = p.copy()
             packets.append(p)
@@ -77,6 +77,9 @@ class TestBpfTraceFilter(VppTestCase):
             "Unexpected packets in the trace buffer",
         )
 
+        reply = self.vapi.cli("show bpf trace filter")
+        self.assertIn("(000)", reply, "Unexpected bpf filter dump")
+
     def test_bpf_trace_filter_vapi(self):
         """BPF Trace filter test [VAPI]"""
         self.vapi.bpf_trace_filter_set(filter="tcp")
@@ -100,6 +103,34 @@ class TestBpfTraceFilter(VppTestCase):
             "Unexpected packets in the trace buffer",
         )
 
+    def test_bpf_trace_filter_vapi_v2(self):
+        """BPF Trace filter test [VAPI v2]"""
+        self.vapi.bpf_trace_filter_set_v2(filter="tcp or dst port 5678")
+        self.vapi.trace_set_filter_function(filter_function_name="bpf_trace_filter")
+
+        packets = self.create_stream(self.pg0, self.pg1, 3)
+        self.pg0.add_stream(packets)
+        self.pg_start(traceFilter=True)
+
+        # verify that bpf trace filter has been selected
+        reply = self.vapi.cli("show trace filter function")
+        self.assertIn(
+            "(*) name:bpf_trace_filter", reply, "BPF Trace filter is not selected"
+        )
+
+        # verify that trace is filtered
+        reply = self.vapi.cli("show trace")
+        self.assertIn(
+            "Packet 1\n",
+            reply,
+            "No expected packets in the trace buffer",
+        )
+        self.assertNotIn(
+            "Packet 2\n",
+            reply,
+            "Unexpected packets in the trace buffer",
+        )
+
 
 if __name__ == "__main__":
     unittest.main(testRunner=VppTestRunner)