On Tue, Jul 27, 2021 at 7:11 PM Han Zhou <hz...@ovn.org> wrote: > > On Tue, Jul 27, 2021 at 3:03 PM Numan Siddique <num...@ovn.org> wrote: > > > > On Tue, Jul 27, 2021 at 4:00 PM Han Zhou <hz...@ovn.org> wrote: > > > > > > On Tue, Jul 27, 2021 at 11:43 AM Numan Siddique <num...@ovn.org> wrote: > > > > > > > > On Tue, Jul 27, 2021 at 3:36 AM Han Zhou <hz...@ovn.org> wrote: > > > > > > > > > > Thanks Numan. The commit title has a typo: s/plow/pflow. > > > > > > > > > > On Fri, Jul 16, 2021 at 4:55 AM <num...@ovn.org> wrote: > > > > > > > > > > > > From: Numan Siddique <num...@ovn.org> > > > > > > > > > > > > physical_run() maintains a local copy of local vif to ofports > > > > > > in a simap along with the chassis tunnel information. This patch > > > > > > removes this from the physical module and now stores it in the > > > > > > runtime_data engine node. This makes it easier to handle runtime > > > > > > data changes in pflow_output engine. > > > > > > > > > > > > The newly added handler pflow_output_runtime_data_handler() > returns > > > > > > false if a datapath is added or removed from the local_datapaths > > > > > > and handles the logical port claims and releases incrementally. > > > > > > > > > > > > Signed-off-by: Numan Siddique <num...@ovn.org> > > > > > > --- > > > > > > controller/binding.c | 39 ++-- > > > > > > controller/binding.h | 23 +++ > > > > > > controller/ldata.c | 169 ++++++++++++++++ > > > > > > controller/ldata.h | 44 +++++ > > > > > > controller/lflow.c | 5 +- > > > > > > controller/lflow.h | 1 + > > > > > > controller/ovn-controller.c | 68 ++++++- > > > > > > controller/ovn-controller.h | 8 - > > > > > > controller/physical.c | 382 > > > +++++++++--------------------------- > > > > > > controller/physical.h | 13 +- > > > > > > 10 files changed, 422 insertions(+), 330 deletions(-) > > > > > > > > > > > > diff --git a/controller/binding.c b/controller/binding.c > > > > > > index 0fd951ad7..b50139726 100644 > > > > > > --- a/controller/binding.c > > > > > > +++ b/controller/binding.c > > > > > > @@ -546,23 +546,6 @@ update_active_pb_ras_pd(const struct > > > > > sbrec_port_binding *pb, > > > > > > } > > > > > > } > > > > > > > > > > > > -/* Corresponds to each Port_Binding.type. */ > > > > > > -enum en_lport_type { > > > > > > - LP_UNKNOWN, > > > > > > - LP_VIF, > > > > > > - LP_CONTAINER, > > > > > > - LP_PATCH, > > > > > > - LP_L3GATEWAY, > > > > > > - LP_LOCALNET, > > > > > > - LP_LOCALPORT, > > > > > > - LP_L2GATEWAY, > > > > > > - LP_VTEP, > > > > > > - LP_CHASSISREDIRECT, > > > > > > - LP_VIRTUAL, > > > > > > - LP_EXTERNAL, > > > > > > - LP_REMOTE > > > > > > -}; > > > > > > - > > > > > > /* Local bindings. binding.c module binds the logical port > > > (represented > > > > > by > > > > > > * Port_Binding rows) and sets the 'chassis' column when it sees > the > > > > > > * OVS interface row (of type "" or "internal") with the > > > > > > @@ -616,7 +599,7 @@ static struct local_binding > *local_binding_create( > > > > > > static void local_binding_add(struct shash *local_bindings, > > > > > > struct local_binding *); > > > > > > static struct local_binding *local_binding_find( > > > > > > - struct shash *local_bindings, const char *name); > > > > > > + const struct shash *local_bindings, const char *name); > > > > > > static void local_binding_destroy(struct local_binding *, > > > > > > struct shash *binding_lports); > > > > > > static void local_binding_delete(struct local_binding *, > > > > > > @@ -701,7 +684,8 @@ local_binding_data_destroy(struct > > > local_binding_data > > > > > *lbinding_data) > > > > > > } > > > > > > > > > > > > const struct sbrec_port_binding * > > > > > > -local_binding_get_primary_pb(struct shash *local_bindings, const > char > > > > > *pb_name) > > > > > > +local_binding_get_primary_pb(struct shash *local_bindings, > > > > > > + const char *pb_name) > > > > > > { > > > > > > struct local_binding *lbinding = > > > > > > local_binding_find(local_bindings, pb_name); > > > > > > @@ -710,6 +694,19 @@ local_binding_get_primary_pb(struct shash > > > > > *local_bindings, const char *pb_name) > > > > > > return b_lport ? b_lport->pb : NULL; > > > > > > } > > > > > > > > > > > > +ofp_port_t > > > > > > +local_binding_get_lport_ofport(const struct shash > *local_bindings, > > > > > > + const char *pb_name) > > > > > > +{ > > > > > > + struct local_binding *lbinding = > > > > > > + local_binding_find(local_bindings, pb_name); > > > > > > + struct binding_lport *b_lport = > > > > > > + local_binding_get_primary_or_localport_lport(lbinding); > > > > > > + > > > > > > + return (b_lport && lbinding->iface && > lbinding->iface->n_ofport) > > > ? > > > > > > + u16_to_ofp(lbinding->iface->ofport[0]) : 0; > > > > > > +} > > > > > > + > > > > > > bool > > > > > > local_binding_is_up(struct shash *local_bindings, const char > > > *pb_name) > > > > > > { > > > > > > @@ -871,7 +868,7 @@ is_lport_vif(const struct sbrec_port_binding > *pb) > > > > > > return !pb->type[0]; > > > > > > } > > > > > > > > > > > > -static enum en_lport_type > > > > > > +enum en_lport_type > > > > > > get_lport_type(const struct sbrec_port_binding *pb) > > > > > > { > > > > > > if (is_lport_vif(pb)) { > > > > > > @@ -2555,7 +2552,7 @@ local_binding_create(const char *name, const > > > struct > > > > > ovsrec_interface *iface) > > > > > > } > > > > > > > > > > > > static struct local_binding * > > > > > > -local_binding_find(struct shash *local_bindings, const char > *name) > > > > > > +local_binding_find(const struct shash *local_bindings, const char > > > *name) > > > > > > { > > > > > > return shash_find_data(local_bindings, name); > > > > > > } > > > > > > diff --git a/controller/binding.h b/controller/binding.h > > > > > > index b1717bd2b..f1abc4b9c 100644 > > > > > > --- a/controller/binding.h > > > > > > +++ b/controller/binding.h > > > > > > @@ -114,6 +114,9 @@ void local_binding_data_destroy(struct > > > > > local_binding_data *); > > > > > > > > > > > > const struct sbrec_port_binding *local_binding_get_primary_pb( > > > > > > struct shash *local_bindings, const char *pb_name); > > > > > > +ofp_port_t local_binding_get_lport_ofport(const struct shash > > > > > *local_bindings, > > > > > > + const char *pb_name); > > > > > > + > > > > > > bool local_binding_is_up(struct shash *local_bindings, const char > > > > > *pb_name); > > > > > > bool local_binding_is_down(struct shash *local_bindings, const > char > > > > > *pb_name); > > > > > > void local_binding_set_up(struct shash *local_bindings, const > char > > > > > *pb_name, > > > > > > @@ -134,4 +137,24 @@ bool > binding_handle_port_binding_changes(struct > > > > > binding_ctx_in *, > > > > > > void binding_tracked_dp_destroy(struct hmap *tracked_datapaths); > > > > > > > > > > > > void binding_dump_local_bindings(struct local_binding_data *, > struct > > > ds > > > > > *); > > > > > > + > > > > > > +/* Corresponds to each Port_Binding.type. */ > > > > > > +enum en_lport_type { > > > > > > + LP_UNKNOWN, > > > > > > + LP_VIF, > > > > > > + LP_CONTAINER, > > > > > > + LP_PATCH, > > > > > > + LP_L3GATEWAY, > > > > > > + LP_LOCALNET, > > > > > > + LP_LOCALPORT, > > > > > > + LP_L2GATEWAY, > > > > > > + LP_VTEP, > > > > > > + LP_CHASSISREDIRECT, > > > > > > + LP_VIRTUAL, > > > > > > + LP_EXTERNAL, > > > > > > + LP_REMOTE > > > > > > +}; > > > > > > + > > > > > > +enum en_lport_type get_lport_type(const struct > sbrec_port_binding *); > > > > > > + > > > > > > #endif /* controller/binding.h */ > > > > > > diff --git a/controller/ldata.c b/controller/ldata.c > > > > > > index a6df9b1da..f55905551 100644 > > > > > > --- a/controller/ldata.c > > > > > > +++ b/controller/ldata.c > > > > > > @@ -18,10 +18,13 @@ > > > > > > /* OVS includes. */ > > > > > > #include "include/openvswitch/json.h" > > > > > > #include "lib/hmapx.h" > > > > > > +#include "lib/flow.h" > > > > > > #include "lib/util.h" > > > > > > +#include "lib/vswitch-idl.h" > > > > > > #include "openvswitch/vlog.h" > > > > > > > > > > > > /* OVN includes. */ > > > > > > +#include "encaps.h" > > > > > > #include "ldata.h" > > > > > > #include "lport.h" > > > > > > #include "lib/ovn-util.h" > > > > > > @@ -275,6 +278,172 @@ tracked_datapaths_destroy(struct hmap > > > > > *tracked_datapaths) > > > > > > hmap_destroy(tracked_datapaths); > > > > > > } > > > > > > > > > > > > +/* Iterates the br_int ports and build the simap of patch to > ofports > > > > > > + * and chassis tunnels. */ > > > > > > +void > > > > > > +ldata_run(const struct ovsrec_bridge *br_int, > > > > > > + const struct sbrec_chassis *chassis_rec, > > > > > > + struct local_nonvif_data *nonvif_data) > > > > > > +{ > > > > > > + for (int i = 0; i < br_int->n_ports; i++) { > > > > > > + const struct ovsrec_port *port_rec = br_int->ports[i]; > > > > > > + if (!strcmp(port_rec->name, br_int->name)) { > > > > > > + continue; > > > > > > + } > > > > > > + > > > > > > + const char *tunnel_id = smap_get(&port_rec->external_ids, > > > > > > + "ovn-chassis-id"); > > > > > > + if (tunnel_id && encaps_tunnel_id_match(tunnel_id, > > > > > > + > chassis_rec->name, > > > > > > + NULL)) { > > > > > > + continue; > > > > > > + } > > > > > > + > > > > > > + const char *localnet = smap_get(&port_rec->external_ids, > > > > > > + "ovn-localnet-port"); > > > > > > + const char *l2gateway = smap_get(&port_rec->external_ids, > > > > > > + "ovn-l2gateway-port"); > > > > > > + > > > > > > + for (int j = 0; j < port_rec->n_interfaces; j++) { > > > > > > + const struct ovsrec_interface *iface_rec = > > > > > port_rec->interfaces[j]; > > > > > > + > > > > > > + /* Get OpenFlow port number. */ > > > > > > + if (!iface_rec->n_ofport) { > > > > > > + continue; > > > > > > + } > > > > > > + int64_t ofport = iface_rec->ofport[0]; > > > > > > + if (ofport < 1 || ofport > ofp_to_u16(OFPP_MAX)) { > > > > > > + continue; > > > > > > + } > > > > > > + > > > > > > + bool is_patch = !strcmp(iface_rec->type, "patch"); > > > > > > + if (is_patch && localnet) { > > > > > > + simap_put(&nonvif_data->patch_ofports, localnet, > > > ofport); > > > > > > + break; > > > > > > + } else if (is_patch && l2gateway) { > > > > > > + /* L2 gateway patch ports can be handled just > like > > > VIFs. > > > > > */ > > > > > > + simap_put(&nonvif_data->patch_ofports, l2gateway, > > > > > ofport); > > > > > > + break; > > > > > > + } else if (tunnel_id) { > > > > > > + enum chassis_tunnel_type tunnel_type; > > > > > > + if (!strcmp(iface_rec->type, "geneve")) { > > > > > > + tunnel_type = GENEVE; > > > > > > + } else if (!strcmp(iface_rec->type, "stt")) { > > > > > > + tunnel_type = STT; > > > > > > + } else if (!strcmp(iface_rec->type, "vxlan")) { > > > > > > + tunnel_type = VXLAN; > > > > > > + } else { > > > > > > + continue; > > > > > > + } > > > > > > + > > > > > > + /* We split the tunnel_id to get the chassis-id > > > > > > + * and hash the tunnel list on the chassis-id. > The > > > > > > + * reason to use the chassis-id alone is because > > > > > > + * there might be cases (multicast, gateway > chassis) > > > > > > + * where we need to tunnel to the chassis, but > won't > > > > > > + * have the encap-ip specifically. > > > > > > + */ > > > > > > + char *hash_id = NULL; > > > > > > + char *ip = NULL; > > > > > > + > > > > > > + if (!encaps_tunnel_id_parse(tunnel_id, &hash_id, > > > &ip)) { > > > > > > + continue; > > > > > > + } > > > > > > + struct chassis_tunnel *tun = xmalloc(sizeof > *tun); > > > > > > + hmap_insert(&nonvif_data->chassis_tunnels, > > > > > &tun->hmap_node, > > > > > > + hash_string(hash_id, 0)); > > > > > > + tun->chassis_id = xstrdup(tunnel_id); > > > > > > + tun->ofport = u16_to_ofp(ofport); > > > > > > + tun->type = tunnel_type; > > > > > > + > > > > > > + free(hash_id); > > > > > > + free(ip); > > > > > > + break; > > > > > > + } > > > > > > + } > > > > > > + } > > > > > > +} > > > > > > + > > > > > > +bool > > > > > > +ldata_handle_ovs_iface_changes( > > > > > > + const struct ovsrec_interface_table *iface_table) > > > > > > +{ > > > > > > + const struct ovsrec_interface *iface_rec; > > > > > > + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, > iface_table) > > > { > > > > > > + if (!strcmp(iface_rec->type, "geneve") || > > > > > > + !strcmp(iface_rec->type, "patch") || > > > > > > + !strcmp(iface_rec->type, "vxlan") || > > > > > > + !strcmp(iface_rec->type, "stt")) { > > > > > > + return false; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > + return true; > > > > > > +} > > > > > > + > > > > > > +bool > > > > > > +get_chassis_tunnel_ofport(const struct hmap *chassis_tunnels, > > > > > > + const char *chassis_name, char > *encap_ip, > > > > > > + ofp_port_t *ofport) > > > > > > +{ > > > > > > + struct chassis_tunnel *tun = NULL; > > > > > > + tun = chassis_tunnel_find(chassis_tunnels, chassis_name, > > > encap_ip); > > > > > > + if (!tun) { > > > > > > + return false; > > > > > > + } > > > > > > + > > > > > > + *ofport = tun->ofport; > > > > > > + return true; > > > > > > +} > > > > > > + > > > > > > +void > > > > > > +local_nonvif_data_init(struct local_nonvif_data *nonvif_data) > > > > > > +{ > > > > > > + simap_init(&nonvif_data->patch_ofports); > > > > > > + hmap_init(&nonvif_data->chassis_tunnels); > > > > > > +} > > > > > > + > > > > > > +void > > > > > > +local_nonvif_data_destroy(struct local_nonvif_data *nonvif_data) > > > > > > +{ > > > > > > + simap_destroy(&nonvif_data->patch_ofports); > > > > > > + struct chassis_tunnel *tun; > > > > > > + HMAP_FOR_EACH_POP (tun, hmap_node, > > > &nonvif_data->chassis_tunnels) { > > > > > > + free(tun->chassis_id); > > > > > > + free(tun); > > > > > > + } > > > > > > + hmap_destroy(&nonvif_data->chassis_tunnels); > > > > > > +} > > > > > > + > > > > > > + > > > > > > +/* > > > > > > + * This function looks up the list of tunnel ports (provided by > > > > > > + * ovn-chassis-id ports) and returns the tunnel for the given > > > chassid-id > > > > > and > > > > > > + * encap-ip. The ovn-chassis-id is formed using the chassis-id > and > > > > > encap-ip. > > > > > > + * The list is hashed using the chassis-id. If the encap-ip is > not > > > > > specified, > > > > > > + * it means we'll just return a tunnel for that chassis-id, i.e. > we > > > just > > > > > check > > > > > > + * for chassis-id and if there is a match, we'll return the > tunnel. > > > > > > + * If encap-ip is also provided we use both chassis-id and > encap-ip > > > to do > > > > > > + * a more specific lookup. > > > > > > + */ > > > > > > +struct chassis_tunnel * > > > > > > +chassis_tunnel_find(const struct hmap *chassis_tunnels, const > char > > > > > *chassis_id, > > > > > > + char *encap_ip) > > > > > > +{ > > > > > > + /* > > > > > > + * If the specific encap_ip is given, look for the > chassisid_ip > > > > > entry, > > > > > > + * else return the 1st found entry for the chassis. > > > > > > + */ > > > > > > + struct chassis_tunnel *tun = NULL; > > > > > > + HMAP_FOR_EACH_WITH_HASH (tun, hmap_node, > hash_string(chassis_id, > > > 0), > > > > > > + chassis_tunnels) { > > > > > > + if (encaps_tunnel_id_match(tun->chassis_id, chassis_id, > > > > > encap_ip)) { > > > > > > + return tun; > > > > > > + } > > > > > > + } > > > > > > + return NULL; > > > > > > +} > > > > > > + > > > > > > /* static functions. */ > > > > > > static struct local_datapath * > > > > > > local_datapath_add__(struct hmap *local_datapaths, > > > > > > diff --git a/controller/ldata.h b/controller/ldata.h > > > > > > index 16ad43c8f..91624d0b4 100644 > > > > > > --- a/controller/ldata.h > > > > > > +++ b/controller/ldata.h > > > > > > @@ -19,10 +19,14 @@ > > > > > > /* OVS includes. */ > > > > > > #include "include/openvswitch/shash.h" > > > > > > #include "lib/smap.h" > > > > > > +#include "lib/simap.h" > > > > > > > > > > > > struct sbrec_datapath_binding; > > > > > > struct sbrec_port_binding; > > > > > > +struct sbrec_chassis; > > > > > > struct ovsdb_idl_index; > > > > > > +struct ovsrec_bridge; > > > > > > +struct ovsrec_interface_table; > > > > > > > > > > > > /* A logical datapath that has some relevance to this > hypervisor. A > > > > > logical > > > > > > * datapath D is relevant to hypervisor H if: > > > > > > @@ -117,4 +121,44 @@ void tracked_datapath_lport_add(const struct > > > > > sbrec_port_binding *, > > > > > > struct hmap *tracked_datapaths); > > > > > > void tracked_datapaths_destroy(struct hmap *tracked_datapaths); > > > > > > > > > > > > +/* Must be a bit-field ordered from most-preferred (higher > number) to > > > > > > + * least-preferred (lower number). */ > > > > > > +enum chassis_tunnel_type { > > > > > > + GENEVE = 1 << 2, > > > > > > + STT = 1 << 1, > > > > > > + VXLAN = 1 << 0 > > > > > > +}; > > > > > > + > > > > > > +/* Maps from a chassis to the OpenFlow port number of the tunnel > that > > > > > can be > > > > > > + * used to reach that chassis. */ > > > > > > +struct chassis_tunnel { > > > > > > + struct hmap_node hmap_node; > > > > > > + char *chassis_id; > > > > > > + ofp_port_t ofport; > > > > > > + enum chassis_tunnel_type type; > > > > > > +}; > > > > > > + > > > > > > +struct local_nonvif_data { > > > > > > + struct simap patch_ofports; /* simap of patch ovs ports. */ > > > > > > + struct hmap chassis_tunnels; /* hmap of 'struct > chassis_tunnel' > > > from > > > > > the > > > > > > + * tunnel OVS ports. */ > > > > > > +}; > > > > > > + > > > > > > +void ldata_run(const struct ovsrec_bridge *br_int, > > > > > > + const struct sbrec_chassis *, > > > > > > + struct local_nonvif_data *nonvif_data); > > > > > > + > > > > > > +bool ldata_handle_ovs_iface_changes(const struct > > > ovsrec_interface_table > > > > > *); > > > > > > + > > > > > > +struct chassis_tunnel *chassis_tunnel_find(const struct hmap > > > > > *chassis_tunnels, > > > > > > + const char > *chassis_id, > > > > > > + char *encap_ip); > > > > > > + > > > > > > +bool get_chassis_tunnel_ofport(const struct hmap > *chassis_tunnels, > > > > > > + const char *chassis_name, char > > > *encap_ip, > > > > > > + ofp_port_t *ofport); > > > > > > + > > > > > > +void local_nonvif_data_init(struct local_nonvif_data > *nonvif_data); > > > > > > +void local_nonvif_data_destroy(struct local_nonvif_data > > > *nonvif_data); > > > > > > + > > > > > > #endif /* controller/ldata.h */ > > > > > > diff --git a/controller/lflow.c b/controller/lflow.c > > > > > > index d1f32077b..4ac671e40 100644 > > > > > > --- a/controller/lflow.c > > > > > > +++ b/controller/lflow.c > > > > > > @@ -58,6 +58,7 @@ struct lookup_port_aux { > > > > > > const struct sbrec_datapath_binding *dp; > > > > > > const struct sbrec_logical_flow *lflow; > > > > > > struct lflow_resource_ref *lfrr; > > > > > > + const struct hmap *chassis_tunnels; > > > > > > }; > > > > > > > > > > > > struct condition_aux { > > > > > > @@ -145,7 +146,8 @@ tunnel_ofport_cb(const void *aux_, const char > > > > > *port_name, ofp_port_t *ofport) > > > > > > return false; > > > > > > } > > > > > > > > > > > > - if (!get_tunnel_ofport(pb->chassis->name, NULL, ofport)) { > > > > > > + if (!get_chassis_tunnel_ofport(aux->chassis_tunnels, > > > > > pb->chassis->name, > > > > > > + NULL, ofport)) { > > > > > > return false; > > > > > > } > > > > > > > > > > > > @@ -591,6 +593,7 @@ add_matches_to_flow_table(const struct > > > > > sbrec_logical_flow *lflow, > > > > > > .dp = dp, > > > > > > .lflow = lflow, > > > > > > .lfrr = l_ctx_out->lfrr, > > > > > > + .chassis_tunnels = l_ctx_in->chassis_tunnels, > > > > > > }; > > > > > > > > > > > > /* Encode OVN logical actions into OpenFlow. */ > > > > > > diff --git a/controller/lflow.h b/controller/lflow.h > > > > > > index c17ff6dd4..e7dd31289 100644 > > > > > > --- a/controller/lflow.h > > > > > > +++ b/controller/lflow.h > > > > > > @@ -146,6 +146,7 @@ struct lflow_ctx_in { > > > > > > const struct shash *port_groups; > > > > > > const struct sset *active_tunnels; > > > > > > const struct sset *related_lport_ids; > > > > > > + const struct hmap *chassis_tunnels; > > > > > > }; > > > > > > > > > > > > struct lflow_ctx_out { > > > > > > diff --git a/controller/ovn-controller.c > b/controller/ovn-controller.c > > > > > > index 8f620e4ad..69d135046 100644 > > > > > > --- a/controller/ovn-controller.c > > > > > > +++ b/controller/ovn-controller.c > > > > > > @@ -1039,6 +1039,11 @@ struct ed_type_runtime_data { > > > > > > struct related_lports related_lports; > > > > > > struct sset active_tunnels; > > > > > > > > > > > > + /* Non VIF OVS interface information (mainly patch OVS > interfacs > > > > > > + * and tunnel interfaces) that are relevant to the local > > > > > > + * chassis (generated by ldata_run()). */ > > > > > > + struct local_nonvif_data nonvif_data; > > > > > > + > > > > > I don't see a benefit of adding this to runtime_data. It is > unrelated to > > > > > the other data in the current runtime_data node. Now if we combine > it to > > > > > runtime_data, it means we are coupling those data unnecessarily, > which > > > > > leads to unflexible handler implementation. An example is that now > > > > > recompute of nonvif_data would trigger recompute of runtime_data, > which > > > > > would trigger recompute of lflow_output. In fact, this can be just > in a > > > > > separate engine node, only as input to pflow_output. Would that be > more > > > > > straightforward and efficient? > > > > > > > > Hi Han, > > > > > > > > Thanks for the review comments. > > > > > > > > I see your point. I'll explore if we can add a new engine node for > this. > > > > > > > > My understanding of engine processing is this and runtime data engine > > > > in particular is this: > > > > > > > > 1. The first level of engine nodes would handle the db changes and > > > > generates/transforms the data > > > > which we can probably call - runtime_data. This was the > > > > motivation for me to add the nonvif_data > > > > in the runtime data engine. Perhaps we need to further divide > > > > the runtime_data engine or rename it > > > > so that we only store the VIF related data in this. > > > > > > Hi Numan, it seems we had different models for I-P engine nodes in our > > > minds. Sorry that I didn't document these thoughts initially for the I-P > > > engine. I don't think we should restrict the engine nodes in such > layers. > > > It should be flexible enough based on the real computation dependencies > - > > > just thinking about how DDlog programs would be written. I am not > against > > > the idea of a separate encapsulation layer for OVSDB IDL, but > runtime_data > > > was not for that purpose. Initially the I-P engine was very coarse > grained, > > > so I just created a big "runtime_data" node trying to contain all the > > > intermediate data, just enough to support the first version of I-P for > the > > > most frequent changes. Now that we are making the I-P engine more and > more > > > fine-grained, we definitely cannot simply put everything in a single > node. > > > The current runtime_data may be renamed to binding_data, or be further > > > split if necessary, and we don't need to put more data into it unless > it is > > > tightly related to the bindings handling. > > > > Thanks for the explanation. I'd agree renaming the current > > runtime_data to binding_data. > > > > Probably a patch later for that. > > > > Since we are discussing I-P, I'd like to bring up another case. > > > > Presently we abort the engine if a recompute needs to be done but > > recompute is not allowed > > due to an sb txn is in progress or due to ofctrl_can_put() returns > > false. We have engine nodes > > like ct_zone which does not have handlers for many inputs and it has > > to recompute. > > And suppose if engine aborts due to the reason I mentioned, then in > > the next engine run, we do > > a full recomputation of all engine nodes whereas only recompute of > > ct_zones (and perhaps pflow_output) > > was required. In our scale testing we noticed that this recomputation > > of all the engine nodes is costing > > a bit. > > > Aha, I just debugged a problem and noticed a similar issue yesterday! It is > on the same ct_zones node, but the trigger was not because of sb txn, but > the ofctrl_can_put() == false (i.e. there are inflight messages). > I think an easy solution is to apply the abort-if-recompute strategy only > for certain nodes that either require high cost of recompute (i.e. > lflow_output) or require DB txn. This way, recompute in ct_zones node > should always be allowed and wouldn't abort and trigger next full recompute > for the whole engine. A property can be added to engine_node for this > purpose.
Thanks for the comments. I noticed the ofctrl_can_put() returning false a lot in our testing. I'll try to address the TXN NULL issue based on your suggestion. And explore if the other one can be solved too. Thanks Numan > > > I think we should address this. And probably we can separate out db > > transactions from the engine > > and can be done separately. The binding module will not set > > sbrec_port_binding_set_chassis() > > for example. > > > This may be worth exploring, too. There may be benefits more than just the > scenario above. However, it cannot solve the scenario I encountered > yesterday, which was not triggered by the SB txn but in-flight OF messages. > > > Any thoughts on this ? > > > > Thanks > > Numan > > > > > > > > > 2. And the flow output engines ideally would use the runtime data for > > > > flow generation. > > > > > > The output nodes should only depend on whatever data that is necessary > for > > > flow computation. What you said is true if "runtime data" here contains > > > everything that is needed for flow computation, but it should never be a > > > goal to keep all these data in a single I-P engine node, because that > would > > > not really benefit from the I-P engine. > > > > > > > 3. Ideally I'd think (or prefer) to engine nodes one or two level > > > > down to not access any db changes at all. > > > > For example, lfow_output or pflow_output engine handles should > > > > not call IDL TRACKED loops. > > > > > > > > Maybe we could try to achieve follow up patches if that makes sense. > > > > > > > Encapsulating IDL to a separate layer may be reasonable, if it provides > > > values, but I think it is another topic. I don't think it changes the > I-P > > > model. > > > > > > Thanks, > > > Han > > > > > > > > > > > > > > > > > > > > > > > > As to the code organization, I think now I understand why you added > the > > > new > > > > > module ldata.c (suppose it is going to be renamed to local_data.c > to be > > > > > more clear). Although it is not harmful to have that new module, it > > > seems > > > > > not quite necessary to me, either. The original localvif_to_ofport > data > > > is > > > > > now split into two parts, VIF related and nonVIF related. I don't > see > > > > > benefit of keeping these two parts in the same file. It is > reasonable to > > > > > combine the VIF related data in runtime_data (which could be > renamed, > > > > > because the name was vague), to avoid maintaining redundant data. I > > > think > > > > > it is ok to stay in binding.c. For the nonVIF part, I think the new > > > engine > > > > > node can be defined in ovn-controller.c and implemented in > physical.c > > > > > because that data is used by physical flow processing only. I don't > > > have a > > > > > strong argument for this - maybe just less change is needed for this > > > series > > > > > (and so the change history would be easier to follow). > > > > > > > > I'll explore and see if this can be separated. Although I think > > > > having a separate file local_data.c would uncomplicate the binding.c a > > > > bit. > > > > > > > > > > > > > > > > > > > > > > > > /* runtime data engine private data. */ > > > > > > struct sset egress_ifaces; > > > > > > struct smap local_iface_ids; > > > > > > @@ -1139,6 +1144,7 @@ en_runtime_data_init(struct engine_node > *node > > > > > OVS_UNUSED, > > > > > > local_binding_data_init(&data->lbinding_data); > > > > > > shash_init(&data->local_active_ports_ipv6_pd); > > > > > > shash_init(&data->local_active_ports_ras); > > > > > > + local_nonvif_data_init(&data->nonvif_data); > > > > > > > > > > > > /* Init the tracked data. */ > > > > > > hmap_init(&data->tracked_dp_bindings); > > > > > > @@ -1160,6 +1166,7 @@ en_runtime_data_cleanup(void *data) > > > > > > > shash_destroy_free_data(&rt_data->local_active_ports_ipv6_pd); > > > > > > shash_destroy_free_data(&rt_data->local_active_ports_ras); > > > > > > local_binding_data_destroy(&rt_data->lbinding_data); > > > > > > + local_nonvif_data_destroy(&rt_data->nonvif_data); > > > > > > } > > > > > > > > > > > > static void > > > > > > @@ -1272,6 +1279,7 @@ en_runtime_data_run(struct engine_node > *node, > > > void > > > > > *data) > > > > > > shash_clear_free_data(local_active_ipv6_pd); > > > > > > shash_clear_free_data(local_active_ras); > > > > > > local_binding_data_destroy(&rt_data->lbinding_data); > > > > > > + local_nonvif_data_destroy(&rt_data->nonvif_data); > > > > > > sset_destroy(local_lports); > > > > > > related_lports_destroy(&rt_data->related_lports); > > > > > > sset_destroy(active_tunnels); > > > > > > @@ -1284,6 +1292,7 @@ en_runtime_data_run(struct engine_node > *node, > > > void > > > > > *data) > > > > > > sset_init(&rt_data->egress_ifaces); > > > > > > smap_init(&rt_data->local_iface_ids); > > > > > > local_binding_data_init(&rt_data->lbinding_data); > > > > > > + local_nonvif_data_init(&rt_data->nonvif_data); > > > > > > } > > > > > > > > > > > > struct binding_ctx_in b_ctx_in; > > > > > > @@ -1303,6 +1312,7 @@ en_runtime_data_run(struct engine_node > *node, > > > void > > > > > *data) > > > > > > bfd_calculate_active_tunnels(b_ctx_in.br_int, > > > active_tunnels); > > > > > > } > > > > > > > > > > > > + ldata_run(b_ctx_in.br_int, b_ctx_in.chassis_rec, > > > > > &rt_data->nonvif_data); > > > > > > binding_run(&b_ctx_in, &b_ctx_out); > > > > > > > > > > > > engine_set_node_state(node, EN_UPDATED); > > > > > > @@ -1318,6 +1328,10 @@ runtime_data_ovs_interface_handler(struct > > > > > engine_node *node, void *data) > > > > > > rt_data->tracked = true; > > > > > > b_ctx_out.tracked_dp_bindings = > &rt_data->tracked_dp_bindings; > > > > > > > > > > > > + if (!ldata_handle_ovs_iface_changes(b_ctx_in.iface_table)) { > > > > > > + return false; > > > > > > + } > > > > > > + > > > > > > > > > > This is the performance problem I mentioned above. Before this > change, > > > > > tunnel/patch interface changes only triggered physical_run(), now it > > > would > > > > > trigger lflow_run(). > > > > > > > > Actually that's not the case. Any change to tunnel/patch OVS > interfaces > > > results > > > > in the function binding_handle_ovs_interface_changes() to return false > > > and hence > > > > lflow_run() would be called - > > > > https://github.com/ovn-org/ovn/blob/master/controller/binding.c#L2198 > > > > > > > > > > > > > > > > > > > > > > > if (!binding_handle_ovs_interface_changes(&b_ctx_in, > > > &b_ctx_out)) { > > > > > > return false; > > > > > > } > > > > > > @@ -2060,6 +2074,7 @@ init_lflow_ctx(struct engine_node *node, > > > > > > l_ctx_in->port_groups = port_groups; > > > > > > l_ctx_in->active_tunnels = &rt_data->active_tunnels; > > > > > > l_ctx_in->related_lport_ids = > &rt_data->related_lports.lport_ids; > > > > > > + l_ctx_in->chassis_tunnels = > > > &rt_data->nonvif_data.chassis_tunnels; > > > > > > > > > > > > l_ctx_out->flow_table = &fo->flow_table; > > > > > > l_ctx_out->group_table = &fo->group_table; > > > > > > @@ -2527,6 +2542,7 @@ static void init_physical_ctx(struct > engine_node > > > > > *node, > > > > > > p_ctx->ct_zones = ct_zones; > > > > > > p_ctx->mff_ovn_geneve = ed_mff_ovn_geneve->mff_ovn_geneve; > > > > > > p_ctx->local_bindings = &rt_data->lbinding_data.bindings; > > > > > > + p_ctx->nonvif_data = &rt_data->nonvif_data; > > > > > > } > > > > > > > > > > > > static void * > > > > > > @@ -2583,7 +2599,11 @@ pflow_output_sb_port_binding_handler(struct > > > > > engine_node *node, > > > > > > * 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); > > > > > > + const struct sbrec_port_binding *pb; > > > > > > + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, > > > > > p_ctx.port_binding_table) { > > > > > > + bool removed = sbrec_port_binding_is_deleted(pb); > > > > > > + physical_handle_flows_for_lport(pb, removed, &p_ctx, > > > > > &pfo->flow_table); > > > > > > + } > > > > > > > > > > nit: I think it is better to keep the interface > > > > > physical_handle_port_binding_changes() and hide this loop details > in the > > > > > physical.c module. > > > > > > > > Ack. > > > > > > > > Thanks > > > > Numan > > > > > > > > > > > > > > Thanks, > > > > > Han > > > > > > > > > > > > > > > > > engine_set_node_state(node, EN_UPDATED); > > > > > > return true; > > > > > > @@ -2613,13 +2633,55 @@ pflow_output_ovs_iface_handler(struct > > > engine_node > > > > > *node OVS_UNUSED, > > > > > > 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); > > > > > > + > > > > > > + engine_set_node_state(node, EN_UPDATED); > > > > > > + return physical_handle_ovs_iface_changes(&p_ctx); > > > > > > +} > > > > > > + > > > > > > +static bool > > > > > > +pflow_output_runtime_data_handler(struct engine_node *node, void > > > *data) > > > > > > +{ > > > > > > + struct ed_type_runtime_data *rt_data = > > > > > > + engine_get_input_data("runtime_data", node); > > > > > > + > > > > > > + /* There is no tracked data. Fall back to full recompute of > > > > > > + * pflow_output. */ > > > > > > + if (!rt_data->tracked) { > > > > > > + return false; > > > > > > + } > > > > > > + > > > > > > + struct hmap *tracked_dp_bindings = > &rt_data->tracked_dp_bindings; > > > > > > + if (hmap_is_empty(tracked_dp_bindings)) { > > > > > > + return true; > > > > > > + } > > > > > > + > > > > > > struct ed_type_pflow_output *pfo = data; > > > > > > > > > > > > struct physical_ctx p_ctx; > > > > > > init_physical_ctx(node, rt_data, &p_ctx); > > > > > > > > > > > > + struct tracked_datapath *tdp; > > > > > > + HMAP_FOR_EACH (tdp, node, tracked_dp_bindings) { > > > > > > + if (tdp->tracked_type != TRACKED_RESOURCE_UPDATED) { > > > > > > + /* Fall back to full recompute when a local datapath > > > > > > + * is added or deleted. */ > > > > > > + return false; > > > > > > + } > > > > > > + > > > > > > + struct shash_node *shash_node; > > > > > > + SHASH_FOR_EACH (shash_node, &tdp->lports) { > > > > > > + struct tracked_lport *lport = shash_node->data; > > > > > > + bool removed = > > > > > > + lport->tracked_type == TRACKED_RESOURCE_REMOVED ? > > > true: > > > > > false; > > > > > > + physical_handle_flows_for_lport(lport->pb, removed, > > > &p_ctx, > > > > > > + &pfo->flow_table); > > > > > > + } > > > > > > + } > > > > > > + > > > > > > engine_set_node_state(node, EN_UPDATED); > > > > > > - return physical_handle_ovs_iface_changes(&p_ctx, > > > &pfo->flow_table); > > > > > > + return true; > > > > > > } > > > > > > > > > > > > static void * > > > > > > @@ -2914,7 +2976,7 @@ main(int argc, char *argv[]) > > > > > > pflow_output_sb_multicast_group_handler); > > > > > > > > > > > > engine_add_input(&en_pflow_output, &en_runtime_data, > > > > > > - NULL); > > > > > > + pflow_output_runtime_data_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); > > > > > > diff --git a/controller/ovn-controller.h > b/controller/ovn-controller.h > > > > > > index 578588305..78a53312f 100644 > > > > > > --- a/controller/ovn-controller.h > > > > > > +++ b/controller/ovn-controller.h > > > > > > @@ -45,14 +45,6 @@ const struct ovsrec_bridge *get_bridge(const > struct > > > > > ovsrec_bridge_table *, > > > > > > > > > > > > struct sbrec_encap *preferred_encap(const struct sbrec_chassis > *); > > > > > > > > > > > > -/* Must be a bit-field ordered from most-preferred (higher > number) to > > > > > > - * least-preferred (lower number). */ > > > > > > -enum chassis_tunnel_type { > > > > > > - GENEVE = 1 << 2, > > > > > > - STT = 1 << 1, > > > > > > - VXLAN = 1 << 0 > > > > > > -}; > > > > > > - > > > > > > uint32_t get_tunnel_type(const char *name); > > > > > > > > > > > > struct pb_ld_binding { > > > > > > diff --git a/controller/physical.c b/controller/physical.c > > > > > > index b244ff1c2..87080d001 100644 > > > > > > --- a/controller/physical.c > > > > > > +++ b/controller/physical.c > > > > > > @@ -86,46 +86,6 @@ physical_register_ovs_idl(struct ovsdb_idl > > > *ovs_idl) > > > > > > ovsdb_idl_track_add_column(ovs_idl, > > > > > &ovsrec_interface_col_external_ids); > > > > > > } > > > > > > > > > > > > -static struct simap localvif_to_ofport = > > > > > > - SIMAP_INITIALIZER(&localvif_to_ofport); > > > > > > -static struct hmap tunnels = HMAP_INITIALIZER(&tunnels); > > > > > > - > > > > > > -/* Maps from a chassis to the OpenFlow port number of the tunnel > that > > > > > can be > > > > > > - * used to reach that chassis. */ > > > > > > -struct chassis_tunnel { > > > > > > - struct hmap_node hmap_node; > > > > > > - char *chassis_id; > > > > > > - ofp_port_t ofport; > > > > > > - enum chassis_tunnel_type type; > > > > > > -}; > > > > > > - > > > > > > -/* > > > > > > - * This function looks up the list of tunnel ports (provided by > > > > > > - * ovn-chassis-id ports) and returns the tunnel for the given > > > chassid-id > > > > > and > > > > > > - * encap-ip. The ovn-chassis-id is formed using the chassis-id > and > > > > > encap-ip. > > > > > > - * The list is hashed using the chassis-id. If the encap-ip is > not > > > > > specified, > > > > > > - * it means we'll just return a tunnel for that chassis-id, i.e. > we > > > just > > > > > check > > > > > > - * for chassis-id and if there is a match, we'll return the > tunnel. > > > > > > - * If encap-ip is also provided we use both chassis-id and > encap-ip > > > to do > > > > > > - * a more specific lookup. > > > > > > - */ > > > > > > -static struct chassis_tunnel * > > > > > > -chassis_tunnel_find(const char *chassis_id, char *encap_ip) > > > > > > -{ > > > > > > - /* > > > > > > - * If the specific encap_ip is given, look for the > chassisid_ip > > > > > entry, > > > > > > - * else return the 1st found entry for the chassis. > > > > > > - */ > > > > > > - struct chassis_tunnel *tun = NULL; > > > > > > - HMAP_FOR_EACH_WITH_HASH (tun, hmap_node, > hash_string(chassis_id, > > > 0), > > > > > > - &tunnels) { > > > > > > - if (encaps_tunnel_id_match(tun->chassis_id, chassis_id, > > > > > encap_ip)) { > > > > > > - return tun; > > > > > > - } > > > > > > - } > > > > > > - return NULL; > > > > > > -} > > > > > > - > > > > > > static void > > > > > > put_load(uint64_t value, enum mf_field_id dst, int ofs, int > n_bits, > > > > > > struct ofpbuf *ofpacts) > > > > > > @@ -166,17 +126,18 @@ put_resubmit(uint8_t table_id, struct ofpbuf > > > > > *ofpacts) > > > > > > * from the associated encap. > > > > > > */ > > > > > > static struct chassis_tunnel * > > > > > > -get_port_binding_tun(const struct sbrec_port_binding *binding) > > > > > > +get_port_binding_tun(const struct sbrec_port_binding *binding, > > > > > > + const struct hmap *chassis_tunnels) > > > > > > { > > > > > > struct sbrec_encap *encap = binding->encap; > > > > > > struct sbrec_chassis *chassis = binding->chassis; > > > > > > struct chassis_tunnel *tun = NULL; > > > > > > > > > > > > if (encap) { > > > > > > - tun = chassis_tunnel_find(chassis->name, encap->ip); > > > > > > + tun = chassis_tunnel_find(chassis_tunnels, chassis->name, > > > > > encap->ip); > > > > > > } > > > > > > if (!tun) { > > > > > > - tun = chassis_tunnel_find(chassis->name, NULL); > > > > > > + tun = chassis_tunnel_find(chassis_tunnels, chassis->name, > > > NULL); > > > > > > } > > > > > > return tun; > > > > > > } > > > > > > @@ -325,12 +286,13 @@ put_remote_port_redirect_overlay(const > struct > > > > > > uint32_t port_key, > > > > > > struct match *match, > > > > > > struct ofpbuf *ofpacts_p, > > > > > > + const struct hmap > *chassis_tunnels, > > > > > > struct ovn_desired_flow_table > > > > > *flow_table) > > > > > > { > > > > > > if (!is_ha_remote) { > > > > > > /* Setup encapsulation */ > > > > > > const struct chassis_tunnel *rem_tun = > > > > > > - get_port_binding_tun(binding); > > > > > > + get_port_binding_tun(binding, chassis_tunnels); > > > > > > if (!rem_tun) { > > > > > > return; > > > > > > } > > > > > > @@ -348,10 +310,10 @@ put_remote_port_redirect_overlay(const > struct > > > > > > continue; > > > > > > } > > > > > > if (!tun) { > > > > > > - tun = chassis_tunnel_find(ch->name, NULL); > > > > > > + tun = chassis_tunnel_find(chassis_tunnels, > ch->name, > > > > > NULL); > > > > > > } else { > > > > > > struct chassis_tunnel *chassis_tunnel = > > > > > > - > chassis_tunnel_find(ch->name, > > > > > NULL); > > > > > > + chassis_tunnel_find(chassis_tunnels, > ch->name, > > > NULL); > > > > > > if (chassis_tunnel && > > > > > > tun->type != chassis_tunnel->type) { > > > > > > static struct vlog_rate_limit rl = > > > > > > @@ -385,7 +347,7 @@ put_remote_port_redirect_overlay(const struct > > > > > > if (!ch) { > > > > > > continue; > > > > > > } > > > > > > - tun = chassis_tunnel_find(ch->name, NULL); > > > > > > + tun = chassis_tunnel_find(chassis_tunnels, ch->name, > > > NULL); > > > > > > if (!tun) { > > > > > > continue; > > > > > > } > > > > > > @@ -925,6 +887,9 @@ consider_port_binding(struct ovsdb_idl_index > > > > > *sbrec_port_binding_by_name, > > > > > > const struct simap *ct_zones, > > > > > > const struct sset *active_tunnels, > > > > > > const struct hmap *local_datapaths, > > > > > > + const struct shash *local_bindings, > > > > > > + const struct simap *patch_ofports, > > > > > > + const struct hmap *chassis_tunnels, > > > > > > const struct sbrec_port_binding *binding, > > > > > > const struct sbrec_chassis *chassis, > > > > > > struct ovn_desired_flow_table *flow_table, > > > > > > @@ -1081,17 +1046,25 @@ consider_port_binding(struct > ovsdb_idl_index > > > > > *sbrec_port_binding_by_name, > > > > > > if (!binding->tag) { > > > > > > goto out; > > > > > > } > > > > > > - ofport = u16_to_ofp(simap_get(&localvif_to_ofport, > > > > > > - binding->parent_port)); > > > > > > + ofport = local_binding_get_lport_ofport(local_bindings, > > > > > > + > > > binding->parent_port); > > > > > > if (ofport) { > > > > > > tag = *binding->tag; > > > > > > nested_container = true; > > > > > > parent_port = lport_lookup_by_name( > > > > > > sbrec_port_binding_by_name, > binding->parent_port); > > > > > > } > > > > > > - } else { > > > > > > - ofport = u16_to_ofp(simap_get(&localvif_to_ofport, > > > > > > + } else if (!strcmp(binding->type, "localnet") > > > > > > + || !strcmp(binding->type, "l2gateway")) { > > > > > > + > > > > > > + ofport = u16_to_ofp(simap_get(patch_ofports, > > > > > > binding->logical_port)); > > > > > > + if (ofport && binding->tag) { > > > > > > + tag = *binding->tag; > > > > > > + } > > > > > > + } else { > > > > > > + ofport = local_binding_get_lport_ofport(local_bindings, > > > > > > + > > > binding->logical_port); > > > > > > const char *requested_chassis = > smap_get(&binding->options, > > > > > > > > > "requested-chassis"); > > > > > > if (ofport && requested_chassis && requested_chassis[0] > && > > > > > > @@ -1102,12 +1075,6 @@ consider_port_binding(struct > ovsdb_idl_index > > > > > *sbrec_port_binding_by_name, > > > > > > */ > > > > > > ofport = 0; > > > > > > } > > > > > > - > > > > > > - if ((!strcmp(binding->type, "localnet") > > > > > > - || !strcmp(binding->type, "l2gateway")) > > > > > > - && ofport && binding->tag) { > > > > > > - tag = *binding->tag; > > > > > > - } > > > > > > } > > > > > > > > > > > > bool is_ha_remote = false; > > > > > > @@ -1118,7 +1085,7 @@ consider_port_binding(struct ovsdb_idl_index > > > > > *sbrec_port_binding_by_name, > > > > > > /* It is remote port, may be reached by tunnel or > localnet > > > port > > > > > */ > > > > > > is_remote = true; > > > > > > if (localnet_port) { > > > > > > - ofport = u16_to_ofp(simap_get(&localvif_to_ofport, > > > > > > + ofport = u16_to_ofp(simap_get(patch_ofports, > > > > > > > > > localnet_port->logical_port)); > > > > > > if (!ofport) { > > > > > > goto out; > > > > > > @@ -1129,7 +1096,8 @@ consider_port_binding(struct ovsdb_idl_index > > > > > *sbrec_port_binding_by_name, > > > > > > if (!binding->chassis) { > > > > > > goto out; > > > > > > } > > > > > > - tun = chassis_tunnel_find(binding->chassis->name, > > > NULL); > > > > > > + tun = chassis_tunnel_find(chassis_tunnels, > > > > > > + binding->chassis->name, > > > NULL); > > > > > > if (!tun) { > > > > > > goto out; > > > > > > } > > > > > > @@ -1383,7 +1351,7 @@ consider_port_binding(struct ovsdb_idl_index > > > > > *sbrec_port_binding_by_name, > > > > > > put_remote_port_redirect_overlay(binding, > is_ha_remote, > > > > > > ha_ch_ordered, > > > > > mff_ovn_geneve, > > > > > > tun, port_key, > &match, > > > > > ofpacts_p, > > > > > > - flow_table); > > > > > > + chassis_tunnels, > > > > > flow_table); > > > > > > } > > > > > > } > > > > > > out: > > > > > > @@ -1396,8 +1364,11 @@ static void > > > > > > consider_mc_group(enum mf_field_id mff_ovn_geneve, > > > > > > const struct simap *ct_zones, > > > > > > const struct hmap *local_datapaths, > > > > > > + struct shash *local_bindings, > > > > > > + struct simap *patch_ofports, > > > > > > const struct sbrec_chassis *chassis, > > > > > > const struct sbrec_multicast_group *mc, > > > > > > + const struct hmap *chassis_tunnels, > > > > > > struct ovn_desired_flow_table *flow_table) > > > > > > { > > > > > > uint32_t dp_key = mc->datapath->tunnel_key; > > > > > > @@ -1444,19 +1415,21 @@ consider_mc_group(enum mf_field_id > > > mff_ovn_geneve, > > > > > > put_load(zone_id, MFF_LOG_CT_ZONE, 0, 32, &ofpacts); > > > > > > } > > > > > > > > > > > > + const char *lport_name = (port->parent_port && > > > > > *port->parent_port) ? > > > > > > + port->parent_port : > > > port->logical_port; > > > > > > + > > > > > > if (!strcmp(port->type, "patch")) { > > > > > > put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, > > > > > > &remote_ofpacts); > > > > > > put_resubmit(OFTABLE_CHECK_LOOPBACK, > &remote_ofpacts); > > > > > > - } else if (simap_contains(&localvif_to_ofport, > > > > > > - (port->parent_port && > *port->parent_port) > > > > > > - ? port->parent_port : > port->logical_port) > > > > > > + } else if (local_binding_get_primary_pb(local_bindings, > > > > > lport_name) > > > > > > + || simap_contains(patch_ofports, > > > port->logical_port) > > > > > > || (!strcmp(port->type, "l3gateway") > > > > > > && port->chassis == chassis)) { > > > > > > put_load(port->tunnel_key, MFF_LOG_OUTPORT, 0, 32, > > > &ofpacts); > > > > > > put_resubmit(OFTABLE_CHECK_LOOPBACK, &ofpacts); > > > > > > - } else if (port->chassis && > > > !get_localnet_port(local_datapaths, > > > > > > - > mc->datapath->tunnel_key)) { > > > > > > + } else if (port->chassis && !get_localnet_port( > > > > > > + local_datapaths, mc->datapath->tunnel_key)) { > > > > > > /* Add remote chassis only when localnet port not > exist, > > > > > > * otherwise multicast will reach remote ports > through > > > > > localnet > > > > > > * port. */ > > > > > > @@ -1497,7 +1470,7 @@ consider_mc_group(enum mf_field_id > > > mff_ovn_geneve, > > > > > > const struct chassis_tunnel *prev = NULL; > > > > > > SSET_FOR_EACH (chassis_name, &remote_chassis) { > > > > > > const struct chassis_tunnel *tun > > > > > > - = chassis_tunnel_find(chassis_name, NULL); > > > > > > + = chassis_tunnel_find(chassis_tunnels, > chassis_name, > > > > > NULL); > > > > > > if (!tun) { > > > > > > continue; > > > > > > } > > > > > > @@ -1524,41 +1497,50 @@ consider_mc_group(enum mf_field_id > > > mff_ovn_geneve, > > > > > > sset_destroy(&remote_chassis); > > > > > > } > > > > > > > > > > > > -/* Replaces 'old' by 'new' (destroying 'new'). Returns true if > 'old' > > > > > and 'new' > > > > > > - * contained different data, false if they were the same. */ > > > > > > -static bool > > > > > > -update_ofports(struct simap *old, struct simap *new) > > > > > > -{ > > > > > > - bool changed = !simap_equal(old, new); > > > > > > - simap_swap(old, new); > > > > > > - simap_destroy(new); > > > > > > - return changed; > > > > > > -} > > > > > > - > > > > > > void > > > > > > -physical_handle_port_binding_changes(struct physical_ctx *p_ctx, > > > > > > - struct > ovn_desired_flow_table > > > > > *flow_table) > > > > > > +physical_handle_flows_for_lport(const struct sbrec_port_binding > *pb, > > > > > > + bool removed, struct physical_ctx > > > *p_ctx, > > > > > > + struct ovn_desired_flow_table > > > > > *flow_table) > > > > > > { > > > > > > - const struct sbrec_port_binding *binding; > > > > > > - struct ofpbuf ofpacts; > > > > > > - ofpbuf_init(&ofpacts, 0); > > > > > > - SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding, > > > > > > - > > > > > p_ctx->port_binding_table) { > > > > > > - if (sbrec_port_binding_is_deleted(binding)) { > > > > > > - ofctrl_remove_flows(flow_table, > &binding->header_.uuid); > > > > > > - } else { > > > > > > - if (!sbrec_port_binding_is_new(binding)) { > > > > > > - ofctrl_remove_flows(flow_table, > > > &binding->header_.uuid); > > > > > > - } > > > > > > + ofctrl_remove_flows(flow_table, &pb->header_.uuid); > > > > > > + > > > > > > + if (!strcmp(pb->type, "external")) { > > > > > > + /* External lports have a dependency on the localnet > port. > > > > > > + * We need to remove the flows of the localnet port as > well > > > > > > + * and re-consider adding the flows for it. > > > > > > + */ > > > > > > + struct local_datapath *ldp = > > > > > > + get_local_datapath(p_ctx->local_datapaths, > > > > > > + pb->datapath->tunnel_key); > > > > > > + if (ldp && ldp->localnet_port) { > > > > > > + struct ofpbuf ofpacts; > > > > > > + ofctrl_remove_flows(flow_table, > > > > > &ldp->localnet_port->header_.uuid); > > > > > > + ofpbuf_init(&ofpacts, 0); > > > > > > > consider_port_binding(p_ctx->sbrec_port_binding_by_name, > > > > > > p_ctx->mff_ovn_geneve, > > > p_ctx->ct_zones, > > > > > > p_ctx->active_tunnels, > > > > > > p_ctx->local_datapaths, > > > > > > - binding, p_ctx->chassis, > > > > > > + p_ctx->local_bindings, > > > > > > + > &p_ctx->nonvif_data->patch_ofports, > > > > > > + > > > &p_ctx->nonvif_data->chassis_tunnels, > > > > > > + ldp->localnet_port, > p_ctx->chassis, > > > > > > flow_table, &ofpacts); > > > > > > + ofpbuf_uninit(&ofpacts); > > > > > > } > > > > > > } > > > > > > - ofpbuf_uninit(&ofpacts); > > > > > > + > > > > > > + if (!removed) { > > > > > > + struct ofpbuf ofpacts; > > > > > > + ofpbuf_init(&ofpacts, 0); > > > > > > + consider_port_binding(p_ctx->sbrec_port_binding_by_name, > > > > > > + p_ctx->mff_ovn_geneve, > p_ctx->ct_zones, > > > > > > + p_ctx->active_tunnels, > > > > > p_ctx->local_datapaths, > > > > > > + p_ctx->local_bindings, > > > > > > + &p_ctx->nonvif_data->patch_ofports, > > > > > > + > &p_ctx->nonvif_data->chassis_tunnels, > > > pb, > > > > > > + p_ctx->chassis, flow_table, > &ofpacts); > > > > > > + ofpbuf_uninit(&ofpacts); > > > > > > + } > > > > > > } > > > > > > > > > > > > void > > > > > > @@ -1574,8 +1556,11 @@ physical_handle_mc_group_changes(struct > > > > > physical_ctx *p_ctx, > > > > > > ofctrl_remove_flows(flow_table, > &mc->header_.uuid); > > > > > > } > > > > > > consider_mc_group(p_ctx->mff_ovn_geneve, > p_ctx->ct_zones, > > > > > > - p_ctx->local_datapaths, > > > > > > - p_ctx->chassis, mc, flow_table); > > > > > > + p_ctx->local_datapaths, > > > > > p_ctx->local_bindings, > > > > > > + &p_ctx->nonvif_data->patch_ofports, > > > > > > + p_ctx->chassis, mc, > > > > > > + > &p_ctx->nonvif_data->chassis_tunnels, > > > > > > + flow_table); > > > > > > } > > > > > > } > > > > > > } > > > > > > @@ -1591,135 +1576,6 @@ physical_run(struct physical_ctx *p_ctx, > > > > > > uuid_generate(hc_uuid); > > > > > > } > > > > > > > > > > > > - /* This bool tracks physical mapping changes. */ > > > > > > - bool physical_map_changed = false; > > > > > > - > > > > > > - struct simap new_localvif_to_ofport = > > > > > > - SIMAP_INITIALIZER(&new_localvif_to_ofport); > > > > > > - struct simap new_tunnel_to_ofport = > > > > > > - SIMAP_INITIALIZER(&new_tunnel_to_ofport); > > > > > > - for (int i = 0; i < p_ctx->br_int->n_ports; i++) { > > > > > > - const struct ovsrec_port *port_rec = > p_ctx->br_int->ports[i]; > > > > > > - if (!strcmp(port_rec->name, p_ctx->br_int->name)) { > > > > > > - continue; > > > > > > - } > > > > > > - > > > > > > - const char *tunnel_id = smap_get(&port_rec->external_ids, > > > > > > - "ovn-chassis-id"); > > > > > > - if (tunnel_id && encaps_tunnel_id_match(tunnel_id, > > > > > > - > p_ctx->chassis->name, > > > > > > - NULL)) { > > > > > > - continue; > > > > > > - } > > > > > > - > > > > > > - const char *localnet = smap_get(&port_rec->external_ids, > > > > > > - "ovn-localnet-port"); > > > > > > - const char *l2gateway = smap_get(&port_rec->external_ids, > > > > > > - "ovn-l2gateway-port"); > > > > > > - > > > > > > - for (int j = 0; j < port_rec->n_interfaces; j++) { > > > > > > - const struct ovsrec_interface *iface_rec = > > > > > port_rec->interfaces[j]; > > > > > > - > > > > > > - /* Get OpenFlow port number. */ > > > > > > - if (!iface_rec->n_ofport) { > > > > > > - continue; > > > > > > - } > > > > > > - int64_t ofport = iface_rec->ofport[0]; > > > > > > - if (ofport < 1 || ofport > ofp_to_u16(OFPP_MAX)) { > > > > > > - continue; > > > > > > - } > > > > > > - > > > > > > - /* Record as patch to local net, logical patch port, > > > > > chassis, or > > > > > > - * local logical port. */ > > > > > > - bool is_patch = !strcmp(iface_rec->type, "patch"); > > > > > > - if (is_patch && localnet) { > > > > > > - /* localnet patch ports can be handled just like > > > VIFs. */ > > > > > > - simap_put(&new_localvif_to_ofport, localnet, > ofport); > > > > > > - break; > > > > > > - } else if (is_patch && l2gateway) { > > > > > > - /* L2 gateway patch ports can be handled just > like > > > VIFs. > > > > > */ > > > > > > - simap_put(&new_localvif_to_ofport, l2gateway, > > > ofport); > > > > > > - break; > > > > > > - } else if (tunnel_id) { > > > > > > - enum chassis_tunnel_type tunnel_type; > > > > > > - if (!strcmp(iface_rec->type, "geneve")) { > > > > > > - tunnel_type = GENEVE; > > > > > > - if (!p_ctx->mff_ovn_geneve) { > > > > > > - continue; > > > > > > - } > > > > > > - } else if (!strcmp(iface_rec->type, "stt")) { > > > > > > - tunnel_type = STT; > > > > > > - } else if (!strcmp(iface_rec->type, "vxlan")) { > > > > > > - tunnel_type = VXLAN; > > > > > > - } else { > > > > > > - continue; > > > > > > - } > > > > > > - > > > > > > - simap_put(&new_tunnel_to_ofport, tunnel_id, > ofport); > > > > > > - /* > > > > > > - * We split the tunnel_id to get the chassis-id > > > > > > - * and hash the tunnel list on the chassis-id. > The > > > > > > - * reason to use the chassis-id alone is because > > > > > > - * there might be cases (multicast, gateway > chassis) > > > > > > - * where we need to tunnel to the chassis, but > won't > > > > > > - * have the encap-ip specifically. > > > > > > - */ > > > > > > - char *hash_id = NULL; > > > > > > - char *ip = NULL; > > > > > > - > > > > > > - if (!encaps_tunnel_id_parse(tunnel_id, &hash_id, > > > &ip)) { > > > > > > - continue; > > > > > > - } > > > > > > - struct chassis_tunnel *tun = > > > > > chassis_tunnel_find(hash_id, ip); > > > > > > - if (tun) { > > > > > > - /* If the tunnel's ofport has changed, > update. */ > > > > > > - if (tun->ofport != u16_to_ofp(ofport) || > > > > > > - tun->type != tunnel_type) { > > > > > > - tun->ofport = u16_to_ofp(ofport); > > > > > > - tun->type = tunnel_type; > > > > > > - physical_map_changed = true; > > > > > > - } > > > > > > - } else { > > > > > > - tun = xmalloc(sizeof *tun); > > > > > > - hmap_insert(&tunnels, &tun->hmap_node, > > > > > > - hash_string(hash_id, 0)); > > > > > > - tun->chassis_id = xstrdup(tunnel_id); > > > > > > - tun->ofport = u16_to_ofp(ofport); > > > > > > - tun->type = tunnel_type; > > > > > > - physical_map_changed = true; > > > > > > - } > > > > > > - free(hash_id); > > > > > > - free(ip); > > > > > > - break; > > > > > > - } else { > > > > > > - const char *iface_id = > > > smap_get(&iface_rec->external_ids, > > > > > > - "iface-id"); > > > > > > - if (iface_id) { > > > > > > - simap_put(&new_localvif_to_ofport, iface_id, > > > ofport); > > > > > > - } > > > > > > - } > > > > > > - } > > > > > > - } > > > > > > - > > > > > > - /* Remove tunnels that are no longer here. */ > > > > > > - struct chassis_tunnel *tun, *tun_next; > > > > > > - HMAP_FOR_EACH_SAFE (tun, tun_next, hmap_node, &tunnels) { > > > > > > - if (!simap_find(&new_tunnel_to_ofport, tun->chassis_id)) > { > > > > > > - hmap_remove(&tunnels, &tun->hmap_node); > > > > > > - physical_map_changed = true; > > > > > > - free(tun->chassis_id); > > > > > > - free(tun); > > > > > > - } > > > > > > - } > > > > > > - > > > > > > - /* Capture changed or removed openflow ports. */ > > > > > > - physical_map_changed |= update_ofports(&localvif_to_ofport, > > > > > > - > &new_localvif_to_ofport); > > > > > > - if (physical_map_changed) { > > > > > > - /* Reprocess logical flow table immediately. */ > > > > > > - poll_immediate_wake(); > > > > > > - } > > > > > > - > > > > > > struct ofpbuf ofpacts; > > > > > > ofpbuf_init(&ofpacts, 0); > > > > > > > > > > > > @@ -1733,16 +1589,20 @@ physical_run(struct physical_ctx *p_ctx, > > > > > > consider_port_binding(p_ctx->sbrec_port_binding_by_name, > > > > > > p_ctx->mff_ovn_geneve, > p_ctx->ct_zones, > > > > > > p_ctx->active_tunnels, > > > > > p_ctx->local_datapaths, > > > > > > - binding, p_ctx->chassis, > > > > > > - flow_table, &ofpacts); > > > > > > + p_ctx->local_bindings, > > > > > > + &p_ctx->nonvif_data->patch_ofports, > > > > > > + > &p_ctx->nonvif_data->chassis_tunnels, > > > > > binding, > > > > > > + p_ctx->chassis, flow_table, > &ofpacts); > > > > > > } > > > > > > > > > > > > /* Handle output to multicast groups, in tables 32 and 33. */ > > > > > > const struct sbrec_multicast_group *mc; > > > > > > SBREC_MULTICAST_GROUP_TABLE_FOR_EACH (mc, > p_ctx->mc_group_table) > > > { > > > > > > consider_mc_group(p_ctx->mff_ovn_geneve, p_ctx->ct_zones, > > > > > > - p_ctx->local_datapaths, p_ctx->chassis, > > > > > > - mc, flow_table); > > > > > > + p_ctx->local_datapaths, > > > p_ctx->local_bindings, > > > > > > + &p_ctx->nonvif_data->patch_ofports, > > > > > p_ctx->chassis, > > > > > > + mc, > &p_ctx->nonvif_data->chassis_tunnels, > > > > > > + flow_table); > > > > > > } > > > > > > > > > > > > /* Table 0, priority 100. > > > > > > @@ -1757,7 +1617,8 @@ physical_run(struct physical_ctx *p_ctx, > > > > > > * We set MFF_LOG_DATAPATH, MFF_LOG_INPORT, and > MFF_LOG_OUTPORT > > > from > > > > > the > > > > > > * tunnel key data where possible, then resubmit to table 33 > to > > > > > handle > > > > > > * packets to the local hypervisor. */ > > > > > > - HMAP_FOR_EACH (tun, hmap_node, &tunnels) { > > > > > > + struct chassis_tunnel *tun; > > > > > > + HMAP_FOR_EACH (tun, hmap_node, > > > &p_ctx->nonvif_data->chassis_tunnels) > > > > > { > > > > > > struct match match = MATCH_CATCHALL_INITIALIZER; > > > > > > match_set_in_port(&match, tun->ofport); > > > > > > > > > > > > @@ -1788,7 +1649,7 @@ physical_run(struct physical_ctx *p_ctx, > > > > > > } > > > > > > > > > > > > /* Handle ramp switch encapsulations. */ > > > > > > - HMAP_FOR_EACH (tun, hmap_node, &tunnels) { > > > > > > + HMAP_FOR_EACH (tun, hmap_node, > > > &p_ctx->nonvif_data->chassis_tunnels) > > > > > { > > > > > > if (tun->type != VXLAN) { > > > > > > continue; > > > > > > } > > > > > > @@ -1925,13 +1786,10 @@ physical_run(struct physical_ctx *p_ctx, > > > > > > &ofpacts, hc_uuid); > > > > > > > > > > > > ofpbuf_uninit(&ofpacts); > > > > > > - > > > > > > - simap_destroy(&new_tunnel_to_ofport); > > > > > > } > > > > > > > > > > > > bool > > > > > > -physical_handle_ovs_iface_changes(struct physical_ctx *p_ctx, > > > > > > - struct ovn_desired_flow_table > > > > > *flow_table) > > > > > > +physical_handle_ovs_iface_changes(struct physical_ctx *p_ctx) > > > > > > { > > > > > > const struct ovsrec_interface *iface_rec; > > > > > > OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, > > > > > p_ctx->iface_table) { > > > > > > @@ -1943,64 +1801,6 @@ physical_handle_ovs_iface_changes(struct > > > > > physical_ctx *p_ctx, > > > > > > } > > > > > > } > > > > > > > > > > > > - struct ofpbuf ofpacts; > > > > > > - ofpbuf_init(&ofpacts, 0); > > > > > > - > > > > > > - OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec, > > > > > p_ctx->iface_table) { > > > > > > - const char *iface_id = smap_get(&iface_rec->external_ids, > > > > > "iface-id"); > > > > > > - if (!iface_id) { > > > > > > - continue; > > > > > > - } > > > > > > - > > > > > > - const struct sbrec_port_binding *lb_pb = > > > > > > - local_binding_get_primary_pb(p_ctx->local_bindings, > > > > > iface_id); > > > > > > - if (!lb_pb) { > > > > > > - /* For regular VIFs (e.g. lsp) the upcoming > port-binding > > > > > update > > > > > > - * will remove lfows related to the unclaimed ovs > port. > > > > > > - * Localport is a special case and it needs to be > managed > > > > > here > > > > > > - * since the port is not binded and otherwise the > related > > > > > lfows > > > > > > - * will not be cleared removing the ovs port. > > > > > > - */ > > > > > > - lb_pb = > > > > > lport_lookup_by_name(p_ctx->sbrec_port_binding_by_name, > > > > > > - iface_id); > > > > > > - if (!lb_pb || strcmp(lb_pb->type, "localport")) { > > > > > > - continue; > > > > > > - } > > > > > > - } > > > > > > - > > > > > > - int64_t ofport = iface_rec->n_ofport ? > *iface_rec->ofport : > > > 0; > > > > > > - if (ovsrec_interface_is_deleted(iface_rec)) { > > > > > > - ofctrl_remove_flows(flow_table, > &lb_pb->header_.uuid); > > > > > > - simap_find_and_delete(&localvif_to_ofport, iface_id); > > > > > > - } else { > > > > > > - if (!ovsrec_interface_is_new(iface_rec)) { > > > > > > - ofctrl_remove_flows(flow_table, > > > &lb_pb->header_.uuid); > > > > > > - } > > > > > > - > > > > > > - simap_put(&localvif_to_ofport, iface_id, ofport); > > > > > > - > consider_port_binding(p_ctx->sbrec_port_binding_by_name, > > > > > > - p_ctx->mff_ovn_geneve, > > > p_ctx->ct_zones, > > > > > > - p_ctx->active_tunnels, > > > > > > - p_ctx->local_datapaths, > > > > > > - lb_pb, p_ctx->chassis, > > > > > > - flow_table, &ofpacts); > > > > > > - } > > > > > > - } > > > > > > - > > > > > > - ofpbuf_uninit(&ofpacts); > > > > > > - return true; > > > > > > -} > > > > > > - > > > > > > -bool > > > > > > -get_tunnel_ofport(const char *chassis_name, char *encap_ip, > > > ofp_port_t > > > > > *ofport) > > > > > > -{ > > > > > > - struct chassis_tunnel *tun = NULL; > > > > > > - tun = chassis_tunnel_find(chassis_name, encap_ip); > > > > > > - if (!tun) { > > > > > > - return false; > > > > > > - } > > > > > > - > > > > > > - *ofport = tun->ofport; > > > > > > return true; > > > > > > } > > > > > > > > > > > > diff --git a/controller/physical.h b/controller/physical.h > > > > > > index feab41df4..3fdc20005 100644 > > > > > > --- a/controller/physical.h > > > > > > +++ b/controller/physical.h > > > > > > @@ -34,6 +34,7 @@ struct simap; > > > > > > struct sbrec_multicast_group_table; > > > > > > struct sbrec_port_binding_table; > > > > > > struct sset; > > > > > > +struct local_nonvif_data; > > > > > > > > > > > > /* OVN Geneve option information. > > > > > > * > > > > > > @@ -56,18 +57,18 @@ struct physical_ctx { > > > > > > const struct simap *ct_zones; > > > > > > enum mf_field_id mff_ovn_geneve; > > > > > > struct shash *local_bindings; > > > > > > + struct local_nonvif_data *nonvif_data; > > > > > > }; > > > > > > > > > > > > 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 > > > > > *); > > > > > > -void physical_handle_port_binding_changes(struct physical_ctx *, > > > > > > - struct > > > ovn_desired_flow_table > > > > > *); > > > > > > void physical_handle_mc_group_changes(struct physical_ctx *, > > > > > > struct > ovn_desired_flow_table > > > *); > > > > > > -bool physical_handle_ovs_iface_changes(struct physical_ctx *, > > > > > > - struct > ovn_desired_flow_table > > > *); > > > > > > -bool get_tunnel_ofport(const char *chassis_name, char *encap_ip, > > > > > > - ofp_port_t *ofport); > > > > > > +bool physical_handle_ovs_iface_changes(struct physical_ctx *); > > > > > > +void physical_handle_flows_for_lport(const struct > sbrec_port_binding > > > *, > > > > > > + bool removed, > > > > > > + struct physical_ctx *, > > > > > > + struct > ovn_desired_flow_table > > > *); > > > > > > #endif /* controller/physical.h */ > > > > > > -- > > > > > > 2.31.1 > > > > > > > > > > > > _______________________________________________ > > > > > > dev mailing list > > > > > > d...@openvswitch.org > > > > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > _______________________________________________ > > > > > dev mailing list > > > > > d...@openvswitch.org > > > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > > > > _______________________________________________ > > > dev mailing list > > > d...@openvswitch.org > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev