Numan Siddique <nusid...@redhat.com> wrote on 08/09/2016 09:39:21 AM:

> From: Numan Siddique <nusid...@redhat.com>
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: ovs dev <dev@openvswitch.org>
> Date: 08/09/2016 09:39 AM
> Subject: Re: [ovs-dev] [PATCH] ovn-controller: Reset flow processing
> after (re)connection to switch
>
> On Tue, Aug 9, 2016 at 7:15 PM, Ryan Moats <rmo...@us.ibm.com> wrote:
> "dev" <dev-boun...@openvswitch.org> wrote on 08/09/2016 07:19:27 AM:
>
> > From: Numan Siddique <nusid...@redhat.com>
> > To: ovs dev <dev@openvswitch.org>
> > Date: 08/09/2016 07:19 AM
> > Subject: [ovs-dev] [PATCH] ovn-controller: Reset flow processing
> > after (re)connection to switch
> > Sent by: "dev" <dev-boun...@openvswitch.org>
> >
> > When ovn-controller reconnects to the ovs-vswitchd, it deletes all the
> > OF flows in the switch. It doesn't install the flows again, leaving
> > the datapath broken unless ovn-controller is restarted or ovn-northd
> > updates the SB DB.
> >
> > The reason for this is
> >   - lflow_reset_processing() is not called after the reconnection
> >   - the hmap "installed_flows" is not cleared, because of which
> >     ofctrl_put skips adding the flows to the switch.
> >
> > This patch fixes the issue and also adds a test case to test
> > this scenario.
> >
> > Signed-off-by: Numan Siddique <nusid...@redhat.com>
> > ---
>
> I'm going to pick a nit on this one - is the behavior you are aiming
> for delete and re-add or just recalculate and leave alone?

>
> ​In my testing I am seeing that all the OF flows are getting de​
> leted when I restart ovs-vswitchd.
> I am testing with the latest master of OVS. I am able to see this on
> 2 different machines and also in sandbox.
>
> I thought that ovn-controller is deleting the flows in the switch
> when it restarts (https://github.com/openvswitch/ovs/blob/master/
> ovn/controller/ofctrl.c#L355)
>
> Now I tested again and before restarting ovs-vswitchd, I killed ovn-
> controller. Looks like ovs-vswitchd is clearing the old flows when
> it restarts. I am not sure if this is the intended behavior. Looks
> like it is. Please correct me if I am wrong here.
>
> ​You can run below commands to reproduce the issue in sandbox​
>
> -----------------------------
>  $make sandbox SANDBOXFLAGS="--ovn"
>  $ovn/env1/setup.sh
>  $ovs-ofctl dump-flows br-int
>  $ovs-appctl -t ovn-controller exit
>  $ovs-appctl -t ovs-vswitchd exit
> ​ $ovs-vswitchd --detach --no-chdir --pidfile -vconsole:off --log-
> file --enable-dummy=override -vvconn -vnetdev_dummy
>  $ ovs-ofctl dump-flows br-int
> NXST_FLOW reply (xid=0x4):
> ------------------------------------
>
> You will see that the flows are deleted even if you don't run -
> "ovs-appctl -t ovn-controller exit".
>
> I ask because if it is "delete and re-add" aren't you still disrupting
> the datapath even if only momentarily?
>

Ok, so we'll assume that your code is valid for when ovswitchd purges
the old flows and that's good.

IIRC there is a way to restart ovswitchd via ovs_ctl so that it doesn't
purge the old flows.  Since I'll argue (as an operator) that is the more
important case, can you add a unit test for this and verify that your
patch doesn't leave that path broken?

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to