[ovs-dev] [PATCH ovn v10 3/8] ovn-controller: I-P for datapath binding

Wed Jun 10 16:50:11 UTC 2020

On 6/10/20 12:41 PM, Numan Siddique wrote:
> 
> 
> On Wed, Jun 10, 2020 at 3:31 PM Numan Siddique <numans at ovn.org
> <mailto:numans at ovn.org>> wrote:
> 
> 
> 
>     On Wed, Jun 10, 2020 at 3:14 PM Dumitru Ceara <dceara at redhat.com
>     <mailto:dceara at redhat.com>> wrote:
> 
>         On 6/9/20 7:10 PM, Numan Siddique wrote:
>         >
>         >
>         > On Mon, Jun 8, 2020 at 9:39 PM Dumitru Ceara
>         <dceara at redhat.com <mailto:dceara at redhat.com>
>         > <mailto:dceara at redhat.com <mailto:dceara at redhat.com>>> wrote:
>         >
>         >     On 6/8/20 3:50 PM, numans at ovn.org <mailto:numans at ovn.org>
>         <mailto:numans at ovn.org <mailto:numans at ovn.org>> wrote:
>         >     > From: Numan Siddique <numans at ovn.org
>         <mailto:numans at ovn.org> <mailto:numans at ovn.org
>         <mailto:numans at ovn.org>>>
>         >     >
>         >     > This patch adds partial support of incremental processing of
>         >     datapath binding.
>         >     > If a datapath is deleted, then a full recompute is
>         triggered if that
>         >     > datapath is present in the 'local_datapaths' hmap of
>         runtime data.
>         >     >
>         >     > Acked-by: Mark Michelson <mmichels at redhat.com
>         <mailto:mmichels at redhat.com>
>         >     <mailto:mmichels at redhat.com <mailto:mmichels at redhat.com>>>
>         >     > Acked-by: Han Zhou <hzhou at ovn.org <mailto:hzhou at ovn.org>
>         <mailto:hzhou at ovn.org <mailto:hzhou at ovn.org>>>
>         >     > Signed-off-by: Numan Siddique <numans at ovn.org
>         <mailto:numans at ovn.org> <mailto:numans at ovn.org
>         <mailto:numans at ovn.org>>>
>         >
>         >     Looks good to me.
>         >
>         >     Acked-by: Dumitru Ceara <dceara at redhat.com
>         <mailto:dceara at redhat.com> <mailto:dceara at redhat.com
>         <mailto:dceara at redhat.com>>>
>         >
>         >
>         >
>         > Thanks Dumitru, Han and Mark  for the reviews. 
>         >
>         > I applied the first 3 patches of this series (addressing the
>         review
>         > comments) to master and also applied to branch-20.06.
>         >
>         > @Han - If you have any additional comments on these patches
>         please let
>         > me know. I'll have follow up patches.
>         >
>         > I'll update v11 of this series addressing the review comments
>         from Dumitru.
>         >
>         > Thanks
>         > Numan
>         >
> 
>         Hi Numan,
> 
>         I spotted a bug introduced by these 3 patches. The following
>         scenario is
>         now broken:
> 
>         ovn-nbctl lr-add rtr
>         ovn-nbctl lrp-add rtr rtr-ls 00:00:00:00:01:00 42.42.42.1/24
>         <http://42.42.42.1/24>
>         ovn-nbctl ls-add ls
>         ovn-nbctl lsp-add ls ls-rtr
>         ovn-nbctl lsp-set-addresses ls-rtr 00:00:00:00:01:00
>         ovn-nbctl lsp-set-type ls-rtr router
>         ovn-nbctl lsp-set-options ls-rtr router-port=rtr-ls
>         ovn-nbctl lsp-add ls vm1
>         ovn-nbctl lsp-set-addresses vm1 00:00:00:00:00:01
> 
>         ovn-nbctl lsp-add ls vm2
>         ovn-nbctl lsp-set-addresses vm2 00:00:00:00:00:02
> 
>         ip netns add vm1
>         ovs-vsctl add-port br-int vm1 -- set interface vm1 type=internal
>         ip link set vm1 netns vm1
>         ip netns exec vm1 ip link set vm1 address 00:00:00:00:00:01
>         ip netns exec vm1 ip addr add 42.42.42.2/24
>         <http://42.42.42.2/24> dev vm1
>         ip netns exec vm1 ip link set vm1 up
>         ovs-vsctl set Interface vm1 external_ids:iface-id=vm1
> 
>         ip netns add vm2
>         ovs-vsctl add-port br-int vm2 -- set interface vm2 type=internal
>         ip link set vm2 netns vm2
>         ip netns exec vm2 ip link set vm2 address 00:00:00:00:00:02
>         ip netns exec vm2 ip addr add 42.42.42.3/24
>         <http://42.42.42.3/24> dev vm2
>         ip netns exec vm2 ip link set vm2 up
>         ovs-vsctl set Interface vm2 external_ids:iface-id=vm2
> 
>         # Works
>         ip netns exec vm1 ping 42.42.42.3 -c 1
> 
>         # Restart ovn-controller
>         ovn-ctl restart_controller
> 
>         # Doesn't work
>         ip netns exec vm1 ping 42.42.42.3 -c 1
> 
>         # Delete port bindings
>         ovn-sbctl destroy port_binding vm1
>         ovn-sbctl destroy port_binding vm2
> 
>         # Works
>         ip netns exec vm1 ping 42.42.42.3 -c 1
> 
> 
>     Oops. Thanks for reporting this Dumitru.
> 
>     So when we restart, a full recompute should have been triggered.
>     Looks like full recompute is not  triggered, after the IDL contents
>     are received.
> 
> 
> As we discussed offline and the reason you pointed out that
> binding_handle_port_binding_changes()
> is not returning true when it processes for the port binding initial
> dump it received and hence not
> calling update_sb_monitors().
>  
> The issue is not seen with the v11 of the I-P series because it does
> return true. And also the issue is not
> seen with ovn-monitor-all is set.
> 
> But of course we should first fix this issue. Thanks for looking into it.
> 

Hi Numan,

I sent a patch to fix this issue:

https://patchwork.ozlabs.org/project/openvswitch/list/?series=182582

Once/if accepted this should also go to branch-20.06.

Regards,
Dumitru

> Thanks
> Numan
> 
> 
>     Thanks
>     Numan
> 
> 
>         Regards,
>         Dumitru
> 
>         _______________________________________________
>         dev mailing list
>         dev at openvswitch.org <mailto:dev at openvswitch.org>
>         https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 