lib/packet: add force-expensive configure option

This option forces expensive Packet operations to reallocate by taking a clone prior to checking shared and then killing the clone later. This is controlled by --enable-force-expensive / CLICK_FORCE_EXPENSIVE and allows one to debug issues where a caller assumes that the returned SKB is the same as the original SKB. Signed-off-by: Derrick Pallas <[email protected]>
kohler · Nov 10, 2018 · 0cddd61 · 0cddd61
1 parent 20319b7
commit 0cddd61
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 1 deletion.
diff --git a/config.h.in b/config.h.in
@@ -20,6 +20,9 @@
 /* Define for Click memory allocation debugging. */
 #undef CLICK_DMALLOC
 
+/* Define for forcing expensive packet operations. */
+#undef CLICK_FORCE_EXPENSIVE
+
 /* Define to generate smaller object files. */
 #undef CLICK_OPTIMIZE_SIZE
 

diff --git a/configure b/configure
@@ -849,6 +849,7 @@ enable_stats
 enable_stride
 enable_task_heap
 enable_dmalloc
+enable_force_expensive
 enable_valgrind
 enable_schedule_debugging
 enable_intel_cpu
@@ -1540,6 +1541,8 @@ Optional Features:
   --disable-stride        disable stride scheduler
   --enable-task-heap      use heap for task list
   --enable-dmalloc        enable debugging malloc
+  --enable-force-expensive
+                          force expensive packet operations
   --enable-valgrind       extra support for debugging with valgrind
   --enable-schedule-debugging[=WHAT] enable Click scheduler debugging
                           (no/yes/extra) [yes]
@@ -10267,6 +10270,21 @@ fi
 
 
 
+# Check whether --enable-force-expensive was given.
+if test "${enable_force_expensive+set}" = set; then :
+  enableval=$enable_force_expensive; :
+else
+  enable_force_expensive=no
+fi
+
+if test $enable_force_expensive = yes; then
+
+$as_echo "#define CLICK_FORCE_EXPENSIVE 1" >>confdefs.h
+
+fi
+
+
+
 # Check whether --enable-valgrind was given.
 if test "${enable_valgrind+set}" = set; then :
   enableval=$enable_valgrind; :

diff --git a/configure.in b/configure.in
@@ -1087,6 +1087,14 @@ if test $enable_dmalloc = yes; then
 fi
 
 
+dnl debug by forcing expensive operations
+
+AC_ARG_ENABLE([force-expensive], [AS_HELP_STRING([--enable-force-expensive], [force expensive packet operations])], :, enable_force_expensive=no)
+if test $enable_force_expensive = yes; then
+    AC_DEFINE([CLICK_FORCE_EXPENSIVE], [1], [Define for forcing expensive packet operations])
+fi
+
+
 dnl valgrind debugging support
 
 AC_ARG_ENABLE([valgrind], [AS_HELP_STRING([--enable-valgrind], [extra support for debugging with valgrind])], :, enable_valgrind=no)

diff --git a/include/click/packet.hh b/include/click/packet.hh
@@ -1432,7 +1432,7 @@ Packet::kill()
     b->list = 0;
 # endif
     skbmgr_recycle_skbs(b);
-#elif HAVE_CLICK_PACKET_POOL
+#elif HAVE_CLICK_PACKET_POOL && !defined(CLICK_FORCE_EXPENSIVE)
     if (_use_count.dec_and_test())
 	WritablePacket::recycle(static_cast<WritablePacket *>(this));
 #else
@@ -1536,6 +1536,19 @@ Packet::shared() const
 #endif
 }
 
+class PacketRef {
+public:
+    PacketRef(Packet* p) : _p(p->clone()) { }
+    ~PacketRef() { if (_p) _p->kill(); }
+    Packet* release() {
+        Packet* tmp = _p;
+        _p = NULL;
+        return tmp;
+    }
+private:
+    Packet* _p;
+};
+
 /** @brief Return an unshared packet containing this packet's data.
  * @return the unshared packet, which is writable
  *
@@ -1562,6 +1575,9 @@ Packet::shared() const
 inline WritablePacket *
 Packet::uniqueify()
 {
+#ifdef CLICK_FORCE_EXPENSIVE
+    PacketRef r(this);
+#endif
     if (!shared())
 	return static_cast<WritablePacket *>(this);
     else
@@ -1571,6 +1587,9 @@ Packet::uniqueify()
 inline WritablePacket *
 Packet::push(uint32_t len)
 {
+#ifdef CLICK_FORCE_EXPENSIVE
+    PacketRef r(this);
+#endif
     if (headroom() >= len && !shared()) {
 	WritablePacket *q = (WritablePacket *)this;
 #if CLICK_LINUXMODULE	/* Linux kernel module */
@@ -1629,6 +1648,9 @@ Packet::pull(uint32_t len)
 inline WritablePacket *
 Packet::put(uint32_t len)
 {
+#ifdef CLICK_FORCE_EXPENSIVE
+    PacketRef r(this);
+#endif
     if (tailroom() >= len && !shared()) {
 	WritablePacket *q = (WritablePacket *)this;
 #if CLICK_LINUXMODULE	/* Linux kernel module */
@@ -1830,6 +1852,9 @@ Packet::clear_mac_header()
 inline WritablePacket *
 Packet::push_mac_header(uint32_t len)
 {
+#ifdef CLICK_FORCE_EXPENSIVE
+    PacketRef r(this);
+#endif
     WritablePacket *q;
     if (headroom() >= len && !shared()) {
 	q = (WritablePacket *)this;

diff --git a/lib/packet.cc b/lib/packet.cc
@@ -888,6 +888,9 @@ Packet::expensive_put(uint32_t nbytes)
 Packet *
 Packet::shift_data(int offset, bool free_on_failure)
 {
+#ifdef CLICK_FORCE_EXPENSIVE
+    PacketRef r(this);
+#endif
     if (offset == 0)
 	return this;