Minor tweaks to the flow layer.

Prevent DistributedStatistics store from logging "rule has no output" for rules that transition to other tables. Change-Id: I85e86965f5609df608cbc19551632153960a5c5b

Minor tweaks to the flow layer.
Prevent DistributedStatistics store from logging "rule has no output" for rules that transition to other tables. Change-Id: I85e86965f5609df608cbc19551632153960a5c5b
Jonathan Hart · Gerrit Code Review
Commit 8ef6d3b9a09b4a9cf9d4d9e847651c87b413573b 8ef6d3b9 1 parent 8b6d2d44
Showing 8 changed files with 71 additions and 58 deletions
core/api/src/main/java/org/onosproject/net/flow/DefaultFlowRule.java
core/api/src/main/java/org/onosproject/net/flow/DefaultTrafficTreatment.java
core/api/src/main/java/org/onosproject/net/flow/TrafficTreatment.java
core/api/src/test/java/org/onosproject/net/intent/IntentTestsMocks.java
core/net/src/test/java/org/onosproject/net/flow/impl/FlowRuleManagerTest.java
core/store/dist/src/main/java/org/onosproject/store/statistic/impl/DistributedStatisticStore.java
providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowEntryBuilder.java
providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/GroupBucketEntryBuilder.java
--- a/core/api/src/main/java/org/onosproject/net/flow/DefaultFlowRule.java
View file @8ef6d3b
+++ b/core/api/src/main/java/org/onosproject/net/flow/DefaultFlowRule.java
View file @8ef6d3b
@@ -15,15 +15,15 @@
  */
 package org.onosproject.net.flow;
-import static com.google.common.base.MoreObjects.toStringHelper;
-
-import java.util.Objects;
-
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.DefaultGroupId;
 import org.onosproject.core.GroupId;
 import org.onosproject.net.DeviceId;
