Thanks Xavier,
Acked-by: Mark Michelson <[email protected]>
In the interest of getting 22.09 released today, I have merged this to
main, branch-22.09, branch-22.06, and branch-22.03 already.
On 9/16/22 04:41, Xavier Simonart wrote:
When SB is read-only when a port needs to be claimed, the claim of the port
is delayed until SB becomes writable.
However, if that port gets deleted while ovn-controller is waiting for SB to
become writable, the claim will crash ovn-controller
Signed-off-by: Xavier Simonart <[email protected]>
---
controller/ovn-controller.c | 3 +-
tests/ovn.at | 92 +++++++++++++++++++++++++++++++++++++
2 files changed, 94 insertions(+), 1 deletion(-)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 89a495a04..07e9b07cf 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -3799,7 +3799,6 @@ main(int argc, char *argv[])
engine_add_input(&en_ovs_interface_shadow, &en_ovs_interface,
ovs_interface_shadow_ovs_interface_handler);
- engine_add_input(&en_runtime_data, &en_sb_ro, runtime_data_sb_ro_handler);
engine_add_input(&en_runtime_data, &en_ofctrl_is_connected, NULL);
engine_add_input(&en_runtime_data, &en_ovs_open_vswitch, NULL);
@@ -3814,6 +3813,8 @@ main(int argc, char *argv[])
/* Reuse the same handler for any previously postponed ports. */
engine_add_input(&en_runtime_data, &en_postponed_ports,
runtime_data_sb_port_binding_handler);
+ /* Run sb_ro_handler after port_binding_handler in case port get deleted */
+ engine_add_input(&en_runtime_data, &en_sb_ro, runtime_data_sb_ro_handler);
/* The OVS interface handler for runtime_data changes MUST be executed
* after the sb_port_binding_handler as port_binding deletes must be
diff --git a/tests/ovn.at b/tests/ovn.at
index 4f4783461..80e9192ca 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -32797,3 +32797,95 @@ check ovn-nbctl lrp-del lr0-sw1
OVN_CLEANUP([hv1],[hv2])
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-controller: deleting a port being claimed])
+ovn_start
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+check ovs-vsctl add-port br-int p0
+check ovs-vsctl add-port br-int p1
+
+ovn-nbctl ls-add sw0
+ovn-nbctl lsp-add sw0 sw0-port0 -- lsp-set-addresses sw0-port0 "50:54:00:00:00:01
192.168.0.2"
+ovn-nbctl lsp-add sw0 sw0-port1 -- lsp-set-addresses sw0-port1 "50:54:00:00:00:02
192.168.0.3"
+check ovn-nbctl --wait=hv sync
+
+# Pause SB
+AT_CHECK([kill -STOP $(cat ovn-sb/ovsdb-server.pid)])
+
+# Make us claim psw0-port0. This will make SB ro
+ovs-vsctl set interface p0 external-ids:iface-id=sw0-port0
+sleep 0.5
+# Make us claim sw0-port1. Claim should be delayed
+ovs-vsctl set interface p1 external-ids:iface-id=sw0-port1
+# Delete sw0-port1
+ovn-nbctl lsp-del sw0-port1
+sleep 0.5
+# Restart SB
+AT_CHECK([kill -CONT $(cat ovn-sb/ovsdb-server.pid)])
+check ovn-nbctl --wait=hv sync
+ovn-nbctl ls-del sw0
+check ovn-nbctl --wait=hv sync
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-controller: checking flows after delaying port claims])
+ovn_start
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+check ovs-vsctl add-port br-int p0
+check ovs-vsctl add-port br-int p1
+
+ovn-nbctl ls-add sw0
+ovn-nbctl lsp-add sw0 sw0-port0 -- lsp-set-addresses sw0-port0 "50:54:00:00:00:01
192.168.0.2"
+check ovn-nbctl --wait=hv sync
+ovn-nbctl lsp-add sw0 sw0-port1 -- lsp-set-addresses sw0-port1 "50:54:00:00:00:02
192.168.0.3"
+ovs-vsctl set interface p0 external-ids:iface-id=sw0-port0
+ovs-vsctl set interface p1 external-ids:iface-id=sw0-port1
+check ovn-nbctl --wait=hv sync
+as hv1 ovs-ofctl dump-flows br-int | sed 's/cookie=0x.*, duration=.*, table/cookie=??,
duration=??, table/' | sed 's/load:0x.->NXM_NX_REG1/load:0x?->NXM_NX_REG1/g' | sed
's/idle_age=[[0-9]], //g' | sort > offlows1
+
+ovs-vsctl set interface p0 external-ids:iface-id=foo0
+ovs-vsctl set interface p1 external-ids:iface-id=foo1
+ovn-nbctl lsp-del sw0-port0
+ovn-nbctl lsp-del sw0-port1
+ovn-nbctl ls-del sw0
+check ovn-nbctl --wait=hv sync
+
+ovn-nbctl ls-add sw0
+ovn-nbctl lsp-add sw0 sw0-port0 -- lsp-set-addresses sw0-port0 "50:54:00:00:00:01
192.168.0.2"
+check ovn-nbctl --wait=hv sync
+ovn-nbctl lsp-add sw0 sw0-port1 -- lsp-set-addresses sw0-port1 "50:54:00:00:00:02
192.168.0.3"
+check ovn-nbctl --wait=hv sync
+
+# Pause SB
+AT_CHECK([kill -STOP $(cat ovn-sb/ovsdb-server.pid)])
+
+# Make us claim psw0-port0. This will make SB ro
+ovs-vsctl set interface p0 external-ids:iface-id=sw0-port0
+sleep 0.5
+# Make us claim sw0-port1. Claim should be delayed
+ovs-vsctl set interface p1 external-ids:iface-id=sw0-port1
+sleep 0.5
+# Restart SB
+AT_CHECK([kill -CONT $(cat ovn-sb/ovsdb-server.pid)])
+check ovn-nbctl --wait=hv sync
+as hv1 ovs-ofctl dump-flows br-int | sed 's/cookie=0x.*, duration=.*, table/cookie=??,
duration=??, table/' | sed 's/load:0x.->NXM_NX_REG1/load:0x?->NXM_NX_REG1/g' | sed
's/idle_age=[[0-9]], //g' | sort > offlows2
+AT_CHECK([diff offlows1 offlows2])
+
+ovn-nbctl ls-del sw0
+check ovn-nbctl --wait=hv sync
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
