Make BundleKey a distinct struct from PacketTrace.

This change refactors BundleKey to be a separate struct. This allows
us to do the conversion (e.g. of sport/dport, or icmp fields in the
future) in the constructor and change types (use optionals) where it
is convenient to simplify the code.

This change also fixes a bunch of desigated initializer order warnings
in the test (fields should be declared in same order as defined).

Test: atest libnetworkstats_test
Change-Id: I6373d2512280e93c54395fde38dcff43705b449e
This commit is contained in:
Ryan Zuklie 2023-09-12 10:28:21 -07:00
commit 1733d3e6be
3 changed files with 69 additions and 49 deletions

View file

@ -75,9 +75,20 @@ struct BundleDetails {
uint32_t bytes = 0;
};
#define AGG_FIELDS(x) \
(x).ifindex, (x).uid, (x).tag, (x).sport, (x).dport, (x).egress, \
(x).ipProto, (x).tcpFlags
BundleKey::BundleKey(const PacketTrace& pkt)
: ifindex(pkt.ifindex),
uid(pkt.uid),
tag(pkt.tag),
egress(pkt.egress),
ipProto(pkt.ipProto),
ipVersion(pkt.ipVersion),
tcpFlags(pkt.tcpFlags),
localPort(ntohs(pkt.egress ? pkt.sport : pkt.dport)),
remotePort(ntohs(pkt.egress ? pkt.dport : pkt.sport)) {}
#define AGG_FIELDS(x) \
(x).ifindex, (x).uid, (x).tag, (x).egress, (x).ipProto, (x).ipVersion, \
(x).tcpFlags, (x).localPort, (x).remotePort
std::size_t BundleHash::operator()(const BundleKey& a) const {
std::size_t seed = 0;
@ -179,7 +190,7 @@ void NetworkTraceHandler::Write(const std::vector<PacketTrace>& packets,
dst->set_timestamp(pkt.timestampNs);
auto* event = dst->set_network_packet();
event->set_length(pkt.length);
Fill(pkt, event);
Fill(BundleKey(pkt), event);
}
return;
}
@ -187,14 +198,13 @@ void NetworkTraceHandler::Write(const std::vector<PacketTrace>& packets,
uint64_t minTs = std::numeric_limits<uint64_t>::max();
std::unordered_map<BundleKey, BundleDetails, BundleHash, BundleEq> bundles;
for (const PacketTrace& pkt : packets) {
BundleKey key = pkt;
BundleKey key(pkt);
// Dropping fields should remove them from the output and remove them from
// the aggregation key. In order to do the latter without changing the hash
// function, set the dropped fields to zero.
if (mDropTcpFlags) key.tcpFlags = 0;
if (mDropLocalPort) (key.egress ? key.sport : key.dport) = 0;
if (mDropRemotePort) (key.egress ? key.dport : key.sport) = 0;
// the aggregation key. Reset the optionals to indicate omission.
if (mDropTcpFlags) key.tcpFlags.reset();
if (mDropLocalPort) key.localPort.reset();
if (mDropRemotePort) key.remotePort.reset();
minTs = std::min(minTs, pkt.timestampNs);
@ -245,22 +255,16 @@ void NetworkTraceHandler::Write(const std::vector<PacketTrace>& packets,
}
}
void NetworkTraceHandler::Fill(const PacketTrace& src,
void NetworkTraceHandler::Fill(const BundleKey& src,
NetworkPacketEvent* event) {
event->set_direction(src.egress ? TrafficDirection::DIR_EGRESS
: TrafficDirection::DIR_INGRESS);
event->set_uid(src.uid);
event->set_tag(src.tag);
if (!mDropLocalPort) {
event->set_local_port(ntohs(src.egress ? src.sport : src.dport));
}
if (!mDropRemotePort) {
event->set_remote_port(ntohs(src.egress ? src.dport : src.sport));
}
if (!mDropTcpFlags) {
event->set_tcp_flags(src.tcpFlags);
}
if (src.tcpFlags.has_value()) event->set_tcp_flags(*src.tcpFlags);
if (src.localPort.has_value()) event->set_local_port(*src.localPort);
if (src.remotePort.has_value()) event->set_remote_port(*src.remotePort);
event->set_ip_proto(src.ipProto);

View file

@ -168,12 +168,12 @@ TEST_F(NetworkTraceHandlerTest, BasicBundling) {
config.set_aggregation_threshold(10);
std::vector<PacketTrace> input = {
PacketTrace{.uid = 123, .timestampNs = 2, .length = 200},
PacketTrace{.uid = 123, .timestampNs = 1, .length = 100},
PacketTrace{.uid = 123, .timestampNs = 4, .length = 300},
PacketTrace{.timestampNs = 2, .length = 200, .uid = 123},
PacketTrace{.timestampNs = 1, .length = 100, .uid = 123},
PacketTrace{.timestampNs = 4, .length = 300, .uid = 123},
PacketTrace{.uid = 456, .timestampNs = 2, .length = 400},
PacketTrace{.uid = 456, .timestampNs = 4, .length = 100},
PacketTrace{.timestampNs = 2, .length = 400, .uid = 456},
PacketTrace{.timestampNs = 4, .length = 100, .uid = 456},
};
std::vector<TracePacket> events;
@ -203,12 +203,12 @@ TEST_F(NetworkTraceHandlerTest, AggregationThreshold) {
config.set_aggregation_threshold(3);
std::vector<PacketTrace> input = {
PacketTrace{.uid = 123, .timestampNs = 2, .length = 200},
PacketTrace{.uid = 123, .timestampNs = 1, .length = 100},
PacketTrace{.uid = 123, .timestampNs = 4, .length = 300},
PacketTrace{.timestampNs = 2, .length = 200, .uid = 123},
PacketTrace{.timestampNs = 1, .length = 100, .uid = 123},
PacketTrace{.timestampNs = 4, .length = 300, .uid = 123},
PacketTrace{.uid = 456, .timestampNs = 2, .length = 400},
PacketTrace{.uid = 456, .timestampNs = 4, .length = 100},
PacketTrace{.timestampNs = 2, .length = 400, .uid = 456},
PacketTrace{.timestampNs = 4, .length = 100, .uid = 456},
};
std::vector<TracePacket> events;
@ -239,10 +239,10 @@ TEST_F(NetworkTraceHandlerTest, DropLocalPort) {
__be16 b = htons(10001);
std::vector<PacketTrace> input = {
// Recall that local is `src` for egress and `dst` for ingress.
PacketTrace{.timestampNs = 1, .length = 2, .egress = true, .sport = a},
PacketTrace{.timestampNs = 2, .length = 4, .egress = false, .dport = a},
PacketTrace{.timestampNs = 3, .length = 6, .egress = true, .sport = b},
PacketTrace{.timestampNs = 4, .length = 8, .egress = false, .dport = b},
PacketTrace{.timestampNs = 1, .length = 2, .sport = a, .egress = true},
PacketTrace{.timestampNs = 2, .length = 4, .dport = a, .egress = false},
PacketTrace{.timestampNs = 3, .length = 6, .sport = b, .egress = true},
PacketTrace{.timestampNs = 4, .length = 8, .dport = b, .egress = false},
};
std::vector<TracePacket> events;
@ -274,10 +274,10 @@ TEST_F(NetworkTraceHandlerTest, DropRemotePort) {
__be16 b = htons(80);
std::vector<PacketTrace> input = {
// Recall that remote is `dst` for egress and `src` for ingress.
PacketTrace{.timestampNs = 1, .length = 2, .egress = true, .dport = a},
PacketTrace{.timestampNs = 2, .length = 4, .egress = false, .sport = a},
PacketTrace{.timestampNs = 3, .length = 6, .egress = true, .dport = b},
PacketTrace{.timestampNs = 4, .length = 8, .egress = false, .sport = b},
PacketTrace{.timestampNs = 1, .length = 2, .dport = a, .egress = true},
PacketTrace{.timestampNs = 2, .length = 4, .sport = a, .egress = false},
PacketTrace{.timestampNs = 3, .length = 6, .dport = b, .egress = true},
PacketTrace{.timestampNs = 4, .length = 8, .sport = b, .egress = false},
};
std::vector<TracePacket> events;
@ -306,10 +306,10 @@ TEST_F(NetworkTraceHandlerTest, DropTcpFlags) {
config.set_aggregation_threshold(10);
std::vector<PacketTrace> input = {
PacketTrace{.timestampNs = 1, .uid = 123, .length = 1, .tcpFlags = 1},
PacketTrace{.timestampNs = 2, .uid = 123, .length = 2, .tcpFlags = 2},
PacketTrace{.timestampNs = 3, .uid = 456, .length = 3, .tcpFlags = 1},
PacketTrace{.timestampNs = 4, .uid = 456, .length = 4, .tcpFlags = 2},
PacketTrace{.timestampNs = 1, .length = 1, .uid = 123, .tcpFlags = 1},
PacketTrace{.timestampNs = 2, .length = 2, .uid = 123, .tcpFlags = 2},
PacketTrace{.timestampNs = 3, .length = 3, .uid = 456, .tcpFlags = 1},
PacketTrace{.timestampNs = 4, .length = 4, .uid = 456, .tcpFlags = 2},
};
std::vector<TracePacket> events;

View file

@ -30,15 +30,31 @@
namespace android {
namespace bpf {
// BundleKeys are PacketTraces where timestamp and length are ignored.
using BundleKey = PacketTrace;
// BundleKey encodes a PacketTrace minus timestamp and length. The key should
// match many packets over time for interning. For convenience, sport/dport
// are parsed here as either local/remote port.
struct BundleKey {
explicit BundleKey(const PacketTrace& pkt);
// BundleKeys are hashed using all fields except timestamp/length.
uint32_t ifindex;
uint32_t uid;
uint32_t tag;
bool egress;
uint8_t ipProto;
uint8_t ipVersion;
std::optional<uint8_t> tcpFlags;
std::optional<uint16_t> localPort;
std::optional<uint16_t> remotePort;
};
// BundleKeys are hashed using a simple hash combine.
struct BundleHash {
std::size_t operator()(const BundleKey& a) const;
};
// BundleKeys are equal if all fields except timestamp/length are equal.
// BundleKeys are equal if all fields are equal.
struct BundleEq {
bool operator()(const BundleKey& a, const BundleKey& b) const;
};
@ -84,13 +100,13 @@ class NetworkTraceHandler
NetworkTraceHandler::TraceContext& ctx);
private:
// Convert a PacketTrace into a Perfetto trace packet.
void Fill(const PacketTrace& src,
// Fills in contextual information from a bundle without interning.
void Fill(const BundleKey& src,
::perfetto::protos::pbzero::NetworkPacketEvent* event);
// Fills in contextual information either inline or via interning.
::perfetto::protos::pbzero::NetworkPacketBundle* FillWithInterning(
NetworkTraceState* state, const BundleKey& key,
NetworkTraceState* state, const BundleKey& src,
::perfetto::protos::pbzero::TracePacket* dst);
static internal::NetworkTracePoller sPoller;