ONOS-1443: Group bucket statistics support and group CLI formatting

Change-Id: Iaa6d8ae1f9222eb9c29d14bf1615a7449e50c4d3

ONOS-1443: Group bucket statistics support and group CLI formatting
Change-Id: Iaa6d8ae1f9222eb9c29d14bf1615a7449e50c4d3
Srikanth Vavilapalli · Gerrit Code Review
Commit 10e75cd1dc81af19bbe1d9d4c1f8afb68d11c261 10e75cd1 1 parent 4db76e30
Showing 10 changed files with 224 additions and 19 deletions
cli/src/main/java/org/onosproject/cli/Comparators.java
cli/src/main/java/org/onosproject/cli/net/GroupsListCommand.java
core/api/src/main/java/org/onosproject/net/group/DefaultGroupBucket.java
core/api/src/main/java/org/onosproject/net/group/GroupBucket.java
core/api/src/main/java/org/onosproject/net/group/GroupBuckets.java
core/api/src/main/java/org/onosproject/net/group/GroupBucketEntry.java → core/api/src/main/java/org/onosproject/net/group/StoredGroupBucketEntry.java
core/store/dist/src/main/java/org/onosproject/store/group/impl/DistributedGroupStore.java
core/store/trivial/src/main/java/org/onosproject/store/trivial/impl/SimpleGroupStore.java
core/store/trivial/src/test/java/org/onosproject/store/trivial/impl/SimpleGroupStoreTest.java
providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProvider.java
--- a/cli/src/main/java/org/onosproject/cli/Comparators.java
View file @10e75cd
+++ b/cli/src/main/java/org/onosproject/cli/Comparators.java
View file @10e75cd
@@ -25,6 +25,7 @@ import org.onosproject.net.Element;
 import org.onosproject.net.ElementId;
 import org.onosproject.net.Port;
 import org.onosproject.net.flow.FlowRule;
+import org.onosproject.net.group.Group;
 import org.onosproject.net.host.PortAddresses;
 import org.onosproject.net.topology.TopologyCluster;
@@ -72,6 +73,13 @@ public final class Comparators {
         }
     };
