On Thu, Aug 17, 2023 at 12:05 AM <num...@ovn.org> wrote: > > From: Numan Siddique <num...@ovn.org> > > When changes to port bindings corresponding to router ports are > handled by northd engine node, incorrect warning logs (like below) > are logged. > > ---- > northd|WARN|A port-binding for lrp0 is created but the LSP is not found. > ---- > > Fix these warnings. > > Fixes: 3b120ccf7f7c ("northd: Incremental processing of SB port_binding in "northd" node.") > > CC: Han Zhou <hz...@ovn.org> > Signed-off-by: Numan Siddique <num...@ovn.org> > --- > > v2 -> v3 > ------ > * Checked if type of the 'pb' and return false if its a router port > before doing a lookup in the ls_ports map. > > v1 -> v2 > ------- > * v1 was not returning false in northd_handle_sb_port_binding_changes() > for router port - port bindings because of which IPv6 PD system test > was failing. Fixed it in v2. > > > controller/binding.c | 75 -------------------------------- > controller/binding.h | 20 +-------- > lib/ovn-util.c | 101 +++++++++++++++++++++++++++++++++++++++++++ > lib/ovn-util.h | 21 +++++++++ > northd/northd.c | 7 +++ > 5 files changed, 130 insertions(+), 94 deletions(-) > > diff --git a/controller/binding.c b/controller/binding.c > index 3ac0c35df..a521f2828 100644 > --- a/controller/binding.c > +++ b/controller/binding.c > @@ -756,7 +756,6 @@ static void binding_lport_clear_port_sec(struct binding_lport *); > static bool binding_lport_update_port_sec( > struct binding_lport *, const struct sbrec_port_binding *); > > -static char *get_lport_type_str(enum en_lport_type lport_type); > static bool ovs_iface_matches_lport_iface_id_ver( > const struct ovsrec_interface *, > const struct sbrec_port_binding *); > @@ -1055,80 +1054,6 @@ binding_dump_local_bindings(struct local_binding_data *lbinding_data, > free(nodes); > } > > -static bool > -is_lport_vif(const struct sbrec_port_binding *pb) > -{ > - return !pb->type[0]; > -} > - > -enum en_lport_type > -get_lport_type(const struct sbrec_port_binding *pb) > -{ > - if (is_lport_vif(pb)) { > - if (pb->parent_port && pb->parent_port[0]) { > - return LP_CONTAINER; > - } > - return LP_VIF; > - } else if (!strcmp(pb->type, "patch")) { > - return LP_PATCH; > - } else if (!strcmp(pb->type, "chassisredirect")) { > - return LP_CHASSISREDIRECT; > - } else if (!strcmp(pb->type, "l3gateway")) { > - return LP_L3GATEWAY; > - } else if (!strcmp(pb->type, "localnet")) { > - return LP_LOCALNET; > - } else if (!strcmp(pb->type, "localport")) { > - return LP_LOCALPORT; > - } else if (!strcmp(pb->type, "l2gateway")) { > - return LP_L2GATEWAY; > - } else if (!strcmp(pb->type, "virtual")) { > - return LP_VIRTUAL; > - } else if (!strcmp(pb->type, "external")) { > - return LP_EXTERNAL; > - } else if (!strcmp(pb->type, "remote")) { > - return LP_REMOTE; > - } else if (!strcmp(pb->type, "vtep")) { > - return LP_VTEP; > - } > - > - return LP_UNKNOWN; > -} > - > -static char * > -get_lport_type_str(enum en_lport_type lport_type) > -{ > - switch (lport_type) { > - case LP_VIF: > - return "VIF"; > - case LP_CONTAINER: > - return "CONTAINER"; > - case LP_VIRTUAL: > - return "VIRTUAL"; > - case LP_PATCH: > - return "PATCH"; > - case LP_CHASSISREDIRECT: > - return "CHASSISREDIRECT"; > - case LP_L3GATEWAY: > - return "L3GATEWAY"; > - case LP_LOCALNET: > - return "PATCH"; > - case LP_LOCALPORT: > - return "LOCALPORT"; > - case LP_L2GATEWAY: > - return "L2GATEWAY"; > - case LP_EXTERNAL: > - return "EXTERNAL"; > - case LP_REMOTE: > - return "REMOTE"; > - case LP_VTEP: > - return "VTEP"; > - case LP_UNKNOWN: > - return "UNKNOWN"; > - } > - > - OVS_NOT_REACHED(); > -} > - > void > set_pb_chassis_in_sbrec(const struct sbrec_port_binding *pb, > const struct sbrec_chassis *chassis_rec, > diff --git a/controller/binding.h b/controller/binding.h > index 235e5860d..24bc84079 100644 > --- a/controller/binding.h > +++ b/controller/binding.h > @@ -22,6 +22,7 @@ > #include "openvswitch/hmap.h" > #include "openvswitch/uuid.h" > #include "openvswitch/list.h" > +# include "lib/ovn-util.h" > #include "sset.h" > #include "lport.h" > > @@ -217,25 +218,6 @@ void port_binding_set_down(const struct sbrec_chassis *chassis_rec, > const char *iface_id, > const struct uuid *pb_uuid); > > -/* Corresponds to each Port_Binding.type. */ > -enum en_lport_type { > - LP_UNKNOWN, > - LP_VIF, > - LP_CONTAINER, > - LP_PATCH, > - LP_L3GATEWAY, > - LP_LOCALNET, > - LP_LOCALPORT, > - LP_L2GATEWAY, > - LP_VTEP, > - LP_CHASSISREDIRECT, > - LP_VIRTUAL, > - LP_EXTERNAL, > - LP_REMOTE > -}; > - > -enum en_lport_type get_lport_type(const struct sbrec_port_binding *); > - > /* This structure represents a logical port (or port binding) > * which is associated with 'struct local_binding'. > * > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > index 080ad4c0c..3a237b7fe 100644 > --- a/lib/ovn-util.c > +++ b/lib/ovn-util.c > @@ -1168,3 +1168,104 @@ encode_fqdn_string(const char *fqdn, size_t *len) > > return encoded; > } > + > +static bool > +is_lport_vif(const struct sbrec_port_binding *pb) > +{ > + return !pb->type[0]; > +} > + > +enum en_lport_type > +get_lport_type(const struct sbrec_port_binding *pb) > +{ > + if (is_lport_vif(pb)) { > + if (pb->parent_port && pb->parent_port[0]) { > + return LP_CONTAINER; > + } > + return LP_VIF; > + } else if (!strcmp(pb->type, "patch")) { > + return LP_PATCH; > + } else if (!strcmp(pb->type, "chassisredirect")) { > + return LP_CHASSISREDIRECT; > + } else if (!strcmp(pb->type, "l3gateway")) { > + return LP_L3GATEWAY; > + } else if (!strcmp(pb->type, "localnet")) { > + return LP_LOCALNET; > + } else if (!strcmp(pb->type, "localport")) { > + return LP_LOCALPORT; > + } else if (!strcmp(pb->type, "l2gateway")) { > + return LP_L2GATEWAY; > + } else if (!strcmp(pb->type, "virtual")) { > + return LP_VIRTUAL; > + } else if (!strcmp(pb->type, "external")) { > + return LP_EXTERNAL; > + } else if (!strcmp(pb->type, "remote")) { > + return LP_REMOTE; > + } else if (!strcmp(pb->type, "vtep")) { > + return LP_VTEP; > + } > + > + return LP_UNKNOWN; > +} > + > +char * > +get_lport_type_str(enum en_lport_type lport_type) > +{ > + switch (lport_type) { > + case LP_VIF: > + return "VIF"; > + case LP_CONTAINER: > + return "CONTAINER"; > + case LP_VIRTUAL: > + return "VIRTUAL"; > + case LP_PATCH: > + return "PATCH"; > + case LP_CHASSISREDIRECT: > + return "CHASSISREDIRECT"; > + case LP_L3GATEWAY: > + return "L3GATEWAY"; > + case LP_LOCALNET: > + return "LOCALNET"; > + case LP_LOCALPORT: > + return "LOCALPORT"; > + case LP_L2GATEWAY: > + return "L2GATEWAY"; > + case LP_EXTERNAL: > + return "EXTERNAL"; > + case LP_REMOTE: > + return "REMOTE"; > + case LP_VTEP: > + return "VTEP"; > + case LP_UNKNOWN: > + return "UNKNOWN"; > + } > + > + OVS_NOT_REACHED(); > +} > + > +bool > +is_pb_router_type(const struct sbrec_port_binding *pb) > +{ > + enum en_lport_type lport_type = get_lport_type(pb); > + > + switch (lport_type) { > + case LP_PATCH: > + case LP_CHASSISREDIRECT: > + case LP_L3GATEWAY: > + case LP_L2GATEWAY: > + return true; > + > + case LP_VIF: > + case LP_CONTAINER: > + case LP_VIRTUAL: > + case LP_LOCALNET: > + case LP_LOCALPORT: > + case LP_REMOTE: > + case LP_VTEP: > + case LP_EXTERNAL: > + case LP_UNKNOWN: > + return false; > + } > + > + return false; > +} > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > index 5ebdd8adb..b056a6c0e 100644 > --- a/lib/ovn-util.h > +++ b/lib/ovn-util.h > @@ -409,4 +409,25 @@ void flow_collector_ids_clear(struct flow_collector_ids *); > * The returned pointer has to be freed by caller. */ > char *encode_fqdn_string(const char *fqdn, size_t *len); > > +/* Corresponds to each Port_Binding.type. */ > +enum en_lport_type { > + LP_UNKNOWN, > + LP_VIF, > + LP_CONTAINER, > + LP_PATCH, > + LP_L3GATEWAY, > + LP_LOCALNET, > + LP_LOCALPORT, > + LP_L2GATEWAY, > + LP_VTEP, > + LP_CHASSISREDIRECT, > + LP_VIRTUAL, > + LP_EXTERNAL, > + LP_REMOTE > +}; > + > +enum en_lport_type get_lport_type(const struct sbrec_port_binding *); > +char *get_lport_type_str(enum en_lport_type lport_type); > +bool is_pb_router_type(const struct sbrec_port_binding *); > + > #endif /* OVN_UTIL_H */ > diff --git a/northd/northd.c b/northd/northd.c > index 9a12a94ae..0a749931e 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -5267,10 +5267,17 @@ northd_handle_sb_port_binding_changes( > const struct sbrec_port_binding *pb; > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, sbrec_port_binding_table) { > + /* Return false if the 'pb' belongs to a router port. We don't handle > + * I-P for router ports yet. */ > + if (is_pb_router_type(pb)) { > + return false; > + } > + > struct ovn_port *op = ovn_port_find(ls_ports, pb->logical_port); > if (op && !op->lsp_can_be_inc_processed) { > return false; > } > + > if (sbrec_port_binding_is_new(pb)) { > /* Most likely the PB was created by northd and this is the > * notification of that trasaction. So we just update the sb > -- > 2.40.1 >
Thanks Numan. Acked-by: Han Zhou <hz...@ovn.org> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev