pcap/Source: Allow more than 32bit for link and dropped stats

The PktSrc::Stats object works with 64bit unsigned integers. Unfortunately,
libpcap's struct pcap_stat is using 32bit values and users have reported
the wrapping of these values being visible in their stats.log roughly every
7.5 hours (~160kpps).

This change moves tracking of link and drop counters into the PktSrc::Stats
object (like is done for received and bytes_received) and updates them
on a call to PcapSource::Statistics() with the difference to the
previous stats values to prevent the wrap from becoming visible to
script land.

This doesn't cover the case of the stats counters wrapping around multiple
times between two invocations of PktSrc::Statistics(). With the default
interval of 5 minutes for the stats script, this seems acceptable.

Closes #2791.
This commit is contained in:
Arne Welzel 2023-04-27 18:25:56 +02:00
parent 667cdd5c27
commit 5541066660
3 changed files with 90 additions and 2 deletions

3
NEWS
View file

@ -297,6 +297,9 @@ Changed Functionality
masked to exclude PCP and DEI bits. Previously, these bits were included
and could cause invalid vlan values > 4095 to be reported.
- Libpcap based packet source now avoids the 32bit wraparound of link and
dropped packet counters as reported by users.
Removed Functionality
---------------------

View file

@ -4,6 +4,8 @@
#include "zeek/zeek-config.h"
#include "zeek/3rdparty/doctest.h"
#ifdef HAVE_PCAP_INT_H
#include <pcap-int.h>
#endif
@ -326,6 +328,23 @@ bool PcapSource::SetFilter(int index)
return true;
}
// Given two pcap_stat structures, compute the difference of linked and dropped
// and add it to the given Stats object.
static void update_pktsrc_stats(PktSrc::Stats* stats, const struct pcap_stat* now,
const struct pcap_stat* prev)
{
decltype(now->ps_drop) ps_drop_diff = 0;
decltype(now->ps_recv) ps_recv_diff = 0;
// This is subtraction of unsigned ints: It's not undefined
// and results in modulo arithmetic.
ps_recv_diff = now->ps_recv - prev->ps_recv;
ps_drop_diff = now->ps_drop - prev->ps_drop;
stats->link += ps_recv_diff;
stats->dropped += ps_drop_diff;
}
void PcapSource::Statistics(Stats* s)
{
char errbuf[PCAP_ERRBUF_SIZE];
@ -344,11 +363,13 @@ void PcapSource::Statistics(Stats* s)
else
{
s->dropped = pstat.ps_drop;
s->link = pstat.ps_recv;
update_pktsrc_stats(&stats, &pstat, &prev_pstat);
prev_pstat = pstat;
}
}
s->link = stats.link;
s->dropped = stats.dropped;
s->received = stats.received;
s->bytes_received = stats.bytes_received;
@ -376,4 +397,67 @@ iosource::PktSrc* PcapSource::Instantiate(const std::string& path, bool is_live)
return new PcapSource(path, is_live);
}
TEST_CASE("pcap source update_pktsrc_stats")
{
PktSrc::Stats stats;
struct pcap_stat now = {0};
struct pcap_stat prev = {0};
SUBCASE("all zero")
{
update_pktsrc_stats(&stats, &now, &prev);
CHECK(stats.link == 0);
CHECK(stats.dropped == 0);
}
SUBCASE("no overflow")
{
now.ps_recv = 7;
now.ps_drop = 3;
update_pktsrc_stats(&stats, &now, &prev);
CHECK(stats.link == 7);
CHECK(stats.dropped == 3);
}
SUBCASE("no overflow prev")
{
stats.link = 2;
stats.dropped = 1;
prev.ps_recv = 2;
prev.ps_drop = 1;
now.ps_recv = 7;
now.ps_drop = 3;
update_pktsrc_stats(&stats, &now, &prev);
CHECK(stats.link == 7);
CHECK(stats.dropped == 3);
}
SUBCASE("overflow")
{
prev.ps_recv = 4294967295;
prev.ps_drop = 4294967294;
now.ps_recv = 0;
now.ps_drop = 1;
update_pktsrc_stats(&stats, &now, &prev);
CHECK(stats.link == 1);
CHECK(stats.dropped == 3);
}
SUBCASE("overflow 2")
{
stats.link = 4294967295;
stats.dropped = 4294967294;
prev.ps_recv = 4294967295;
prev.ps_drop = 4294967294;
now.ps_recv = 10;
now.ps_drop = 3;
update_pktsrc_stats(&stats, &now, &prev);
CHECK(stats.link == 4294967306); // 2**32 - 1 + 11
CHECK(stats.dropped == 4294967299); // 2**32 - 2 + 5
}
}
} // namespace zeek::iosource::pcap

View file

@ -43,6 +43,7 @@ private:
Stats stats;
pcap_t* pd;
struct pcap_stat prev_pstat = {0};
};
} // namespace zeek::iosource::pcap