From: Dave Wallace <dwallacelf@gmail.com>
Date: Mon, 3 Jul 2017 17:11:38 +0000 (-0400)
Subject: Fix unlinking of /dev/shm files.
X-Git-Tag: v17.10-rc1~339
X-Git-Url: https://gerrit.fd.io/r/gitweb?p=vpp.git;a=commitdiff_plain;h=d756b35032cdf7fdaaf0d6611388a54d32d72e92

Fix unlinking of /dev/shm files.

- api-segment prefix not used when unlinking shm files
- unlink root region on exit if no clients referenced
- stale reference to freed segment name
- don't add fake client to /db unless CLIB_DEBUG > 2
- turn off the gmond plugin
- clean up unused vars in vpp/api

Change-Id: I66451fcfd6ee64a12466c2d6c209050e3cdb74b7
Signed-off-by: Dave Wallace <dwallacelf@gmail.com>
Signed-off-by: Dave Barach <dave@barachs.net>
---

diff --git a/Makefile b/Makefile
index 0d21f335c5f..46c51dd891e 100644
--- a/Makefile
+++ b/Makefile
@@ -55,8 +55,10 @@ else ifeq ($(filter rhel centos fedora opensuse,$(OS_ID)),$(OS_ID))
 PKG=rpm
 endif
 
+# +libganglia1-dev if building the gmond plugin
+
 DEB_DEPENDS  = curl build-essential autoconf automake bison libssl-dev ccache
-DEB_DEPENDS += debhelper dkms git libtool libganglia1-dev libapr1-dev dh-systemd
+DEB_DEPENDS += debhelper dkms git libtool libapr1-dev dh-systemd
 DEB_DEPENDS += libconfuse-dev git-review exuberant-ctags cscope pkg-config
 DEB_DEPENDS += lcov chrpath autoconf nasm indent
 DEB_DEPENDS += python-all python-dev python-virtualenv python-pip libffi6
@@ -79,9 +81,12 @@ else
 	RPM_DEPENDS += python-virtualenv
 	RPM_DEPENDS_GROUPS = 'Development Tools'
 endif
+
+# +ganglia-devel if building the ganglia plugin
+
 RPM_DEPENDS += chrpath libffi-devel rpm-build
 RPM_DEPENDS += https://kojipkgs.fedoraproject.org//packages/nasm/2.12.02/2.fc26/x86_64/nasm-2.12.02-2.fc26.x86_64.rpm
-EPEL_DEPENDS = libconfuse-devel ganglia-devel epel-rpm-macros
+EPEL_DEPENDS = libconfuse-devel epel-rpm-macros
 ifeq ($(filter rhel centos,$(OS_ID)),$(OS_ID))
 	EPEL_DEPENDS += lcov
 else
diff --git a/build-data/platforms/vpp.mk b/build-data/platforms/vpp.mk
index 4577fa2e8a4..acbe0e7fb13 100644
--- a/build-data/platforms/vpp.mk
+++ b/build-data/platforms/vpp.mk
@@ -36,7 +36,7 @@ vpp_uses_dpdk = yes
 # Uncoment to enable building unit tests
 # vpp_enable_tests = yes
 
-vpp_root_packages = vpp gmod
+vpp_root_packages = vpp
 
 # DPDK configuration parameters
 # vpp_uses_dpdk_cryptodev_sw = yes
diff --git a/src/svm/ssvm.c b/src/svm/ssvm.c
index 6cda1f279e8..23e3cf443c3 100644
--- a/src/svm/ssvm.c
+++ b/src/svm/ssvm.c
@@ -29,6 +29,9 @@ ssvm_master_init (ssvm_private_t * ssvm, u32 master_index)
   if (ssvm->ssvm_size == 0)
     return SSVM_API_ERROR_NO_SIZE;
 
+  if (CLIB_DEBUG > 1)
+    clib_warning ("[%d] creating segment '%s'", getpid (), ssvm->name);
+
   ssvm_filename = format (0, "/dev/shm/%s%c", ssvm->name, 0);
 
   unlink ((char *) ssvm_filename);
