an On Thu, Feb 25, 2021 at 3:42 PM Mark Gray <[email protected]> wrote: > > On 18/02/2021 15:54, [email protected] wrote: > > From: Numan Siddique <[email protected]> > > Hi Numans, I can't comment on the design change but I have a few > comments/questions.
Hi Mark, Thanks for the review. > > It needs to be rebased again. Just to help move it along, I found a > commit that it applies to and reviewed at that point. > > > > Presently, the 'flow_output' engine node recomputes > > physical flows by calling physical_run() in the > > 'physical_flow_changes' handler.in some scenarios. > > typo: .. handler in ... Ack. > > > Because of this, an engine run can do a full recompute > > of physical flows but not full recompute of logical flows. > > Although this works now, it is problematic as the same > > desired flow table is used for both physical and logical flows. > > > > This patch now separates the handling of logical flows and > > physical flows. Two separate engine nodes are added - > > lflow_output and pflow_output with their own flow tables and > > these two nodes are now inputs to the main engine node - > > flow_output. > > > > CC: Han Zhou <[email protected]> > > Signed-off-by: Numan Siddique <[email protected]> > > --- > > > > v2 -> v3 > > ----- > > * Rebased to resolve conflicts. > > > > v1 -> v2 > > ----- > > * Rebased to resolve conflicts. > > > > TODO.rst | 6 + > > controller/lflow.c | 14 +- > > controller/lflow.h | 6 +- > > controller/ofctrl.c | 215 +++++++---- > > controller/ofctrl.h | 37 +- > > controller/ovn-controller.c | 691 ++++++++++++++++++------------------ > > controller/physical.c | 28 +- > > controller/physical.h | 12 +- > > 8 files changed, 543 insertions(+), 466 deletions(-) > > > > diff --git a/TODO.rst b/TODO.rst > > index c15815539f..910531e7e3 100644 > > --- a/TODO.rst > > +++ b/TODO.rst > > @@ -160,3 +160,9 @@ OVN To-do List > > to find a way of determining if routing has already been executed (on a > > different hypervisor) for the IP multicast packet being processed > > locally > > in the router pipeline. > > + > > +* ovn-controller Incremental processing > > + > > + * physical.c has a global simap -localvif_to_ofport which stores the > > + local OVS interfaces and the ofport numbers. Move this to the engine > > data > > + of the engine data node - ed_type_pflow_output. > > diff --git a/controller/lflow.c b/controller/lflow.c > > index 4bdd953bee..8b1e4770ba 100644 > > --- a/controller/lflow.c > > +++ b/controller/lflow.c > > @@ -966,7 +966,7 @@ static void > > consider_neighbor_flow(struct ovsdb_idl_index *sbrec_port_binding_by_name, > > const struct hmap *local_datapaths, > > const struct sbrec_mac_binding *b, > > - struct ovn_desired_flow_table *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > const struct sbrec_port_binding *pb > > = lport_lookup_by_name(sbrec_port_binding_by_name, > > b->logical_port); > > @@ -1046,7 +1046,7 @@ static void > > add_neighbor_flows(struct ovsdb_idl_index *sbrec_port_binding_by_name, > > const struct sbrec_mac_binding_table *mac_binding_table, > > const struct hmap *local_datapaths, > > - struct ovn_desired_flow_table *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > const struct sbrec_mac_binding *b; > > SBREC_MAC_BINDING_TABLE_FOR_EACH (b, mac_binding_table) { > > @@ -1232,7 +1232,7 @@ add_lb_vip_hairpin_flows(struct ovn_controller_lb *lb, > > struct ovn_lb_vip *lb_vip, > > struct ovn_lb_backend *lb_backend, > > uint8_t lb_proto, > > - struct ovn_desired_flow_table *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > uint64_t stub[1024 / 8]; > > struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub); > > @@ -1301,7 +1301,7 @@ static void > > add_lb_ct_snat_vip_flows(struct ovn_controller_lb *lb, > > struct ovn_lb_vip *lb_vip, > > uint8_t lb_proto, > > - struct ovn_desired_flow_table *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > uint64_t stub[1024 / 8]; > > struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub); > > @@ -1369,7 +1369,7 @@ add_lb_ct_snat_vip_flows(struct ovn_controller_lb *lb, > > static void > > consider_lb_hairpin_flows(const struct sbrec_load_balancer *sbrec_lb, > > const struct hmap *local_datapaths, > > - struct ovn_desired_flow_table *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > /* Check if we need to add flows or not. If there is one datapath > > * in the local_datapaths, it means all the datapaths of the lb > > @@ -1417,7 +1417,7 @@ consider_lb_hairpin_flows(const struct > > sbrec_load_balancer *sbrec_lb, > > static void > > add_lb_hairpin_flows(const struct sbrec_load_balancer_table *lb_table, > > const struct hmap *local_datapaths, > > - struct ovn_desired_flow_table *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > const struct sbrec_load_balancer *lb; > > SBREC_LOAD_BALANCER_TABLE_FOR_EACH (lb, lb_table) { > > @@ -1431,7 +1431,7 @@ lflow_handle_changed_neighbors( > > struct ovsdb_idl_index *sbrec_port_binding_by_name, > > const struct sbrec_mac_binding_table *mac_binding_table, > > const struct hmap *local_datapaths, > > - struct ovn_desired_flow_table *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > const struct sbrec_mac_binding *mb; > > /* Handle deleted mac_bindings first, to avoid *duplicated flow* > > problem > > diff --git a/controller/lflow.h b/controller/lflow.h > > index 833b42fa64..0c785d9dd4 100644 > > --- a/controller/lflow.h > > +++ b/controller/lflow.h > > @@ -41,7 +41,7 @@ > > > > struct ovn_extend_table; > > struct ovsdb_idl_index; > > -struct ovn_desired_flow_table; > > +struct ovn_flow_table; > > struct hmap; > > struct hmap_node; > > struct sbrec_chassis; > > @@ -147,7 +147,7 @@ struct lflow_ctx_in { > > }; > > > > struct lflow_ctx_out { > > - struct ovn_desired_flow_table *flow_table; > > + struct ovn_flow_table *flow_table; > > struct ovn_extend_table *group_table; > > struct ovn_extend_table *meter_table; > > struct lflow_resource_ref *lfrr; > > @@ -168,7 +168,7 @@ void lflow_handle_changed_neighbors( > > struct ovsdb_idl_index *sbrec_port_binding_by_name, > > const struct sbrec_mac_binding_table *, > > const struct hmap *local_datapaths, > > - struct ovn_desired_flow_table *); > > + struct ovn_flow_table *); > > bool lflow_handle_changed_lbs(struct lflow_ctx_in *, struct lflow_ctx_out > > *); > > void lflow_destroy(void); > > > > diff --git a/controller/ofctrl.c b/controller/ofctrl.c > > index 1c9694a633..2032e60946 100644 > > --- a/controller/ofctrl.c > > +++ b/controller/ofctrl.c > > @@ -51,6 +51,26 @@ > > > > VLOG_DEFINE_THIS_MODULE(ofctrl); > > > > +struct ovn_desired_flow_table { > > + /* Hash map flow table using flow match conditions as hash key.*/ > > + struct hmap match_flow_table; > > + > > + /* SB uuid index for the cross reference nodes that link to the nodes > > in > > + * match_flow_table.*/ > > + struct hmap uuid_flow_table; > > + > > + /* Is flow changes tracked. */ > > + bool change_tracked; > > + /* Tracked flow changes. */ > > + struct ovs_list tracked_flows; > > +}; > > + > > +struct ovn_flow_table { > > + struct ovn_desired_flow_table desired_ftable; > > + struct hmap installed_flows; > > +}; > > + > > + > > Remove additional newline Ack. I'll remove it in v4. > > > /* An OpenFlow flow. */ > > struct ovn_flow { > > /* Key. */ > > @@ -172,7 +192,7 @@ struct sb_flow_ref { > > struct uuid sb_uuid; > > }; > > > > -/* A installed flow, in static variable installed_flows. > > +/* An installed flow, in installed_flows hmap of 'ovn_flow_table'. > > * > > * Installed flows are updated in ofctrl_put for maintaining the flow > > * installation to OVS. They are updated according to desired flows: > > either by > > @@ -211,6 +231,10 @@ struct installed_flow { > > struct ovs_list desired_refs; > > }; > > > > +static void ovn_desired_flow_table_init(struct ovn_desired_flow_table *); > > +static void ovn_desired_flow_table_clear(struct ovn_desired_flow_table *); > > +static void ovn_desired_flow_table_destroy(struct ovn_desired_flow_table > > *); > > + > > typedef bool > > (*desired_flow_match_cb)(const struct desired_flow *candidate, > > const void *arg); > > @@ -233,7 +257,7 @@ static struct desired_flow > > *desired_flow_lookup_conjunctive( > > static void desired_flow_destroy(struct desired_flow *); > > > > static struct installed_flow *installed_flow_lookup( > > - const struct ovn_flow *target); > > + const struct ovn_flow *target, struct hmap *installed_flows); > > static void installed_flow_destroy(struct installed_flow *); > > static struct installed_flow *installed_flow_dup(struct desired_flow *); > > static struct desired_flow *installed_flow_get_active(struct > > installed_flow *); > > @@ -301,9 +325,15 @@ static ovs_be32 xid, xid2; > > * zero, to avoid unbounded buffering. */ > > static struct rconn_packet_counter *tx_counter; > > > > -/* Flow table of "struct ovn_flow"s, that holds the flow table currently > > - * installed in the switch. */ > > -static struct hmap installed_flows; > > +/* Reference to flow table of "struct ovn_flow"s, that holds the flow table > > + * currently installed in the switch. > > + * > > + * 'installed_lflows' - Represents flows for logical flows processed by > > + * lflow.c. > > + * 'installed_pflows' - Represents physical flows generated by physical.c. > > + */ > > +static struct hmap *installed_lflows; > > +static struct hmap *installed_pflows; > > > > /* A reference to the group_table. */ > > static struct ovn_extend_table *groups; > > @@ -328,25 +358,51 @@ static struct ofpbuf *encode_group_mod(const struct > > ofputil_group_mod *); > > > > static struct ofpbuf *encode_meter_mod(const struct ofputil_meter_mod *); > > > > -static void ovn_installed_flow_table_clear(void); > > -static void ovn_installed_flow_table_destroy(void); > > - > > +static void ovn_installed_flow_table_clear(struct hmap *installed_flows); > > > > static void ofctrl_recv(const struct ofp_header *, enum ofptype); > > > > +/* ovn_flow_table operations. */ > > +struct ovn_flow_table * > > +ovn_flow_table_alloc(void) > > +{ > > + struct ovn_flow_table *flow_table = xzalloc(sizeof *flow_table); > > + ovn_desired_flow_table_init(&flow_table->desired_ftable); > > + hmap_init(&flow_table->installed_flows); > > + return flow_table; > > +} > > + > > +void > > +ovn_flow_table_clear(struct ovn_flow_table *flow_table) > > +{ > > + ovn_desired_flow_table_clear(&flow_table->desired_ftable); > > +} > > + > > +void > > +ovn_flow_table_destroy(struct ovn_flow_table *flow_table) > > +{ > > + ovn_desired_flow_table_destroy(&flow_table->desired_ftable); > > + ovn_installed_flow_table_clear(&flow_table->installed_flows); > > + hmap_destroy(&flow_table->installed_flows); > > + free(flow_table); > > +} > > + > > void > > ofctrl_init(struct ovn_extend_table *group_table, > > struct ovn_extend_table *meter_table, > > + struct ovn_flow_table *lflow_table, > > + struct ovn_flow_table *pflow_table, > > int inactivity_probe_interval) > > { > > swconn = rconn_create(inactivity_probe_interval, 0, > > DSCP_DEFAULT, 1 << OFP15_VERSION); > > tx_counter = rconn_packet_counter_create(); > > - hmap_init(&installed_flows); > > ovs_list_init(&flow_updates); > > ovn_init_symtab(&symtab); > > groups = group_table; > > meters = meter_table; > > + installed_lflows = &lflow_table->installed_flows; > > + installed_pflows = &pflow_table->installed_flows; > > } > > > > /* S_NEW, for a new connection. > > @@ -573,7 +629,8 @@ run_S_CLEAR_FLOWS(void) > > ofputil_uninit_group_mod(&gm); > > > > /* Clear installed_flows, to match the state of the switch. */ > > - ovn_installed_flow_table_clear(); > > + ovn_installed_flow_table_clear(installed_lflows); > > + ovn_installed_flow_table_clear(installed_pflows); > > > > /* Clear existing groups, to match the state of the switch. */ > > if (groups) { > > @@ -758,7 +815,6 @@ void > > ofctrl_destroy(void) > > { > > rconn_destroy(swconn); > > - ovn_installed_flow_table_destroy(); > > rconn_packet_counter_destroy(tx_counter); > > expr_symtab_destroy(&symtab); > > shash_destroy(&symtab); > > @@ -1019,17 +1075,18 @@ link_flow_to_sb(struct ovn_desired_flow_table > > *flow_table, > > * > > * The caller should initialize its own hmap to hold the flows. */ > > void > > -ofctrl_check_and_add_flow(struct ovn_desired_flow_table *flow_table, > > +ofctrl_check_and_add_flow(struct ovn_flow_table *flow_table, > > uint8_t table_id, uint16_t priority, > > uint64_t cookie, const struct match *match, > > const struct ofpbuf *actions, > > const struct uuid *sb_uuid, > > bool log_duplicate_flow) > > { > > + struct ovn_desired_flow_table *d_ftable = &flow_table->desired_ftable; > > struct desired_flow *f = desired_flow_alloc(table_id, priority, cookie, > > match, actions); > > > > - if (desired_flow_lookup_check_uuid(flow_table, &f->flow, sb_uuid)) { > > + if (desired_flow_lookup_check_uuid(d_ftable, &f->flow, sb_uuid)) { > > if (log_duplicate_flow) { > > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); > > if (!VLOG_DROP_DBG(&rl)) { > > @@ -1042,20 +1099,20 @@ ofctrl_check_and_add_flow(struct > > ovn_desired_flow_table *flow_table, > > return; > > } > > > > - hmap_insert(&flow_table->match_flow_table, &f->match_hmap_node, > > + hmap_insert(&d_ftable->match_flow_table, &f->match_hmap_node, > > f->flow.hash); > > - link_flow_to_sb(flow_table, f, sb_uuid); > > - track_flow_add_or_modify(flow_table, f); > > + link_flow_to_sb(d_ftable, f, sb_uuid); > > + track_flow_add_or_modify(d_ftable, f); > > ovn_flow_log(&f->flow, "ofctrl_add_flow"); > > } > > > > void > > -ofctrl_add_flow(struct ovn_desired_flow_table *desired_flows, > > +ofctrl_add_flow(struct ovn_flow_table *flow_table, > > uint8_t table_id, uint16_t priority, uint64_t cookie, > > const struct match *match, const struct ofpbuf *actions, > > const struct uuid *sb_uuid) > > { > > - ofctrl_check_and_add_flow(desired_flows, table_id, priority, cookie, > > + ofctrl_check_and_add_flow(flow_table, table_id, priority, cookie, > > match, actions, sb_uuid, true); > > } > > > > @@ -1063,7 +1120,7 @@ ofctrl_add_flow(struct ovn_desired_flow_table > > *desired_flows, > > * flow existed, a new link will also be created between the new sb_uuid > > * and the existing flow. */ > > void > > -ofctrl_add_or_append_flow(struct ovn_desired_flow_table *desired_flows, > > +ofctrl_add_or_append_flow(struct ovn_flow_table *flow_table, > > uint8_t table_id, uint16_t priority, uint64_t > > cookie, > > const struct match *match, > > const struct ofpbuf *actions, > > @@ -1072,6 +1129,7 @@ ofctrl_add_or_append_flow(struct > > ovn_desired_flow_table *desired_flows, > > struct desired_flow *existing; > > struct desired_flow *f; > > > > + struct ovn_desired_flow_table *desired_flows = > > &flow_table->desired_ftable; > > f = desired_flow_alloc(table_id, priority, cookie, match, actions); > > existing = desired_flow_lookup_conjunctive(desired_flows, &f->flow); > > if (existing) { > > @@ -1136,13 +1194,14 @@ remove_flows_from_sb_to_flow(struct > > ovn_desired_flow_table *flow_table, > > } > > > > void > > -ofctrl_remove_flows(struct ovn_desired_flow_table *flow_table, > > +ofctrl_remove_flows(struct ovn_flow_table *flow_table, > > const struct uuid *sb_uuid) > > { > > - struct sb_to_flow *stf = sb_to_flow_find(&flow_table->uuid_flow_table, > > + struct ovn_desired_flow_table *d_ftable = &flow_table->desired_ftable; > > + struct sb_to_flow *stf = sb_to_flow_find(&d_ftable->uuid_flow_table, > > sb_uuid); > > if (stf) { > > - remove_flows_from_sb_to_flow(flow_table, stf, > > "ofctrl_remove_flow"); > > + remove_flows_from_sb_to_flow(d_ftable, stf, "ofctrl_remove_flow"); > > } > > > > /* remove any related group and meter info */ > > @@ -1243,9 +1302,10 @@ flood_remove_flows_for_sb_uuid(struct > > ovn_desired_flow_table *flow_table, > > } > > > > void > > -ofctrl_flood_remove_flows(struct ovn_desired_flow_table *flow_table, > > +ofctrl_flood_remove_flows(struct ovn_flow_table *flow_table, > > struct hmap *flood_remove_nodes) > > { > > + struct ovn_desired_flow_table *d_ftable = &flow_table->desired_ftable; > > struct ofctrl_flood_remove_node *ofrn; > > int i, n = 0; > > > > @@ -1260,7 +1320,7 @@ ofctrl_flood_remove_flows(struct > > ovn_desired_flow_table *flow_table, > > } > > > > for (i = 0; i < n; i++) { > > - flood_remove_flows_for_sb_uuid(flow_table, &sb_uuids[i], > > + flood_remove_flows_for_sb_uuid(d_ftable, &sb_uuids[i], > > flood_remove_nodes); > > } > > free(sb_uuids); > > @@ -1425,11 +1485,12 @@ desired_flow_lookup_conjunctive(struct > > ovn_desired_flow_table *flow_table, > > /* Finds and returns an installed_flow in installed_flows whose key is > > * identical to 'target''s key, or NULL if there is none. */ > > static struct installed_flow * > > -installed_flow_lookup(const struct ovn_flow *target) > > +installed_flow_lookup(const struct ovn_flow *target, > > + struct hmap *installed_flows) > > { > > struct installed_flow *i; > > HMAP_FOR_EACH_WITH_HASH (i, match_hmap_node, target->hash, > > - &installed_flows) { > > + installed_flows) { > > struct ovn_flow *f = &i->flow; > > if (f->table_id == target->table_id > > && f->priority == target->priority > > @@ -1494,23 +1555,23 @@ installed_flow_destroy(struct installed_flow *f) > > } > > > > /* Desired flow table operations. */ > > -void > > -ovn_desired_flow_table_init(struct ovn_desired_flow_table *flow_table) > > +static void > > +ovn_desired_flow_table_init(struct ovn_desired_flow_table *desired_ftable) > > { > > - hmap_init(&flow_table->match_flow_table); > > - hmap_init(&flow_table->uuid_flow_table); > > - ovs_list_init(&flow_table->tracked_flows); > > - flow_table->change_tracked = false; > > + hmap_init(&desired_ftable->match_flow_table); > > + hmap_init(&desired_ftable->uuid_flow_table); > > + ovs_list_init(&desired_ftable->tracked_flows); > > + desired_ftable->change_tracked = false; > > } > > > > -void > > -ovn_desired_flow_table_clear(struct ovn_desired_flow_table *flow_table) > > +static void > > +ovn_desired_flow_table_clear(struct ovn_desired_flow_table *desired_ftable) > > { > > - flow_table->change_tracked = false; > > + desired_ftable->change_tracked = false; > > > > struct desired_flow *f, *f_next; > > LIST_FOR_EACH_SAFE (f, f_next, track_list_node, > > - &flow_table->tracked_flows) { > > + &desired_ftable->tracked_flows) { > > ovs_list_remove(&f->track_list_node); > > if (f->is_deleted) { > > if (f->installed_flow) { > > @@ -1522,38 +1583,32 @@ ovn_desired_flow_table_clear(struct > > ovn_desired_flow_table *flow_table) > > > > struct sb_to_flow *stf, *next; > > HMAP_FOR_EACH_SAFE (stf, next, hmap_node, > > - &flow_table->uuid_flow_table) { > > - remove_flows_from_sb_to_flow(flow_table, stf, NULL); > > + &desired_ftable->uuid_flow_table) { > > + remove_flows_from_sb_to_flow(desired_ftable, stf, NULL); > > } > > } > > > > -void > > -ovn_desired_flow_table_destroy(struct ovn_desired_flow_table *flow_table) > > +static void > > +ovn_desired_flow_table_destroy(struct ovn_desired_flow_table > > *desired_ftable) > > { > > - ovn_desired_flow_table_clear(flow_table); > > - hmap_destroy(&flow_table->match_flow_table); > > - hmap_destroy(&flow_table->uuid_flow_table); > > + ovn_desired_flow_table_clear(desired_ftable); > > + hmap_destroy(&desired_ftable->match_flow_table); > > + hmap_destroy(&desired_ftable->uuid_flow_table); > > } > > > > > > /* Installed flow table operations. */ > > static void > > -ovn_installed_flow_table_clear(void) > > +ovn_installed_flow_table_clear(struct hmap *installed_flows) > > { > > struct installed_flow *f, *next; > > - HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, &installed_flows) { > > - hmap_remove(&installed_flows, &f->match_hmap_node); > > + HMAP_FOR_EACH_SAFE (f, next, match_hmap_node, installed_flows) { > > + hmap_remove(installed_flows, &f->match_hmap_node); > > unlink_all_refs_for_installed_flow(f); > > installed_flow_destroy(f); > > } > > } > > > > -static void > > -ovn_installed_flow_table_destroy(void) > > -{ > > - ovn_installed_flow_table_clear(); > > - hmap_destroy(&installed_flows); > > -} > > > > /* Flow table update. */ > > > > @@ -1809,24 +1864,27 @@ installed_flow_del(struct ovn_flow *i, struct > > ovs_list *msgs) > > } > > > > static void > > -update_installed_flows_by_compare(struct ovn_desired_flow_table > > *flow_table, > > +update_installed_flows_by_compare(struct ovn_flow_table *flow_table, > > struct ovs_list *msgs) > > { > > - ovs_assert(ovs_list_is_empty(&flow_table->tracked_flows)); > > + struct ovn_desired_flow_table *d_ftable = &flow_table->desired_ftable; > > + > > + ovs_assert(ovs_list_is_empty(&d_ftable->tracked_flows)); > > /* Iterate through all of the installed flows. If any of them are no > > * longer desired, delete them; if any of them should have different > > * actions, update them. */ > > struct installed_flow *i, *next; > > - HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, &installed_flows) { > > + HMAP_FOR_EACH_SAFE (i, next, match_hmap_node, > > + &flow_table->installed_flows) { > > unlink_all_refs_for_installed_flow(i); > > - struct desired_flow *d = desired_flow_lookup(flow_table, &i->flow); > > + struct desired_flow *d = desired_flow_lookup(d_ftable, &i->flow); > > if (!d) { > > /* Installed flow is no longer desirable. Delete it from the > > * switch and from installed_flows. */ > > installed_flow_del(&i->flow, msgs); > > ovn_flow_log(&i->flow, "removing installed"); > > > > - hmap_remove(&installed_flows, &i->match_hmap_node); > > + hmap_remove(&flow_table->installed_flows, &i->match_hmap_node); > > installed_flow_destroy(i); > > } else { > > if (!ofpacts_equal(i->flow.ofpacts, i->flow.ofpacts_len, > > @@ -1843,15 +1901,16 @@ update_installed_flows_by_compare(struct > > ovn_desired_flow_table *flow_table, > > /* Iterate through the desired flows and add those that aren't found > > * in the installed flow table. */ > > struct desired_flow *d; > > - HMAP_FOR_EACH (d, match_hmap_node, &flow_table->match_flow_table) { > > - i = installed_flow_lookup(&d->flow); > > + HMAP_FOR_EACH (d, match_hmap_node, &d_ftable->match_flow_table) { > > + i = installed_flow_lookup(&d->flow, &flow_table->installed_flows); > > if (!i) { > > ovn_flow_log(&d->flow, "adding installed"); > > installed_flow_add(&d->flow, msgs); > > > > /* Copy 'd' from 'flow_table' to installed_flows. */ > > i = installed_flow_dup(d); > > - hmap_insert(&installed_flows, &i->match_hmap_node, > > i->flow.hash); > > + hmap_insert(&flow_table->installed_flows, &i->match_hmap_node, > > + i->flow.hash); > > link_installed_to_desired(i, d); > > } else if (!d->installed_flow) { > > /* This is a desired_flow that conflicts with one installed > > @@ -1940,13 +1999,13 @@ merge_tracked_flows(struct ovn_desired_flow_table > > *flow_table) > > } > > > > static void > > -update_installed_flows_by_track(struct ovn_desired_flow_table *flow_table, > > +update_installed_flows_by_track(struct ovn_flow_table *flow_table, > > struct ovs_list *msgs) > > { > > - merge_tracked_flows(flow_table); > > + merge_tracked_flows(&flow_table->desired_ftable); > > struct desired_flow *f, *f_next; > > LIST_FOR_EACH_SAFE (f, f_next, track_list_node, > > - &flow_table->tracked_flows) { > > + &flow_table->desired_ftable.tracked_flows) { > > ovs_list_remove(&f->track_list_node); > > if (f->is_deleted) { > > /* The desired flow was deleted */ > > @@ -1959,7 +2018,8 @@ update_installed_flows_by_track(struct > > ovn_desired_flow_table *flow_table, > > installed_flow_del(&i->flow, msgs); > > ovn_flow_log(&i->flow, "removing installed (tracked)"); > > > > - hmap_remove(&installed_flows, &i->match_hmap_node); > > + hmap_remove(&flow_table->installed_flows, > > + &i->match_hmap_node); > > installed_flow_destroy(i); > > } else if (was_active) { > > /* There are other desired flow(s) referencing this > > @@ -1973,7 +2033,8 @@ update_installed_flows_by_track(struct > > ovn_desired_flow_table *flow_table, > > desired_flow_destroy(f); > > } else { > > /* The desired flow was added or modified. */ > > - struct installed_flow *i = installed_flow_lookup(&f->flow); > > + struct installed_flow *i = > > + installed_flow_lookup(&f->flow, > > &flow_table->installed_flows); > > if (!i) { > > /* Adding a new flow. */ > > installed_flow_add(&f->flow, msgs); > > @@ -1981,7 +2042,8 @@ update_installed_flows_by_track(struct > > ovn_desired_flow_table *flow_table, > > > > /* Copy 'f' from 'flow_table' to installed_flows. */ > > struct installed_flow *new_node = installed_flow_dup(f); > > - hmap_insert(&installed_flows, &new_node->match_hmap_node, > > + hmap_insert(&flow_table->installed_flows, > > + &new_node->match_hmap_node, > > new_node->flow.hash); > > link_installed_to_desired(new_node, f); > > } else if (installed_flow_get_active(i) == f) { > > @@ -2035,7 +2097,8 @@ ofctrl_can_put(void) > > * > > * This should be called after ofctrl_run() within the main loop. */ > > void > > -ofctrl_put(struct ovn_desired_flow_table *flow_table, > > +ofctrl_put(struct ovn_flow_table *lflow_table, > > + struct ovn_flow_table *pflow_table, > > struct shash *pending_ct_zones, > > const struct sbrec_meter_table *meter_table, > > uint64_t req_cfg, > > @@ -2126,10 +2189,16 @@ ofctrl_put(struct ovn_desired_flow_table > > *flow_table, > > } > > } > > > > - if (flow_table->change_tracked) { > > - update_installed_flows_by_track(flow_table, &msgs); > > + if (lflow_table->desired_ftable.change_tracked) { > > + update_installed_flows_by_track(lflow_table, &msgs); > > + } else { > > + update_installed_flows_by_compare(lflow_table, &msgs); > > + } > > + > > + if (pflow_table->desired_ftable.change_tracked) { > > + update_installed_flows_by_track(pflow_table, &msgs); > > } else { > > - update_installed_flows_by_compare(flow_table, &msgs); > > + update_installed_flows_by_compare(pflow_table, &msgs); > > } > > > > /* Iterate through the installed groups from previous runs. If they > > @@ -2243,8 +2312,10 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table, > > cur_cfg = req_cfg; > > } > > > > - flow_table->change_tracked = true; > > - ovs_assert(ovs_list_is_empty(&flow_table->tracked_flows)); > > + lflow_table->desired_ftable.change_tracked = true; > > + > > ovs_assert(ovs_list_is_empty(&lflow_table->desired_ftable.tracked_flows)); > > + pflow_table->desired_ftable.change_tracked = true; > > + > > ovs_assert(ovs_list_is_empty(&pflow_table->desired_ftable.tracked_flows)); > > } > > > > /* Looks up the logical port with the name 'port_name' in 'br_int_'. If > > diff --git a/controller/ofctrl.h b/controller/ofctrl.h > > index 88769566ac..61b3f166f2 100644 > > --- a/controller/ofctrl.h > > +++ b/controller/ofctrl.h > > @@ -31,28 +31,23 @@ struct ovsrec_bridge; > > struct sbrec_meter_table; > > struct shash; > > > > -struct ovn_desired_flow_table { > > - /* Hash map flow table using flow match conditions as hash key.*/ > > - struct hmap match_flow_table; > > - > > - /* SB uuid index for the cross reference nodes that link to the nodes > > in > > - * match_flow_table.*/ > > - struct hmap uuid_flow_table; > > - > > - /* Is flow changes tracked. */ > > - bool change_tracked; > > - /* Tracked flow changes. */ > > - struct ovs_list tracked_flows; > > -}; > > +struct ovn_flow_table; > > + > > +struct ovn_flow_table *ovn_flow_table_alloc(void); > > +void ovn_flow_table_clear(struct ovn_flow_table *); > > +void ovn_flow_table_destroy(struct ovn_flow_table *); > > > > /* Interface for OVN main loop. */ > > void ofctrl_init(struct ovn_extend_table *group_table, > > struct ovn_extend_table *meter_table, > > + struct ovn_flow_table *lflow_table, > > + struct ovn_flow_table *pflow_table, > > int inactivity_probe_interval); > > void ofctrl_run(const struct ovsrec_bridge *br_int, > > struct shash *pending_ct_zones); > > enum mf_field_id ofctrl_get_mf_field_id(void); > > -void ofctrl_put(struct ovn_desired_flow_table *, > > +void ofctrl_put(struct ovn_flow_table *lflow_table, > > + struct ovn_flow_table *pflow_table, > > struct shash *pending_ct_zones, > > const struct sbrec_meter_table *, > > uint64_t nb_cfg, > > @@ -69,12 +64,12 @@ char *ofctrl_inject_pkt(const struct ovsrec_bridge > > *br_int, > > const struct shash *port_groups); > > > > /* Flow table interfaces to the rest of ovn-controller. */ > > -void ofctrl_add_flow(struct ovn_desired_flow_table *, uint8_t table_id, > > +void ofctrl_add_flow(struct ovn_flow_table *, uint8_t table_id, > > uint16_t priority, uint64_t cookie, > > const struct match *, const struct ofpbuf *ofpacts, > > const struct uuid *); > > > > -void ofctrl_add_or_append_flow(struct ovn_desired_flow_table > > *desired_flows, > > +void ofctrl_add_or_append_flow(struct ovn_flow_table *, > > uint8_t table_id, uint16_t priority, > > uint64_t cookie, const struct match *match, > > const struct ofpbuf *actions, > > @@ -84,7 +79,7 @@ void ofctrl_add_or_append_flow(struct > > ovn_desired_flow_table *desired_flows, > > * flows are removed only if they are not referenced by any other > > sb_uuid(s). > > * For flood-removing all related flows referenced by other sb_uuid(s), use > > * ofctrl_flood_remove_flows(). */ > > -void ofctrl_remove_flows(struct ovn_desired_flow_table *, > > +void ofctrl_remove_flows(struct ovn_flow_table *, > > const struct uuid *sb_uuid); > > > > /* The function ofctrl_flood_remove_flows flood-removes flows from the > > desired > > @@ -100,15 +95,11 @@ struct ofctrl_flood_remove_node { > > struct hmap_node hmap_node; > > struct uuid sb_uuid; > > }; > > -void ofctrl_flood_remove_flows(struct ovn_desired_flow_table *, > > +void ofctrl_flood_remove_flows(struct ovn_flow_table *, > > struct hmap *flood_remove_nodes); > > void ofctrl_flood_remove_add_node(struct hmap *flood_remove_nodes, > > const struct uuid *sb_uuid); > > -void ovn_desired_flow_table_init(struct ovn_desired_flow_table *); > > -void ovn_desired_flow_table_clear(struct ovn_desired_flow_table *); > > -void ovn_desired_flow_table_destroy(struct ovn_desired_flow_table *); > > - > > -void ofctrl_check_and_add_flow(struct ovn_desired_flow_table *, > > +void ofctrl_check_and_add_flow(struct ovn_flow_table *, > > uint8_t table_id, uint16_t priority, > > uint64_t cookie, const struct match *, > > const struct ofpbuf *ofpacts, > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > > index 4343650fc0..e150548f66 100644 > > --- a/controller/ovn-controller.c > > +++ b/controller/ovn-controller.c > > @@ -1645,107 +1645,14 @@ en_mff_ovn_geneve_run(struct engine_node *node, > > void *data) > > engine_set_node_state(node, EN_UNCHANGED); > > } > > > > -/* Engine node en_physical_flow_changes indicates whether > > - * there is a need to > > - * - recompute only physical flows or > > - * - we can incrementally process the physical flows. > > - * > > - * en_physical_flow_changes is an input to flow_output engine node. > > - * If the engine node 'en_physical_flow_changes' gets updated during > > - * engine run, it means the handler for this - > > - * flow_output_physical_flow_changes_handler() will either > > - * - recompute the physical flows by calling 'physical_run() or > > - * - incrementlly process some of the changes for physical flow > > - * calculation. Right now we handle OVS interfaces changes > > - * for physical flow computation. > > - * > > - * When ever a port binding happens, the follow up > > - * activity is the zone id allocation for that port binding. > > - * With this intermediate engine node, we avoid full recomputation. > > - * Instead we do physical flow computation (either full recomputation > > - * by calling physical_run() or handling the changes incrementally. > > - * > > - * Hence this is an intermediate engine node to indicate the > > - * flow_output engine to recomputes/compute the physical flows. > > - * > > - * TODO 1. Ideally this engine node should recompute/compute the physical > > - * flows instead of relegating it to the flow_output node. > > - * But this requires splitting the flow_output node to > > - * logical_flow_output and physical_flow_output. > > - * > > - * TODO 2. We can further optimise the en_ct_zone changes to > > - * compute the phsyical flows for changed zone ids. > > - * > > - * TODO 3: physical.c has a global simap -localvif_to_ofport which stores > > the > > - * local OVS interfaces and the ofport numbers. Ideally this > > should be > > - * part of the engine data. > > - */ > > -struct ed_type_pfc_data { > > - /* Both these variables are tracked and set in each engine run. */ > > - bool recompute_physical_flows; > > - bool ovs_ifaces_changed; > > -}; > > - > > -static void > > -en_physical_flow_changes_clear_tracked_data(void *data_) > > -{ > > - struct ed_type_pfc_data *data = data_; > > - data->recompute_physical_flows = false; > > - data->ovs_ifaces_changed = false; > > -} > > - > > -static void * > > -en_physical_flow_changes_init(struct engine_node *node OVS_UNUSED, > > - struct engine_arg *arg OVS_UNUSED) > > -{ > > - struct ed_type_pfc_data *data = xzalloc(sizeof *data); > > - return data; > > -} > > - > > -static void > > -en_physical_flow_changes_cleanup(void *data OVS_UNUSED) > > -{ > > -} > > - > > -/* Indicate to the flow_output engine that we need to recompute physical > > - * flows. */ > > -static void > > -en_physical_flow_changes_run(struct engine_node *node, void *data) > > -{ > > - struct ed_type_pfc_data *pfc_tdata = data; > > - pfc_tdata->recompute_physical_flows = true; > > - engine_set_node_state(node, EN_UPDATED); > > -} > > - > > -/* ct_zone changes are not handled incrementally but a handler is required > > - * to avoid skipping the ovs_iface incremental change handler. > > - */ > > -static bool > > -physical_flow_changes_ct_zones_handler(struct engine_node *node OVS_UNUSED, > > - void *data OVS_UNUSED) > > -{ > > - return false; > > -} > > - > > -/* There are OVS interface changes. Indicate to the flow_output engine > > - * to handle these OVS interface changes for physical flow computations. */ > > -static bool > > -physical_flow_changes_ovs_iface_handler(struct engine_node *node, void > > *data) > > -{ > > - struct ed_type_pfc_data *pfc_tdata = data; > > - pfc_tdata->ovs_ifaces_changed = true; > > - engine_set_node_state(node, EN_UPDATED); > > - return true; > > -} > > - > > -struct flow_output_persistent_data { > > +struct lflow_output_persistent_data { > > uint32_t conj_id_ofs; > > struct lflow_cache *lflow_cache; > > }; > > > > -struct ed_type_flow_output { > > - /* desired flows */ > > - struct ovn_desired_flow_table flow_table; > > +struct ed_type_lflow_output { > > + /* Logical flow table */ > > + struct ovn_flow_table *flow_table; > > You have changed its type but it is also now a pointer meaning that you > have to allocate and deallocate memory. What was the reason for this change? Prior to this change, the structure 'struct ovn_desired_flow_table' is defined in ofctrl.h and hence we could create an instance of it in 'struct ed_type_flow_output'. But now the struct ovn_flow_table is made opaque. It is now defined in ofctrl.c and hence here we need to allocate and deallocate dynamically. Also the struct ovn_desired_flow_table is part of 'struct ovn_flow_table'. *** struct ovn_flow_table { struct ovn_desired_flow_table desired_ftable; struct hmap installed_flows; }; *** We now need to maintain hmap of 'installed_flows' for both logical flows and physical flows. And I think there is no need to expose this outside of ofctrl.c. > > > /* group ids for load balancing */ > > struct ovn_extend_table group_table; > > /* meter ids for QoS */ > > @@ -1755,81 +1662,15 @@ struct ed_type_flow_output { > > > > /* Data which is persistent and not cleared during > > * full recompute. */ > > - struct flow_output_persistent_data pd; > > + struct lflow_output_persistent_data pd; > > }; > > > > -static void init_physical_ctx(struct engine_node *node, > > - struct ed_type_runtime_data *rt_data, > > - struct physical_ctx *p_ctx) > > -{ > > - struct ovsdb_idl_index *sbrec_port_binding_by_name = > > - engine_ovsdb_node_get_index( > > - engine_get_input("SB_port_binding", node), > > - "name"); > > - > > - struct sbrec_multicast_group_table *multicast_group_table = > > - (struct sbrec_multicast_group_table *)EN_OVSDB_GET( > > - engine_get_input("SB_multicast_group", node)); > > - > > - struct sbrec_port_binding_table *port_binding_table = > > - (struct sbrec_port_binding_table *)EN_OVSDB_GET( > > - engine_get_input("SB_port_binding", node)); > > - > > - struct sbrec_chassis_table *chassis_table = > > - (struct sbrec_chassis_table *)EN_OVSDB_GET( > > - engine_get_input("SB_chassis", node)); > > - > > - struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve = > > - engine_get_input_data("mff_ovn_geneve", node); > > - > > - struct ovsrec_open_vswitch_table *ovs_table = > > - (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET( > > - engine_get_input("OVS_open_vswitch", node)); > > - struct ovsrec_bridge_table *bridge_table = > > - (struct ovsrec_bridge_table *)EN_OVSDB_GET( > > - engine_get_input("OVS_bridge", node)); > > - const struct ovsrec_bridge *br_int = get_br_int(bridge_table, > > ovs_table); > > - const char *chassis_id = get_ovs_chassis_id(ovs_table); > > - const struct sbrec_chassis *chassis = NULL; > > - struct ovsdb_idl_index *sbrec_chassis_by_name = > > - engine_ovsdb_node_get_index( > > - engine_get_input("SB_chassis", node), > > - "name"); > > - if (chassis_id) { > > - chassis = chassis_lookup_by_name(sbrec_chassis_by_name, > > chassis_id); > > - } > > - > > - 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; > > - > > - p_ctx->sbrec_port_binding_by_name = sbrec_port_binding_by_name; > > - p_ctx->port_binding_table = port_binding_table; > > - 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; > > - p_ctx->ct_updated_datapaths = &rt_data->ct_updated_datapaths; > > -} > > - > > -static void init_lflow_ctx(struct engine_node *node, > > - struct ed_type_runtime_data *rt_data, > > - struct ed_type_flow_output *fo, > > - struct lflow_ctx_in *l_ctx_in, > > - struct lflow_ctx_out *l_ctx_out) > > +static void > > +init_lflow_ctx(struct engine_node *node, > > + struct ed_type_runtime_data *rt_data, > > + struct ed_type_lflow_output *fo, > > + struct lflow_ctx_in *l_ctx_in, > > + struct lflow_ctx_out *l_ctx_out) > > { > > struct ovsdb_idl_index *sbrec_port_binding_by_name = > > engine_ovsdb_node_get_index( > > @@ -1924,7 +1765,7 @@ static void init_lflow_ctx(struct engine_node *node, > > l_ctx_in->active_tunnels = &rt_data->active_tunnels; > > l_ctx_in->local_lport_ids = &rt_data->local_lport_ids; > > > > - l_ctx_out->flow_table = &fo->flow_table; > > + l_ctx_out->flow_table = fo->flow_table; > > l_ctx_out->group_table = &fo->group_table; > > l_ctx_out->meter_table = &fo->meter_table; > > l_ctx_out->lfrr = &fo->lflow_resource_ref; > > @@ -1934,12 +1775,12 @@ static void init_lflow_ctx(struct engine_node *node, > > } > > > > static void * > > -en_flow_output_init(struct engine_node *node OVS_UNUSED, > > - struct engine_arg *arg OVS_UNUSED) > > +en_lflow_output_init(struct engine_node *node OVS_UNUSED, > > + struct engine_arg *arg OVS_UNUSED) > > { > > - struct ed_type_flow_output *data = xzalloc(sizeof *data); > > + struct ed_type_lflow_output *data = xzalloc(sizeof *data); > > > > - ovn_desired_flow_table_init(&data->flow_table); > > + data->flow_table = ovn_flow_table_alloc(); > > ovn_extend_table_init(&data->group_table); > > ovn_extend_table_init(&data->meter_table); > > data->pd.conj_id_ofs = 1; > > @@ -1948,10 +1789,10 @@ en_flow_output_init(struct engine_node *node > > OVS_UNUSED, > > } > > > > static void > > -en_flow_output_cleanup(void *data) > > +en_lflow_output_cleanup(void *data) > > { > > - struct ed_type_flow_output *flow_output_data = data; > > - ovn_desired_flow_table_destroy(&flow_output_data->flow_table); > > + struct ed_type_lflow_output *flow_output_data = data; > > + ovn_flow_table_destroy(flow_output_data->flow_table); > > ovn_extend_table_destroy(&flow_output_data->group_table); > > ovn_extend_table_destroy(&flow_output_data->meter_table); > > lflow_resource_destroy(&flow_output_data->lflow_resource_ref); > > @@ -1959,7 +1800,7 @@ en_flow_output_cleanup(void *data) > > } > > > > static void > > -en_flow_output_run(struct engine_node *node, void *data) > > +en_lflow_output_run(struct engine_node *node, void *data) > > { > > struct ed_type_runtime_data *rt_data = > > engine_get_input_data("runtime_data", node); > > @@ -1985,8 +1826,8 @@ en_flow_output_run(struct engine_node *node, void > > *data) > > > > ovs_assert(br_int && chassis); > > > > - struct ed_type_flow_output *fo = data; > > - struct ovn_desired_flow_table *flow_table = &fo->flow_table; > > + struct ed_type_lflow_output *fo = data; > > + struct ovn_flow_table *lflow_table = fo->flow_table; > > struct ovn_extend_table *group_table = &fo->group_table; > > struct ovn_extend_table *meter_table = &fo->meter_table; > > struct lflow_resource_ref *lfrr = &fo->lflow_resource_ref; > > @@ -1995,7 +1836,7 @@ en_flow_output_run(struct engine_node *node, void > > *data) > > if (first_run) { > > first_run = false; > > } else { > > - ovn_desired_flow_table_clear(flow_table); > > + ovn_flow_table_clear(lflow_table); > > ovn_extend_table_clear(group_table, false /* desired */); > > ovn_extend_table_clear(meter_table, false /* desired */); > > lflow_resource_clear(lfrr); > > @@ -2017,7 +1858,7 @@ en_flow_output_run(struct engine_node *node, void > > *data) > > if (l_ctx_out.conj_id_overflow) { > > /* Conjunction ids overflow. There can be many holes in between. > > * Destroy lflow cache and call lflow_run() again. */ > > - ovn_desired_flow_table_clear(flow_table); > > + ovn_flow_table_clear(lflow_table); > > ovn_extend_table_clear(group_table, false /* desired */); > > ovn_extend_table_clear(meter_table, false /* desired */); > > lflow_resource_clear(lfrr); > > @@ -2030,16 +1871,11 @@ en_flow_output_run(struct engine_node *node, void > > *data) > > } > > } > > > > - struct physical_ctx p_ctx; > > - init_physical_ctx(node, rt_data, &p_ctx); > > - > > - physical_run(&p_ctx, &fo->flow_table); > > - > > engine_set_node_state(node, EN_UPDATED); > > } > > > > static bool > > -flow_output_sb_logical_flow_handler(struct engine_node *node, void *data) > > +lflow_output_sb_logical_flow_handler(struct engine_node *node, void *data) > > { > > struct ed_type_runtime_data *rt_data = > > engine_get_input_data("runtime_data", node); > > @@ -2052,7 +1888,7 @@ flow_output_sb_logical_flow_handler(struct > > engine_node *node, void *data) > > const struct ovsrec_bridge *br_int = get_br_int(bridge_table, > > ovs_table); > > ovs_assert(br_int); > > > > - struct ed_type_flow_output *fo = data; > > + struct ed_type_lflow_output *fo = data; > > struct lflow_ctx_in l_ctx_in; > > struct lflow_ctx_out l_ctx_out; > > init_lflow_ctx(node, rt_data, fo, &l_ctx_in, &l_ctx_out); > > @@ -2064,7 +1900,7 @@ flow_output_sb_logical_flow_handler(struct > > engine_node *node, void *data) > > } > > > > static bool > > -flow_output_sb_mac_binding_handler(struct engine_node *node, void *data) > > +lflow_output_sb_mac_binding_handler(struct engine_node *node, void *data) > > { > > struct ovsdb_idl_index *sbrec_port_binding_by_name = > > engine_ovsdb_node_get_index( > > @@ -2079,60 +1915,17 @@ flow_output_sb_mac_binding_handler(struct > > engine_node *node, void *data) > > engine_get_input_data("runtime_data", node); > > const struct hmap *local_datapaths = &rt_data->local_datapaths; > > > > - struct ed_type_flow_output *fo = data; > > - struct ovn_desired_flow_table *flow_table = &fo->flow_table; > > + struct ed_type_lflow_output *lfo = data; > > > > lflow_handle_changed_neighbors(sbrec_port_binding_by_name, > > - mac_binding_table, local_datapaths, flow_table); > > + mac_binding_table, local_datapaths, lfo->flow_table); > > > > engine_set_node_state(node, EN_UPDATED); > > return true; > > } > > > > static bool > > -flow_output_sb_port_binding_handler(struct engine_node *node, > > - void *data) > > -{ > > - struct ed_type_runtime_data *rt_data = > > - engine_get_input_data("runtime_data", node); > > - > > - struct ed_type_flow_output *fo = data; > > - struct ovn_desired_flow_table *flow_table = &fo->flow_table; > > - > > - struct physical_ctx p_ctx; > > - init_physical_ctx(node, rt_data, &p_ctx); > > - > > - /* We handle port-binding changes for physical flow processing > > - * only. flow_output runtime data handler takes care of processing > > - * logical flows for any port binding changes. > > - */ > > - physical_handle_port_binding_changes(&p_ctx, flow_table); > > - > > - engine_set_node_state(node, EN_UPDATED); > > - return true; > > -} > > - > > -static bool > > -flow_output_sb_multicast_group_handler(struct engine_node *node, void > > *data) > > -{ > > - struct ed_type_runtime_data *rt_data = > > - engine_get_input_data("runtime_data", node); > > - > > - struct ed_type_flow_output *fo = data; > > - struct ovn_desired_flow_table *flow_table = &fo->flow_table; > > - > > - struct physical_ctx p_ctx; > > - init_physical_ctx(node, rt_data, &p_ctx); > > - > > - physical_handle_mc_group_changes(&p_ctx, flow_table); > > - > > - engine_set_node_state(node, EN_UPDATED); > > - return true; > > - > > -} > > - > > -static bool > > -_flow_output_resource_ref_handler(struct engine_node *node, void *data, > > +_lflow_output_resource_ref_handler(struct engine_node *node, void *data, > > enum ref_type ref_type) > > { > > struct ed_type_runtime_data *rt_data = > > @@ -2164,7 +1957,7 @@ _flow_output_resource_ref_handler(struct engine_node > > *node, void *data, > > > > ovs_assert(br_int && chassis); > > > > - struct ed_type_flow_output *fo = data; > > + struct ed_type_lflow_output *fo = data; > > > > struct lflow_ctx_in l_ctx_in; > > struct lflow_ctx_out l_ctx_out; > > @@ -2233,53 +2026,20 @@ _flow_output_resource_ref_handler(struct > > engine_node *node, void *data, > > } > > > > static bool > > -flow_output_addr_sets_handler(struct engine_node *node, void *data) > > +lflow_output_addr_sets_handler(struct engine_node *node, void *data) > > { > > - return _flow_output_resource_ref_handler(node, data, REF_TYPE_ADDRSET); > > + return _lflow_output_resource_ref_handler(node, data, > > REF_TYPE_ADDRSET); > > } > > > > static bool > > -flow_output_port_groups_handler(struct engine_node *node, void *data) > > +lflow_output_port_groups_handler(struct engine_node *node, void *data) > > { > > - return _flow_output_resource_ref_handler(node, data, > > REF_TYPE_PORTGROUP); > > -} > > - > > -static bool > > -flow_output_physical_flow_changes_handler(struct engine_node *node, void > > *data) > > -{ > > - 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); > > - struct ed_type_pfc_data *pfc_data = > > - engine_get_input_data("physical_flow_changes", node); > > - > > - /* If there are OVS interface changes. Try to handle them > > incrementally. */ > > - if (pfc_data->ovs_ifaces_changed) { > > - if (!physical_handle_ovs_iface_changes(&p_ctx, &fo->flow_table)) { > > - return false; > > - } > > - } > > - > > - if (pfc_data->recompute_physical_flows) { > > - /* This indicates that we need to recompute the physical flows. */ > > - physical_clear_unassoc_flows_with_db(&fo->flow_table); > > - physical_clear_dp_flows(&p_ctx, &rt_data->ct_updated_datapaths, > > - &fo->flow_table); > > - physical_run(&p_ctx, &fo->flow_table); > > - return true; > > - } > > - > > - return true; > > + return _lflow_output_resource_ref_handler(node, data, > > REF_TYPE_PORTGROUP); > > } > > > > static bool > > -flow_output_runtime_data_handler(struct engine_node *node, > > - void *data OVS_UNUSED) > > +lflow_output_runtime_data_handler(struct engine_node *node, > > + void *data OVS_UNUSED) > > { > > struct ed_type_runtime_data *rt_data = > > engine_get_input_data("runtime_data", node); > > @@ -2300,12 +2060,9 @@ flow_output_runtime_data_handler(struct engine_node > > *node, > > > > struct lflow_ctx_in l_ctx_in; > > struct lflow_ctx_out l_ctx_out; > > - struct ed_type_flow_output *fo = data; > > + struct ed_type_lflow_output *fo = data; > > init_lflow_ctx(node, rt_data, fo, &l_ctx_in, &l_ctx_out); > > > > - struct physical_ctx p_ctx; > > - init_physical_ctx(node, rt_data, &p_ctx); > > - > > struct tracked_binding_datapath *tdp; > > HMAP_FOR_EACH (tdp, node, tracked_dp_bindings) { > > if (tdp->is_new) { > > @@ -2330,12 +2087,12 @@ flow_output_runtime_data_handler(struct engine_node > > *node, > > } > > > > static bool > > -flow_output_sb_load_balancer_handler(struct engine_node *node, void *data) > > +lflow_output_sb_load_balancer_handler(struct engine_node *node, void *data) > > { > > struct ed_type_runtime_data *rt_data = > > engine_get_input_data("runtime_data", node); > > > > - struct ed_type_flow_output *fo = data; > > + struct ed_type_lflow_output *fo = data; > > struct lflow_ctx_in l_ctx_in; > > struct lflow_ctx_out l_ctx_out; > > init_lflow_ctx(node, rt_data, fo, &l_ctx_in, &l_ctx_out); > > @@ -2346,6 +2103,235 @@ flow_output_sb_load_balancer_handler(struct > > engine_node *node, void *data) > > return handled; > > } > > > > +struct ed_type_pflow_output { > > + /* Desired physical flows. */ > > + struct ovn_flow_table *flow_table; > > +}; > > + > > +static void init_physical_ctx(struct engine_node *node, > > + struct ed_type_runtime_data *rt_data, > > + struct physical_ctx *p_ctx) > > +{ > > + struct ovsdb_idl_index *sbrec_port_binding_by_name = > > + engine_ovsdb_node_get_index( > > + engine_get_input("SB_port_binding", node), > > + "name"); > > + > > + struct sbrec_multicast_group_table *multicast_group_table = > > + (struct sbrec_multicast_group_table *)EN_OVSDB_GET( > > + engine_get_input("SB_multicast_group", node)); > > + > > + struct sbrec_port_binding_table *port_binding_table = > > + (struct sbrec_port_binding_table *)EN_OVSDB_GET( > > + engine_get_input("SB_port_binding", node)); > > + > > + struct sbrec_chassis_table *chassis_table = > > + (struct sbrec_chassis_table *)EN_OVSDB_GET( > > + engine_get_input("SB_chassis", node)); > > + > > + struct ed_type_mff_ovn_geneve *ed_mff_ovn_geneve = > > + engine_get_input_data("mff_ovn_geneve", node); > > + > > + struct ovsrec_open_vswitch_table *ovs_table = > > + (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET( > > + engine_get_input("OVS_open_vswitch", node)); > > + struct ovsrec_bridge_table *bridge_table = > > + (struct ovsrec_bridge_table *)EN_OVSDB_GET( > > + engine_get_input("OVS_bridge", node)); > > + const struct ovsrec_bridge *br_int = get_br_int(bridge_table, > > ovs_table); > > + const char *chassis_id = get_ovs_chassis_id(ovs_table); > > + const struct sbrec_chassis *chassis = NULL; > > + struct ovsdb_idl_index *sbrec_chassis_by_name = > > + engine_ovsdb_node_get_index( > > + engine_get_input("SB_chassis", node), > > + "name"); > > + if (chassis_id) { > > + chassis = chassis_lookup_by_name(sbrec_chassis_by_name, > > chassis_id); > > + } > > + > > + 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; > > + > > + p_ctx->sbrec_port_binding_by_name = sbrec_port_binding_by_name; > > + p_ctx->port_binding_table = port_binding_table; > > + 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; > > + p_ctx->ct_updated_datapaths = &rt_data->ct_updated_datapaths; > > +} > > + > > +static void * > > +en_pflow_output_init(struct engine_node *node OVS_UNUSED, > > + struct engine_arg *arg OVS_UNUSED) > > +{ > > + struct ed_type_pflow_output *data = xzalloc(sizeof *data); > > + data->flow_table = ovn_flow_table_alloc(); > > + return data; > > +} > > + > > +static void > > +en_pflow_output_cleanup(void *data OVS_UNUSED) > > +{ > > + struct ed_type_pflow_output *pfo = data; > > + ovn_flow_table_destroy(pfo->flow_table); > > +} > > + > > +/* Indicate to the flow_output engine that we need to recompute physical > > + * flows. */ > > +static void > > +en_pflow_output_run(struct engine_node *node, void *data) > > +{ > > + struct ed_type_pflow_output *pfo = data; > > + struct ovn_flow_table *pflow_table = pfo->flow_table; > > + static bool first_run = true; > > + if (first_run) { > > + first_run = false; > > + } else { > > + ovn_flow_table_clear(pflow_table); > > + } > > + > > + struct ed_type_runtime_data *rt_data = > > + engine_get_input_data("runtime_data", node); > > + > > + struct physical_ctx p_ctx; > > + init_physical_ctx(node, rt_data, &p_ctx); > > + physical_run(&p_ctx, pflow_table); > > + > > + engine_set_node_state(node, EN_UPDATED); > > +} > > + > > +static bool > > +pflow_output_sb_port_binding_handler(struct engine_node *node, > > + void *data) > > +{ > > + struct ed_type_runtime_data *rt_data = > > + engine_get_input_data("runtime_data", node); > > + > > + struct ed_type_pflow_output *pfo = data; > > + > > + struct physical_ctx p_ctx; > > + init_physical_ctx(node, rt_data, &p_ctx); > > + > > + /* We handle port-binding changes for physical flow processing > > + * only. flow_output runtime data handler takes care of processing > > + * logical flows for any port binding changes. > > + */ > > + physical_handle_port_binding_changes(&p_ctx, pfo->flow_table); > > + > > + engine_set_node_state(node, EN_UPDATED); > > + return true; > > +} > > + > > +static bool > > +pflow_output_sb_multicast_group_handler(struct engine_node *node, void > > *data) > > +{ > > + struct ed_type_runtime_data *rt_data = > > + engine_get_input_data("runtime_data", node); > > + > > + struct ed_type_pflow_output *pfo = data; > > + > > + struct physical_ctx p_ctx; > > + init_physical_ctx(node, rt_data, &p_ctx); > > + > > + physical_handle_mc_group_changes(&p_ctx, pfo->flow_table); > > + > > + engine_set_node_state(node, EN_UPDATED); > > + return true; > > +} > > + > > +/* There are OVS interface changes. Indicate to the flow_output engine > > + * to handle these OVS interface changes for physical flow computations. */ > > +static bool > > +pflow_output_ovs_iface_handler(struct engine_node *node OVS_UNUSED, > > + void *data OVS_UNUSED) > > +{ > > + struct ed_type_runtime_data *rt_data = > > + engine_get_input_data("runtime_data", node); > > + > > + struct ed_type_pflow_output *pfo = data; > > + > > + struct physical_ctx p_ctx; > > + init_physical_ctx(node, rt_data, &p_ctx); > > + > > + engine_set_node_state(node, EN_UPDATED); > > + return physical_handle_ovs_iface_changes(&p_ctx, pfo->flow_table); > > +} > > + > > +/* Handles sbrec_chassis changes. > > + * If a new chassis is added or removed return false, so that > > + * physical flows are programmed. > > + * For any updates, there is no need for any flow computation. > > + * Encap changes will also result in sbrec_chassis changes, > > + * but we handle encap changes separately. > > + */ > > +static bool > > +pflow_output_sb_chassis_handler(struct engine_node *node, > > + void *data OVS_UNUSED) > > +{ > > + struct sbrec_chassis_table *chassis_table = > > + (struct sbrec_chassis_table *)EN_OVSDB_GET( > > + engine_get_input("SB_chassis", node)); > > + > > + const struct sbrec_chassis *ch; > > + SBREC_CHASSIS_TABLE_FOR_EACH_TRACKED (ch, chassis_table) { > > + if (sbrec_chassis_is_deleted(ch) || sbrec_chassis_is_new(ch)) { > > + return false; > > + } > > + } > > + > > + return true; > > +} > > + > > +static void * > > +en_flow_output_init(struct engine_node *node OVS_UNUSED, > > + struct engine_arg *arg OVS_UNUSED) > > +{ > > + return NULL; > > +} > > + > > +static void > > +en_flow_output_cleanup(void *data OVS_UNUSED) > > +{ > > + > > +} > > + > > +static void > > +en_flow_output_run(struct engine_node *node OVS_UNUSED, void *data > > OVS_UNUSED) > > +{ > > + engine_set_node_state(node, EN_UPDATED); > > +} > > + > > +static bool > > +flow_output_pflow_output_handler(struct engine_node *node, > > + void *data OVS_UNUSED) > > +{ > > + engine_set_node_state(node, EN_UPDATED); > > + return true; > > +} > > + > > +static bool > > +flow_output_lflow_output_handler(struct engine_node *node, > > + void *data OVS_UNUSED) > > +{ > > + engine_set_node_state(node, EN_UPDATED); > > + return true; > > +} > > + > > struct ovn_controller_exit_args { > > bool *exiting; > > bool *restart; > > @@ -2532,8 +2518,8 @@ main(int argc, char *argv[]) > > ENGINE_NODE_WITH_CLEAR_TRACK_DATA(runtime_data, "runtime_data"); > > ENGINE_NODE(mff_ovn_geneve, "mff_ovn_geneve"); > > ENGINE_NODE(ofctrl_is_connected, "ofctrl_is_connected"); > > - ENGINE_NODE_WITH_CLEAR_TRACK_DATA(physical_flow_changes, > > - "physical_flow_changes"); > > + ENGINE_NODE(pflow_output, "physical_flow_output"); > > + ENGINE_NODE(lflow_output, "logical_flow_output"); > > ENGINE_NODE(flow_output, "flow_output"); > > ENGINE_NODE(addr_sets, "addr_sets"); > > ENGINE_NODE(port_groups, "port_groups"); > > @@ -2553,56 +2539,63 @@ main(int argc, char *argv[]) > > engine_add_input(&en_port_groups, &en_sb_port_group, > > port_groups_sb_port_group_handler); > > > > - /* Engine node physical_flow_changes indicates whether > > - * we can recompute only physical flows or we can > > - * incrementally process the physical flows. > > - * > > - * Note: The order of inputs is important, all OVS interface changes > > must > > + /* Note: The order of inputs is important, all OVS interface changes > > must > > * be handled before any ct_zone changes. > > */ > > - engine_add_input(&en_physical_flow_changes, &en_ovs_interface, > > - physical_flow_changes_ovs_iface_handler); > > - engine_add_input(&en_physical_flow_changes, &en_ct_zones, > > - physical_flow_changes_ct_zones_handler); > > - > > - 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, > > - flow_output_runtime_data_handler); > > - 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_pflow_output, &en_ovs_interface, > > + pflow_output_ovs_iface_handler); > > + engine_add_input(&en_pflow_output, &en_ct_zones, > > + NULL); > > + engine_add_input(&en_pflow_output, &en_sb_chassis, > > + pflow_output_sb_chassis_handler); > > + engine_add_input(&en_pflow_output, &en_sb_port_binding, > > + pflow_output_sb_port_binding_handler); > > + engine_add_input(&en_pflow_output, &en_sb_multicast_group, > > + pflow_output_sb_multicast_group_handler); > > + engine_add_input(&en_pflow_output, &en_runtime_data, > > + engine_noop_handler); > > + engine_add_input(&en_pflow_output, &en_sb_encap, NULL); > > + engine_add_input(&en_pflow_output, &en_mff_ovn_geneve, NULL); > > + engine_add_input(&en_pflow_output, &en_ovs_open_vswitch, NULL); > > + engine_add_input(&en_pflow_output, &en_ovs_bridge, NULL); > > + > > + engine_add_input(&en_lflow_output, &en_addr_sets, > > + lflow_output_addr_sets_handler); > > + engine_add_input(&en_lflow_output, &en_port_groups, > > + lflow_output_port_groups_handler); > > + engine_add_input(&en_lflow_output, &en_runtime_data, > > + lflow_output_runtime_data_handler); > > > > /* We need this input nodes for only data. Hence the noop handler. */ > > - engine_add_input(&en_flow_output, &en_ct_zones, engine_noop_handler); > > - engine_add_input(&en_flow_output, &en_ovs_interface, > > engine_noop_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_sb_chassis, NULL); > > - engine_add_input(&en_flow_output, &en_sb_encap, NULL); > > - engine_add_input(&en_flow_output, &en_sb_multicast_group, > > - flow_output_sb_multicast_group_handler); > > - engine_add_input(&en_flow_output, &en_sb_port_binding, > > - flow_output_sb_port_binding_handler); > > - engine_add_input(&en_flow_output, &en_sb_mac_binding, > > - flow_output_sb_mac_binding_handler); > > - engine_add_input(&en_flow_output, &en_sb_logical_flow, > > - flow_output_sb_logical_flow_handler); > > + engine_add_input(&en_lflow_output, &en_ct_zones, > > + engine_noop_handler); > > + engine_add_input(&en_lflow_output, &en_ovs_interface, > > + engine_noop_handler); > > + engine_add_input(&en_lflow_output, &en_sb_chassis, > > + engine_noop_handler); > > + engine_add_input(&en_lflow_output, &en_sb_multicast_group, > > + engine_noop_handler); > > + engine_add_input(&en_lflow_output, &en_sb_port_binding, > > + engine_noop_handler); > > + > > + engine_add_input(&en_lflow_output, &en_ovs_open_vswitch, NULL); > > + engine_add_input(&en_lflow_output, &en_ovs_bridge, NULL); > > + > > + engine_add_input(&en_lflow_output, &en_sb_mac_binding, > > + lflow_output_sb_mac_binding_handler); > > + engine_add_input(&en_lflow_output, &en_sb_logical_flow, > > + lflow_output_sb_logical_flow_handler); > > /* Using a noop handler since we don't really need any data from > > datapath > > * groups or a full recompute. Update of a datapath group will put > > * logical flow into the tracked list, so the logical flow handler will > > * process all changes. */ > > - engine_add_input(&en_flow_output, &en_sb_logical_dp_group, > > + engine_add_input(&en_lflow_output, &en_sb_logical_dp_group, > > engine_noop_handler); > > - engine_add_input(&en_flow_output, &en_sb_dhcp_options, NULL); > > - engine_add_input(&en_flow_output, &en_sb_dhcpv6_options, NULL); > > - engine_add_input(&en_flow_output, &en_sb_dns, NULL); > > - engine_add_input(&en_flow_output, &en_sb_load_balancer, > > - flow_output_sb_load_balancer_handler); > > + engine_add_input(&en_lflow_output, &en_sb_dhcp_options, NULL); > > + engine_add_input(&en_lflow_output, &en_sb_dhcpv6_options, NULL); > > + engine_add_input(&en_lflow_output, &en_sb_dns, NULL); > > + engine_add_input(&en_lflow_output, &en_sb_load_balancer, > > + lflow_output_sb_load_balancer_handler); > > > > engine_add_input(&en_ct_zones, &en_ovs_open_vswitch, NULL); > > engine_add_input(&en_ct_zones, &en_ovs_bridge, NULL); > > @@ -2630,6 +2623,11 @@ main(int argc, char *argv[]) > > engine_add_input(&en_runtime_data, &en_ovs_interface, > > runtime_data_ovs_interface_handler); > > > > + engine_add_input(&en_flow_output, &en_lflow_output, > > + flow_output_lflow_output_handler); > > + engine_add_input(&en_flow_output, &en_pflow_output, > > + flow_output_pflow_output_handler); > > + > > struct engine_arg engine_arg = { > > .sb_idl = ovnsb_idl_loop.idl, > > .ovs_idl = ovs_idl_loop.idl, > > @@ -2652,24 +2650,28 @@ main(int argc, char *argv[]) > > engine_ovsdb_node_add_index(&en_sb_datapath_binding, "key", > > sbrec_datapath_binding_by_key); > > > > - struct ed_type_flow_output *flow_output_data = > > - engine_get_internal_data(&en_flow_output); > > + struct ed_type_lflow_output *lflow_output_data = > > + engine_get_internal_data(&en_lflow_output); > > + struct ed_type_lflow_output *pflow_output_data = > > + engine_get_internal_data(&en_pflow_output); > > struct ed_type_ct_zones *ct_zones_data = > > engine_get_internal_data(&en_ct_zones); > > struct ed_type_runtime_data *runtime_data = NULL; > > > > - ofctrl_init(&flow_output_data->group_table, > > - &flow_output_data->meter_table, > > + ofctrl_init(&lflow_output_data->group_table, > > + &lflow_output_data->meter_table, > > + lflow_output_data->flow_table, > > + pflow_output_data->flow_table, > > get_ofctrl_probe_interval(ovs_idl_loop.idl)); > > ofctrl_seqno_init(); > > > > unixctl_command_register("group-table-list", "", 0, 0, > > extend_table_list, > > - &flow_output_data->group_table); > > + &lflow_output_data->group_table); > > > > unixctl_command_register("meter-table-list", "", 0, 0, > > extend_table_list, > > - &flow_output_data->meter_table); > > + &lflow_output_data->meter_table); > > > > unixctl_command_register("ct-zone-list", "", 0, 0, > > ct_zone_list, > > @@ -2683,14 +2685,14 @@ main(int argc, char *argv[]) > > NULL); > > unixctl_command_register("lflow-cache/flush", "", 0, 0, > > lflow_cache_flush_cmd, > > - &flow_output_data->pd); > > + &lflow_output_data->pd); > > /* Keep deprecated 'flush-lflow-cache' command for now. */ ..> > unixctl_command_register("flush-lflow-cache", "[deprecated]", 0, 0, > > lflow_cache_flush_cmd, > > - &flow_output_data->pd); > > + &lflow_output_data->pd); > > unixctl_command_register("lflow-cache/show-stats", "", 0, 0, > > lflow_cache_show_stats_cmd, > > - &flow_output_data->pd); > > + &lflow_output_data->pd); > > > > bool reset_ovnsb_idl_min_index = false; > > unixctl_command_register("sb-cluster-state-reset", "", 0, 0, > > @@ -2928,13 +2930,20 @@ main(int argc, char *argv[]) > > binding_seqno_run(&runtime_data->local_bindings); > > } > > > > - flow_output_data = engine_get_data(&en_flow_output); > > - if (flow_output_data && ct_zones_data) { > > - ofctrl_put(&flow_output_data->flow_table, > > + lflow_output_data = engine_get_data(&en_lflow_output); > > + pflow_output_data = engine_get_data(&en_pflow_output); > > + if (lflow_output_data && pflow_output_data && > > What is this ^ "if" clause for? Why would these nodes not have the > associated data? Should this be an || operator? engine_get_data() would return NULL if the engine state is not in EN_UPDATED or EN_UNCHANGED. Before this patch we were having the if condition - if (flow_output_data && ct_zones_data) { ..} And now since we have split flow_output_data into lflow_output_data and pflow_output_data, hence the if clause. Thanks Numan > > + ct_zones_data) { > > + bool flow_changed = > > + (engine_node_changed(&en_lflow_output) || > > + engine_node_changed(&en_pflow_output)); > > + > > + ofctrl_put(lflow_output_data->flow_table, > > + pflow_output_data->flow_table, > > &ct_zones_data->pending, > > > > sbrec_meter_table_get(ovnsb_idl_loop.idl), > > ofctrl_seqno_get_req_cfg(), > > - engine_node_changed(&en_flow_output)); > > + flow_changed); > > } > > ofctrl_seqno_run(ofctrl_get_cur_cfg()); > > if (runtime_data && ovs_idl_txn && ovnsb_idl_txn) { > > @@ -3290,7 +3299,7 @@ lflow_cache_flush_cmd(struct unixctl_conn *conn > > OVS_UNUSED, > > void *arg_) > > { > > VLOG_INFO("User triggered lflow cache flush."); > > - struct flow_output_persistent_data *fo_pd = arg_; > > + struct lflow_output_persistent_data *fo_pd = arg_; > > lflow_cache_flush(fo_pd->lflow_cache); > > fo_pd->conj_id_ofs = 1; > > engine_set_force_recompute(true); > > @@ -3302,7 +3311,7 @@ static void > > lflow_cache_show_stats_cmd(struct unixctl_conn *conn, int argc OVS_UNUSED, > > const char *argv[] OVS_UNUSED, void *arg_) > > { > > - struct flow_output_persistent_data *fo_pd = arg_; > > + struct lflow_output_persistent_data *fo_pd = arg_; > > struct lflow_cache *lc = fo_pd->lflow_cache; > > struct ds ds = DS_EMPTY_INITIALIZER; > > > > diff --git a/controller/physical.c b/controller/physical.c > > index fa5d0d692d..7ecf6f72ef 100644 > > --- a/controller/physical.c > > +++ b/controller/physical.c > > @@ -249,7 +249,7 @@ put_remote_port_redirect_bridged(const struct > > struct local_datapath *ld, > > struct match *match, > > struct ofpbuf *ofpacts_p, > > - struct ovn_desired_flow_table *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > if (strcmp(binding->type, "chassisredirect")) { > > /* bridged based redirect is only supported for chassisredirect > > @@ -321,7 +321,7 @@ put_remote_port_redirect_overlay(const struct > > uint32_t port_key, > > struct match *match, > > struct ofpbuf *ofpacts_p, > > - struct ovn_desired_flow_table *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > if (!is_ha_remote) { > > /* Setup encapsulation */ > > @@ -482,7 +482,7 @@ static void > > put_chassis_mac_conj_id_flow(const struct sbrec_chassis_table > > *chassis_table, > > const struct sbrec_chassis *chassis, > > struct ofpbuf *ofpacts_p, > > - struct ovn_desired_flow_table *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > struct match match; > > struct remote_chassis_mac *mac; > > @@ -525,7 +525,7 @@ put_replace_chassis_mac_flows(const struct simap > > *ct_zones, > > const struct hmap *local_datapaths, > > struct ofpbuf *ofpacts_p, > > ofp_port_t ofport, > > - struct ovn_desired_flow_table *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > /* Packets arriving on localnet port, could have been routed on > > * source chassis and hence will have a chassis mac. > > @@ -619,7 +619,7 @@ put_replace_router_port_mac_flows(struct ovsdb_idl_index > > const struct hmap *local_datapaths, > > struct ofpbuf *ofpacts_p, > > ofp_port_t ofport, > > - struct ovn_desired_flow_table > > *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > struct local_datapath *ld = get_local_datapath(local_datapaths, > > > > localnet_port->datapath-> > > @@ -715,7 +715,7 @@ put_local_common_flows(uint32_t dp_key, uint32_t > > port_key, > > uint32_t parent_port_key, > > const struct zone_ids *zone_ids, > > struct ofpbuf *ofpacts_p, > > - struct ovn_desired_flow_table *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > struct match match; > > > > @@ -891,7 +891,7 @@ consider_port_binding(struct ovsdb_idl_index > > *sbrec_port_binding_by_name, > > const struct hmap *local_datapaths, > > const struct sbrec_port_binding *binding, > > const struct sbrec_chassis *chassis, > > - struct ovn_desired_flow_table *flow_table, > > + struct ovn_flow_table *flow_table, > > struct ofpbuf *ofpacts_p) > > { > > uint32_t dp_key = binding->datapath->tunnel_key; > > @@ -1287,7 +1287,7 @@ consider_mc_group(enum mf_field_id mff_ovn_geneve, > > const struct hmap *local_datapaths, > > const struct sbrec_chassis *chassis, > > const struct sbrec_multicast_group *mc, > > - struct ovn_desired_flow_table *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > uint32_t dp_key = mc->datapath->tunnel_key; > > if (!get_local_datapath(local_datapaths, dp_key)) { > > @@ -1426,7 +1426,7 @@ update_ofports(struct simap *old, struct simap *new) > > > > void > > physical_handle_port_binding_changes(struct physical_ctx *p_ctx, > > - struct ovn_desired_flow_table > > *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > const struct sbrec_port_binding *binding; > > struct ofpbuf ofpacts; > > @@ -1452,7 +1452,7 @@ physical_handle_port_binding_changes(struct > > physical_ctx *p_ctx, > > > > void > > physical_handle_mc_group_changes(struct physical_ctx *p_ctx, > > - struct ovn_desired_flow_table *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > const struct sbrec_multicast_group *mc; > > SBREC_MULTICAST_GROUP_TABLE_FOR_EACH_TRACKED (mc, > > p_ctx->mc_group_table) { > > @@ -1471,7 +1471,7 @@ physical_handle_mc_group_changes(struct physical_ctx > > *p_ctx, > > > > void > > physical_run(struct physical_ctx *p_ctx, > > - struct ovn_desired_flow_table *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > if (!hc_uuid) { > > hc_uuid = xmalloc(sizeof(struct uuid)); > > @@ -1818,7 +1818,7 @@ physical_run(struct physical_ctx *p_ctx, > > > > bool > > physical_handle_ovs_iface_changes(struct physical_ctx *p_ctx, > > - struct ovn_desired_flow_table > > *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > const struct ovsrec_interface *iface_rec; > > OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, > > p_ctx->iface_table) { > > @@ -1883,7 +1883,7 @@ get_tunnel_ofport(const char *chassis_name, char > > *encap_ip, ofp_port_t *ofport) > > } > > > > void > > -physical_clear_unassoc_flows_with_db(struct ovn_desired_flow_table > > *flow_table) > > +physical_clear_unassoc_flows_with_db(struct ovn_flow_table *flow_table) > > { > > if (hc_uuid) { > > ofctrl_remove_flows(flow_table, hc_uuid); > > @@ -1893,7 +1893,7 @@ physical_clear_unassoc_flows_with_db(struct > > ovn_desired_flow_table *flow_table) > > void > > physical_clear_dp_flows(struct physical_ctx *p_ctx, > > struct hmapx *ct_updated_datapaths, > > - struct ovn_desired_flow_table *flow_table) > > + struct ovn_flow_table *flow_table) > > { > > const struct sbrec_port_binding *binding; > > SBREC_PORT_BINDING_TABLE_FOR_EACH (binding, p_ctx->port_binding_table) > > { > > diff --git a/controller/physical.h b/controller/physical.h > > index 0bf13f2683..ccd0672ec4 100644 > > --- a/controller/physical.h > > +++ b/controller/physical.h > > @@ -61,17 +61,17 @@ struct physical_ctx { > > > > void physical_register_ovs_idl(struct ovsdb_idl *); > > void physical_run(struct physical_ctx *, > > - struct ovn_desired_flow_table *); > > -void physical_clear_unassoc_flows_with_db(struct ovn_desired_flow_table *); > > + struct ovn_flow_table *); > > +void physical_clear_unassoc_flows_with_db(struct ovn_flow_table *); > > void physical_clear_dp_flows(struct physical_ctx *p_ctx, > > struct hmapx *ct_updated_datapaths, > > - struct ovn_desired_flow_table *flow_table); > > + struct ovn_flow_table *flow_table); > > void physical_handle_port_binding_changes(struct physical_ctx *, > > - struct ovn_desired_flow_table *); > > + struct ovn_flow_table *); > > void physical_handle_mc_group_changes(struct physical_ctx *, > > - struct ovn_desired_flow_table *); > > + struct ovn_flow_table *); > > bool physical_handle_ovs_iface_changes(struct physical_ctx *, > > - struct ovn_desired_flow_table *); > > + struct ovn_flow_table *); > > bool get_tunnel_ofport(const char *chassis_name, char *encap_ip, > > ofp_port_t *ofport); > > #endif /* controller/physical.h */ > > > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
