Re: [PATCH bpf-next v2 2/4] selftests/bpf: Monitor traffic for tc_redirect/tc_redirect_dtime.

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

On 7/24/24 01:36, Geliang Tang wrote:
> On Tue, 2024-07-23 at 11:24 -0700, Kui-Feng Lee wrote:
>> Enable traffic monitoring for the test case
>> tc_redirect/tc_redirect_dtime.
>>
>> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
>> ---
>>   tools/testing/selftests/bpf/prog_tests/tc_redirect.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
>> b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
>> index 327d51f59142..1be6ea8d6c64 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/tc_redirect.c
>> @@ -900,6 +900,7 @@ static void test_udp_dtime(struct test_tc_dtime
>> *skel, int family, bool bpf_fwd)
>>   static void test_tc_redirect_dtime(struct netns_setup_result
>> *setup_result)
>>   {
>>   	struct test_tc_dtime *skel;
>> +	struct tmonitor_ctx *tmon = NULL;
> 
> nit: No need to set "tmon" to NULL here.

Earlier "goto done" statements would need tmon to be initialized.
Does that make sense?


> 
>>   	struct nstoken *nstoken;
>>   	int hold_tstamp_fd, err;
>>   
>> @@ -934,6 +935,9 @@ static void test_tc_redirect_dtime(struct
>> netns_setup_result *setup_result)
>>   	if (!ASSERT_OK(err, "disable forwarding"))
>>   		goto done;
>>   
>> +	tmon = traffic_monitor_start(NS_DST);
>> +	ASSERT_NEQ(tmon, NULL, "traffic_monitor_start");
>> +
>>   	test_tcp_clear_dtime(skel);
>>   
>>   	test_tcp_dtime(skel, AF_INET, true);
>> @@ -958,6 +962,7 @@ static void test_tc_redirect_dtime(struct
>> netns_setup_result *setup_result)
>>   	test_udp_dtime(skel, AF_INET6, false);
>>   
>>   done:
>> +	traffic_monitor_stop(tmon);
>>   	test_tc_dtime__destroy(skel);
>>   	close(hold_tstamp_fd);
>>   }
>