On Tue, Jan 7, 2025 at 10:42 AM Frode Nordahl <[email protected]> wrote:

> Commit ac1dc2ba050c introduced re-use of the add_to_routes_ad()
> function for both connected and static routes, however it did
> not add conditionals for handling warning level logging of
> duplicate routes.
>
> Reported-at: https://github.com/ovn-org/ovn/issues/268
> Fixes: ac1dc2ba050c ("ic: prevent advertising/learning multiple same
> routes")
> Signed-off-by: Frode Nordahl <[email protected]>
> ---
>  ic/ovn-ic.c     | 14 +++++++++---
>  tests/ovn-ic.at | 61 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 72 insertions(+), 3 deletions(-)
>
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index 8320cbea5..75b5d1787 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -1128,6 +1128,8 @@ add_to_routes_ad(struct hmap *routes_ad, const
> struct in6_addr prefix,
>                   const struct nbrec_logical_router *nb_lr,
>                   const char *route_tag)
>  {
> +    ovs_assert(nb_route || nb_lrp);
> +
>      if (route_table == NULL) {
>          route_table = "";
>      }
> @@ -1149,9 +1151,15 @@ add_to_routes_ad(struct hmap *routes_ad, const
> struct in6_addr prefix,
>          hmap_insert(routes_ad, &ic_route->node, hash);
>      } else {
>          static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> -        VLOG_WARN_RL(&rl, "Duplicate route advertisement was suppressed!
> NB "
> -                     "route uuid: "UUID_FMT,
> -                     UUID_ARGS(&nb_route->header_.uuid));
> +        const char *msg_fmt = "Duplicate %s route advertisement was "
> +                              "suppressed! NB %s uuid: "UUID_FMT;
> +        if (nb_route) {
> +            VLOG_WARN_RL(&rl, msg_fmt, origin, "route",
> +                         UUID_ARGS(&nb_route->header_.uuid));
> +        } else {
> +            VLOG_WARN_RL(&rl, msg_fmt, origin, "lrp",
> +                         UUID_ARGS(&nb_lrp->header_.uuid));
> +        }
>      }
>  }
>
> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
> index 13150a453..e3b2a62ab 100644
> --- a/tests/ovn-ic.at
> +++ b/tests/ovn-ic.at
> @@ -282,6 +282,67 @@ OVN_CLEANUP_IC
>  AT_CLEANUP
>  ])
>
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn-ic -- local duplicate connected route])
> +
> +ovn_init_ic_db
> +net_add n1
> +
> +# 1 GW per AZ
> +for i in 1 2; do
> +    az=az$i
> +    ovn_start $az
> +    sim_add gw-$az
> +    as gw-$az
> +    check ovs-vsctl add-br br-phys
> +    ovn_az_attach $az n1 br-phys 192.168.1.$i
> +    check ovs-vsctl set open . external-ids:ovn-is-interconn=true
> +    check ovn-nbctl set nb-global . \
> +        options:ic-route-adv=true \
> +        options:ic-route-adv-default=true \
> +        options:ic-route-learn=true \
> +        options:ic-route-learn-default=true
> +done
> +
> +ovn_as az1
> +
> +# create transit switch and connect to LR
> +check ovn-ic-nbctl --wait=sb ts-add ts1
> +for i in 1 2; do
> +    ovn_as az$i
> +
> +    check \
> +       grep -vq "connected route advertisement was suppressed! NB lrp" \
> +       az$i/ic/ovn-ic.log
> +
> +    check ovn-nbctl lr-add lr1
> +    check ovn-nbctl lrp-add lr1 lrp$i 00:00:00:00:0$i:01 10.0.$i.1/24
> +    check ovn-nbctl lrp-add lr1 lrp-dup1 00:00:00:00:0$i:42 10.0.42.1/24
> +    check ovn-nbctl lrp-add lr1 lrp-dup2 00:00:00:00:0$i:51 10.0.42.1/24
> +    check ovn-nbctl lrp-set-gateway-chassis lrp$i gw-az$i
> +
> +    check ovn-nbctl --wait=sb lsp-add ts1 lsp$i -- \
> +        lsp-set-addresses lsp$i router -- \
> +        lsp-set-type lsp$i router -- \
> +        lsp-set-options lsp$i router-port=lrp$i
> +    check ovn-ic-nbctl --wait=sb sync
> +
> +    check \
> +       grep -q "connected route advertisement was suppressed! NB lrp" \
> +       az$i/ic/ovn-ic.log
> +done
> +
> +for i in 1 2; do
> +    az=az$i
> +    ovn_as $az
> +    OVN_CLEANUP_SBOX(gw-$az)
> +    OVN_CLEANUP_AZ([$az])
> +done
> +
> +OVN_CLEANUP_IC
> +AT_CLEANUP
> +])
> +
>  OVN_FOR_EACH_NORTHD([
>  AT_SETUP([ovn-ic -- gateway sync])
>
> --
> 2.47.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Looks good to me, thanks.

Acked-by: Ales Musil <[email protected]>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to