[ovs-dev] [PATCH v4 02/10] ovn-controller: Store the local port bindings in the runtime data I-P state.
Numan Siddique
numans at ovn.org
Mon May 11 09:50:51 UTC 2020
On Thu, May 7, 2020 at 12:54 PM Numan Siddique <numans at ovn.org> wrote:
>
>
> On Thu, May 7, 2020 at 12:27 PM Han Zhou <hzhou at ovn.org> wrote:
>
>> Hi Numan,
>>
>> Please see my comments below.
>>
>> On Thu, Apr 30, 2020 at 9:59 AM <numans at ovn.org> wrote:
>> >
>> > From: Numan Siddique <numans at ovn.org>
>> >
>> > This patch adds a new data structure - 'struct local_binding' which
>> represents
>> > a probable local port binding. A new object of this structure is creared
>> for
>> > each interface present in the integration bridge (br-int) with the
>> > external_ids:iface-id set. This struct has 2 main fields
>> > - 'struct ovsrec_interface *' and 'struct sbrec_port_binding *'. These
>> fields
>> > are updated during port claim and release.
>> >
>> > A shash of the local bindings is maintained with the 'iface-id' as the
>> hash key
>> > in the runtime data of the incremental processing engine.
>> >
>> > This patch helps in the upcoming patch to add incremental processing
>> support
>> > for OVS interface and SB port binding changes.
>> >
>> > Signed-off-by: Numan Siddique <numans at ovn.org>
>> > ---
>> > controller/binding.c | 722 ++++++++++++++++++++++--------------
>> > controller/binding.h | 16 +-
>> > controller/ovn-controller.c | 46 +--
>> > controller/pinctrl.c | 1 +
>> > controller/pinctrl.h | 1 +
>> > 5 files changed, 467 insertions(+), 319 deletions(-)
>> >
>> > diff --git a/controller/binding.c b/controller/binding.c
>> > index 007a94866..66f4f65e3 100644
>> > --- a/controller/binding.c
>> > +++ b/controller/binding.c
>> > @@ -69,47 +69,6 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>> > ovsdb_idl_add_column(ovs_idl, &ovsrec_qos_col_type);
>> > }
>> >
>> > -static void
>> > -get_local_iface_ids(const struct ovsrec_bridge *br_int,
>> > - struct shash *lport_to_iface,
>> > - struct sset *local_lports,
>> > - struct sset *egress_ifaces)
>> > -{
>> > - int i;
>> > -
>> > - for (i = 0; i < br_int->n_ports; i++) {
>> > - const struct ovsrec_port *port_rec = br_int->ports[i];
>> > - const char *iface_id;
>> > - int j;
>> > -
>> > - if (!strcmp(port_rec->name, br_int->name)) {
>> > - continue;
>> > - }
>> > -
>> > - for (j = 0; j < port_rec->n_interfaces; j++) {
>> > - const struct ovsrec_interface *iface_rec;
>> > -
>> > - iface_rec = port_rec->interfaces[j];
>> > - 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) {
>> > - shash_add(lport_to_iface, iface_id, iface_rec);
>> > - sset_add(local_lports, iface_id);
>> > - }
>> > -
>> > - /* Check if this is a tunnel interface. */
>> > - if (smap_get(&iface_rec->options, "remote_ip")) {
>> > - const char *tunnel_iface
>> > - = smap_get(&iface_rec->status,
>> "tunnel_egress_iface");
>> > - if (tunnel_iface) {
>> > - sset_add(egress_ifaces, tunnel_iface);
>> > - }
>> > - }
>> > - }
>> > - }
>> > -}
>> > -
>> > static void
>> > add_local_datapath__(struct ovsdb_idl_index
>> *sbrec_datapath_binding_by_key,
>> > struct ovsdb_idl_index
>> *sbrec_port_binding_by_datapath,
>> > @@ -441,25 +400,11 @@ static bool
>> > is_our_chassis(const struct sbrec_chassis *chassis_rec,
>> > const struct sbrec_port_binding *binding_rec,
>> > const struct sset *active_tunnels,
>> > - const struct ovsrec_interface *iface_rec,
>> > const struct sset *local_lports)
>> > {
>> > - /* Ports of type "virtual" should never be explicitly bound to an
>> OVS
>> > - * port in the integration bridge. If that's the case, ignore the
>> binding
>> > - * and log a warning.
>> > - */
>> > - if (iface_rec && !strcmp(binding_rec->type, "virtual")) {
>> > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>> > - VLOG_WARN_RL(&rl,
>> > - "Virtual port %s should not be bound to OVS port
>> %s",
>> > - binding_rec->logical_port, iface_rec->name);
>> > - return false;
>> > - }
>> > -
>> > bool our_chassis = false;
>> > - if (iface_rec
>> > - || (binding_rec->parent_port && binding_rec->parent_port[0] &&
>> > - sset_contains(local_lports, binding_rec->parent_port))) {
>> > + if (binding_rec->parent_port && binding_rec->parent_port[0] &&
>> > + sset_contains(local_lports, binding_rec->parent_port)) {
>> > /* This port is in our chassis unless it is a localport. */
>> > our_chassis = strcmp(binding_rec->type, "localport");
>> > } else if (!strcmp(binding_rec->type, "l2gateway")) {
>> > @@ -481,175 +426,6 @@ is_our_chassis(const struct sbrec_chassis
>> *chassis_rec,
>> > return our_chassis;
>> > }
>> >
>> > -/* Updates 'binding_rec' and if the port binding is local also updates
>> the
>> > - * local datapaths and ports.
>> > - * Updates and returns the array of local virtual ports that will
>> require
>> > - * additional processing.
>> > - */
>> > -static const struct sbrec_port_binding **
>> > -consider_local_datapath(const struct sbrec_port_binding *binding_rec,
>> > - struct hmap *qos_map,
>> > - const struct ovsrec_interface *iface_rec,
>> > - struct binding_ctx_in *b_ctx_in,
>> > - struct binding_ctx_out *b_ctx_out,
>> > - const struct sbrec_port_binding **vpbs,
>> > - size_t *n_vpbs, size_t *n_allocated_vpbs)
>> > -{
>> > - bool our_chassis = is_our_chassis(b_ctx_in->chassis_rec,
>> binding_rec,
>> > - b_ctx_in->active_tunnels,
>> iface_rec,
>> > - b_ctx_out->local_lports);
>> > - if (iface_rec
>> > - || (binding_rec->parent_port && binding_rec->parent_port[0] &&
>> > - sset_contains(b_ctx_out->local_lports,
>> > - binding_rec->parent_port))) {
>> > - if (binding_rec->parent_port && binding_rec->parent_port[0]) {
>> > - /* Add child logical port to the set of all local ports. */
>> > - sset_add(b_ctx_out->local_lports,
>> binding_rec->logical_port);
>> > - }
>> > - 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,
>> > - binding_rec->datapath, false,
>> > - b_ctx_out->local_datapaths);
>> > - if (iface_rec && qos_map && b_ctx_in->ovs_idl_txn) {
>> > - get_qos_params(binding_rec, qos_map);
>> > - }
>> > - } else if (!strcmp(binding_rec->type, "l2gateway")) {
>> > - if (our_chassis) {
>> > - sset_add(b_ctx_out->local_lports,
>> binding_rec->logical_port);
>> > - 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,
>> > - binding_rec->datapath, false,
>> > - b_ctx_out->local_datapaths);
>> > - }
>> > - } else if (!strcmp(binding_rec->type, "chassisredirect")) {
>> > - if (ha_chassis_group_contains(binding_rec->ha_chassis_group,
>> > - b_ctx_in->chassis_rec)) {
>> > - 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,
>> > - binding_rec->datapath, false,
>> > - b_ctx_out->local_datapaths);
>> > - }
>> > - } else if (!strcmp(binding_rec->type, "l3gateway")) {
>> > - if (our_chassis) {
>> > - 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,
>> > - binding_rec->datapath, true,
>> > - b_ctx_out->local_datapaths);
>> > - }
>> > - } else if (!strcmp(binding_rec->type, "localnet")) {
>> > - /* Add all localnet ports to local_lports so that we allocate
>> ct
>> zones
>> > - * for them. */
>> > - sset_add(b_ctx_out->local_lports, binding_rec->logical_port);
>> > - if (qos_map && b_ctx_in->ovs_idl_txn) {
>> > - get_qos_params(binding_rec, qos_map);
>> > - }
>> > - } else if (!strcmp(binding_rec->type, "external")) {
>> > - if (ha_chassis_group_contains(binding_rec->ha_chassis_group,
>> > - b_ctx_in->chassis_rec)) {
>> > - 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,
>> > - binding_rec->datapath, false,
>> > - b_ctx_out->local_datapaths);
>> > - }
>> > - }
>> > -
>> > - if (our_chassis
>> > - || !strcmp(binding_rec->type, "patch")
>> > - || !strcmp(binding_rec->type, "localport")
>> > - || !strcmp(binding_rec->type, "vtep")
>> > - || !strcmp(binding_rec->type, "localnet")) {
>> > - update_local_lport_ids(b_ctx_out->local_lport_ids,
>> binding_rec);
>> > - }
>> > -
>> > - ovs_assert(b_ctx_in->ovnsb_idl_txn);
>> > - const char *vif_chassis = smap_get(&binding_rec->options,
>> > - "requested-chassis");
>> > - bool can_bind = !vif_chassis || !vif_chassis[0]
>> > - || !strcmp(vif_chassis,
>> b_ctx_in->chassis_rec->name)
>> > - || !strcmp(vif_chassis,
>> b_ctx_in->chassis_rec->hostname);
>> > -
>> > - if (can_bind && our_chassis) {
>> > - if (binding_rec->chassis != b_ctx_in->chassis_rec) {
>> > - if (binding_rec->chassis) {
>> > - VLOG_INFO("Changing chassis for lport %s from %s to
>> %s.",
>> > - binding_rec->logical_port,
>> > - binding_rec->chassis->name,
>> > - b_ctx_in->chassis_rec->name);
>> > - } else {
>> > - VLOG_INFO("Claiming lport %s for this chassis.",
>> > - binding_rec->logical_port);
>> > - }
>> > - for (int i = 0; i < binding_rec->n_mac; i++) {
>> > - VLOG_INFO("%s: Claiming %s",
>> > - binding_rec->logical_port,
>> binding_rec->mac[i]);
>> > - }
>> > - sbrec_port_binding_set_chassis(binding_rec,
>> b_ctx_in->chassis_rec);
>> > - }
>> > - /* Check if the port encap binding, if any, has changed */
>> > - struct sbrec_encap *encap_rec =
>> > - sbrec_get_port_encap(b_ctx_in->chassis_rec, iface_rec);
>> > - if (encap_rec && binding_rec->encap != encap_rec) {
>> > - sbrec_port_binding_set_encap(binding_rec, encap_rec);
>> > - }
>> > - } else if (binding_rec->chassis == b_ctx_in->chassis_rec) {
>> > - if (!strcmp(binding_rec->type, "virtual")) {
>> > - if (*n_vpbs == *n_allocated_vpbs) {
>> > - vpbs = x2nrealloc(vpbs, n_allocated_vpbs, sizeof
>> *vpbs);
>> > - }
>> > - vpbs[(*n_vpbs)] = binding_rec;
>> > - (*n_vpbs)++;
>> > - } else {
>> > - VLOG_INFO("Releasing lport %s from this chassis.",
>> > - binding_rec->logical_port);
>> > - if (binding_rec->encap) {
>> > - sbrec_port_binding_set_encap(binding_rec, NULL);
>> > - }
>> > - sbrec_port_binding_set_chassis(binding_rec, NULL);
>> > - }
>> > - } else if (our_chassis) {
>> > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>> > - VLOG_INFO_RL(&rl,
>> > - "Not claiming lport %s, chassis %s "
>> > - "requested-chassis %s",
>> > - binding_rec->logical_port,
>> b_ctx_in->chassis_rec->name,
>> > - vif_chassis);
>> > - }
>> > -
>> > - return vpbs;
>> > -}
>> > -
>> > -static void
>> > -consider_local_virtual_port(struct ovsdb_idl_index
>> *sbrec_port_binding_by_name,
>> > - const struct sbrec_chassis *chassis_rec,
>> > - const struct sbrec_port_binding
>> *binding_rec)
>> > -{
>> > - if (binding_rec->virtual_parent) {
>> > - const struct sbrec_port_binding *parent =
>> > - lport_lookup_by_name(sbrec_port_binding_by_name,
>> > - binding_rec->virtual_parent);
>> > - if (parent && parent->chassis == chassis_rec) {
>> > - return;
>> > - }
>> > - }
>> > -
>> > - /* pinctrl module takes care of binding the ports of type
>> 'virtual'.
>> > - * Release such ports if their virtual parents are no longer
>> claimed
>> by
>> > - * this chassis.
>> > - */
>> > - VLOG_INFO("Releasing lport %s from this chassis.",
>> > - binding_rec->logical_port);
>> > - if (binding_rec->encap) {
>> > - sbrec_port_binding_set_encap(binding_rec, NULL);
>> > - }
>> > - sbrec_port_binding_set_chassis(binding_rec, NULL);
>> > - sbrec_port_binding_set_virtual_parent(binding_rec, NULL);
>> > -}
>> > -
>> > static void
>> > add_localnet_egress_interface_mappings(
>> > const struct sbrec_port_binding *port_binding,
>> > @@ -712,6 +488,374 @@ consider_localnet_port(const struct
>> sbrec_port_binding *binding_rec,
>> > ld->localnet_port = binding_rec;
>> > }
>> >
>> > +enum local_binding_type {
>> > + BT_VIF,
>> > + BT_CHILD,
>> > + BT_VIRTUAL
>> > +};
>> > +
>> > +struct local_binding {
>> > + struct ovs_list node; /* In parent if any. */
>> > + char *name;
>> > + enum local_binding_type type;
>> > + const struct ovsrec_interface *iface;
>> > + const struct sbrec_port_binding *pb;
>> > +
>> > + struct ovs_list children;
>> > +};
>> > +
>> > +static struct local_binding *
>> > +local_binding_create(const char *name, const struct ovsrec_interface
>> *iface,
>> > + const struct sbrec_port_binding *pb,
>> > + enum local_binding_type type)
>> > +{
>> > + struct local_binding *lbinding = xzalloc(sizeof *lbinding);
>> > + lbinding->name = xstrdup(name);
>> > + lbinding->type = type;
>> > + lbinding->pb = pb;
>> > + lbinding->iface = iface;
>> > + ovs_list_init(&lbinding->children);
>> > + return lbinding;
>> > +}
>> > +
>> > +static void
>> > +local_binding_add(struct shash *local_bindings, struct local_binding
>> *lbinding)
>> > +{
>> > + shash_add(local_bindings, lbinding->name, lbinding);
>> > +}
>> > +
>> > +static struct local_binding *
>> > +local_binding_find(struct shash *local_bindings, const char *name)
>> > +{
>> > + return shash_find_data(local_bindings, name);
>> > +}
>> > +
>> > +static void
>> > +local_binding_destroy(struct local_binding *lbinding)
>> > +{
>> > + struct local_binding *c, *next;
>> > + LIST_FOR_EACH_SAFE (c, next, node, &lbinding->children) {
>> > + ovs_list_remove(&c->node);
>> > + free(c->name);
>> > + free(c);
>> > + }
>> > + free(lbinding->name);
>> > + free(lbinding);
>> > +}
>> > +
>> > +void
>> > +local_bindings_destroy(struct shash *local_bindings)
>> > +{
>> > + struct shash_node *node, *next;
>> > + SHASH_FOR_EACH_SAFE (node, next, local_bindings) {
>> > + struct local_binding *lbinding = node->data;
>> > + struct local_binding *c, *n;
>> > + LIST_FOR_EACH_SAFE (c, n, node, &lbinding->children) {
>> > + ovs_list_remove(&c->node);
>> > + free(c->name);
>> > + free(c);
>> > + }
>> > + }
>> > +
>> > + shash_destroy(local_bindings);
>> > +}
>> > +
>> > +static void
>> > +local_binding_add_child(struct local_binding *lbinding,
>> > + struct local_binding *child)
>> > +{
>> > + struct local_binding *l;
>> > + LIST_FOR_EACH (l, node, &lbinding->children) {
>> > + if (l == child) {
>> > + return;
>> > + }
>> > + }
>> > +
>> > + ovs_list_push_back(&lbinding->children, &child->node);
>> > +}
>> > +
>> > +static struct local_binding *
>> > +local_binding_find_child(struct local_binding *lbinding,
>> > + const char *child_name)
>> > +{
>> > + struct local_binding *l;
>> > + LIST_FOR_EACH (l, node, &lbinding->children) {
>> > + if (!strcmp(l->name, child_name)) {
>> > + return l;
>> > + }
>> > + }
>> > +
>> > + return NULL;
>> > +}
>> > +
>> > +void
>> > +binding_add_vport_to_local_bindings(struct shash *local_bindings,
>> > + const struct sbrec_port_binding
>> *parent,
>> > + const struct sbrec_port_binding
>> *vport)
>> > +{
>> > + struct local_binding *lbinding = local_binding_find(local_bindings,
>> > +
>> parent->logical_port);
>> > + ovs_assert(lbinding);
>> > + struct local_binding *vbinding =
>> > + local_binding_find_child(lbinding, vport->logical_port);
>> > + if (!vbinding) {
>> > + vbinding = local_binding_create(vport->logical_port,
>> lbinding->iface,
>> > + vport, BT_VIRTUAL);
>> > + local_binding_add_child(lbinding, vbinding);
>> > + } else {
>> > + vbinding->type = BT_VIRTUAL;
>> > + }
>> > +}
>> > +
>> > +static void
>> > +claim_lport(const struct sbrec_port_binding *pb,
>> > + const struct sbrec_chassis *chassis_rec,
>> > + const struct ovsrec_interface *iface_rec)
>> > +{
>> > + if (pb->chassis != chassis_rec) {
>> > + if (pb->chassis) {
>> > + VLOG_INFO("Changing chassis for lport %s from %s to %s.",
>> > + pb->logical_port, pb->chassis->name,
>> > + chassis_rec->name);
>> > + } else {
>> > + VLOG_INFO("Claiming lport %s for this chassis.",
>> pb->logical_port);
>> > + }
>> > + for (int i = 0; i < pb->n_mac; i++) {
>> > + VLOG_INFO("%s: Claiming %s", pb->logical_port, pb->mac[i]);
>> > + }
>> > +
>> > + sbrec_port_binding_set_chassis(pb, chassis_rec);
>> > + }
>> > +
>> > + /* Check if the port encap binding, if any, has changed */
>> > + struct sbrec_encap *encap_rec =
>> > + sbrec_get_port_encap(chassis_rec, iface_rec);
>> > + if (encap_rec && pb->encap != encap_rec) {
>> > + sbrec_port_binding_set_encap(pb, encap_rec);
>> > + }
>> > +}
>> > +
>> > +static void
>> > +release_lport(const struct sbrec_port_binding *pb)
>> > +{
>> > + VLOG_INFO("Releasing lport %s from this chassis.",
>> pb->logical_port);
>> > + if (pb->encap) {
>> > + sbrec_port_binding_set_encap(pb, NULL);
>> > + }
>> > + sbrec_port_binding_set_chassis(pb, NULL);
>> > +
>> > + if (pb->virtual_parent) {
>> > + sbrec_port_binding_set_virtual_parent(pb, NULL);
>> > + }
>> > +}
>> > +
>> > +static void
>> > +consider_port_binding_for_vif(const struct sbrec_port_binding *pb,
>> > + struct binding_ctx_in *b_ctx_in,
>> > + enum local_binding_type binding_type,
>> > + struct local_binding *lbinding,
>> > + struct binding_ctx_out *b_ctx_out,
>> > + struct hmap *qos_map)
>> > +{
>> > + const char *vif_chassis = smap_get(&pb->options,
>> "requested-chassis");
>> > + bool can_bind = !vif_chassis || !vif_chassis[0]
>> > + || !strcmp(vif_chassis,
>> b_ctx_in->chassis_rec->name)
>> > + || !strcmp(vif_chassis,
>> b_ctx_in->chassis_rec->hostname);
>> > +
>> > + /* Ports of type "virtual" should never be explicitly bound to an
>> OVS
>> > + * port in the integration bridge. If that's the case, ignore the
>> binding
>> > + * and log a warning.
>> > + */
>> > + if (!strcmp(pb->type, "virtual") && lbinding && lbinding->iface &&
>> > + binding_type == BT_VIF) {
>> > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>> > + VLOG_WARN_RL(&rl,
>> > + "Virtual port %s should not be bound to OVS port
>> %s",
>> > + pb->logical_port, lbinding->iface->name);
>> > + lbinding->pb = NULL;
>> > + return;
>> > + }
>> > +
>> > + if (lbinding && lbinding->pb && can_bind) {
>> > + claim_lport(pb, b_ctx_in->chassis_rec, lbinding->iface);
>> > +
>> > + switch (binding_type) {
>> > + case BT_VIF:
>> > + lbinding->pb = pb;
>> > + break;
>> > + case BT_CHILD:
>> > + case BT_VIRTUAL:
>> > + {
>>
>> I think this pair of '{}' is not needed in this project's coding style. It
>> looks unnatural. Correct me if I am wrong.
>>
>
> I think when we create local variables inside switch case, we need {}, but
> I think I should have used as .. case BT_VIRTUAL: {
> ...
> }
>
> I'll fix this in the next version.
>
>
>> > + /* Add child logical port to the set of all local ports. */
>> > + sset_add(b_ctx_out->local_lports, pb->logical_port);
>> > + struct local_binding *child =
>> > + local_binding_find_child(lbinding, pb->logical_port);
>> > + if (!child) {
>> > + child = local_binding_create(pb->logical_port,
>> lbinding->iface,
>> > + pb, binding_type);
>> > + local_binding_add_child(lbinding, child);
>> > + } else {
>> > + ovs_assert(child->type == BT_CHILD ||
>> > + child->type == BT_VIRTUAL);
>> > + child->pb = pb;
>> > + child->iface = lbinding->iface;
>> > + }
>> > + break;
>> > + }
>> > + default:
>> > + OVS_NOT_REACHED();
>> > + }
>> > +
>> > + 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,
>> > + pb->datapath, false,
>> b_ctx_out->local_datapaths);
>> > + update_local_lport_ids(b_ctx_out->local_lport_ids, pb);
>> > + if (lbinding->iface && qos_map && b_ctx_in->ovs_idl_txn) {
>> > + get_qos_params(pb, qos_map);
>> > + }
>> > + } else if (lbinding && lbinding->pb && !can_bind) {
>> > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>> > + VLOG_INFO_RL(&rl,
>> > + "Not claiming lport %s, chassis %s "
>> > + "requested-chassis %s",
>> > + pb->logical_port,
>> > + b_ctx_in->chassis_rec->name,
>> > + vif_chassis);
>> > + }
>> > +
>> > + if (pb->chassis == b_ctx_in->chassis_rec) {
>> > + if (!lbinding || !lbinding->pb || !can_bind) {
>> > + release_lport(pb);
>> > + }
>> > + }
>> > +}
>> > +
>> > +static void
>> > +consider_port_binding(const struct sbrec_port_binding *pb,
>> > + struct binding_ctx_in *b_ctx_in,
>> > + struct binding_ctx_out *b_ctx_out,
>> > + struct hmap *qos_map)
>> > +{
>> > +
>> > + bool our_chassis = is_our_chassis(b_ctx_in->chassis_rec, pb,
>> > + b_ctx_in->active_tunnels,
>> > + b_ctx_out->local_lports);
>> > +
>> > + if (!strcmp(pb->type, "l2gateway")) {
>> > + if (our_chassis) {
>> > + sset_add(b_ctx_out->local_lports, pb->logical_port);
>> > + 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,
>> > + pb->datapath, false,
>> > + b_ctx_out->local_datapaths);
>> > + }
>> > + } else if (!strcmp(pb->type, "chassisredirect")) {
>> > + if (ha_chassis_group_contains(pb->ha_chassis_group,
>> > + b_ctx_in->chassis_rec)) {
>> > + 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,
>> > + pb->datapath, false,
>> > + b_ctx_out->local_datapaths);
>> > + }
>> > + } else if (!strcmp(pb->type, "l3gateway")) {
>> > + if (our_chassis) {
>> > + 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,
>> > + pb->datapath, true,
>> b_ctx_out->local_datapaths);
>> > + }
>> > + } else if (!strcmp(pb->type, "localnet")) {
>> > + /* Add all localnet ports to local_lports so that we allocate
>> ct
>> zones
>> > + * for them. */
>> > + sset_add(b_ctx_out->local_lports, pb->logical_port);
>> > + if (qos_map && b_ctx_in->ovs_idl_txn) {
>> > + get_qos_params(pb, qos_map);
>> > + }
>> > + } else if (!strcmp(pb->type, "external")) {
>> > + if (ha_chassis_group_contains(pb->ha_chassis_group,
>> > + b_ctx_in->chassis_rec)) {
>> > + 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,
>> > + pb->datapath, false,
>> > + b_ctx_out->local_datapaths);
>> > + }
>> > + }
>> > +
>> > + if (our_chassis || !strcmp(pb->type, "localnet")) {
>> > + update_local_lport_ids(b_ctx_out->local_lport_ids, pb);
>> > + }
>> > +
>> > + ovs_assert(b_ctx_in->ovnsb_idl_txn);
>> > + if (our_chassis) {
>> > + claim_lport(pb, b_ctx_in->chassis_rec, NULL);
>> > + } else if (pb->chassis == b_ctx_in->chassis_rec) {
>> > + release_lport(pb);
>> > + }
>> > +}
>> > +
>> > +static void
>> > +build_local_bindings_from_local_ifaces(struct binding_ctx_in *b_ctx_in,
>> > + struct binding_ctx_out
>> *b_ctx_out)
>> > +{
>> > + int i;
>> > + for (i = 0; i < b_ctx_in->br_int->n_ports; i++) {
>> > + const struct ovsrec_port *port_rec =
>> b_ctx_in->br_int->ports[i];
>> > + const char *iface_id;
>> > + int j;
>> > +
>> > + if (!strcmp(port_rec->name, b_ctx_in->br_int->name)) {
>> > + continue;
>> > + }
>> > +
>> > + for (j = 0; j < port_rec->n_interfaces; j++) {
>> > + const struct ovsrec_interface *iface_rec;
>> > +
>> > + iface_rec = port_rec->interfaces[j];
>> > + 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) {
>> > + const struct sbrec_port_binding *pb
>> > + = lport_lookup_by_name(
>> > + b_ctx_in->sbrec_port_binding_by_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,
>> pb,
>> > + BT_VIF);
>> > + local_binding_add(b_ctx_out->local_bindings,
>> lbinding);
>> > + } else {
>> > + lbinding->type = BT_VIF;
>> > + lbinding->pb = pb;
>> > + }
>> > + sset_add(b_ctx_out->local_lports, iface_id);
>> > + }
>> > +
>> > + /* Check if this is a tunnel interface. */
>> > + if (smap_get(&iface_rec->options, "remote_ip")) {
>> > + const char *tunnel_iface
>> > + = smap_get(&iface_rec->status,
>> "tunnel_egress_iface");
>> > + if (tunnel_iface) {
>> > + sset_add(b_ctx_out->egress_ifaces, tunnel_iface);
>> > + }
>> > + }
>> > + }
>> > + }
>> > +
>> > + struct shash_node *node, *next;
>> > + SHASH_FOR_EACH_SAFE (node, next, b_ctx_out->local_bindings) {
>> > + struct local_binding *lbinding = node->data;
>> > + if (!sset_contains(b_ctx_out->local_lports, lbinding->name)) {
>> > + local_binding_destroy(lbinding);
>> > + shash_delete(b_ctx_out->local_bindings, node);
>> > + }
>> > + }
>>
>> It seems this function is called only once in the binding_run() which
>> means
>> it is called at recomputing. So the local_bindings should get rebuild (see
>> my another comment about it not getting reset in ovn-controller.c in
>> runtime_data_run). If we are rebuilding it, then this last loop of
>> deleting
>> the non-exist local_bindings is unnecessary.
>>
>
> I'm not intentionally rebuilding local_bindings during recompute.
> My idea was to keep in sync rather than recreating it. Do you see any
> issues with that ?
>
> I know that you would prefer recreating the resources at every recompute.
> I'm fine
> doing as you suggest, but I think it should be OK not to recreate as long
> as we maintain correctness.
>
> Right now it would seem that the local_binding_find() will always fail.
> But with the next patch which handles
> ovs interface and port binding changes incrementally, that will not be the
> case.
>
> Let me know what you think.
>
>
I rethought about this and to be consistent with the other engine data, I
changed to as per you suggested
and I submitted v5. Kindly take a look -
https://patchwork.ozlabs.org/project/openvswitch/list/?series=176066
I would like to revisit this later and see if it's really required to
clean up all the runtime data in every recompute or
it can be preserved.
Thanks
Numan
Thanks
> Numan
>
>
>>
>> > +}
>> > +
>> > void
>> > binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out
>> *b_ctx_out)
>> > {
>> > @@ -721,49 +865,62 @@ binding_run(struct binding_ctx_in *b_ctx_in,
>> struct
>> binding_ctx_out *b_ctx_out)
>> >
>> > const struct sbrec_port_binding *binding_rec;
>> > struct shash bridge_mappings = SHASH_INITIALIZER(&bridge_mappings);
>> > - struct shash lport_to_iface = SHASH_INITIALIZER(&lport_to_iface);
>> > - struct sset egress_ifaces = SSET_INITIALIZER(&egress_ifaces);
>> > struct hmap qos_map;
>> >
>> > hmap_init(&qos_map);
>> > if (b_ctx_in->br_int) {
>> > - get_local_iface_ids(b_ctx_in->br_int, &lport_to_iface,
>> > - b_ctx_out->local_lports, &egress_ifaces);
>> > + build_local_bindings_from_local_ifaces(b_ctx_in, b_ctx_out);
>> > }
>> >
>> > - /* Array to store pointers to local virtual ports. It is populated
>> by
>> > - * consider_local_datapath.
>> > - */
>> > - const struct sbrec_port_binding **vpbs = NULL;
>> > - size_t n_vpbs = 0;
>> > - size_t n_allocated_vpbs = 0;
>> > + struct hmap *qos_map_ptr =
>> > + !sset_is_empty(b_ctx_out->egress_ifaces) ? &qos_map : NULL;
>> >
>> > /* Run through each binding record to see if it is resident on this
>> > * chassis and update the binding accordingly. This includes both
>> > - * directly connected logical ports and children of those ports.
>> > - * Virtual ports are just added to vpbs array and will be processed
>> > - * later. This is special case for virtual ports is needed in order
>> to
>> > - * make sure we update the virtual_parent port bindings first.
>> > + * directly connected logical ports and children of those ports
>> > + * (which also includes virtual ports).
>> > */
>> > SBREC_PORT_BINDING_TABLE_FOR_EACH (binding_rec,
>> > b_ctx_in->port_binding_table) {
>> > - const struct ovsrec_interface *iface_rec
>> > - = shash_find_data(&lport_to_iface,
>> binding_rec->logical_port);
>> > - vpbs =
>> > - consider_local_datapath(binding_rec,
>> > - sset_is_empty(&egress_ifaces) ?
>> NULL
>> :
>> > - &qos_map, iface_rec, b_ctx_in,
>> b_ctx_out,
>> > - vpbs, &n_vpbs, &n_allocated_vpbs);
>> > - }
>> > + if (!strcmp(binding_rec->type, "patch") ||
>> > + !strcmp(binding_rec->type, "localport") ||
>> > + !strcmp(binding_rec->type, "vtep")) {
>> > + update_local_lport_ids(b_ctx_out->local_lport_ids,
>> binding_rec);
>> > + continue;
>> > + }
>> >
>> > - /* Now also update the virtual ports in case their parent ports
>> were
>> > - * updated above.
>> > - */
>> > - for (size_t i = 0; i < n_vpbs; i++) {
>> > -
>> consider_local_virtual_port(b_ctx_in->sbrec_port_binding_by_name,
>> > - b_ctx_in->chassis_rec, vpbs[i]);
>> > + bool consider_for_vif = false;
>> > + struct local_binding *lbinding = NULL;
>> > + enum local_binding_type binding_type = BT_VIF;
>> > + if (!binding_rec->type[0]) {
>> > + if (!binding_rec->parent_port ||
>> !binding_rec->parent_port[0]) {
>> > + lbinding =
>> local_binding_find(b_ctx_out->local_bindings,
>> > +
>> binding_rec->logical_port);
>> > + } else {
>> > + lbinding =
>> local_binding_find(b_ctx_out->local_bindings,
>> > +
>> binding_rec->parent_port);
>> > + binding_type = BT_CHILD;
>> > + }
>> > +
>> > + consider_for_vif = true;
>> > + } else if (!strcmp(binding_rec->type, "virtual") &&
>> > + binding_rec->virtual_parent &&
>> > + binding_rec->virtual_parent[0]) {
>> > + lbinding = local_binding_find(b_ctx_out->local_bindings,
>> > + binding_rec->virtual_parent);
>> > + consider_for_vif = true;
>> > + binding_type = BT_VIRTUAL;
>> > + }
>> > +
>> > + if (consider_for_vif) {
>> > + consider_port_binding_for_vif(binding_rec, b_ctx_in,
>> > + binding_type, lbinding,
>> b_ctx_out,
>> > + qos_map_ptr);
>> > + } else {
>> > + consider_port_binding(binding_rec, b_ctx_in, b_ctx_out,
>> > + qos_map_ptr);
>> > + }
>> > }
>> > - free(vpbs);
>> >
>> > add_ovs_bridge_mappings(b_ctx_in->ovs_table,
>> b_ctx_in->bridge_table,
>> > &bridge_mappings);
>> > @@ -775,49 +932,39 @@ binding_run(struct binding_ctx_in *b_ctx_in,
>> struct
>> binding_ctx_out *b_ctx_out)
>> > b_ctx_in->port_binding_table) {
>> > if (!strcmp(binding_rec->type, "localnet")) {
>> > consider_localnet_port(binding_rec, &bridge_mappings,
>> > - &egress_ifaces,
>> b_ctx_out->local_datapaths);
>> > + b_ctx_out->egress_ifaces,
>> > + b_ctx_out->local_datapaths);
>> > }
>> > }
>> > shash_destroy(&bridge_mappings);
>> >
>> > - if (!sset_is_empty(&egress_ifaces)
>> > + if (!sset_is_empty(b_ctx_out->egress_ifaces)
>> > && set_noop_qos(b_ctx_in->ovs_idl_txn, b_ctx_in->port_table,
>> > - b_ctx_in->qos_table, &egress_ifaces)) {
>> > + b_ctx_in->qos_table,
>> b_ctx_out->egress_ifaces)) {
>> > const char *entry;
>> > - SSET_FOR_EACH (entry, &egress_ifaces) {
>> > + SSET_FOR_EACH (entry, b_ctx_out->egress_ifaces) {
>> > setup_qos(entry, &qos_map);
>> > }
>> > }
>> >
>> > - shash_destroy(&lport_to_iface);
>> > - sset_destroy(&egress_ifaces);
>> > hmap_destroy(&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.
>> */
>> > bool
>> > -binding_evaluate_port_binding_changes(
>> > - const struct sbrec_port_binding_table *pb_table,
>> > - const struct ovsrec_bridge *br_int,
>> > - const struct sbrec_chassis *chassis_rec,
>> > - struct sset *active_tunnels,
>> > - struct sset *local_lports)
>> > +binding_evaluate_port_binding_changes(struct binding_ctx_in *b_ctx_in,
>> > + struct binding_ctx_out
>> *b_ctx_out)
>> > {
>> > - if (!chassis_rec) {
>> > + if (!b_ctx_in->chassis_rec) {
>> > return true;
>> > }
>> >
>> > bool changed = false;
>> >
>> > const struct sbrec_port_binding *binding_rec;
>> > - struct shash lport_to_iface = SHASH_INITIALIZER(&lport_to_iface);
>> > - struct sset egress_ifaces = SSET_INITIALIZER(&egress_ifaces);
>> > - if (br_int) {
>> > - get_local_iface_ids(br_int, &lport_to_iface, local_lports,
>> > - &egress_ifaces);
>> > - }
>> > - SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (binding_rec, pb_table) {
>> > + 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
>> > @@ -829,24 +976,31 @@ binding_evaluate_port_binding_changes(
>> > * interface table will be updated, which will trigger
>> recompute.
>> > *
>> > * - If the port is not a regular VIF, always trigger
>> recompute.
>> */
>> > - if (binding_rec->chassis == chassis_rec) {
>> > + if (binding_rec->chassis == b_ctx_in->chassis_rec) {
>> > changed = true;
>> > break;
>> > }
>> >
>> > - const struct ovsrec_interface *iface_rec
>> > - = shash_find_data(&lport_to_iface,
>> binding_rec->logical_port);
>> > - if (is_our_chassis(chassis_rec, binding_rec, active_tunnels,
>> iface_rec,
>> > - local_lports) ||
>> > - (strcmp(binding_rec->type, "") && strcmp(binding_rec->type,
>> > - "remote"))) {
>> > + if (strcmp(binding_rec->type, "")) {
>>
>> strcmp(binding_rec->type, "remote") should be kept here.
>>
>> > + changed = true;
>> > + break;
>> > + }
>> > +
>> > + 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);
>> > + } else {
>> > + lbinding = local_binding_find(b_ctx_out->local_bindings,
>> > + binding_rec->parent_port);
>> > + }
>> > +
>> > + if (lbinding) {
>> > changed = true;
>> > break;
>> > }
>> > }
>> >
>> > - shash_destroy(&lport_to_iface);
>> > - sset_destroy(&egress_ifaces);
>> > return changed;
>> > }
>> >
>> > diff --git a/controller/binding.h b/controller/binding.h
>> > index d0b8331af..6bee1d12e 100644
>> > --- a/controller/binding.h
>> > +++ b/controller/binding.h
>> > @@ -31,6 +31,7 @@ struct ovsrec_open_vswitch_table;
>> > struct sbrec_chassis;
>> > struct sbrec_port_binding_table;
>> > struct sset;
>> > +struct sbrec_port_binding;
>> >
>> > struct binding_ctx_in {
>> > struct ovsdb_idl_txn *ovnsb_idl_txn;
>> > @@ -51,8 +52,10 @@ struct binding_ctx_in {
>> >
>> > struct binding_ctx_out {
>> > struct hmap *local_datapaths;
>> > + struct shash *local_bindings;
>> > struct sset *local_lports;
>> > struct sset *local_lport_ids;
>> > + struct sset *egress_ifaces;
>> > };
>> >
>> > void binding_register_ovs_idl(struct ovsdb_idl *);
>> > @@ -60,11 +63,10 @@ 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(
>> > - const struct sbrec_port_binding_table *,
>> > - const struct ovsrec_bridge *br_int,
>> > - const struct sbrec_chassis *,
>> > - struct sset *active_tunnels,
>> > - struct sset *local_lports);
>> > -
>> > +bool binding_evaluate_port_binding_changes(struct binding_ctx_in *,
>> > + struct binding_ctx_out *);
>> > +void local_bindings_destroy(struct shash *local_bindings);
>> > +void binding_add_vport_to_local_bindings(
>> > + struct shash *local_bindings, const struct sbrec_port_binding
>> *parent,
>> > + const struct sbrec_port_binding *vport);
>> > #endif /* controller/binding.h */
>> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
>> > index 24c89e617..fdfa60e9c 100644
>> > --- a/controller/ovn-controller.c
>> > +++ b/controller/ovn-controller.c
>> > @@ -958,6 +958,9 @@ struct ed_type_runtime_data {
>> > /* Contains "struct local_datapath" nodes. */
>> > struct hmap local_datapaths;
>> >
>> > + /* Contains "struct local_bindings" nodes. */
>> > + struct shash local_bindings;
>> > +
>> > /* Contains the name of each logical port resident on the local
>> > * hypervisor. These logical ports include the VIFs (and their
>> child
>> > * logical ports, if any) that belong to VMs running on the
>> hypervisor,
>> > @@ -969,6 +972,8 @@ struct ed_type_runtime_data {
>> > * <datapath-tunnel-key>_<port-tunnel-key> */
>> > struct sset local_lport_ids;
>> > struct sset active_tunnels;
>> > +
>> > + struct sset egress_ifaces;
>> > };
>> >
>> > static void *
>> > @@ -981,6 +986,8 @@ en_runtime_data_init(struct engine_node *node
>> OVS_UNUSED,
>> > sset_init(&data->local_lports);
>> > sset_init(&data->local_lport_ids);
>> > sset_init(&data->active_tunnels);
>> > + sset_init(&data->egress_ifaces);
>> > + shash_init(&data->local_bindings);
>> > return data;
>> > }
>> >
>> > @@ -992,6 +999,7 @@ en_runtime_data_cleanup(void *data)
>> > sset_destroy(&rt_data->local_lports);
>> > sset_destroy(&rt_data->local_lport_ids);
>> > sset_destroy(&rt_data->active_tunnels);
>> > + sset_init(&rt_data->egress_ifaces);
>>
>> Should this be sset_destroy?
>>
>> > struct local_datapath *cur_node, *next_node;
>> > HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node,
>> > &rt_data->local_datapaths) {
>> > @@ -1000,6 +1008,7 @@ en_runtime_data_cleanup(void *data)
>> > free(cur_node);
>> > }
>> > hmap_destroy(&rt_data->local_datapaths);
>> > + local_bindings_destroy(&rt_data->local_bindings);
>> > }
>> >
>> > static void
>> > @@ -1072,6 +1081,8 @@ init_binding_ctx(struct engine_node *node,
>> > b_ctx_out->local_datapaths = &rt_data->local_datapaths;
>> > b_ctx_out->local_lports = &rt_data->local_lports;
>> > 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;
>> > }
>> >
>> > static void
>> > @@ -1098,9 +1109,11 @@ en_runtime_data_run(struct engine_node *node,
>> void
>> *data)
>> > sset_destroy(local_lports);
>> > sset_destroy(local_lport_ids);
>> > sset_destroy(active_tunnels);
>> > + sset_destroy(&rt_data->egress_ifaces);
>>
>> Should the local_bindings structure get reset here as well?
>>
>> > sset_init(local_lports);
>> > sset_init(local_lport_ids);
>> > sset_init(active_tunnels);
>> > + sset_init(&rt_data->egress_ifaces);
>> > }
>> >
>> > struct binding_ctx_in b_ctx_in;
>> > @@ -1129,35 +1142,12 @@ static bool
>> > runtime_data_sb_port_binding_handler(struct engine_node *node, void
>> *data)
>> > {
>> > struct ed_type_runtime_data *rt_data = data;
>> > - struct sset *local_lports = &rt_data->local_lports;
>> > - struct sset *active_tunnels = &rt_data->active_tunnels;
>> > -
>> > - struct ovsrec_open_vswitch_table *ovs_table =
>> > - (struct ovsrec_open_vswitch_table *)EN_OVSDB_GET(
>> > - engine_get_input("OVS_open_vswitch", node));
>> > - struct ovsrec_bridge_table *bridge_table =
>> > - (struct ovsrec_bridge_table *)EN_OVSDB_GET(
>> > - engine_get_input("OVS_bridge", node));
>> > - const char *chassis_id = chassis_get_id();
>> > - const struct ovsrec_bridge *br_int = get_br_int(bridge_table,
>> ovs_table);
>> > -
>> > - ovs_assert(br_int && chassis_id);
>> > -
>> > - struct ovsdb_idl_index *sbrec_chassis_by_name =
>> > - engine_ovsdb_node_get_index(
>> > - engine_get_input("SB_chassis", node),
>> > - "name");
>> > -
>> > - const struct sbrec_chassis *chassis
>> > - = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
>> > - ovs_assert(chassis);
>> > -
>> > - struct sbrec_port_binding_table *pb_table =
>> > - (struct sbrec_port_binding_table *)EN_OVSDB_GET(
>> > - engine_get_input("SB_port_binding", node));
>> > + 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 = binding_evaluate_port_binding_changes(
>> > - pb_table, br_int, chassis, active_tunnels, local_lports);
>> > + bool changed = binding_evaluate_port_binding_changes(&b_ctx_in,
>> > + &b_ctx_out);
>> >
>> > return !changed;
>> > }
>> > diff --git a/controller/pinctrl.c b/controller/pinctrl.c
>>
>> The change in pinctrl.h and pinctrl.c seem not needed. It may be left from
>> the earlier versions.
>>
>
> That's right. Thanks for pointing it out. I'll fix it in v5.
>
> Thanks
> Numan
>
>
>> > index 3230bb386..824be5e7a 100644
>> > --- a/controller/pinctrl.c
>> > +++ b/controller/pinctrl.c
>> > @@ -18,6 +18,7 @@
>> >
>> > #include "pinctrl.h"
>> >
>> > +#include "binding.h"
>> > #include "coverage.h"
>> > #include "csum.h"
>> > #include "dirs.h"
>> > diff --git a/controller/pinctrl.h b/controller/pinctrl.h
>> > index 8fa4baae9..12fb3b080 100644
>> > --- a/controller/pinctrl.h
>> > +++ b/controller/pinctrl.h
>> > @@ -31,6 +31,7 @@ struct sbrec_chassis;
>> > struct sbrec_dns_table;
>> > struct sbrec_controller_event_table;
>> > struct sbrec_service_monitor_table;
>> > +struct shash;
>> >
>> > void pinctrl_init(void);
>> > void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
>> > --
>> > 2.25.4
>> >
>> >
>> > _______________________________________________
>> > 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