On Wed, Aug 10, 2016 at 6:07 AM, Numan Siddique <nusid...@redhat.com> wrote:
> On Aug 9, 2016 8:28 PM, "Ryan Moats" <rmo...@us.ibm.com> wrote: > > > > 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? > > > > > Sure. I will do that. Ryan - I tested using ovs-ctl and I could see that ovn-controller is indeed deleting all the flows when ever it reconnects to the vswitchd even when flows are restored by ovs-ctl ( https://github.com/openvswitch/ovs/blob/master/ovn/controller/ofctrl.c#L350 ) I also enhanced the test case as you suggested. The test case will fail when run without the fix in this patch. I will send out the v2 soon. Thanks > Numan > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
