snort: support multiple instances per interface

Implements load balancing between snort instances via flow hash.
New CLI commands have been made to support these changes:

snort attach instance <name1>
    [instance <name2> ... ] interface <ifname> [input|output|inout]
snort attach all-instances interface <ifname> [input|output|inout]
snort detach instance <name1> interface <ifname>
snort detach all-instances interface <ifname>

The output of "show snort interfaces" has an extra column to show the
direction of each attachment:

interface   instances   direction
Ethernet0:  snort1      inout
            snort2      inout
            snort3      inout
Ethernet1:  snort1      input
            snort3      output

To maintain backwards compatibility for the snort api, the
snort_interface_get api endpoint only returns one of the attached
instances and the snort_interface_detach endpoint detaches all
attached instances.

Type: improvement
Signed-off-by: Agathiyan Bragadeesh <agathiyan.bragadeesh2@arm.com>
Change-Id: I6b7c26c203496d6a1dba244620907f28c04bb478

6 files changed, 554 insertions, 168 deletions

diff --git a/src/plugins/snort/cli.c b/src/plugins/snort/cli.c
index 4b6dbc742a7..d4b69adae7d 100644
--- a/src/plugins/snort/cli.c
+++ b/src/plugins/snort/cli.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: Apache-2.0
  * Copyright(c) 2021 Cisco Systems, Inc.
+ * Copyright(c) 2024 Arm Limited
  */
 
 #include <vlib/vlib.h>
@@ -17,6 +18,70 @@ format_snort_instance (u8 *s, va_list *args)
 }
 
 static clib_error_t *
