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

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

On 8/6/24 4:18 PM, Kui-Feng Lee wrote:
> 
> 
> On 8/6/24 15:22, Martin KaFai Lau wrote:
>> On 8/6/24 3:07 PM, Kui-Feng Lee wrote:
>>>
>>>
>>> On 8/1/24 21:35, Kui-Feng Lee wrote:
>>>>
>>>>
>>>> On 8/1/24 20:43, Martin KaFai Lau wrote:
>>>>> On 7/31/24 12:31 PM, Kui-Feng Lee wrote:
>>>>>> diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/ 
>>>>>> selftests/bpf/test_progs.h
>>>>>> index cb9d6d46826b..5d4e61fa26a1 100644
>>>>>> --- a/tools/testing/selftests/bpf/test_progs.h
>>>>>> +++ b/tools/testing/selftests/bpf/test_progs.h
>>>>>> @@ -473,4 +473,20 @@ extern void test_loader_fini(struct test_loader 
>>>>>> *tester);
>>>>>>       test_loader_fini(&tester);                           \
>>>>>>   })
>>>>>> +struct tmonitor_ctx;
>>>>>> +
>>>>>> +#ifdef TRAFFIC_MONITOR
>>>>>> +struct tmonitor_ctx *traffic_monitor_start(const char *netns);
>>>>>> +void traffic_monitor_stop(struct tmonitor_ctx *ctx);
>>>>>> +#else
>>>>>> +static inline struct tmonitor_ctx *traffic_monitor_start(const char *netns)
>>>>>> +{
>>>>>> +    return (struct tmonitor_ctx *)-1;
>>>>>
>>>>> hmm... from peeking patch 3, only NULL is checked.
>>>
>>> When traffic monitor is disable, these two functions are noop.
>>> Returning -1 (not NULL) is convenient for the callers. They don't need
>>> to tell if the error caused by a real error or by the disabled
>>> feature.
>>
>> I pasted the code from patch 3 here only to ensure I understand the above 
>> explanation correctly:
>>
>> +        netns_obj->tmon = traffic_monitor_start(name);
>> +        if (!netns_obj->tmon)
>>              ^^^^^^^^^^^^^^^^
>>
>> +            goto fail;
>>
>> Does it mean the traffic_monitor_start() above will never be called if 
>> TRAFFIC_MONITOR macro is not defined such that traffic_monitor_start() 
>> returning -1 but testing for NULL here does not matter?
> 
> Correct!

Got it. Then I missed some understanding. Can you explain why the above 
traffic_monitor_start() will never be called?