[ovs-dev] [PATCH ovn v3] ovn-controller: Split logical flow and phsyical flow processing.
Mark Gray
mark.d.gray at redhat.com
Tue Mar 2 09:48:37 UTC 2021
On 01/03/2021 18:55, Numan Siddique wrote:
> an
>
> On Thu, Feb 25, 2021 at 3:42 PM Mark Gray <mark.d.gray at redhat.com> wrote:
>>
>> On 18/02/2021 15:54, numans at ovn.org wrote:
>>> From: Numan Siddique <numans at ovn.org>
>>
>> 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 <hzhou at ovn.org>
>>> Signed-off-by: Numan Siddique <numans at ovn.org>
>>> ---
>>>
>>> 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.
Yes, the makes sense now. I understand the motivation.
>
>
>>
>>> /* 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.
>
This makes it a bit more difficult to understand as we are checking for
a side effect of the states rather than the state itself. Why can we not
just check the state? I understand that this is a little outside the
scope of this patch.
> 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
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
More information about the dev
mailing list