pcap: 32bit counters can wrap-around

Fixes issue 2845.

pcap_stats is based on 32bit counters and given a big enough throughput
will overflow them. This was reported by people using Myricom cards which
should only be a happenstance. The problem exists for all pcap-based
interfaces.

Let's use internal 64bit counters that drag along the pcap_stats and
handle pcap_stats wrap-around as we update the 64bit stats "often enough"
before the pcap_stats can wrap around twice.

diff --git a/src/source-pcap.c b/src/source-pcap.c
index 06b420579411e95998fdefb8537a1df7d66a89e3..7ee7be502ba4fa129032c379bac82aa2ffea01f1 100644 (file)
--- a/src/source-pcap.c
+++ b/src/source-pcap.c
@@ -47,6 +47,22 @@
 
 #define PCAP_RECONNECT_TIMEOUT 500000
 
+/**
+ * \brief 64bit pcap stats counters.
+ *
+ * libpcap only supports 32bit counters. They will eventually wrap around.
+ *
+ * Keep track of libpcap counters as 64bit counters to keep on counting even
+ * if libpcap's 32bit counters wrap around.
+ * Requires pcap_stats() to be called before 32bit stats wrap around twice,
+ * which we do.
+ */
+typedef struct PcapStats64_ {
+    uint64_t ps_recv;
+    uint64_t ps_drop;
+    uint64_t ps_ifdrop;
+} PcapStats64;
+
 /**
  * \brief Structure to hold thread specific variables.
  */
@@ -67,9 +83,8 @@ typedef struct PcapThreadVars_
     int datalink;
 
     /* counters */
-    uint32_t pkts;
+    uint64_t pkts;
     uint64_t bytes;
-    uint32_t errs;
 
     uint16_t capture_kernel_packets;
     uint16_t capture_kernel_drops;
@@ -88,6 +103,8 @@ typedef struct PcapThreadVars_
     ChecksumValidationMode checksum_mode;
 
     LiveDevice *livedev;
+
+    PcapStats64 last_stats64;
 } PcapThreadVars;
 
 static TmEcode ReceivePcapThreadInit(ThreadVars *, const void *, void **);
@@ -99,12 +116,14 @@ static TmEcode DecodePcapThreadInit(ThreadVars *, const void *, void **);
 static TmEcode DecodePcapThreadDeinit(ThreadVars *tv, void *data);
 static TmEcode DecodePcap(ThreadVars *, Packet *, void *);
 
+void SourcePcapRegisterTests(void);
+
 /** protect pcap_compile and pcap_setfilter, as they are not thread safe:
  *  http://seclists.org/tcpdump/2009/q1/62 */
 static SCMutex pcap_bpf_compile_lock = SCMUTEX_INITIALIZER;
 
 /**
- * \brief Registration Function for RecievePcap.
+ * \brief Registration Function for ReceivePcap.
  */
 void TmModuleReceivePcapRegister (void)
 {
@@ -115,6 +134,7 @@ void TmModuleReceivePcapRegister (void)
     tmm_modules[TMM_RECEIVEPCAP].ThreadExitPrintStats = ReceivePcapThreadExitStats;
     tmm_modules[TMM_RECEIVEPCAP].cap_flags = SC_CAP_NET_RAW;
     tmm_modules[TMM_RECEIVEPCAP].flags = TM_FLAG_RECEIVE_TM;
+    tmm_modules[TMM_RECEIVEPCAP].RegisterTests = SourcePcapRegisterTests;
 }
 
 /**
@@ -129,14 +149,52 @@ void TmModuleDecodePcapRegister (void)
     tmm_modules[TMM_DECODEPCAP].flags = TM_FLAG_DECODE_TM;
 }
 
+/**
+ * \brief Update 64 bit |last| value from |current32| value taking one
+ * wrap-around into account.
+ */
+static inline void UpdatePcapStatsValue64(uint64_t *last, uint32_t current32)
+{
+    /* uint64_t -> uint32_t is defined behaviour. It slices lower 32bits. */
+    uint32_t last32 = *last;
+
+    /* Branchless code as wrap-around is defined for unsigned */
+    *last += (uint32_t)(current32 - last32);
+
+    /* Same calculation as:
+    if (likely(current32 >= last32)) {
+        *last += current32 - last32;
+    } else {
+        *last += (1ull << 32) + current32 - last32;
+    }
+    */
+}
+
+/**
+ * \brief Update 64 bit |last| stat values with values from |current|
+ * 32 bit pcap_stat.
+ */
+static inline void UpdatePcapStats64(
+        PcapStats64 *last, const struct pcap_stat *current)
+{
+    UpdatePcapStatsValue64(&last->ps_recv, current->ps_recv);
+    UpdatePcapStatsValue64(&last->ps_drop, current->ps_drop);
+    UpdatePcapStatsValue64(&last->ps_ifdrop, current->ps_ifdrop);
+}
+
 static inline void PcapDumpCounters(PcapThreadVars *ptv)
 {
     struct pcap_stat pcap_s;
     if (likely((pcap_stats(ptv->pcap_handle, &pcap_s) >= 0))) {
-        StatsSetUI64(ptv->tv, ptv->capture_kernel_packets, pcap_s.ps_recv);
-        StatsSetUI64(ptv->tv, ptv->capture_kernel_drops, pcap_s.ps_drop);
-        (void) SC_ATOMIC_SET(ptv->livedev->drop, pcap_s.ps_drop);
-        StatsSetUI64(ptv->tv, ptv->capture_kernel_ifdrops, pcap_s.ps_ifdrop);
+        UpdatePcapStats64(&ptv->last_stats64, &pcap_s);
+
+        StatsSetUI64(ptv->tv, ptv->capture_kernel_packets,
+                ptv->last_stats64.ps_recv);
+        StatsSetUI64(
+                ptv->tv, ptv->capture_kernel_drops, ptv->last_stats64.ps_drop);
+        (void)SC_ATOMIC_SET(ptv->livedev->drop, ptv->last_stats64.ps_drop);
+        StatsSetUI64(ptv->tv, ptv->capture_kernel_ifdrops,
+                ptv->last_stats64.ps_ifdrop);
     }
 }
 
@@ -506,11 +564,11 @@ static void ReceivePcapThreadExitStats(ThreadVars *tv, void *data)
     if (pcap_stats(ptv->pcap_handle, &pcap_s) < 0) {
         SCLogError(SC_ERR_STAT,"(%s) Failed to get pcap_stats: %s",
                 tv->name, pcap_geterr(ptv->pcap_handle));
-        SCLogInfo("(%s) Packets %" PRIu32 ", bytes %" PRIu64 "",
-                tv->name, ptv->pkts, ptv->bytes);
+        SCLogInfo("(%s) Packets %" PRIu64 ", bytes %" PRIu64 "", tv->name,
+                ptv->pkts, ptv->bytes);
     } else {
-        SCLogInfo("(%s) Packets %" PRIu32 ", bytes %" PRIu64 "",
-                tv->name, ptv->pkts, ptv->bytes);
+        SCLogInfo("(%s) Packets %" PRIu64 ", bytes %" PRIu64 "", tv->name,
+                ptv->pkts, ptv->bytes);
 
         /* these numbers are not entirely accurate as ps_recv contains packets
          * that are still waiting to be processed at exit. ps_drop only contains
@@ -520,11 +578,18 @@ static void ReceivePcapThreadExitStats(ThreadVars *tv, void *data)
          * Note: ps_recv includes dropped packets and should be considered total.
          * Unless we start to look at ps_ifdrop which isn't supported everywhere.
          */
-        SCLogInfo("(%s) Pcap Total:%" PRIu64 " Recv:%" PRIu64 " Drop:%" PRIu64 " (%02.1f%%).",
-                tv->name, (uint64_t)pcap_s.ps_recv,
-                (uint64_t)pcap_s.ps_recv - (uint64_t)pcap_s.ps_drop,
-                (uint64_t)pcap_s.ps_drop,
-                (((float)(uint64_t)pcap_s.ps_drop)/(float)(uint64_t)pcap_s.ps_recv)*100);
+        UpdatePcapStats64(&ptv->last_stats64, &pcap_s);
+        float drop_percent =
+                likely(ptv->last_stats64.ps_recv > 0)
+                        ? (((float)ptv->last_stats64.ps_drop) /
+                                  (float)ptv->last_stats64.ps_recv) *
+                                  100
+                        : 0;
+        SCLogInfo("(%s) Pcap Total:%" PRIu64 " Recv:%" PRIu64 " Drop:%" PRIu64
+                  " (%02.1f%%).",
+                tv->name, ptv->last_stats64.ps_recv,
+                ptv->last_stats64.ps_recv - ptv->last_stats64.ps_drop,
+                ptv->last_stats64.ps_drop, drop_percent);
     }
 }
 
@@ -639,3 +704,21 @@ void PcapTranslateIPToDevice(char *pcap_dev, size_t len)
 
     pcap_freealldevs(alldevsp);
 }
+
+/*
+ *  unittests
+ */
+
+#ifdef UNITTESTS
+#include "tests/source-pcap.c"
+#endif /* UNITTESTS */
+
+/**
+ *  \brief  Register the Unit tests for pcap source
+ */
+void SourcePcapRegisterTests(void)
+{
+#ifdef UNITTESTS
+    SourcePcapRegisterStatsTests();
+#endif /* UNITTESTS */
+}

