On 8/27/25 3:20 PM, Ales Musil wrote:
> The update of logical port or IP in the NB wouldn't be properly
> reflected in the SB entry. Make sure we check and update
> those columns too if needed.
> 
> Fixes: 42ffb62a5392 ("northd: Use the same UUID for SB representation of 
> Static_Mac_Binding.")
> Reported-by: Ilya Maximets <i.maxim...@ovn.org>
> Signed-off-by: Ales Musil <amu...@redhat.com>
> ---

Hi Ales,

I guess it depends a bit on the outcome of the discussion that's going
on in
https://patchwork.ozlabs.org/project/ovn/patch/20250826162634.476387-1-amu...@redhat.com/
whether we'll also have to update the SB.MAC_Binding.datapath but if
that's the case we can do that with a separate patch.

I only have two nits on the test and those can be fixed up at apply time:

Acked-by: Dumitru Ceara <dce...@redhat.com>

>  northd/northd.c     | 10 ++++++++++
>  tests/ovn-northd.at |  9 +++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/northd/northd.c b/northd/northd.c
> index 015f30a35..7b524e958 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -18900,6 +18900,16 @@ build_static_mac_binding_table(
>                      if (strcmp(mb->mac, nb_smb->mac)) {
>                          sbrec_static_mac_binding_set_mac(mb, nb_smb->mac);
>                      }
> +
> +                    if (strcmp(mb->logical_port, nb_smb->logical_port)) {
> +                        sbrec_static_mac_binding_set_logical_port(mb,
> +                            nb_smb->logical_port);
> +                    }
> +
> +                    if (strcmp(mb->ip, nb_smb->ip)) {
> +                        sbrec_static_mac_binding_set_ip(mb, nb_smb->ip);
> +                    }
> +
>                      if (mb->override_dynamic_mac !=
>                          nb_smb->override_dynamic_mac) {
>                          sbrec_static_mac_binding_set_override_dynamic_mac(mb,
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 11bbb211d..28282f0ca 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -8505,6 +8505,15 @@ wait_row_count Static_MAC_Binding 1 
> logical_port=lr0-p1 ip=10.0.0.10 mac="00\:00
>  check ovn-nbctl --may-exist static-mac-binding-add lr0-p0 192.168.10.100 
> 00:00:22:33:55:66
>  wait_row_count Static_MAC_Binding 1 logical_port=lr0-p0 ip=192.168.10.100 
> mac="00\:00\:22\:33\:55\:66"
>  
> +# Update the logical port.
> +mb_uuid=$(fetch_column nb:Static_MAC_Binding _uuid logical_port=lr0-p0 
> ip=192.168.10.100)
> +check ovn-nbctl --wait=sb set Static_MAC_Binding $mb_uuid logical_port=lr0-p1
> +wait_row_count Static_MAC_Binding 1 logical_port=lr0-p1 ip=192.168.10.100 
> mac="00\:00\:22\:33\:55\:66"

Nit: I think we should either remove --wait=sb or use check_row_count.
I'd prefer the latter as it might be better at detecting potential
--wait=sb unintended semantics changes.

> +
> +# Update the IP.
> +check ovn-nbctl --wait=sb set Static_MAC_Binding $mb_uuid ip=192.168.10.200
> +wait_row_count Static_MAC_Binding 1 logical_port=lr0-p1 ip=192.168.10.200 
> mac="00\:00\:22\:33\:55\:66"

Same here.

> +
>  AT_CLEANUP
>  ])
>  

Regards,
Dumitru

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to