+snort_attach_detach_instance (vlib_main_t *vm, vnet_main_t *vnm,
+			      char *instance_name, u32 sw_if_index,
+			      int is_enable, snort_attach_dir_t dir)
+{
+  clib_error_t *err = NULL;
+  int rv = snort_interface_enable_disable (vm, instance_name, sw_if_index,
+					   is_enable, dir);
+  switch (rv)
+    {
+    case 0:
+      break;
+    case VNET_API_ERROR_FEATURE_ALREADY_ENABLED:
+      /* already attached to same instance */
+      break;
+    case VNET_API_ERROR_INVALID_INTERFACE:
+      err = clib_error_return (0,
+			       "interface %U is not assigned to snort "
+			       "instance %s!",
+			       format_vnet_sw_if_index_name, vnm, sw_if_index,
+			       instance_name);
+      break;
+    case VNET_API_ERROR_NO_SUCH_ENTRY:
+      err = clib_error_return (0, "unknown instance '%s'", instance_name);
+      break;
+    case VNET_API_ERROR_INSTANCE_IN_USE:
+      err = clib_error_return (
+	0, "interface %U is currently up, set state down first",
+	format_vnet_sw_if_index_name, vnm, sw_if_index);
+      break;
+    default:
+      err = clib_error_return (0, "snort_interface_enable_disable returned %d",
+			       rv);
+      break;
+    }
+  return err;
+}
+
+static clib_error_t *
+snort_detach_all_instance (vlib_main_t *vm, vnet_main_t *vnm, u32 sw_if_index)
+{
+  clib_error_t *err = NULL;
+  int rv = snort_interface_disable_all (vm, sw_if_index);
+  switch (rv)
+    {
+    case 0:
+      break;
+    case VNET_API_ERROR_INSTANCE_IN_USE:
+      err = clib_error_return (
+	0, "interface %U is currently up, set state down first",
+	format_vnet_sw_if_index_name, vnm, sw_if_index);
+      break;
+    case VNET_API_ERROR_INVALID_INTERFACE:
+      err = clib_error_return (0, "interface %U has no attached instances",
+			       format_vnet_sw_if_index_name, vnm, sw_if_index);
+      break;
+    default:
+      err =
+	clib_error_return (0, "snort_interface_disable_all returned %d", rv);
+      break;
+    }
+  return err;
+}
+
+static clib_error_t *
 snort_create_instance_command_fn (vlib_main_t *vm, unformat_input_t *input,
 				  vlib_cli_command_t *cmd)
 {
@@ -94,7 +159,7 @@ done:
 
 VLIB_CLI_COMMAND (snort_create_instance_command, static) = {
   .path = "snort create-instance",
-  .short_help = "snort create-instaince name <name> [queue-size <size>] "
+  .short_help = "snort create-instance name <name> [queue-size <size>] "
 		"[on-disconnect drop|pass]",
   .function = snort_create_instance_command_fn,
 };
@@ -217,11 +282,15 @@ snort_attach_command_fn (vlib_main_t *vm, unformat_input_t *input,
 {
   unformat_input_t _line_input, *line_input = &_line_input;
   vnet_main_t *vnm = vnet_get_main ();
-  clib_error_t *err = 0;
-  u8 *name = 0;
+  snort_main_t *sm = &snort_main;
+  snort_instance_t *si;
+  clib_error_t *err = NULL;
+  u8 *name = NULL;
+  u8 **names = NULL;
   u32 sw_if_index = ~0;
-  snort_attach_dir_t dir = SNORT_INOUT;
-  int rv = 0;
+  snort_attach_dir_t direction = SNORT_INOUT;
+  u8 is_all_instances = 0;
+  int i;
 
   /* Get a line of input. */
   if (!unformat_user (input, unformat_line_input, line_input))
@@ -233,13 +302,15 @@ snort_attach_command_fn (vlib_main_t *vm, unformat_input_t *input,
 		    vnm, &sw_if_index))
 	;
       else if (unformat (line_input, "instance %s", &name))
-	;
+	vec_add1 (names, name);
+      else if (unformat (line_input, "all-instances"))
+	is_all_instances = 1;
       else if (unformat (line_input, "input"))
-	dir = SNORT_INPUT;
+	direction = SNORT_INPUT;
       else if (unformat (line_input, "output"))
-	dir = SNORT_OUTPUT;
+	direction = SNORT_OUTPUT;
       else if (unformat (line_input, "inout"))
-	dir = SNORT_INOUT;
+	direction = SNORT_INOUT;
       else
 	{
 	  err = clib_error_return (0, "unknown input `%U'",
@@ -254,46 +325,53 @@ snort_attach_command_fn (vlib_main_t *vm, unformat_input_t *input,
       goto done;
     }
 
-  if (!name)
+  if (vec_len (names) == 0 && is_all_instances == 0)
     {
-      err = clib_error_return (0, "please specify instance name");
+      err = clib_error_return (0, "please specify instances");
       goto done;
     }
 
-  rv = snort_interface_enable_disable (vm, (char *) name, sw_if_index, 1, dir);
+  if (is_all_instances)
+    {
+      if (vec_len (sm->instances) == 0)
+	{
+	  err = clib_error_return (0, "no snort instances have been created");
+	  goto done;
+	}
 
-  switch (rv)
+      pool_foreach (si, sm->instances)
+	{
+	  snort_attach_detach_instance (vm, vnm, (char *) si->name,
+					sw_if_index, 1 /* is_enable */,
+					direction);
+	}
+    }
+  else
     {
-    case 0:
-      break;
-    case VNET_API_ERROR_FEATURE_ALREADY_ENABLED:
-      /* already attached to same instance */
-      break;
-    case VNET_API_ERROR_INSTANCE_IN_USE:
-      err = clib_error_return (0,
-			       "interface %U already assigned to "
-			       "an instance",
-			       format_vnet_sw_if_index_name, vnm, sw_if_index);
-      break;
-    case VNET_API_ERROR_NO_SUCH_ENTRY:
-      err = clib_error_return (0, "unknown instance '%s'", name);
-      break;
-    default:
-      err = clib_error_return (0, "snort_interface_enable_disable returned %d",
-			       rv);
-      break;
+      vec_foreach_index (i, names)
+	{
+	  snort_attach_detach_instance (vm, vnm, (char *) names[i],
+					sw_if_index, 1 /* is_enable */,
+					direction);
+	}
     }
 
 done:
-  vec_free (name);
+  vec_foreach_index (i, names)
+    {
+      vec_free (names[i]);
+    }
+  vec_free (names);
   unformat_free (line_input);
   return err;
 }
 
 VLIB_CLI_COMMAND (snort_attach_command, static) = {
   .path = "snort attach",
-  .short_help = "snort attach instance <name> interface <if-name> "
-		"[input|ouput|inout]",
+  .short_help =
+    "snort attach all-instances|(instance <name> [instance <name> [...]]) "
+    "interface <if-name> "
+    "[input|output|inout]",
   .function = snort_attach_command_fn,
 };
 
@@ -303,9 +381,12 @@ snort_detach_command_fn (vlib_main_t *vm, unformat_input_t *input,
 {
   unformat_input_t _line_input, *line_input = &_line_input;
   vnet_main_t *vnm = vnet_get_main ();
-  clib_error_t *err = 0;
+  clib_error_t *err = NULL;
+  u8 *name = NULL;
+  u8 **names = NULL;
   u32 sw_if_index = ~0;
-  int rv = 0;
+  u8 is_all_instances = 0;
+  int i = 0;
 
   /* Get a line of input. */
   if (!unformat_user (input, unformat_line_input, line_input))
@@ -313,8 +394,12 @@ snort_detach_command_fn (vlib_main_t *vm, unformat_input_t *input,
 
   while (unformat_check_input (line_input) != UNFORMAT_END_OF_INPUT)
     {
-      if (unformat (line_input, "interface %U", unformat_vnet_sw_interface,
-		    vnm, &sw_if_index))
+      if (unformat (line_input, "instance %s", &name))
+	vec_add1 (names, name);
+      else if (unformat (line_input, "all-instances"))
+	is_all_instances = 1;
+      else if (unformat (line_input, "interface %U",
+			 unformat_vnet_sw_interface, vnm, &sw_if_index))
 	;
       else
 	{
@@ -330,32 +415,41 @@ snort_detach_command_fn (vlib_main_t *vm, unformat_input_t *input,
       goto done;
     }
 
-  rv = snort_interface_enable_disable (vm, 0, sw_if_index, 0, SNORT_INOUT);
+  if (vec_len (names) == 0)
+    {
+      /* To maintain backwards compatibility */
+      is_all_instances = 1;
+    }
 
-  switch (rv)
+  if (is_all_instances)
     {
-    case 0:
-      break;
-    case VNET_API_ERROR_INVALID_INTERFACE:
-      err = clib_error_return (0,
-			       "interface %U is not assigned to snort "
-			       "instance!",
-			       format_vnet_sw_if_index_name, vnm, sw_if_index);
-      break;
-    default:
-      err = clib_error_return (0, "snort_interface_enable_disable returned %d",
-			       rv);
-      break;
+      err = snort_detach_all_instance (vm, vnm, sw_if_index);
+    }
+  else
+    {
+      vec_foreach_index (i, names)
+	{
+	  snort_attach_detach_instance (vm, vnm, (char *) names[i],
+					sw_if_index, 0 /* is_enable */,
+					SNORT_INOUT);
+	}
     }
 
 done:
+  vec_foreach_index (i, names)
+    {
+      vec_free (names[i]);
+    }
+  vec_free (names);
   unformat_free (line_input);
   return err;
 }
 
 VLIB_CLI_COMMAND (snort_detach_command, static) = {
   .path = "snort detach",
-  .short_help = "snort detach interface <if-name>",
+  .short_help =
+    "snort detach all-instances|(instance <name> [instance <name> [...]]) "
+    "interface <if-name> ",
   .function = snort_detach_command_fn,
 };
 
@@ -384,17 +478,57 @@ snort_show_interfaces_command_fn (vlib_main_t *vm, unformat_input_t *input,
 {
   snort_main_t *sm = &snort_main;
   vnet_main_t *vnm = vnet_get_main ();
-  snort_instance_t *si;
-  u32 *index;
-
-  vlib_cli_output (vm, "interface\t\tsnort instance");
-  vec_foreach (index, sm->instance_by_sw_if_index)
+  snort_interface_data_t *interface;
+  snort_instance_t *instance;
+  snort_attach_dir_t direction;
+  u32 instance_index;
+  u32 sw_if_index;
+  u8 is_input;
+  int i, j;
+
+  vlib_cli_output (vm, "interface\tinstances\tdirection");
+  vec_foreach_index (sw_if_index, sm->interfaces)
     {
-      if (index[0] != ~0)
+      interface = vec_elt_at_index (sm->interfaces, sw_if_index);
+
+      /* Loop over input instances and prints all of them (with direction
+       * indicated), then continues over output instances while ignoring
+       * previously printed input instances */
+      for (i = 0; i < vec_len (interface->input_instance_indices) +
+			vec_len (interface->output_instance_indices);
+	   i++)
 	{
-	  si = vec_elt_at_index (sm->instances, index[0]);
-	  vlib_cli_output (vm, "%U:\t%s", format_vnet_sw_if_index_name, vnm,
-			   index - sm->instance_by_sw_if_index, si->name);
+	  is_input = i < vec_len (interface->input_instance_indices);
+
+	  instance_index =
+	    is_input ? interface->input_instance_indices[i] :
+		       interface->output_instance_indices
+			 [i - vec_len (interface->input_instance_indices)];
+
+	  /* When printing the output instances ignore the ones present in
+	   * input instances as we have already printed them */
+	  if (!is_input)
+	    {
+	      j =
+		vec_search (interface->input_instance_indices, instance_index);
+	      if (j != ~0)
+		continue;
+	    }
+
+	  instance = snort_get_instance_by_index (instance_index);
+	  direction = snort_get_instance_direction (instance_index, interface);
+	  if (i == 0)
+	    {
+	      vlib_cli_output (vm, "%U:\t%s\t\t%s",
+			       format_vnet_sw_if_index_name, vnm, sw_if_index,
+			       instance->name,
+			       snort_get_direction_name_by_enum (direction));
+	    }
+	  else
+	    {
+	      vlib_cli_output (vm, "\t\t%s\t\t%s", instance->name,
+			       snort_get_direction_name_by_enum (direction));
+	    }
 	}
     }
   return 0;
diff --git a/src/plugins/snort/enqueue.c b/src/plugins/snort/enqueue.c
index ce4f34491ec..84efb4d432f 100644
--- a/src/plugins/snort/enqueue.c
+++ b/src/plugins/snort/enqueue.c
@@ -1,7 +1,10 @@
 /* SPDX-License-Identifier: Apache-2.0
  * Copyright(c) 2021 Cisco Systems, Inc.
+ * Copyright(c) 2024 Arm Limited
  */
 
+#include <vnet/ip/ip4_inlines.h>
+#include <vnet/ip/ip4_packet.h>
 #include <vlib/vlib.h>
 #include <vnet/feature/feature.h>
 #include <snort/snort.h>
@@ -56,6 +59,33 @@ static char *snort_enq_error_strings[] = {
 #undef _
 };
 
+static_always_inline u32
+get_snort_instance_index_ip4 (snort_main_t *sm, vlib_buffer_t *b, u32 fa_data)
+{
+  u32 hash;
+  u32 sw_if_index = vnet_buffer (b)->sw_if_index[VLIB_RX];
+  ip4_header_t *ip = NULL;
+  u32 *instances = (fa_data == SNORT_INPUT) ?
+		     sm->interfaces[sw_if_index].input_instance_indices :
+		     sm->interfaces[sw_if_index].output_instance_indices;
+  int n_instances = vec_len (instances);
+
+  if (n_instances == 1)
+    {
+      return instances[0];
+    }
+  ip = vlib_buffer_get_current (b);
+  hash = ip4_compute_flow_hash (ip, IP_FLOW_HASH_DEFAULT);
+  return instances[hash % n_instances];
+}
+
+static_always_inline snort_instance_t *
+get_snort_instance (snort_main_t *sm, vlib_buffer_t *b, u32 fa_data)
+{
+  u32 instance_index = get_snort_instance_index_ip4 (sm, b, fa_data);
+  return snort_get_instance_by_index (instance_index);
+}
+
 static_always_inline uword
 snort_enq_node_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
 		       vlib_frame_t *frame, int with_trace)
@@ -66,26 +96,24 @@ snort_enq_node_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
   u32 thread_index = vm->thread_index;
   u32 n_left = frame->n_vectors;
   u32 n_trace = 0;
-  u32 total_enq = 0, n_processed = 0;
+  u32 total_enq = 0, n_unprocessed = 0;
   u32 *from = vlib_frame_vector_args (frame);
   vlib_buffer_t *bufs[VLIB_FRAME_SIZE], **b = bufs;
   u16 nexts[VLIB_FRAME_SIZE], *next = nexts;
+  u32 unprocessed_bufs[VLIB_FRAME_SIZE];
 
   vlib_get_buffers (vm, from, bufs, n_left);
 
   while (n_left)
     {
-      u64 fa_data;
-      u32 instance_index, next_index, n;
-      u32 l3_offset;
-
-      fa_data =
-	*(u64 *) vnet_feature_next_with_data (&next_index, b[0], sizeof (u64));
-
-      instance_index = (u32) (fa_data & 0xffffffff);
-      l3_offset =
-	(fa_data >> 32) ? vnet_buffer (b[0])->ip.save_rewrite_length : 0;
-      si = vec_elt_at_index (sm->instances, instance_index);
+      u32 next_index, n;
+      /* fa_data is either SNORT_INPUT or SNORT_OUTPUT */
+      u32 fa_data =
+	*(u32 *) vnet_feature_next_with_data (&next_index, b[0], sizeof (u32));
+      u32 l3_offset = (fa_data == SNORT_INPUT) ?
+			0 :
+			vnet_buffer (b[0])->ip.save_rewrite_length;
+      si = get_snort_instance (sm, b[0], fa_data);
 
       /* if client isn't connected skip enqueue and take default action */
       if (PREDICT_FALSE (si->client_index == ~0))
@@ -95,7 +123,8 @@ snort_enq_node_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
 	  else
 	    next[0] = next_index;
 	  next++;
-	  n_processed++;
+	  unprocessed_bufs[n_unprocessed] = from[0];
+	  n_unprocessed++;
 	}
       else
 	{
@@ -108,7 +137,7 @@ snort_enq_node_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
 
 	  vlib_buffer_chain_linearize (vm, b[0]);
 
-	  /* If this pkt is traced, snapshoot the data */
+	  /* If this pkt is traced, snapshot the data */
 	  if (with_trace && b[0]->flags & VLIB_BUFFER_IS_TRACED)
 	    n_trace++;
 
@@ -125,12 +154,12 @@ snort_enq_node_inline (vlib_main_t *vm, vlib_node_runtime_t *node,
       b++;
     }
 
-  if (n_processed)
+  if (n_unprocessed)
     {
       vlib_node_increment_counter (vm, snort_enq_node.index,
-				   SNORT_ENQ_ERROR_NO_INSTANCE, n_processed);
-      vlib_buffer_enqueue_to_next (vm, node, vlib_frame_vector_args (frame),
-				   nexts, n_processed);
+				   SNORT_ENQ_ERROR_NO_INSTANCE, n_unprocessed);
+      vlib_buffer_enqueue_to_next (vm, node, unprocessed_bufs, nexts,
+				   n_unprocessed);
     }
 
   pool_foreach (si, sm->instances)
diff --git a/src/plugins/snort/main.c b/src/plugins/snort/main.c
index 50bff027a13..9bab1185b60 100644
--- a/src/plugins/snort/main.c
+++ b/src/plugins/snort/main.c
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: Apache-2.0
  * Copyright(c) 2021 Cisco Systems, Inc.
+ * Copyright(c) 2024 Arm Limited
  */
 
 #include <vlib/vlib.h>
@@ -96,6 +97,38 @@ snort_instance_disconnect (vlib_main_t *vm, u32 instance_index)
   return rv;
 }
 
+const char *
+snort_get_direction_name_by_enum (snort_attach_dir_t dir)
+{
+  switch (dir)
+    {
+    case SNORT_INPUT:
+      return "input";
+    case SNORT_OUTPUT:
+      return "output";
+    case SNORT_INOUT:
+      return "inout";
+    default:
+      return "none";
+    }
+}
+
+/* Returns SNORT_INVALID if the instance is not attached */
+snort_attach_dir_t
+snort_get_instance_direction (u32 instance_index,
+			      snort_interface_data_t *interface)
+{
+  snort_attach_dir_t direction = SNORT_INVALID;
+  int i;
+  i = vec_search (interface->input_instance_indices, instance_index);
+  if (i != ~0)
+    direction = direction | SNORT_INPUT;
+  i = vec_search (interface->output_instance_indices, instance_index);
+  if (i != ~0)
+    direction = direction | SNORT_OUTPUT;
+  return direction;
+}
+
 snort_instance_t *
 snort_get_instance_by_name (char *name)
 {
@@ -470,6 +503,30 @@ done:
   return rv;
 }
 
+static void
+snort_vnet_feature_enable_disable (snort_attach_dir_t snort_dir,
+				   u32 sw_if_index, int is_enable)
+{
+  u32 fa_data;
+  switch (snort_dir)
+    {
+    case SNORT_INPUT:
+      fa_data = SNORT_INPUT;
+      vnet_feature_enable_disable ("ip4-unicast", "snort-enq", sw_if_index,
+				   is_enable, &fa_data, sizeof (fa_data));
+      break;
+    case SNORT_OUTPUT:
+      fa_data = SNORT_OUTPUT;
+      vnet_feature_enable_disable ("ip4-output", "snort-enq", sw_if_index,
+				   is_enable, &fa_data, sizeof (fa_data));
+      break;
+    default:
+      vlib_log_err (snort_log.class,
+		    "Invalid direction given to enable/disable snort");
+      break;
+    }
+}
+
 int
 snort_interface_enable_disable (vlib_main_t *vm, char *instance_name,
 				u32 sw_if_index, int is_enable,
@@ -477,92 +534,216 @@ snort_interface_enable_disable (vlib_main_t *vm, char *instance_name,
 {
   snort_main_t *sm = &snort_main;
   vnet_main_t *vnm = vnet_get_main ();
-  snort_instance_t *si;
-  u64 fa_data;
-  u32 index;
+  vnet_sw_interface_t *software_interface =
+    vnet_get_sw_interface (vnm, sw_if_index);
+  snort_interface_data_t *interface_data;
+  snort_instance_t *instance;
+  u32 **instance_indices;
+  u32 instance_index;
+  const snort_attach_dir_t dirs[2] = { SNORT_INPUT, SNORT_OUTPUT };
   int rv = 0;
+  int index, i;
 
-  if (is_enable)
+  /* If interface is up, do not allow modifying attached instances */
+  if (software_interface->flags & VNET_SW_INTERFACE_FLAG_ADMIN_UP)
     {
-      if ((si = snort_get_instance_by_name (instance_name)) == 0)
-	{
-	  log_err ("unknown instance '%s'", instance_name);
-	  return VNET_API_ERROR_NO_SUCH_ENTRY;
-	}
+      rv = VNET_API_ERROR_INSTANCE_IN_USE;
+      log_err ("interface '%U' is currently up", format_vnet_sw_if_index_name,
+	       vnm, sw_if_index);
+      goto done;
+    }
+
+  /* Check if provided instance name exists */
+  instance = snort_get_instance_by_name (instance_name);
+  if (instance == NULL)
+    {
+      rv = VNET_API_ERROR_NO_SUCH_ENTRY;
+      log_err ("unknown instance '%s'", instance_name);
+      goto done;
+    }
+
+  /* Check if interface is attached before unnecessarily increasing size of
+   * vector */
+  if (!is_enable && vec_len (sm->interfaces) <= sw_if_index)
+    {
+      rv = VNET_API_ERROR_INVALID_INTERFACE;
+      log_err ("interface %U is not assigned to snort instance %s!",
+	       format_vnet_sw_if_index_name, vnm, sw_if_index, instance->name);
+      goto done;
+    }
 
-      vec_validate_init_empty (sm->instance_by_sw_if_index, sw_if_index, ~0);
+  /* vec_validate initialises empty space to 0s, which corresponds to null
+   * pointers (i.e. empty vectors) in the snort_interface_data_t structs which
+   * is precisely what we need */
+  vec_validate (sm->interfaces, sw_if_index);
 
-      index = sm->instance_by_sw_if_index[sw_if_index];
-      if (index != ~0)
+  interface_data = vec_elt_at_index (sm->interfaces, sw_if_index);
+  instance_index = instance->index;
+
+  /* When detaching with direction SNORT_INOUT choose currently attached
+   * directions */
+  if (!is_enable)
+    {
+      snort_dir =
+	snort_get_instance_direction (instance_index, interface_data);
+      /* If snort_dir is SNORT_INVALID then the instance is not attached */
+      if (snort_dir == SNORT_INVALID)
 	{
-	  if (index == si->index)
-	    rv = VNET_API_ERROR_FEATURE_ALREADY_ENABLED;
-	  else
-	    rv = VNET_API_ERROR_INSTANCE_IN_USE;
-	  si = vec_elt_at_index (sm->instances, index);
-	  log_err ("interface %U already assgined to instance '%s'",
-		   format_vnet_sw_if_index_name, vnm, sw_if_index, si->name);
+	  rv = VNET_API_ERROR_INVALID_INTERFACE;
+	  log_err ("interface %U is not assigned to snort instance %s!",
+		   format_vnet_sw_if_index_name, vnm, sw_if_index,
+		   instance->name);
 	  goto done;
 	}
+    }
 
-      index = sm->instance_by_sw_if_index[sw_if_index] = si->index;
-      if (snort_dir & SNORT_INPUT)
-	{
-	  fa_data = (u64) index;
-	  vnet_feature_enable_disable ("ip4-unicast", "snort-enq", sw_if_index,
-				       1, &fa_data, sizeof (fa_data));
-	}
-      if (snort_dir & SNORT_OUTPUT)
-	{
-	  fa_data = (1LL << 32 | index);
-	  vnet_feature_enable_disable ("ip4-output", "snort-enq", sw_if_index,
-				       1, &fa_data, sizeof (fa_data));
-	}
+  /* Error if direction is invalid */
+  if (snort_dir == SNORT_INVALID)
+    {
+      rv = VNET_API_ERROR_INVALID_ARGUMENT;
+      vlib_log_err (snort_log.class,
+		    "cannot attach/detach with invalid direction ");
+      goto done;
     }
-  else
+
+  /* Loop evaluates input instances and then output instances */
+  for (i = 0; i < 2; i++)
     {
-      if (sw_if_index >= vec_len (sm->instance_by_sw_if_index) ||
-	  sm->instance_by_sw_if_index[sw_if_index] == ~0)
+      if (!(snort_dir & dirs[i]))
+	continue;
+
+      instance_indices = (dirs[i] == SNORT_INPUT) ?
+			   &(interface_data->input_instance_indices) :
+			   &(interface_data->output_instance_indices);
+      index = vec_search (*instance_indices, instance_index);
+
+      if (is_enable)
 	{
-	  rv = VNET_API_ERROR_INVALID_INTERFACE;
-	  log_err ("interface %U is not assigned to snort instance!",
-		   format_vnet_sw_if_index_name, vnm, sw_if_index);
-	  goto done;
+	  /* Error if instance is already attached when trying to attach */
+	  if (index != ~0)
+	    {
+	      rv = VNET_API_ERROR_FEATURE_ALREADY_ENABLED;
+	      log_err ("interface %U already assgined to instance '%s' on "
+		       "direction '%s'",
+		       format_vnet_sw_if_index_name, vnm, sw_if_index,
+		       instance->name,
+		       snort_get_direction_name_by_enum (dirs[i]));
+	      goto done;
+	    }
+	}
+      else
+	{
+	  /* Error if instance is not attached when trying to detach */
+	  if (index == ~0)
+	    {
+	      rv = VNET_API_ERROR_INVALID_INTERFACE;
+	      log_err ("interface %U is not assigned to snort instance %s on "
+		       "direction '%s'!",
+		       format_vnet_sw_if_index_name, vnm, sw_if_index,
+		       instance->name,
+		       snort_get_direction_name_by_enum (dirs[i]));
+	      goto done;
+	    }
 	}
-      index = sm->instance_by_sw_if_index[sw_if_index];
-      si = vec_elt_at_index (sm->instances, index);
 
-      sm->instance_by_sw_if_index[sw_if_index] = ~0;
-      if (snort_dir & SNORT_INPUT)
+      if (is_enable)
 	{
-	  fa_data = (u64) index;
-	  vnet_feature_enable_disable ("ip4-unicast", "snort-enq", sw_if_index,
-				       0, &fa_data, sizeof (fa_data));
+	  /* Enable feature if not previously enabled */
+	  if (vec_len (*instance_indices) == 0)
+	    {
+	      snort_vnet_feature_enable_disable (dirs[i], sw_if_index,
+						 1 /* is_enable */);
+	    }
+	  vec_add1 (*instance_indices, instance_index);
 	}
-      if (snort_dir & SNORT_OUTPUT)
+      else
 	{
-	  fa_data = (1LL << 32 | index);
-	  vnet_feature_enable_disable ("ip4-output", "snort-enq", sw_if_index,
-				       0, &fa_data, sizeof (fa_data));
+	  /* Disable feature when removing last instance */
+	  if (vec_len (*instance_indices) == 1)
+	    {
+	      snort_vnet_feature_enable_disable (dirs[i], sw_if_index,
+						 0 /* is_enable */);
+	    }
+	  vec_del1 (*instance_indices, index);
 	}
     }
+done:
+  return rv;
+}
+
+int
+snort_interface_disable_all (vlib_main_t *vm, u32 sw_if_index)
+{
+  snort_main_t *sm = &snort_main;
+  vnet_main_t *vnm = vnet_get_main ();
+  vnet_sw_interface_t *software_interface =
+    vnet_get_sw_interface (vnm, sw_if_index);
+  snort_interface_data_t *interface_data;
+  int rv = 0;
+
+  if (software_interface->flags & VNET_SW_INTERFACE_FLAG_ADMIN_UP)
+    {
+      rv = VNET_API_ERROR_INSTANCE_IN_USE;
+      log_err ("interface '%U' is currently up", format_vnet_sw_if_index_name,
+	       vnm, sw_if_index);
+      goto done;
+    }
+
+  if (vec_len (sm->interfaces) <= sw_if_index)
+    {
+      rv = VNET_API_ERROR_INVALID_INTERFACE;
+      log_err ("no instances attached to interface %U",
+	       format_vnet_sw_if_index_name, vnm, sw_if_index);
+      goto done;
+    }
+
+  interface_data = vec_elt_at_index (sm->interfaces, sw_if_index);
+
+  if (vec_len (interface_data->input_instance_indices) == 0 &&
+      vec_len (interface_data->output_instance_indices) == 0)
+    {
+      rv = VNET_API_ERROR_INVALID_INTERFACE;
+      log_err ("no instances attached to interface %U",
+	       format_vnet_sw_if_index_name, vnm, sw_if_index);
+      goto done;
+    }
+
+  if (vec_len (interface_data->input_instance_indices) > 0)
+    {
+      snort_vnet_feature_enable_disable (SNORT_INPUT, sw_if_index,
+					 0 /* is_enable */);
+      vec_free (interface_data->input_instance_indices);
+    }
+  if (vec_len (interface_data->output_instance_indices) > 0)
+    {
+      snort_vnet_feature_enable_disable (SNORT_OUTPUT, sw_if_index,
+					 0 /* is_enable */);
+      vec_free (interface_data->output_instance_indices);
+    }
 
 done:
   return rv;
 }
 
 static int
-snort_strip_instance_interfaces (vlib_main_t *vm, u32 instance_index)
+snort_strip_instance_interfaces (vlib_main_t *vm, snort_instance_t *instance)
 {
   snort_main_t *sm = &snort_main;
-  u32 *index;
+  snort_interface_data_t *interface;
+  snort_attach_dir_t direction;
+  int i;
   int rv = 0;
 
-  vec_foreach (index, sm->instance_by_sw_if_index)
+  /* Find all interfaces containing the given snort instance to disable */
+  vec_foreach_index (i, sm->interfaces)
     {
-      if (*index == instance_index)
-	rv = snort_interface_enable_disable (
-	  vm, NULL, index - sm->instance_by_sw_if_index, 0, 0);
+      /* Check if the snort_instance is attached by checking if the direction
+       * is SNORT_INVALID */
+      interface = vec_elt_at_index (sm->interfaces, i);
+      direction = snort_get_instance_direction (instance->index, interface);
+      if (direction != SNORT_INVALID)
+	rv = snort_interface_enable_disable (vm, (char *) instance->name, i,
+					     0 /* is_enable */, direction);
       if (rv)
 	break;
     }
@@ -585,7 +766,7 @@ snort_instance_delete (vlib_main_t *vm, u32 instance_index)
   if (si->client_index != ~0)
     return VNET_API_ERROR_INSTANCE_IN_USE;
 
-  if ((rv = snort_strip_instance_interfaces (vm, si->index)))
+  if ((rv = snort_strip_instance_interfaces (vm, si)))
     return rv;
 
   hash_unset_mem (sm->instance_by_name, si->name);
diff --git a/src/plugins/snort/snort.h b/src/plugins/snort/snort.h
index c7e856c0127..76f0652df10 100644
--- a/src/plugins/snort/snort.h
+++ b/src/plugins/snort/snort.h
@@ -1,5 +1,6 @@
 /* SPDX-License-Identifier: Apache-2.0
  * Copyright(c) 2021 Cisco Systems, Inc.
+ * Copyright(c) 2024 Arm Limited
  */
 
 #ifndef __snort_snort_h__
@@ -68,13 +69,20 @@ typedef struct
   void *interrupts;
 } snort_per_thread_data_t;
 
+/* Holds snort plugin related information for an interface */
+typedef struct
+{
+  u32 *input_instance_indices;
+  u32 *output_instance_indices;
+} snort_interface_data_t;
+
 typedef struct
 {
   clib_socket_t *listener;
   snort_client_t *clients;
   snort_instance_t *instances;
   uword *instance_by_name;
-  u32 *instance_by_sw_if_index;
+  snort_interface_data_t *interfaces;
   u8 **buffer_pool_base_addrs;
   snort_per_thread_data_t *per_thread_data;
   u32 input_mode;
@@ -96,9 +104,11 @@ typedef enum
 
 typedef enum
 {
-  SNORT_INPUT = 1,
-  SNORT_OUTPUT = 2,
-  SNORT_INOUT = 3
+  SNORT_INVALID = 0x00,
+  SNORT_INPUT = 0x01,
+  SNORT_OUTPUT = 0x02,
+  /* SNORT_INOUT === SNORT_INPUT | SNORT_OUTPUT */
+  SNORT_INOUT = 0x03
 } snort_attach_dir_t;
 
 #define SNORT_ENQ_NEXT_NODES                                                  \
@@ -108,6 +118,10 @@ typedef enum
 
 /* functions */
 snort_main_t *snort_get_main ();
+const char *snort_get_direction_name_by_enum (snort_attach_dir_t dir);
+snort_attach_dir_t
+snort_get_instance_direction (u32 instance_index,
+			      snort_interface_data_t *interface);
 snort_instance_t *snort_get_instance_by_index (u32 instance_index);
 snort_instance_t *snort_get_instance_by_name (char *name);
 int snort_instance_create (vlib_main_t *vm, char *name, u8 log2_queue_sz,
@@ -115,6 +129,7 @@ int snort_instance_create (vlib_main_t *vm, char *name, u8 log2_queue_sz,
 int snort_interface_enable_disable (vlib_main_t *vm, char *instance_name,
 				    u32 sw_if_index, int is_enable,
 				    snort_attach_dir_t dir);
+int snort_interface_disable_all (vlib_main_t *vm, u32 sw_if_index);
 int snort_set_node_mode (vlib_main_t *vm, u32 mode);
 int snort_instance_delete (vlib_main_t *vm, u32 instance_index);
 int snort_instance_disconnect (vlib_main_t *vm, u32 instance_index);
diff --git a/src/plugins/snort/snort_api.c b/src/plugins/snort/snort_api.c
index adad0d8763f..4016dfad63f 100644
--- a/src/plugins/snort/snort_api.c
+++ b/src/plugins/snort/snort_api.c
@@ -185,7 +185,8 @@ vl_api_snort_interface_get_t_handler (vl_api_snort_interface_get_t *mp)
   snort_main_t *sm = snort_get_main ();
   vl_api_snort_interface_get_reply_t *rmp;
   u32 sw_if_index;
-  u32 *index;
+  u32 *instances;
+  u32 index;
   int rv = 0;
 
   sw_if_index = clib_net_to_host_u32 (mp->sw_if_index);
@@ -193,7 +194,7 @@ vl_api_snort_interface_get_t_handler (vl_api_snort_interface_get_t *mp)
   if (sw_if_index == INDEX_INVALID)
     {
       /* clang-format off */
-      if (vec_len (sm->instance_by_sw_if_index) == 0)
+      if (vec_len (sm->interfaces) == 0)
 	{
 	  REPLY_MACRO2 (VL_API_SNORT_INTERFACE_GET_REPLY, ({ rmp->cursor = ~0; }));
 	  return;
@@ -201,17 +202,36 @@ vl_api_snort_interface_get_t_handler (vl_api_snort_interface_get_t *mp)
 
       REPLY_AND_DETAILS_VEC_MACRO(
 	VL_API_SNORT_INTERFACE_GET_REPLY,
-	sm->instance_by_sw_if_index,
+	sm->interfaces,
 	mp, rmp, rv, ({
-          index = vec_elt_at_index (sm->instance_by_sw_if_index, cursor);
-          send_snort_interface_details (cursor, *index, rp, mp->context);
+          instances = vec_len(sm->interfaces[cursor].input_instance_indices) ?
+           sm->interfaces[cursor].input_instance_indices : sm->interfaces[cursor].output_instance_indices;
+          if (vec_len(instances) == 0)
+          {
+            index = ~0;
+          }
+          else {
+            index = instances[0];
+          }
+          send_snort_interface_details (cursor, index, rp, mp->context);
 	}))
       /* clang-format on */
     }
   else
     {
-      index = vec_elt_at_index (sm->instance_by_sw_if_index, sw_if_index);
-      if (snort_get_instance_by_index (index[0]))
+      instances =
+	vec_len (sm->interfaces[sw_if_index].input_instance_indices) ?
+	  sm->interfaces[sw_if_index].input_instance_indices :
+	  sm->interfaces[sw_if_index].output_instance_indices;
+      if (vec_len (instances) == 0)
+	{
+	  index = ~0;
+	}
+      else
+	{
+	  index = instances[0];
+	}
+      if (snort_get_instance_by_index (index))
 	{
 	  vl_api_registration_t *rp =
 	    vl_api_client_index_to_registration (mp->client_index);
@@ -221,7 +241,8 @@ vl_api_snort_interface_get_t_handler (vl_api_snort_interface_get_t *mp)
 	      return;
 	    }
 
-	  send_snort_interface_details (sw_if_index, *index, rp, mp->context);
+	  send_snort_interface_details (sw_if_index, *instances, rp,
+					mp->context);
 	}
       else
 	{
@@ -352,11 +373,9 @@ vl_api_snort_interface_detach_t_handler (vl_api_snort_interface_detach_t *mp)
   vlib_main_t *vm = vlib_get_main ();
   vl_api_snort_interface_detach_reply_t *rmp;
   u32 sw_if_index = clib_net_to_host_u32 (mp->sw_if_index);
-  int rv = VNET_API_ERROR_NO_MATCHING_INTERFACE;
+  int rv;
 
-  if (sw_if_index != INDEX_INVALID)
-    rv = snort_interface_enable_disable (vm, NULL, sw_if_index,
-					 0 /* is_enable */, SNORT_INOUT);
+  rv = snort_interface_disable_all (vm, sw_if_index);
 
   REPLY_MACRO (VL_API_SNORT_INTERFACE_DETACH_REPLY);
 }
diff --git a/test/test_snort.py b/test/test_snort.py
index 19401cb7b85..c25c0e65145 100644
--- a/test/test_snort.py
+++ b/test/test_snort.py
@@ -12,10 +12,10 @@ class TestSnort(VppTestCase):
     def setUpClass(cls):
         super(TestSnort, cls).setUpClass()
         try:
-            cls.create_pg_interfaces(range(2))
+            cls.create_pg_interfaces(range(4))
             for i in cls.pg_interfaces:
                 i.config_ip4().resolve_arp()
-                i.admin_up()
+                i.admin_down()
         except Exception:
             cls.tearDownClass()
             raise
@@ -24,7 +24,6 @@ class TestSnort(VppTestCase):
     def tearDownClass(cls):
         for i in cls.pg_interfaces:
             i.unconfig_ip4()
-            i.admin_down()
         super(TestSnort, cls).tearDownClass()
 
     def test_snort_cli(self):
@@ -36,14 +35,18 @@ class TestSnort(VppTestCase):
             "snort create-instance name snortTest2 queue-size 16 on-disconnect pass": "",
             "snort attach instance snortTest interface pg0 output": "",
             "snort attach instance snortTest2 interface pg1 input": "",
+            "snort attach all-instances interface pg2 inout": "",
+            "snort attach instance snortTest instance snortTest2 interface pg3 inout": "",
             "show snort instances": "snortTest",
             "show snort interfaces": "pg0",
             "show snort clients": "number of clients",
             "show snort mode": "input mode: interrupt",
             "snort mode polling": "",
             "snort mode interrupt": "",
-            "snort detach interface pg0": "",
-            "snort detach interface pg1": "",
+            "snort detach instance snortTest interface pg0": "",
+            "snort detach instance snortTest2 interface pg1": "",
+            "snort detach all-instances interface pg2": "",
+            "snort detach instance snortTest instance snortTest2 interface pg3": "",
             "snort delete instance snortTest": "",
         }
 
@@ -64,7 +67,7 @@ class TestSnortVapi(VppTestCase):
             for i in cls.pg_interfaces:
                 i.config_ip4()
                 i.resolve_arp()
-                i.admin_up()
+                i.admin_down()
         except Exception:
             cls.tearDownClass()
             raise
@@ -73,7 +76,6 @@ class TestSnortVapi(VppTestCase):
     def tearDownClass(cls):
         for i in cls.pg_interfaces:
             i.unconfig_ip4()
-            i.admin_down()
         super(TestSnortVapi, cls).tearDownClass()
 
     def test_snort_01_modes_set_interrupt(self):
@@ -109,20 +111,27 @@ class TestSnortVapi(VppTestCase):
         reply = self.vapi.snort_interface_attach(
             instance_index=0, sw_if_index=1, snort_dir=1
         )
+        reply = self.vapi.snort_interface_attach(
+            instance_index=0, sw_if_index=2, snort_dir=2
+        )
+        reply = self.vapi.snort_interface_attach(
+            instance_index=1, sw_if_index=2, snort_dir=3
+        )
+        reply = self.vapi.cli("show snort interfaces")
+        self.assertIn("snortTest0", reply)
+        self.assertIn("snortTest1", reply)
+        self.assertIn("input", reply)
+        self.assertIn("inout", reply)
+        self.assertIn("output", reply)
         try:
             reply = self.vapi.snort_interface_attach(
-                instance_index=1, sw_if_index=1, snort_dir=1
+                instance_index=1, sw_if_index=2, snort_dir=2
             )
         except:
             pass
         else:
             self.assertNotEqual(reply.retval, 0)
-
-        reply = self.vapi.snort_interface_attach(
-            instance_index=1, sw_if_index=2, snort_dir=3
-        )
         reply = self.vapi.cli("show snort interfaces")
-        self.assertIn("snortTest0", reply)
         self.assertIn("snortTest1", reply)
 
     def test_snort_05_delete_instance(self):
@@ -131,14 +140,13 @@ class TestSnortVapi(VppTestCase):
         reply = self.vapi.cli("show snort interfaces")
         self.assertNotIn("snortTest0", reply)
         self.assertIn("snortTest1", reply)
-        reply = self.vapi.cli("show snort interfaces")
         self.assertNotIn("pg0", reply)
         self.assertIn("pg1", reply)
 
     def test_snort_06_detach_if(self):
         """Interfaces can be detached"""
         try:
-            reply = self.vapi.snort_interface_detach(sw_if_index=1)
+            reply = self.vapi.snort_interface_detach(sw_if_index=3)
         except:
             pass
         else: