Hi Ales, Thank you very much for the review!
On Mon, 20 Jul 2026 at 10:18, Ales Musil <[email protected]> wrote: > Should we also add a NEWS entry for this? > Yes, definitely. I completely missed adding it but it is surely worth noting this added capability. I have added it in the "Dynamic Routing" section since it seems to be the proper place, but I can move it outside as a normal entry if you prefer. On Mon, 20 Jul 2026 at 10:18, Ales Musil <[email protected]> wrote: > > + >> + return peer_iface; >> } >> >> static void >> @@ -275,11 +361,7 @@ route_run(struct route_ctx_in *r_ctx_in, >> route_get_table_id(ad->db)); >> } >> >> - if (!port_name) { >> - /* No port-name set, so we learn routes from all ports. >> */ >> - smap_add_nocopy(&ad->bound_ports, >> - xstrdup(local_peer->logical_port), NULL); >> - } else { >> + if (port_name) { >> /* If a port_name is set the we filter for the name as >> set in >> * the port-mapping or the interface name of the local >> * binding. If the port is not in the port_mappings and >> not >> @@ -292,6 +374,44 @@ route_run(struct route_ctx_in *r_ctx_in, >> ifname); >> } >> sset_add(r_ctx_out->filtered_ports, port_name); >> + } else { >> > > Could we avoid this deep nesting? > > >> + const char *redirect_port = smap_get(&repb->options, >> + >> "routing-protocol-redirect"); >> + if (redirect_port) { >> + /* routing-protocol-redirect points to a LSP. If >> that LSP >> + * is bound locally and connected through a veth >> pair, we >> + * can auto-discover its peer interface and use it to >> + * scope route learning, without requiring >> + * dynamic-routing-port-name/port-mapping to be >> manually >> + * configured. Track 'redirect_port' so that we >> + * recompute if its binding changes. */ >> + sset_add(r_ctx_out->filtered_ports, redirect_port); >> + >> + char *peer_iface = find_veth_peer_for_redirect_port( >> + r_ctx_in->local_bindings, r_ctx_in->chassis, >> + redirect_port); >> + if (peer_iface) { >> + /* Auto-discovery succeeded: this LRP now >> filters on >> + * a specific interface, just like an explicit >> + * dynamic-routing-port-name would. */ >> + lr_has_port_name_filter = true; >> + smap_add(&ad->bound_ports, >> local_peer->logical_port, >> + peer_iface); > > + free(peer_iface); >> + } else { >> + /* Auto-discovery failed (redirect port not bound >> + * locally, not a veth device, or its peer >> cannot be >> + * resolved): fall back to learning routes from >> all >> + * interfaces on this LRP. */ >> + smap_add_nocopy(&ad->bound_ports, >> + >> xstrdup(local_peer->logical_port), >> + NULL); > > + } >> + } else { >> + /* No port-name set, so we learn routes from all >> ports. */ >> + smap_add_nocopy(&ad->bound_ports, >> + xstrdup(local_peer->logical_port), >> NULL); >> + } >> > > This whole section can be greatly simplified with a few modifications: > I have reworked this section a little bit. To avoid nesting, I extracted the whole per-peer bound_ports decision into a new helper, which allows to make a single flat call instead of a four-level-deep if/else. The helper rework also includes the suggested collapsed success/failure branches inside route_add_bound_port(). Thank you for these two suggestions, I think I managed to make everything way tidier this way. I will send the v2 (including the other minor changes) very shortly. Best Regards, Matteo P.S. Do not worry about the name mispelling, it happens to me as well sometimes when I am typing the mail signature too quickly :) _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
