commit 1ae095876df3e6f7199ae5a79042b9ee8c2a99c7
author pierventre <pier@opennetworking.org> Wed Jun 23 09:02:13 2021 +0200
committer pierventre <pier@opennetworking.org> Tue Jun 29 08:33:32 2021 +0200
tree a54f7c1b8419de58feffe261cf8109eec2574b61
parent 9e1e5df971f13c98a61fcdee768d90f6c6367684

[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

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()