Hi Lorenzo,

As noted on the cover letter for this series, this patch is acked by
me. However, I have one note below:

On Tue, May 26, 2026 at 2:51 AM Lorenzo Bianconi via dev
<[email protected]> wrote:
>
> Introduce the capability to enable/disable the advertisement of a
> specific EVPN MAC/IP binding configuring the LSP dynamic-routing-advertise
> option. This parameter is relavant if the CMS configures
> dynamic-routing-redistribute option to fdb,ip in the Logical_Switch
> other_config column.
>
> Reported-at: https://redhat.atlassian.net/browse/FDP-2743
> Signed-off-by: Lorenzo Bianconi <[email protected]>
> ---
>  controller/neighbor.c             |  4 ++++
>  northd/en-advertised-route-sync.c |  5 +++++
>  northd/northd.c                   |  8 ++++++++
>  ovn-sb.xml                        |  9 +++++++++
>  tests/system-ovn.at               | 23 +++++++++++++++++++++++
>  5 files changed, 49 insertions(+)
>
> diff --git a/controller/neighbor.c b/controller/neighbor.c
> index 57df1e90c..72fabe205 100644
> --- a/controller/neighbor.c
> +++ b/controller/neighbor.c
> @@ -280,6 +280,10 @@ neighbor_collect_mac_to_advertise(const struct 
> neighbor_ctx_in *n_ctx_in,
>              continue;
>          }
>
> +        if (!smap_get_bool(&pb->options, "dynamic-routing-advertise", true)) 
> {
> +            continue;
> +        }
> +
>          for (size_t i = 0; i < pb->n_mac; i++) {
>              struct lport_addresses addresses;
>              if (!extract_lsp_addresses(pb->mac[i], &addresses)) {
> diff --git a/northd/en-advertised-route-sync.c 
> b/northd/en-advertised-route-sync.c
> index adccfd5e3..4a8d13232 100644
> --- a/northd/en-advertised-route-sync.c
> +++ b/northd/en-advertised-route-sync.c
> @@ -939,6 +939,11 @@ build_advertised_mac_binding(const struct ovn_datapath 
> *od, struct hmap *map)
>              continue;
>          }
>
> +        if (!smap_get_bool(&op->nbsp->options,
> +                           "dynamic-routing-advertise", true)) {
> +            continue;
> +        }
> +
>          if (lsp_is_router(op->nbsp) && op->peer) {
>              advertised_mac_binding_add(map, od->sdp->sb_dp, op->sb,
>                                         &op->peer->lrp_networks);
> diff --git a/northd/northd.c b/northd/northd.c
> index b9de69fae..2d1483948 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -2907,6 +2907,14 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn,
>                  smap_add(&options, "peer", op->peer->key);
>              }
>
> +            const char *dynamic_routing_advertise =
> +                smap_get(&op->nbsp->options, "dynamic-routing-advertise");
> +            if (dynamic_routing_advertise) {
> +                smap_add(&options, "dynamic-routing-advertise",
> +                         !strcasecmp(dynamic_routing_advertise, "false")
> +                         ? "false" : "true");
> +            }
> +

The entire block above can be deleted. The "options" smap is cloned
from op->nbsp->options when it is initialized. Since the option name
is the same in the NB Logical_Switch_Port and the SB Port_Binding, and
they have the same default values, you don't have to explicitly set
the value. It will automatically be copied.

>              sbrec_port_binding_set_options(op->sb, &options);
>              smap_destroy(&options);
>              if (lsp_is_switch(op->nbsp)) {
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index 46568da62..d839c7088 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -3889,6 +3889,15 @@ tcp.flags = RST;
>          <code>queue_id</code> used in OpenFlow in <code>struct
>          ofp_action_enqueue</code>.
>        </column>
> +
> +      <column name="options" key="dynamic-routing-advertise">
> +        If the CSM set <ref column="other_config"
> +        key="dynamic-routing-redistribute" table="Logical_Switch"/> to
> +        <code>fdb</code> and/or <code>ip</code>, this parameter is used
> +        by the CMS to enable/disable the advertisement of LSP IPs/MAC 
> binding.
> +        Default: <code>true</code>.
> +      </column>
> +
>      </group>
>
>      <group title="Distributed Gateway Port Options">
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 649e85ec1..f7ecab7d8 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -18491,6 +18491,22 @@ f0:00:0f:16:01:20 dev $LO_NAME master $BR_NAME static
>  f0:00:0f:16:01:20 dev $LO_NAME vlan 1 master $BR_NAME static
>  ])
>
> +ls_evpn_w1_uuid=$(fetch_column nb:Logical_Switch_Port _uuid name="workload1")
> +check ovn-nbctl --wait=hv set Logical_Switch_Port $ls_evpn_w1_uuid 
> options:dynamic-routing-advertise=false
> +ls_evpn_w2_uuid=$(fetch_column nb:Logical_Switch_Port _uuid name="workload2")
> +check ovn-nbctl --wait=hv set Logical_Switch_Port $ls_evpn_w2_uuid 
> options:dynamic-routing-advertise=false
> +OVS_WAIT_FOR_OUTPUT_UNQUOTED([bridge fdb show | grep $LO_NAME | grep 
> "f0:00:0f:16:01" | sort], [0], [dnl
> +])
> +
> +check ovn-nbctl --wait=hv set Logical_Switch_Port $ls_evpn_w1_uuid 
> options:dynamic-routing-advertise=true
> +check ovn-nbctl --wait=hv set Logical_Switch_Port $ls_evpn_w2_uuid 
> options:dynamic-routing-advertise=true
> +OVS_WAIT_FOR_OUTPUT_UNQUOTED([bridge fdb show | grep $LO_NAME | grep 
> "f0:00:0f:16:01" | sort], [0], [dnl
> +f0:00:0f:16:01:10 dev $LO_NAME master $BR_NAME static
> +f0:00:0f:16:01:10 dev $LO_NAME vlan 1 master $BR_NAME static
> +f0:00:0f:16:01:20 dev $LO_NAME master $BR_NAME static
> +f0:00:0f:16:01:20 dev $LO_NAME vlan 1 master $BR_NAME static
> +])
> +
>  check ovn-nbctl --wait=hv lsp-del workload2
>  OVS_WAIT_FOR_OUTPUT_UNQUOTED([bridge fdb show | grep $LO_NAME | grep 
> "f0:00:0f:16:01" | sort], [0], [dnl
>  f0:00:0f:16:01:10 dev $LO_NAME master $BR_NAME static
> @@ -18737,6 +18753,13 @@ AT_CHECK([ip neigh show dev $BR_NAME nud noarp | 
> grep -q '172.16.1.1 lladdr f0:0
>  AT_CHECK([ip -6 neigh show dev $BR_NAME nud noarp | grep -q '172:16::10 
> lladdr f0:00:0f:16:01:10 NOARP'])
>  AT_CHECK([ip -6 neigh show dev $BR_NAME nud noarp | grep -q '172:16::1 
> lladdr f0:00:0f:16:01:01 NOARP'])
>
> +check ovn-nbctl --wait=hv set Logical_Switch_Port $ls_evpn_w1_uuid 
> options:dynamic-routing-advertise=false
> +AT_CHECK([ip neigh show dev $BR_NAME nud noarp | grep -q '172.16.1.10 lladdr 
> f0:00:0f:16:01:10 NOARP'], [1])
> +AT_CHECK([ip -6 neigh show dev $BR_NAME nud noarp | grep -q '172:16::10 
> lladdr f0:00:0f:16:01:10 NOARP'], [1])
> +check ovn-nbctl --wait=hv set Logical_Switch_Port $ls_evpn_w1_uuid 
> options:dynamic-routing-advertise=true
> +AT_CHECK([ip neigh show dev $BR_NAME nud noarp | grep -q '172.16.1.10 lladdr 
> f0:00:0f:16:01:10 NOARP'])
> +AT_CHECK([ip -6 neigh show dev $BR_NAME nud noarp | grep -q '172:16::10 
> lladdr f0:00:0f:16:01:10 NOARP'])
> +
>  OVS_WAIT_FOR_OUTPUT_UNQUOTED([bridge fdb show | grep $LO_NAME | grep 
> "f0:00:0f:16:01" | sort], [0], [dnl
>  f0:00:0f:16:01:01 dev $LO_NAME master $BR_NAME static
>  f0:00:0f:16:01:01 dev $LO_NAME vlan 1 master $BR_NAME static
> --
> 2.54.0
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to