On Fri, Jun 17, 2022 at 2:08 AM Ales Musil <[email protected]> wrote: > > Add MAC binding aging mechanism, that > should take care of stale MAC bindings. > > The mechanism works on "ownership" of the > MAC binding row. The chassis that creates > the row is then checking if the "idle_age" > of the flow is over the aging threshold. > In that case the MAC binding is removed > from database. The "owner" might change > when another chassis saw an update of the > MAC address. > > This approach has downside, the chassis > that "owns" the MAC binding might not actually be > the one that is using it actively. This > might lead some delays in packet flow when > the row is removed. >
Thanks Ales for working on this! The stale entries in MAC_Binding table was a big TODO of OVN and a difficult problem. It is great to see a solution finally, and I think utilizing the "idle_age" is brilliant. Before reviewing it in more detail, I'd like to discuss the "downside" first. I think the "downside" here is indeed a problem with this approach. The MAC binding in OVN is in fact the ARP cache (or neighbour table) of the router, but OVN logical router is distributed (except for gateway-router and DGP), so in most cases by nature of OVN LR the user of MAC binding wouldn't be the one "owns" it. It would be a big dataplane performance impact, thinking about a chassis that has a flow with high throughput of packets suddenly needs to pause and wait for ovn-controller (and SB DB) to complete the ARP resolution process. I saw this being pointed out and discussed in the first version, but I'd raise more attention to it, because the problem introduced would be much bigger than the stale entries in the MAC binding table. I think the proposal from Daniel that transfers owner with "expire timestamp" set would help, but I am also thinking that since the logical router is distributed, it may be unreasonable to have an owner at all. My suggestion is, instead of assigning "owner" for each entry, a central controller can just be responsible for checking if any chassis still uses the entry and removing it when no one uses it anymore. Naturally the central controller can be hosted in ovn-northd. Here is the detailed algorithm I am thinking: * when an entry is created (by any ovn-controller), an expire_timestamp is set (e.g. 10 min from now - can be configurable) * Each ovn-controller: check the entries it uses and if the expire_timestamp of the entry is past, but its own "idle_age" indicates the entry is still needed, it will update the SB DB entry with a new expire_timestamp. Note: before updating the SB DB, ovn-controller needs a random delay, to avoid update storm to SB unnecessarily - in most cases only one ovn-controller would update/refresh the SB DB when an entry is expired. * ovn-northd periodically checks if there are entries with expire_timestamp past longer than 1 min (this is related to the random delay of ovn-controller, may be configurable, too), it will go ahead and delete the entry. What do you think? In addition, such a change may still be risky in large scale environments, and I think it worth experimenting first with a knob to enable it (and disabled by default). Thanks, Han > The threshold can be configured in > NB_global table with key "mac_binding_age_threshold" > in seconds with default value being 60. > > The test case is present as separate patch of the series. > > Add delay to ARP response processing to prevent > race condition between multiple controllers > that received the same ARP. > > Ales Musil (6): > Add chassis column to MAC_Binding table > Add MAC binding aging mechanism > Add stopwatch for MAC binding aging > Allow the MAC binding age threshold to be configurable > ovn.at: Add test case covering the MAC binding aging > pinctrl.c: Add delay after ARP packet > > controller/automake.mk | 4 +- > controller/mac-binding-aging.c | 241 +++++++++++++++++++++++++++++++++ > controller/mac-binding-aging.h | 32 +++++ > controller/ovn-controller.c | 32 +++++ > controller/pinctrl.c | 73 ++++++++-- > northd/northd.c | 12 ++ > northd/ovn-northd.c | 2 +- > ovn-nb.xml | 5 + > ovn-sb.ovsschema | 6 +- > ovn-sb.xml | 5 + > tests/ovn.at | 212 +++++++++++++++++++++++++++-- > 11 files changed, 595 insertions(+), 29 deletions(-) > create mode 100644 controller/mac-binding-aging.c > create mode 100644 controller/mac-binding-aging.h > > -- > 2.35.3 > > _______________________________________________ > 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
