diff mbox series

[ovs-dev,v2,22/26] ovn-northd-ddlog: Intern selected input relations.

Message ID 20210401232108.3902274-23-blp@ovn.org
State Not Applicable
Headers show
Series ddlog 5x performance improvement | expand

Commit Message

Ben Pfaff April 1, 2021, 11:21 p.m. UTC
From: Leonid Ryzhyk <lryzhyk@vmware.com>

DDlog 0.38.0 adds the `--intern-table` CLI flag to the `ovsdb2ddlog`
compiler to declare input tables coming from OVSDB as `Intern<...>`.
This is useful for tables whose records are copied around as a whole and
can therefore benefit from interning performance- and memory-wise.  In
the past we had to create separate tables in `helpers.dl` and copy
records from the original input table to them while wrapping them in
`Intern<>`.  With this change, we avoid the extra copy and intern
records as we ingest them for selected tables.

We use the `--intern-table` flag to eliminate all intermediate tables in
`helpers.dl`.

Signed-off-by: Leonid Ryzhyk <lryzhyk@vmware.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 northd/helpers.dl    | 36 ------------------------------------
 northd/lrouter.dl    | 12 ++++++------
 northd/lswitch.dl    | 26 +++++++++++++-------------
 northd/ovn-nb.dlopts |  8 ++++++++
 northd/ovn-sb.dlopts |  1 +
 northd/ovn_northd.dl | 30 +++++++++++++++---------------
 northd/ovsdb2ddlog2c |  4 +++-
 7 files changed, 46 insertions(+), 71 deletions(-)
diff mbox series

Patch

diff --git a/northd/helpers.dl b/northd/helpers.dl
index 49281fcafc9a..33a8d15d8b32 100644
--- a/northd/helpers.dl
+++ b/northd/helpers.dl
@@ -20,42 +20,6 @@  import ovn
 
 output relation Warning[string]
 
-/* ACLRef: reference to nb::ACL */
-relation ACLRef[Intern<nb::ACL>]
-ACLRef[acl.intern()] :- nb::ACL[acl].
-
-/* DHCP_Options: reference to nb::DHCP_Options */
-relation DHCP_OptionsRef[Intern<nb::DHCP_Options>]
-DHCP_OptionsRef[options.intern()] :- nb::DHCP_Options[options].
-
-/* QoS: reference to nb::QoS */
-relation QoSRef[Intern<nb::QoS>]
-QoSRef[qos.intern()] :- nb::QoS[qos].
-
-/* LoadBalancerRef: reference to nb::Load_Balancer */
-relation LoadBalancerRef[Intern<nb::Load_Balancer>]
-LoadBalancerRef[lb.intern()] :- nb::Load_Balancer[lb].
-
-/* LoadBalancerHealthCheckRef: reference to nb::Load_Balancer_Health_Check */
-relation LoadBalancerHealthCheckRef[Intern<nb::Load_Balancer_Health_Check>]
-LoadBalancerHealthCheckRef[lbhc.intern()] :- nb::Load_Balancer_Health_Check[lbhc].
-
-/* MeterRef: reference to nb::Meter*/
-relation MeterRef[Intern<nb::Meter>]
-MeterRef[meter.intern()] :- nb::Meter[meter].
-
-/* NATRef: reference to nb::NAT*/
-relation NATRef[Intern<nb::NAT>]
-NATRef[nat.intern()] :- nb::NAT[nat].
-
-/* AddressSetRef: reference to nb::Address_Set */
-relation AddressSetRef[Intern<nb::Address_Set>]
-AddressSetRef[__as.intern()] :- nb::Address_Set[__as].
-
-/* ServiceMonitor: reference to sb::Service_Monitor */
-relation ServiceMonitorRef[Intern<sb::Service_Monitor>]
-ServiceMonitorRef[sm.intern()] :- sb::Service_Monitor[sm].
-
 /* Switch-to-router logical port connections */
 relation SwitchRouterPeer(lsp: uuid, lsp_name: string, lrp: uuid)
 SwitchRouterPeer(lsp, lsp_name, lrp) :-
