ons. 29. jan. 2025, 15:13 skrev Frode Nordahl <fnord...@ubuntu.com>:

> On Wed, Jan 29, 2025 at 9:20 AM Frode Nordahl <fnord...@ubuntu.com> wrote:
> >
> > The commit in the fixes tag introduced support for adding LRPs
> > without networks, leaving them with only the IPv6 Link Local
> > Address (LLA).
> >
> > This is a very useful feature for routing between LRs, avoiding
> > waste of a scarce resource such as IPv4 addresses.
> >
> > It did however miss a couple of direct references to
> > router_port->lrp_networks.ipv4_addrs[0].
> >
> > Check for presence of IPv4 addresses, and use IPv6 LLA when none
> > are present.
> >
> > Expand "ovn -- NAT and Load Balancer flows" test case to include
> > a neighbour LR with LRP without IPv4 address, as that is enough
> > to expose the crash.
> >
> > Note that more elaborate flow checks will be added in a subsequent
> > patch that fixes a logic bug in the routing IPv4 over IPv6 next
> > hop patch.
> >
> > Cc: MJ Ponsonby <mj.ponso...@canonical.com>
> > Fixes: 0ee90e29eb8b ("Allow creation of a LRP without ipv4.")
> > Signed-off-by: Frode Nordahl <fnord...@ubuntu.com>
> > ---
>
> Recheck-request: github-robot
>

One more time, the test in question does look flaky and does not fail in
GHA on a different  but otherwise equal fork.

Knowing we might meet this test again in downstream CI infrastructure I'll
make a note for us to revisit it at a later time.

Recheck-request: github-robot

--
Frode Nordahl

>  northd/northd.c     | 31 ++++++++++++++++++++++++-------
> >  tests/ovn-northd.at | 13 +++++++++++--
> >  2 files changed, 35 insertions(+), 9 deletions(-)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 3ff4326e6..74a33a432 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -15181,9 +15181,8 @@ build_dhcp_relay_flows_for_lrouter_port(struct
> ovn_port *op,
> >                                          struct ds *match, struct ds
> *actions,
> >                                          struct lflow_ref *lflow_ref)
> >  {
> > -    if (!op->nbrp || !op->nbrp->dhcp_relay) {
> > +    if (!op->nbrp || !op->nbrp->dhcp_relay ||
> !op->lrp_networks.n_ipv4_addrs) {
> >          return;
> > -
> >      }
> >
> >      /* configure dhcp relay flows only when peer switch has
> > @@ -17089,28 +17088,46 @@ build_routable_flows_for_router_port(
> >      struct ovn_port_routable_addresses ra =
> >          get_op_routable_addresses(lrp, lr_stateful_rec);
> >
> > -    struct ovn_port *router_port;
> > -
> >      for (size_t i = 0; i < peer_ls->n_router_ports; i++) {
> > -        router_port = peer_ls->router_ports[i]->peer;
> > +        struct ovn_port *router_port = peer_ls->router_ports[i]->peer;
> > +        struct lport_addresses *lrpaddrs = &router_port->lrp_networks;
> > +        char *router_port_lla_s = NULL;
> >
> >          if (router_port == lrp) {
> >              continue;
> >          }
> >
> > +        bool is_ipv4_nexthop = true;
> > +        if (!lrpaddrs->n_ipv4_addrs) {
> > +            for (size_t v = 0; v < lrpaddrs->n_ipv6_addrs; v++) {
> > +                struct ipv6_netaddr *addrs = &lrpaddrs->ipv6_addrs[v];
> > +                if (in6_is_lla(&addrs->network)) {
> > +                    router_port_lla_s = addrs->addr_s;
> > +                    is_ipv4_nexthop = false;
> > +                }
> > +            }
> > +            if (!router_port_lla_s) {
> > +                continue;
> > +            }
> > +        }
> > +
> >          if (lrp->nbrp->ha_chassis_group ||
> >                  lrp->nbrp->n_gateway_chassis || lrp->od->is_gw_router) {
> > +
> >              for (size_t j = 0; j < ra.n_addrs; j++) {
> >                  struct lport_addresses *laddrs = &ra.laddrs[j];
> > +
> >                  for (size_t k = 0; k < laddrs->n_ipv4_addrs; k++) {
> >                      add_route(lflows, router_port->od, router_port,
> > -
> router_port->lrp_networks.ipv4_addrs[0].addr_s,
> > +                              is_ipv4_nexthop ?
> > +
> router_port->lrp_networks.ipv4_addrs[0].addr_s :
> > +                              router_port_lla_s,
> >                                laddrs->ipv4_addrs[k].network_s,
> >                                laddrs->ipv4_addrs[k].plen, NULL, false,
> 0,
> >                                bfd_ports, &router_port->nbrp->header_,
> >                                false, ROUTE_SOURCE_CONNECTED,
> >                                lrp->stateful_lflow_ref,
> > -                              true, true);
> > +                              true, is_ipv4_nexthop ? true : false);
> >                  }
> >              }
> >          }
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index df646ec68..c157b00da 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -5223,7 +5223,12 @@ check ovn-nbctl lsp-add sw sw-ro1
> >
> >  check ovn-nbctl lr-add ro2
> >  check ovn-nbctl lrp-add ro2 ro2-sw 00:00:00:00:00:02 20.0.0.1/24
> > -check ovn-nbctl --wait=sb lsp-add sw sw-ro2
> > +check ovn-nbctl lsp-add sw sw-ro2
> > +
> > +# add neighbor with no IPv4 address to confirm routable NAT/LB flows
> > +check ovn-nbctl lr-add ro3
> > +check ovn-nbctl lrp-add ro3 ro3-sw 00:00:00:00:00:03
> > +check ovn-nbctl --wait=sb lsp-add sw sw-ro3
> >
> >  check ovn-nbctl ls-add ls1
> >  check ovn-nbctl lsp-add ls1 vm1
> > @@ -5263,7 +5268,11 @@ check ovn-nbctl lsp-set-options sw-ro1
> router-port=ro1-sw
> >
> >  check ovn-nbctl lsp-set-type sw-ro2 router
> >  check ovn-nbctl lsp-set-addresses sw-ro2 router
> > -check ovn-nbctl --wait=sb lsp-set-options sw-ro2 router-port=ro2-sw
> > +check ovn-nbctl lsp-set-options sw-ro2 router-port=ro2-sw
> > +
> > +check ovn-nbctl lsp-set-type sw-ro3 router
> > +check ovn-nbctl lsp-set-addresses sw-ro3 router
> > +check ovn-nbctl --wait=sb lsp-set-options sw-ro3 router-port=ro3-sw
> >
> >  check_lflows 0
> >
> > --
> > 2.43.0
> >
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to