On Wed, Feb 26, 2025 at 03:44:52PM +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.
> 
> Fix that by checking the port type instead of assuming.
> 
> The issue is a result of two features being developed at the same time
> and the code not being re-checked before merging the routing patches.

Thanks a lot, thats a nice one.

Acked-by: Felix Huettner <felix.huettner@stackit.cloud>

> 
> Fixes: 93f541f342f9 ("northd: Allow announcing individual host routes.")
> Signed-off-by: Ilya Maximets <i.maxim...@ovn.org>
> ---
>  northd/en-advertised-route-sync.c |  2 +-
>  tests/ovn-northd.at               | 15 +++++++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/northd/en-advertised-route-sync.c 
> b/northd/en-advertised-route-sync.c
> index 794a87cb3..f28073a80 100644
> --- a/northd/en-advertised-route-sync.c
> +++ b/northd/en-advertised-route-sync.c
> @@ -386,7 +386,7 @@ publish_host_routes(struct hmap *sync_routes,
>  
>      struct ovn_port *port;
>      HMAP_FOR_EACH (port, dp_node, &peer_od->ports) {
> -        if (port->peer) {
> +        if (port->peer && port->peer->nbrp) {
>              /* This is a LSP connected to an LRP */
>              publish_host_routes_lrp(sync_routes, lr_stateful_table, route,
>                                      data, port->peer);
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index bee6ed7b1..714da6826 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -15507,6 +15507,21 @@ check_row_count Advertised_Route 6 
> tracked_port!='[[]]'
>  check_row_count Advertised_Route 1 datapath=$datapath logical_port=$lr0lr2 \
>      ip_prefix=10.10.0.20 tracked_port=$lr2lr0
>  
> +# Add a directly connected switch and make sure that northd is not confused.
> +# The IP on a switch-switch port should be advertised.
> +check ovn-nbctl ls-add sw1
> +check ovn-nbctl lsp-add sw0 sw0-sw1
> +check ovn-nbctl lsp-add sw1 sw1-sw0
> +check ovn-nbctl lsp-set-type sw0-sw1 switch peer=sw1-sw0 -- \
> +                lsp-set-type sw1-sw0 switch peer=sw0-sw1
> +check ovn-nbctl --wait=sb lsp-set-addresses sw0-sw1 \
> +    "11:aa:bb:cc:dd:ee 10.0.0.42"
> +sw0sw1=$(fetch_column port_binding _uuid logical_port=sw0-sw1)
> +check_row_count Advertised_Route 9
> +check_row_count Advertised_Route 7 tracked_port!='[[]]'
> +check_column $sw0sw1 Advertised_Route tracked_port \
> +    datapath=$datapath logical_port=$sw0 ip_prefix=10.0.0.42
> +
>  AT_CLEANUP
>  ])
>  
> -- 
> 2.47.0
> 
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to