I enabled ASan on MSVC
(https://devblogs.microsoft.com/cppblog/addresssanitizer-asan-for-windows-wi
th-msvc/)
and this patch alleviates the issues found by the CI and my local testing.

Tested-by: Alin-Gabriel Serdean <[email protected]>
Acked-by: Alin-Gabriel Serdean <[email protected]>

-----Original Message-----
From: dev <[email protected]> On Behalf Of Gaetan Rivet
Sent: Wednesday, February 23, 2022 8:48 PM
To: [email protected]
Subject: [ovs-dev] [PATCH] ofproto: Use xlate map for uuid lookups

The ofproto map 'all_ofproto_dpifs_by_uuid' does not support concurrent
accesses. It is however read by upcall handler threads and written by the
main thread at the same time.

Additionally, handler threads will change the ams_seq while an ofproto is
being destroyed, triggering crashes with the following backtrace:

(gdb) bt
  hmap_next (hmap.h:398)
  seq_wake_waiters (seq.c:326)
  seq_change_protected (seq.c:134)
  seq_change (seq.c:144)
  ofproto_dpif_send_async_msg (ofproto_dpif.c:263)
  process_upcall (ofproto_dpif_upcall.c:1782)
  recv_upcalls (ofproto_dpif_upcall.c:1026)
  udpif_upcall_handler (ofproto/ofproto_dpif_upcall.c:945)
  ovsthread_wrapper (ovs_thread.c:734)

To solve both issues, remove the 'all_ofproto_dpifs_by_uuid'.
Instead, another map already storing ofprotos in xlate can be used.

During an ofproto destruction, its reference is removed from the current
xlate xcfg. Such change is committed only after all threads have quiesced at
least once during xlate_txn_commit(). This wait ensures that the removal is
seen by all threads, rendering impossible for a thread to still hold a
reference while the destruction proceeds.

Furthermore, the xlate maps are copied during updates instead of being
written in place. It is thus correct to read xcfg->xbridges while inserting
or removing from new_xcfg->xbridges.

Finally, now that ofproto_dpifs lookups are done through xcfg->xbridges, it
is important to use a high level of entropy. As it used the ofproto pointer
hashed, fewer bits were random compared to the uuid key used in
'all_ofproto_dpifs_by_uuid'. To solve this, use the ofproto uuid as the key
in xbridges as well, improving entropy.

Fixes: fcb9579be3c7 ("ofproto: Add 'ofproto_uuid' and 'ofp_in_port' to user
action cookie.")
Suggested-by: Adrian Moreno <[email protected]>
Signed-off-by: Yunjian Wang <[email protected]>
Signed-off-by: Gaetan Rivet <[email protected]>
---

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to