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.
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
