On Fri, Feb 07, 2025 at 08:17:42PM +0100, Dumitru Ceara wrote:
> On 2/6/25 3:19 PM, Felix Huettner via dev wrote:
> > northd allows us to announce host routes instead of connected routes of
> > LRs.
> > 
> > For the host routes we also know the LSP that host the address.
> > We can then check if this LSP is pointing to a LRP that is also local to
> > the current chassis, so if a LR is chained behind an LR and both use
> > chassisredirect ports.
> > In this case we can announce the route with a more preferable priority
> > than otherwise.
> > 
> > This helps in the following case:
> > * the backend router is bound on only a single chassis
> > * the frontend router is bound to multiple chassis (with multiple LRPs)
> > * one of the chassis of the frontend router matches the backend router
> > 
> > In this case it would be preferable if the network fabric sends the
> > traffic to the chassis that hosts both the frontend and backend router.
> > Other chassis would work as well, but then OVN would redirect the
> > traffic from one chassis to the other.
> > So this allows us to skip tunneling traffic in one case.
> > 
> > Acked-by: Lorenzo Bianconi <[email protected]>
> > Acked-by: Dumitru Ceara <[email protected]>
> > Signed-off-by: Felix Huettner <[email protected]>
> > ---
> 
> Hi Felix,
> 
> > @@ -5089,6 +5126,24 @@ route_sb_port_binding_data_handler(struct 
> > engine_node *node, void *data)
> >              return false;
> >          }
> >  
> > +        if (sset_contains(&re_data->tracked_ports_local,
> > +                          sbrec_pb->logical_port)) {
> > +            if (!route_exchange_find_port(sbrec_port_binding_by_name, 
> > chassis,
> > +                                        &rt_data->active_tunnels, 
> > sbrec_pb)) {
> 
> Nit: indentation, &rt_data should be under sbrec_..
> 
> > +                /* The port was previously local but now it no longer is. 
> > */
> > +                return false;
> > +            }
> > +        }
> > +
> > +        if (sset_contains(&re_data->tracked_ports_remote,
> > +                          sbrec_pb->logical_port)) {
> > +            if (route_exchange_find_port(sbrec_port_binding_by_name, 
> > chassis,
> > +                                       &rt_data->active_tunnels, 
> > sbrec_pb)) {
> 
> Nit: indentation, &rt_data should be under sbrec_..
> 
> > +                /* The port was previously remote but now we bound it. */
> > +                return false;
> > +            }
> > +        }
> > +
> >      }
> >      return true;
> >  }
> 
> [...]
> 
> > diff --git a/controller/route.h b/controller/route.h
> > index 9115ba438..ac57ed22c 100644
> > --- a/controller/route.h
> > +++ b/controller/route.h
> > @@ -42,6 +42,13 @@ struct route_ctx_in {
> >  
> >  struct route_ctx_out {
> >      struct hmap *tracked_re_datapaths;
> > +
> > +    /* Contains the tracked_ports that in the last run were bound locally. 
> > */
> > +    struct sset *tracked_ports_local;
> > +
> > +    /* Contains the tracked_ports that in the last run were not bound 
> > local. */
> 
> s/local./locally./
> 
> With these fixed, my ack stands:
> Acked-by: Dumitru Ceara <[email protected]>

Thanks a lot for the review. I'll add it in v8.

Thanks,
Felix

> 
> Thanks,
> Dumitru
> 
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to