[ovs-dev] [PATCH ovn] northd: Fix duplicate logical port detection.
Numan Siddique
numans at ovn.org
Mon Jan 25 10:03:40 UTC 2021
On Thu, Jan 21, 2021 at 6:04 PM Dumitru Ceara <dceara at redhat.com> 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 <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>
>
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
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
More information about the dev
mailing list