> From: Numan Siddique <num...@ovn.org>
>
> If this option is set on the logical switch port of type router,
> then the IPs of its peer router port is excluded from the
> nat_addresses so that ovn-controller doesn't send the GARPs
> for these IPs.
>
> This option will be useful for the deployments where the SNAT ips
> configured on the routers is different from the router port IP
> and no external entity wants to reach to the router port IPs.
> In a highly scaled environment, this reduces the load on the
> pinctrl thread.
>
> CMS should not set this option if the router port IPs are also
> used for SNAT.
>
> Signed-off-by: Numan Siddique <num...@ovn.org>
Hi Numan,
the patch is fine for me, just a small nit inline.
Regards,
Lorenzo
Acked-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com>
> ---
> NEWS | 2 +
> northd/northd.c | 113 ++++++++++++++++++++++++++++----------------
> ovn-nb.xml | 9 ++++
> tests/ovn-northd.at | 33 +++++++++++++
> 4 files changed, 115 insertions(+), 42 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index f7813184ed..a84495c351 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -17,6 +17,8 @@ Post v25.03.0
> - SSL/TLS:
> * Support for deprecated TLSv1 and TLSv1.1 protocols on OpenFlow and
> database connections is now removed.
> + - Introduce exclude-router-ips-from-garp in logical_switch_port column
> + so that the router port IPs are not advertised in the GARPs.
>
> OVN v25.03.0 - 07 Mar 2025
> --------------------------
> diff --git a/northd/northd.c b/northd/northd.c
> index 1f9340e555..74792e38bf 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -4036,6 +4036,58 @@ build_lb_port_related_data(
> build_lswitch_lbs_from_lrouter(lr_datapaths, lb_dps_map,
> lb_group_dps_map);
> }
>
> +/* Returns true if the peer port IPs of op should be added in the
> nat_addresses
> + * column of Port_Binding of op or not.
> + *
> + * Returns false, if the options:exclude-router-ips-from-garp is set to true.
> + *
> + * Otherwise, return true if:
> + * - op->peer has 'reside-on-redirect-chassis' set and the
> + * the logical router datapath has distributed router port.
> + *
> + * - op->peer is distributed gateway router port.
> + *
> + * - op->peer's router is a gateway router and op has a localnet
> + * port.
> + *
> + * else, false.
> + */
> +static bool
> +should_add_router_port_garp(const struct ovn_port *op, const char *chassis)
> +{
> + if (smap_get_bool(&op->nbsp->options, "exclude-router-ips-from-garp",
> + false)) {
> + return false;
> + }
> +
> + bool add_router_port_garp = false;
> + if (op->peer && op->peer->nbrp && op->peer->od->n_l3dgw_ports) {
> + if (lrp_is_l3dgw(op->peer)) {
> + add_router_port_garp = true;
> + } else if (smap_get_bool(&op->peer->nbrp->options,
> + "reside-on-redirect-chassis", false)) {
> + if (op->peer->od->n_l3dgw_ports == 1) {
> + add_router_port_garp = true;
> + } else {
> + static struct vlog_rate_limit rl =
> + VLOG_RATE_LIMIT_INIT(1, 1);
> + VLOG_WARN_RL(&rl, "\"reside-on-redirect-chassis\" is "
> + "set on logical router port %s, which "
> + "is on logical router %s, which has %"
> + PRIuSIZE" distributed gateway ports. This"
> + "option can only be used when there is "
> + "a single distributed gateway port.",
> + op->peer->key, op->peer->od->nbr->name,
> + op->peer->od->n_l3dgw_ports);
> + }
> + }
> + } else if (chassis && op->od->n_localnet_ports) {
> + add_router_port_garp = true;
> + }
> +
> + return add_router_port_garp;
> +}
> +
> /* Syncs the SB port binding for the ovn_port 'op' of a logical switch port.
> * Caller should make sure that the OVN SB IDL txn is not NULL. Presently it
> * only syncs the nat column of port binding corresponding to the 'op->nbsp'
> */
> @@ -4098,47 +4150,24 @@ sync_pb_for_lsp(struct ovn_port *op,
> }
>
> /* Add the router mac and IPv4 addresses to
> - * Port_Binding.nat_addresses so that GARP is sent for these
> - * IPs by the ovn-controller on which the distributed gateway
> - * router port resides if:
> - *
> - * - op->peer has 'reside-on-redirect-chassis' set and the
> - * the logical router datapath has distributed router port.
> - *
> - * - op->peer is distributed gateway router port.
> - *
> - * - op->peer's router is a gateway router and op has a localnet
> - * port.
> - *
> - * Note: Port_Binding.nat_addresses column is also used for
> - * sending the GARPs for the router port IPs.
> - * */
> - bool add_router_port_garp = false;
> - if (op->peer && op->peer->nbrp && op->peer->od->n_l3dgw_ports) {
> - if (lrp_is_l3dgw(op->peer)) {
> - add_router_port_garp = true;
> - } else if (smap_get_bool(&op->peer->nbrp->options,
> - "reside-on-redirect-chassis", false)) {
> - if (op->peer->od->n_l3dgw_ports == 1) {
> - add_router_port_garp = true;
> - } else {
> - static struct vlog_rate_limit rl =
> - VLOG_RATE_LIMIT_INIT(1, 1);
> - VLOG_WARN_RL(&rl, "\"reside-on-redirect-chassis\" is "
> - "set on logical router port %s, which "
> - "is on logical router %s, which has %"
> - PRIuSIZE" distributed gateway ports.
> This"
> - "option can only be used when there is "
> - "a single distributed gateway port.",
> - op->peer->key, op->peer->od->nbr->name,
> - op->peer->od->n_l3dgw_ports);
> - }
> - }
> - } else if (chassis && op->od->n_localnet_ports) {
> - add_router_port_garp = true;
> - }
> -
> - if (add_router_port_garp) {
> + * Port_Binding.nat_addresses so that GARP is sent for these
> + * IPs by the ovn-controller on which the distributed gateway
> + * router port resides if:
> + *
> + * - options:exclude-router-ips-from-garp is not set to true for op.
> + *
> + * - op->peer has 'reside-on-redirect-chassis' set and the
> + * the logical router datapath has distributed router port.
> + *
> + * - op->peer is distributed gateway router port.
> + *
> + * - op->peer's router is a gateway router and op has a localnet
> + * port.
> + *
> + * Note: Port_Binding.nat_addresses column is also used for
> + * sending the GARPs for the router port IPs.
> + * */
> + if (should_add_router_port_garp(op, chassis)) {
> struct ds garp_info = DS_EMPTY_INITIALIZER;
> ds_put_format(&garp_info, "%s", op->peer->lrp_networks.ea_s);
>
> @@ -4163,7 +4192,7 @@ sync_pb_for_lsp(struct ovn_port *op,
> ds_destroy(&garp_info);
> }
> sbrec_port_binding_set_nat_addresses(op->sb,
> - (const char **) nats,
> n_nats);
> + (const char **) nats, n_nats);
> for (size_t i = 0; i < n_nats; i++) {
> free(nats[i]);
> }
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index bf1f1628bd..054f8f530e 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -1223,6 +1223,15 @@
> not cosidering configured load balancers.
> </column>
>
> + <column name="options" key="exclude-router-ips-from-garp">
> + If <ref column="options" key="nat-addresses"/> is set to
> + <code>router</code>, Gratuitous ARPs will not be sent for
> + router port IP addresses and SNAT IP addresses (if SNAT IP
> + is same as the router port IP) defined on its peer router
> + port. Do not set this option if the router port IPs are
> + also used as SNAT IPs.
> + </column>
I guess this is true even if the router IP is used for DNAT, right?
> +
> <column name="options" key="arp_proxy">
> Optional. A list of MAC and addresses/cidrs or just addresses/cidrs
> that this logical switch <code>router</code> port will reply to
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 12d6611b69..55c7edf036 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -7719,6 +7719,39 @@ AT_CHECK([ovn-sbctl get Port_Binding S1-R1
> nat_addresses |grep -q 172.16.1.10],
> AT_CLEANUP
> ])
>
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([check exclude-router-ips-from-garp option])
> +ovn_start
> +
> +check ovn-sbctl chassis-add gw1 geneve 127.0.0.1
> +
> +check ovn-nbctl lr-add R1
> +check ovn-nbctl lrp-add R1 R1-S1 02:ac:10:01:00:01 172.16.1.1/24
> +check ovn-nbctl lrp-set-gateway-chassis R1-S1 gw1
> +check ovn-nbctl ls-add S1
> +check ovn-nbctl lsp-add S1 S1-R1
> +check ovn-nbctl lsp-set-type S1-R1 router
> +check ovn-nbctl lsp-set-addresses S1-R1 02:ac:10:01:00:01
> +check ovn-nbctl --wait=sb lsp-set-options S1-R1 router-port=R1-S1
> nat-addresses="router"
> +
> +check ovn-nbctl lr-nat-add R1 snat 172.16.1.1 10.0.0.0/24
> +check ovn-nbctl lr-nat-add R1 dnat 172.16.1.2 10.0.0.1
> +check ovn-nbctl --wait=sb sync
> +
> +AT_CHECK([ovn-sbctl get Port_Binding S1-R1 nat_addresses | grep -q
> 172.16.1.1], [0])
> +
> +check ovn-nbctl --wait=sb lsp-set-options S1-R1 router-port=R1-S1
> nat-addresses="router" \
> + exclude-router-ips-from-garp="true"
> +AT_CHECK([ovn-sbctl get Port_Binding S1-R1 nat_addresses | grep -q
> 172.16.1.1], [1])
> +
> +check ovn-nbctl clear logical_router R1 nat
> +check ovn-nbctl --wait=sb lr-nat-add R1 snat 172.16.1.4 10.0.0.0/24
> +AT_CHECK([ovn-sbctl get Port_Binding S1-R1 nat_addresses | grep -q
> 172.16.1.1], [1])
> +AT_CHECK([ovn-sbctl get Port_Binding S1-R1 nat_addresses | grep -q
> 172.16.1.4], [0])
> +
> +AT_CLEANUP
> +])
> +
> OVN_FOR_EACH_NORTHD_NO_HV([
> AT_SETUP([check broadcast-arps-to-all-routers option])
> ovn_start
> --
> 2.49.0
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
