[ovs-dev] [PATCH ovn] northd: Fix duplicate logical port detection.
Dumitru Ceara
dceara at redhat.com
Thu Jan 21 12:34:11 UTC 2021
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 <jishi at redhat.com>
Fixes: 8bf9075968ac ("ovn-northd: Fix tunnel_key allocation for SB Port_Bindings.")
Signed-off-by: Dumitru Ceara <dceara at redhat.com>
---
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
More information about the dev
mailing list