diff --git a/src/main/java/net/floodlightcontroller/staticflowentry/StaticFlowEntryPusher.java b/src/main/java/net/floodlightcontroller/staticflowentry/StaticFlowEntryPusher.java index 7154396b37dbd56b3ac4c10769ae0793302c8543..1b8c405f47ecc2f642e70da9e5613cccbe37d400 100644 --- a/src/main/java/net/floodlightcontroller/staticflowentry/StaticFlowEntryPusher.java +++ b/src/main/java/net/floodlightcontroller/staticflowentry/StaticFlowEntryPusher.java @@ -466,41 +466,55 @@ implements IOFSwitchListener, IFloodlightModule, IStaticFlowEntryPusherService, entriesFromStorage.put(dpid, new HashMap<String, OFFlowMod>()); List<OFMessage> outQueue = new ArrayList<OFMessage>(); + + /* For every flow per dpid, decide how to "add" the flow. */ for (String entry : entriesToAdd.get(dpid).keySet()) { OFFlowMod newFlowMod = entriesToAdd.get(dpid).get(entry); OFFlowMod oldFlowMod = null; + String dpidOldFlowMod = entry2dpid.get(entry); if (dpidOldFlowMod != null) { oldFlowMod = entriesFromStorage.get(dpidOldFlowMod).remove(entry); } - // pre-existing case. should modify or delete, but not add - if (oldFlowMod != null && newFlowMod != null) { - // set the new flow mod to modify a pre-existing rule if these fields match + + /* Modify, which can be either a Flow MODIFY_STRICT or a Flow DELETE_STRICT with a side of Flow ADD */ + if (oldFlowMod != null && newFlowMod != null) { + /* MODIFY_STRICT b/c the match is still the same */ if (oldFlowMod.getMatch().equals(newFlowMod.getMatch()) - && oldFlowMod.getCookie() == newFlowMod.getCookie() + && oldFlowMod.getCookie().equals(newFlowMod.getCookie()) && oldFlowMod.getPriority() == newFlowMod.getPriority()) { + entriesFromStorage.get(dpid).put(entry, newFlowMod); + entry2dpid.put(entry, dpid); newFlowMod = FlowModUtils.toFlowModifyStrict(newFlowMod); - // if they don't match delete the old flow + outQueue.add(newFlowMod); + /* DELETE_STRICT and then ADD b/c the match is now different */ } else { oldFlowMod = FlowModUtils.toFlowDeleteStrict(oldFlowMod); + OFFlowAdd addTmp = FlowModUtils.toFlowAdd(newFlowMod); + /* If the flow's dpid and the current switch we're looking at are the same, add to the queue. */ if (dpidOldFlowMod.equals(dpid)) { outQueue.add(oldFlowMod); + outQueue.add(addTmp); + /* Otherwise, go ahead and send the flows now (since queuing them will send to the wrong switch). */ } else { writeOFMessageToSwitch(DatapathId.of(dpidOldFlowMod), oldFlowMod); + writeOFMessageToSwitch(DatapathId.of(dpid), FlowModUtils.toFlowAdd(newFlowMod)); } + entriesFromStorage.get(dpid).put(entry, addTmp); + entry2dpid.put(entry, dpid); } - } - // new case. should add a flow, not modify or delete - if (newFlowMod != null) { + /* Add a brand-new flow with ADD */ + } else if (newFlowMod != null && oldFlowMod == null) { OFFlowAdd addTmp = FlowModUtils.toFlowAdd(newFlowMod); entriesFromStorage.get(dpid).put(entry, addTmp); outQueue.add(addTmp); - entry2dpid.put(entry, dpid); - } else { + /* Something strange happened, so remove the flow */ + } else if (newFlowMod == null) { entriesFromStorage.get(dpid).remove(entry); entry2dpid.remove(entry); } } + /* Batch-write all queued messages to the switch */ writeOFMessagesToSwitch(DatapathId.of(dpid), outQueue); } } diff --git a/src/main/java/net/floodlightcontroller/util/ActionUtils.java b/src/main/java/net/floodlightcontroller/util/ActionUtils.java index 28d9a206f09cc2aae84f4fed2f6d1e3860124ed1..589f17402a34e0bb0f5ff810055db25458663623 100644 --- a/src/main/java/net/floodlightcontroller/util/ActionUtils.java +++ b/src/main/java/net/floodlightcontroller/util/ActionUtils.java @@ -645,32 +645,24 @@ public class ActionUtils { explanation="A static flow entry contained an invalid subaction", recommendation=LogMessageDoc.REPORT_CONTROLLER_BUG) private static OFActionOutput decode_output(String actionToDecode, OFVersion version, Logger log) { - Matcher n = Pattern.compile("((controller)|(local)|(ingress-port)|(normal)|(flood))").matcher(actionToDecode); + Matcher n = Pattern.compile("((all)|(controller)|(local)|(ingress-port)|(normal)|(flood))").matcher(actionToDecode); OFActionOutput.Builder ab = OFFactories.getFactory(version).actions().buildOutput(); OFPort port = OFPort.ANY; if (n.matches()) { - if (n.group(1) != null) { - try { - port = OFPort.of(Integer.parseInt(n.group(1))); - } - catch (NumberFormatException e) { - log.debug("Invalid port in: '{}' (error ignored)", actionToDecode); - return null; - } - } - else if (n.group(2) != null) + if (n.group(1) != null && n.group(1).equals("all")) port = OFPort.ALL; - else if (n.group(3) != null) + else if (n.group(1) != null && n.group(1).equals("controller")) port = OFPort.CONTROLLER; - else if (n.group(4) != null) + else if (n.group(1) != null && n.group(1).equals("local")) port = OFPort.LOCAL; - else if (n.group(5) != null) + else if (n.group(1) != null && n.group(1).equals("ingress-port")) port = OFPort.IN_PORT; - else if (n.group(6) != null) + else if (n.group(1) != null && n.group(1).equals("normal")) port = OFPort.NORMAL; - else if (n.group(7) != null) + else if (n.group(1) != null && n.group(1).equals("flood")) port = OFPort.FLOOD; ab.setPort(port); + ab.setMaxLen(Integer.MAX_VALUE); log.debug("action {}", ab.build()); return ab.build(); } @@ -678,6 +670,7 @@ public class ActionUtils { try { port = OFPort.of(Integer.parseInt(actionToDecode)); ab.setPort(port); + ab.setMaxLen(Integer.MAX_VALUE); return ab.build(); } catch (NumberFormatException e) { log.error("Could not parse Integer port: '{}'", actionToDecode); diff --git a/src/test/java/net/floodlightcontroller/flowcache/PortDownReconciliationTest.java b/src/test/java/net/floodlightcontroller/flowcache/PortDownReconciliationTest.java index 9eb15be4fe2e86b199d2883ebbc2cc8ffe94c1be..4a82b3756e7f2e50b4a47caae48827f44d9cb5b0 100644 --- a/src/test/java/net/floodlightcontroller/flowcache/PortDownReconciliationTest.java +++ b/src/test/java/net/floodlightcontroller/flowcache/PortDownReconciliationTest.java @@ -36,19 +36,16 @@ import org.easymock.CaptureType; import org.easymock.EasyMock; import org.junit.Before; import org.junit.Test; -import org.openflow.protocol.OFFlowMod; -import org.openflow.protocol.OFMatch; -import org.openflow.protocol.OFMatchWithSwDpid; -import org.openflow.protocol.OFMessage; -import org.openflow.protocol.OFStatisticsRequest; -import org.openflow.protocol.OFType; -import org.openflow.protocol.action.OFAction; -import org.openflow.protocol.action.OFActionOutput; -import org.openflow.protocol.statistics.OFFlowStatisticsReply; -import org.openflow.protocol.statistics.OFFlowStatisticsRequest; -import org.openflow.protocol.statistics.OFStatistics; -import org.openflow.protocol.statistics.OFStatisticsType; -import org.openflow.util.U16; +import org.projectfloodlight.openflow.protocol.OFFlowMod; +import org.projectfloodlight.openflow.protocol.match.Match; +import org.projectfloodlight.openflow.protocol.OFMessage; +import org.projectfloodlight.openflow.protocol.OFStatsRequest; +import org.projectfloodlight.openflow.protocol.OFType; +import org.projectfloodlight.openflow.protocol.action.OFAction; +import org.projectfloodlight.openflow.protocol.action.OFActionOutput; +import org.projectfloodlight.openflow.protocol.OFFlowStatsReply; +import org.projectfloodlight.openflow.protocol.OFStatsType; +import org.projectfloodlight.openflow.types.U16; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/src/test/java/net/floodlightcontroller/learningswitch/LearningSwitchTest.java b/src/test/java/net/floodlightcontroller/learningswitch/LearningSwitchTest.java index de9800d27803690f993cffa7dd13f11cc332c010..05b48a91b4a3e95a049cd43b2638d4eaeca9ca9b 100644 --- a/src/test/java/net/floodlightcontroller/learningswitch/LearningSwitchTest.java +++ b/src/test/java/net/floodlightcontroller/learningswitch/LearningSwitchTest.java @@ -32,10 +32,6 @@ import net.floodlightcontroller.core.IOFMessageListener; import net.floodlightcontroller.core.IOFSwitch; import net.floodlightcontroller.core.internal.IOFSwitchService; import net.floodlightcontroller.core.module.FloodlightModuleContext; -import net.floodlightcontroller.debugcounter.IDebugCounterService; -import net.floodlightcontroller.debugcounter.MockDebugCounterService; -import net.floodlightcontroller.debugevent.IDebugEventService; -import net.floodlightcontroller.debugevent.MockDebugEventService; import net.floodlightcontroller.packet.Data; import net.floodlightcontroller.packet.Ethernet; import net.floodlightcontroller.packet.IPacket; @@ -45,15 +41,12 @@ import net.floodlightcontroller.restserver.IRestApiService; import net.floodlightcontroller.restserver.RestApiServer; import net.floodlightcontroller.test.FloodlightTestCase; -import org.easymock.EasyMock; -import org.hamcrest.core.AnyOf; import org.junit.Before; import org.junit.Test; import org.projectfloodlight.openflow.protocol.OFFactories; import org.projectfloodlight.openflow.protocol.OFFactory; import org.projectfloodlight.openflow.protocol.OFFlowAdd; import org.projectfloodlight.openflow.protocol.OFFlowModFlags; -import org.projectfloodlight.openflow.protocol.OFMessage; import org.projectfloodlight.openflow.protocol.OFPacketIn; import org.projectfloodlight.openflow.protocol.OFPacketInReason; import org.projectfloodlight.openflow.protocol.OFPacketOut; @@ -90,8 +83,6 @@ public class LearningSwitchTest extends FloodlightTestCase { private OFFactory factory = OFFactories.getFactory(OFVersion.OF_13); private FloodlightModuleContext fmc; private RestApiServer restApiService; - private IDebugCounterService dcs; - private IDebugEventService des; @Override @Before @@ -168,8 +159,6 @@ public class LearningSwitchTest extends FloodlightTestCase { this.learningSwitch = new LearningSwitch(); this.restApiService = new RestApiServer(); - this.dcs = new MockDebugCounterService(); - this.des = new MockDebugEventService(); this.fmc = new FloodlightModuleContext(); fmc.addService(IOFSwitchService.class, getMockSwitchService()); diff --git a/src/test/java/net/floodlightcontroller/staticflowentry/StaticFlowTests.java b/src/test/java/net/floodlightcontroller/staticflowentry/StaticFlowTests.java index 315c062be3c32207081c2aa4994bc71d4f110b51..785052957d20f8d186ab807d47c787c17e543448 100644 --- a/src/test/java/net/floodlightcontroller/staticflowentry/StaticFlowTests.java +++ b/src/test/java/net/floodlightcontroller/staticflowentry/StaticFlowTests.java @@ -89,7 +89,8 @@ public class StaticFlowTests extends FloodlightTestCase { .setActions(actions) .setBufferId(OFBufferId.NO_BUFFER) .setOutPort(OFPort.ANY) - .setPriority(Short.MAX_VALUE) + .setPriority(Integer.MAX_VALUE) + .setXid(4) .build(); } @@ -103,8 +104,9 @@ public class StaticFlowTests extends FloodlightTestCase { TestRule2.put(COLUMN_SWITCH, TestSwitch1DPID); // setup match Match match; + TestRule2.put(COLUMN_DL_TYPE, "0x800"); TestRule2.put(COLUMN_NW_DST, "192.168.1.0/24"); - match = MatchUtils.fromString("nw_dst=192.168.1.0/24", factory.getVersion()); + match = MatchUtils.fromString("dl_type=0x800,nw_dst=192.168.1.0/24", factory.getVersion()); // setup actions List<OFAction> actions = new LinkedList<OFAction>(); TestRule2.put(COLUMN_ACTIONS, "output=1"); @@ -114,7 +116,8 @@ public class StaticFlowTests extends FloodlightTestCase { .setActions(actions) .setBufferId(OFBufferId.NO_BUFFER) .setOutPort(OFPort.ANY) - .setPriority(Short.MAX_VALUE) + .setPriority(Integer.MAX_VALUE) + .setXid(5) .build(); } @@ -125,7 +128,6 @@ public class StaticFlowTests extends FloodlightTestCase { private IOFSwitchService switchService; private IOFSwitch mockSwitch; private Capture<OFMessage> writeCapture; - private Capture<FloodlightContext> contextCapture; private Capture<List<OFMessage>> writeCaptureList; private long dpid; private IStorageSourceService storage; @@ -137,7 +139,7 @@ public class StaticFlowTests extends FloodlightTestCase { // setup match Match match; TestRule3.put(COLUMN_DL_DST, "00:20:30:40:50:60"); - TestRule3.put(COLUMN_DL_VLAN, 4096); + TestRule3.put(COLUMN_DL_VLAN, 96); match = MatchUtils.fromString("dl_dst=00:20:30:40:50:60,dl_vlan=96", factory.getVersion()); // setup actions TestRule3.put(COLUMN_ACTIONS, "output=controller"); @@ -148,7 +150,8 @@ public class StaticFlowTests extends FloodlightTestCase { .setActions(actions) .setBufferId(OFBufferId.NO_BUFFER) .setOutPort(OFPort.ANY) - .setPriority(Short.MAX_VALUE) + .setPriority(Integer.MAX_VALUE) + .setXid(6) .build(); } @@ -177,7 +180,6 @@ public class StaticFlowTests extends FloodlightTestCase { for(int i = 0; i < goodActions.size(); i++) { assertEquals(goodActions.get(i), testActions.get(i)); } - } @@ -193,14 +195,14 @@ public class StaticFlowTests extends FloodlightTestCase { writeCapture = new Capture<OFMessage>(CaptureType.ALL); writeCaptureList = new Capture<List<OFMessage>>(CaptureType.ALL); - //OFMessageSafeOutStream mockOutStream = createNiceMock(OFMessageSafeOutStream.class); mockSwitch.write(capture(writeCapture)); expectLastCall().anyTimes(); mockSwitch.write(capture(writeCaptureList)); expectLastCall().anyTimes(); mockSwitch.flush(); expectLastCall().anyTimes(); - + expect(mockSwitch.getOFFactory()).andReturn(factory).anyTimes(); + replay(mockSwitch); FloodlightModuleContext fmc = new FloodlightModuleContext(); fmc.addService(IStorageSourceService.class, storage); @@ -209,12 +211,12 @@ public class StaticFlowTests extends FloodlightTestCase { MockFloodlightProvider mockFloodlightProvider = getMockFloodlightProvider(); Map<DatapathId, IOFSwitch> switchMap = new HashMap<DatapathId, IOFSwitch>(); switchMap.put(DatapathId.of(dpid), mockSwitch); - // NO ! expect(mockFloodlightProvider.getSwitches()).andReturn(switchMap).anyTimes(); getMockSwitchService().setSwitches(switchMap); fmc.addService(IFloodlightProviderService.class, mockFloodlightProvider); RestApiServer restApi = new RestApiServer(); fmc.addService(IRestApiService.class, restApi); fmc.addService(IOFSwitchService.class, switchService); + restApi.init(fmc); staticFlowEntryPusher.init(fmc); staticFlowEntryPusher.startUp(fmc); // again, to hack unittest @@ -230,8 +232,15 @@ public class StaticFlowTests extends FloodlightTestCase { //expect(mockSwitch.getOutputStream()).andReturn(mockOutStream).anyTimes(); // if someone calls getId(), return this dpid instead + resetToNice(mockSwitch); + mockSwitch.write(capture(writeCapture)); + expectLastCall().anyTimes(); + mockSwitch.write(capture(writeCaptureList)); + expectLastCall().anyTimes(); + mockSwitch.flush(); + expectLastCall().anyTimes(); + expect(mockSwitch.getOFFactory()).andReturn(factory).anyTimes(); expect(mockSwitch.getId()).andReturn(DatapathId.of(dpid)).anyTimes(); - expect(mockSwitch.getId()).andReturn(DatapathId.of(TestSwitch1DPID)).anyTimes(); replay(mockSwitch); // hook the static pusher up to the fake switch @@ -253,8 +262,6 @@ public class StaticFlowTests extends FloodlightTestCase { verifyFlowMod(thirdFlowMod, FlowMod3); writeCapture.reset(); - contextCapture.reset(); - // delete two rules and verify they've been removed // this should invoke staticFlowPusher.rowsDeleted() @@ -274,7 +281,8 @@ public class StaticFlowTests extends FloodlightTestCase { // add rules back to make sure that staticFlowPusher.rowsInserted() works writeCapture.reset(); - FlowMod2= FlowModUtils.toFlowAdd(FlowMod1); + FlowMod2 = FlowModUtils.toFlowAdd(FlowMod2); + FlowMod2 = FlowMod2.createBuilder().setXid(12).build(); storage.insertRow(StaticFlowEntryPusher.TABLE_NAME, TestRule2); assertEquals(2, staticFlowEntryPusher.countEntries()); assertEquals(1, writeCaptureList.getValues().size()); @@ -284,7 +292,6 @@ public class StaticFlowTests extends FloodlightTestCase { OFFlowMod firstAdd = (OFFlowMod) outList.get(0); verifyFlowMod(firstAdd, FlowMod2); writeCapture.reset(); - contextCapture.reset(); writeCaptureList.reset(); // now try an overwriting update, calling staticFlowPusher.rowUpdated() @@ -299,13 +306,13 @@ public class StaticFlowTests extends FloodlightTestCase { FlowMod3 = FlowModUtils.toFlowDeleteStrict(FlowMod3); verifyFlowMod(removeFlowMod, FlowMod3); FlowMod3 = FlowModUtils.toFlowAdd(FlowMod3); - FlowMod3 = FlowMod3.createBuilder().setMatch(MatchUtils.fromString("dl_dst=00:20:30:40:50:60,dl_vlan=333", factory.getVersion())).build(); + FlowMod3 = FlowMod3.createBuilder().setMatch(MatchUtils.fromString("dl_dst=00:20:30:40:50:60,dl_vlan=333", factory.getVersion())).setXid(14).build(); OFFlowMod updateFlowMod = (OFFlowMod) outList.get(1); verifyFlowMod(updateFlowMod, FlowMod3); writeCaptureList.reset(); // now try an action modifying update, calling staticFlowPusher.rowUpdated() - TestRule3.put(COLUMN_ACTIONS, "output=controller,strip-vlan"); // added strip-vlan + TestRule3.put(COLUMN_ACTIONS, "output=controller,pop_vlan"); // added pop-vlan storage.updateRow(StaticFlowEntryPusher.TABLE_NAME, TestRule3); assertEquals(2, staticFlowEntryPusher.countEntries()); assertEquals(1, writeCaptureList.getValues().size()); @@ -315,8 +322,8 @@ public class StaticFlowTests extends FloodlightTestCase { OFFlowMod modifyFlowMod = (OFFlowMod) outList.get(0); FlowMod3 = FlowModUtils.toFlowModifyStrict(FlowMod3); List<OFAction> modifiedActions = FlowMod3.getActions(); - modifiedActions.add(factory.actions().stripVlan()); // add the new action to what we should expect - FlowMod3 = FlowMod3.createBuilder().setActions(modifiedActions).build(); + modifiedActions.add(factory.actions().popVlan()); // add the new action to what we should expect + FlowMod3 = FlowMod3.createBuilder().setActions(modifiedActions).setXid(19).build(); verifyFlowMod(modifyFlowMod, FlowMod3); }