On 8/26/25 5:01 PM, Dumitru Ceara wrote: > Hi Ilya, Ales, > > Thanks for the discussion! > > On 8/26/25 3:59 PM, Ilya Maximets wrote: >> On 8/26/25 3:23 PM, Ales Musil wrote: >>> >>> >>> On Tue, Aug 26, 2025 at 2:37 PM Ilya Maximets <[email protected] >>> <mailto:[email protected]>> wrote: >>> >>> On 8/26/25 12:38 PM, Ales Musil 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. >>> >>> Hi, Ales. Thanks for the fix! >>> >>> >>> Hi Ilya, >>> thank you for the review. >>> >>> >>> >>> The commit message is focusing on entirely wrong thing. Reading it >>> it sounds like just a refactoring patch, which is not the case. >>> Please, describe what the actual issue is and which condition is >>> causing it. >>> >>> >>> It's actually focusing on the right thing. I can add more details >>> on the scenario when that happens, but it's still unification of >>> the logic. >>> > > +1 for more details. I might have had more insider context on the issue > so the commit log seemed clear enough for me but I can understand how it > might not be as clear for others. > >>> >>> >>> I'm also not sure if we need to combine the checks for both types >>> of mac bindings as it will complicate the code and make it more >>> error prone, see below. >>> >>> >>> I disagree, the issue is caused by a difference between how we remove >>> Static MAC Bindings and MAC Bindings. There shouldn't be any difference >>> whatsoever. >> >> The dynamic MAC bindings are created by ovn-controller, while the static >> ones are coming from the user and SB entries are created by northd. >> The logic for creation of these entries is different. And the logic of >> when they should be removed is also different. Dynamic entries must be >> cleaned up when the datapath goes away, because it's the controller that >> will need to re-create them. Static ones can just be updated without any >> logical issues. So, I don't think we should unify the logic of removal. >> > > Aren't we just unifying the "staleness check"? Stale > SB.Static_Mac_Binding records need to be removed too.
It depends on how do you define "stale". From my perspective the static mac binding entry that is present in NB is not stale, it just needs an update. This is especially true since the newly created row will have the exact same UUID. Dynamic entries, OTOH, are no longer valid and must be removed. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
