[ovs-dev] [PATCH v2 13/26] ovn-northd-ddlog: Intern the `Router` table.
Ben Pfaff
blp at ovn.org
Thu Apr 1 23:20:55 UTC 2021
From: Leonid Ryzhyk <lryzhyk at 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 <lryzhyk at vmware.com>
Signed-off-by: Ben Pfaff <blp at 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
More information about the dev
mailing list