On Tue, Aug 26, 2025 at 12:38 PM Ales Musil <amu...@redhat.com> wrote:
> The SB MAC_Binding and Static_MAC_Binding use the same columns
> as an indication of logical port and datapath. However, the check
> if any of those rows is stale wasn't consistent. Unify it in a single
> function to make sure that both tables have the same condition to
> identify stale entries.
>
> Fixes: 417f1415b2f5 ("northd: Clean up SB MAC bindings for deleted ports.")
> Fixes: 6856adc616a7 ("ovn-northd: Clear SB records depending on stale
> datapaths.")
>
I forgot to add Reported-at: https://issues.redhat.com/browse/FDP-1623,
I'll add it later in v2 or during marge depending on the reviews.
> Signed-off-by: Ales Musil <amu...@redhat.com>
> ---
> northd/northd.c | 35 ++++++++++++++++++++++++++---------
> tests/ovn-northd.at | 8 ++++++++
> 2 files changed, 34 insertions(+), 9 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 015f30a35..f9f132386 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -2867,6 +2867,26 @@ common:
> }
> }
>
> +static bool
> +mac_binding_is_stale(const struct hmap *lr_datapaths,
> + const struct hmap *lr_ports,
> + const struct sbrec_datapath_binding *dp,
> + const char *logical_port)
> +{
> + const struct ovn_datapath *od =
> + ovn_datapath_from_sbrec(NULL, lr_datapaths, dp);
> + if (!od || ovn_datapath_is_stale(od)) {
> + return true;
> + }
> +
> + const struct ovn_port *op = ovn_port_find(lr_ports, logical_port);
> + if (!op || !op->nbrp) {
> + return true;
> + }
> +
> + return false;
> +}
> +
> /* Remove mac_binding entries that refer to logical_ports which are
> * deleted. */
> static void
> @@ -2876,11 +2896,8 @@ cleanup_mac_bindings(
> {
> const struct sbrec_mac_binding *b;
> SBREC_MAC_BINDING_TABLE_FOR_EACH_SAFE (b, sbrec_mac_binding_table) {
> - const struct ovn_datapath *od =
> - ovn_datapath_from_sbrec(NULL, lr_datapaths, b->datapath);
> -
> - if (!od || ovn_datapath_is_stale(od) ||
> - !ovn_port_find(lr_ports, b->logical_port)) {
> + if (mac_binding_is_stale(lr_datapaths, lr_ports, b->datapath,
> + b->logical_port)) {
> sbrec_mac_binding_delete(b);
> }
> }
> @@ -18850,7 +18867,7 @@ build_static_mac_binding_table(
> struct ovsdb_idl_txn *ovnsb_txn,
> const struct nbrec_static_mac_binding_table *nbrec_static_mb_table,
> const struct sbrec_static_mac_binding_table *sbrec_static_mb_table,
> - struct hmap *lr_ports)
> + const struct hmap *lr_datapaths, const struct hmap *lr_ports)
> {
> /* Cleanup SB Static_MAC_Binding entries which do not have
> corresponding
> * NB Static_MAC_Binding entries, and SB Static_MAC_Binding entries
> for
> @@ -18866,8 +18883,8 @@ build_static_mac_binding_table(
> continue;
> }
>
> - struct ovn_port *op = ovn_port_find(lr_ports,
> nb_smb->logical_port);
> - if (!op || !op->nbrp || !op->od || !op->od->sdp->sb_dp) {
> + if (mac_binding_is_stale(lr_datapaths, lr_ports, sb_smb->datapath,
> + sb_smb->logical_port)) {
> sbrec_static_mac_binding_delete(sb_smb);
> }
> }
> @@ -19187,7 +19204,7 @@ ovnnb_db_run(struct northd_input *input_data,
> build_static_mac_binding_table(ovnsb_txn,
> input_data->nbrec_static_mac_binding_table,
> input_data->sbrec_static_mac_binding_table,
> - &data->lr_ports);
> + &data->lr_datapaths.datapaths, &data->lr_ports);
> stopwatch_stop(BUILD_LFLOWS_CTX_STOPWATCH_NAME, time_msec());
> stopwatch_start(CLEAR_LFLOWS_CTX_STOPWATCH_NAME, time_msec());
>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 11bbb211d..1a4910924 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -8518,6 +8518,14 @@ wait_row_count sb:Static_MAC_Binding 1
> logical_port=lr-lrp ip=1.1.1.2 mac="00\:0
> check ovn-nbctl lr-del lr
> wait_row_count sb:Static_MAC_Binding 0
>
> +dnl Also check for router (and router port) being recreated in the same
> +dnl transaction.
> +check ovn-nbctl lr-add lr -- lrp-add lr lr-lrp 00:00:00:00:00:01
> 1.1.1.1/24
> +wait_row_count sb:Static_MAC_Binding 1 logical_port=lr-lrp ip=1.1.1.2
> mac="00\:00\:00\:00\:00\:02"
> +check ovn-nbctl lrp-del lr-lrp -- lr-del lr -- lr-add lr -- lrp-add lr
> lr-lrp 00:00:00:00:00:01 1.1.1.1/24
> +wait_row_count sb:Static_MAC_Binding 1
> +check ovn-nbctl --wait=sb sync
> +
> AT_CLEANUP
> ])
>
> --
> 2.50.1
>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
