On Thu, Aug 28, 2025 at 12:24 AM Dumitru Ceara <dce...@redhat.com> wrote:
> 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
>
>
Thank you Dumitru,
I have changed the test as suggested, merged this into main and backported
to 25.03.
Regards,
Ales
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
