cosmetic and minor changes to OpenFlowController
- No functional changes
- sliced out stats proessing into separate method to reduce method length
- removed unused code
- use ImmutableList for defensive copying
Change-Id: I1a49d2cb94a4c52c926d5900a0c4384ea1ba1ec9
diff --git a/protocols/openflow/ctl/src/main/java/org/onosproject/openflow/controller/impl/OpenFlowControllerImpl.java b/protocols/openflow/ctl/src/main/java/org/onosproject/openflow/controller/impl/OpenFlowControllerImpl.java
index 474f824..3c123bc 100644
--- a/protocols/openflow/ctl/src/main/java/org/onosproject/openflow/controller/impl/OpenFlowControllerImpl.java
+++ b/protocols/openflow/ctl/src/main/java/org/onosproject/openflow/controller/impl/OpenFlowControllerImpl.java
@@ -16,7 +16,7 @@
package org.onosproject.openflow.controller.impl;
import com.google.common.collect.ArrayListMultimap;
-import com.google.common.collect.Lists;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Multimap;
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
@@ -59,7 +59,6 @@
import org.projectfloodlight.openflow.protocol.OFPacketIn;
import org.projectfloodlight.openflow.protocol.OFPortDesc;
import org.projectfloodlight.openflow.protocol.OFPortStatsEntry;
-import org.projectfloodlight.openflow.protocol.OFPortStatsReply;
import org.projectfloodlight.openflow.protocol.OFPortStatus;
import org.projectfloodlight.openflow.protocol.OFStatsReply;
import org.projectfloodlight.openflow.protocol.OFStatsReplyFlags;
@@ -70,9 +69,9 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
-import java.util.LinkedList;
import java.util.List;
import java.util.Optional;
import java.util.Set;
@@ -171,6 +170,8 @@
protected Multimap<Dpid, OFGroupDescStatsEntry> fullGroupDescStats =
ArrayListMultimap.create();
+ // deprecated in 1.11.0, no longer referenced from anywhere
+ @Deprecated
protected Multimap<Dpid, OFPortStatsEntry> fullPortStats =
ArrayListMultimap.create();
@@ -303,15 +304,8 @@
return future;
}
- // CHECKSTYLE IGNORE MethodLength FOR NEXT 300 LINES
@Override
public void processPacket(Dpid dpid, OFMessage msg) {
- Collection<OFFlowStatsEntry> flowStats;
- Collection<OFTableStatsEntry> tableStats;
- Collection<OFGroupStatsEntry> groupStats;
- Collection<OFGroupDescStatsEntry> groupDescStats;
- Collection<OFQueueStatsEntry> queueStatsEntries;
-
OpenFlowSwitch sw = this.getSwitch(dpid);
// Check if someone is waiting for this message
@@ -356,123 +350,7 @@
executorErrorMsgs.execute(new OFMessageHandler(dpid, msg));
break;
case STATS_REPLY:
- OFStatsReply reply = (OFStatsReply) msg;
- switch (reply.getStatsType()) {
- case QUEUE:
- queueStatsEntries = publishQueueStats(dpid, (OFQueueStatsReply) reply);
- if (queueStatsEntries != null) {
- OFQueueStatsReply.Builder rep =
- OFFactories.getFactory(msg.getVersion()).buildQueueStatsReply();
- rep.setEntries(Lists.newLinkedList(queueStatsEntries));
- rep.setXid(reply.getXid());
- executorMsgs.execute(new OFMessageHandler(dpid, rep.build()));
- }
- break;
- case PORT_DESC:
- for (OpenFlowSwitchListener l : ofSwitchListener) {
- l.switchChanged(dpid);
- }
- break;
- case FLOW:
- flowStats = publishFlowStats(dpid, (OFFlowStatsReply) reply);
- if (flowStats != null) {
- OFFlowStatsReply.Builder rep =
- OFFactories.getFactory(msg.getVersion()).buildFlowStatsReply();
- rep.setEntries(Lists.newLinkedList(flowStats));
- rep.setXid(reply.getXid());
- executorMsgs.execute(new OFMessageHandler(dpid, rep.build()));
- }
- break;
- case TABLE:
- tableStats = publishTableStats(dpid, (OFTableStatsReply) reply);
- if (tableStats != null) {
- OFTableStatsReply.Builder rep =
- OFFactories.getFactory(msg.getVersion()).buildTableStatsReply();
- rep.setEntries(Lists.newLinkedList(tableStats));
- executorMsgs.execute(new OFMessageHandler(dpid, rep.build()));
- }
- break;
- case GROUP:
- groupStats = publishGroupStats(dpid, (OFGroupStatsReply) reply);
- if (groupStats != null) {
- OFGroupStatsReply.Builder rep =
- OFFactories.getFactory(msg.getVersion()).buildGroupStatsReply();
- rep.setEntries(Lists.newLinkedList(groupStats));
- rep.setXid(reply.getXid());
- executorMsgs.execute(new OFMessageHandler(dpid, rep.build()));
- }
- break;
- case GROUP_DESC:
- groupDescStats = publishGroupDescStats(dpid,
- (OFGroupDescStatsReply) reply);
- if (groupDescStats != null) {
- OFGroupDescStatsReply.Builder rep =
- OFFactories.getFactory(msg.getVersion()).buildGroupDescStatsReply();
- rep.setEntries(Lists.newLinkedList(groupDescStats));
- rep.setXid(reply.getXid());
- executorMsgs.execute(new OFMessageHandler(dpid, rep.build()));
- }
- break;
- case PORT:
- executorMsgs.execute(new OFMessageHandler(dpid, reply));
- break;
- case METER:
- executorMsgs.execute(new OFMessageHandler(dpid, reply));
- break;
- case EXPERIMENTER:
- if (reply instanceof OFCalientFlowStatsReply) {
- // Convert Calient flow statistics to regular flow stats
- // TODO: parse remaining fields such as power levels etc. when we have proper monitoring API
- if (sw == null) {
- log.error("Switch {} is not found", dpid);
- break;
- }
- OFFlowStatsReply.Builder fsr = sw.factory().buildFlowStatsReply();
- List<OFFlowStatsEntry> entries = new LinkedList<>();
- for (OFCalientFlowStatsEntry entry : ((OFCalientFlowStatsReply) msg).getEntries()) {
-
- // Single instruction, i.e., output to port
- OFActionOutput action = OFFactories
- .getFactory(msg.getVersion())
- .actions()
- .buildOutput()
- .setPort(entry.getOutPort())
- .build();
- OFInstruction instruction = OFFactories
- .getFactory(msg.getVersion())
- .instructions()
- .applyActions(Collections.singletonList(action));
- OFFlowStatsEntry fs = sw.factory().buildFlowStatsEntry()
- .setMatch(entry.getMatch())
- .setTableId(entry.getTableId())
- .setDurationSec(entry.getDurationSec())
- .setDurationNsec(entry.getDurationNsec())
- .setPriority(entry.getPriority())
- .setIdleTimeout(entry.getIdleTimeout())
- .setHardTimeout(entry.getHardTimeout())
- .setFlags(entry.getFlags())
- .setCookie(entry.getCookie())
- .setInstructions(Collections.singletonList(instruction))
- .build();
- entries.add(fs);
- }
- fsr.setEntries(entries);
-
- flowStats = publishFlowStats(dpid, fsr.build());
- if (flowStats != null) {
- OFFlowStatsReply.Builder rep =
- OFFactories.getFactory(msg.getVersion()).buildFlowStatsReply();
- rep.setEntries(Lists.newLinkedList(flowStats));
- executorMsgs.execute(new OFMessageHandler(dpid, rep.build()));
- }
- } else {
- executorMsgs.execute(new OFMessageHandler(dpid, reply));
- }
- break;
- default:
- log.warn("Discarding unknown stats reply type {}", reply.getStatsType());
- break;
- }
+ processStatsReply(dpid, (OFStatsReply) msg);
break;
case BARRIER_REPLY:
if (errorMsgs.containsKey(msg.getXid())) {
@@ -515,6 +393,132 @@
}
}
+ private void processStatsReply(Dpid dpid, OFStatsReply reply) {
+ switch (reply.getStatsType()) {
+ case QUEUE:
+ Collection<OFQueueStatsEntry> queueStatsEntries = publishQueueStats(dpid, (OFQueueStatsReply) reply);
+ if (queueStatsEntries != null) {
+ OFQueueStatsReply.Builder rep =
+ OFFactories.getFactory(reply.getVersion()).buildQueueStatsReply();
+ rep.setEntries(ImmutableList.copyOf(queueStatsEntries));
+ rep.setXid(reply.getXid());
+ executorMsgs.execute(new OFMessageHandler(dpid, rep.build()));
+ }
+ break;
+
+ case PORT_DESC:
+ for (OpenFlowSwitchListener l : ofSwitchListener) {
+ l.switchChanged(dpid);
+ }
+ break;
+
+ case FLOW:
+ Collection<OFFlowStatsEntry> flowStats = publishFlowStats(dpid, (OFFlowStatsReply) reply);
+ if (flowStats != null) {
+ OFFlowStatsReply.Builder rep =
+ OFFactories.getFactory(reply.getVersion()).buildFlowStatsReply();
+ rep.setEntries(ImmutableList.copyOf(flowStats));
+ rep.setXid(reply.getXid());
+ executorMsgs.execute(new OFMessageHandler(dpid, rep.build()));
+ }
+ break;
+
+ case TABLE:
+ Collection<OFTableStatsEntry> tableStats = publishTableStats(dpid, (OFTableStatsReply) reply);
+ if (tableStats != null) {
+ OFTableStatsReply.Builder rep =
+ OFFactories.getFactory(reply.getVersion()).buildTableStatsReply();
+ rep.setEntries(ImmutableList.copyOf(tableStats));
+ executorMsgs.execute(new OFMessageHandler(dpid, rep.build()));
+ }
+ break;
+
+ case GROUP:
+ Collection<OFGroupStatsEntry> groupStats = publishGroupStats(dpid, (OFGroupStatsReply) reply);
+ if (groupStats != null) {
+ OFGroupStatsReply.Builder rep =
+ OFFactories.getFactory(reply.getVersion()).buildGroupStatsReply();
+ rep.setEntries(ImmutableList.copyOf(groupStats));
+ rep.setXid(reply.getXid());
+ executorMsgs.execute(new OFMessageHandler(dpid, rep.build()));
+ }
+ break;
+
+ case GROUP_DESC:
+ Collection<OFGroupDescStatsEntry> groupDescStats = publishGroupDescStats(dpid,
+ (OFGroupDescStatsReply) reply);
+ if (groupDescStats != null) {
+ OFGroupDescStatsReply.Builder rep =
+ OFFactories.getFactory(reply.getVersion()).buildGroupDescStatsReply();
+ rep.setEntries(ImmutableList.copyOf(groupDescStats));
+ rep.setXid(reply.getXid());
+ executorMsgs.execute(new OFMessageHandler(dpid, rep.build()));
+ }
+ break;
+
+ case PORT:
+ executorMsgs.execute(new OFMessageHandler(dpid, reply));
+ break;
+
+ case METER:
+ executorMsgs.execute(new OFMessageHandler(dpid, reply));
+ break;
+
+ case EXPERIMENTER:
+ if (reply instanceof OFCalientFlowStatsReply) {
+ OpenFlowSwitch sw = this.getSwitch(dpid);
+ // Convert Calient flow statistics to regular flow stats
+ // TODO: parse remaining fields such as power levels etc. when we have proper monitoring API
+ if (sw == null) {
+ log.error("Switch {} is not found", dpid);
+ break;
+ }
+ OFFlowStatsReply.Builder fsr = sw.factory().buildFlowStatsReply();
+ List<OFFlowStatsEntry> entries = new ArrayList<>();
+ for (OFCalientFlowStatsEntry entry : ((OFCalientFlowStatsReply) reply).getEntries()) {
+
+ // Single instruction, i.e., output to port
+ OFActionOutput action = sw.factory()
+ .actions()
+ .buildOutput()
+ .setPort(entry.getOutPort())
+ .build();
+ OFInstruction instruction = sw.factory()
+ .instructions()
+ .applyActions(Collections.singletonList(action));
+ OFFlowStatsEntry fs = sw.factory().buildFlowStatsEntry()
+ .setMatch(entry.getMatch())
+ .setTableId(entry.getTableId())
+ .setDurationSec(entry.getDurationSec())
+ .setDurationNsec(entry.getDurationNsec())
+ .setPriority(entry.getPriority())
+ .setIdleTimeout(entry.getIdleTimeout())
+ .setHardTimeout(entry.getHardTimeout())
+ .setFlags(entry.getFlags())
+ .setCookie(entry.getCookie())
+ .setInstructions(Collections.singletonList(instruction))
+ .build();
+ entries.add(fs);
+ }
+ fsr.setEntries(entries);
+
+ flowStats = publishFlowStats(dpid, fsr.build());
+ if (flowStats != null) {
+ OFFlowStatsReply.Builder rep =
+ sw.factory().buildFlowStatsReply();
+ rep.setEntries(ImmutableList.copyOf(flowStats));
+ executorMsgs.execute(new OFMessageHandler(dpid, rep.build()));
+ }
+ } else {
+ executorMsgs.execute(new OFMessageHandler(dpid, reply));
+ }
+ break;
+ default:
+ log.warn("Discarding unknown stats reply type {}", reply.getStatsType());
+ break;
+ }
+ }
+
private synchronized Collection<OFFlowStatsEntry> publishFlowStats(Dpid dpid,
OFFlowStatsReply reply) {
//TODO: Get rid of synchronized
@@ -555,15 +559,6 @@
return null;
}
- private synchronized Collection<OFPortStatsEntry> publishPortStats(Dpid dpid,
- OFPortStatsReply reply) {
- fullPortStats.putAll(dpid, reply.getEntries());
- if (!reply.getFlags().contains(OFStatsReplyFlags.REPLY_MORE)) {
- return fullPortStats.removeAll(dpid);
- }
- return null;
- }
-
private synchronized Collection<OFQueueStatsEntry> publishQueueStats(Dpid dpid, OFQueueStatsReply reply) {
fullQueueStats.putAll(dpid, reply.getEntries());
if (!reply.getFlags().contains(OFStatsReplyFlags.REPLY_MORE)) {