Re: [PATCH bpf-next v4 1/6] selftests/bpf: Add traffic monitor functions.

[Martin KaFai Lau <martin.lau@linux.dev>]

On 8/2/24 1:37 PM, Kui-Feng Lee wrote:
>> 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)
> 

The macro looks ok. I am not sure if it is easier as a macro in .h or just a 
func in test_progs.c. A quick look is the struct of env.test is not defined in 
.h. Just a thought.

If we have this macro/func, a quick thought is there is not much upside to 
create a new traffic_monitor.c instead of putting everything in 
network_helpers.c. I am fine either way. The other non test_progs binaries just 
need to link another traffic_monitor.o in the future if it wants to do 
traffic_monitor_start().