On 8/2/24 11:58, Martin KaFai Lau wrote:
On 8/1/24 9:31 PM, Kui-Feng Lee wrote:
On 8/1/24 20:29, Martin KaFai Lau wrote:
On 7/31/24 12:31 PM, Kui-Feng Lee wrote:
Add functions that capture packets and print log in the background.
They
are supposed to be used for debugging flaky network test cases. A
monitored
test case should call traffic_monitor_start() to start a thread to
capture
packets in the background for a given namespace and call
traffic_monitor_stop() to stop capturing. (Or, option '-m'
implemented by
the later patches.)
IPv4 TCP packet: 127.0.0.1:48165 -> 127.0.0.1:36707, len 68,
ifindex 1, SYN
IPv4 TCP packet: 127.0.0.1:36707 -> 127.0.0.1:48165, len 60,
ifindex 1, SYN, ACK
IPv4 TCP packet: 127.0.0.1:48165 -> 127.0.0.1:36707, len 60,
ifindex 1, ACK
IPv4 TCP packet: 127.0.0.1:36707 -> 127.0.0.1:48165, len 52,
ifindex 1, ACK
IPv4 TCP packet: 127.0.0.1:48165 -> 127.0.0.1:36707, len 52,
ifindex 1, FIN, ACK
IPv4 TCP packet: 127.0.0.1:36707 -> 127.0.0.1:48165, len 52,
ifindex 1, RST, ACK
nit. Instead of ifindex, it should be ifname now.
Sure! I will update it.
Packet file: packets-2172-86-select_reuseport:sockhash-test.log
#280/87 select_reuseport/sockhash IPv4/TCP LOOPBACK
test_detach_bpf:OK
The above is the output of an example. It shows the packets of a
connection
and the name of the file that contains captured packets in the
directory
/tmp/tmon_pcap. The file can be loaded by tcpdump or wireshark.
This feature only works if TRAFFIC_MONITOR variable has been passed to
build BPF selftests. For example,
make TRAFFIC_MONITOR=1 -C tools/testing/selftests/bpf
This command will build BPF selftests with this feature enabled.
Signed-off-by: Kui-Feng Lee <thinker.li@xxxxxxxxx>
---
tools/testing/selftests/bpf/Makefile | 5 +
tools/testing/selftests/bpf/test_progs.c | 432
+++++++++++++++++++++++
In the cover letter, it mentioned the traffic monitoring
implementation is moved from the network_helpers.c to test_progs.c.
Can you share more about the reason?
network_helpers.c has been used by several test programs.
However, they don't have env that we found in test_progs.c.
That means we could not access env directly. Instead, the caller
have to pass the test name and subtest name to the function.
Leter, we also need to check if a test name matches the patterns. It is
inconvient for users. So, I move these functions to test_progs.c to make
user's life eaiser.
Is it because the traffic monitor now depends on the test_progs's
test name, should_tmon...etc ? Can the test name and should_tmon be
exported for the network_helpers to use?
Yes! And in later patches, we also introduce a list of patterns.
The list of patterns matching is summarized in "should_tmon" which can
be exported through a function?
Yes! Even with a functio, it still depends on test_progs.c.
or I have missed another criteria when deciding tmon should be enabled
for a test?
What other compilation issues did it hit if the traffic monitor codes
stay in the network_helpers.c? Some individual binaries (with main())
like test_tcp_check_syncookie_user that links to network_helpers.o
but not to test_progs.o?
Yes, they are problems as well. These binary also need to link to
libpcap even they don't use it although this is not an important issue.
I don't think linking the non test_progs binaries to libpcap or not is
important.
I am positive there are ways out of it without adding the networking
codes to the test_progs.c. It sounds like an unnecessary nit now but I
believe it is useful going forward when making changes and extension to
the traffic monitoring. May be brainstorm a little to see if there is an
way out.
One way could be putting them in a new traffic_monitor.c such that the
non test_progs binaries won't link to it. and exports the test name and
shmod_tmon in test_progs.h (e.g. through function).
Another way (better and my preference if it works out) is to ask the
traffic_monitor_start() to take the the pcap file name args and makeup a
reasonable default if no filename is given. Not that I am promoting non
test_progs tests, traffic_monitor_start() can then be reused by others
for legit reason. The test_progs's tests usually should not use
traffic_monitor_start() directly and they should stay with the
netns_{new, free}. I think only netns_new needs the env to figure out
the should_tmon and the pcap filename. May be netns_new() can stay in
test_progs.c, or rename it to test__netns_new().
wdyt?
How about put two ideas together?
Have traffic_monitor.c and macros in test_progs.h to collect
data from env, and pass the data to netns_new() in traffic_monitor.c.
For example,
#define test__netns_new(ns) netns_new(ns, env.test->should_tmon || \
(env.subtest_state && env.subtest_state->should_tmon), \
env.test->test_name, \
env.subtest_state ? env.subtest_state->name: NULL)
