Clang 21 now properly implements GUARDED annotations for structure fields, and so it complains about those fields being accessed in init/create/destroy type of functions that normally do not take any locks, as the data is just allocated and not available to other threads. Add some locks where it is simple to do, so we don't have to turn off the analysis entirely, in other places just turn off the analysis.
While at it, adding a couple of missed annotations that clang started to complain about due to actual support for the GUARDED annotation on structure fields. Added some unused arguments to a couple of functions to be able to specify which lock is required. It seemed better than turning off the analysis. Signed-off-by: Ilya Maximets <[email protected]> --- lib/dpif-netdev.c | 2 ++ lib/fat-rwlock.c | 2 ++ lib/mac-learning.c | 1 + lib/mcast-snooping.c | 22 ++++++++++++++-------- lib/timeval.c | 2 ++ ofproto/bond.c | 4 +++- ofproto/ofproto-dpif.c | 10 ++++++++-- ofproto/ofproto.c | 3 +++ 8 files changed, 35 insertions(+), 11 deletions(-) diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 99d802bde..224ce7086 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -7841,6 +7841,7 @@ dp_netdev_pmd_get_next(struct dp_netdev *dp, struct cmap_position *pos) static void dp_netdev_configure_pmd(struct dp_netdev_pmd_thread *pmd, struct dp_netdev *dp, unsigned core_id, int numa_id) + OVS_NO_THREAD_SAFETY_ANALYSIS { pmd->dp = dp; pmd->core_id = core_id; @@ -7895,6 +7896,7 @@ dp_netdev_configure_pmd(struct dp_netdev_pmd_thread *pmd, struct dp_netdev *dp, static void dp_netdev_destroy_pmd(struct dp_netdev_pmd_thread *pmd) + OVS_NO_THREAD_SAFETY_ANALYSIS { struct dpcls *cls; diff --git a/lib/fat-rwlock.c b/lib/fat-rwlock.c index 771ccc973..50fcec145 100644 --- a/lib/fat-rwlock.c +++ b/lib/fat-rwlock.c @@ -104,9 +104,11 @@ fat_rwlock_destroy(struct fat_rwlock *rwlock) * data destructor can't race with our loop below. */ ovsthread_key_delete(rwlock->key); + ovs_mutex_lock(&rwlock->mutex); LIST_FOR_EACH_SAFE (slot, list_node, &rwlock->threads) { free_slot(slot); } + ovs_mutex_unlock(&rwlock->mutex); ovs_mutex_destroy(&rwlock->mutex); } diff --git a/lib/mac-learning.c b/lib/mac-learning.c index 5932e2709..affb2faf5 100644 --- a/lib/mac-learning.c +++ b/lib/mac-learning.c @@ -215,6 +215,7 @@ mac_learning_clear_statistics(struct mac_learning *ml) * entries. */ struct mac_learning * mac_learning_create(unsigned int idle_time) + OVS_NO_THREAD_SAFETY_ANALYSIS { struct mac_learning *ml; diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c index 7e1a55120..c236343a2 100644 --- a/lib/mcast-snooping.c +++ b/lib/mcast-snooping.c @@ -187,6 +187,7 @@ normalize_idle_time(unsigned int idle_time) * MCAST_DEFAULT_MAX entries. */ struct mcast_snooping * mcast_snooping_create(void) + OVS_NO_THREAD_SAFETY_ANALYSIS { struct mcast_snooping *ms; @@ -321,7 +322,9 @@ mcast_group_insert_bundle(struct mcast_snooping *ms OVS_UNUSED, /* Return true if multicast still has bundles associated. * Return false if there is no bundles. */ static bool -mcast_group_has_bundles(struct mcast_group *grp) +mcast_group_has_bundles(const struct mcast_snooping *ms OVS_UNUSED, + const struct mcast_group *grp) + OVS_REQ_RDLOCK(ms->rwlock) { return !ovs_list_is_empty(&grp->bundle_lru); } @@ -331,6 +334,7 @@ mcast_group_has_bundles(struct mcast_group *grp) static void mcast_snooping_flush_group__(struct mcast_snooping *ms, struct mcast_group *grp) + OVS_REQ_WRLOCK(ms->rwlock) { ovs_assert(ovs_list_is_empty(&grp->bundle_lru)); hmap_remove(&ms->table, &grp->hmap_node); @@ -394,7 +398,7 @@ mcast_snooping_prune_expired(struct mcast_snooping *ms, expired++; } - if (!mcast_group_has_bundles(grp)) { + if (!mcast_group_has_bundles(ms, grp)) { mcast_snooping_flush_group__(ms, grp); expired++; } @@ -713,7 +717,9 @@ mcast_snooping_add_mrouter(struct mcast_snooping *ms, uint16_t vlan, } static void -mcast_snooping_flush_mrouter(struct mcast_mrouter_bundle *mrouter) +mcast_snooping_flush_mrouter(const struct mcast_snooping *ms OVS_UNUSED, + struct mcast_mrouter_bundle *mrouter) + OVS_REQ_WRLOCK(ms->rwlock) { ovs_list_remove(&mrouter->mrouter_node); free(mrouter); @@ -826,7 +832,7 @@ mcast_snooping_mdb_flush__(struct mcast_snooping *ms) hmap_shrink(&ms->table); while (mrouter_get_lru(ms, &mrouter)) { - mcast_snooping_flush_mrouter(mrouter); + mcast_snooping_flush_mrouter(ms, mrouter); } } @@ -859,7 +865,7 @@ mcast_snooping_flush__(struct mcast_snooping *ms) /* flush multicast routers */ while (mrouter_get_lru(ms, &mrouter)) { - mcast_snooping_flush_mrouter(mrouter); + mcast_snooping_flush_mrouter(ms, mrouter); } /* flush flood ports */ @@ -909,7 +915,7 @@ mcast_snooping_run__(struct mcast_snooping *ms) mrouter_expired = 0; while (mrouter_get_lru(ms, &mrouter) && time_now() >= mrouter->expires) { - mcast_snooping_flush_mrouter(mrouter); + mcast_snooping_flush_mrouter(ms, mrouter); mrouter_expired++; } @@ -1001,7 +1007,7 @@ mcast_snooping_flush_bundle(struct mcast_snooping *ms, void *port) if (mcast_group_delete_bundle(ms, g, port)) { ms->need_revalidate = true; - if (!mcast_group_has_bundles(g)) { + if (!mcast_group_has_bundles(ms, g)) { mcast_snooping_flush_group__(ms, g); } } @@ -1009,7 +1015,7 @@ mcast_snooping_flush_bundle(struct mcast_snooping *ms, void *port) LIST_FOR_EACH_SAFE (m, mrouter_node, &ms->mrouter_lru) { if (m->port == port) { - mcast_snooping_flush_mrouter(m); + mcast_snooping_flush_mrouter(ms, m); ms->need_revalidate = true; } } diff --git a/lib/timeval.c b/lib/timeval.c index 10c1b9ca1..8258e89d9 100644 --- a/lib/timeval.c +++ b/lib/timeval.c @@ -114,6 +114,7 @@ static void timespec_add(struct timespec *sum, static void init_clock(struct clock *c, clockid_t id) + OVS_NO_THREAD_SAFETY_ANALYSIS { memset(c, 0, sizeof *c); c->id = id; @@ -124,6 +125,7 @@ init_clock(struct clock *c, clockid_t id) static void do_init_time(void) + OVS_NO_THREAD_SAFETY_ANALYSIS { struct timespec ts; diff --git a/ofproto/bond.c b/ofproto/bond.c index 00f5dc754..584c38f07 100644 --- a/ofproto/bond.c +++ b/ofproto/bond.c @@ -242,8 +242,10 @@ bond_create(const struct bond_settings *s, struct ofproto_dpif *ofproto) bond = xzalloc(sizeof *bond); bond->ofproto = ofproto; hmap_init(&bond->members); - ovs_list_init(&bond->enabled_members); ovs_mutex_init(&bond->mutex); + ovs_mutex_lock(&bond->mutex); + ovs_list_init(&bond->enabled_members); + ovs_mutex_unlock(&bond->mutex); ovs_refcount_init(&bond->ref_cnt); hmap_init(&bond->pr_rule_ops); diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index ed9e44ce2..4a200dd08 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -709,8 +709,10 @@ close_dpif_backer(struct dpif_backer *backer, bool del) } } simap_destroy(&backer->tnl_backers); - ovs_rwlock_destroy(&backer->odp_to_ofport_lock); + ovs_rwlock_wrlock(&backer->odp_to_ofport_lock); hmap_destroy(&backer->odp_to_ofport_map); + ovs_rwlock_unlock(&backer->odp_to_ofport_lock); + ovs_rwlock_destroy(&backer->odp_to_ofport_lock); shash_find_and_delete(&all_dpif_backers, backer->type); free(backer->type); free(backer->dp_version_string); @@ -784,8 +786,10 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp) backer->type = xstrdup(type); backer->refcount = 1; - hmap_init(&backer->odp_to_ofport_map); ovs_rwlock_init(&backer->odp_to_ofport_lock); + ovs_rwlock_wrlock(&backer->odp_to_ofport_lock); + hmap_init(&backer->odp_to_ofport_map); + ovs_rwlock_unlock(&backer->odp_to_ofport_lock); backer->need_revalidate = 0; simap_init(&backer->tnl_backers); backer->recv_set_enable = !ofproto_get_flow_restore_wait(); @@ -1841,7 +1845,9 @@ construct(struct ofproto *ofproto_) hmap_insert(&all_ofproto_dpifs_by_name, &ofproto->all_ofproto_dpifs_by_name_node, hash_string(ofproto->up.name, 0)); + ovs_mutex_lock(&ofproto->stats_mutex); memset(&ofproto->stats, 0, sizeof ofproto->stats); + ovs_mutex_unlock(&ofproto->stats_mutex); ofproto_init_tables(ofproto_, N_TABLES); error = add_internal_flows(ofproto); diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 6fa18228b..ec6d60a44 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -7337,12 +7337,14 @@ ofproto_group_exists(const struct ofproto *ofproto, uint32_t group_id) static void group_add_rule(struct ofgroup *group, struct rule *rule) + OVS_REQUIRES(ofproto_mutex) { rule_collection_add(&group->rules, rule); } static void group_remove_rule(struct ofgroup *group, struct rule *rule) + OVS_REQUIRES(ofproto_mutex) { rule_collection_remove(&group->rules, rule); } @@ -7559,6 +7561,7 @@ handle_queue_get_config_request(struct ofconn *ofconn, static enum ofperr init_group(struct ofproto *ofproto, const struct ofputil_group_mod *gm, ovs_version_t version, struct ofgroup **ofgroup) + OVS_REQUIRES(ofproto_mutex) { enum ofperr error; const long long int now = time_msec(); -- 2.51.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