@@ -176,12 +179,18 @@ ssvm_delete (ssvm_private_t * ssvm)
 
   fn = format (0, "/dev/shm/%s%c", ssvm->name, 0);
 
+  if (CLIB_DEBUG > 1)
+    clib_warning ("[%d] unlinking ssvm (%s) backing file '%s'", getpid (),
+		  ssvm->name, fn);
+
   /* Throw away the backing file */
   if (unlink ((char *) fn) < 0)
     clib_unix_warning ("unlink segment '%s'", ssvm->name);
 
-  munmap ((void *) ssvm->requested_va, ssvm->ssvm_size);
   vec_free (fn);
+  vec_free (ssvm->name);
+
+  munmap ((void *) ssvm->requested_va, ssvm->ssvm_size);
 }
 
 
diff --git a/src/svm/svm.c b/src/svm/svm.c
index c96135cfd5b..600fa7448ac 100644
--- a/src/svm/svm.c
+++ b/src/svm/svm.c
@@ -458,14 +458,15 @@ svm_map_region (svm_map_region_args_t * a)
   struct stat stat;
   struct timespec ts, tsrem;
 
-  if (CLIB_DEBUG > 1)
-    clib_warning ("[%d] map region %s", getpid (), a->name);
-
   ASSERT ((a->size & ~(MMAP_PAGESIZE - 1)) == a->size);
   ASSERT (a->name);
 
   shm_name = shm_name_from_svm_map_region_args (a);
 
+  if (CLIB_DEBUG > 1)
+    clib_warning ("[%d] map region %s: shm_open (%s)",
+		  getpid (), a->name, shm_name);
+
   svm_fd = shm_open ((char *) shm_name, O_RDWR | O_CREAT | O_EXCL, 0777);
 
   if (svm_fd >= 0)
@@ -947,6 +948,29 @@ svm_region_find_or_create (svm_map_region_args_t * a)
   return (rp);
 }
 
+void
+svm_region_unlink (svm_region_t * rp)
+{
+  svm_map_region_args_t _a, *a = &_a;
+  svm_main_region_t *mp;
+  u8 *shm_name;
+
+  ASSERT (root_rp);
+  ASSERT (rp);
+  ASSERT (vec_c_string_is_terminated (rp->region_name));
+
+  mp = root_rp->data_base;
+  ASSERT (mp);
+
+  a->root_path = (char *) mp->root_path;
+  a->name = rp->region_name;
+  shm_name = shm_name_from_svm_map_region_args (a);
+  if (CLIB_DEBUG > 1)
+    clib_warning ("[%d] shm_unlink (%s)", getpid (), shm_name);
+  shm_unlink ((const char *) shm_name);
+  vec_free (shm_name);
+}
+
 /*
  * svm_region_unmap
  *
@@ -1056,7 +1080,7 @@ found:
       vec_free (name);
 
       region_unlock (rp);
-      shm_unlink (rp->region_name);
+      svm_region_unlink (rp);
       munmap ((void *) virtual_base, virtual_size);
       region_unlock (root_rp);
       svm_pop_heap (oldheap);
@@ -1071,9 +1095,6 @@ found:
 
 /*
  * svm_region_exit
- * There is no clean way to unlink the
- * root region when all clients go away,
- * so remove the pid entry and call it a day.
  */
 void
 svm_region_exit ()
@@ -1116,6 +1137,9 @@ svm_region_exit ()
 
 found:
 
+  if (vec_len (root_rp->client_pids) == 0)
+    svm_region_unlink (root_rp);
+
   region_unlock (root_rp);
   svm_pop_heap (oldheap);
 
diff --git a/src/svm/svm_fifo_segment.c b/src/svm/svm_fifo_segment.c
index 69d4ecb9387..c80374a7ffe 100644
--- a/src/svm/svm_fifo_segment.c
+++ b/src/svm/svm_fifo_segment.c
@@ -105,7 +105,7 @@ svm_fifo_segment_create (svm_fifo_segment_create_args_t * a)
   s->ssvm.ssvm_size = a->segment_size;
   s->ssvm.i_am_master = 1;
   s->ssvm.my_pid = getpid ();
-  s->ssvm.name = (u8 *) a->segment_name;
+  s->ssvm.name = format (0, "%s", a->segment_name);
   s->ssvm.requested_va = sm->next_baseva;
 
   rv = ssvm_master_init (&s->ssvm, s - sm->segments);
diff --git a/src/svm/svmdb.c b/src/svm/svmdb.c
index 03dfe7c33d3..043b0924d84 100644
--- a/src/svm/svmdb.c
+++ b/src/svm/svmdb.c
@@ -106,11 +106,16 @@ svmdb_map (svmdb_map_args_t * dba)
     }
   /* Nope, it's our problem... */
 
-  /* Add a bogus client (pid=0) so the svm won't be deallocated */
-  oldheap = svm_push_pvt_heap (db_rp);
-  vec_add1 (client->db_rp->client_pids, 0);
-  svm_pop_heap (oldheap);
-
+  if (CLIB_DEBUG > 2)
+    {
+      /* Add a bogus client (pid=0) so the svm won't be deallocated */
+      clib_warning
+	("[%d] adding fake client (pid=0) so '%s' won't be unlinked",
+	 getpid (), db_rp->region_name);
+      oldheap = svm_push_pvt_heap (db_rp);
+      vec_add1 (client->db_rp->client_pids, 0);
+      svm_pop_heap (oldheap);
+    }
   oldheap = svm_push_data_heap (db_rp);
 
   vec_validate (hp, 0);
diff --git a/src/vlib/buffer.h b/src/vlib/buffer.h
index c810db4e860..77528e7717e 100644
--- a/src/vlib/buffer.h
+++ b/src/vlib/buffer.h
@@ -87,17 +87,17 @@ typedef struct
 /* any change to the following line requres update of
  * vlib_buffer_get_free_list_index(...) and
  * vlib_buffer_set_free_list_index(...) functions */
-#define VLIB_BUFFER_FREE_LIST_INDEX_MASK ((1 << 4) - 1)
+#define VLIB_BUFFER_FREE_LIST_INDEX_MASK ((1 << 5) - 1)
 
-#define VLIB_BUFFER_IS_TRACED (1 << 4)
-#define VLIB_BUFFER_LOG2_NEXT_PRESENT (5)
+#define VLIB_BUFFER_IS_TRACED (1 << 5)
+#define VLIB_BUFFER_LOG2_NEXT_PRESENT (6)
 #define VLIB_BUFFER_NEXT_PRESENT (1 << VLIB_BUFFER_LOG2_NEXT_PRESENT)
-#define VLIB_BUFFER_IS_RECYCLED (1 << 6)
-#define VLIB_BUFFER_TOTAL_LENGTH_VALID (1 << 7)
-#define VLIB_BUFFER_REPL_FAIL (1 << 8)
-#define VLIB_BUFFER_RECYCLE (1 << 9)
-#define VLIB_BUFFER_FLOW_REPORT (1 << 10)
-#define VLIB_BUFFER_EXT_HDR_VALID (1 << 11)
+#define VLIB_BUFFER_IS_RECYCLED (1 << 7)
+#define VLIB_BUFFER_TOTAL_LENGTH_VALID (1 << 8)
+#define VLIB_BUFFER_REPL_FAIL (1 << 9)
+#define VLIB_BUFFER_RECYCLE (1 << 10)
+#define VLIB_BUFFER_FLOW_REPORT (1 << 11)
+#define VLIB_BUFFER_EXT_HDR_VALID (1 << 12)
 
   /* User defined buffer flags. */
 #define LOG2_VLIB_BUFFER_FLAG_USER(n) (32 - (n))
diff --git a/src/vpp.am b/src/vpp.am
index 614bd26abda..10a4e311252 100644
--- a/src/vpp.am
+++ b/src/vpp.am
@@ -33,11 +33,11 @@ if WITH_APICLI
   vpp/api/plugin.h
 endif
 
