On 10/27/25 11:50 AM, Eelco Chaudron wrote:
>
>
> On 24 Oct 2025, at 18:56, Ilya Maximets wrote:
>
>> In a few places the list sizes are checked without holding the lock.
>> While this probably speeds up the processing a bit, it's technically
>> incorrect as the access is not atomic. Caught by thread-safety
>> analyzer of clang 21, as it now properly supports GUARDED annotations
>> on structure fields:
>>
>> lib/ipf.c:1117:33:
>> error: reading the value pointed to by 'frag_exp_list' requires
>> holding any mutex [-Werror,-Wthread-safety-analysis]
>> 1117 | if (ovs_list_is_empty(&ipf->frag_exp_list)) {
>> | ^
>> lib/ipf.c:1154:33:
>> error: reading the value pointed to by 'reassembled_pkt_list'
>> requires holding any mutex [-Werror,-Wthread-safety-analysis]
>> 1154 | if (ovs_list_is_empty(&ipf->reassembled_pkt_list)) {
>> | ^
>>
>> Not trying to fix the actual non-atomic access to the list as getting
>> the wrong result in these particular calls will not meaningfully
>> impact the correctness of the code, but taking the lock before checking
>> will likely reduce performance.
>>
>> For now, just working around the problem by turning off thread safety
>> analysis for these particular calls. The new _unsafe() access function
>> is also a bit more obvious in a way that it's more clear that these
>> calls should not be relied upon for logic that requires precision.
>>
>> Signed-off-by: Ilya Maximets <[email protected]>
>
> hanks for fixing this, Ilya. I guess this will do for now.
>
> Acked-by: Eelco Chaudron <[email protected]>
>
Thanks! Applied and backported down to 3.3.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev