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

Reply via email to