Re: [PATCH bpf-next 0/4] monitor network traffic for flaky test cases

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

On 7/15/24 3:07 PM, Kui-Feng Lee wrote:
> 
> 
> On 7/15/24 14:33, Stanislav Fomichev wrote:
>> On 07/12, Kui-Feng Lee wrote:
>>> Run tcpdump in the background for flaky test cases related to network
>>> features.
>>
>> Have you considered linking against libpcap instead of shelling out
>> to tcpdump? As long as we have this lib installed on the runners
>> (likely?) that should be a bit cleaner than doing tcpdump.. WDYT?
> 
> I just checked the script building the root image for vmtest. [1]
> It doesn't install libpcap.
> 
> If our approach is to capture the packets in a file, and let developers
> download the file, it would be a simple and straight forward solution.
> If we want a log in text, it would be more complicated to parse
> packets.
> 
> Martin & Stanislay,
> 
> WDYT about capture packets in a file and using libpcap directly?
> Developers can download the file and parse it with tcpdump locally.

thinking out loud...

Re: libpcap (instead of tcpdump) part. I am not very experienced in libpcap. I 
don't have a strong preference. I do hope patch 1 could be more straight forward 
that no need to use loops and artificial udp packets to ensure the tcpdump is 
fully ready to capture. I assume using libpcap can make this sync part 
easier/cleaner (pthread_cond?) and not too much code is needed to use libpcap?

Re: dump to file and download by developer. If I read patch 1 correctly, it only 
dumps everything at the end of the test (by calling traffic_monitor_report). 
imo, it lost the chronological ordering with other ASSERT_* logs and does not 
make a big difference (vs downloading as a file).

The developer needs to go through another exercise to figure out (e.g.) a 
captured packet may be related to a ASSERT_* failure by connecting the timestamp 
between the ASSERT_* log and the captured packet (afaik, there is a timestamp in 
the CI raw log). Ideally, the packet can be logged to stderr/out as soon as it 
is captured such that the developer can still sort of relate the packet with the 
other ASSERT_*() log around it.