diff --git a/src/tests/source-pcap.c b/src/tests/source-pcap.c
new file mode 100644 (file)
index 0000000..fc1b275
--- /dev/null
+++ b/src/tests/source-pcap.c
@@ -0,0 +1,232 @@
+/* Copyright (C) 2020 Open Information Security Foundation
+ *
+ * You can copy, redistribute or modify this Program under the terms of
+ * the GNU General Public License version 2 as published by the Free
+ * Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 2 along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA.
+ */
+
+#include "../suricata-common.h"
+#include "../util-unittest.h"
+
+static uint32_t Upper32(uint64_t value)
+{
+    /* uint64_t -> uint32_t is defined behaviour. It slices lower 32bits. */
+    return value >> 32;
+}
+static uint32_t Lower32(uint64_t value)
+{
+    /* uint64_t -> uint32_t is defined behaviour. It slices lower 32bits. */
+    return value;
+}
+
+/* Structured test data to make it easier on my eyes */
+typedef struct TestData_ {
+    uint64_t last; /* internal 64bit counter to drag along */
+    u_int current; /* 32bit pcap stat */
+} TestData;
+
+static int UpdatePcapStatsValue64NoChange01(void)
+{
+    /*
+     * No change in counter values.
+     * Last count is within first 32bit range, i.e. same as pcap_stat range.
+     */
+    TestData data[] = {{.last = 0, .current = 0},
+            {.last = 12345, .current = 12345},
+            {.last = (uint64_t)UINT32_MAX, .current = UINT_MAX}};
+
+    for (size_t i = 0; i < ARRAY_SIZE(data); ++i) {
+        FAIL_IF_NOT(data[i].last == data[i].current);
+
+        UpdatePcapStatsValue64(&data[i].last, data[i].current);
+        FAIL_IF_NOT(data[i].last == data[i].current);
+    }
+
+    PASS;
+}
+
+static int UpdatePcapStatsValue64NoChange02(void)
+{
+    /*
+     * No change in counter values.
+     * Last count is outside 32bits range.
+     */
+    TestData data[] = {{.last = (2ull << 32) + 0, .current = 0},
+            {.last = (3ull << 32) + 12345, .current = 12345},
+            {.last = (3ull << 32) + (uint64_t)UINT32_MAX, .current = UINT_MAX},
+            {.last = UINT64_MAX, .current = UINT_MAX}};
+
+    for (size_t i = 0; i < ARRAY_SIZE(data); ++i) {
+        uint32_t upper = Upper32(data[i].last);
+        FAIL_IF_NOT(Lower32(data[i].last) == data[i].current);
+
+        UpdatePcapStatsValue64(&data[i].last, data[i].current);
+        FAIL_IF_NOT(Lower32(data[i].last) == data[i].current);
+        FAIL_IF_NOT(Upper32(data[i].last) == upper);
+    }
+
+    PASS;
+}
+
+static int UpdatePcapStatsValue64NoOverflow01(void)
+{
+    /*
+     * Non-overflowing counter value is simply taken over in lower 32bits.
+     * Last count is within first 32bit range, i.e. same as pcap_stat range.
+     * Also test edges and simple +1.
+     */
+    TestData data[] = {{.last = 0, .current = 1},
+            {.last = 12345, .current = 34567},
+            {.last = (uint64_t)UINT32_MAX - 1, .current = UINT_MAX}};
+
+    for (size_t i = 0; i < ARRAY_SIZE(data); ++i) {
+        FAIL_IF_NOT(data[i].last < data[i].current);
+
+        UpdatePcapStatsValue64(&data[i].last, data[i].current);
+        FAIL_IF_NOT(data[i].last == data[i].current);
+    }
+
+    PASS;
+}
+
+static int UpdatePcapStatsValue64NoOverflow02(void)
+{
+    /*
+     * Non-overflowing counter value is simply taken over in lower 32bits.
+     * Last count is outside 32bits range.
+     */
+    TestData data[] = {{.last = (2ull << 32) + 0, .current = 1},
+            {.last = (3ull << 32) + 12345, .current = 34567},
+            {.last = UINT64_MAX - 1, .current = UINT_MAX}};
+
+    for (size_t i = 0; i < ARRAY_SIZE(data); ++i) {
+        uint32_t upper = Upper32(data[i].last);
+        FAIL_IF_NOT(Lower32(data[i].last) < data[i].current);
+
+        UpdatePcapStatsValue64(&data[i].last, data[i].current);
+        FAIL_IF_NOT(Lower32(data[i].last) == data[i].current);
+        FAIL_IF_NOT(Upper32(data[i].last) == upper);
+    }
+
+    PASS;
+}
+
+static int UpdatePcapStatsValue64Overflow01(void)
+{
+    /*
+     * Overflowing counter value is simply taken over in lower 32bits.
+     * Last count is within first 32bit range, i.e. same as pcap_stat range.
+     */
+    TestData data[] = {{.last = 1, .current = 0},
+            {.last = 12345, .current = 22}, {.last = 12345, .current = 12344},
+            {.last = (uint64_t)UINT32_MAX, .current = UINT_MAX - 1}};
+
+    for (size_t i = 0; i < ARRAY_SIZE(data); ++i) {
+        FAIL_IF_NOT(data[i].last > data[i].current);
+
+        UpdatePcapStatsValue64(&data[i].last, data[i].current);
+        FAIL_IF_NOT(Lower32(data[i].last) == data[i].current);
+        FAIL_IF_NOT(Upper32(data[i].last) == 1); /* wrap around */
+    }
+
+    PASS;
+}
+
+static int UpdatePcapStatsValue64Overflow02(void)
+{
+    /*
+     * Overflowing counter value is simply taken over in lower 32bits.
+     * Last count is outside 32bits range.
+     */
+    TestData data[] = {{.last = (2ull << 32) + 1, .current = 0},
+            {.last = (3ull << 32) + 12345, .current = 22},
+            {.last = (3ull << 32) + 12345, .current = 12344},
+            {.last = UINT64_MAX, .current = UINT_MAX - 1}};
+
+    for (size_t i = 0; i < ARRAY_SIZE(data); ++i) {
+        uint32_t upper = Upper32(data[i].last);
+        FAIL_IF_NOT(Lower32(data[i].last) > data[i].current);
+
+        UpdatePcapStatsValue64(&data[i].last, data[i].current);
+        FAIL_IF_NOT(Lower32(data[i].last) == data[i].current);
+        FAIL_IF_NOT(Upper32(data[i].last) == upper + 1); /* wrap around */
+    }
+
+    PASS;
+}
+
+static int UpdatePcapStatsValue64Overflow03(void)
+{
+    /*
+     * Overflowing counter value is simply taken over in lower 32bits.
+     * Edge cases where upper32 bit wrap around to 0.
+     */
+    TestData data[] = {{.last = UINT64_MAX, .current = 0},
+            {.last = UINT64_MAX, .current = 3333}};
+
+    for (size_t i = 0; i < ARRAY_SIZE(data); ++i) {
+        FAIL_IF_NOT(Lower32(data[i].last) > data[i].current);
+
+        UpdatePcapStatsValue64(&data[i].last, data[i].current);
+        FAIL_IF_NOT(Lower32(data[i].last) == data[i].current);
+        FAIL_IF_NOT(Upper32(data[i].last) == 0); /* wrap around */
+    }
+
+    PASS;
+}
+
+static int UpdatePcapStats64Assorted01(void)
+{
+    /*
+     * Test that all fields of the struct are correctly updated.
+     *
+     * Full testing of value behaviour is done in UpdatePcapStatsValue64...()
+     * tests.
+     */
+    PcapStats64 last = {.ps_recv = 0, .ps_drop = 1234, .ps_ifdrop = 8765};
+    struct pcap_stat current = {
+            .ps_recv = 12, .ps_drop = 2345, .ps_ifdrop = 9876};
+
+    // test setup sanity check
+    FAIL_IF_NOT(last.ps_recv < current.ps_recv);
+    FAIL_IF_NOT(last.ps_drop < current.ps_drop);
+    FAIL_IF_NOT(last.ps_ifdrop < current.ps_ifdrop);
+
+    UpdatePcapStats64(&last, &current);
+
+    FAIL_IF_NOT(last.ps_recv == current.ps_recv);
+    FAIL_IF_NOT(last.ps_drop == current.ps_drop);
+    FAIL_IF_NOT(last.ps_ifdrop == current.ps_ifdrop);
+
+    PASS;
+}
+
+static void SourcePcapRegisterStatsTests(void)
+{
+    UtRegisterTest("UpdatePcapStatsValue64NoChange01",
+            UpdatePcapStatsValue64NoChange01);
+    UtRegisterTest("UpdatePcapStatsValue64NoChange02",
+            UpdatePcapStatsValue64NoChange02);
+    UtRegisterTest("UpdatePcapStatsValue64NoOverflow01",
+            UpdatePcapStatsValue64NoOverflow01);
+    UtRegisterTest("UpdatePcapStatsValue64NoOverflow02",
+            UpdatePcapStatsValue64NoOverflow02);
+    UtRegisterTest("UpdatePcapStatsValue64Overflow01",
+            UpdatePcapStatsValue64Overflow01);
+    UtRegisterTest("UpdatePcapStatsValue64Overflow02",
+            UpdatePcapStatsValue64Overflow02);
+    UtRegisterTest("UpdatePcapStatsValue64Overflow03",
+            UpdatePcapStatsValue64Overflow03);
+
+    UtRegisterTest("UpdatePcapStats64Assorted01", UpdatePcapStats64Assorted01);
+}