sflow: replace VAPI with DLAPI

Remove the requirement to coordinate between linux-cp and sflow at
build time via cmake. Also, remove 350 lines of awkward thread-forking
VAPI code. Replace it with a dynamically retrieved function from the
linux-cp plugin, called lcp_itf_pair_get_vif_index_by_phy().

Remove build inhibit based on Netlink headers, and only inhibit the
build for FreeBSD. This plugin can now run regardless of Linux CP
being built or loaded, and then return VPP ifIndex numbers instead.

Also, fix a flaky test where non-ephemeral source ports throw off
packet captures.

Type: improvement
Change-Id: I5486742fa0e343e382630a22672a05fe3dcf7836
Signed-off-by: neil.mckee@inmon.com
Signed-off-by: pim@ipng.nl
(cherry picked from commit f1467f6be578088586abad1cc2c96038d8247794)

9 files changed, 75 insertions, 345 deletions

diff --git a/src/plugins/linux-cp/lcp_interface.c b/src/plugins/linux-cp/lcp_interface.c
index 61665ad4146..9a6b9b11be5 100644
--- a/src/plugins/linux-cp/lcp_interface.c
+++ b/src/plugins/linux-cp/lcp_interface.c
@@ -162,6 +162,22 @@ lcp_itf_pair_get (u32 index)
   return pool_elt_at_index (lcp_itf_pair_pool, index);
 }
 
+/* binary-direct API: for access from other plugins, bypassing VAPI.
+ * Important for parameters and return types to be simple C types, rather
+ * than structures. See src/plugins/sflow/sflow_dlapi.h for an example.
+ */
+u32
+lcp_itf_pair_get_vif_index_by_phy (u32 phy_sw_if_index)
+{
+  if (phy_sw_if_index < vec_len (lip_db_by_phy))
+    {
+      lcp_itf_pair_t *lip = lcp_itf_pair_get (lip_db_by_phy[phy_sw_if_index]);
+      if (lip)
+	return lip->lip_vif_index;
+    }
+  return INDEX_INVALID;
+}
+
 index_t
 lcp_itf_pair_find_by_vif (u32 vif_index)
 {
diff --git a/src/plugins/sflow/CMakeLists.txt b/src/plugins/sflow/CMakeLists.txt
index 35433bd24df..c966fcc4480 100644
--- a/src/plugins/sflow/CMakeLists.txt
+++ b/src/plugins/sflow/CMakeLists.txt
@@ -12,39 +12,23 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-vpp_find_path(NETLINK_INCLUDE_DIR NAMES linux/netlink.h)
-if (NOT NETLINK_INCLUDE_DIR)
-  message(WARNING "netlink headers not found - sflow plugin disabled")
-  return()
-endif()
-
 if ("${CMAKE_SYSTEM_NAME}" STREQUAL "FreeBSD")
   message(WARNING "sflow is not supported on FreeBSD - sflow plugin disabled")
   return()
 endif()
 
-LIST(FIND excluded_plugins linux-cp exc_index)
-if(${exc_index} EQUAL "-1")
-  message(WARNING "sflow plugin - linux-cp plugin included: compiling VAPI calls")
-  add_compile_definitions(SFLOW_USE_VAPI)
-else()
-  message(WARNING "sflow plugin - linux-cp plugin excluded: not compiling VAPI calls")
-endif()
-
-include_directories(${CMAKE_SOURCE_DIR}/vpp-api ${CMAKE_CURRENT_BINARY_DIR}/../../vpp-api)
 add_vpp_plugin(sflow
   SOURCES
   sflow.c
   node.c
   sflow_common.h
   sflow.h
+  sflow_dlapi.h
   sflow_psample.c
   sflow_psample.h
   sflow_psample_fields.h
   sflow_usersock.c
   sflow_usersock.h
-  sflow_vapi.c
-  sflow_vapi.h
 
   MULTIARCH_SOURCES
   node.c
diff --git a/src/plugins/sflow/sflow.c b/src/plugins/sflow/sflow.c
index 5aa65062330..02a74d2c7f5 100644
--- a/src/plugins/sflow/sflow.c
+++ b/src/plugins/sflow/sflow.c
@@ -25,6 +25,7 @@
 #include <sflow/sflow.api_enum.h>
 #include <sflow/sflow.api_types.h>
 #include <sflow/sflow_psample.h>
+#include <sflow/sflow_dlapi.h>
 
 #include <vpp-api/client/stat_client.h>
 #include <vlib/stats/stats.h>
@@ -181,8 +182,15 @@ retry:
   SFLOWUSSpec_setMsgType (&spec, SFLOW_VPP_MSG_IF_COUNTERS);
   SFLOWUSSpec_setAttr (&spec, SFLOW_VPP_ATTR_PORTNAME, hw->name,
 		       vec_len (hw->name));
-  SFLOWUSSpec_setAttrInt (&spec, SFLOW_VPP_ATTR_IFINDEX, sfif->hw_if_index);
-  if (sfif->linux_if_index)
+  SFLOWUSSpec_setAttrInt (&spec, SFLOW_VPP_ATTR_IFINDEX, sfif->sw_if_index);
+
+  if (smp->lcp_itf_pair_get_vif_index_by_phy)
+    {
+      sfif->linux_if_index =
+	(*smp->lcp_itf_pair_get_vif_index_by_phy) (sfif->sw_if_index);
+    }
+
+  if (sfif->linux_if_index != INDEX_INVALID)
     {
       // We know the corresponding Linux ifIndex for this interface, so include
       // that here.
@@ -433,15 +441,6 @@ sflow_process_samples (vlib_main_t *vm, vlib_node_runtime_t *node,
 	  continue;
 	}
 
-#ifdef SFLOW_USE_VAPI
-#ifdef SFLOW_TEST_HAMMER_VAPI
-      sflow_vapi_check_for_linux_if_index_results (&smp->vac,
-						   smp->per_interface_data);
-      sflow_vapi_read_linux_if_index_numbers (&smp->vac,
-					      smp->per_interface_data);
-#endif
-#endif
-
       // PSAMPLE channel may need extra step (e.g. to learn family_id)
       // before it is ready to send
       EnumSFLOWPSState psState = SFLOWPS_state (&smp->sflow_psample);
@@ -458,23 +457,6 @@ sflow_process_samples (vlib_main_t *vm, vlib_node_runtime_t *node,
 	{
 	  // second rollover
 	  smp->now_mono_S = tnow_S;
-#ifdef SFLOW_USE_VAPI
-	  if (!smp->vac.vapi_unavailable)
-	    {
-	      // look up linux if_index numbers
-	      sflow_vapi_check_for_linux_if_index_results (
-		&smp->vac, smp->per_interface_data);
-	      if (smp->vapi_requests == 0 ||
-		  (tnow_S % SFLOW_VAPI_POLL_INTERVAL) == 0)
-		{
-		  if (sflow_vapi_read_linux_if_index_numbers (
-			&smp->vac, smp->per_interface_data))
-		    {
-		      smp->vapi_requests++;
-		    }
-		}
-	    }
-#endif
 	  // send status info
 	  send_sampling_status_info (smp);
 	  // poll counters for interfaces that are due
@@ -539,11 +521,6 @@ sflow_sampling_start (sflow_main_t *smp)
   smp->psample_seq_egress = 0;
   smp->psample_send_drops = 0;
 
-#ifdef SFLOW_USE_VAPI
-  // reset vapi request count so that we make a request the first time
-  smp->vapi_requests = 0;
-#endif
-
   /* open PSAMPLE netlink channel for writing packet samples */
   SFLOWPS_open (&smp->sflow_psample);
   /* open USERSOCK netlink channel for writing counters */
@@ -1027,6 +1004,18 @@ sflow_init (vlib_main_t *vm)
 
   /* access to counters - TODO: should this only happen on sflow enable? */
   sflow_stat_segment_client_init ();
+
+  smp->lcp_itf_pair_get_vif_index_by_phy =
+    vlib_get_plugin_symbol (SFLOW_LCP_LIB, SFLOW_LCP_SYM_GET_VIF_BY_PHY);
+  if (smp->lcp_itf_pair_get_vif_index_by_phy)
+    {
+      SFLOW_NOTICE ("linux-cp found - using LIP vif_index, where available");
+    }
+  else
+    {
+      SFLOW_NOTICE ("linux-cp not found - using VPP sw_if_index");
+    }
+
   return error;
 }
 
diff --git a/src/plugins/sflow/sflow.h b/src/plugins/sflow/sflow.h
index 609ff723816..0ec5ac90688 100644
--- a/src/plugins/sflow/sflow.h
+++ b/src/plugins/sflow/sflow.h
@@ -22,7 +22,6 @@
 #include <vppinfra/hash.h>
 #include <vppinfra/error.h>
 #include <sflow/sflow_common.h>
-#include <sflow/sflow_vapi.h>
 #include <sflow/sflow_psample.h>
 #include <sflow/sflow_usersock.h>
 
@@ -124,6 +123,8 @@ typedef struct
   sflow_fifo_t fifo;
 } sflow_per_thread_data_t;
 
+typedef u32 (*IfIndexLookupFn) (u32);
+
 typedef struct
 {
   /* API message ID base */
@@ -164,12 +165,7 @@ typedef struct
   u32 csample_send;
   u32 csample_send_drops;
   u32 unixsock_seq;
-#ifdef SFLOW_USE_VAPI
-  /* vapi query helper thread (transient) */
-  CLIB_CACHE_LINE_ALIGN_MARK (_vapi);
-  sflow_vapi_client_t vac;
-  int vapi_requests;
-#endif
+  IfIndexLookupFn lcp_itf_pair_get_vif_index_by_phy;
 } sflow_main_t;
 
 extern sflow_main_t sflow_main;
diff --git a/src/plugins/sflow/sflow_common.h b/src/plugins/sflow/sflow_common.h
index 29784638bb9..26f306b5741 100644
--- a/src/plugins/sflow/sflow_common.h
+++ b/src/plugins/sflow/sflow_common.h
@@ -15,8 +15,6 @@
 #ifndef __included_sflow_common_h__
 #define __included_sflow_common_h__
 
-// #define SFLOW_USE_VAPI (set by CMakeLists.txt)
-
 extern vlib_log_class_t sflow_logger;
 #define SFLOW_DBG(...)	  vlib_log_debug (sflow_logger, __VA_ARGS__);
 #define SFLOW_INFO(...)	  vlib_log_info (sflow_logger, __VA_ARGS__);
diff --git a/src/plugins/sflow/sflow_dlapi.h b/src/plugins/sflow/sflow_dlapi.h
new file mode 100644
index 00000000000..e983bc8f6fe
--- /dev/null
+++ b/src/plugins/sflow/sflow_dlapi.h
@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2025 InMon Corp.
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef __included_sflow_dlapi_h__
+#define __included_sflow_dlapi_h__
+/* Dynamic-link API
+ * If present, linux-cp plugin will be queried to learn the
+ * Linux if_index for each VPP if_index. If that plugin is not
+ * compiled and loaded, or if the function symbol is not found,
+ * then the interfaces will be reported to NETLINK_USERSOCK
+ * without this extra mapping.
+ */
+#define SFLOW_LCP_LIB		     "linux_cp_plugin.so"
+#define SFLOW_LCP_SYM_GET_VIF_BY_PHY "lcp_itf_pair_get_vif_index_by_phy"
+#endif /* __included_sflow_dyn_api_h__ */
+/*
+ * fd.io coding-style-patch-verification: ON
+ *
+ * Local Variables:
+ * eval: (c-set-style "gnu")
+ * End:
+ */
diff --git a/src/plugins/sflow/sflow_psample.c b/src/plugins/sflow/sflow_psample.c
index 0e4fcfbe790..41df454d999 100644
--- a/src/plugins/sflow/sflow_psample.c
+++ b/src/plugins/sflow/sflow_psample.c
@@ -13,11 +13,6 @@
  * limitations under the License.
  */
 
-#if defined(__cplusplus)
-extern "C"
-{
-#endif
-
 #include <vlib/vlib.h>
 #include <vnet/vnet.h>
 #include <vnet/pg/pg.h>
diff --git a/src/plugins/sflow/sflow_vapi.c b/src/plugins/sflow/sflow_vapi.c
deleted file mode 100644
index cdc89a54c80..00000000000
--- a/src/plugins/sflow/sflow_vapi.c
+++ /dev/null
@@ -1,226 +0,0 @@
-/*
- * Copyright (c) 2024 InMon Corp.
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at:
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <sflow/sflow_vapi.h>
-
-#ifdef SFLOW_USE_VAPI
-
-#include <vlibapi/api.h>
-#include <vlibmemory/api.h>
-#include <vpp/app/version.h>
-#include <stdbool.h>
-
-#include <vapi/vapi.h>
-#include <vapi/memclnt.api.vapi.h>
-#include <vapi/vlib.api.vapi.h>
-
-#ifdef included_interface_types_api_types_h
-#define defined_vapi_enum_if_status_flags
-#define defined_vapi_enum_mtu_proto
-#define defined_vapi_enum_link_duplex
-#define defined_vapi_enum_sub_if_flags
-#define defined_vapi_enum_rx_mode
-#define defined_vapi_enum_if_type
-#define defined_vapi_enum_direction
-#endif
-#include <vapi/lcp.api.vapi.h>
-
-DEFINE_VAPI_MSG_IDS_LCP_API_JSON;
-
-static vapi_error_e
-my_pair_get_cb (struct vapi_ctx_s *ctx, void *callback_ctx, vapi_error_e rv,
-		bool is_last, vapi_payload_lcp_itf_pair_get_v2_reply *reply)
-{
-  // this is a no-op, but it seems like it's presence is still required.  For
-  // example, it is called if the pair lookup does not find anything.
-  return VAPI_OK;
-}
-
-static vapi_error_e
-my_pair_details_cb (struct vapi_ctx_s *ctx, void *callback_ctx,
-		    vapi_error_e rv, bool is_last,
-		    vapi_payload_lcp_itf_pair_details *details)
-{
-  sflow_per_interface_data_t *sfif =
-    (sflow_per_interface_data_t *) callback_ctx;
-  // Setting this here will mean it is sent to hsflowd with the interface
-  // counters.
-  sfif->linux_if_index = details->vif_index;
-  return VAPI_OK;
-}
-
-static vapi_error_e
-sflow_vapi_connect (sflow_vapi_client_t *vac)
-{
-  vapi_error_e rv = VAPI_OK;
-  vapi_ctx_t ctx = vac->vapi_ctx;
-  if (ctx == NULL)
-    {
-      // first time - open and connect.
-      if ((rv = vapi_ctx_alloc (&ctx)) != VAPI_OK)
-	{
-	  SFLOW_ERR ("vap_ctx_alloc() returned %d", rv);
-	}
-      else
-	{
-	  vac->vapi_ctx = ctx;
-	  if ((rv = vapi_connect_from_vpp (
-		 ctx, "api_from_sflow_plugin", SFLOW_VAPI_MAX_REQUEST_Q,
-		 SFLOW_VAPI_MAX_RESPONSE_Q, VAPI_MODE_BLOCKING, true)) !=
-	      VAPI_OK)
-	    {
-	      SFLOW_ERR ("vapi_connect_from_vpp() returned %d", rv);
-	    }
-	  else
-	    {
-	      // Connected - but is there a handler for the request we want to
-	      // send?
-	      if (!vapi_is_msg_available (ctx,
-					  vapi_msg_id_lcp_itf_pair_add_del_v2))
-		{
-		  SFLOW_WARN ("vapi_is_msg_available() returned false => "
-			      "linux-cp plugin not loaded");
-		  rv = VAPI_EUSER;
-		}
-	    }
-	}
-    }
-  return rv;
-}
-
-// in forked thread
-static void *
-get_lcp_itf_pairs (void *magic)
-{
-  sflow_vapi_client_t *vac = magic;
-  vapi_error_e rv = VAPI_OK;
-
-  sflow_per_interface_data_t *intfs = vac->vapi_itfs;
-  vlib_set_thread_name (SFLOW_VAPI_THREAD_NAME);
-  if ((rv = sflow_vapi_connect (vac)) != VAPI_OK)
-    {
-      vac->vapi_unavailable = true;
-    }
-  else
-    {
-      vapi_ctx_t ctx = vac->vapi_ctx;
-
-      for (int ii = 1; ii < vec_len (intfs); ii++)
-	{
-	  sflow_per_interface_data_t *sfif = vec_elt_at_index (intfs, ii);
-	  if (sfif && sfif->sflow_enabled)
-	    {
-	      // TODO: if we try non-blocking we might not be able to just pour
-	      // all the requests in here. Might be better to do them one at a
-	      // time - e.g. when we poll for counters.
-	      vapi_msg_lcp_itf_pair_get_v2 *msg =
-		vapi_alloc_lcp_itf_pair_get_v2 (ctx);
-	      if (msg)
-		{
-		  msg->payload.sw_if_index = sfif->sw_if_index;
-		  if ((rv = vapi_lcp_itf_pair_get_v2 (ctx, msg, my_pair_get_cb,
-						      sfif, my_pair_details_cb,
-						      sfif)) != VAPI_OK)
-		    {
-		      SFLOW_ERR ("vapi_lcp_itf_pair_get_v2 returned %d", rv);
-		      // vapi.h: "message must be freed by vapi_msg_free if not
-		      // consumed by vapi_send"
-		      vapi_msg_free (ctx, msg);
-		    }
-		}
-	    }
-	}
-      // We no longer disconnect or free the client structures
-      // vapi_disconnect_from_vpp (ctx);
-      // vapi_ctx_free (ctx);
-    }
-  // indicate that we are done - more portable that using pthread_tryjoin_np()
-  vac->vapi_request_status = (int) rv;
-  clib_atomic_store_rel_n (&vac->vapi_request_active, false);
-  // TODO: how to tell if heap-allocated data is stored separately per thread?
-  // And if so, how to tell the allocator to GC all data for the thread when it
-  // exits?
-  return (void *) rv;
-}
-
-int
-sflow_vapi_read_linux_if_index_numbers (sflow_vapi_client_t *vac,
-					sflow_per_interface_data_t *itfs)
-{
-
-#ifdef SFLOW_VAPI_TEST_PLUGIN_SYMBOL
-  // don't even fork the query thread if the symbol is not there
-  if (!vlib_get_plugin_symbol ("linux_cp_plugin.so", "lcp_itf_pair_get"))
-    {
-      return false;
-    }
-#endif
-  // previous query is done and results extracted?
-  int req_active = clib_atomic_load_acq_n (&vac->vapi_request_active);
-  if (req_active == false && vac->vapi_itfs == NULL)
-    {
-      // make a copy of the current interfaces vector for the lookup thread to
-      // write into
-      vac->vapi_itfs = vec_dup (itfs);
-      pthread_attr_t attr;
-      pthread_attr_init (&attr);
-      pthread_attr_setdetachstate (&attr, PTHREAD_CREATE_DETACHED);
-      pthread_attr_setstacksize (&attr, VLIB_THREAD_STACK_SIZE);
-      vac->vapi_request_active = true;
-      pthread_create (&vac->vapi_thread, &attr, get_lcp_itf_pairs, vac);
-      pthread_attr_destroy (&attr);
-      return true;
-    }
-  return false;
-}
-
-int
-sflow_vapi_check_for_linux_if_index_results (sflow_vapi_client_t *vac,
-					     sflow_per_interface_data_t *itfs)
-{
-  // request completed?
-  // TODO: if we use non-blocking mode do we have to call something here to
-  // receive results?
-  int req_active = clib_atomic_load_acq_n (&vac->vapi_request_active);
-  if (req_active == false && vac->vapi_itfs != NULL)
-    {
-      // yes, extract what we learned
-      // TODO: would not have to do this if vector were array of pointers
-      // to sflow_per_interface_data_t rather than an actual array, but
-      // it does mean we have very clear separation between the threads.
-      for (int ii = 1; ii < vec_len (vac->vapi_itfs); ii++)
-	{
-	  sflow_per_interface_data_t *sfif1 =
-	    vec_elt_at_index (vac->vapi_itfs, ii);
-	  sflow_per_interface_data_t *sfif2 = vec_elt_at_index (itfs, ii);
-	  if (sfif1 && sfif2 && sfif1->sflow_enabled && sfif2->sflow_enabled)
-	    sfif2->linux_if_index = sfif1->linux_if_index;
-	}
-      vec_free (vac->vapi_itfs);
-      vac->vapi_itfs = NULL;
-      return true;
-    }
-  return false;
-}
-
-#endif /* SFLOW_USE_VAPI */
-
-/*
- * fd.io coding-style-patch-verification: ON
- *
- * Local Variables:
- * eval: (c-set-style "gnu")
- * End:
- */
diff --git a/src/plugins/sflow/sflow_vapi.h b/src/plugins/sflow/sflow_vapi.h
deleted file mode 100644
index 640fe997684..00000000000
--- a/src/plugins/sflow/sflow_vapi.h
+++ /dev/null
@@ -1,55 +0,0 @@
-/*
- * Copyright (c) 2024 InMon Corp.
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at:
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-#ifndef __included_sflow_vapi_h__
-#define __included_sflow_vapi_h__
-
-#include <vnet/vnet.h>
-#include <sflow/sflow_common.h>
-
-#ifdef SFLOW_USE_VAPI
-
-#define SFLOW_VAPI_POLL_INTERVAL  5
-#define SFLOW_VAPI_MAX_REQUEST_Q  8
-#define SFLOW_VAPI_MAX_RESPONSE_Q 16
-#define SFLOW_VAPI_THREAD_NAME	  "sflow_vapi" // must be <= 15 characters
-
-// #define SFLOW_VAPI_TEST_PLUGIN_SYMBOL
-
-typedef struct
-{
-  volatile int vapi_request_active; // to sync main <-> vapi_thread
-  pthread_t vapi_thread;
-  sflow_per_interface_data_t *vapi_itfs;
-  int vapi_unavailable;
-  int vapi_request_status; // written by vapi_thread
-  void *vapi_ctx;
-} sflow_vapi_client_t;
-
-int sflow_vapi_read_linux_if_index_numbers (sflow_vapi_client_t *vac,
-					    sflow_per_interface_data_t *itfs);
-int
-sflow_vapi_check_for_linux_if_index_results (sflow_vapi_client_t *vac,
-					     sflow_per_interface_data_t *itfs);
-
-#endif /* SFLOW_USE_VAPI */
-#endif /* __included_sflow_vapi_h__ */
-
-/*
- * fd.io coding-style-patch-verification: ON
- *
- * Local Variables:
- * eval: (c-set-style "gnu")
- * End:
- */