From: Leonid Ryzhyk <lryz...@vmware.com> This is the first in a series of commits that will replace the use of the DDlog's `Ref<>` type with `Intern<>` throughout the OVN code base. `Ref` and `Intern` are the two forms of smart pointers supported by DDlog at the moment. `Ref` is a reference counted pointer. Copying a `Ref<>` simply increments its reference count. `Intern<>` is an interned object reference. It guarantees that there exists exactly one copy of each unique interned value. Interned objects are slightly more expensive to create, but they have several important advantages: (1) they save memory by deduplicating identical values, (2) they allow by-pointer comparisons, and (3) they avoid unnecessary recomputations in some scenarios. See DDlog docs [1], [2] for more detail.
In this commit we change the type of records in the `Router` table from `Ref<Router>` to `Intern<Router>`. This reduces the amount of churn and speeds up northd significantly in scenarios where the set of router ports changes frequently, which triggers updates to `nb::Logical_Router`, which in turn updates corresponding records in the `Router` table. Interning guarantees that these updates are no-ops and do not trigger any other rules. [1] https://github.com/vmware/differential-datalog/blob/master/doc/tutorial/tutorial.md#reference-type-ref [2] https://github.com/vmware/differential-datalog/blob/master/doc/tutorial/tutorial.md#interned-values-intern-istring Signed-off-by: Leonid Ryzhyk <lryz...@vmware.com> Signed-off-by: Ben Pfaff <b...@ovn.org> --- northd/lrouter.dl | 21 ++++++++++++--------- northd/multicast.dl | 6 +++--- northd/ovn_northd.dl | 31 +++++++++++++++---------------- 3 files changed, 30 insertions(+), 28 deletions(-) diff --git a/northd/lrouter.dl b/northd/lrouter.dl index 574926b73b67..b2b429af3c96 100644 --- a/northd/lrouter.dl +++ b/northd/lrouter.dl @@ -430,7 +430,7 @@ LogicalRouterLBs(lr, vec_empty()) :- function chassis_redirect_name(port_name: string): string = "cr-${port_name}" -relation &Router( +typedef Router = Router { /* Fields copied from nb::Logical_Router. */ _uuid: uuid, name: string, @@ -452,9 +452,12 @@ relation &Router( mcast_cfg: Ref<McastRouterCfg>, learn_from_arp_request: bool, force_lb_snat: bool, -) +} + +relation Router[Intern<Router>] -&Router(._uuid = lr._uuid, +Router[Router{ + ._uuid = lr._uuid, .name = lr.name, .static_routes = lr.static_routes, .policies = lr.policies, @@ -476,7 +479,7 @@ relation &Router( .lbs = lbs, .mcast_cfg = mcast_cfg, .learn_from_arp_request = learn_from_arp_request, - .force_lb_snat = force_lb_snat) :- + .force_lb_snat = force_lb_snat}.intern()] :- lr in nb::Logical_Router(), lr.is_enabled(), LogicalRouterRedirectPort(lr._uuid, l3dgw_port), @@ -488,7 +491,7 @@ relation &Router( var force_lb_snat = lb_force_snat_router_ip(lr.options). /* RouterLB: many-to-many relation between logical routers and nb::LB */ -relation RouterLB(router: Ref<Router>, lb: Ref<nb::Load_Balancer>) +relation RouterLB(router: Intern<Router>, lb: Ref<nb::Load_Balancer>) RouterLB(router, lb) :- router in &Router(.lbs = lbs), @@ -496,7 +499,7 @@ RouterLB(router, lb) :- /* Load balancer VIPs associated with routers */ relation RouterLBVIP( - router: Ref<Router>, + router: Intern<Router>, lb: Ref<nb::Load_Balancer>, vip: string, backends: string) @@ -576,7 +579,7 @@ relation &RouterPort( lrp: nb::Logical_Router_Port, json_name: string, networks: lport_addresses, - router: Ref<Router>, + router: Intern<Router>, is_redirect: bool, peer: RouterPeer, mcast_cfg: Ref<McastPortCfg>, @@ -711,7 +714,7 @@ function find_lrp_member_ip(networks: lport_addresses, ip: v46_ip): Option<v46_i /* Step 1: compute router-route pairs */ relation RouterStaticRoute_( - router : Ref<Router>, + router : Intern<Router>, key : route_key, nexthop : v46_ip, output_port : Option<string>, @@ -735,7 +738,7 @@ typedef route_dst = RouteDst { } relation RouterStaticRoute( - router : Ref<Router>, + router : Intern<Router>, key : route_key, dsts : Set<route_dst>) diff --git a/northd/multicast.dl b/northd/multicast.dl index 990203bffe25..9b0fa80738d7 100644 --- a/northd/multicast.dl +++ b/northd/multicast.dl @@ -160,7 +160,7 @@ SwitchMcastFloodReportPorts(switch, set_empty()) :- /* Mapping between Router and the set of port uuids on which to * flood IP multicast reports statically. */ -relation RouterMcastFloodPorts(sw: Ref<Router>, ports: Set<uuid>) +relation RouterMcastFloodPorts(sw: Intern<Router>, ports: Set<uuid>) RouterMcastFloodPorts(router, flood_ports) :- &RouterPort( @@ -213,7 +213,7 @@ IgmpSwitchMulticastGroup(address, switch, ports) :- */ relation IgmpRouterGroupPort( address: string, - router : Ref<Router>, + router : Intern<Router>, port : uuid ) @@ -236,7 +236,7 @@ IgmpRouterGroupPort(address, rtr_port.router, rtr_port.lrp._uuid) :- */ relation IgmpRouterMulticastGroup( address: string, - router : Ref<Router>, + router : Intern<Router>, ports : Set<uuid> ) diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl index bb8be08dc55e..80d8598bd7dc 100644 --- a/northd/ovn_northd.dl +++ b/northd/ovn_northd.dl @@ -289,7 +289,7 @@ OutProxy_Port_Binding(._uuid = lrp._uuid, }. /* */ -function get_router_load_balancer_ips(router: Router) : +function get_router_load_balancer_ips(router: Intern<Router>) : (Set<string>, Set<string>) = { var all_ips_v4 = set_empty(); @@ -319,8 +319,7 @@ function get_router_load_balancer_ips(router: Router) : function get_nat_addresses(rport: RouterPort): Set<string> = { var addresses = set_empty(); - var router = deref(rport.router); - var has_redirect = router.l3dgw_port.is_some(); + var has_redirect = rport.router.l3dgw_port.is_some(); match (eth_addr_from_string(rport.lrp.mac)) { None -> addresses, Some{mac} -> { @@ -328,7 +327,7 @@ function get_nat_addresses(rport: RouterPort): Set<string> = var central_ip_address = false; /* Get NAT IP addresses. */ - for (nat in router.nats) { + for (nat in rport.router.nats) { /* Determine whether this NAT rule satisfies the conditions for * distributed NAT processing. */ if (has_redirect and nat.nat.__type == "dnat_and_snat" and @@ -371,7 +370,7 @@ function get_nat_addresses(rport: RouterPort): Set<string> = }; /* A set to hold all load-balancer vips. */ - (var all_ips_v4, var all_ips_v6) = get_router_load_balancer_ips(router); + (var all_ips_v4, var all_ips_v6) = get_router_load_balancer_ips(rport.router); for (ip_address in set_union(all_ips_v4, all_ips_v6)) { c_addresses = c_addresses ++ " ${ip_address}"; @@ -382,7 +381,7 @@ function get_nat_addresses(rport: RouterPort): Set<string> = /* Gratuitous ARP for centralized NAT rules on distributed gateway * ports should be restricted to the gateway chassis. */ if (has_redirect) { - c_addresses = c_addresses ++ " is_chassis_resident(${router.redirect_port_name})" + c_addresses = c_addresses ++ " is_chassis_resident(${rport.router.redirect_port_name})" } else (); addresses.insert(c_addresses) @@ -4083,7 +4082,7 @@ function get_arp_forward_ips(rp: Ref<RouterPort>): (Set<string>, Set<string>) = var all_ips_v6 = set_empty(); (var lb_ips_v4, var lb_ips_v6) - = get_router_load_balancer_ips(deref(rp.router)); + = get_router_load_balancer_ips(rp.router); for (a in lb_ips_v4) { /* Check if the ovn port has a network configured on which we could * expect ARP requests for the LB VIP. @@ -4852,7 +4851,7 @@ LogicalRouterNatArpNdFlow(router, nat) :- (var ip, var nats) = snat_ip, Some{var nat} = nats.nth(0). -relation LogicalRouterNatArpNdFlow(router: Ref<Router>, nat: NAT) +relation LogicalRouterNatArpNdFlow(router: Intern<Router>, nat: NAT) LogicalRouterArpNdFlow(router, nat, None, rEG_INPORT_ETH_ADDR(), None, false, 90) :- LogicalRouterNatArpNdFlow(router, nat). @@ -4882,7 +4881,7 @@ LogicalRouterPortNatArpNdFlow(router, nat, l3dgw_port) :- /* Respond to ARP/NS requests on the chassis that binds the gw * port. Drop the ARP/NS requests on other chassis. */ -relation LogicalRouterPortNatArpNdFlow(router: Ref<Router>, nat: NAT, lrp: nb::Logical_Router_Port) +relation LogicalRouterPortNatArpNdFlow(router: Intern<Router>, nat: NAT, lrp: nb::Logical_Router_Port) LogicalRouterArpNdFlow(router, nat, Some{lrp}, mac, Some{extra_match}, false, 92), LogicalRouterArpNdFlow(router, nat, Some{lrp}, mac, None, true, 91) :- LogicalRouterPortNatArpNdFlow(router, nat, lrp), @@ -4913,7 +4912,7 @@ LogicalRouterArpNdFlow(router, nat, Some{lrp}, mac, None, true, 91) :- /* Now divide the ARP/ND flows into ARP and ND. */ relation LogicalRouterArpNdFlow( - router: Ref<Router>, + router: Intern<Router>, nat: NAT, lrp: Option<nb::Logical_Router_Port>, mac: string, @@ -4930,7 +4929,7 @@ LogicalRouterNdFlow(router, lrp, "nd_na", ipv6, true, mac, extra_match, drop, pr mac, extra_match, drop, priority). relation LogicalRouterArpFlow( - lr: Ref<Router>, + lr: Intern<Router>, lrp: Option<nb::Logical_Router_Port>, ip: in_addr, mac: string, @@ -4973,7 +4972,7 @@ Flow(.logical_datapath = lr._uuid, }. relation LogicalRouterNdFlow( - lr: Ref<Router>, + lr: Intern<Router>, lrp: Option<nb::Logical_Router_Port>, action: string, ip: in6_addr, @@ -5393,7 +5392,7 @@ function lrouter_nat_is_stateless(nat: NAT): bool = { * and action says "next" instead of ct*. */ function lrouter_nat_add_ext_ip_match( - router: Ref<Router>, + router: Intern<Router>, nat: NAT, __match: string, ipX: string, @@ -6433,7 +6432,7 @@ function numbered_vec(set: Set<'A>) : Vec<(bit<16>, 'A)> = { relation EcmpGroup( group_id: bit<16>, - router: Ref<Router>, + router: Intern<Router>, key: route_key, dsts: Set<route_dst>, route_match: string, // This is build_route_match(key).0 @@ -6490,7 +6489,7 @@ Flow(.logical_datapath = router._uuid, * an ECMP route need to go through conntrack. */ relation EcmpSymmetricReply( - router: Ref<Router>, + router: Intern<Router>, dst: route_dst, route_match: string, tunkey: integer) @@ -6712,7 +6711,7 @@ function all_same_addr_family(addrs: Set<string>): bool { } relation EcmpReroutePolicy( - r: Ref<Router>, + r: Intern<Router>, policy: nb::Logical_Router_Policy, ecmp_group_id: usize ) -- 2.29.2 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev