From patchwork Fri May 7 04:06:46 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 1475329 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FbxlS6gBmz9sPf for ; Fri, 7 May 2021 14:08:52 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 7C126844FC; Fri, 7 May 2021 04:08:50 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ouZtZZ_tyijj; Fri, 7 May 2021 04:08:47 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTP id 9F37E84451; Fri, 7 May 2021 04:08:15 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 90509C0022; Fri, 7 May 2021 04:08:14 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 17979C0001 for ; Fri, 7 May 2021 04:08:13 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id A6FD283CC5 for ; Fri, 7 May 2021 04:07:34 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ELCuZBVKaTzk for ; Fri, 7 May 2021 04:07:31 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net [217.70.183.198]) by smtp1.osuosl.org (Postfix) with ESMTPS id C8F2C8275A for ; Fri, 7 May 2021 04:07:30 +0000 (UTC) X-Originating-IP: 75.54.222.30 Received: from sigfpe.attlocal.net (75-54-222-30.lightspeed.rdcyca.sbcglobal.net [75.54.222.30]) (Authenticated sender: blp@ovn.org) by relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 15548C0006; Fri, 7 May 2021 04:07:27 +0000 (UTC) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 6 May 2021 21:06:46 -0700 Message-Id: <20210507040659.26830-15-blp@ovn.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210507040659.26830-1-blp@ovn.org> References: <20210507040659.26830-1-blp@ovn.org> MIME-Version: 1.0 Cc: Leonid Ryzhyk , Ben Pfaff Subject: [ovs-dev] [PATCH ovn v3 14/27] ovn-northd-ddlog: Intern the `Router` table. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" From: Leonid Ryzhyk 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` to `Intern`. 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 Signed-off-by: Ben Pfaff --- 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 07579fb45902..87550c4fcb36 100644 --- a/northd/lrouter.dl +++ b/northd/lrouter.dl @@ -440,7 +440,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, @@ -462,9 +462,12 @@ relation &Router( mcast_cfg: Ref, learn_from_arp_request: bool, force_lb_snat: bool, -) +} + +relation Router[Intern] -&Router(._uuid = lr._uuid, +Router[Router{ + ._uuid = lr._uuid, .name = lr.name, .static_routes = lr.static_routes, .policies = lr.policies, @@ -486,7 +489,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), @@ -498,7 +501,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, lb: Ref) +relation RouterLB(router: Intern, lb: Ref) RouterLB(router, lb) :- router in &Router(.lbs = lbs), @@ -506,7 +509,7 @@ RouterLB(router, lb) :- /* Load balancer VIPs associated with routers */ relation RouterLBVIP( - router: Ref, + router: Intern, lb: Ref, vip: string, backends: string) @@ -586,7 +589,7 @@ relation &RouterPort( lrp: nb::Logical_Router_Port, json_name: string, networks: lport_addresses, - router: Ref, + router: Intern, is_redirect: bool, peer: RouterPeer, mcast_cfg: Ref, @@ -721,7 +724,7 @@ function find_lrp_member_ip(networks: lport_addresses, ip: v46_ip): Option, + router : Intern, key : route_key, nexthop : v46_ip, output_port : Option, @@ -745,7 +748,7 @@ typedef route_dst = RouteDst { } relation RouterStaticRoute( - router : Ref, + router : Intern, key : route_key, dsts : Set) 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, ports: Set) +relation RouterMcastFloodPorts(sw: Intern, ports: Set) RouterMcastFloodPorts(router, flood_ports) :- &RouterPort( @@ -213,7 +213,7 @@ IgmpSwitchMulticastGroup(address, switch, ports) :- */ relation IgmpRouterGroupPort( address: string, - router : Ref, + router : Intern, port : uuid ) @@ -236,7 +236,7 @@ IgmpRouterGroupPort(address, rtr_port.router, rtr_port.lrp._uuid) :- */ relation IgmpRouterMulticastGroup( address: string, - router : Ref, + router : Intern, ports : Set ) diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl index fad633b1870e..51bbc832b7b6 100644 --- a/northd/ovn_northd.dl +++ b/northd/ovn_northd.dl @@ -297,7 +297,7 @@ OutProxy_Port_Binding(._uuid = lrp._uuid, }. /* */ -function get_router_load_balancer_ips(router: Router) : +function get_router_load_balancer_ips(router: Intern) : (Set, Set) = { var all_ips_v4 = set_empty(); @@ -327,8 +327,7 @@ function get_router_load_balancer_ips(router: Router) : function get_nat_addresses(rport: RouterPort): Set = { 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} -> { @@ -336,7 +335,7 @@ function get_nat_addresses(rport: RouterPort): Set = 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 @@ -379,7 +378,7 @@ function get_nat_addresses(rport: RouterPort): Set = }; /* 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}"; @@ -390,7 +389,7 @@ function get_nat_addresses(rport: RouterPort): Set = /* 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) @@ -4048,7 +4047,7 @@ function get_arp_forward_ips(rp: Ref): (Set, Set) = 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. @@ -4817,7 +4816,7 @@ LogicalRouterNatArpNdFlow(router, nat) :- (var ip, var nats) = snat_ip, Some{var nat} = nats.nth(0). -relation LogicalRouterNatArpNdFlow(router: Ref, nat: NAT) +relation LogicalRouterNatArpNdFlow(router: Intern, nat: NAT) LogicalRouterArpNdFlow(router, nat, None, rEG_INPORT_ETH_ADDR(), None, false, 90) :- LogicalRouterNatArpNdFlow(router, nat). @@ -4847,7 +4846,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, nat: NAT, lrp: nb::Logical_Router_Port) +relation LogicalRouterPortNatArpNdFlow(router: Intern, 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), @@ -4878,7 +4877,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: Intern, nat: NAT, lrp: Option, mac: string, @@ -4895,7 +4894,7 @@ LogicalRouterNdFlow(router, lrp, "nd_na", ipv6, true, mac, extra_match, drop, pr mac, extra_match, drop, priority). relation LogicalRouterArpFlow( - lr: Ref, + lr: Intern, lrp: Option, ip: in_addr, mac: string, @@ -4938,7 +4937,7 @@ Flow(.logical_datapath = lr._uuid, }. relation LogicalRouterNdFlow( - lr: Ref, + lr: Intern, lrp: Option, action: string, ip: in6_addr, @@ -5368,7 +5367,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: Intern, nat: NAT, __match: string, ipX: string, @@ -6414,7 +6413,7 @@ function numbered_vec(set: Set<'A>) : Vec<(bit<16>, 'A)> = { relation EcmpGroup( group_id: bit<16>, - router: Ref, + router: Intern, key: route_key, dsts: Set, route_match: string, // This is build_route_match(key).0 @@ -6471,7 +6470,7 @@ Flow(.logical_datapath = router._uuid, * an ECMP route need to go through conntrack. */ relation EcmpSymmetricReply( - router: Ref, + router: Intern, dst: route_dst, route_match: string, tunkey: integer) @@ -6693,7 +6692,7 @@ function all_same_addr_family(addrs: Set): bool { } relation EcmpReroutePolicy( - r: Ref, + r: Intern, policy: nb::Logical_Router_Policy, ecmp_group_id: usize )