-# comment out to disable stats upload to gmond
+# uncomment to enable stats upload to gmond
+# bin_vpp_SOURCES +=				\
+#  vpp/api/gmon.c
 
 bin_vpp_CFLAGS = @APICLI@
-bin_vpp_SOURCES +=				\
-  vpp/api/gmon.c
 
 nobase_include_HEADERS +=			\
   vpp/api/vpe_all_api_h.h			\
diff --git a/src/vpp/api/api.c b/src/vpp/api/api.c
index de9a24779dd..4e892431376 100644
--- a/src/vpp/api/api.c
+++ b/src/vpp/api/api.c
@@ -686,7 +686,6 @@ static void
 
   BAD_SW_IF_INDEX_LABEL;
 
-out:
   REPLY_MACRO (VL_API_PROXY_ARP_INTFC_ENABLE_DISABLE_REPLY);
 }
 
diff --git a/src/vpp/api/api_main.c b/src/vpp/api/api_main.c
index ac09cd15335..c355a5fdff0 100644
--- a/src/vpp/api/api_main.c
+++ b/src/vpp/api/api_main.c
@@ -232,8 +232,6 @@ unformat_sw_if_index (unformat_input_t * input, va_list * args)
   u32 *result = va_arg (*args, u32 *);
   vnet_main_t *vnm = vnet_get_main ();
   u32 sw_if_index = ~0;
-  u8 *if_name;
-  uword *p;
 
   if (unformat (input, "%U", unformat_vnet_sw_interface, vnm, &sw_if_index))
     {
diff --git a/src/vpp/api/custom_dump.c b/src/vpp/api/custom_dump.c
index 3ac8874e852..7f3a58d9523 100644
--- a/src/vpp/api/custom_dump.c
+++ b/src/vpp/api/custom_dump.c
@@ -1174,7 +1174,6 @@ static void *vl_api_sr_policy_mod_t_print
   (vl_api_sr_policy_mod_t * mp, void *handle)
 {
   u8 *s;
-  u32 weight;
 
   ip6_address_t *segments = 0, *seg;
   ip6_address_t *this_address = (ip6_address_t *) mp->segments;
@@ -1216,8 +1215,6 @@ static void *vl_api_sr_policy_del_t_print
   u8 *s;
 
   s = format (0, "SCRIPT: sr_policy_del ");
-  u8 bsid_addr[16];
-  u32 sr_policy_index;
   s = format (s, "To be delivered. Good luck.");
   FINISH;
 }
@@ -2432,7 +2429,7 @@ static void *vl_api_lisp_add_del_remote_mapping_t_print
   (vl_api_lisp_add_del_remote_mapping_t * mp, void *handle)
 {
   u8 *s;
-  u32 i, rloc_num = 0;
+  u32 rloc_num = 0;
 
   s = format (0, "SCRIPT: lisp_add_del_remote_mapping ");
 
@@ -2574,7 +2571,6 @@ static void *vl_api_lisp_add_del_locator_set_t_print
   (vl_api_lisp_add_del_locator_set_t * mp, void *handle)
 {
   u8 *s;
-  u32 loc_num = 0, i;
 
   s = format (0, "SCRIPT: lisp_add_del_locator_set ");
 
@@ -2583,8 +2579,6 @@ static void *vl_api_lisp_add_del_locator_set_t_print
 
   s = format (s, "locator-set %s ", mp->locator_set_name);
 
-  loc_num = clib_net_to_host_u32 (mp->locator_num);
-
   FINISH;
 }
 
diff --git a/src/vpp/stats/stats.c b/src/vpp/stats/stats.c
index 38821da7801..422b7b3b7fe 100644
--- a/src/vpp/stats/stats.c
+++ b/src/vpp/stats/stats.c
@@ -577,7 +577,6 @@ do_ip4_fibs (stats_main_t * sm)
   ip4_route_t *r;
   fib_table_t *fib;
   ip4_fib_t *v4_fib;
-  ip_lookup_main_t *lm = &im4->lookup_main;
   static uword *results;
   vl_api_vnet_ip4_fib_counters_t *mp = 0;
   u32 items_this_message;