+    public static final Comparator<Group> GROUP_COMPARATOR = new Comparator<Group>() {
+        @Override
+        public int compare(Group g1, Group g2) {
+            return Long.valueOf(g1.id().id()).compareTo(Long.valueOf(g2.id().id()));
+        }
+    };
+
     public static final Comparator<Port> PORT_COMPARATOR = new Comparator<Port>() {
         @Override
         public int compare(Port p1, Port p2) {
--- a/cli/src/main/java/org/onosproject/cli/net/GroupsListCommand.java
View file @10e75cd
+++ b/cli/src/main/java/org/onosproject/cli/net/GroupsListCommand.java
View file @10e75cd
@@ -15,11 +15,23 @@
  */
 package org.onosproject.cli.net;
+import static com.google.common.collect.Lists.newArrayList;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.SortedMap;
+import java.util.TreeMap;
+
+import org.apache.karaf.shell.commands.Argument;
 import org.apache.karaf.shell.commands.Command;
 import org.onosproject.cli.AbstractShellCommand;
+import org.onosproject.cli.Comparators;
+import org.onosproject.net.Device;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.device.DeviceService;
 import org.onosproject.net.group.Group;
+import org.onosproject.net.group.Group.GroupState;
+import org.onosproject.net.group.GroupBucket;
 import org.onosproject.net.group.GroupService;
 /**
@@ -30,23 +42,75 @@ import org.onosproject.net.group.GroupService;
 public class GroupsListCommand extends AbstractShellCommand {
     private static final String FORMAT =
-            "   key=%s, id=%s, state=%s, bytes=%s, packets=%s, appId=%s, buckets=%s";
+            "   id=%s, state=%s, bytes=%s, packets=%s, appId=%s";
+    private static final String BUCKET_FORMAT =
+            "   id=%s, bucket=%s, bytes=%s, packets=%s, actions=%s";
+
+    @Argument(index = 1, name = "uri", description = "Device ID",
+            required = false, multiValued = false)
+    String uri = null;
+
+    @Argument(index = 0, name = "state", description = "Group state",
+            required = false, multiValued = false)
+    String state;
     @Override
     protected void execute() {
         DeviceService deviceService = get(DeviceService.class);
         GroupService groupService = get(GroupService.class);
+        SortedMap<Device, List<Group>> sortedGroups =
+                getSortedGroups(deviceService, groupService);
-        deviceService.getDevices().forEach(d ->
+        sortedGroups.forEach((device, groups) -> printGroups(device.id(), groups));
-                printGroups(d.id(), groupService.getGroups(d.id()))
+    }
-        );
+
+    /**
+     * Returns the list of devices sorted using the device ID URIs.
+     *
+     * @param deviceService device service
+     * @param groupService group service
+     * @return sorted device list
+     */
+    protected SortedMap<Device, List<Group>>
+        getSortedGroups(DeviceService deviceService,
+                        GroupService groupService) {
+        SortedMap<Device, List<Group>> sortedGroups =
+                new TreeMap<>(Comparators.ELEMENT_COMPARATOR);
+        List<Group> groups;
+        GroupState s = null;
+        if (state != null && !state.equals("any")) {
+            s = GroupState.valueOf(state.toUpperCase());
+        }
+        Iterable<Device> devices = (uri == null) ? deviceService.getDevices() :
+                Collections.singletonList(deviceService.getDevice(DeviceId.deviceId(uri)));
+        for (Device d : devices) {
+            if (s == null) {
+                groups = newArrayList(groupService.getGroups(d.id()));
+            } else {
+                groups = newArrayList();
+                for (Group g : groupService.getGroups(d.id())) {
+                    if (g.state().equals(s)) {
+                        groups.add(g);
+                    }
+                }
+            }
+            groups.sort(Comparators.GROUP_COMPARATOR);
+            sortedGroups.put(d, groups);
+        }
+        return sortedGroups;
     }
-    private void printGroups(DeviceId deviceId, Iterable<Group> groups) {
+    private void printGroups(DeviceId deviceId, List<Group> groups) {
         print("deviceId=%s", deviceId);
         for (Group group : groups) {
-            print(FORMAT, group.appCookie(), group.id(), group.state(),
+            print(FORMAT, group.id().id(), group.state(),
-                  group.bytes(), group.packets(), group.appId(), group.buckets());
+                  group.bytes(), group.packets(), group.appId().name());
+            int i = 0;
+            for (GroupBucket bucket:group.buckets().buckets()) {
+                print(BUCKET_FORMAT, group.id().id(), ++i,
+                      bucket.bytes(), bucket.packets(),
+                      bucket.treatment().allInstructions());
+            }
         }
     }
 }
--- a/core/api/src/main/java/org/onosproject/net/group/DefaultGroupBucket.java
View file @10e75cd
+++ b/core/api/src/main/java/org/onosproject/net/group/DefaultGroupBucket.java
View file @10e75cd
@@ -38,12 +38,14 @@ import static com.google.common.base.Preconditions.checkNotNull;
  * in the group. A failover group bucket is associated with a
  * specific port or group that controls its liveness.
  */
-public final class DefaultGroupBucket implements GroupBucket {
+public final class DefaultGroupBucket implements GroupBucket, StoredGroupBucketEntry {
     private final GroupDescription.Type type;
     private final TrafficTreatment treatment;
     private final short weight;
     private final PortNumber watchPort;
     private final GroupId watchGroup;
+    private long packets;
+    private long bytes;
     /**
      * Group bucket constructor with the parameters.
@@ -223,6 +225,28 @@ public final class DefaultGroupBucket implements GroupBucket {
         return toStringHelper(this)
                 .add("type", type)
                 .add("treatment", treatment)
+                .add("packets", packets)
+                .add("bytes", bytes)
                 .toString();
     }
+
+    @Override
+    public long packets() {
+        return packets;
+    }
+
+    @Override
+    public long bytes() {
+        return bytes;
+    }
+
+    @Override
+    public void setPackets(long packets) {
+        this.packets = packets;
+    }
+
+    @Override
+    public void setBytes(long bytes) {
+        this.bytes = bytes;
+    }
 }
--- a/core/api/src/main/java/org/onosproject/net/group/GroupBucket.java
View file @10e75cd
+++ b/core/api/src/main/java/org/onosproject/net/group/GroupBucket.java
View file @10e75cd
@@ -64,4 +64,17 @@ public interface GroupBucket {
      */
     public GroupId watchGroup();
+    /**
+     * Returns the number of packets processed by this group bucket.
+     *
+     * @return number of packets
+     */
+    long packets();
+
+    /**
+     * Returns the number of bytes processed by this group bucket.
+     *
+     * @return number of bytes
+     */
+    long bytes();
 }
--- a/core/api/src/main/java/org/onosproject/net/group/GroupBuckets.java
View file @10e75cd
+++ b/core/api/src/main/java/org/onosproject/net/group/GroupBuckets.java
View file @10e75cd
@@ -70,7 +70,7 @@ public final class GroupBuckets {
     @Override
     public String toString() {
         return toStringHelper(this)
-                .add("buckets", buckets)
+                .add("buckets", buckets.toString())
                 .toString();
     }
 }
\ No newline at end of file
--- a/core/api/src/main/java/org/onosproject/net/group/GroupBucketEntry.java → core/api/src/main/java/org/onosproject/net/group/StoredGroupBucketEntry.java
View file @10e75cd
+++ b/core/api/src/main/java/org/onosproject/net/group/GroupBucketEntry.java → core/api/src/main/java/org/onosproject/net/group/StoredGroupBucketEntry.java
View file @10e75cd
@@ -20,18 +20,18 @@ package org.onosproject.net.group;
  * group object. A group bucket entry provides additional info of
  * group bucket like statistics...etc
  */
-public interface GroupBucketEntry extends GroupBucket {
+public interface StoredGroupBucketEntry extends GroupBucket {
     /**
-     * Returns Number of packets processed by bucket.
+     * Sets number of packets processed by this group bucket entry.
      *
-     * @return long
+     * @param packets a long value
      */
-    public long packets();
+    void setPackets(long packets);
     /**
-     * Returns Number of bytes processed by bucket.
+     * Sets number of bytes processed by this group bucket entry.
      *
-     * @return long
+     * @param bytes a long value
      */
-    public long bytes();
+    void setBytes(long bytes);
 }
--- a/core/store/dist/src/main/java/org/onosproject/store/group/impl/DistributedGroupStore.java
View file @10e75cd
+++ b/core/store/dist/src/main/java/org/onosproject/store/group/impl/DistributedGroupStore.java
View file @10e75cd
@@ -15,8 +15,10 @@
  */
 package org.onosproject.store.group.impl;
+import com.google.common.base.Optional;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.Iterables;
+
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
@@ -54,6 +56,7 @@ import org.onosproject.net.group.GroupKey;
 import org.onosproject.net.group.GroupOperation;
 import org.onosproject.net.group.GroupStore;
 import org.onosproject.net.group.GroupStoreDelegate;
+import org.onosproject.net.group.StoredGroupBucketEntry;
 import org.onosproject.net.group.StoredGroupEntry;
 import org.onosproject.store.AbstractStore;
 import org.onosproject.store.Timestamp;
@@ -624,6 +627,22 @@ public class DistributedGroupStore
                     group.id(),
                     group.deviceId());
             synchronized (existing) {
+                for (GroupBucket bucket:group.buckets().buckets()) {
+                    java.util.Optional<GroupBucket> matchingBucket =
+                            existing.buckets().buckets()
+                            .stream()
+                            .filter((existingBucket)->(existingBucket.equals(bucket)))
+                            .findFirst();
+                    if (matchingBucket.isPresent()) {
+                        ((StoredGroupBucketEntry) matchingBucket.
+                                get()).setPackets(bucket.packets());
+                        ((StoredGroupBucketEntry) matchingBucket.
+                                get()).setBytes(bucket.bytes());
+                    } else {
+                        log.warn("addOrUpdateGroupEntry: No matching "
+                                + "buckets to update stats");
+                    }
+                }
                 existing.setLife(group.life());
                 existing.setPackets(group.packets());
                 existing.setBytes(group.bytes());
--- a/core/store/trivial/src/main/java/org/onosproject/store/trivial/impl/SimpleGroupStore.java
View file @10e75cd
+++ b/core/store/trivial/src/main/java/org/onosproject/store/trivial/impl/SimpleGroupStore.java
View file @10e75cd
@@ -46,6 +46,7 @@ import org.onosproject.net.group.GroupKey;
 import org.onosproject.net.group.GroupOperation;
 import org.onosproject.net.group.GroupStore;
 import org.onosproject.net.group.GroupStoreDelegate;
+import org.onosproject.net.group.StoredGroupBucketEntry;
 import org.onosproject.net.group.StoredGroupEntry;
 import org.onosproject.store.AbstractStore;
 import org.slf4j.Logger;
@@ -416,6 +417,22 @@ public class SimpleGroupStore
         if (existing != null) {
             synchronized (existing) {
+                for (GroupBucket bucket:group.buckets().buckets()) {
+                    java.util.Optional<GroupBucket> matchingBucket =
+                            existing.buckets().buckets()
+                            .stream()
+                            .filter((existingBucket)->(existingBucket.equals(bucket)))
+                            .findFirst();
+                    if (matchingBucket.isPresent()) {
+                        ((StoredGroupBucketEntry) matchingBucket.
+                                get()).setPackets(bucket.packets());
+                        ((StoredGroupBucketEntry) matchingBucket.
+                                get()).setBytes(bucket.bytes());
+                    } else {
+                        log.warn("addOrUpdateGroupEntry: No matching "
+                                + "buckets to update stats");
+                    }
+                }
                 existing.setLife(group.life());
                 existing.setPackets(group.packets());
                 existing.setBytes(group.bytes());
@@ -424,7 +441,7 @@ public class SimpleGroupStore
                     event = new GroupEvent(Type.GROUP_ADDED, existing);
                 } else {
                     if (existing.state() == GroupState.PENDING_UPDATE) {
-                        existing.setState(GroupState.PENDING_UPDATE);
+                        existing.setState(GroupState.ADDED);
                     }
                     event = new GroupEvent(Type.GROUP_UPDATED, existing);
                 }
--- a/core/store/trivial/src/test/java/org/onosproject/store/trivial/impl/SimpleGroupStoreTest.java
View file @10e75cd
+++ b/core/store/trivial/src/test/java/org/onosproject/store/trivial/impl/SimpleGroupStoreTest.java
View file @10e75cd
@@ -34,6 +34,7 @@ import org.onosproject.net.DeviceId;
 import org.onosproject.net.PortNumber;
 import org.onosproject.net.flow.DefaultTrafficTreatment;
 import org.onosproject.net.flow.TrafficTreatment;
+import org.onosproject.net.group.DefaultGroup;
 import org.onosproject.net.group.DefaultGroupBucket;
 import org.onosproject.net.group.DefaultGroupDescription;
 import org.onosproject.net.group.DefaultGroupKey;
@@ -46,6 +47,8 @@ import org.onosproject.net.group.GroupKey;
 import org.onosproject.net.group.GroupOperation;
 import org.onosproject.net.group.GroupStore.UpdateType;
 import org.onosproject.net.group.GroupStoreDelegate;
+import org.onosproject.net.group.StoredGroupBucketEntry;
+import org.onosproject.net.group.StoredGroupEntry;
 import com.google.common.collect.Iterables;
@@ -100,6 +103,26 @@ public class SimpleGroupStoreTest {
                 createdGroupId = event.subject().id();
                 assertEquals(Group.GroupState.ADDED,
                              event.subject().state());
+            } else if (expectedEvent == GroupEvent.Type.GROUP_UPDATED) {
+                createdGroupId = event.subject().id();
+                assertEquals(true,
+                             event.subject().buckets().
+                             buckets().containsAll(createdBuckets.buckets()));
+                assertEquals(true,
+                             createdBuckets.buckets().
+                             containsAll(event.subject().buckets().buckets()));
+                for (GroupBucket bucket:event.subject().buckets().buckets()) {
+                    java.util.Optional<GroupBucket> matched = createdBuckets.buckets()
+                            .stream()
+                            .filter((expected) -> expected.equals(bucket))
+                            .findFirst();
+                    assertEquals(matched.get().packets(),
+                                 bucket.packets());
+                    assertEquals(matched.get().bytes(),
+                                 bucket.bytes());
+                }
+                assertEquals(Group.GroupState.ADDED,
+                             event.subject().state());
             } else if (expectedEvent == GroupEvent.Type.GROUP_UPDATE_REQUESTED) {
                 assertEquals(Group.GroupState.PENDING_UPDATE,
                              event.subject().state());
@@ -243,13 +266,33 @@ public class SimpleGroupStoreTest {
     // Testing addOrUpdateGroupEntry operation from southbound
     private void testUpdateGroupEntryFromSB(GroupKey currKey) {
         Group existingGroup = simpleGroupStore.getGroup(D1, currKey);
+        int totalPkts = 0;
+        int totalBytes = 0;
+        List<GroupBucket> newBucketList = new ArrayList<GroupBucket>();
+        for (GroupBucket bucket:existingGroup.buckets().buckets()) {
+            StoredGroupBucketEntry newBucket =
+                    (StoredGroupBucketEntry)
+                    DefaultGroupBucket.createSelectGroupBucket(bucket.treatment());
+            newBucket.setPackets(10);
+            newBucket.setBytes(10 * 256 * 8);
+            totalPkts += 10;
+            totalBytes += 10 * 256 * 8;
+            newBucketList.add(newBucket);
+        }
+        GroupBuckets updatedBuckets = new GroupBuckets(newBucketList);
+        Group updatedGroup = new DefaultGroup(existingGroup.id(),
+                                              existingGroup.deviceId(),
+                                              existingGroup.type(),
+                                              updatedBuckets);
+        ((StoredGroupEntry) updatedGroup).setPackets(totalPkts);
+        ((StoredGroupEntry) updatedGroup).setBytes(totalBytes);
         InternalGroupStoreDelegate updateGroupEntryDelegate =
                 new InternalGroupStoreDelegate(currKey,
-                                               existingGroup.buckets(),
+                                               updatedBuckets,
                                                GroupEvent.Type.GROUP_UPDATED);
         simpleGroupStore.setDelegate(updateGroupEntryDelegate);
-        simpleGroupStore.addOrUpdateGroupEntry(existingGroup);
+        simpleGroupStore.addOrUpdateGroupEntry(updatedGroup);
         simpleGroupStore.unsetDelegate(updateGroupEntryDelegate);
     }
--- a/providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProvider.java
View file @10e75cd
+++ b/providers/openflow/group/src/main/java/org/onosproject/provider/of/group/impl/OpenFlowGroupProvider.java
View file @10e75cd
@@ -17,6 +17,7 @@
 package org.onosproject.provider.of.group.impl;
 import com.google.common.collect.Maps;
+
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
@@ -34,6 +35,7 @@ import org.onosproject.net.group.GroupOperations;
 import org.onosproject.net.group.GroupProvider;
 import org.onosproject.net.group.GroupProviderRegistry;
 import org.onosproject.net.group.GroupProviderService;
+import org.onosproject.net.group.StoredGroupBucketEntry;
 import org.onosproject.net.provider.AbstractProvider;
 import org.onosproject.net.provider.ProviderId;
 import org.onosproject.openflow.controller.Dpid;
@@ -42,6 +44,7 @@ import org.onosproject.openflow.controller.OpenFlowEventListener;
 import org.onosproject.openflow.controller.OpenFlowSwitch;
 import org.onosproject.openflow.controller.OpenFlowSwitchListener;
 import org.onosproject.openflow.controller.RoleState;
+import org.projectfloodlight.openflow.protocol.OFBucketCounter;
 import org.projectfloodlight.openflow.protocol.OFErrorMsg;
 import org.projectfloodlight.openflow.protocol.OFErrorType;
 import org.projectfloodlight.openflow.protocol.OFGroupDescStatsEntry;
@@ -208,6 +211,7 @@ public class OpenFlowGroupProvider extends AbstractProvider implements GroupProv
         Map<Integer, Group> groups = Maps.newHashMap();
+
         for (OFGroupDescStatsEntry entry: groupDescStatsReply.getEntries()) {
             int id = entry.getGroup().getGroupNumber();
             GroupId groupId = new DefaultGroupId(id);
@@ -226,6 +230,19 @@ public class OpenFlowGroupProvider extends AbstractProvider implements GroupProv
                 group.setLife(entry.getDurationSec());
                 group.setPackets(entry.getPacketCount().getValue());
                 group.setReferenceCount(entry.getRefCount());
+                int bucketIndex = 0;
+                for (OFBucketCounter bucketStats:entry.getBucketStats()) {
+                    ((StoredGroupBucketEntry) group.buckets().buckets()
+                            .get(bucketIndex))
+                            .setPackets(bucketStats
+                                        .getPacketCount().getValue());
+                    ((StoredGroupBucketEntry) group.buckets().buckets()
+                            .get(bucketIndex))
+                            .setBytes(entry.getBucketStats()
+                                      .get(bucketIndex)
+                                      .getByteCount().getValue());
+                    bucketIndex++;
+                }
             }
         }