On 8/5/26 1:20 AM, Han Zhou wrote: > ovn-ic typically interconnects zones by attaching LRPs to a transit > switch. On that switch, remote LRPs appear as remote LSPs, so OVN floods > IP multicast to them via _MC_flood_l2. Multicast to real LSPs on the > transit switch is still desired, but flooding toward remote LRPs is > useless (they drop it) and scales poorly.
Hi Han, Thanks for the patch! > > Add LSP options:is_router so CMS can mark those remote LRP-backed ports. Would it make sense to call this "remote_router" or something like that? > Omit them from _MC_flood_l2 like type=router. This makes sense, but please see below for some concerns. > > Assisted-by: Claude Opus 5, Cursor > Signed-off-by: Han Zhou <[email protected]> > --- > Documentation/ref/ovn-logical-flows.7.rst | 9 ++-- > NEWS | 3 ++ > lib/mcast-group-index.h | 3 +- > northd/en-multicast.c | 5 +- > northd/northd.h | 11 ++++ > ovn-nb.xml | 14 +++++ > tests/ovn-northd.at | 65 +++++++++++++++++++++++ > 7 files changed, 105 insertions(+), 5 deletions(-) > > diff --git a/Documentation/ref/ovn-logical-flows.7.rst > b/Documentation/ref/ovn-logical-flows.7.rst > index 0ec69e39ee71..735e26ffd6f8 100644 > --- a/Documentation/ref/ovn-logical-flows.7.rst > +++ b/Documentation/ref/ovn-logical-flows.7.rst > @@ -1460,7 +1460,8 @@ This table implements switching behavior. It contains > these logical flows: > - Priority-90 flows for each IPv4 address/VIP/NAT address owned by a router > port connected to the switch. These flows match GARP packets for the > specific > IP addresses. Matched packets are forwarded to the ``MC_FLOOD_L2`` > multicast > - group which contains all non-router logical ports. > + group which contains all non-router logical ports (excluding ports of type > + ``router`` and ports with ``options:is_router=true``). > > - Priority-90 flows for transit switches that forward registered IP multicast > traffic to their corresponding multicast group , which ``ovn-northd`` > creates > @@ -1475,7 +1476,8 @@ This table implements switching behavior. It contains > these logical flows: > > - A priority-85 flow that forwards all IP multicast traffic destined to > 224.0.0.X to the ``MC_FLOOD_L2`` multicast group, which ``ovn-northd`` > - populates with all non-router logical ports. > + populates with all non-router logical ports (excluding ports of type > + ``router`` and ports with ``options:is_router=true``). > > - A priority-85 flow that forwards all IP multicast traffic destined to > reserved > multicast IPv6 addresses (RFC 4291, 2.7.1, e.g., Solicited-Node multicast) > to > @@ -1503,7 +1505,8 @@ This table implements switching behavior. It contains > these logical flows: > > - Priority-75 flows for each port connected to a logical router matching self > originated ARP request/RARP request/ND packets. These packets are flooded > to > - the ``MC_FLOOD_L2`` which contains all non-router logical ports. > + the ``MC_FLOOD_L2`` which contains all non-router logical ports (excluding > + ports of type ``router`` and ports with ``options:is_router=true``). > > - A priority-72 flow that outputs all ND NA (Neighbor Advertisement), ND RS > (Router Solicitation) and ND RA (Router Advertisement) packets with an > diff --git a/NEWS b/NEWS > index 44f117807af5..4d466251b397 100644 > --- a/NEWS > +++ b/NEWS > @@ -94,6 +94,9 @@ Post v26.03.0 > - Added a new "ovn-debug lflow-pipeline-oftable-start-list" command that > prints the starting OpenFlow table number of the logical ingress and > egress pipelines. > + - northd: Add Logical_Switch_Port option "is_router" to mark an LSP as > + representing a logical router port when type is not "router" (e.g. IC > + remote LSPs). Currently used to omit such ports from _MC_flood_l2. > > OVN v26.03.0 - xxx xx xxxx > -------------------------- > diff --git a/lib/mcast-group-index.h b/lib/mcast-group-index.h > index 9664a94dd675..122ab65a5902 100644 > --- a/lib/mcast-group-index.h > +++ b/lib/mcast-group-index.h > @@ -43,7 +43,8 @@ enum ovn_mcast_tunnel_keys { > */ > OVN_MCAST_FLOOD_L2_TUNNEL_KEY, /* Logical switch broadcast domain > * excluding ports towards logical > - * routers. > + * routers (type=router or > + * options:is_router=true). > */ > OVN_MIN_IP_MULTICAST, > OVN_MAX_IP_MULTICAST = OVN_MAX_MULTICAST, > diff --git a/northd/en-multicast.c b/northd/en-multicast.c > index 5148d88408ae..049afb3fcc38 100644 > --- a/northd/en-multicast.c > +++ b/northd/en-multicast.c > @@ -233,7 +233,10 @@ build_mcast_groups(struct multicast_igmp_data *data, > if (lsp_is_enabled(op->nbsp) && lsp_can_receive_multicast(op->nbsp)) > { > ovn_multicast_add(&data->mcast_groups, &mc_flood, op); > > - if (!lsp_is_router(op->nbsp)) { > + /* Omit type=router ports and ports with options:is_router=true > + * from MC_FLOOD_L2 (same treatment as real router ports). > + */ > + if (!lsp_is_router_for_mcast(op->nbsp)) { > ovn_multicast_add(&data->mcast_groups, &mc_flood_l2, op); > } > > diff --git a/northd/northd.h b/northd/northd.h > index d27f519d6e33..9385015f7a13 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -1155,6 +1155,17 @@ lsp_can_learn_mac(const struct > nbrec_logical_switch_port *nbsp) > return smap_get_bool( ->options, "lsp_learn_fdb", true); > } > > +/* True for type=router ports, and for ports marked options:is_router=true > + * (LSP represents an LRP even if type is not "router"). Currently used to > + * omit such ports from MC_FLOOD_L2. > + */ > +static inline bool > +lsp_is_router_for_mcast(const struct nbrec_logical_switch_port *nbsp) > +{ > + return lsp_is_router(nbsp) > + || smap_get_bool( ->options, "is_router", false); > +} To be honest, this seems a bit short sighted to me. As you mentioned in the commit log the "remote" transit switch port is actually a port connecting to a remote router. So logically there's no real difference between these ports and the ones lsp_is_router() returns true for, right? Instead of this very specific helper, shouldn't we just update lsp_is_router() to also check the nbsp option if nbsp->type == "remote"? > + > const char *lrp_find_member_ip(const struct ovn_port *op, const char *ip_s); > > /* This function returns true if 'op' is a gateway router port. > diff --git a/ovn-nb.xml b/ovn-nb.xml > index 8a9d19fa9214..43568ea7c911 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -1795,6 +1795,20 @@ > </column> > </group> > > + <group title="Options for ports representing routers"> > + <column name="options" key="is_router" > + type='{"type": "boolean"}'> > + If set to <code>true</code>, indicate that this logical switch port > + represents a logical router port, even when > + <ref column="type"/> is not <code>router</code>. CMS may set this > + on ports such as <code>remote</code> LSPs that back LRPs in another > + OVN interconnection zone. OVN may use this hint wherever router > + ports are handled specially; for example, such ports are omitted > + from the <code>_MC_flood_l2</code> multicast group so IP multicast > + is not flooded toward them. Default: <code>false</code>. > + </column> > + </group> > + > </group> > > <group title="Containers"> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index d3d9de3b3ce5..54c0eeae1990 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -8745,6 +8745,71 @@ OVN_CLEANUP_NORTHD > AT_CLEANUP > ]) > > +OVN_FOR_EACH_NORTHD_NO_HV([ > +AT_SETUP([LSP options:is_router excludes from MC_FLOOD_L2]) > +ovn_start > + > +check ovn-nbctl ls-add ls1 > +check ovn-nbctl lsp-add ls1 vif1 > +check ovn-nbctl lsp-set-addresses vif1 "00:00:00:00:00:01 10.0.0.1" > +check ovn-nbctl lsp-add ls1 remote1 > +check ovn-nbctl lsp-set-type remote1 remote > +check ovn-nbctl lsp-set-addresses remote1 "00:00:00:00:00:02" > +check ovn-nbctl lr-add lr1 > +check ovn-nbctl lrp-add lr1 lr1-ls1 00:00:00:00:00:03 10.0.0.254/24 > +check ovn-nbctl lsp-add ls1 ls1-lr1 > +check ovn-nbctl lsp-set-type ls1-lr1 router > +check ovn-nbctl lsp-set-addresses ls1-lr1 router > +check ovn-nbctl lsp-set-options ls1-lr1 router-port=lr1-ls1 > +check ovn-nbctl --wait=sb sync > + > +vif1_uuid=$(fetch_column Port_Binding _uuid logical_port=vif1) > +remote1_uuid=$(fetch_column Port_Binding _uuid logical_port=remote1) > +lr1_uuid=$(fetch_column Port_Binding _uuid logical_port=ls1-lr1) > + > +dnl type=router is never in _MC_flood_l2. > +AT_CHECK([ovn-sbctl --bare --columns=ports find Multicast_Group \ > + name=_MC_flood_l2 | grep -c "$lr1_uuid"], [1], [dnl > +0 > +]) > + > +dnl remote LSP is in _MC_flood_l2 by default (looks like a normal L2 port). > +AT_CHECK([ovn-sbctl --bare --columns=ports find Multicast_Group \ > + name=_MC_flood_l2 | grep -c "$remote1_uuid"], [0], [dnl > +1 > +]) > +AT_CHECK([ovn-sbctl --bare --columns=ports find Multicast_Group \ > + name=_MC_flood_l2 | grep -c "$vif1_uuid"], [0], [dnl > +1 > +]) > + > +dnl Mark remote LSP as router-facing: omit from _MC_flood_l2. > +check ovn-nbctl --wait=sb set Logical_Switch_Port remote1 > options:is_router=true > +AT_CHECK([ovn-sbctl --bare --columns=ports find Multicast_Group \ > + name=_MC_flood_l2 | grep -c "$remote1_uuid"], [1], [dnl > +0 > +]) > +AT_CHECK([ovn-sbctl --bare --columns=ports find Multicast_Group \ > + name=_MC_flood_l2 | grep -c "$vif1_uuid"], [0], [dnl > +1 > +]) > + > +dnl Still a member of _MC_flood (ARP/ND flood domain). > +AT_CHECK([ovn-sbctl --bare --columns=ports find Multicast_Group \ > + name=_MC_flood | grep -c "$remote1_uuid"], [0], [dnl > +1 > +]) > + > +dnl Clearing the option restores _MC_flood_l2 membership. > +check ovn-nbctl --wait=sb remove Logical_Switch_Port remote1 options > is_router > +AT_CHECK([ovn-sbctl --bare --columns=ports find Multicast_Group \ > + name=_MC_flood_l2 | grep -c "$remote1_uuid"], [0], [dnl > +1 > +]) > + Missing OVN_CLEANUP_NORTHD. > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD_NO_HV_PARALLELIZATION([ > AT_SETUP([ACLs after lb]) > AT_KEYWORDS([acl]) Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
