[ovs-dev] [PATCH ovn v9 2/8] ovn-controller: I-P for SB port binding and OVS interface in runtime_data.
Han Zhou
hzhou at ovn.org
Wed Jun 3 23:17:42 UTC 2020
On Wed, Jun 3, 2020 at 12:42 AM Numan Siddique <numans at ovn.org> wrote:
>
>
>
> On Wed, Jun 3, 2020 at 10:49 AM Han Zhou <hzhou at ovn.org> wrote:
>>
>> On Thu, May 28, 2020 at 4:05 AM <numans at ovn.org> wrote:
>> >
>> > From: Numan Siddique <numans at ovn.org>
>> >
>> > This patch handles SB port binding changes and OVS interface changes
>> > incrementally in the runtime_data stage which otherwise would have
>> > resulted in calls to binding_run().
>> >
>> > Prior to this patch, port binding changes were handled differently.
>> > The changes were only evaluated to see if they need full recompute
>> > or they can be ignored.
>> >
>> > With this patch, the runtime data is updated with the changes (both
>> > SB port binding and OVS interface) and ports are claimed/released in
>> > the handlers itself, avoiding the need to trigger recompute of runtime
>> > data stage.
>> >
>> > Acked-by: Mark Michelson <mmichels at redhat.com>
>> > Signed-off-by: Numan Siddique <numans at ovn.org>
>> > ---
>> > controller/binding.c | 1002 ++++++++++++++++++++++++++++++-----
>> > controller/binding.h | 9 +-
>> > controller/ovn-controller.c | 61 ++-
>> > tests/ovn-performance.at | 13 +
>> > 4 files changed, 953 insertions(+), 132 deletions(-)
>> >
>> > diff --git a/controller/binding.c b/controller/binding.c
>> > index 6a13d1a0e..74e0e0710 100644
>> > --- a/controller/binding.c
>> > +++ b/controller/binding.c
>> > @@ -360,16 +360,6 @@ destroy_qos_map(struct hmap *qos_map)
>> > hmap_destroy(qos_map);
>> > }
>> >
>> > -static void
>> > -update_local_lport_ids(struct sset *local_lport_ids,
>> > - const struct sbrec_port_binding *pb)
>> > -{
>> > - char buf[16];
>> > - snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
>> > - pb->datapath->tunnel_key, pb->tunnel_key);
>> > - sset_add(local_lport_ids, buf);
>> > -}
>> > -
>> > /*
>> > * Get the encap from the chassis for this port. The interface
>> > * may have an external_ids:encap-ip=<encap-ip> set; if so we
>> > @@ -448,10 +438,10 @@ is_network_plugged(const struct
sbrec_port_binding
>> *binding_rec,
>> > }
>> >
>> > static void
>> > -consider_localnet_port(const struct sbrec_port_binding *binding_rec,
>> > - struct shash *bridge_mappings,
>> > - struct sset *egress_ifaces,
>> > - struct hmap *local_datapaths)
>> > +update_ld_localnet_port(const struct sbrec_port_binding *binding_rec,
>> > + struct shash *bridge_mappings,
>> > + struct sset *egress_ifaces,
>> > + struct hmap *local_datapaths)
>> > {
>> > /* Ignore localnet ports for unplugged networks. */
>> > if (!is_network_plugged(binding_rec, bridge_mappings)) {
>> > @@ -481,6 +471,27 @@ consider_localnet_port(const struct
>> sbrec_port_binding *binding_rec,
>> > ld->localnet_port = binding_rec;
>> > }
>> >
>> > +static void
>> > +update_local_lport_ids(struct sset *local_lport_ids,
>> > + const struct sbrec_port_binding *pb)
>> > +{
>> > + char buf[16];
>> > + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
>> > + pb->datapath->tunnel_key, pb->tunnel_key);
>> > + sset_add(local_lport_ids, buf);
>> > +}
>> > +
>> > +static void
>> > +remove_local_lport_ids(const struct sbrec_port_binding *binding_rec,
>> > + struct sset *local_lport_ids)
>> > +{
>> > + char buf[16];
>> > + snprintf(buf, sizeof(buf), "%"PRId64"_%"PRId64,
>> > + binding_rec->datapath->tunnel_key,
>> > + binding_rec->tunnel_key);
>> > + sset_find_and_delete(local_lport_ids, buf);
>> > +}
>> > +
>> > /* 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
>> > @@ -520,6 +531,10 @@ consider_localnet_port(const struct
>> sbrec_port_binding *binding_rec,
>> > * BT_VIF. Its Port_Binding row's 'parent' column is
>> set to
>> > * its parent's Port_Binding. It shares the OVS
>> interface row
>> > * with the parent.
>> > + * Each ovn-controller when it sees a container
>> Port_Binding,
>> > + * it creates 'struct local_binding' for the parent
>> > + * Port_Binding and for its even if the OVS interface
>> row for
>> > + * the parent is not present.
>> > *
>> > * BT_VIRTUAL: Represents a local binding which has a parent of type
>> BT_VIF.
>> > * Its Port_Binding type is "virtual" and it shares the
OVS
>> > @@ -527,6 +542,17 @@ consider_localnet_port(const struct
>> sbrec_port_binding *binding_rec,
>> > * Port_Binding of type "virtual" is claimed by pinctrl
>> module
>> > * when it sees the ARP packet from the parent's VIF.
>> > *
>> > + *
>> > + * An object of 'struct local_binding' is created:
>> > + * - For each interface that has iface-id configured with the type
-
>> BT_VIF.
>> > + *
>> > + * - For each container Port Binding (of type BT_CONTAINER) and its
>> > + * parent Port_Binding (of type BT_VIF), no matter if
>> > + * they are bound to this chassis i.e even if OVS interface row
for
>> the
>> > + * parent is not present.
>> > + *
>> > + * - For each 'virtual' Port Binding (of type BT_VIRTUAL) provided
its
>> parent
>> > + * is bound to this chassis.
>>
>> Numan, thanks for more detailed comments! Still one question here. Why
>> would the handling of 'virtual' ports be different from 'container'
ports?
>> Both of them have parents and if parents are bound to a chassis, the
>> children should be considered. Now we create local_binding for all
>> container ports, but create local_bindings for virtual ports only if its
>> parent is bound to this chassis. Could you explain why this difference?
>
>
> Thanks for the review.
>
> In the case of virtual ports, the port is bound by the pinctrl module
when it sees an ARP
> packet from the parent port. So we don't have to create lbindings for
the parent port
> and the virtual port if there is no VIF interface for it.
OK, I see. Thanks for explaining.
>
> Let me know if I was not clear enough. I'll add the comment about the
same in v10 p2.
>
> Thanks
> Numan
>
>>
>> > */
>> > enum local_binding_type {
>> > BT_VIF,
>> > @@ -598,6 +624,14 @@ local_bindings_destroy(struct shash
*local_bindings)
>> > shash_destroy(local_bindings);
>> > }
>> >
>> > +static
>> > +void local_binding_delete(struct shash *local_bindings,
>> > + struct local_binding *lbinding)
>> > +{
>> > + shash_find_and_delete(local_bindings, lbinding->name);
>> > + local_binding_destroy(lbinding);
>> > +}
>> > +
>> > static void
>> > local_binding_add_child(struct local_binding *lbinding,
>> > struct local_binding *child)
>> > @@ -624,6 +658,7 @@ is_lport_container(const struct sbrec_port_binding
>> *pb)
>> > return is_lport_vif(pb) && pb->parent_port && pb->parent_port[0];
>> > }
>> >
>> > +/* Corresponds to each Port_Binding.type. */
>> > enum en_lport_type {
>> > LP_UNKNOWN,
>> > LP_VIF,
>> > @@ -669,12 +704,20 @@ get_lport_type(const struct sbrec_port_binding
*pb)
>> > return LP_UNKNOWN;
>> > }
>> >
>> > -static void
>> > +/* Returns false if lport is not claimed due to 'sb_readonly'.
>> > + * Returns true otherwise.
>> > + */
>> > +static bool
>> > claim_lport(const struct sbrec_port_binding *pb,
>> > const struct sbrec_chassis *chassis_rec,
>> > - const struct ovsrec_interface *iface_rec)
>> > + const struct ovsrec_interface *iface_rec,
>> > + bool sb_readonly)
>> > {
>> > if (pb->chassis != chassis_rec) {
>> > + if (sb_readonly) {
>> > + return false;
>> > + }
>> > +
>> > if (pb->chassis) {
>> > VLOG_INFO("Changing chassis for lport %s from %s to %s.",
>> > pb->logical_port, pb->chassis->name,
>> > @@ -693,22 +736,44 @@ claim_lport(const struct sbrec_port_binding *pb,
>> > struct sbrec_encap *encap_rec =
>> > sbrec_get_port_encap(chassis_rec, iface_rec);
>> > if (encap_rec && pb->encap != encap_rec) {
>> > + if (sb_readonly) {
>> > + return false;
>> > + }
>> > sbrec_port_binding_set_encap(pb, encap_rec);
>> > }
>> > +
>> > + return true;
>> > }
>> >
>> > -static void
>> > -release_lport(const struct sbrec_port_binding *pb)
>> > +/* Returns false if lport is not released due to 'sb_readonly'.
>> > + * Returns true otherwise.
>> > + */
>> > +static bool
>> > +release_lport(const struct sbrec_port_binding *pb, bool sb_readonly)
>> > {
>> > - VLOG_INFO("Releasing lport %s from this chassis.",
pb->logical_port);
>> > if (pb->encap) {
>> > + if (sb_readonly) {
>> > + return false;
>> > + }
>> > sbrec_port_binding_set_encap(pb, NULL);
>> > }
>> > - sbrec_port_binding_set_chassis(pb, NULL);
>> > +
>> > + if (pb->chassis) {
>> > + if (sb_readonly) {
>> > + return false;
>> > + }
>> > + sbrec_port_binding_set_chassis(pb, NULL);
>> > + }
>> >
>> > if (pb->virtual_parent) {
>> > + if (sb_readonly) {
>> > + return false;
>> > + }
>> > sbrec_port_binding_set_virtual_parent(pb, NULL);
>> > }
>> > +
>> > + VLOG_INFO("Releasing lport %s from this chassis.",
pb->logical_port);
>> > + return true;
>> > }
>> >
>> > static bool
>> > @@ -733,7 +798,63 @@ can_bind_on_this_chassis(const struct
sbrec_chassis
>> *chassis_rec,
>> > || !strcmp(requested_chassis, chassis_rec->hostname);
>> > }
>> >
>> > -static void
>> > +/* Returns 'true' if the 'lbinding' has children of type BT_CONTAINER,
>> > + * 'false' otherwise. */
>> > +static bool
>> > +is_lbinding_container_parent(struct local_binding *lbinding)
>> > +{
>> > + struct shash_node *node;
>> > + SHASH_FOR_EACH (node, &lbinding->children) {
>> > + struct local_binding *l = node->data;
>> > + if (l->type == BT_CONTAINER) {
>> > + return true;
>> > + }
>> > + }
>> > +
>> > + return false;
>> > +}
>> > +
>> > +static bool
>> > +release_local_binding_children(const struct sbrec_chassis
*chassis_rec,
>> > + struct local_binding *lbinding,
>> > + bool sb_readonly)
>> > +{
>> > + struct shash_node *node;
>> > + SHASH_FOR_EACH (node, &lbinding->children) {
>> > + struct local_binding *l = node->data;
>> > + if (is_lbinding_this_chassis(l, chassis_rec)) {
>> > + if (!release_lport(l->pb, sb_readonly)) {
>> > + return false;
>> > + }
>> > + }
>> > +
>> > + /* Clear the local bindings' 'pb' and 'iface'. */
>> > + l->pb = NULL;
>> > + l->iface = NULL;
>> > + }
>> > +
>> > + return true;
>> > +}
>> > +
>> > +static bool
>> > +release_local_binding(const struct sbrec_chassis *chassis_rec,
>> > + struct local_binding *lbinding, bool
sb_readonly)
>> > +{
>> > + if (!release_local_binding_children(chassis_rec, lbinding,
>> > + sb_readonly)) {
>> > + return false;
>> > + }
>> > +
>> > + if (is_lbinding_this_chassis(lbinding, chassis_rec)) {
>> > + return release_lport(lbinding->pb, sb_readonly);
>> > + }
>> > +
>> > + lbinding->pb = NULL;
>> > + lbinding->iface = NULL;
>> > + return true;
>> > +}
>> > +
>> > +static bool
>> > consider_vif_lport_(const struct sbrec_port_binding *pb,
>> > bool can_bind, const char *vif_chassis,
>> > struct binding_ctx_in *b_ctx_in,
>> > @@ -745,7 +866,10 @@ consider_vif_lport_(const struct
sbrec_port_binding
>> *pb,
>> > if (lbinding_set) {
>> > if (can_bind) {
>> > /* We can claim the lport. */
>> > - claim_lport(pb, b_ctx_in->chassis_rec, lbinding->iface);
>> > + if (!claim_lport(pb, b_ctx_in->chassis_rec,
lbinding->iface,
>> > + !b_ctx_in->ovnsb_idl_txn)){
>> > + return false;
>> > + }
>> >
>> >
add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
>> >
b_ctx_in->sbrec_port_binding_by_datapath,
>> > @@ -771,13 +895,14 @@ consider_vif_lport_(const struct
sbrec_port_binding
>> *pb,
>> > if (pb->chassis == b_ctx_in->chassis_rec) {
>> > /* Release the lport if there is no lbinding. */
>> > if (!lbinding_set || !can_bind) {
>> > - release_lport(pb);
>> > + return release_lport(pb, !b_ctx_in->ovnsb_idl_txn);
>> > }
>> > }
>> >
>> > + return true;
>> > }
>> >
>> > -static void
>> > +static bool
>> > consider_vif_lport(const struct sbrec_port_binding *pb,
>> > struct binding_ctx_in *b_ctx_in,
>> > struct binding_ctx_out *b_ctx_out,
>> > @@ -797,11 +922,11 @@ consider_vif_lport(const struct
sbrec_port_binding
>> *pb,
>> > lbinding->pb = pb;
>> > }
>> >
>> > - consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in,
>> > - b_ctx_out, lbinding, qos_map);
>> > + return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in,
>> > + b_ctx_out, lbinding, qos_map);
>> > }
>> >
>> > -static void
>> > +static bool
>> > consider_container_lport(const struct sbrec_port_binding *pb,
>> > struct binding_ctx_in *b_ctx_in,
>> > struct binding_ctx_out *b_ctx_out,
>> > @@ -811,7 +936,39 @@ consider_container_lport(const struct
>> sbrec_port_binding *pb,
>> > parent_lbinding = local_binding_find(b_ctx_out->local_bindings,
>> > pb->parent_port);
>> >
>> > - if (parent_lbinding && !parent_lbinding->pb) {
>> > + if (!parent_lbinding) {
>> > + /* There is no local_binding for parent port. Create it
>> > + * without OVS interface row. This is the only exception
>> > + * for creating the 'struct local_binding' object without
>> > + * corresponding OVS interface row.
>> > + *
>> > + * This is required for the following reasons:
>> > + * - If a logical port P1 is created and then
>> > + * few container ports - C1, C2, .. are created first by
CMS.
>> > + * - And later when OVS interface row is created for P1,
then
>> > + * we want the these container ports also be claimed by
the
>> > + * chassis.
>> > + * */
>> > + parent_lbinding = local_binding_create(pb->parent_port, NULL,
>> NULL,
>> > + BT_VIF);
>> > + local_binding_add(b_ctx_out->local_bindings, parent_lbinding);
>> > + }
>> > +
>> > + struct local_binding *container_lbinding =
>> > + local_binding_find_child(parent_lbinding, pb->logical_port);
>> > +
>> > + if (!container_lbinding) {
>> > + container_lbinding = local_binding_create(pb->logical_port,
>> > +
parent_lbinding->iface,
>> > + pb, BT_CONTAINER);
>> > + local_binding_add_child(parent_lbinding, container_lbinding);
>> > + } else {
>> > + ovs_assert(container_lbinding->type == BT_CONTAINER);
>> > + container_lbinding->pb = pb;
>> > + container_lbinding->iface = parent_lbinding->iface;
>> > + }
>> > +
>> > + if (!parent_lbinding->pb) {
>> > parent_lbinding->pb = lport_lookup_by_name(
>> > b_ctx_in->sbrec_port_binding_by_name, pb->parent_port);
>> >
>> > @@ -820,37 +977,28 @@ consider_container_lport(const struct
>> sbrec_port_binding *pb,
>> > * So call consider_vif_lport() to process it first. */
>> > consider_vif_lport(parent_lbinding->pb, b_ctx_in,
b_ctx_out,
>> > parent_lbinding, qos_map);
>> > - }
>> > - }
>> > + } else {
>> > + /* The parent lport doesn't exist. Call release_lport() to
>> > + * release the container lport, if it was bound earlier.
*/
>> > + if (is_lbinding_this_chassis(container_lbinding,
>> > + b_ctx_in->chassis_rec)) {
>> > + return release_lport(pb, !b_ctx_in->ovnsb_idl_txn);
>> > + }
>> >
>> > - if (!parent_lbinding || !parent_lbinding->pb) {
>> > - /* Call release_lport, to release the container lport, if
>> > - * it was bound earlier. */
>> > - if (pb->chassis == b_ctx_in->chassis_rec) {
>> > - release_lport(pb);
>> > + return true;
>> > }
>> > - return;
>> > }
>> >
>> > - struct local_binding *container_lbinding =
>> > - local_binding_find_child(parent_lbinding, pb->logical_port);
>> > - ovs_assert(!container_lbinding);
>> > -
>> > - container_lbinding = local_binding_create(pb->logical_port,
>> > - parent_lbinding->iface,
>> > - pb, BT_CONTAINER);
>> > - local_binding_add_child(parent_lbinding, container_lbinding);
>> > -
>> > const char *vif_chassis = smap_get(&parent_lbinding->pb->options,
>> > "requested-chassis");
>> > bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec,
>> > vif_chassis);
>> >
>> > - consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in,
b_ctx_out,
>> > - container_lbinding, qos_map);
>> > + return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in,
>> b_ctx_out,
>> > + container_lbinding, qos_map);
>> > }
>> >
>> > -static void
>> > +static bool
>> > consider_virtual_lport(const struct sbrec_port_binding *pb,
>> > struct binding_ctx_in *b_ctx_in,
>> > struct binding_ctx_out *b_ctx_out,
>> > @@ -877,11 +1025,16 @@ consider_virtual_lport(const struct
>> sbrec_port_binding *pb,
>> > if (is_lbinding_this_chassis(parent_lbinding,
>> b_ctx_in->chassis_rec)) {
>> > virtual_lbinding =
>> > local_binding_find_child(parent_lbinding,
pb->logical_port);
>> > - ovs_assert(!virtual_lbinding);
>> > - virtual_lbinding = local_binding_create(pb->logical_port,
>> > -
parent_lbinding->iface,
>> > - pb, BT_VIRTUAL);
>> > - local_binding_add_child(parent_lbinding, virtual_lbinding);
>> > + if (!virtual_lbinding) {
>> > + virtual_lbinding = local_binding_create(pb->logical_port,
>> > +
>> parent_lbinding->iface,
>> > + pb, BT_VIRTUAL);
>> > + local_binding_add_child(parent_lbinding,
virtual_lbinding);
>> > + } else {
>> > + ovs_assert(virtual_lbinding->type == BT_VIRTUAL);
>> > + virtual_lbinding->pb = pb;
>> > + virtual_lbinding->iface = parent_lbinding->iface;
>> > + }
>> > }
>> >
>> > return consider_vif_lport_(pb, true, NULL, b_ctx_in, b_ctx_out,
>> > @@ -891,14 +1044,13 @@ consider_virtual_lport(const struct
>> sbrec_port_binding *pb,
>> > /* Considers either claiming the lport or releasing the lport
>> > * for non VIF lports.
>> > */
>> > -static void
>> > +static bool
>> > consider_nonvif_lport_(const struct sbrec_port_binding *pb,
>> > bool our_chassis,
>> > bool has_local_l3gateway,
>> > struct binding_ctx_in *b_ctx_in,
>> > struct binding_ctx_out *b_ctx_out)
>> > {
>> > - ovs_assert(b_ctx_in->ovnsb_idl_txn);
>> > if (our_chassis) {
>> > sset_add(b_ctx_out->local_lports, pb->logical_port);
>> > add_local_datapath(b_ctx_in->sbrec_datapath_binding_by_key,
>> > @@ -908,13 +1060,16 @@ consider_nonvif_lport_(const struct
>> sbrec_port_binding *pb,
>> > b_ctx_out->local_datapaths);
>> >
>> > update_local_lport_ids(b_ctx_out->local_lport_ids, pb);
>> > - claim_lport(pb, b_ctx_in->chassis_rec, NULL);
>> > + return claim_lport(pb, b_ctx_in->chassis_rec, NULL,
>> > + !b_ctx_in->ovnsb_idl_txn);
>> > } else if (pb->chassis == b_ctx_in->chassis_rec) {
>> > - release_lport(pb);
>> > + return release_lport(pb, !b_ctx_in->ovnsb_idl_txn);
>> > }
>> > +
>> > + return true;
>> > }
>> >
>> > -static void
>> > +static bool
>> > consider_l2gw_lport(const struct sbrec_port_binding *pb,
>> > struct binding_ctx_in *b_ctx_in,
>> > struct binding_ctx_out *b_ctx_out)
>> > @@ -923,10 +1078,10 @@ consider_l2gw_lport(const struct
>> sbrec_port_binding *pb,
>> > bool our_chassis = chassis_id && !strcmp(chassis_id,
>> >
>> b_ctx_in->chassis_rec->name);
>> >
>> > - consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in,
b_ctx_out);
>> > + return consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in,
>> b_ctx_out);
>> > }
>> >
>> > -static void
>> > +static bool
>> > consider_l3gw_lport(const struct sbrec_port_binding *pb,
>> > struct binding_ctx_in *b_ctx_in,
>> > struct binding_ctx_out *b_ctx_out)
>> > @@ -935,7 +1090,7 @@ consider_l3gw_lport(const struct
sbrec_port_binding
>> *pb,
>> > bool our_chassis = chassis_id && !strcmp(chassis_id,
>> >
>> b_ctx_in->chassis_rec->name);
>> >
>> > - consider_nonvif_lport_(pb, our_chassis, true, b_ctx_in,
b_ctx_out);
>> > + return consider_nonvif_lport_(pb, our_chassis, true, b_ctx_in,
>> b_ctx_out);
>> > }
>> >
>> > static void
>> > @@ -954,7 +1109,7 @@ consider_localnet_lport(const struct
>> sbrec_port_binding *pb,
>> > update_local_lport_ids(b_ctx_out->local_lport_ids, pb);
>> > }
>> >
>> > -static void
>> > +static bool
>> > consider_ha_lport(const struct sbrec_port_binding *pb,
>> > struct binding_ctx_in *b_ctx_in,
>> > struct binding_ctx_out *b_ctx_out)
>> > @@ -982,18 +1137,18 @@ consider_ha_lport(const struct
sbrec_port_binding
>> *pb,
>> > b_ctx_out->local_datapaths);
>> > }
>> >
>> > - consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in,
b_ctx_out);
>> > + return consider_nonvif_lport_(pb, our_chassis, false, b_ctx_in,
>> b_ctx_out);
>> > }
>> >
>> > -static void
>> > +static bool
>> > consider_cr_lport(const struct sbrec_port_binding *pb,
>> > struct binding_ctx_in *b_ctx_in,
>> > struct binding_ctx_out *b_ctx_out)
>> > {
>> > - consider_ha_lport(pb, b_ctx_in, b_ctx_out);
>> > + return consider_ha_lport(pb, b_ctx_in, b_ctx_out);
>> > }
>> >
>> > -static void
>> > +static bool
>> > consider_external_lport(const struct sbrec_port_binding *pb,
>> > struct binding_ctx_in *b_ctx_in,
>> > struct binding_ctx_out *b_ctx_out)
>> > @@ -1046,6 +1201,8 @@ build_local_bindings(struct binding_ctx_in
>> *b_ctx_in,
>> > }
>> >
>> > sset_add(b_ctx_out->local_lports, iface_id);
>> > + smap_replace(b_ctx_out->local_iface_ids,
iface_rec->name,
>> > + iface_id);
>> > }
>> >
>> > /* Check if this is a tunnel interface. */
>> > @@ -1161,9 +1318,9 @@ binding_run(struct binding_ctx_in *b_ctx_in,
struct
>> binding_ctx_out *b_ctx_out)
>> > * corresponding local datapath accordingly. */
>> > struct localnet_lport *lnet_lport;
>> > LIST_FOR_EACH_POP (lnet_lport, list_node, &localnet_lports) {
>> > - consider_localnet_port(lnet_lport->pb, &bridge_mappings,
>> > - b_ctx_out->egress_ifaces,
>> > - b_ctx_out->local_datapaths);
>> > + update_ld_localnet_port(lnet_lport->pb, &bridge_mappings,
>> > + b_ctx_out->egress_ifaces,
>> > + b_ctx_out->local_datapaths);
>> > free(lnet_lport);
>> > }
>> >
>> > @@ -1181,95 +1338,688 @@ binding_run(struct binding_ctx_in *b_ctx_in,
>> struct binding_ctx_out *b_ctx_out)
>> > destroy_qos_map(&qos_map);
>> > }
>> >
>> > -/* Returns true if port-binding changes potentially require flow
changes
>> on
>> > - * the current chassis. Returns false if we are sure there is no
impact.
>> */
>> > +/* Returns true if the database is all cleaned up, false if more work
is
>> > + * required. */
>> > bool
>> > -binding_evaluate_port_binding_changes(struct binding_ctx_in *b_ctx_in,
>> > - struct binding_ctx_out
*b_ctx_out)
>> > +binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
>> > + const struct sbrec_port_binding_table
>> *port_binding_table,
>> > + const struct sbrec_chassis *chassis_rec)
>> > {
>> > - if (!b_ctx_in->chassis_rec) {
>> > + if (!ovnsb_idl_txn) {
>> > + return false;
>> > + }
>> > + if (!chassis_rec) {
>> > return true;
>> > }
>> >
>> > - bool changed = false;
>> > -
>> > const struct sbrec_port_binding *binding_rec;
>> > - SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding_rec,
>> > -
>> b_ctx_in->port_binding_table) {
>> > - /* XXX: currently OVSDB change tracking doesn't support
getting
>> old
>> > - * data when the operation is update, so if a port-binding
moved
>> from
>> > - * this chassis to another, there is no easy way to find out
the
>> > - * change. To workaround this problem, we just makes sure if
>> > - * any port *related to* this chassis has any change, then
>> trigger
>> > - * recompute.
>> > - *
>> > - * - If a regular VIF is unbound from this chassis, the local
>> ovsdb
>> > - * interface table will be updated, which will trigger
>> recompute.
>> > - *
>> > - * - If the port is not a regular VIF, always trigger
recompute.
>> */
>> > - if (binding_rec->chassis == b_ctx_in->chassis_rec) {
>> > - changed = true;
>> > + bool any_changes = false;
>> > + SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec,
port_binding_table) {
>> > + if (binding_rec->chassis == chassis_rec) {
>> > + if (binding_rec->encap) {
>> > + sbrec_port_binding_set_encap(binding_rec, NULL);
>> > + }
>> > + sbrec_port_binding_set_chassis(binding_rec, NULL);
>> > + any_changes = true;
>> > + }
>> > + }
>> > +
>> > + if (any_changes) {
>> > + ovsdb_idl_txn_add_comment(
>> > + ovnsb_idl_txn,
>> > + "ovn-controller: removing all port bindings for '%s'",
>> > + chassis_rec->name);
>> > + }
>> > +
>> > + return !any_changes;
>> > +}
>> > +
>> > +/* This function adds the local datapath of the 'peer' of
>> > + * lport 'pb' to the local datapaths if it is not yet added.
>> > + */
>> > +static void
>> > +add_local_datapath_peer_port(const struct sbrec_port_binding *pb,
>> > + struct binding_ctx_in *b_ctx_in,
>> > + struct binding_ctx_out *b_ctx_out,
>> > + struct local_datapath *ld)
>> > +{
>> > + const char *peer_name = smap_get(&pb->options, "peer");
>> > + if (strcmp(pb->type, "patch") || !peer_name) {
>> > + return;
>> > + }
>> > +
>> > + const struct sbrec_port_binding *peer;
>> > + peer = lport_lookup_by_name(b_ctx_in->sbrec_port_binding_by_name,
>> > + peer_name);
>> > +
>> > + if (!peer || !peer->datapath) {
>> > + return;
>> > + }
>> > +
>> > + bool present = false;
>> > + for (size_t i = 0; i < ld->n_peer_ports; i++) {
>> > + if (ld->peer_ports[i].local == pb) {
>> > + present = true;
>> > break;
>> > }
>> > + }
>> >
>> > - if (!strcmp(binding_rec->type, "remote")) {
>> > - continue;
>> > + if (!present) {
>> > + ld->n_peer_ports++;
>> > + if (ld->n_peer_ports > ld->n_allocated_peer_ports) {
>> > + ld->peer_ports =
>> > + x2nrealloc(ld->peer_ports,
>> > + &ld->n_allocated_peer_ports,
>> > + sizeof *ld->peer_ports);
>> > }
>> > + ld->peer_ports[ld->n_peer_ports - 1].local = pb;
>> > + ld->peer_ports[ld->n_peer_ports - 1].remote = peer;
>> > + }
>> > +
>> > + struct local_datapath *peer_ld =
>> > + get_local_datapath(b_ctx_out->local_datapaths,
>> > + peer->datapath->tunnel_key);
>> > + if (!peer_ld) {
>> > + add_local_datapath__(b_ctx_in->sbrec_datapath_binding_by_key,
>> > + b_ctx_in->sbrec_port_binding_by_datapath,
>> > + b_ctx_in->sbrec_port_binding_by_name,
>> > + peer->datapath, false,
>> > + 1, b_ctx_out->local_datapaths);
>> > + return;
>> > + }
>> >
>> > - if (strcmp(binding_rec->type, "")) {
>> > - changed = true;
>> > + for (size_t i = 0; i < peer_ld->n_peer_ports; i++) {
>> > + if (peer_ld->peer_ports[i].local == peer) {
>> > + return;
>> > + }
>> > + }
>> > +
>> > + peer_ld->n_peer_ports++;
>> > + if (peer_ld->n_peer_ports > peer_ld->n_allocated_peer_ports) {
>> > + peer_ld->peer_ports =
>> > + x2nrealloc(peer_ld->peer_ports,
>> > + &peer_ld->n_allocated_peer_ports,
>> > + sizeof *peer_ld->peer_ports);
>> > + }
>> > + peer_ld->peer_ports[peer_ld->n_peer_ports - 1].local = peer;
>> > + peer_ld->peer_ports[peer_ld->n_peer_ports - 1].remote = pb;
>> > +}
>> > +
>> > +static void
>> > +remove_local_datapath_peer_port(const struct sbrec_port_binding *pb,
>> > + struct local_datapath *ld,
>> > + struct hmap *local_datapaths)
>> > +{
>> > + size_t i = 0;
>> > + for (i = 0; i < ld->n_peer_ports; i++) {
>> > + if (ld->peer_ports[i].local == pb) {
>> > break;
>> > }
>> > + }
>> > +
>> > + if (i == ld->n_peer_ports) {
>> > + return;
>> > + }
>> > +
>> > + const struct sbrec_port_binding *peer = ld->peer_ports[i].remote;
>> >
>> > - struct local_binding *lbinding = NULL;
>> > - if (!binding_rec->parent_port ||
!binding_rec->parent_port[0]) {
>> > - lbinding = local_binding_find(b_ctx_out->local_bindings,
>> > - binding_rec->logical_port);
>> > + /* Possible improvement: We can shrint the allocated peer ports
>> > + * if (ld->n_peer_ports < ld->n_allocated_peer_ports / 2).
>> > + */
>> > + ld->peer_ports[i].local = ld->peer_ports[ld->n_peer_ports -
1].local;
>> > + ld->peer_ports[i].remote = ld->peer_ports[ld->n_peer_ports -
>> 1].remote;
>> > + ld->n_peer_ports--;
>> > +
>> > + struct local_datapath *peer_ld =
>> > + get_local_datapath(local_datapaths,
peer->datapath->tunnel_key);
>> > + if (peer_ld) {
>> > + /* Remove the peer port from the peer datapath. The peer
>> > + * datapath also tries to remove its peer lport, but that
would
>> > + * be no-op. */
>> > + remove_local_datapath_peer_port(peer, peer_ld,
local_datapaths);
>> > + }
>> > +}
>> > +
>> > +static void
>> > +remove_pb_from_local_datapath(const struct sbrec_port_binding *pb,
>> > + const struct sbrec_chassis *chassis_rec,
>> > + struct binding_ctx_out *b_ctx_out,
>> > + struct local_datapath *ld)
>> > +{
>> > + remove_local_lport_ids(pb, b_ctx_out->local_lport_ids);
>> > + if (!strcmp(pb->type, "patch") ||
>> > + !strcmp(pb->type, "l3gateway")) {
>> > + remove_local_datapath_peer_port(pb, ld,
>> b_ctx_out->local_datapaths);
>> > + } else if (!strcmp(pb->type, "localnet")) {
>> > + if (ld->localnet_port &&
!strcmp(ld->localnet_port->logical_port,
>> > + pb->logical_port)) {
>> > + ld->localnet_port = NULL;
>> > + }
>> > + } else if (!strcmp(pb->type, "l3gateway")) {
>> > + const char *chassis_id = smap_get(&pb->options,
>> > + "l3gateway-chassis");
>> > + if (chassis_id && !strcmp(chassis_id, chassis_rec->name)) {
>> > + ld->has_local_l3gateway = false;
>> > + }
>> > + }
>> > +}
>> > +
>> > +/* Considers the ovs iface 'iface_rec' for claiming.
>> > + * This function should be called if the external_ids:iface-id
>> > + * and 'ofport' are set for the 'iface_rec'.
>> > + *
>> > + * If the local_binding for this 'iface_rec' already exists and its
>> > + * already claimed, then this function will be no-op.
>> > + */
>> > +static bool
>> > +consider_iface_claim(const struct ovsrec_interface *iface_rec,
>> > + const char *iface_id,
>> > + struct binding_ctx_in *b_ctx_in,
>> > + struct binding_ctx_out *b_ctx_out,
>> > + struct hmap *qos_map)
>> > +{
>> > + sset_add(b_ctx_out->local_lports, iface_id);
>> > + smap_replace(b_ctx_out->local_iface_ids, iface_rec->name,
iface_id);
>> > +
>> > + struct local_binding *lbinding =
>> > + local_binding_find(b_ctx_out->local_bindings, iface_id);
>> > +
>> > + if (!lbinding) {
>> > + lbinding = local_binding_create(iface_id, iface_rec, NULL,
>> BT_VIF);
>> > + local_binding_add(b_ctx_out->local_bindings, lbinding);
>> > + } else {
>> > + lbinding->iface = iface_rec;
>> > + }
>> > +
>> > + if (!lbinding->pb || strcmp(lbinding->name,
>> lbinding->pb->logical_port)) {
>> > + lbinding->pb = lport_lookup_by_name(
>> > + b_ctx_in->sbrec_port_binding_by_name, lbinding->name);
>> > + if (lbinding->pb && !strcmp(lbinding->pb->type, "virtual")) {
>> > + lbinding->pb = NULL;
>> > + }
>> > + }
>> > +
>> > + if (lbinding->pb) {
>> > + if (!consider_vif_lport(lbinding->pb, b_ctx_in, b_ctx_out,
>> > + lbinding, qos_map)) {
>> > + return false;
>> > + }
>> > + }
>> > +
>> > + /* Update the child local_binding's iface (if any children) and
try
>> to
>> > + * claim the container lbindings. */
>> > + struct shash_node *node;
>> > + SHASH_FOR_EACH (node, &lbinding->children) {
>> > + struct local_binding *child = node->data;
>> > + child->iface = iface_rec;
>> > + if (child->type == BT_CONTAINER) {
>> > + if (!consider_container_lport(child->pb, b_ctx_in,
b_ctx_out,
>> > + qos_map)) {
>> > + return false;
>> > + }
>> > + }
>> > + }
>> > +
>> > + return true;
>> > +}
>> > +
>> > +/* Considers the ovs interface 'iface_rec' for
>> > + * releasing from this chassis if local_binding for this
>> > + * 'iface_rec' (with 'iface_id' as key) already exists and
>> > + * it is claimed by the chassis.
>> > + *
>> > + * The 'iface_id' could be cleared from the 'iface_rec'
>> > + * and hence it is passed separately.
>> > + *
>> > + * This fuction should be called if
>> > + * - OVS interface 'iface_rec' is deleted.
>> > + * - OVS interface 'iface_rec' external_ids:iface-id is updated
>> > + * (with the old value being 'iface_id'.)
>> > + * - OVS interface ofport is reset to 0.
>> > + * */
>> > +static bool
>> > +consider_iface_release(const struct ovsrec_interface *iface_rec,
>> > + const char *iface_id,
>> > + struct binding_ctx_in *b_ctx_in,
>> > + struct binding_ctx_out *b_ctx_out, bool
*changed)
>> > +{
>> > + struct local_binding *lbinding;
>> > + lbinding = local_binding_find(b_ctx_out->local_bindings,
>> > + iface_id);
>> > + if (is_lbinding_this_chassis(lbinding, b_ctx_in->chassis_rec)) {
>> > +
>> > + if (!release_local_binding(b_ctx_in->chassis_rec, lbinding,
>> > + !b_ctx_in->ovnsb_idl_txn)) {
>> > + return false;
>> > + }
>> > +
>> > + struct local_datapath *ld =
>> > + get_local_datapath(b_ctx_out->local_datapaths,
>> > + lbinding->pb->datapath->tunnel_key);
>> > + if (ld) {
>> > + remove_pb_from_local_datapath(lbinding->pb,
>> > + b_ctx_in->chassis_rec,
>> > + b_ctx_out, ld);
>> > + }
>> > +
>> > + /* Check if the lbinding has children of type PB_CONTAINER.
>> > + * If so, don't delete the local_binding. */
>> > + if (!is_lbinding_container_parent(lbinding)) {
>> > + local_binding_delete(b_ctx_out->local_bindings, lbinding);
>> > + }
>> > + *changed = true;
>> > + }
>> > +
>> > + sset_find_and_delete(b_ctx_out->local_lports, iface_id);
>> > + smap_remove(b_ctx_out->local_iface_ids, iface_rec->name);
>> > +
>> > + return true;
>> > +}
>> > +
>> > +static bool
>> > +is_iface_vif(const struct ovsrec_interface *iface_rec)
>> > +{
>> > + if (iface_rec->type && iface_rec->type[0] &&
>> > + strcmp(iface_rec->type, "internal")) {
>> > + return false;
>> > + }
>> > +
>> > + return true;
>> > +}
>> > +
>> > +/* Returns true if the ovs interface changes were handled
successfully,
>> > + * false otherwise.
>> > + */
>> > +bool
>> > +binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in,
>> > + struct binding_ctx_out
*b_ctx_out,
>> > + bool *changed)
>> > +{
>> > + if (!b_ctx_in->chassis_rec) {
>> > + return false;
>> > + }
>> > +
>> > + bool handled = true;
>> > + *changed = false;
>> > +
>> > + /* Run the tracked interfaces loop twice. One to handle deleted
>> > + * changes. And another to handle add/update changes.
>> > + * This will ensure correctness.
>> > + * *
>> > + * We consider an OVS interface for release if one of the
following
>> > + * happens:
>> > + * 1. OVS interface is deleted.
>> > + * 2. external_ids:iface-id is cleared in which case we need to
>> > + * release the port binding corresponding to the previously
set
>> > + * 'old-iface-id' (which is stored in the smap
>> > + * 'b_ctx_out->local_iface_ids').
>> > + * 3. external_ids:iface-id is updated with a different value
>> > + * in which case we need to release the port binding
>> corresponding
>> > + * to the previously set 'old-iface-id' (which is stored in
the
>> smap
>> > + * 'b_ctx_out->local_iface_ids').
>> > + * 4. ofport of the OVS interface is 0.
>> > + *
>> > + */
>> > + const struct ovsrec_interface *iface_rec;
>> > + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec,
>> > + b_ctx_in->iface_table) {
>> > + if (!is_iface_vif(iface_rec)) {
>> > + /* Right now are not handling ovs_interface changes of
>> > + * other types. This can be enhanced to handle of
>> > + * types - patch and tunnel. */
>> > + handled = false;
>> > + break;
>> > + }
>> > +
>> > + const char *iface_id = smap_get(&iface_rec->external_ids,
>> "iface-id");
>> > + const char *old_iface_id =
smap_get(b_ctx_out->local_iface_ids,
>> > + iface_rec->name);
>> > + const char *cleared_iface_id = NULL;
>> > + if (!ovsrec_interface_is_deleted(iface_rec)) {
>> > + int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport
:
>> 0;
>> > + if (iface_id) {
>> > + /* Check if iface_id is changed. If so we need to
>> > + * release the old port binding and associate this
>> > + * inteface to new port binding. */
>> > + if (old_iface_id && strcmp(iface_id, old_iface_id)) {
>> > + cleared_iface_id = old_iface_id;
>> > + } else if (!ofport) {
>> > + /* If ofport is 0, we need to release the iface if
>> already
>> > + * claimed. */
>> > + cleared_iface_id = iface_id;
>> > + }
>> > + } else if (old_iface_id) {
>> > + cleared_iface_id = old_iface_id;
>> > + }
>> > } else {
>> > - lbinding = local_binding_find(b_ctx_out->local_bindings,
>> > - binding_rec->parent_port);
>> > + cleared_iface_id = iface_id;
>> > }
>> >
>> > - if (lbinding) {
>> > - changed = true;
>> > + if (cleared_iface_id) {
>> > + handled = consider_iface_release(iface_rec,
cleared_iface_id,
>> > + b_ctx_in, b_ctx_out,
>> changed);
>> > + }
>> > +
>> > + if (!handled) {
>> > break;
>> > }
>> > }
>> >
>> > - return changed;
>> > + if (!handled) {
>> > + /* This can happen if any non vif OVS interface is in the
tracked
>> > + * list or if consider_iface_release() returned false.
>> > + * There is no need to process further. */
>> > + return false;
>> > + }
>> > +
>> > + struct hmap qos_map = HMAP_INITIALIZER(&qos_map);
>> > + struct hmap *qos_map_ptr =
>> > + sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map;
>> > +
>> > + /*
>> > + * We consider an OVS interface for claiming if the following
>> > + * 2 conditions are met:
>> > + * 1. external_ids:iface-id is set.
>> > + * 2. ofport of the OVS interface is > 0.
>> > + *
>> > + * So when an update of an OVS interface happens we see if these
>> > + * conditions are still true. If so consider this interface for
>> > + * claiming. This would be no-op if the update of the OVS
interface
>> > + * didn't change the above two fields.
>> > + */
>> > + OVSREC_INTERFACE_TABLE_FOR_EACH_TRACKED (iface_rec,
>> > + b_ctx_in->iface_table) {
>> > + /* Loop to handle create and update changes only. */
>> > + if (ovsrec_interface_is_deleted(iface_rec)) {
>> > + continue;
>> > + }
>> > +
>> > + const char *iface_id = smap_get(&iface_rec->external_ids,
>> "iface-id");
>> > + int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0;
>> > + if (iface_id && ofport > 0) {
>> > + *changed = true;
>> > + handled = consider_iface_claim(iface_rec, iface_id,
b_ctx_in,
>> > + b_ctx_out, qos_map_ptr);
>> > + if (!handled) {
>> > + break;
>> > + }
>> > + }
>> > + }
>> > +
>> > + if (handled && qos_map_ptr && set_noop_qos(b_ctx_in->ovs_idl_txn,
>> > + b_ctx_in->port_table,
>> > + b_ctx_in->qos_table,
>> > +
>> b_ctx_out->egress_ifaces)) {
>> > + const char *entry;
>> > + SSET_FOR_EACH (entry, b_ctx_out->egress_ifaces) {
>> > + setup_qos(entry, &qos_map);
>> > + }
>> > + }
>> > +
>> > + destroy_qos_map(&qos_map);
>> > + return handled;
>> > }
>> >
>> > -/* Returns true if the database is all cleaned up, false if more work
is
>> > - * required. */
>> > -bool
>> > -binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
>> > - const struct sbrec_port_binding_table
>> *port_binding_table,
>> > - const struct sbrec_chassis *chassis_rec)
>> > +static void
>> > +handle_deleted_lport(const struct sbrec_port_binding *pb,
>> > + struct binding_ctx_in *b_ctx_in,
>> > + struct binding_ctx_out *b_ctx_out)
>> > {
>> > - if (!ovnsb_idl_txn) {
>> > + struct local_datapath *ld =
>> > + get_local_datapath(b_ctx_out->local_datapaths,
>> > + pb->datapath->tunnel_key);
>> > + if (ld) {
>> > + remove_pb_from_local_datapath(pb, b_ctx_in->chassis_rec,
>> > + b_ctx_out, ld);
>> > + }
>> > +}
>> > +
>> > +static struct local_binding *
>> > +get_lbinding_for_lport(const struct sbrec_port_binding *pb,
>> > + enum en_lport_type lport_type,
>> > + struct binding_ctx_out *b_ctx_out)
>> > +{
>> > + ovs_assert(lport_type == LP_VIF || lport_type == LP_VIRTUAL);
>> > +
>> > + if (lport_type == LP_VIF && !is_lport_container(pb)) {
>> > + return local_binding_find(b_ctx_out->local_bindings,
>> pb->logical_port);
>> > + }
>> > +
>> > + struct local_binding *parent_lbinding = NULL;
>> > +
>> > + if (lport_type == LP_VIRTUAL) {
>> > + parent_lbinding =
local_binding_find(b_ctx_out->local_bindings,
>> > + pb->virtual_parent);
>> > + } else {
>> > + parent_lbinding =
local_binding_find(b_ctx_out->local_bindings,
>> > + pb->parent_port);
>> > + }
>> > +
>> > + return parent_lbinding
>> > + ? local_binding_find(&parent_lbinding->children,
>> pb->logical_port)
>> > + : NULL;
>> > +}
>> > +
>> > +static bool
>> > +handle_deleted_vif_lport(const struct sbrec_port_binding *pb,
>> > + enum en_lport_type lport_type,
>> > + struct binding_ctx_in *b_ctx_in,
>> > + struct binding_ctx_out *b_ctx_out,
>> > + bool *changed)
>> > +{
>> > + struct local_binding *lbinding =
>> > + get_lbinding_for_lport(pb, lport_type, b_ctx_out);
>> > +
>> > + if (lbinding) {
>> > + lbinding->pb = NULL;
>> > + /* The port_binding 'pb' is deleted. So there is no need to
>> > + * clear the 'chassis' column of 'pb'. But we need to do
>> > + * for the local_binding's children. */
>> > + if (lbinding->type == BT_VIF &&
>> > + !release_local_binding_children(b_ctx_in->chassis_rec,
>> > + lbinding,
>> > +
>> !b_ctx_in->ovnsb_idl_txn)) {
>> > + return false;
>> > + }
>> > +
>> > + *changed = true;
>> > + }
>> > +
>> > + handle_deleted_lport(pb, b_ctx_in, b_ctx_out);
>> > + return true;
>> > +}
>> > +
>> > +static bool
>> > +handle_updated_vif_lport(const struct sbrec_port_binding *pb,
>> > + enum en_lport_type lport_type,
>> > + struct binding_ctx_in *b_ctx_in,
>> > + struct binding_ctx_out *b_ctx_out,
>> > + struct hmap *qos_map,
>> > + bool *changed)
>> > +{
>> > + bool claimed = (pb->chassis == b_ctx_in->chassis_rec);
>> > + bool handled = true;
>> > +
>> > + if (lport_type == LP_VIRTUAL) {
>> > + handled = consider_virtual_lport(pb, b_ctx_in, b_ctx_out,
>> qos_map);
>> > + } else if (lport_type == LP_VIF && is_lport_container(pb)) {
>> > + handled = consider_container_lport(pb, b_ctx_in, b_ctx_out,
>> qos_map);
>> > + } else {
>> > + handled = consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL,
>> qos_map);
>> > + }
>> > +
>> > + if (!handled) {
>> > return false;
>> > }
>> > - if (!chassis_rec) {
>> > +
>> > + bool now_claimed = (pb->chassis == b_ctx_in->chassis_rec);
>> > + bool changed_ = (claimed != now_claimed);
>> > +
>> > + if (changed_) {
>> > + *changed = true;
>> > + }
>> > +
>> > + if (lport_type == LP_VIRTUAL ||
>> > + (lport_type == LP_VIF && is_lport_container(pb)) ||
!changed_) {
>> > return true;
>> > }
>> >
>> > - const struct sbrec_port_binding *binding_rec;
>> > - bool any_changes = false;
>> > - SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec,
port_binding_table) {
>> > - if (binding_rec->chassis == chassis_rec) {
>> > - if (binding_rec->encap)
>> > - sbrec_port_binding_set_encap(binding_rec, NULL);
>> > - sbrec_port_binding_set_chassis(binding_rec, NULL);
>> > - any_changes = true;
>> > + struct local_binding *lbinding =
>> > + local_binding_find(b_ctx_out->local_bindings,
pb->logical_port);
>> > +
>> > + ovs_assert(lbinding);
>> > +
>> > + struct shash_node *node;
>> > + SHASH_FOR_EACH (node, &lbinding->children) {
>> > + struct local_binding *child = node->data;
>> > + if (child->type == BT_CONTAINER) {
>> > + handled = consider_container_lport(child->pb, b_ctx_in,
>> b_ctx_out,
>> > + qos_map);
>> > + if (!handled) {
>> > + return false;
>> > + }
>> > }
>> > }
>> >
>> > - if (any_changes) {
>> > - ovsdb_idl_txn_add_comment(
>> > - ovnsb_idl_txn,
>> > - "ovn-controller: removing all port bindings for '%s'",
>> > - chassis_rec->name);
>> > + return true;
>> > +}
>> > +
>> > +/* Returns true if the port binding changes resulted in local binding
>> > + * updates, false otherwise.
>> > + */
>> > +bool
>> > +binding_handle_port_binding_changes(struct binding_ctx_in *b_ctx_in,
>> > + struct binding_ctx_out *b_ctx_out,
>> > + bool *changed)
>> > +{
>> > + bool handled = true;
>> > + *changed = false;
>> > +
>> > + const struct sbrec_port_binding *pb;
>> > +
>> > + /* Run the tracked port binding loop twice. One to handle deleted
>> > + * changes. And another to handle add/update changes.
>> > + * This will ensure correctness.
>> > + */
>> > + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb,
>> > +
>> b_ctx_in->port_binding_table) {
>> > + if (!sbrec_port_binding_is_deleted(pb)) {
>> > + continue;
>> > + }
>> > +
>> > + enum en_lport_type lport_type = get_lport_type(pb);
>> > + if (lport_type == LP_VIF || lport_type == LP_VIRTUAL) {
>> > + handled = handle_deleted_vif_lport(pb, lport_type,
b_ctx_in,
>> > + b_ctx_out, changed);
>> > + } else {
>> > + handle_deleted_lport(pb, b_ctx_in, b_ctx_out);
>> > + }
>> > +
>> > + if (!handled) {
>> > + break;
>> > + }
>> > }
>> >
>> > - return !any_changes;
>> > + if (!handled) {
>> > + return false;
>> > + }
>> > +
>> > + struct hmap qos_map = HMAP_INITIALIZER(&qos_map);
>> > + struct hmap *qos_map_ptr =
>> > + sset_is_empty(b_ctx_out->egress_ifaces) ? NULL : &qos_map;
>> > +
>> > + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb,
>> > +
>> b_ctx_in->port_binding_table) {
>> > + /* Loop to handle create and update changes only. */
>> > + if (sbrec_port_binding_is_deleted(pb)) {
>> > + continue;
>> > + }
>> > +
>> > + enum en_lport_type lport_type = get_lport_type(pb);
>> > +
>> > + struct local_datapath *ld =
>> > + get_local_datapath(b_ctx_out->local_datapaths,
>> > + pb->datapath->tunnel_key);
>> > +
>> > + switch (lport_type) {
>> > + case LP_VIF:
>> > + case LP_VIRTUAL:
>> > + handled = handle_updated_vif_lport(pb, lport_type,
b_ctx_in,
>> > + b_ctx_out, qos_map_ptr,
>> > + changed);
>> > + break;
>> > +
>> > + case LP_PATCH:
>> > + case LP_LOCALPORT:
>> > + case LP_VTEP:
>> > + *changed = true;
>> > + update_local_lport_ids(b_ctx_out->local_lport_ids, pb);
>> > + if (lport_type == LP_PATCH) {
>> > + /* Add the peer datapath to the local datapaths if
it's
>> > + * not present yet.
>> > + */
>> > + if (ld) {
>> > + add_local_datapath_peer_port(pb, b_ctx_in,
>> b_ctx_out, ld);
>> > + }
>> > + }
>> > + break;
>> > +
>> > + case LP_L2GATEWAY:
>> > + *changed = true;
>> > + handled = consider_l2gw_lport(pb, b_ctx_in, b_ctx_out);
>> > + break;
>> > +
>> > + case LP_L3GATEWAY:
>> > + *changed = true;
>> > + handled = consider_l3gw_lport(pb, b_ctx_in, b_ctx_out);
>> > + break;
>> > +
>> > + case LP_CHASSISREDIRECT:
>> > + *changed = true;
>> > + handled = consider_cr_lport(pb, b_ctx_in, b_ctx_out);
>> > + break;
>> > +
>> > + case LP_EXTERNAL:
>> > + *changed = true;
>> > + handled = consider_external_lport(pb, b_ctx_in,
b_ctx_out);
>> > + break;
>> > +
>> > + case LP_LOCALNET: {
>> > + *changed = true;
>> > + consider_localnet_lport(pb, b_ctx_in, b_ctx_out,
>> qos_map_ptr);
>> > +
>> > + struct shash bridge_mappings =
>> > + SHASH_INITIALIZER(&bridge_mappings);
>> > + add_ovs_bridge_mappings(b_ctx_in->ovs_table,
>> > + b_ctx_in->bridge_table,
>> > + &bridge_mappings);
>> > + update_ld_localnet_port(pb, &bridge_mappings,
>> > + b_ctx_out->egress_ifaces,
>> > + b_ctx_out->local_datapaths);
>> > + shash_destroy(&bridge_mappings);
>> > + break;
>> > + }
>> > +
>> > + case LP_REMOTE:
>> > + case LP_UNKNOWN:
>> > + break;
>> > + }
>> > +
>> > + if (!handled) {
>> > + break;
>> > + }
>> > + }
>> > +
>> > + if (handled && qos_map_ptr && set_noop_qos(b_ctx_in->ovs_idl_txn,
>> > + b_ctx_in->port_table,
>> > + b_ctx_in->qos_table,
>> > +
>> b_ctx_out->egress_ifaces)) {
>> > + const char *entry;
>> > + SSET_FOR_EACH (entry, b_ctx_out->egress_ifaces) {
>> > + setup_qos(entry, &qos_map);
>> > + }
>> > + }
>> > +
>> > + destroy_qos_map(&qos_map);
>> > + return handled;
>> > }
>> > diff --git a/controller/binding.h b/controller/binding.h
>> > index 9affc9a96..f7ace6faf 100644
>> > --- a/controller/binding.h
>> > +++ b/controller/binding.h
>> > @@ -57,6 +57,7 @@ struct binding_ctx_out {
>> > struct sset *local_lports;
>> > struct sset *local_lport_ids;
>> > struct sset *egress_ifaces;
>> > + struct smap *local_iface_ids;
>> > };
>> >
>> > void binding_register_ovs_idl(struct ovsdb_idl *);
>> > @@ -64,9 +65,13 @@ void binding_run(struct binding_ctx_in *, struct
>> binding_ctx_out *);
>> > bool binding_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn,
>> > const struct sbrec_port_binding_table *,
>> > const struct sbrec_chassis *);
>> > -bool binding_evaluate_port_binding_changes(struct binding_ctx_in *,
>> > - struct binding_ctx_out *);
>> >
>> > void local_bindings_init(struct shash *local_bindings);
>> > void local_bindings_destroy(struct shash *local_bindings);
>> > +bool binding_handle_ovs_interface_changes(struct binding_ctx_in *,
>> > + struct binding_ctx_out *,
>> > + bool *changed);
>> > +bool binding_handle_port_binding_changes(struct binding_ctx_in *,
>> > + struct binding_ctx_out *,
>> > + bool *changed);
>> > #endif /* controller/binding.h */
>> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
>> > index 386a64059..9ce9684e9 100644
>> > --- a/controller/ovn-controller.c
>> > +++ b/controller/ovn-controller.c
>> > @@ -753,6 +753,7 @@ enum sb_engine_node {
>> > OVS_NODE(open_vswitch, "open_vswitch") \
>> > OVS_NODE(bridge, "bridge") \
>> > OVS_NODE(port, "port") \
>> > + OVS_NODE(interface, "interface") \
>> > OVS_NODE(qos, "qos")
>> >
>> > enum ovs_engine_node {
>> > @@ -974,6 +975,7 @@ struct ed_type_runtime_data {
>> > struct sset active_tunnels;
>> >
>> > struct sset egress_ifaces;
>> > + struct smap local_iface_ids;
>> > };
>> >
>> > static void *
>> > @@ -987,6 +989,7 @@ en_runtime_data_init(struct engine_node *node
>> OVS_UNUSED,
>> > sset_init(&data->local_lport_ids);
>> > sset_init(&data->active_tunnels);
>> > sset_init(&data->egress_ifaces);
>> > + smap_init(&data->local_iface_ids);
>> > local_bindings_init(&data->local_bindings);
>> > return data;
>> > }
>> > @@ -1000,6 +1003,7 @@ en_runtime_data_cleanup(void *data)
>> > sset_destroy(&rt_data->local_lport_ids);
>> > sset_destroy(&rt_data->active_tunnels);
>> > sset_destroy(&rt_data->egress_ifaces);
>> > + smap_destroy(&rt_data->local_iface_ids);
>> > struct local_datapath *cur_node, *next_node;
>> > HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node,
>> > &rt_data->local_datapaths) {
>> > @@ -1041,6 +1045,10 @@ init_binding_ctx(struct engine_node *node,
>> > (struct ovsrec_port_table *)EN_OVSDB_GET(
>> > engine_get_input("OVS_port", node));
>> >
>> > + struct ovsrec_interface_table *iface_table =
>> > + (struct ovsrec_interface_table *)EN_OVSDB_GET(
>> > + engine_get_input("OVS_interface", node));
>> > +
>> > struct ovsrec_qos_table *qos_table =
>> > (struct ovsrec_qos_table *)EN_OVSDB_GET(
>> > engine_get_input("OVS_qos", node));
>> > @@ -1070,6 +1078,7 @@ init_binding_ctx(struct engine_node *node,
>> > b_ctx_in->sbrec_port_binding_by_datapath =
>> sbrec_port_binding_by_datapath;
>> > b_ctx_in->sbrec_port_binding_by_name = sbrec_port_binding_by_name;
>> > b_ctx_in->port_table = port_table;
>> > + b_ctx_in->iface_table = iface_table;
>> > b_ctx_in->qos_table = qos_table;
>> > b_ctx_in->port_binding_table = pb_table;
>> > b_ctx_in->br_int = br_int;
>> > @@ -1083,6 +1092,7 @@ init_binding_ctx(struct engine_node *node,
>> > b_ctx_out->local_lport_ids = &rt_data->local_lport_ids;
>> > b_ctx_out->egress_ifaces = &rt_data->egress_ifaces;
>> > b_ctx_out->local_bindings = &rt_data->local_bindings;
>> > + b_ctx_out->local_iface_ids = &rt_data->local_iface_ids;
>> > }
>> >
>> > static void
>> > @@ -1111,10 +1121,12 @@ en_runtime_data_run(struct engine_node *node,
>> void *data)
>> > sset_destroy(local_lport_ids);
>> > sset_destroy(active_tunnels);
>> > sset_destroy(&rt_data->egress_ifaces);
>> > + smap_destroy(&rt_data->local_iface_ids);
>> > sset_init(local_lports);
>> > sset_init(local_lport_ids);
>> > sset_init(active_tunnels);
>> > sset_init(&rt_data->egress_ifaces);
>> > + smap_init(&rt_data->local_iface_ids);
>> > local_bindings_init(&rt_data->local_bindings);
>> > }
>> >
>> > @@ -1140,6 +1152,34 @@ en_runtime_data_run(struct engine_node *node,
void
>> *data)
>> > engine_set_node_state(node, EN_UPDATED);
>> > }
>> >
>> > +static bool
>> > +runtime_data_ovs_interface_handler(struct engine_node *node, void
*data)
>> > +{
>> > + struct ed_type_runtime_data *rt_data = data;
>> > + struct binding_ctx_in b_ctx_in;
>> > + struct binding_ctx_out b_ctx_out;
>> > + init_binding_ctx(node, rt_data, &b_ctx_in, &b_ctx_out);
>> > +
>> > + bool changed = false;
>> > + if (!binding_handle_ovs_interface_changes(&b_ctx_in, &b_ctx_out,
>> > + &changed)) {
>> > + return false;
>> > + }
>> > +
>> > + if (changed) {
>> > + engine_set_node_state(node, EN_UPDATED);
>> > + }
>> > +
>> > + return true;
>> > +}
>> > +
>> > +static bool
>> > +runtime_data_noop_handler(struct engine_node *node OVS_UNUSED,
>> > + void *data OVS_UNUSED)
>> > +{
>> > + return true;
>> > +}
>> > +
>> > static bool
>> > runtime_data_sb_port_binding_handler(struct engine_node *node, void
>> *data)
>> > {
>> > @@ -1147,11 +1187,21 @@ runtime_data_sb_port_binding_handler(struct
>> engine_node *node, void *data)
>> > struct binding_ctx_in b_ctx_in;
>> > struct binding_ctx_out b_ctx_out;
>> > init_binding_ctx(node, rt_data, &b_ctx_in, &b_ctx_out);
>> > + if (!b_ctx_in.chassis_rec) {
>> > + return false;
>> > + }
>> > +
>> > + bool changed = false;
>> > + if (!binding_handle_port_binding_changes(&b_ctx_in, &b_ctx_out,
>> > + &changed)) {
>> > + return false;
>> > + }
>> >
>> > - bool changed = binding_evaluate_port_binding_changes(&b_ctx_in,
>> > - &b_ctx_out);
>> > + if (changed) {
>> > + engine_set_node_state(node, EN_UPDATED);
>> > + }
>> >
>> > - return !changed;
>> > + return true;
>> > }
>> >
>> > /* Connection tracking zones. */
>> > @@ -1895,7 +1945,10 @@ main(int argc, char *argv[])
>> >
>> > engine_add_input(&en_runtime_data, &en_ovs_open_vswitch, NULL);
>> > engine_add_input(&en_runtime_data, &en_ovs_bridge, NULL);
>> > - engine_add_input(&en_runtime_data, &en_ovs_port, NULL);
>> > + engine_add_input(&en_runtime_data, &en_ovs_port,
>> > + runtime_data_noop_handler);
>> > + engine_add_input(&en_runtime_data, &en_ovs_interface,
>> > + runtime_data_ovs_interface_handler);
>> > engine_add_input(&en_runtime_data, &en_ovs_qos, NULL);
>> >
>> > engine_add_input(&en_runtime_data, &en_sb_chassis, NULL);
>> > diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at
>> > index a8a15f8fe..5dfc6f7ca 100644
>> > --- a/tests/ovn-performance.at
>> > +++ b/tests/ovn-performance.at
>> > @@ -239,6 +239,19 @@ for i in 1 2; do
>> > ovn_attach n1 br-phys 192.168.0.$i
>> > done
>> >
>> > +# Wait for the tunnel ports to be created and up.
>> > +# Otherwise this may affect the lflow_run count.
>> > +
>> > +OVS_WAIT_UNTIL([
>> > + test $(as hv1 ovs-vsctl list interface ovn-hv2-0 | \
>> > +grep tunnel_egress_iface_carrier=up | wc -l) -eq 1
>> > +])
>> > +
>> > +OVS_WAIT_UNTIL([
>> > + test $(as hv2 ovs-vsctl list interface ovn-hv1-0 | \
>> > +grep tunnel_egress_iface_carrier=up | wc -l) -eq 1
>> > +])
>> > +
>> > # Add router lr1
>> > OVN_CONTROLLER_EXPECT_HIT(
>> > [hv1 hv2], [lflow_run],
>> > --
>> > 2.26.2
>> >
>> > _______________________________________________
>> > dev mailing list
>> > dev at openvswitch.org
>> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
More information about the dev
mailing list