On Tue, Mar 20, 2018 at 3:27 PM, Jakub Sitnicki <[email protected]> wrote:
> Hi Numan, > > On Tue, 20 Mar 2018 13:06:33 +0530 > [email protected] wrote: > > > From: Numan Siddique <[email protected]> > > > > 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 <[email protected]> > > Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-March/ > 345266.html > > > > Signed-off-by: Numan Siddique <[email protected]> > > --- > > > > 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? > Thanks for pointing it out. I agree. I will add the additional check. > -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 [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
