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

[Kui-Feng Lee <sinquersw@gmail.com>]

On 7/15/24 16:56, Martin KaFai Lau wrote:
> 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?

Yes, it would be easier and cleaner if we don't parse the payload
of packets.

> 
> 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.
> 

We can print an ordered number for each received packets to stderr asap
and use libpcap without parsing packets. Developers use tcpdump or
wireshark to parse packets.

Or, we can just run tcpdump on background and let it write to stderr
directly. This is convenient for developers. However, we still need to
wait for tcpdump ready.

Another hybrid solution is to have a libpcap thread to capture packets 
and feed packets to tcpdump through a pipe to parse packets. With this,
we don't need to wait for tcpdump anymore. However, I am not sure
how long tcpdump gets ready. It may make all these efforts of keeping
order useless. But, I can try it.