On Wed, Apr 1, 2020 at 1:14 AM Han Zhou <hz...@ovn.org> wrote: > > On Tue, Mar 31, 2020 at 12:23 PM Numan Siddique <num...@ovn.org> wrote: > > > > > > Thanks Han for the comments. I will go through the comments and come back > to > > you with further comments or questions tomorrow. > > > > I have one comment on the last part. Please see below. > > > > Thanks > > Numan > > > > > > On Tue, Mar 31, 2020, 11:25 PM Han Zhou <hz...@ovn.org> wrote: > >> > >> On Tue, Mar 31, 2020 at 5:40 AM Numan Siddique <num...@ovn.org> wrote: > >> > > >> > On Wed, Mar 25, 2020 at 12:23 PM Han Zhou <hz...@ovn.org> wrote: > >> > > > >> > > On Mon, Mar 16, 2020 at 4:16 AM <num...@ovn.org> wrote: > >> > > > > >> > > > From: Numan Siddique <num...@ovn.org> > >> > > > > >> > > > This patch handles ct zone changes and OVS interface changes > >> incrementally > >> > > > in the flow output stage. > >> > > > > >> > > > Any changes to ct zone can be handled by running physical_run() > >> instead > >> > > of running > >> > > > flow_output_run(). And any changes to OVS interfaces can be either > >> handled > >> > > > incrementally (for OVS interfaces representing VIFs) or just > running > >> > > > physical_run() (for tunnel and other types of interfaces). > >> > > > > >> > > > To better handle this, a new engine node 'physical_flow_changes' is > >> added > >> > > which > >> > > > handles changes to ct zone and OVS interfaces. > >> > > > > >> > > > After this patch, We don't trigger full recompute in flow output > >> stage for > >> > > > runtime data changes. There's noop handler for this. With this we > >> avoid > >> > > > calls to lflow_run() a lot. > >> > > > > >> > > > For the below commands the lflow_run counter without this patch is > 16 > >> > > > and with this patch is 3. > >> > > > > >> > > > ----------------------- > >> > > > ovn-nbctl ls-add sw0 > >> > > > ovn-nbctl lsp-add sw0 sw0-port1 > >> > > > ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 > 192.168.0.2" > >> > > > > >> > > > ovn-nbctl ls-add sw1 > >> > > > ovn-nbctl lsp-add sw1 sw1-port1 > >> > > > ovn-nbctl lsp-set-addresses sw1-port1 "50:54:00:00:00:03 11.0.0.2" > >> > > > > >> > > > ovn-nbctl lr-add lr0 > >> > > > ovn-nbctl lrp-add lr0 lrp0 00:00:00:00:ff:01 192.168.0.1/24 > >> > > > ovn-nbctl lsp-add sw0 lrp0-attachment > >> > > > ovn-nbctl lsp-set-type lrp0-attachment router > >> > > > ovn-nbctl lsp-set-addresses lrp0-attachment 00:00:00:00:ff:01 > >> > > > ovn-nbctl lsp-set-options lrp0-attachment router-port=lrp0 > >> > > > ovn-nbctl lrp-add lr0 lrp1 00:00:00:00:ff:02 11.0.0.1/24 > >> > > > ovn-nbctl lsp-add sw1 lrp1-attachment > >> > > > ovn-nbctl lsp-set-type lrp1-attachment router > >> > > > ovn-nbctl lsp-set-addresses lrp1-attachment 00:00:00:00:ff:02 > >> > > > ovn-nbctl lsp-set-options lrp1-attachment router-port=lrp1 > >> > > > > >> > > > ovs-vsctl add-port br-int p1 -- \ > >> > > > set Interface p1 external_ids:iface-id=sw0-port1 > >> > > > ovs-vsctl add-port br-int p2 -- \ > >> > > > set Interface p2 external_ids:iface-id=sw1-port1 > >> > > > ovn-appctl -t ovn-controller coverage/read-counter lflow_run > >> > > > ------------------- > >> > > > > >> > > > Signed-off-by: Numan Siddique <num...@ovn.org> > >> > > > --- > >> > > > controller/binding.c | 22 --------- > >> > > > controller/lflow.c | 13 +++++ > >> > > > controller/lflow.h | 2 + > >> > > > controller/ovn-controller.c | 96 > >> ++++++++++++++++++++++++++++++++++++- > >> > > > controller/ovn-controller.h | 22 +++++++++ > >> > > > controller/physical.c | 48 +++++++++++++++++++ > >> > > > controller/physical.h | 5 +- > >> > > > tests/ovn-performance.at | 14 +++--- > >> > > > 8 files changed, 190 insertions(+), 32 deletions(-) > >> > > > > >> > > > diff --git a/controller/binding.c b/controller/binding.c > >> > > > index ce4467f31..9dc1c3f38 100644 > >> > > > --- a/controller/binding.c > >> > > > +++ b/controller/binding.c > >> > > > @@ -501,22 +501,6 @@ remove_local_lport_ids(const struct > >> > > sbrec_port_binding *binding_rec, > >> > > > sset_find_and_delete(local_lport_ids, buf); > >> > > > } > >> > > > > >> > > > -enum local_binding_type { > >> > > > - BT_VIF, > >> > > > - BT_CHILD, > >> > > > - BT_VIRTUAL > >> > > > -}; > >> > > > - > >> > > > -struct local_binding { > >> > > > - struct ovs_list node; /* In parent if any. */ > >> > > > - char *name; > >> > > > - enum local_binding_type type; > >> > > > - const struct ovsrec_interface *iface; > >> > > > - const struct sbrec_port_binding *pb; > >> > > > - > >> > > > - struct ovs_list children; > >> > > > -}; > >> > > > - > >> > > > static struct local_binding * > >> > > > local_binding_create(const char *name, const struct > ovsrec_interface > >> > > *iface, > >> > > > const struct sbrec_port_binding *pb, > >> > > > @@ -537,12 +521,6 @@ local_binding_add(struct shash > *local_bindings, > >> > > struct local_binding *lbinding) > >> > > > shash_add(local_bindings, lbinding->name, lbinding); > >> > > > } > >> > > > > >> > > > -static struct local_binding * > >> > > > -local_binding_find(struct shash *local_bindings, const char *name) > >> > > > -{ > >> > > > - return shash_find_data(local_bindings, name); > >> > > > -} > >> > > > - > >> > > > static void > >> > > > local_binding_destroy(struct local_binding *lbinding) > >> > > > { > >> > > > diff --git a/controller/lflow.c b/controller/lflow.c > >> > > > index 01214a3a6..512258cd3 100644 > >> > > > --- a/controller/lflow.c > >> > > > +++ b/controller/lflow.c > >> > > > @@ -847,3 +847,16 @@ lflow_destroy(void) > >> > > > expr_symtab_destroy(&symtab); > >> > > > shash_destroy(&symtab); > >> > > > } > >> > > > + > >> > > > +bool > >> > > > +lflow_evaluate_pb_changes(const struct sbrec_port_binding_table > >> > > *pb_table) > >> > > > +{ > >> > > > + const struct sbrec_port_binding *binding; > >> > > > + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding, pb_table) > { > >> > > > + if (!strcmp(binding->type, "remote")) { > >> > > > + return false; > >> > > > + } > >> > > > + } > >> > > > + > >> > > > + return true; > >> > > > +} > >> > > > diff --git a/controller/lflow.h b/controller/lflow.h > >> > > > index f02f709d7..fa54d4de2 100644 > >> > > > --- a/controller/lflow.h > >> > > > +++ b/controller/lflow.h > >> > > > @@ -48,6 +48,7 @@ struct sbrec_dhcp_options_table; > >> > > > struct sbrec_dhcpv6_options_table; > >> > > > struct sbrec_logical_flow_table; > >> > > > struct sbrec_mac_binding_table; > >> > > > +struct sbrec_port_binding_table; > >> > > > struct simap; > >> > > > struct sset; > >> > > > struct uuid; > >> > > > @@ -153,6 +154,7 @@ void lflow_handle_changed_neighbors( > >> > > > const struct hmap *local_datapaths, > >> > > > struct ovn_desired_flow_table *); > >> > > > > >> > > > +bool lflow_evaluate_pb_changes(const struct > sbrec_port_binding_table > >> *); > >> > > > void lflow_destroy(void); > >> > > > > >> > > > void lflow_expr_destroy(struct hmap *lflow_expr_cache); > >> > > > diff --git a/controller/ovn-controller.c > b/controller/ovn-controller.c > >> > > > index 07823f020..448e9908e 100644 > >> > > > --- a/controller/ovn-controller.c > >> > > > +++ b/controller/ovn-controller.c > >> > > > @@ -1360,6 +1360,10 @@ static void init_physical_ctx(struct > >> engine_node > >> > > *node, > >> > > > > >> > > > ovs_assert(br_int && chassis); > >> > > > > >> > > > + struct ovsrec_interface_table *iface_table = > >> > > > + (struct ovsrec_interface_table *)EN_OVSDB_GET( > >> > > > + engine_get_input("OVS_interface", node)); > >> > > > + > >> > > > struct ed_type_ct_zones *ct_zones_data = > >> > > > engine_get_input_data("ct_zones", node); > >> > > > struct simap *ct_zones = &ct_zones_data->current; > >> > > > @@ -1369,12 +1373,14 @@ static void init_physical_ctx(struct > >> engine_node > >> > > *node, > >> > > > p_ctx->mc_group_table = multicast_group_table; > >> > > > p_ctx->br_int = br_int; > >> > > > p_ctx->chassis_table = chassis_table; > >> > > > + p_ctx->iface_table = iface_table; > >> > > > p_ctx->chassis = chassis; > >> > > > p_ctx->active_tunnels = &rt_data->active_tunnels; > >> > > > p_ctx->local_datapaths = &rt_data->local_datapaths; > >> > > > p_ctx->local_lports = &rt_data->local_lports; > >> > > > p_ctx->ct_zones = ct_zones; > >> > > > p_ctx->mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve; > >> > > > + p_ctx->local_bindings = &rt_data->local_bindings; > >> > > > } > >> > > > > >> > > > static void init_lflow_ctx(struct engine_node *node, > >> > > > @@ -1637,6 +1643,8 @@ flow_output_sb_port_binding_handler(struct > >> > > engine_node *node, void *data) > >> > > > * always logical router port, and any change to logical > >> router > >> > > port > >> > > > * would just trigger recompute. > >> > > > * > >> > > > + * - When a port binding of type 'remote' is updated by > ovn-ic. > >> > > > + * > >> > > > * Although there is no correctness issue so far (except the > >> unusual > >> > > ACL > >> > > > * use case, which doesn't seem to be a real problem), it > might > >> be > >> > > better > >> > > > * to handle this more gracefully, without the need to > consider > >> these > >> > > > @@ -1647,6 +1655,14 @@ flow_output_sb_port_binding_handler(struct > >> > > engine_node *node, void *data) > >> > > > struct physical_ctx p_ctx; > >> > > > init_physical_ctx(node, rt_data, &p_ctx); > >> > > > > >> > > > + if (!lflow_evaluate_pb_changes(p_ctx.port_binding_table)) { > >> > > > + /* If this returns false, it means there is an impact on > >> > > > + * the logical flow processing because of some changes in > >> > > > + * port bindings. Return false so that recompute is > triggered > >> > > > + * for this stage. */ > >> > > > + return false; > >> > > > + } > >> > > > + > >> > > > physical_handle_port_binding_changes(&p_ctx, flow_table); > >> > > > > >> > > > engine_set_node_state(node, EN_UPDATED); > >> > > > @@ -1782,6 +1798,70 @@ flow_output_port_groups_handler(struct > >> engine_node > >> > > *node, void *data) > >> > > > return _flow_output_resource_ref_handler(node, data, > >> > > REF_TYPE_PORTGROUP); > >> > > > } > >> > > > > >> > > > +struct ed_type_physical_flow_changes { > >> > > > + bool ct_zones_changed; > >> > > > +}; > >> > > > + > >> > > > +static bool > >> > > > +flow_output_physical_flow_changes_handler(struct engine_node > *node, > >> void > >> > > *data) > >> > > > +{ > >> > > > + struct ed_type_physical_flow_changes *pfc = > >> > > > + engine_get_input_data("physical_flow_changes", node); > >> > > > + struct ed_type_runtime_data *rt_data = > >> > > > + engine_get_input_data("runtime_data", node); > >> > > > + > >> > > > + struct ed_type_flow_output *fo = data; > >> > > > + struct physical_ctx p_ctx; > >> > > > + init_physical_ctx(node, rt_data, &p_ctx); > >> > > > + > >> > > > + engine_set_node_state(node, EN_UPDATED); > >> > > > + if (pfc->ct_zones_changed) { > >> > > > + pfc->ct_zones_changed = false; > >> > > > + physical_run(&p_ctx, &fo->flow_table); > >> > > > + return true; > >> > > > + } > >> > > > + > >> > > > + return physical_handle_ovs_iface_changes(&p_ctx, > >> &fo->flow_table); > >> > > > +} > >> > > > + > >> > > > +static void * > >> > > > +en_physical_flow_changes_init(struct engine_node *node OVS_UNUSED, > >> > > > + struct engine_arg *arg OVS_UNUSED) > >> > > > +{ > >> > > > + struct ed_type_physical_flow_changes *data = xzalloc(sizeof > >> *data); > >> > > > + return data; > >> > > > +} > >> > > > + > >> > > > +static void > >> > > > +en_physical_flow_changes_cleanup(void *data OVS_UNUSED) > >> > > > +{ > >> > > > + > >> > > > +} > >> > > > + > >> > > > +static void > >> > > > +en_physical_flow_changes_run(struct engine_node *node, void *data > >> > > OVS_UNUSED) > >> > > > +{ > >> > > > + engine_set_node_state(node, EN_UPDATED); > >> > > > +} > >> > > > + > >> > > > +static bool > >> > > > +physical_flow_changes_ct_zones_handler(struct engine_node *node > >> > > OVS_UNUSED, > >> > > > + void *data OVS_UNUSED) > >> > > > +{ > >> > > > + struct ed_type_physical_flow_changes *pfc = data; > >> > > > + pfc->ct_zones_changed = true; > >> > > > + engine_set_node_state(node, EN_UPDATED); > >> > > > + return false; > >> > > > +} > >> > > > + > >> > > > +static bool > >> > > > +flow_output_noop_handler(struct engine_node *node OVS_UNUSED, > >> > > > + void *data OVS_UNUSED) > >> > > > +{ > >> > > > + return true; > >> > > > +} > >> > > > + > >> > > > + > >> > > > struct ovn_controller_exit_args { > >> > > > bool *exiting; > >> > > > bool *restart; > >> > > > @@ -1904,6 +1984,7 @@ main(int argc, char *argv[]) > >> > > > ENGINE_NODE(runtime_data, "runtime_data"); > >> > > > ENGINE_NODE(mff_ovn_geneve, "mff_ovn_geneve"); > >> > > > ENGINE_NODE(ofctrl_is_connected, "ofctrl_is_connected"); > >> > > > + ENGINE_NODE(physical_flow_changes, "physical_flow_changes"); > >> > > > ENGINE_NODE(flow_output, "flow_output"); > >> > > > ENGINE_NODE(addr_sets, "addr_sets"); > >> > > > ENGINE_NODE(port_groups, "port_groups"); > >> > > > @@ -1923,16 +2004,27 @@ main(int argc, char *argv[]) > >> > > > engine_add_input(&en_port_groups, &en_sb_port_group, > >> > > > port_groups_sb_port_group_handler); > >> > > > > >> > > > + engine_add_input(&en_physical_flow_changes, &en_ct_zones, > >> > > > + physical_flow_changes_ct_zones_handler); > >> > > > + engine_add_input(&en_physical_flow_changes, &en_ovs_interface, > >> > > > + NULL); > >> > > > + > >> > > > engine_add_input(&en_flow_output, &en_addr_sets, > >> > > > flow_output_addr_sets_handler); > >> > > > engine_add_input(&en_flow_output, &en_port_groups, > >> > > > flow_output_port_groups_handler); > >> > > > - engine_add_input(&en_flow_output, &en_runtime_data, NULL); > >> > > > - engine_add_input(&en_flow_output, &en_ct_zones, NULL); > >> > > > + engine_add_input(&en_flow_output, &en_runtime_data, > >> > > > + flow_output_noop_handler); > >> > > > >> > > I think this breaks the dependency and would result in correctness > >> problem. > >> > > Because flow_output depends on runtime_data, which includes lots of > data > >> > > structures such as local_lports, local_datapaths, active_tunnels, > etc., > >> all > >> > > needed for flow compute to produce correct OVS flow output. Now with > the > >> > > noop handler, the changes in these data structures are ignored. How > >> could > >> > > we make sure the flows computed are still up to date? > >> > > > >> > > >> > Thanks for the comments Han. I get your point. > >> > > >> > Let's say we have data structures 'A' and 'B' in runtime data. And if > >> > events 'X' and 'Y' result in the modifications of these data > structures in > >> > runtime_data handlers, then handling (properly) these events - 'X' > >> > and 'Y' in the > >> > flow_output engine should be enough to ensure the correctness of the > >> > flows right ? > >> > > >> Well, this is not how the I-P engine supposed to work. The engine is > purely > >> data-driven. In your example, X or Y is just input data (another engine > >> node) change of runtime data. They need to be handled in runtime data > node. > >> If flow_output engine also directly depend on X or Y, of course > flow_output > >> node also need to handle the change of X or Y, in addition to handle the > >> change of runtime data. But handling X and Y in flow_output doesn't > replace > >> the handling in runtime data. A short cut is, if handling of X or Y > always > >> trigger recompute in flow_output, of course we can say it is still > correct > >> even if they are not handled in runtime data. I hope this clarifies the > >> design. > > > > > > > >> > >> > >> > The present master has the below data structures in run_time data > >> > - local_datapaths (hmap) > >> > - local_lports (sset) > >> > - local_lport_ids (sset) > >> > - active_tunnels (sset) > >> > > >> > This I-P series has the below data structures in run_time data > >> > - local_datapaths (hmap) > >> > - local_lports (sset) > >> > - local_lport_ids (sset) > >> > - local_bindings (shash) > >> > - active_tunnels (sset) > >> > - egress_ifaces (sset) > >> > - local_iface_ids (smap) > >> > > >> > Except for active_tunnels, rest of the data structures are calculated > >> > in binding.c > >> > and in ovn-controller.c (during init, run and destroy of runtime_data) > >> > And these data structures are modified in binding_run and in the ovs > >> > interface change > >> > handler and SB port binding change handler. > >> > > >> > I think if we handle the events - ovs interface changes and sb port > >> > binding changes in > >> > flow output engine, then it should ensure correctness provided we > >> > handle these changes > >> > properly. > >> > > >> Since flow_output depends on runtime data, and it take into consideration > >> all the data structures you listed above for flow computing, it means if > >> any of these data changes we need to evaluate them in flow_output. > >> Otherwise, it is either incorrect or some data in runtime data node is > >> useless. It doesn't seem right that the data is needed but we just ignore > >> the change. If, some of the data is needed as output but not needed as > >> input of flow_output, then we can simply split them out from runtime data > >> and don't add them as input of flow_output node. In this case, not > handling > >> those changes is not a problem. > >> > >> > This patch series handles ovs interface changes in flow_output > >> > engine. We already > >> > handle port binding changes, but we don't do any flow handling. I > >> > think if we can > >> > enhance this, we can solve this problem. > >> > > >> > The present patch series definitely has issues with the runtime noop > >> handler, > >> > but it still has no issues with correctness because the ovn sb idl txn > is > >> NULL > >> > for any updates ovn-controller does to SB DB, resulting in flow > >> recomputation. > >> > But of course this is wrong. And we should probably enhance the IDL > >> tracker. > >> > > >> > > From my point of view, to handle the runtime data changes > >> incrementally, we > >> > > should split the runtime data node into separate engine nodes, so > that > >> > > different changes can be handled differently for flow output compute. > >> Some > >> > > of the changes may be handled incrementally if it is easy to handle > and > >> if > >> > > it is frequently changed, and leave those less frequent and > >> hard-to-handle > >> > > changes and let them trigger recompute. > >> > > >> > I don't think this is straightforward to split them. If you see, most > >> > of the runtime data > >> > is related to port binding and is updated during port binding. > >> > >> If a data structure depends on port_binding, it is straightforward to > split > >> it as a new node and add the SB_port_binding node as its input node, and > >> then add itself as input node for flow_output. Then implement the run() > and > >> change_handler for SB_port_binding. > >> Usually it may be not that simple because the new node may also depend on > >> some other data structures in runtime data, so the dependency needs to be > >> added and handled as well. > >> The split strategy should be based on the need of I-P, i.e. split the > part > >> of data that triggers most recomputes, and handle it incrementally in > most > >> situation. This ensures least effort for best outcome. > >> > >> > > >> > Please let me know what you think about the approach I mentioned above > to > >> > handle the correctness issue. > >> > > >> > I have addressed your comments for the other patches. > >> > Shall I go ahead and submit v2 of first 4 patches of this series ? > >> > I think these patches can be considered. > >> > >> Since those are not bug fixes and more related to the rest of the series, > >> I'd suggest to submit v2 together for the whole series in case you want > to > >> change them in a different way. > >> > > > > Thanks for the comments. Patch 3 and 4 add I-P for ovs interface and port > binding > > changes in the runtime data engine. Even if we drop patch 5 and 6 from > the series > > (lets just assume for now that it just complicates further), I think we > can definitely > > handle ovs interface and port binding changes for runtime data. This > avoids > > running binding_run(). In binding_run() we run a for loop for all the > port binding rows. > > > > I will explore further on patch 6 approach. But I would like to pursue on > the rest > > of the patches. > > > > Ok, I will take a closer look at patch 3 and 4 if you believe it is right > approach to proceed, but remember that even we handle changes incrementally > in runtime_data, since there is no change_handler for runtime_data in > flow_output, it will still trigger flow recompute, which makes the > improvement of avoiding binding_run() not quite obvious compare with the > cost of lflow_run. It would be effective only if the node is splited with > e2e I-P. >
I've submitted v2 dropping patch 5 and 6 from the series. Request to be please take a look - https://patchwork.ozlabs.org/project/openvswitch/list/?series=168278 Thanks Numan > > > > Thanks > > Numan > > > > > > > >> Thanks again for taking the heavy-lift part of I-P! > >> > >> > > >> > Thanks > >> > Numan > >> > > >> > > > >> > > > engine_add_input(&en_flow_output, &en_mff_ovn_geneve, NULL); > >> > > > + engine_add_input(&en_flow_output, &en_physical_flow_changes, > >> > > > + flow_output_physical_flow_changes_handler); > >> > > > > >> > > > engine_add_input(&en_flow_output, &en_ovs_open_vswitch, NULL); > >> > > > engine_add_input(&en_flow_output, &en_ovs_bridge, NULL); > >> > > > + engine_add_input(&en_flow_output, &en_ovs_interface, > >> > > > + flow_output_noop_handler); > >> > > > + engine_add_input(&en_flow_output, &en_ct_zones, > >> > > > + flow_output_noop_handler); > >> > > > > >> > > > engine_add_input(&en_flow_output, &en_sb_chassis, NULL); > >> > > > engine_add_input(&en_flow_output, &en_sb_encap, NULL); > >> > > > diff --git a/controller/ovn-controller.h > b/controller/ovn-controller.h > >> > > > index 5d9466880..40c358cef 100644 > >> > > > --- a/controller/ovn-controller.h > >> > > > +++ b/controller/ovn-controller.h > >> > > > @@ -72,6 +72,28 @@ struct local_datapath { > >> > > > struct local_datapath *get_local_datapath(const struct hmap *, > >> > > > uint32_t tunnel_key); > >> > > > > >> > > > +enum local_binding_type { > >> > > > + BT_VIF, > >> > > > + BT_CHILD, > >> > > > + BT_VIRTUAL > >> > > > +}; > >> > > > + > >> > > > +struct local_binding { > >> > > > + struct ovs_list node; /* In parent if any. */ > >> > > > + char *name; > >> > > > + enum local_binding_type type; > >> > > > + const struct ovsrec_interface *iface; > >> > > > + const struct sbrec_port_binding *pb; > >> > > > + > >> > > > + struct ovs_list children; > >> > > > +}; > >> > > > + > >> > > > +static inline struct local_binding * > >> > > > +local_binding_find(struct shash *local_bindings, const char *name) > >> > > > +{ > >> > > > + return shash_find_data(local_bindings, name); > >> > > > +} > >> > > > + > >> > > > const struct ovsrec_bridge *get_bridge(const struct > >> ovsrec_bridge_table > >> > > *, > >> > > > const char *br_name); > >> > > > > >> > > > diff --git a/controller/physical.c b/controller/physical.c > >> > > > index 144aeb7bd..09a5d9b39 100644 > >> > > > --- a/controller/physical.c > >> > > > +++ b/controller/physical.c > >> > > > @@ -1780,6 +1780,54 @@ physical_run(struct physical_ctx *p_ctx, > >> > > > simap_destroy(&new_tunnel_to_ofport); > >> > > > } > >> > > > > >> > > > +bool > >> > > > +physical_handle_ovs_iface_changes(struct physical_ctx *p_ctx, > >> > > > + struct ovn_desired_flow_table > >> > > *flow_table) > >> > > > +{ > >> > > > + const struct ovsrec_interface *iface_rec; > >> > > > + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, > >> > > p_ctx->iface_table) { > >> > > > + if (!strcmp(iface_rec->type, "geneve") || > >> > > > + !strcmp(iface_rec->type, "patch")) { > >> > > > + return false; > >> > > > + } > >> > > > + } > >> > > > + > >> > > > + struct ofpbuf ofpacts; > >> > > > + ofpbuf_init(&ofpacts, 0); > >> > > > + > >> > > > + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, > >> > > p_ctx->iface_table) { > >> > > > + const char *iface_id = smap_get(&iface_rec->external_ids, > >> > > "iface-id"); > >> > > > + if (!iface_id) { > >> > > > + continue; > >> > > > + } > >> > > > + > >> > > > + const struct local_binding *lb = > >> > > > + local_binding_find(p_ctx->local_bindings, iface_id); > >> > > > + > >> > > > + if (!lb || !lb->pb) { > >> > > > + continue; > >> > > > + } > >> > > > + > >> > > > + if (ovsrec_interface_is_deleted(iface_rec)) { > >> > > > + ofctrl_remove_flows(flow_table, > &lb->pb->header_.uuid); > >> > > > + } else { > >> > > > + if (!ovsrec_interface_is_new(iface_rec)) { > >> > > > + ofctrl_remove_flows(flow_table, > >> &lb->pb->header_.uuid); > >> > > > + } > >> > > > + > >> > > > + > consider_port_binding(p_ctx->sbrec_port_binding_by_name, > >> > > > + p_ctx->mff_ovn_geneve, > >> p_ctx->ct_zones, > >> > > > + p_ctx->active_tunnels, > >> > > > + p_ctx->local_datapaths, > >> > > > + lb->pb, p_ctx->chassis, > >> > > > + flow_table, &ofpacts); > >> > > > + } > >> > > > + } > >> > > > + > >> > > > + ofpbuf_uninit(&ofpacts); > >> > > > + return true; > >> > > > +} > >> > > > + > >> > > > bool > >> > > > get_tunnel_ofport(const char *chassis_name, char *encap_ip, > >> ofp_port_t > >> > > *ofport) > >> > > > { > >> > > > diff --git a/controller/physical.h b/controller/physical.h > >> > > > index dadf84ea1..9ca34436a 100644 > >> > > > --- a/controller/physical.h > >> > > > +++ b/controller/physical.h > >> > > > @@ -49,11 +49,13 @@ struct physical_ctx { > >> > > > const struct ovsrec_bridge *br_int; > >> > > > const struct sbrec_chassis_table *chassis_table; > >> > > > const struct sbrec_chassis *chassis; > >> > > > + const struct ovsrec_interface_table *iface_table; > >> > > > const struct sset *active_tunnels; > >> > > > struct hmap *local_datapaths; > >> > > > struct sset *local_lports; > >> > > > const struct simap *ct_zones; > >> > > > enum mf_field_id mff_ovn_geneve; > >> > > > + struct shash *local_bindings; > >> > > > }; > >> > > > > >> > > > void physical_register_ovs_idl(struct ovsdb_idl *); > >> > > > @@ -63,7 +65,8 @@ void physical_handle_port_binding_changes(struct > >> > > physical_ctx *, > >> > > > struct > >> ovn_desired_flow_table > >> > > *); > >> > > > void physical_handle_mc_group_changes(struct physical_ctx *, > >> > > > struct > ovn_desired_flow_table > >> *); > >> > > > - > >> > > > +bool physical_handle_ovs_iface_changes(struct physical_ctx *, > >> > > > + struct > ovn_desired_flow_table > >> *); > >> > > > bool get_tunnel_ofport(const char *chassis_name, char *encap_ip, > >> > > > ofp_port_t *ofport); > >> > > > #endif /* controller/physical.h */ > >> > > > diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at > >> > > > index 5c21f6dd7..555434484 100644 > >> > > > --- a/tests/ovn-performance.at > >> > > > +++ b/tests/ovn-performance.at > >> > > > @@ -263,7 +263,7 @@ for i in 1 2; do > >> > > > ) > >> > > > > >> > > > # Add router port to $ls > >> > > > - OVN_CONTROLLER_EXPECT_HIT( > >> > > > + OVN_CONTROLLER_EXPECT_NO_HIT( > >> > > > [hv1 hv2], [lflow_run], > >> > > > [ovn-nbctl --wait=hv lrp-add lr1 $lrp 02:00:00:00:0$i:01 > >> > > 10.0.$i.1/24] > >> > > > ) > >> > > > @@ -271,15 +271,15 @@ for i in 1 2; do > >> > > > [hv1 hv2], [lflow_run], > >> > > > [ovn-nbctl --wait=hv lsp-add $ls $lsp] > >> > > > ) > >> > > > - OVN_CONTROLLER_EXPECT_HIT( > >> > > > + OVN_CONTROLLER_EXPECT_NO_HIT( > >> > > > [hv1 hv2], [lflow_run], > >> > > > [ovn-nbctl --wait=hv lsp-set-type $lsp router] > >> > > > ) > >> > > > - OVN_CONTROLLER_EXPECT_HIT( > >> > > > + OVN_CONTROLLER_EXPECT_NO_HIT( > >> > > > [hv1 hv2], [lflow_run], > >> > > > [ovn-nbctl --wait=hv lsp-set-options $lsp > router-port=$lrp] > >> > > > ) > >> > > > - OVN_CONTROLLER_EXPECT_HIT( > >> > > > + OVN_CONTROLLER_EXPECT_NO_HIT( > >> > > > [hv1 hv2], [lflow_run], > >> > > > [ovn-nbctl --wait=hv lsp-set-addresses $lsp router] > >> > > > ) > >> > > > @@ -393,8 +393,8 @@ for i in 1 2; do > >> > > > lp=lp$i > >> > > > > >> > > > # Delete port $lp > >> > > > - OVN_CONTROLLER_EXPECT_HIT_COND( > >> > > > - [hv$i hv$j], [lflow_run], [>0 =0], > >> > > > + OVN_CONTROLLER_EXPECT_NO_HIT( > >> > > > + [hv$i hv$j], [lflow_run], > >> > > > [ovn-nbctl --wait=hv lsp-del $lp] > >> > > > ) > >> > > > done > >> > > > @@ -409,7 +409,7 @@ for i in 1 2; do > >> > > > ls=ls$i > >> > > > > >> > > > # Delete switch $ls > >> > > > - OVN_CONTROLLER_EXPECT_HIT( > >> > > > + OVN_CONTROLLER_EXPECT_NO_HIT( > >> > > > [hv1 hv2], [lflow_run], > >> > > > [ovn-nbctl --wait=hv ls-del $ls] > >> > > > ) > >> > > > -- > >> > > > 2.24.1 > >> > > > > >> > > > _______________________________________________ > >> > > > dev mailing list > >> > > > d...@openvswitch.org > >> > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >> > > _______________________________________________ > >> > > dev mailing list > >> > > d...@openvswitch.org > >> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >> > > > >> _______________________________________________ > >> dev mailing list > >> d...@openvswitch.org > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >> > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev