| Message ID | 20200608135009.1378992-1-numans@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Incremental processing improvements. | expand |
On 6/8/20 3:50 PM, numans@ovn.org wrote: > From: Numan Siddique <numans@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@redhat.com> > Acked-by: Han Zhou <hzhou@ovn.org> > Signed-off-by: Numan Siddique <numans@ovn.org> Looks good to me. Acked-by: Dumitru Ceara <dceara@redhat.com> Thanks, Dumitru > --- > controller/ovn-controller.c | 25 ++++++++++++++++++++++++- > tests/ovn-performance.at | 6 +++--- > 2 files changed, 27 insertions(+), 4 deletions(-) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 871291874..687a33c88 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -1203,6 +1203,28 @@ runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) > return true; > } > > +static bool > +runtime_data_sb_datapath_binding_handler(struct engine_node *node OVS_UNUSED, > + void *data OVS_UNUSED) > +{ > + struct sbrec_datapath_binding_table *dp_table = > + (struct sbrec_datapath_binding_table *)EN_OVSDB_GET( > + engine_get_input("SB_datapath_binding", node)); > + const struct sbrec_datapath_binding *dp; > + struct ed_type_runtime_data *rt_data = data; > + > + SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (dp, dp_table) { > + if (sbrec_datapath_binding_is_deleted(dp)) { > + if (get_local_datapath(&rt_data->local_datapaths, > + dp->tunnel_key)) { > + return false; > + } > + } > + } > + > + return true; > +} > + > /* Connection tracking zones. */ > struct ed_type_ct_zones { > unsigned long bitmap[BITMAP_N_LONGS(MAX_CT_ZONES)]; > @@ -1951,7 +1973,8 @@ main(int argc, char *argv[]) > engine_add_input(&en_runtime_data, &en_ovs_qos, NULL); > > engine_add_input(&en_runtime_data, &en_sb_chassis, NULL); > - engine_add_input(&en_runtime_data, &en_sb_datapath_binding, NULL); > + engine_add_input(&en_runtime_data, &en_sb_datapath_binding, > + runtime_data_sb_datapath_binding_handler); > engine_add_input(&en_runtime_data, &en_sb_port_binding, > runtime_data_sb_port_binding_handler); > > diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at > index 5dfc6f7ca..5cc1960b6 100644 > --- a/tests/ovn-performance.at > +++ b/tests/ovn-performance.at > @@ -253,7 +253,7 @@ grep tunnel_egress_iface_carrier=up | wc -l) -eq 1 > ]) > > # Add router lr1 > -OVN_CONTROLLER_EXPECT_HIT( > +OVN_CONTROLLER_EXPECT_NO_HIT( > [hv1 hv2], [lflow_run], > [ovn-nbctl --wait=hv lr-add lr1] > ) > @@ -264,7 +264,7 @@ for i in 1 2; do > lrp=lr1-$ls > > # Add switch $ls > - OVN_CONTROLLER_EXPECT_HIT( > + OVN_CONTROLLER_EXPECT_NO_HIT( > [hv1 hv2], [lflow_run], > [ovn-nbctl --wait=hv ls-add $ls] > ) > @@ -427,7 +427,7 @@ for i in 1 2; do > done > > # Delete router lr1 > -OVN_CONTROLLER_EXPECT_HIT( > +OVN_CONTROLLER_EXPECT_NO_HIT( > [hv1 hv2], [lflow_run], > [ovn-nbctl --wait=hv lr-del lr1] > ) >
On Mon, Jun 8, 2020 at 9:39 PM Dumitru Ceara <dceara@redhat.com> wrote: > On 6/8/20 3:50 PM, numans@ovn.org wrote: > > From: Numan Siddique <numans@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@redhat.com> > > Acked-by: Han Zhou <hzhou@ovn.org> > > Signed-off-by: Numan Siddique <numans@ovn.org> > > Looks good to me. > > Acked-by: Dumitru Ceara <dceara@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 > Thanks, > Dumitru > > > --- > > controller/ovn-controller.c | 25 ++++++++++++++++++++++++- > > tests/ovn-performance.at | 6 +++--- > > 2 files changed, 27 insertions(+), 4 deletions(-) > > > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > > index 871291874..687a33c88 100644 > > --- a/controller/ovn-controller.c > > +++ b/controller/ovn-controller.c > > @@ -1203,6 +1203,28 @@ runtime_data_sb_port_binding_handler(struct > engine_node *node, void *data) > > return true; > > } > > > > +static bool > > +runtime_data_sb_datapath_binding_handler(struct engine_node *node > OVS_UNUSED, > > + void *data OVS_UNUSED) > > +{ > > + struct sbrec_datapath_binding_table *dp_table = > > + (struct sbrec_datapath_binding_table *)EN_OVSDB_GET( > > + engine_get_input("SB_datapath_binding", node)); > > + const struct sbrec_datapath_binding *dp; > > + struct ed_type_runtime_data *rt_data = data; > > + > > + SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (dp, dp_table) { > > + if (sbrec_datapath_binding_is_deleted(dp)) { > > + if (get_local_datapath(&rt_data->local_datapaths, > > + dp->tunnel_key)) { > > + return false; > > + } > > + } > > + } > > + > > + return true; > > +} > > + > > /* Connection tracking zones. */ > > struct ed_type_ct_zones { > > unsigned long bitmap[BITMAP_N_LONGS(MAX_CT_ZONES)]; > > @@ -1951,7 +1973,8 @@ main(int argc, char *argv[]) > > engine_add_input(&en_runtime_data, &en_ovs_qos, NULL); > > > > engine_add_input(&en_runtime_data, &en_sb_chassis, NULL); > > - engine_add_input(&en_runtime_data, &en_sb_datapath_binding, NULL); > > + engine_add_input(&en_runtime_data, &en_sb_datapath_binding, > > + runtime_data_sb_datapath_binding_handler); > > engine_add_input(&en_runtime_data, &en_sb_port_binding, > > runtime_data_sb_port_binding_handler); > > > > diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at > > index 5dfc6f7ca..5cc1960b6 100644 > > --- a/tests/ovn-performance.at > > +++ b/tests/ovn-performance.at > > @@ -253,7 +253,7 @@ grep tunnel_egress_iface_carrier=up | wc -l) -eq 1 > > ]) > > > > # Add router lr1 > > -OVN_CONTROLLER_EXPECT_HIT( > > +OVN_CONTROLLER_EXPECT_NO_HIT( > > [hv1 hv2], [lflow_run], > > [ovn-nbctl --wait=hv lr-add lr1] > > ) > > @@ -264,7 +264,7 @@ for i in 1 2; do > > lrp=lr1-$ls > > > > # Add switch $ls > > - OVN_CONTROLLER_EXPECT_HIT( > > + OVN_CONTROLLER_EXPECT_NO_HIT( > > [hv1 hv2], [lflow_run], > > [ovn-nbctl --wait=hv ls-add $ls] > > ) > > @@ -427,7 +427,7 @@ for i in 1 2; do > > done > > > > # Delete router lr1 > > -OVN_CONTROLLER_EXPECT_HIT( > > +OVN_CONTROLLER_EXPECT_NO_HIT( > > [hv1 hv2], [lflow_run], > > [ovn-nbctl --wait=hv lr-del lr1] > > ) > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
On 6/9/20 7:10 PM, Numan Siddique wrote: > > > On Mon, Jun 8, 2020 at 9:39 PM Dumitru Ceara <dceara@redhat.com > <mailto:dceara@redhat.com>> wrote: > > On 6/8/20 3:50 PM, numans@ovn.org <mailto:numans@ovn.org> wrote: > > From: Numan Siddique <numans@ovn.org <mailto:numans@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@redhat.com > <mailto:mmichels@redhat.com>> > > Acked-by: Han Zhou <hzhou@ovn.org <mailto:hzhou@ovn.org>> > > Signed-off-by: Numan Siddique <numans@ovn.org <mailto:numans@ovn.org>> > > Looks good to me. > > Acked-by: Dumitru Ceara <dceara@redhat.com <mailto:dceara@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 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 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 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 Regards, Dumitru
On Wed, Jun 10, 2020 at 3:14 PM Dumitru Ceara <dceara@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@redhat.com > > <mailto:dceara@redhat.com>> wrote: > > > > On 6/8/20 3:50 PM, numans@ovn.org <mailto:numans@ovn.org> wrote: > > > From: Numan Siddique <numans@ovn.org <mailto:numans@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@redhat.com > > <mailto:mmichels@redhat.com>> > > > Acked-by: Han Zhou <hzhou@ovn.org <mailto:hzhou@ovn.org>> > > > Signed-off-by: Numan Siddique <numans@ovn.org <mailto: > numans@ovn.org>> > > > > Looks good to me. > > > > Acked-by: Dumitru Ceara <dceara@redhat.com <mailto:dceara@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 > 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 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 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. Thanks Numan > Regards, > Dumitru > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Wed, Jun 10, 2020 at 3:31 PM Numan Siddique <numans@ovn.org> wrote: > > > On Wed, Jun 10, 2020 at 3:14 PM Dumitru Ceara <dceara@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@redhat.com >> > <mailto:dceara@redhat.com>> wrote: >> > >> > On 6/8/20 3:50 PM, numans@ovn.org <mailto:numans@ovn.org> wrote: >> > > From: Numan Siddique <numans@ovn.org <mailto:numans@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@redhat.com >> > <mailto:mmichels@redhat.com>> >> > > Acked-by: Han Zhou <hzhou@ovn.org <mailto:hzhou@ovn.org>> >> > > Signed-off-by: Numan Siddique <numans@ovn.org <mailto: >> numans@ovn.org>> >> > >> > Looks good to me. >> > >> > Acked-by: Dumitru Ceara <dceara@redhat.com <mailto: >> dceara@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 >> 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 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 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. Thanks Numan > Thanks > Numan > > >> Regards, >> Dumitru >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >
On 6/10/20 12:41 PM, Numan Siddique wrote: > > > On Wed, Jun 10, 2020 at 3:31 PM Numan Siddique <numans@ovn.org > <mailto:numans@ovn.org>> wrote: > > > > On Wed, Jun 10, 2020 at 3:14 PM Dumitru Ceara <dceara@redhat.com > <mailto:dceara@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@redhat.com <mailto:dceara@redhat.com> > > <mailto:dceara@redhat.com <mailto:dceara@redhat.com>>> wrote: > > > > On 6/8/20 3:50 PM, numans@ovn.org <mailto:numans@ovn.org> > <mailto:numans@ovn.org <mailto:numans@ovn.org>> wrote: > > > From: Numan Siddique <numans@ovn.org > <mailto:numans@ovn.org> <mailto:numans@ovn.org > <mailto:numans@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@redhat.com > <mailto:mmichels@redhat.com> > > <mailto:mmichels@redhat.com <mailto:mmichels@redhat.com>>> > > > Acked-by: Han Zhou <hzhou@ovn.org <mailto:hzhou@ovn.org> > <mailto:hzhou@ovn.org <mailto:hzhou@ovn.org>>> > > > Signed-off-by: Numan Siddique <numans@ovn.org > <mailto:numans@ovn.org> <mailto:numans@ovn.org > <mailto:numans@ovn.org>>> > > > > Looks good to me. > > > > Acked-by: Dumitru Ceara <dceara@redhat.com > <mailto:dceara@redhat.com> <mailto:dceara@redhat.com > <mailto:dceara@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@openvswitch.org <mailto:dev@openvswitch.org> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 871291874..687a33c88 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1203,6 +1203,28 @@ runtime_data_sb_port_binding_handler(struct engine_node *node, void *data) return true; } +static bool +runtime_data_sb_datapath_binding_handler(struct engine_node *node OVS_UNUSED, + void *data OVS_UNUSED) +{ + struct sbrec_datapath_binding_table *dp_table = + (struct sbrec_datapath_binding_table *)EN_OVSDB_GET( + engine_get_input("SB_datapath_binding", node)); + const struct sbrec_datapath_binding *dp; + struct ed_type_runtime_data *rt_data = data; + + SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (dp, dp_table) { + if (sbrec_datapath_binding_is_deleted(dp)) { + if (get_local_datapath(&rt_data->local_datapaths, + dp->tunnel_key)) { + return false; + } + } + } + + return true; +} + /* Connection tracking zones. */ struct ed_type_ct_zones { unsigned long bitmap[BITMAP_N_LONGS(MAX_CT_ZONES)]; @@ -1951,7 +1973,8 @@ main(int argc, char *argv[]) engine_add_input(&en_runtime_data, &en_ovs_qos, NULL); engine_add_input(&en_runtime_data, &en_sb_chassis, NULL); - engine_add_input(&en_runtime_data, &en_sb_datapath_binding, NULL); + engine_add_input(&en_runtime_data, &en_sb_datapath_binding, + runtime_data_sb_datapath_binding_handler); engine_add_input(&en_runtime_data, &en_sb_port_binding, runtime_data_sb_port_binding_handler); diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at index 5dfc6f7ca..5cc1960b6 100644 --- a/tests/ovn-performance.at +++ b/tests/ovn-performance.at @@ -253,7 +253,7 @@ grep tunnel_egress_iface_carrier=up | wc -l) -eq 1 ]) # Add router lr1 -OVN_CONTROLLER_EXPECT_HIT( +OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lr-add lr1] ) @@ -264,7 +264,7 @@ for i in 1 2; do lrp=lr1-$ls # Add switch $ls - OVN_CONTROLLER_EXPECT_HIT( + OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv ls-add $ls] ) @@ -427,7 +427,7 @@ for i in 1 2; do done # Delete router lr1 -OVN_CONTROLLER_EXPECT_HIT( +OVN_CONTROLLER_EXPECT_NO_HIT( [hv1 hv2], [lflow_run], [ovn-nbctl --wait=hv lr-del lr1] )