Re: [ovs-dev] [PATCH 2/7] ovn-northd-ddlog: Use cheaper representation for stage_hint.

2021-08-12 Thread 0-day Robot
Bleep bloop.  Greetings Ben Pfaff, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


Patch skipped due to previous failure.

Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH 2/7] ovn-northd-ddlog: Use cheaper representation for stage_hint.

2021-08-12 Thread Ben Pfaff
From: Ben Pfaff 

The stage_hint only shows 32 bits of the uuid, so it's cheaper to omit
the rest for the internal representation.  Also, this is just a hint, so
we might as well use zero to mean None and save the cost of the Option
wrapper.

With the benchmark at
https://mail.openvswitch.org/pipermail/ovs-dev/2021-July/385333.html,
this reduces memory consumption by 1.3 GB.

Signed-off-by: Ben Pfaff 
---
 northd/ovn_northd.dl | 579 ++-
 1 file changed, 292 insertions(+), 287 deletions(-)

diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index 844add024..2365372fb 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -1644,9 +1644,13 @@ relation Flow(
 actions:  string,
 io_port:  Option,
 controller_meter: Option,
-stage_hint:   Option
+stage_hint:   bit<32>
 )
 
+function stage_hint(_uuid: uuid): bit<32> {
+_uuid[127:96]
+}
+
 /* If this option is 'true' northd will combine logical flows that differ by
  * logical datapath only by creating a datapath group. */
 relation UseLogicalDatapathGroups[bool]
@@ -1673,11 +1677,12 @@ function make_flow_tags(io_port: Option): 
Map {
 Some{s} -> [ "in_out_port" -> s ]
 }
 }
-function make_flow_external_ids(stage_hint: Option, stage: Stage): 
Map {
-match (stage_hint) {
-None -> ["stage-name" -> stage.table_name],
-Some{uuid} -> ["stage-name" -> stage.table_name,
-   "stage-hint" -> "${hex(uuid[127:96])}"]
+function make_flow_external_ids(stage_hint: bit<32>, stage: Stage): 
Map {
+if (stage_hint == 0) {
+["stage-name" -> stage.table_name]
+} else {
+["stage-name" -> stage.table_name,
+ "stage-hint" -> "${hex(stage_hint)}"]
 }
 }
 AggregatedFlow(.logical_datapaths = g.to_set(),
@@ -1744,7 +1749,7 @@ Flow(.logical_datapath = sw._uuid,
  .priority = 50,
  .__match  = __match,
  .actions  = actions,
- .stage_hint   = Some{fg_uuid},
+ .stage_hint   = stage_hint(fg_uuid),
  .io_port  = None,
  .controller_meter = None) :-
 sw in &Switch(),
@@ -1776,7 +1781,7 @@ Flow(.logical_datapath = sw._uuid,
  .priority = 50,
  .__match  = __match,
  .actions  = actions,
- .stage_hint   = None,
+ .stage_hint   = 0,
  .io_port  = None,
  .controller_meter = None) :-
 sw in &Switch(),
@@ -1799,7 +1804,7 @@ for (sw in &Switch()) {
  .priority = 100,
  .__match  = "vlan.present",
  .actions  = "drop;",
- .stage_hint   = None /*TODO: check*/,
+ .stage_hint   = 0 /*TODO: check*/,
  .io_port  = None,
  .controller_meter = None)
 };
@@ -1810,7 +1815,7 @@ for (sw in &Switch()) {
  .priority = 100,
  .__match  = "eth.src[40]",
  .actions  = "drop;",
- .stage_hint   = None /*TODO: check*/,
+ .stage_hint   = 0 /*TODO: check*/,
  .io_port  = None,
  .controller_meter = None)
 /* Port security flows have priority 50 (see below) and will continue to 
the next table
@@ -1872,7 +1877,7 @@ for (&Switch(._uuid =ls_uuid)) {
  .priority = 0,
  .__match  = "1",
  .actions  = "next;",
- .stage_hint   = None,
+ .stage_hint   = 0,
  .io_port  = None,
  .controller_meter = None);
 Flow(.logical_datapath = ls_uuid,
@@ -1880,7 +1885,7 @@ for (&Switch(._uuid =ls_uuid)) {
  .priority = 0,
  .__match  = "1",
  .actions  = "next;",
- .stage_hint   = None,
+ .stage_hint   = 0,
  .io_port  = None,
  .controller_meter = None);
 
@@ -1889,7 +1894,7 @@ for (&Switch(._uuid =ls_uuid)) {
  .priority = 110,
  .__match  = "eth.dst == $svc_monitor_mac",
  .actions  = "next;",
- .stage_hint   = None,
+ .stage_hint   = 0,
  .io_port  = None,
  .controller_meter = None);
 Flow(.logical_datapath = ls_uuid,
@@ -1897,7 +1902,7 @@ for (&Switch(._uuid =ls_uuid)) {
  .priority = 110,
  .__match  = "eth.src == $svc_monitor_mac",
  .actions  = "next;",
- .stage_hint   = None,
+ .stage_hint   = 0,
  .io_port  = None,
  .controller_meter = None)
 }
@@ -1912,7 +1917,7 @@ for (&SwitchACL(.sw = sw@&Switch{._uuid = ls_uuid}, .acl 
= &acl, .has_fair_meter
  .priority = acl.priority + oVN_ACL_PRI_OFFSET(),
  .__match  = acl.__match,
  .actions  = "next;",
- .stage_hint   = Some{acl._uuid},
+