On Wed, 2025-02-26 at 21:53 +0100, Dumitru Ceara wrote:
> On 2/26/25 5:28 PM, martin.kal...@canonical.com wrote:
> > On Wed, 2025-02-26 at 17:07 +0100, Ilya Maximets wrote:
> > > On 2/26/25 16:50, Dumitru Ceara wrote:
> > > > On 2/26/25 4:47 PM, martin.kal...@canonical.com wrote:
> > > > > On Wed, 2025-02-26 at 15:44 +0100, Ilya Maximets wrote:
> > > > > > A peer of a switch can be another switch, so the port type
> > > > > > has
> > > > > > to be
> > > > > > checked.  The missing check doesn't seem to lead to
> > > > > > crashes,
> > > > > > but it
> > > > > > leads to addresses of switch-switch ports not being
> > > > > > advertised.
> > > > > 
> > > > > Oh, interesting. Is the switch-switch connection introduced
> > > > > as
> > > > > part of
> > > > > your spine-leaf topology Ilya?
> > > 
> > > Yep.
> > 
> > Nice.
> > 
> > > 
> > > > > 
> > > > > I wonder if this affects also
> > > > > `build_{lb,nat}_connected_parsed_routes`
> > > > > [0][1]. We employ similar logic in these functions, but
> > > > > instead
> > > > > of 
> > > > > 
> > > > >   HMAP_FOR_EACH (port, dp_node, &peer_od->ports)
> > > > > 
> > > > > we iterate only router ports in peer_od
> > > > > 
> > > > >   for (size_t i = 0; i < peer_od->n_router_ports; i++)
> > > > > 
> > > > > Do you think we need to update our logic in there too? E.g
> > > > > 
> > > > >   LR0 --- LS0 --- LS1 --- LR1
> > > > > 
> > > > > If we redistribute NAT/LB on LR0, do we need to traverse all
> > > > > the
> > > > > way to
> > > > > the LS1 and search for connected routers in there too, to
> > > > > find
> > > > > LR1? 
> > > > > 
> > > > 
> > > > On a similar note, I was wondering initially if we should do a
> > > > full
> > > > traversal and advertise everything that's indirectly connected
> > > > but
> > > > I was
> > > > never convinced about it.
> > > > 
> > > > However, for switch-switch connections that might be good to do
> > > > automatically indeed.
> > > > 
> > > 
> > > I'm not 100% sure about this.  We do not leak a lot of
> > > information
> > > between directly connected switches sort of on purpose, to let
> > > them
> > > learn what they need to learn on their own via standard
> > > networking.
> > > 
> > > Is that necessary to redistribute those NAT/LB rules?  i.e. does
> > > it affect correctness of the network if we do not? (I'm not
> > > familiar
> > > with that part of the code well enough)
> > 
> > Just in case it's not a typo, I want to say that this is about
> > redistributing NAT/LB routes, not rules. I.e. in the topology
> > above,
> > LR0 should be able to advertise external IPs/LB VIPs of LR1 too.
> > That's
> > why we currently iterate through peer LS's router_ports, to
> > discover
> > neighboring routers, and fetch external IPs/VIPs to advertise.
> > 
> > So it's fundamentally very similar to publish_host_routes that is
> > being
> > fixed in this patch. The difference being that publish_host_routes
> > advertises everything on the neighboring routers (LRP IPs, NAT ext.
> > IPs, LB VIPs), while [0] and [1] do it only for NAT ext. IPs a LB
> > VIPs
> > respectively.
> > 
> > If the consensus ends up being that we should do it, I can try to
> > whip
> > up something tomorrow. (Unless you want to take care of it
> > Dumitru).
> > 
> 
> Given that Felix's alternate topology [0] allows us to combine both
> advertising LB/NAT IPs and switch-switch connections without any code
> changes, I think we should at most just document the limitation.  At
> least for 25.03 that is.
> 

+1

Thanks,
Martin.

> Regards,
> Dumitru
> 
> [0]
> https://mail.openvswitch.org/pipermail/ovs-dev/2025-February/421480.html
> 
> > Thanks,
> > Martin.
> > 
> > > 
> > > Also, such a logic will need to be implemented for ovn-ic daemon
> > > as well.  Right?  It does some route synchronization today for
> > > the
> > > routers directly connected to a transit switch, IIRC.
> > > 
> > > > > [0]
> > > > > https://github.com/ovn-org/ovn/blob/dde3bdca2ddd82d0aaab6fad8638b4b321e82a43/northd/northd.c#L11478
> > > > > [1]
> > > > > https://github.com/ovn-org/ovn/blob/dde3bdca2ddd82d0aaab6fad8638b4b321e82a43/northd/northd.c#L11384
> > > > > 
> > > > > 
> > > > > Thanks,
> > > > > Martin.
> > > > > 
> > > > 
> > > > Regards,
> > > > Dumitru
> > > 
> > 
> 

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to