I'll follow up with some sort of VTS test to block
RiscV until the compiler/language/libraries etc
can support something like CMPXCHG16B or ldp
Bug: 444697997
Flag: EXEMPT BUGFIX
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I553cffe3c4181842c38baf37258262e7b43b9ca4
Maciej Żenczykowskicommited 2025-09-12 11:16:29 -07:00
This seems to be the most common usecase.
Test: TreeHugger
Flag: EXEMPT PURE_REFACTOR
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I2de4d067b2f20680539cbddb31555864bcf957b1
Maciej Żenczykowskicommited 2025-09-12 10:14:12 -07:00
Limited to non-32-bit due to build failures on:
cf_gwear_x86-trunk_staging-userdebug
eos-trunk_staging-userdebug
seluna-next-userdebug
seluna-trunk_staging-userdebug
test_suites_x86_64-trunk_staging
test_suites_x86_64-trunk_staging
test_suites_x86_64-trunk_staging_coverage
This is unfortunate, as it likely means those platforms are
inefficient and possibly even buggy.
Test: TreeHugger
Flag: EXEMPT TEST_ONLY
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Ibf7eaa0707b0ec2e8eb9ac8ee9de8af20b94683b
Maciej Żenczykowskicommited 2025-09-12 07:09:13 -07:00
Without a set min_sdk_version the tests are built for
tip-of-tree which implies a 5.10+ kernel which then
results in them failing when installed on pixel 4/5
test devices with 4.14/4.19 kernels.
(Not sure how this ever worked, that should in
theory always have been illegal... I guess we've
been lucky...)
Bug: 439093090
Bug: 439218498
Flag: EXEMPT mainline
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I2b02d314f70f771cb4d29758fe5dfe557a664638
Maciej Żenczykowskicommited 2025-08-18 13:46:28 -07:00
The only real way readValue(K) can fail while iterating
is if K gets deleted *while* we're walking the map.
That shouldn't be able to happen, and even if it does,
we're better off simply continuing and getting what data we can.
Test: TreeHugger
Flag: EXEMPT mainline/noop
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Ia286b4067546ef550cd1d9e6864fcddd9d0e5951
Maciej Żenczykowskicommited 2025-08-14 20:10:43 -07:00
Most spots autoconverted, but this one needs special love.
Test: TreeHugger
Flag: EXEMPT mainline/noop
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I40b29d989c5a51082ac8e9ab09b7644eee46d7cb
Maciej Żenczykowskicommited 2025-08-14 20:10:43 -07:00
Uses 16KiB ram, only caches ifindex < 1024, but in practice
that should usually be the case, as these are assigned
in sequential order by the kernel.
Note that in progs/netd.c:86 we have:
DEFINE_BPF_MAP_NO_NETD(iface_index_name_map, HASH, uint32_t, IfaceValue, 1000)
so stuff doesn't really work right once we go past 1000-ish
tracked interfaces...
(and worst case we're no worse off then we are now...)
Benefit should be significant as all our bpf map stats
dumping logic has to convert from ifindex to ifname
in order to strcmp against the interface we're trying to match,
ie. that's the first thing 3 of the 4 cases we support
do at the very top of their innermost loop.
This effectively eliminates 1 bpf() lookup systemcall, thus
getting us down to 2 (getNextKey + readValue/readValueAndDelete)
Note: the tests work because they're consistent in how they
map ifindex/ifname so they do not require cache clearing.
I believe we don't have to worry about threading, as
we're being called while traversing/consuming maps.
Thus I believe there must be locking external to us.
Also writes should be *super* uncommon, so even if this
were to run multithreaded, races should happen very rarely.
If they did, we'd just write in the same value twice (since
the mapping from ifindex->ifname should rarely if ever change)
Worst case once in a blue moon we'd fail to match an interface
that had *just* shown up (because of reading only half the name),
and we'd lose some stats...
(and since the entire value fits in 16 bytes, and thus in a single
cache line, in practice most cpus will probably write combine
and thus avoid a half write/read anyway, both x86-64 and aarch64
can potentially even do a 16-byte read/write in 1 instruction...
although unclear if that will happen, but worst case should be 2)
Test: TreeHugger, atest libnetworkstats_test
Flag: EXEMPT mainline
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I0d3ea6c5f1f2945adccd479af6c65008bf7d7c51
Maciej Żenczykowskicommited 2025-08-12 20:39:20 -07:00
(in the future consume() can be changed to use kernel bpf map bulk api)
Test: TreeHugger, atest libnetworkstats_test
Flag: EXEMPT mainline
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I43c3e7139cf38058da4c187d1cfeec4bf7fb0ca1
Maciej Żenczykowskicommited 2025-08-11 12:45:51 -07:00
Note: this is not quite a no-op, as we no longer do the ifname
lookup, which could (but shouldn't) fail and cause us to ignore the stats
(for that interface).
But since it shouldn't ever fail, the value lookup should basically always
happen. Switching to forAll() will allow bulk api optimization...
Test: TreeHugger, atest libbpf_android_test libnetworkstats_test netd_updatable_unit_test
Flag: EXEMPT mainline
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Id30fa1d2b4f30c5b526b5eba80e401d1a201d95d
Maciej Żenczykowskicommited 2025-08-09 19:18:48 -07:00
which allows writes, but not deletes.
This is technically what bpf map arrays are capable of.
Test: TreeHugger
Flag: EXEMPT mainline
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I1f73bea5428c190af17442539c296282ce9e0786
Maciej Żenczykowskicommited 2025-08-09 16:03:46 -07:00
It's so much better to just pass in the map (if needed)
as part of the lambda capture...
Test: TreeHugger
Flag: EXEMPT mainline
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: If16fa9c8d0ade41628dc4f7ac1fa73127001b6b1
Maciej Żenczykowskicommited 2025-08-09 14:25:34 -07:00
Some builds (e.g. MSVC) use `#define interface struct` which causes
issues with the field or argument name `interface`.
Change-Id: If18b00028818337c9fabe4a271c570f456d62a9a
Test: Build
Flag: EXEMPT PURE_REFACTOR
This lock causes a deadlock in the handler where OnStop is holding it
waiting for the poller to complete, but the poller is waiting for the
lock in GetDataSourceLocked.
As a result Write can be called concurrent to OnStart/OnStop, but not
OnSetup (guaranteed by Perfetto). The state Write reads is const from
OnSetup onwards, so is fine to read without a lock.
Bug: 320700806
Test: atest libnetworkstats_test
Change-Id: I2a8390268aa8f55c617c22e48129b5cb6fbaf673
Fixes the deadlock described by the below two thread states:
Thread 1:
- ConsumeAll() (takes mBufferMutex)
- perfetto::DataSource::Trace() (takes Perfetto mutex)
Thread 2:
- StartDataSourceImpl() (takes Perfetto mutex)
- NetworkTracePoller::Start() (takes mBufferMutex)
Locks should always be taken in the same order. To achieve this,
ConsumeAll only locks mBufferMutex while accessing the buffer, and
releases it before calling the callback (which under typical use calls
DataSource::Trace() as in "Thread 1").
As a fun side note, Start() always calls ConsumeAll(), so there exists a
lock sequence of perfetto mutex -> mBufferMutex -> perfetto mutex, thus
this order was always "exercised". However, Perfetto uses a recursive
mutex, so the second lock was a no-op, making this hard to catch.
Bug: 320700806
Test: atest libnetworkstats_test
Change-Id: If8c5e0e218666ec53a287ec56333a6d3b8cfe08f
There no longer needs to be separate locked and unlocked methods now
that the buffer has its own mutex.
The most notable change is that within PollAndSchedule, we go from using
try_lock to using the scoped_lock in ConsumeAll. The try_lock was meant
to prevent the following deadlock:
- Stop() takes mMutex and calls mTaskRunner.reset()
- mTaskRunner awaits completion of final closure (PollAndSchedule)
- PollAndSchedule is blocked trying to acquire mMutex
Now that ConsumeAll depends only on mBufferMutex, a more elegant
solution is to just ensure that Stop() isn't holding mBufferMutex while
calling mTaskRunner.reset().
Bug: 320700806
Test: atest libnetworkstats_test
Change-Id: Id5df18bc5809ba193bbe1aa0b97121bd5a8afdcd
This covers the ring buffer and ConsumeAll methods. This change also
removes unnecessary GUARDED_BY for TraceIfaces (making it static) and
mCallback (making it const).
This should be close to a no-op. The new lock is taken in addition to,
or instead of in the few places it's needed.
Bug: 320700806
Test: atest libnetworkstats_test
Change-Id: I45d36eba033afae1c03eac2f8d5b2ac4c9e78716
Ryan Zuklieauthored and Maciej Żenczykowskicommited 2024-09-18 18:34:17 +00:00
This CL is created as a best effort to migrate test targets
to the new android ownership model. If you find incorrect or unnecessary
attribution in this CL, please create a separate CL to fix that.
For more details please refer to the link below,
<add g3 doc link>
Bug: 304529413
Test: N/A
Change-Id: I243f17b3f0ad7af9ffa15ca242456e36688733f5
Merged-In: I243f17b3f0ad7af9ffa15ca242456e36688733f5
(this should fix the 4 ipsec tests which don't call the right
higher APIs that would eventually call into bpfRegisterIface)
Test: TreeHugger, atest libnetworkstats_test
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I3069f3a2f809948cd2e7755745586b78ff3362c9
Maciej Żenczykowskicommited 2024-01-26 15:24:18 -08:00
this helper doesn't really help...
all it does is a bpf map lookup and call a function on failure.
Test: TreeHugger, atest libnetworkstats_test
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I7846eee3a37bcc8be82497cdf22ade5b214795da
Maciej Żenczykowskicommited 2024-01-25 16:36:09 -08:00
b/310046696 introduced a breaking ABI change in libbase, and
the following unit tests fail to launch on previous
platforms because of missing symbols:
- libnetworkstats_test
- netd_updatable_unit_test
Bug: 316464217
Test: atest libnetworkstats_test
Test: atest netd_updatable_unit_test
Change-Id: Icef58de0b46c6042a9459d3d4c45ea9ce2baefed
This reports the total bytes sent and received per interface using
atrace. The totals are taken from the interface counters, rather than
the ringbuffer, since the ring buffer can run out of space.
Interfaces are only reported for interfaces which have packets
sent on them as identified by Nettrace. This helps reduce the cost
significantly for idle periods and on devices with a large number of
interfaces accumulated in the iface stats maps.
Test: TreeHugger
Change-Id: I0e6a3b11de0d1e153b9f8ebb97c338f425381a9c
This adds support for additional protocols and also provides more clear
semantics of protocol specific fields, such as only including tcpFlags
when the protocol is IPPROTO_TCP.
Test: atest libnetworkstats_test & flash and trace
Change-Id: Ic69fc75e85ebf8734027c942a253af4972ad14d8
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
while this is a little bit more code,
it seems much better for the accumulation operation
to be next to the struct definition itself
(in case we ever add more fields)
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I26022db4566e69c964298d7b3f2cc4fa4a9a5152
Maciej Żenczykowskicommited 2023-08-13 09:16:23 +00:00
(next step is to replace use of Stats struct with
identical (except field order) StatsValue struct)
Test: TreeHugger
Bug: 294604315
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I9be3c411f9592bf4edc75386b1c5b386ebeb5905
Maciej Żenczykowskicommited 2023-08-12 08:20:39 +00:00
This change adds the nativeGetIfIndexStat JNI method, which can be used
to read the traffic stats associated with a specific iface index.
Test: Included unit test.
Bug: 241098920
Change-Id: I7a4adcb9610562ff01fe5cd1234d6dfc57f9408d
Although the time between when Trace starts and GetDataSourceLocked is
called is small, it's still possible for the handle to be invalidated
between those two points in time. Check validity prior to use.
Test: atest libnetworkstats_test
bug: 289476598
Change-Id: Ie4b3d65178e6ca24382efde3f38162f332144e81
Calling reset on mTaskRunner waits for any in progress tasks to finish.
Since Stop takes mMutex, it's possible that in-progress task is waiting
for mMutex in order to finish, resulting in a deadlock.
This breaks the loop by using try_lock to remove the blocking dependency
on the mutex. Since we _always_ want to re-schedule ourselves, the poll
duration and runner are taken as arguments (if rescheduling is also in
the try_lock, any class interactions could stop the polling sequence).
Test: local start/stop stress test and atest libnetworkstats_test
Bug: 285411033
Change-Id: I4b8bea9c5474a37e8c081bcfe542d4fe57f5206c
Around 10% of the traces with Nettrace have "traced_final_flush_failed"
errors. It is believed that Network Tracing isn't writing enough data to
fill one "Chunk" in Perfetto's buffer. Although this should still be
saved by Perfetto, it doesn't seem to be.
This change records the number of packets read from the ring buffer to
understand whether the error coincided with low-data cases. It also
tries to flush the data OnStop to potentially fix the issue.
Test: flash and run trace
Change-Id: I92c8d2d8d47d1ed123585e1cfdde802d286f120f
Instead of also accounting tag!=0 traffic against tag==0 slot,
while the bpf code writes into the map, move this logic into
the userspace jni code which reads from the map.
Simplifies the bpf program making things easier on the
kernel's bpf verifier, and is better for performance,
since a per-packet fixup operation becomes a per-poll fixup.
Test: TreeHugger, atest libnetworkstats_test FrameworksNetTests
Bug: 276296921
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Ic220a201781a1170bcffe327fe5664fc12b65dd9
Maciej Żenczykowskicommited 2023-04-14 07:18:01 +00:00
only the test code ever passes in anything that isn't
a limit {UID_ALL, INTERFACES_ALL, TAG_ALL} (ie. no limit)
Test: TreeHugger, atest libnetworkstats_test FrameworksNetTests
Bug: 276296921
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Ida489f25c4da4b12541c6001b41d9e4b30804eff
Maciej Żenczykowskicommited 2023-04-11 19:24:28 +00:00
This should ensure that we don't have stale packets next time we start a
trace.
Bug: 246985031
Test: flash & trace with long (1m+) poll (used to get stale, now not)
Change-Id: I6085d4a97688a221d26c095ef9073360292fd1ec
It's not obvious from code how the simple false value reduces the binary
size. Adds a comment to explain how it works and a warning about
changing the flag value.
Bug: 272207884
Test: It's a comment
Change-Id: I5ea733ef879e8e6ea1a2a6bfb3aa22b1fbedd6da
Maciej Żenczykowski
commited 
Android (Google) Code Review
commited 
Ryan Zuklie
commited 
Ryan Prichard
commited 
Aditya Choudhary
commited 
Henri Chataing
commited 
Kevin Jeon
commited 
Treehugger Robot
authored and
Gerrit Code Review
commited