+import java.util.Objects;
+
+import static com.google.common.base.MoreObjects.toStringHelper;
+
 public class DefaultFlowRule implements FlowRule {
     private final DeviceId deviceId;
@@ -233,7 +233,7 @@ public class DefaultFlowRule implements FlowRule {
                 .add("deviceId", deviceId)
                 .add("priority", priority)
                 .add("selector", selector.criteria())
-                .add("treatment", treatment == null ? "N/A" : treatment.instructions())
+                .add("treatment", treatment == null ? "N/A" : treatment.allInstructions())
                 .add("table type", type)
                 .add("created", created)
                 .toString();
--- a/core/api/src/main/java/org/onosproject/net/flow/DefaultTrafficTreatment.java
View file @8ef6d3b
+++ b/core/api/src/main/java/org/onosproject/net/flow/DefaultTrafficTreatment.java
View file @8ef6d3b
@@ -18,6 +18,7 @@ package org.onosproject.net.flow;
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
+import org.apache.commons.collections.ListUtils;
 import org.onlab.packet.IpAddress;
 import org.onlab.packet.MacAddress;
 import org.onlab.packet.MplsLabel;
@@ -84,6 +85,11 @@ public final class DefaultTrafficTreatment implements TrafficTreatment {
     }
     @Override
+    public List<Instruction> allInstructions() {
+        return ListUtils.union(immediate, deferred);
+    }
+
+    @Override
     public Instructions.TableTypeTransition tableTransition() {
         return table;
     }
--- a/core/api/src/main/java/org/onosproject/net/flow/TrafficTreatment.java
View file @8ef6d3b
+++ b/core/api/src/main/java/org/onosproject/net/flow/TrafficTreatment.java
View file @8ef6d3b
@@ -54,6 +54,14 @@ public interface TrafficTreatment {
     List<Instruction> immediate();
     /**
+     * Returns the list of all instructions in the treatment, both immediate and
+     * deferred.
+     *
+     * @return list of treatment instructions
+     */
+    List<Instruction> allInstructions();
+
+    /**
      * Returns the next table in the pipeline.
      * @return a table transition; may be null.
      */
--- a/core/api/src/test/java/org/onosproject/net/intent/IntentTestsMocks.java
View file @8ef6d3b
+++ b/core/api/src/test/java/org/onosproject/net/intent/IntentTestsMocks.java
View file @8ef6d3b
@@ -15,21 +15,8 @@
  */
 package org.onosproject.net.intent;
-import static org.onosproject.net.NetTestTools.createPath;
+import com.google.common.base.MoreObjects;
-import static org.onosproject.net.NetTestTools.did;
+import com.google.common.collect.ImmutableSet;
-import static org.onosproject.net.NetTestTools.link;
-
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Objects;
-import java.util.Set;
-import java.util.concurrent.atomic.AtomicLong;
-
 import org.onosproject.core.DefaultGroupId;
 import org.onosproject.core.GroupId;
 import org.onosproject.net.DeviceId;
@@ -67,8 +54,20 @@ import org.onosproject.net.topology.PathService;
 import org.onosproject.net.topology.TopologyVertex;
 import org.onosproject.store.Timestamp;
-import com.google.common.base.MoreObjects;
+import java.util.ArrayList;
-import com.google.common.collect.ImmutableSet;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicLong;
+
+import static org.onosproject.net.NetTestTools.createPath;
+import static org.onosproject.net.NetTestTools.did;
+import static org.onosproject.net.NetTestTools.link;
 /**
  * Common mocks used by the intent framework tests.
@@ -109,6 +108,11 @@ public class IntentTestsMocks {
         }
         @Override
+        public List<Instruction> allInstructions() {
+            return null;
+        }
+
+        @Override
         public Instructions.TableTypeTransition tableTransition() {
             return null;
         }
--- a/core/net/src/test/java/org/onosproject/net/flow/impl/FlowRuleManagerTest.java
View file @8ef6d3b
+++ b/core/net/src/test/java/org/onosproject/net/flow/impl/FlowRuleManagerTest.java
View file @8ef6d3b
@@ -15,19 +15,12 @@
  */
 package org.onosproject.net.flow.impl;
-import java.util.ArrayList;
+import com.google.common.collect.ImmutableList;
-import java.util.Arrays;
+import com.google.common.collect.ImmutableMap;
-import java.util.Collections;
+import com.google.common.collect.Lists;
-import java.util.HashMap;
+import com.google.common.collect.Sets;
-import java.util.List;
+import com.google.common.util.concurrent.ListenableFuture;
-import java.util.Map;
+import com.google.common.util.concurrent.MoreExecutors;
-import java.util.Set;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Executor;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
-import java.util.concurrent.atomic.AtomicLong;
-
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -68,12 +61,18 @@ import org.onosproject.net.provider.AbstractProvider;
 import org.onosproject.net.provider.ProviderId;
 import org.onosproject.store.trivial.impl.SimpleFlowRuleStore;
-import com.google.common.collect.ImmutableList;
+import java.util.ArrayList;
-import com.google.common.collect.ImmutableMap;
+import java.util.Arrays;
-import com.google.common.collect.Lists;
+import java.util.Collections;
-import com.google.common.collect.Sets;
+import java.util.HashMap;
-import com.google.common.util.concurrent.ListenableFuture;
+import java.util.List;
-import com.google.common.util.concurrent.MoreExecutors;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Executor;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicLong;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -573,6 +572,11 @@ public class FlowRuleManagerTest {
         }
         @Override
+        public List<Instruction> allInstructions() {
+            return null;
+        }
+
+        @Override
         public Instructions.TableTypeTransition tableTransition() {
             return null;
         }
--- a/core/store/dist/src/main/java/org/onosproject/store/statistic/impl/DistributedStatisticStore.java
View file @8ef6d3b
+++ b/core/store/dist/src/main/java/org/onosproject/store/statistic/impl/DistributedStatisticStore.java
View file @8ef6d3b
@@ -16,7 +16,6 @@
 package org.onosproject.store.statistic.impl;
 import com.google.common.collect.Sets;
-import org.apache.commons.collections.ListUtils;
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
@@ -45,7 +44,6 @@ import org.slf4j.Logger;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
@@ -290,15 +288,7 @@ public class DistributedStatisticStore implements StatisticStore {
     private ConnectPoint buildConnectPoint(FlowRule rule) {
         PortNumber port = getOutput(rule);
-        boolean hasGoto = rule.treatment().instructions()
-                .stream()
-                .anyMatch(i -> (i instanceof Instructions.GroupInstruction)
-                        || (i instanceof Instructions.TableTypeTransition));
-
         if (port == null) {
-            if (!hasGoto) {
-                log.debug("Rule {} has no output.", rule);
-            }
             return null;
         }
         ConnectPoint cp = new ConnectPoint(rule.deviceId(), port);
@@ -306,9 +296,7 @@ public class DistributedStatisticStore implements StatisticStore {
     }
     private PortNumber getOutput(FlowRule rule) {
-        List<Instruction> all = ListUtils.union(rule.treatment().immediate(),
+        for (Instruction i : rule.treatment().allInstructions()) {
-                                                rule.treatment().deferred());
-        for (Instruction i : all) {
             if (i.type() == Instruction.Type.OUTPUT) {
                 Instructions.OutputInstruction out = (Instructions.OutputInstruction) i;
                 return out.port();
--- a/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowEntryBuilder.java
View file @8ef6d3b
+++ b/providers/openflow/flow/src/main/java/org/onosproject/provider/of/flow/impl/FlowEntryBuilder.java
View file @8ef6d3b
@@ -172,8 +172,9 @@ public class FlowEntryBuilder {
     private List<OFInstruction> getInstructions(OFFlowMod entry) {
         switch (entry.getVersion()) {
             case OF_10:
-                return Lists.newArrayList(
+                return Lists.newArrayList(OFFactoryVer13.INSTANCE.instructions()
-                        OFFactoryVer13.INSTANCE.instructions().applyActions(entry.getActions()));
+                                                  .applyActions(
+                                                          entry.getActions()));
             case OF_11:
             case OF_12:
             case OF_13:
@@ -316,6 +317,9 @@ public class FlowEntryBuilder {
                 case POP_VLAN:
                     builder.popVlan();
                     break;
+                case PUSH_VLAN:
+                    builder.pushVlan();
+                    break;
                 case STRIP_VLAN:
                     builder.stripVlan();
                     break;
@@ -323,7 +327,6 @@ public class FlowEntryBuilder {
                 case SET_TP_SRC:
                 case POP_PBB:
                 case PUSH_PBB:
-                case PUSH_VLAN:
                 case SET_MPLS_LABEL:
                 case SET_MPLS_TC:
                 case SET_MPLS_TTL:
--- a/providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/GroupBucketEntryBuilder.java
View file @8ef6d3b
+++ b/providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/GroupBucketEntryBuilder.java
View file @8ef6d3b
@@ -84,7 +84,7 @@ public class GroupBucketEntryBuilder {
     /**
      * Builds a GroupBuckets.
      *
-     * @return GroupBuckets object, a list of GroupBuck
+     * @return GroupBuckets object, a list of GroupBuckets
      */
     public GroupBuckets build() {
         List<GroupBucket> bucketList = Lists.newArrayList();