diff --git a/northd/lrouter.dl b/northd/lrouter.dl
index 23d320be6cc7..c51f0fbe6c44 100644
--- a/northd/lrouter.dl
+++ b/northd/lrouter.dl
@@ -281,7 +281,7 @@  relation LogicalRouterNAT0(
 LogicalRouterNAT0(lr, nat, external_ip, external_mac) :-
     nb::Logical_Router(._uuid = lr, .nat = nats),
     var nat_uuid = FlatMap(nats),
-    nat in &NATRef[nb::NAT{._uuid = nat_uuid}],
+    nat in &nb::NAT(._uuid = nat_uuid),
     Some{var external_ip} = ip46_parse(nat.external_ip),
     var external_mac = match (nat.external_mac) {
         Some{s} -> eth_addr_from_string(s),
@@ -290,12 +290,12 @@  LogicalRouterNAT0(lr, nat, external_ip, external_mac) :-
 Warning["Bad ip address ${nat.external_ip} in nat configuration for router ${lr_name}."] :-
     nb::Logical_Router(._uuid = lr, .nat = nats, .name = lr_name),
     var nat_uuid = FlatMap(nats),
-    nat in &NATRef[nb::NAT{._uuid = nat_uuid}],
+    nat in &nb::NAT(._uuid = nat_uuid),
     None = ip46_parse(nat.external_ip).
 Warning["Bad MAC address ${s} in nat configuration for router ${lr_name}."] :-
     nb::Logical_Router(._uuid = lr, .nat = nats, .name = lr_name),
     var nat_uuid = FlatMap(nats),
-    nat in &NATRef[nb::NAT{._uuid = nat_uuid}],
+    nat in &nb::NAT(._uuid = nat_uuid),
     Some{var s} = nat.external_mac,
     None = eth_addr_from_string(s).
 
@@ -308,12 +308,12 @@  LogicalRouterNAT(lr, NAT{nat, external_ip, external_mac, Some{AllowedExtIps{__as
     LogicalRouterNAT0(lr, nat, external_ip, external_mac),
     nat.exempted_ext_ips == None,
     Some{var __as_uuid} = nat.allowed_ext_ips,
-    __as in &AddressSetRef[nb::Address_Set{._uuid = __as_uuid}].
+    __as in &nb::Address_Set(._uuid = __as_uuid).
 LogicalRouterNAT(lr, NAT{nat, external_ip, external_mac, Some{ExemptedExtIps{__as}}}) :-
     LogicalRouterNAT0(lr, nat, external_ip, external_mac),
     nat.allowed_ext_ips == None,
     Some{var __as_uuid} = nat.exempted_ext_ips,
-    __as in &AddressSetRef[nb::Address_Set{._uuid = __as_uuid}].
+    __as in &nb::Address_Set(._uuid = __as_uuid).
 Warning["NAT rule: ${nat._uuid} not applied, since"
         "both allowed and exempt external ips set"] :-
     LogicalRouterNAT0(lr, nat, _, _),
@@ -404,7 +404,7 @@  relation LogicalRouterLB(lr: uuid, nat: Intern<nb::Load_Balancer>)
 LogicalRouterLB(lr, lb) :-
     nb::Logical_Router(._uuid = lr, .load_balancer = lbs),
     var lb_uuid = FlatMap(lbs),
-    lb in &LoadBalancerRef[nb::Load_Balancer{._uuid = lb_uuid}].
+    lb in &nb::Load_Balancer(._uuid = lb_uuid).
 
 relation LogicalRouterLBs(lr: uuid, nat: Vec<Intern<nb::Load_Balancer>>)
 
diff --git a/northd/lswitch.dl b/northd/lswitch.dl
index df74a3731c1d..f1456366f3cb 100644
--- a/northd/lswitch.dl
+++ b/northd/lswitch.dl
@@ -113,7 +113,7 @@  relation LogicalSwitchStatefulACL(ls: uuid, acl: uuid)
 
 LogicalSwitchStatefulACL(ls, acl) :-
     LogicalSwitchACL(ls, acl),
-    nb::ACL(._uuid = acl, .action = "allow-related").
+    &nb::ACL(._uuid = acl, .action = "allow-related").
 
 relation LogicalSwitchHasStatefulACL(ls: uuid, has_stateful_acl: bool)
 
@@ -280,7 +280,7 @@  relation SwitchLB(sw_uuid: uuid, lb: Intern<nb::Load_Balancer>)
 SwitchLB(sw_uuid, lb) :-
     nb::Logical_Switch(._uuid = sw_uuid, .load_balancer = lb_ids),
     var lb_id = FlatMap(lb_ids),
-    lb in &LoadBalancerRef[nb::Load_Balancer{._uuid = lb_id}].
+    lb in &nb::Load_Balancer(._uuid = lb_id).
 
 /* Load balancer VIPs associated with switch */
 relation SwitchLBVIP(sw_uuid: uuid, lb: Intern<nb::Load_Balancer>, vip: string, backends: string)
@@ -309,10 +309,10 @@  relation LBVIP0(
     backend_ips: string,
     health_check: Intern<nb::Load_Balancer_Health_Check>)
 LBVIP0(lb, vip_key, backend_ips, health_check) :-
-    LoadBalancerRef[lb],
+    lb in &nb::Load_Balancer(),
     var vip = FlatMap(lb.vips),
     (var vip_key, var backend_ips) = vip,
-    LoadBalancerHealthCheckRef[health_check@&nb::Load_Balancer_Health_Check{.vip = vip_key}],
+    health_check in &nb::Load_Balancer_Health_Check(.vip = vip_key),
     lb.health_check.contains(health_check._uuid).
 
 relation LBVIP1(
@@ -323,7 +323,7 @@  relation LBVIP1(
 LBVIP1(lb, vip_key, backend_ips, Some{health_check}) :-
     LBVIP0(lb, vip_key, backend_ips, health_check).
 LBVIP1(lb, vip_key, backend_ips, None) :-
-    LoadBalancerRef[lb],
+    lb in &nb::Load_Balancer(),
     var vip = FlatMap(lb.vips),
     (var vip_key, var backend_ips) = vip,
     not LBVIP0(lb, vip_key, backend_ips, _).
@@ -434,7 +434,7 @@  LBVIPBackendStatus0(lbvip, backend, is_online(sm.status)) :-
     LBVIP[lbvip@&LBVIP{.lb = lb}],
     var backend = FlatMap(lbvip.backends),
     Some{var svc_monitor} = backend.svc_monitor,
-    sm in sb::Service_Monitor(.port = backend.port as integer),
+    sm in &sb::Service_Monitor(.port = backend.port as integer),
     ip46_parse(sm.ip) == Some{backend.ip},
     svc_monitor.port_name == sm.logical_port,
     default_protocol(lb.protocol) == default_protocol(sm.protocol).
@@ -458,7 +458,7 @@  SwitchPortDHCPv4Options(port, options) :-
     port in &SwitchPort(.lsp = lsp),
     port.lsp.__type != "external",
     Some{var dhcpv4_uuid} = lsp.dhcpv4_options,
-    options in &DHCP_OptionsRef[nb::DHCP_Options{._uuid = dhcpv4_uuid}].
+    options in &nb::DHCP_Options(._uuid = dhcpv4_uuid).
 
 /* SwitchPortDHCPv6Options: many-to-one relation between logical switches and DHCPv4 options */
 relation SwitchPortDHCPv6Options(
@@ -469,7 +469,7 @@  SwitchPortDHCPv6Options(port, options) :-
     port in &SwitchPort(.lsp = lsp),
     port.lsp.__type != "external",
     Some{var dhcpv6_uuid} = lsp.dhcpv6_options,
-    options in &DHCP_OptionsRef[nb::DHCP_Options{._uuid = dhcpv6_uuid}].
+    options in &nb::DHCP_Options(._uuid = dhcpv6_uuid).
 
 /* SwitchQoS: many-to-one relation between logical switches and nb::QoS */
 relation SwitchQoS(sw: Intern<Switch>, qos: Intern<nb::QoS>)
@@ -478,7 +478,7 @@  SwitchQoS(sw, qos) :-
     sw in &Switch(),
     nb::Logical_Switch(._uuid = sw._uuid, .qos_rules = qos_rules),
     var qos_rule = FlatMap(qos_rules),
-    qos in &QoSRef[nb::QoS{._uuid = qos_rule}].
+    qos in &nb::QoS(._uuid = qos_rule).
 
 /* Reports whether a given ACL is associated with a fair meter.
  * 'has_fair_meter' is false if 'acl' has no meter, or if has a meter
@@ -489,14 +489,14 @@  relation ACLHasFairMeter(acl: Intern<nb::ACL>, has_fair_meter: bool)
 ACLHasFairMeter(acl, true) :-
     ACLWithFairMeter(acl, _).
 ACLHasFairMeter(acl, false) :-
-    acl in &ACLRef[_],
+    acl in &nb::ACL(),
     not ACLWithFairMeter(acl, _).
 
 /* All the ACLs associated with a fair meter, with their fair meters. */
 relation ACLWithFairMeter(acl: Intern<nb::ACL>, meter: Intern<nb::Meter>)
 ACLWithFairMeter(acl, meter) :-
-    acl in &ACLRef[nb::ACL{.meter = Some{meter_name}}],
-    meter in &MeterRef[nb::Meter{.name = meter_name, .fair = Some{true}}].
+    acl in &nb::ACL(.meter = Some{meter_name}),
+    meter in &nb::Meter(.name = meter_name, .fair = Some{true}).
 
 /* SwitchACL: many-to-many relation between logical switches and ACLs */
 relation &SwitchACL(sw: Intern<Switch>,
@@ -506,7 +506,7 @@  relation &SwitchACL(sw: Intern<Switch>,
 &SwitchACL(.sw = sw, .acl = acl, .has_fair_meter = has_fair_meter) :-
     LogicalSwitchACL(sw_uuid, acl_uuid),
     sw in &Switch(._uuid = sw_uuid),
-    acl in &ACLRef[nb::ACL{._uuid = acl_uuid}],
+    acl in &nb::ACL(._uuid = acl_uuid),
     ACLHasFairMeter(acl, has_fair_meter).
 
 function oVN_FEATURE_PORT_UP_NOTIF(): string { "port-up-notif" }
diff --git a/northd/ovn-nb.dlopts b/northd/ovn-nb.dlopts
index c9c171fe1c84..c4390c904f33 100644
--- a/northd/ovn-nb.dlopts
+++ b/northd/ovn-nb.dlopts
@@ -13,3 +13,11 @@ 
 --rw NB_Global.ipsec
 --rw NB_Global.nb_cfg_timestamp
 --rw NB_Global.hv_cfg_timestamp
+--intern-table DHCP_Options
+--intern-table ACL
+--intern-table QoS
+--intern-table Load_Balancer
+--intern-table Load_Balancer_Health_Check
+--intern-table Meter
+--intern-table NAT
+--intern-table Address_Set
diff --git a/northd/ovn-sb.dlopts b/northd/ovn-sb.dlopts
index 1f99118a2b8d..ea4952758b80 100644
--- a/northd/ovn-sb.dlopts
+++ b/northd/ovn-sb.dlopts
@@ -30,3 +30,4 @@ 
 --ro SB_Global.external_ids
 --ro SB_Global.ssl
 --ro Service_Monitor.status
+--intern-table Service_Monitor
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index 5476a6f2e85f..f4c6f6d1f62a 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -35,7 +35,7 @@  for (mb in nb::Meter_Band) {
 }
 
 /* Meter table */
-for (meter in nb::Meter) {
+for (meter in &nb::Meter) {
     sb::Out_Meter(._uuid = meter._uuid,
                  .name = meter.name,
                  .unit = meter.unit,
@@ -704,7 +704,7 @@  CheckLspIsUp[true] :-
 sb::Out_Address_Set(._uuid     = nb_as._uuid,
                     .name      = nb_as.name,
                     .addresses = nb_as.addresses) :-
-    AddressSetRef[nb_as].
+    nb_as in &nb::Address_Set().
 
 sb::Out_Address_Set(._uuid = hash128("svc_monitor_mac"),
                    .name = "svc_monitor_mac",
@@ -715,7 +715,7 @@  sb::Out_Address_Set(hash128(as_name), as_name, pg_ip4addrs.union()) :-
     PortGroupPort(.pg_name = pg_name, .port = port_uuid),
     var as_name = pg_name ++ "_ip4",
     // avoid name collisions with user-defined Address_Sets
-    not nb::Address_Set(.name = as_name),
+    not &nb::Address_Set(.name = as_name),
     PortStaticAddresses(.lsport = port_uuid, .ip4addrs = stat),
     SwitchPortNewDynamicAddress(&SwitchPort{.lsp = nb::Logical_Switch_Port{._uuid = port_uuid}},
                                 dyn_addr),
@@ -734,13 +734,13 @@  sb::Out_Address_Set(hash128(as_name), as_name, set_empty()) :-
     nb::Port_Group(.ports = set_empty(), .name = pg_name),
     var as_name = pg_name ++ "_ip4",
     // avoid name collisions with user-defined Address_Sets
-    not nb::Address_Set(.name = as_name).
+    not &nb::Address_Set(.name = as_name).
 
 sb::Out_Address_Set(hash128(as_name), as_name, pg_ip6addrs.union()) :-
     PortGroupPort(.pg_name = pg_name, .port = port_uuid),
     var as_name = pg_name ++ "_ip6",
     // avoid name collisions with user-defined Address_Sets
-    not nb::Address_Set(.name = as_name),
+    not &nb::Address_Set(.name = as_name),
     PortStaticAddresses(.lsport = port_uuid, .ip6addrs = stat),
     SwitchPortNewDynamicAddress(&SwitchPort{.lsp = nb::Logical_Switch_Port{._uuid = port_uuid}},
                                 dyn_addr),
@@ -759,7 +759,7 @@  sb::Out_Address_Set(hash128(as_name), as_name, set_empty()) :-
     nb::Port_Group(.ports = set_empty(), .name = pg_name),
     var as_name = pg_name ++ "_ip6",
     // avoid name collisions with user-defined Address_Sets
-    not nb::Address_Set(.name = as_name).
+    not &nb::Address_Set(.name = as_name).
 
 /*
  * Port_Group
@@ -1993,11 +1993,11 @@  if (lsp.__type == "router" or lsp.__type == "localnet") {
 relation HasEventElbMeter(has_meter: bool)
 
 HasEventElbMeter(true) :-
-    nb::Meter(.name = "event-elb").
+    &nb::Meter(.name = "event-elb").
 
 HasEventElbMeter(false) :-
     Unit(),
-    not nb::Meter(.name = "event-elb").
+    not &nb::Meter(.name = "event-elb").
 
 /* Empty LoadBalancer Controller event */
 function build_empty_lb_event_flow(key: string, lb: Intern<nb::Load_Balancer>,
@@ -2049,7 +2049,7 @@  relation LoadBalancerEmptyEvents(lb: Intern<nb::Load_Balancer>)
 LoadBalancerEmptyEvents(lb) :-
     nb::NB_Global(.options = global_options),
     var global_events = global_options.get_bool_def("controller_event", false),
-    lb in &LoadBalancerRef[nb::Load_Balancer{.options = local_options}],
+    lb in &nb::Load_Balancer(.options = local_options),
     var local_events = local_options.get_bool_def("event", false),
     global_events or local_events.
 
@@ -2659,7 +2659,7 @@  for (SwitchPortDHCPv6Options(.port = &SwitchPort{.lsp = lsp, .sw = sw},
 relation QoSAction(qos: uuid, key_action: string, value_action: integer)
 
 QoSAction(qos, k, v) :-
-    nb::QoS(._uuid = qos, .action = actions),
+    &nb::QoS(._uuid = qos, .action = actions),
     var action = FlatMap(actions),
     (var k, var v) = action.
 
@@ -3420,7 +3420,7 @@  Flow(.logical_datapath = sp.sw._uuid,
 
 function build_dhcpv4_action(
     lsp_json_key: string,
-    dhcpv4_options: nb::DHCP_Options,
+    dhcpv4_options: Intern<nb::DHCP_Options>,
     offer_ip: in_addr) : Option<(string, string, string)> =
 {
     match (ip_parse_masked(dhcpv4_options.cidr)) {
@@ -3478,7 +3478,7 @@  function build_dhcpv4_action(
 
 function build_dhcpv6_action(
     lsp_json_key: string,
-    dhcpv6_options: nb::DHCP_Options,
+    dhcpv6_options: Intern<nb::DHCP_Options>,
     offer_ip: in6_addr): Option<(string, string)> =
 {
     match (ipv6_parse_masked(dhcpv6_options.cidr)) {
@@ -3605,7 +3605,7 @@  for (lsp in &SwitchPort
             /* DHCPv4 options enabled for this port */
             Some{var dhcpv4_options_uuid} = lsp.lsp.dhcpv4_options in
             {
-                for (dhcpv4_options in nb::DHCP_Options(._uuid = dhcpv4_options_uuid)) {
+                for (dhcpv4_options in &nb::DHCP_Options(._uuid = dhcpv4_options_uuid)) {
                     for (SwitchPortIPv4Address(.port = &SwitchPort{.lsp = nb::Logical_Switch_Port{._uuid = lsp.lsp._uuid}}, .ea = ea, .addr = addr)) {
                         Some{(var options_action, var response_action, var ipv4_addr_match)} =
                             build_dhcpv4_action(json_key, dhcpv4_options, addr.addr) in
@@ -3662,7 +3662,7 @@  for (lsp in &SwitchPort
             /* DHCPv6 options enabled for this port */
             Some{var dhcpv6_options_uuid} = lsp.lsp.dhcpv6_options in
             {
-                for (dhcpv6_options in nb::DHCP_Options(._uuid = dhcpv6_options_uuid)) {
+                for (dhcpv6_options in &nb::DHCP_Options(._uuid = dhcpv6_options_uuid)) {
                     for (SwitchPortIPv6Address(.port = &SwitchPort{.lsp = nb::Logical_Switch_Port{._uuid = lsp.lsp._uuid}}, .ea = ea, .addr = addr)) {
                         Some{(var options_action, var response_action)} =
                             build_dhcpv6_action(json_key, dhcpv6_options, addr.addr) in
@@ -7745,7 +7745,7 @@  sb::Out_Load_Balancer(._uuid = lb._uuid,
     nb in nb::Logical_Switch(._uuid = ls_uuid, .load_balancer = lb_uuids),
     var lb_uuid = FlatMap(lb_uuids),
     var datapaths = ls_uuid.group_by(lb_uuid).to_set(),
-    lb in nb::Load_Balancer(._uuid = lb_uuid),
+    lb in &nb::Load_Balancer(._uuid = lb_uuid),
     /* Store the fact that northd provides the original (destination IP +
      * transport port) tuple.
      */
diff --git a/northd/ovsdb2ddlog2c b/northd/ovsdb2ddlog2c
index d0cc7bade9d3..e489c3c760a5 100755
--- a/northd/ovsdb2ddlog2c
+++ b/northd/ovsdb2ddlog2c
@@ -36,6 +36,7 @@  The following ovsdb2ddlog options are supported:
   --output-only-table=TABLE  Mark TABLE as output-only.  DDlog will send updates to this table directly to OVSDB without comparing it with current OVSDB state.
   --ro=TABLE.COLUMN          Ignored.
   --rw=TABLE.COLUMN          Ignored.
+  --intern-table=TABLE       Ignored.
   --output-file=FILE.inc     Write output to FILE.inc. If this option is not specified, output will be written to stdout.
 
 The following options are also available:
@@ -52,6 +53,7 @@  if __name__ == "__main__":
                                                'schema-file=',
                                                'output-table=',
                                                'output-only-table=',
+                                               'intern-table=',
                                                'ro=',
                                                'rw=',
                                                'output-file='])
@@ -77,7 +79,7 @@  if __name__ == "__main__":
                 output_tables.add(value)
             elif key == '--output-only-table':
                 output_only_tables.add(value)
-            elif key in ['--ro', '--rw']:
+            elif key in ['--ro', '--rw', '--intern-table']:
                 pass
             elif key == '--output-file':
                 output_file = value