On 10/24/25 10:01 AM, Eelco Chaudron wrote:
>
>
> On 17 Oct 2025, at 19:11, Ilya Maximets wrote:
>
>> This annotation never worked on structure fields, but it does now with
>> clang 21. The problem is that structures like cmap or rculist are
>> designed for lockless access for readers and so we're not holding any
>> locks while reading. That makes new clang generate thread-safety
>> warnings:
>>
>> lib/dpif-netdev.c:3632:40:
>> warning: reading the value pointed to by 'flow_table' requires
>> holding any mutex [-Wthread-safety-analysis]
>> 3632 | &pmd->flow_table) {
>> | ^
>>
>> Signed-off-by: Ilya Maximets <[email protected]>
>
> Thanks for fixing these. The change looks good to me and tested with clang21.
> Two nits below ;)
>
> Cheers,
> Eelco
>
> Acked-by: Eelco Chaudron <[email protected]>
>
>
> <snip>
>
>> diff --git a/lib/dpif-netdev-private-dpcls.h
>> b/lib/dpif-netdev-private-dpcls.h
>> index 2a9279437..03f07c621 100644
>> --- a/lib/dpif-netdev-private-dpcls.h
>> +++ b/lib/dpif-netdev-private-dpcls.h
>> @@ -66,7 +66,7 @@ uint32_t (*dpcls_subtable_lookup_func)(struct
>> dpcls_subtable *subtable,
>> /* A set of rules that all have the same fields wildcarded. */
>> struct dpcls_subtable {
>> /* The fields are only used by writers. */
>> - struct cmap_node cmap_node OVS_GUARDED; /* Within dpcls
>> 'subtables_map'. */
>> + struct cmap_node cmap_node; /* Within dpcls 'subtables_map'. */
>
> Maybe add one space to align the comment with the ones below?
OK.
>
> <snip>
>
>> diff --git a/lib/dpif-netdev-private-thread.h
>> b/lib/dpif-netdev-private-thread.h
>> index 8715b3837..1992ba44b 100644
>> --- a/lib/dpif-netdev-private-thread.h
>> +++ b/lib/dpif-netdev-private-thread.h
>> @@ -92,13 +92,13 @@ struct dp_netdev_pmd_thread {
>> * made while still holding the 'flow_mutex'.
>> */
>> struct ovs_mutex flow_mutex;
>> - struct cmap flow_table OVS_GUARDED; /* Flow table. */
>> - struct cmap simple_match_table OVS_GUARDED; /* Flow table with simple
>> - match flows only. */
>> + struct cmap flow_table; /* Flow table. */
>> + struct cmap simple_match_table; /* Flow table with simple
>> + * match flows only. */
>
> Remove space before match to align comments. Or maybe change the comment to
> fit on a single line;
>
> struct cmap simple_match_table; /* Flow table with simple match flows. */
>
I think, the word 'only' is carrying some weight here, so I fixed the double
space, but kept it to be two lines.
With that, applied and backported down to 3.3.
Thanks!
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
