On Thu, Jan 21, 2021 at 6:04 PM Dumitru Ceara <[email protected]> wrote:
> When reconciling SB Port_Bindings and NB Logical_Switch_Ports or > Logical_Router_Ports make sure we properly detect ports that are > incorrectly attached to multiple logical datapaths. > > Reported-at: https://bugzilla.redhat.com/1918582 > Reported-by: Jianlin Shi <[email protected]> > Fixes: 8bf9075968ac ("ovn-northd: Fix tunnel_key allocation for SB > Port_Bindings.") > Signed-off-by: Dumitru Ceara <[email protected]> > Thanks Dumitru for fixing this issue. I applied this patch to master and upto branch-20.03. But looks like the patch was not really required for the 20.03 branch. I see no harm though. Thanks Numan --- > northd/ovn-northd.c | 64 > ++++++++++++++++++++++++++++++++++------------------- > tests/ovn-northd.at | 45 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 86 insertions(+), 23 deletions(-) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 27df6a3..e25bd92 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -1409,17 +1409,38 @@ ovn_port_destroy(struct hmap *ports, struct > ovn_port *port) > } > } > > +/* Returns the ovn_port that matches 'name'. If 'prefer_bound' is true > and > + * multiple ports share the same name, gives precendence to ports bound to > + * an ovn_datapath. > + */ > static struct ovn_port * > -ovn_port_find(const struct hmap *ports, const char *name) > +ovn_port_find__(const struct hmap *ports, const char *name, > + bool prefer_bound) > { > + struct ovn_port *matched_op = NULL; > struct ovn_port *op; > > HMAP_FOR_EACH_WITH_HASH (op, key_node, hash_string(name, 0), ports) { > if (!strcmp(op->key, name)) { > - return op; > + matched_op = op; > + if (!prefer_bound || op->od) { > + return op; > + } > } > } > - return NULL; > + return matched_op; > +} > + > +static struct ovn_port * > +ovn_port_find(const struct hmap *ports, const char *name) > +{ > + return ovn_port_find__(ports, name, false); > +} > + > +static struct ovn_port * > +ovn_port_find_bound(const struct hmap *ports, const char *name) > +{ > + return ovn_port_find__(ports, name, true); > } > > /* Returns true if the logical switch port 'enabled' column is empty or > @@ -2107,15 +2128,13 @@ join_logical_ports(struct northd_context *ctx, > for (size_t i = 0; i < od->nbs->n_ports; i++) { > const struct nbrec_logical_switch_port *nbsp > = od->nbs->ports[i]; > - struct ovn_port *op = ovn_port_find(ports, nbsp->name); > - if (op && op->sb->datapath == od->sb) { > - if (op->nbsp || op->nbrp) { > - static struct vlog_rate_limit rl > - = VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, "duplicate logical port %s", > - nbsp->name); > - continue; > - } > + struct ovn_port *op = ovn_port_find_bound(ports, > nbsp->name); > + if (op && (op->od || op->nbsp || op->nbrp)) { > + static struct vlog_rate_limit rl > + = VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, "duplicate logical port %s", > nbsp->name); > + continue; > + } else if (op && (!op->sb || op->sb->datapath == od->sb)) > { > ovn_port_set_nb(op, nbsp, NULL); > ovs_list_remove(&op->list); > > @@ -2206,16 +2225,15 @@ join_logical_ports(struct northd_context *ctx, > continue; > } > > - struct ovn_port *op = ovn_port_find(ports, nbrp->name); > - if (op && op->sb->datapath == od->sb) { > - if (op->nbsp || op->nbrp) { > - static struct vlog_rate_limit rl > - = VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, "duplicate logical router port > %s", > - nbrp->name); > - destroy_lport_addresses(&lrp_networks); > - continue; > - } > + struct ovn_port *op = ovn_port_find_bound(ports, > nbrp->name); > + if (op && (op->od || op->nbsp || op->nbrp)) { > + static struct vlog_rate_limit rl > + = VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, "duplicate logical router port %s", > + nbrp->name); > + destroy_lport_addresses(&lrp_networks); > + continue; > + } else if (op && (!op->sb || op->sb->datapath == od->sb)) > { > ovn_port_set_nb(op, NULL, nbrp); > ovs_list_remove(&op->list); > ovs_list_push_back(both, &op->list); > @@ -2258,7 +2276,7 @@ join_logical_ports(struct northd_context *ctx, > char *redirect_name = > ovn_chassis_redirect_name(nbrp->name); > struct ovn_port *crp = ovn_port_find(ports, > redirect_name); > - if (crp && crp->sb->datapath == od->sb) { > + if (crp && crp->sb && crp->sb->datapath == od->sb) { > crp->derived = true; > ovn_port_set_nb(crp, NULL, nbrp); > ovs_list_remove(&crp->list); > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index df03b6e..d22cad8 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -2399,3 +2399,48 @@ ovn-nbctl destroy bfd $uuid > check_row_count bfd 2 > > AT_CLEANUP > + > +AT_SETUP([ovn -- check LSP attached to multiple LS]) > +ovn_start > + > +check ovn-nbctl ls-add ls1 \ > + -- ls-add ls2 \ > + -- lsp-add ls1 p1 > +check ovn-nbctl --wait=sb sync > + > +uuid=$(fetch_column nb:Logical_Switch_Port _uuid name=p1) > +check ovn-nbctl set Logical_Switch ls2 ports=$uuid > +check ovn-nbctl --wait=sb sync > + > +AT_CHECK([grep -qE 'duplicate logical port p1' northd/ovn-northd.log], > [0]) > + > +AT_CLEANUP > + > +AT_SETUP([ovn -- check LRP attached to multiple LR]) > +ovn_start > + > +check ovn-nbctl lr-add lr1 \ > + -- lr-add lr2 \ > + -- lrp-add lr1 p1 00:00:00:00:00:01 10.0.0.1/24 > +check ovn-nbctl --wait=sb sync > + > +uuid=$(fetch_column nb:Logical_Router_Port _uuid name=p1) > +check ovn-nbctl set Logical_Router lr2 ports=$uuid > +check ovn-nbctl --wait=sb sync > + > +AT_CHECK([grep -qE 'duplicate logical router port p1' > northd/ovn-northd.log], [0]) > + > +AT_CLEANUP > + > +AT_SETUP([ovn -- check duplicate LSP/LRP]) > +ovn_start > + > +check ovn-nbctl ls-add ls \ > + -- lsp-add ls p1 \ > + -- lr-add lr \ > + -- lrp-add lr p1 00:00:00:00:00:01 10.0.0.1/24 > +check ovn-nbctl --wait=sb sync > + > +AT_CHECK([grep -qE 'duplicate logical.*port p1' northd/ovn-northd.log], > [0]) > + > +AT_CLEANUP > -- > 1.8.3.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
