Hi Numan, On Tue, 20 Mar 2018 13:06:33 +0530 nusid...@redhat.com wrote:
> 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> > --- > > v1 -> v2 : Updated the commit message with more details > > 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..84113ebd2 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 && > + strcmp(requested_chassis, chassis->name) && > + strcmp(requested_chassis, chassis->hostname)) { Is it possible to get an unintended match here on an empty string? We never should, right? But it seems ->hostname could be empty if things go wrong with gethostname() in chassis_run(). It caught my attention because in the only other place where we match on "requested-chassis", in consider_local_datapath(), there is a check to rule out the empty string: const char *vif_chassis = smap_get(&binding_rec->options, "requested-chassis"); bool can_bind = !vif_chassis || !vif_chassis[0] || !strcmp(vif_chassis, chassis_rec->name) || !strcmp(vif_chassis, chassis_rec->hostname); WDYT? -Jakub > + /* 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) { _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
