From: Numan Siddique <[email protected]>
ovn-controller maintains a shash of pending ct zone entries
to flush the ct zone ids and to store/remove the allocated zone id
in/from the OVS bridge.external_ids. While adding an entry to
the shash, the function 'add_pending_ct_zone_entry()' doesn't
check for existing entry for the same key in shash. If suppose
there are multiple entries for the samestring key, then it results in
an infinite loop of adding and deleting the key entries in
OVS bridge.external_ids.
The pending ct zone entries are deleted from the shash when they
reach the state - CT_ZONE_DB_SENT and when
ovsdb_idl_loop_commit_and_wait(ovsidl) returns 1. In a highly loaded
compute node this loop gets triggered when this function doesn't return
1 and there are duplicate ct zone entries.
These duplicate entries are mostly observed for logical ports of
type 'virtual' when this virtual port keeps moving from one chassis
to another. But this scenario can get triggered for other logical ports
too.
*********************
2021-02-12T17:25:56.974Z|04363|jsonrpc|DBG|unix:/run/openvswitch/db.sock: send
request, method="transact",
params=["Open_vSwitch",{"mutations":[["external_ids","delete",["set",["ct-zone-313f5395-5170-4c4c-8820-e6288a765d27","ct-zone-efa8530b-93d5-45b3-b16f-edfe1856233c"]]],["external_ids","insert",["map",[["ct-zone-da4a2f90-aef9-4fea-ad6d-aed364fa9988","63"],["ct-zone-7a97e014-a753-473b-a0f3-b2ec6e9f11d6","71"]]]]],"where":[["_uuid","==",["uuid","653e7315-47b3-4c39-a5f9-665aa3dddb9e"]]],"op":"mutate","table":"Bridge"},{"comment":"ovn-controller\novn-controller:
modifying OVS tunnels
'e3af60d7-3942-4aa2-84ad-e02dcd3b183d'","op":"comment"}], id=4336
2021-02-12T17:25:56.979Z|04364|jsonrpc|DBG|unix:/run/openvswitch/db.sock:
received notification, method="update3",
params=[["monid","Open_vSwitch"],"00000000-0000-0000-0000-000000000000",{"Bridge":{"653e7315-47b3-4c39-a5f9-665aa3dddb9e":{"modify":{"external_ids":["map",[["ct-zone-313f5395-5170-4c4c-8820-e6288a765d27","71"],["ct-zone-7a97e014-a753-473b-a0f3-b2ec6e9f11d6","71"],["ct-zone-da4a2f90-aef9-4fea-ad6d-aed364fa9988","63"],["ct-zone-efa8530b-93d5-45b3-b16f-edfe1856233c","63"]]]}}}}]
2021-02-12T17:25:56.988Z|04365|jsonrpc|DBG|unix:/run/openvswitch/db.sock:
received reply, result=[{"count":1},{}], id=4336
2021-02-12T17:25:57.006Z|04366|jsonrpc|DBG|unix:/run/openvswitch/db.sock: send
request, method="transact",
params=["Open_vSwitch",{"mutations":[["external_ids","delete",["set",["ct-zone-da4a2f90-aef9-4fea-ad6d-aed364fa9988","ct-zone-7a97e014-a753-473b-a0f3-b2ec6e9f11d6"]]],["external_ids","insert",["map",[["ct-zone-313f5395-5170-4c4c-8820-e6288a765d27","71"],["ct-zone-efa8530b-93d5-45b3-b16f-edfe1856233c","63"]]]]],"where":[["_uuid","==",["uuid","653e7315-47b3-4c39-a5f9-665aa3dddb9e"]]],"op":"mutate","table":"Bridge"},{"comment":"ovn-controller\novn-controller:
modifying OVS tunnels
'e3af60d7-3942-4aa2-84ad-e02dcd3b183d'","op":"comment"}], id=4337
2021-02-12T17:25:57.011Z|04367|jsonrpc|DBG|unix:/run/openvswitch/db.sock:
received notification, method="update3",
params=[["monid","Open_vSwitch"],"00000000-0000-0000-0000-000000000000",{"Bridge":{"653e7315-47b3-4c39-a5f9-665aa3dddb9e":{"modify":{"external_ids":["map",[["ct-zone-313f5395-5170-4c4c-8820-e6288a765d27","71"],["ct-zone-7a97e014-a753-473b-a0f3-b2ec6e9f11d6","71"],["ct-zone-da4a2f90-aef9-4fea-ad6d-aed364fa9988","63"],["ct-zone-efa8530b-93d5-45b3-b16f-edfe1856233c","63"]]]}}}}]
...
...
*************************
This patch fixes this issue by using shash_replace() when adding the
entry to the shash.
Note: I was not able to reproduce the issue with a test setup and
hence couldn't add test cases.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1903210
Signed-off-by: Numan Siddique <[email protected]>
---
controller/ovn-controller.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 61b809593..4343650fc 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -608,7 +608,18 @@ add_pending_ct_zone_entry(struct shash *pending_ct_zones,
pending->state = state; /* Skip flushing zone. */
pending->zone = zone;
pending->add = add;
- shash_add(pending_ct_zones, name, pending);
+
+ /* Its important that we add only one entry for the key 'name'.
+ * Replace 'pending' with 'existing' and free up 'existing'.
+ * Otherwise, we may end up in a continuous loop of adding
+ * and deleting the zone entry in the 'external_ids' of
+ * integration bridge.
+ */
+ struct ct_zone_pending_entry *existing =
+ shash_replace(pending_ct_zones, name, pending);
+ if (existing) {
+ free(existing);
+ }
}
static void
--
2.29.2
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
