From: Numan Siddique <nusid...@redhat.com> When a Logical_Switch_Port P's options is set with 'requested-chassis=hv1' and if the user has bound this logical port to two OVS interfaces each in different host (eg. hv1 and hv2), then ovn-controller in hv1 sets the P's Port_Binding.chassis to hv1 which is as expected. But on hv2, ovn-controller is adding OF flows in table 0 and table 65 for the OVS interface instead of considering 'P' as a remote port. When another logical port bound on hv2, pings to the logical port 'P', the packet gets delivered to hv2 OVS interface instead of hv1 OVS interface, which is wrong.
This scenario is most likely to happen when requested-chassis option is used by CMS during migration of a VM from one chassis to another. This patch fixes this issue by checking the Port_Binding's "requested-chassis" option in physical.c before adding the flows in table 0 an 65. Reported-by: Marcin Mirecki <mmire...@redhat.com> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-March/345266.html Signed-off-by: Numan Siddique <nusid...@redhat.com> Tested-by: Marcin Mirecki <mmire...@redhat.com> --- v2 -> v3: Addressed the review comment from Jakub Sitnicki v1 -> v2: Updated the commit message ovn/controller/physical.c | 11 +++++++++++ tests/ovn.at | 32 +++++++++++++++++++++++++++----- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c index 5a80e2cda..8c92c1d9b 100644 --- a/ovn/controller/physical.c +++ b/ovn/controller/physical.c @@ -466,6 +466,17 @@ consider_port_binding(struct controller_ctx *ctx, } else { ofport = u16_to_ofp(simap_get(&localvif_to_ofport, binding->logical_port)); + const char *requested_chassis = smap_get(&binding->options, + "requested-chassis"); + if (ofport && requested_chassis && requested_chassis[0] && + strcmp(requested_chassis, chassis->name) && + strcmp(requested_chassis, chassis->hostname)) { + /* Even though there is an ofport for this port_binding, it is + * requested on a different chassis. So ignore this ofport. + */ + ofport = 0; + } + if ((!strcmp(binding->type, "localnet") || !strcmp(binding->type, "l2gateway")) && ofport && binding->tag) { diff --git a/tests/ovn.at b/tests/ovn.at index 5f985f345..75b32ef1a 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -9112,13 +9112,15 @@ sim_add hv1 as hv1 ovs-vsctl add-br br-phys ovn_attach n1 br-phys 192.168.0.11 -ovs-vsctl -- add-port br-int hv1-vif0 +ovs-vsctl -- add-port br-int hv1-vif0 -- \ +set Interface hv1-vif0 ofport-request=1 sim_add hv2 as hv2 ovs-vsctl add-br br-phys ovn_attach n1 br-phys 192.168.0.12 -ovs-vsctl -- add-port br-int hv2-vif0 +ovs-vsctl -- add-port br-int hv2-vif0 -- \ +set Interface hv2-vif0 ofport-request=1 # Allow only chassis hv1 to bind logical port lsp0. ovn-nbctl lsp-set-options lsp0 requested-chassis=hv1 @@ -9138,7 +9140,11 @@ ovs-vsctl set interface hv2-vif0 external-ids:iface-id=lsp0 OVS_WAIT_UNTIL([test 1 = $(grep -c "Not claiming lport lsp0" hv2/ovn-controller.log)]) AT_CHECK([test x$(ovn-sbctl --bare --columns chassis find port_binding logical_port=lsp0) = x], [0], []) -# (2) Chassis hv1 should bind lsp0 when physical to logical mapping exists on hv1. +# (2) Chassis hv2 should not add flows in OFTABLE_PHY_TO_LOG and OFTABLE_LOG_TO_PHY tables. +AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=0 | grep in_port=1], [1], []) +AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=65 | grep output], [1], []) + +# (3) Chassis hv1 should bind lsp0 when physical to logical mapping exists on hv1. echo "verifying that hv1 binds lsp0 when hv1 physical/logical mapping is added" as hv1 ovs-vsctl set interface hv1-vif0 external-ids:iface-id=lsp0 @@ -9146,7 +9152,12 @@ ovs-vsctl set interface hv1-vif0 external-ids:iface-id=lsp0 OVS_WAIT_UNTIL([test 1 = $(grep -c "Claiming lport lsp0" hv1/ovn-controller.log)]) AT_CHECK([test $(ovn-sbctl --bare --columns chassis find port_binding logical_port=lsp0) = "$hv1_uuid"], [0], []) -# (3) Chassis hv1 should release lsp0 binding and chassis hv2 should bind lsp0 when +# (4) Chassis hv1 should add flows in OFTABLE_PHY_TO_LOG and OFTABLE_LOG_TO_PHY tables. +as hv1 ovs-ofctl dump-flows br-int +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=0 | grep in_port=1], [0], [ignore]) +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=65 | grep actions=output:1], [0], [ignore]) + +# (5) Chassis hv1 should release lsp0 binding and chassis hv2 should bind lsp0 when # the requested chassis for lsp0 is changed from hv1 to hv2. echo "verifying that lsp0 binding moves when requested-chassis is changed" @@ -9154,6 +9165,13 @@ ovn-nbctl lsp-set-options lsp0 requested-chassis=hv2 OVS_WAIT_UNTIL([test 1 = $(grep -c "Releasing lport lsp0 from this chassis" hv1/ovn-controller.log)]) OVS_WAIT_UNTIL([test $(ovn-sbctl --bare --columns chassis find port_binding logical_port=lsp0) = "$hv2_uuid"]) +# (6) Chassis hv2 should add flows and hv1 should not. +AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=0 | grep in_port=1], [0], [ignore]) +AT_CHECK([as hv2 ovs-ofctl dump-flows br-int table=65 | grep actions=output:1], [0], [ignore]) + +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=0 | grep in_port=1], [1], []) +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=65 | grep output], [1], []) + OVN_CLEANUP([hv1],[hv2]) AT_CLEANUP @@ -9170,7 +9188,7 @@ sim_add hv1 as hv1 ovs-vsctl add-br br-phys ovn_attach n1 br-phys 192.168.0.11 -ovs-vsctl -- add-port br-int hv1-vif0 +ovs-vsctl -- add-port br-int hv1-vif0 -- set Interface hv1-vif0 ofport-request=1 hv1_hostname=$(ovn-sbctl --bare --columns hostname find Chassis name=hv1) echo "hv1_hostname=${hv1_hostname}" @@ -9181,11 +9199,15 @@ hv1_uuid=$(ovn-sbctl --bare --columns _uuid find Chassis name=hv1) echo "hv1_uuid=${hv1_uuid}" OVS_WAIT_UNTIL([test 1 = $(grep -c "Claiming lport lsp0" hv1/ovn-controller.log)]) AT_CHECK([test x$(ovn-sbctl --bare --columns chassis find port_binding logical_port=lsp0) = x"$hv1_uuid"], [0], []) +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=0 | grep in_port=1], [0], [ignore]) +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=65 | grep actions=output:1], [0], [ignore]) ovn-nbctl --wait=hv --timeout=3 lsp-set-options lsp0 requested-chassis=non-existant-chassis OVS_WAIT_UNTIL([test 1 = $(grep -c "Releasing lport lsp0 from this chassis" hv1/ovn-controller.log)]) ovn-nbctl --wait=hv --timeout=3 sync AT_CHECK([test x$(ovn-sbctl --bare --columns chassis find port_binding logical_port=lsp0) = x], [0], []) +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=0 | grep in_port=1], [1], []) +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=65 | grep output], [1], []) OVN_CLEANUP([hv1]) -- 2.14.3 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
