commit b91b0a85f8d710aadf4d927afa073ed2f3c9e8c1
author pierventre <pier@opennetworking.org> Wed Jun 23 09:02:13 2021 +0200
committer Pier Luigi Ventre <pier@opennetworking.org> Tue Jun 29 18:37:20 2021 +0000
tree 9b69b3320ecddc30dfb2e65a6746a2ed3e984840
parent 68f9c3221dc549864be9a8a1bf938ee8c7f7e66f

[SDFAB-293] Fix UPF call setup performance degradation

It has been observed that calling setupBehavior and computeHardwareResources
for each flow introduce a not negligible delay and these operations were not
performed before the refactoring for each flow.

There is no need to call the setup steps of setupBehavior each time but should
be called only the very first time. This patch makes setupBehavior more smart.

Then, looking at the performance with the profiler, we noticed that computeHwResources
is the most expensive operation we have introduced recently - it does not seem necessary
to compute the hw resources each time. With this patch we compute the hw resources/limits
only at the behavior init.

Change-Id: If6352f25c23cf93c25fcf3cffae3d3834bc0e75f
(cherry picked from commit 1ae095876df3e6f7199ae5a79042b9ee8c2a99c7)

pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/FabricUpfProgrammable.java

diff --git a/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/FabricUpfProgrammable.java b/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/FabricUpfProgrammable.java
index 61345c4..3c81482 100644
--- a/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/FabricUpfProgrammable.java
+++ b/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/FabricUpfProgrammable.java
@@ -99,9 +99,20 @@
 
     @Override
     protected boolean setupBehaviour(String opName) {
+        // Already initialized.
+        if (appId != null) {
+            return true;
+        }
+
         if (!super.setupBehaviour(opName)) {
             return false;
         }
+
+        if (!computeHardwareResourceSizes()) {
+            // error message will be printed by computeHardwareResourceSizes()
+            return false;
+        }
+
         flowRuleService = handler().get(FlowRuleService.class);
         packetService = handler().get(PacketService.class);
         fabricUpfStore = handler().get(FabricUpfStore.class);
@@ -126,10 +137,6 @@
     @Override
     public boolean init() {
         if (setupBehaviour("init()")) {
-            if (!computeHardwareResourceSizes()) {
-                // error message will be printed by computeHardwareResourceSizes()
-                return false;
-            }
             log.info("UpfProgrammable initialized for appId {} and deviceId {}", appId, deviceId);
             return true;
         }
@@ -327,7 +334,6 @@
         if (!setupBehaviour("pdrCounterSize()")) {
             return -1;
         }
-        computeHardwareResourceSizes();
         return pdrCounterSize;
     }
 
@@ -336,7 +342,6 @@
         if (!setupBehaviour("farTableSize()")) {
             return -1;
         }
-        computeHardwareResourceSizes();
         return farTableSize;
     }
 
@@ -345,7 +350,6 @@
         if (!setupBehaviour("pdrTableSize()")) {
             return -1;
         }
-        computeHardwareResourceSizes();
         return Math.min(encappedPdrTableSize, unencappedPdrTableSize) * 2;
     }
 
@@ -438,9 +442,6 @@
 
     private boolean removeEntry(PiCriterion match, PiTableId tableId, boolean failSilent)
             throws UpfProgrammableException {
-        if (!setupBehaviour("removeEntry()")) {
-            return false;
-        }
         FlowRule entry = DefaultFlowRule.builder()
                 .forDevice(deviceId).fromApp(appId).makePermanent()
                 .forTable(tableId)
@@ -534,6 +535,9 @@
 
     @Override
     public void removeFar(ForwardingActionRule far) throws UpfProgrammableException {
+        if (!setupBehaviour("removeFar()")) {
+            return;
+        }
         log.info("Removing {}", far.toString());
 
         PiCriterion match = PiCriterion.builder()