From f3eeca0abba44fb3c772662e3f958f5ddddb046d Mon Sep 17 00:00:00 2001
From: Ryan Izard <rizard@g.clemson.edu>
Date: Fri, 20 Feb 2015 11:18:47 -0500
Subject: [PATCH] Don't use the Match from the Firewall in Forwarding if a rule
 allowed the packet. If a packet has been allowed, that means the Forwarding
 module can send it where it needs to go matching on its header files (which
 have already been given the OK by the Firewall). If a Firewall rule is very
 general, e.g. allow all packets through switch 1, then the first packet that
 traverses the switch will cause Forwarding to insert a general from from port
 A to port B with no specific hheader field matches (since they weren't
 specified in the Firewall rule).

---
 .../forwarding/Forwarding.java                | 128 ++++++++----------
 .../testmodule/TestModule.java                |   9 +-
 2 files changed, 63 insertions(+), 74 deletions(-)

diff --git a/src/main/java/net/floodlightcontroller/forwarding/Forwarding.java b/src/main/java/net/floodlightcontroller/forwarding/Forwarding.java
index eb15b903d..8e9920621 100644
--- a/src/main/java/net/floodlightcontroller/forwarding/Forwarding.java
+++ b/src/main/java/net/floodlightcontroller/forwarding/Forwarding.java
@@ -251,76 +251,64 @@ public class Forwarding extends ForwardingBase implements IFloodlightModule {
 										srcDap.getPort(),
 										dstDap.getSwitchDPID(),
 										dstDap.getPort(), U64.of(0)); //cookie = 0, i.e., default route
-										if (route != null) {
-											if (log.isTraceEnabled()) {
-												log.trace("pushRoute inPort={} route={} " +
-														"destination={}:{}",
-														new Object[] { inPort, route,
-														dstDap.getSwitchDPID(),
-														dstDap.getPort()});
-											}
-											U64 cookie = AppCookie.makeCookie(FORWARDING_APP_ID, 0);
-
-											// if there is prior routing decision use route's match
-											Match routeMatch = null;
-											IRoutingDecision decision = null;
-											if (cntx != null) {
-												decision = IRoutingDecision.rtStore.get(cntx, IRoutingDecision.CONTEXT_DECISION);
-											}
-											if (decision != null) {
-												routeMatch = decision.getMatch();
-											} else {
-												// The packet in match will only contain the port number.
-												// We need to add in specifics for the hosts we're routing between.
-												Ethernet eth = IFloodlightProviderService.bcStore.get(cntx, IFloodlightProviderService.CONTEXT_PI_PAYLOAD);
-												VlanVid vlan = VlanVid.ofVlan(eth.getVlanID());
-												MacAddress srcMac = eth.getSourceMACAddress();
-												MacAddress dstMac = eth.getDestinationMACAddress();
-												
-												// A retentive builder will remember all MatchFields of the parent the builder was generated from
-												// With a normal builder, all parent MatchFields will be lost if any MatchFields are added, mod, del
-												// TODO (This is a bug in Loxigen and the retentive builder is a workaround.)
-												Match.Builder mb = sw.getOFFactory().buildMatch();
-												mb.setExact(MatchField.IN_PORT, inPort)
-												.setExact(MatchField.ETH_SRC, srcMac)
-												.setExact(MatchField.ETH_DST, dstMac);
-												
-												if (!vlan.equals(VlanVid.ZERO)) {
-													mb.setExact(MatchField.VLAN_VID, OFVlanVidMatch.ofVlanVid(vlan));
-												}
-												
-												// TODO Detect switch type and match to create hardware-implemented flow
-												// TODO Set option in config file to support specific or MAC-only matches
-												if (eth.getEtherType() == Ethernet.TYPE_IPv4) {
-													IPv4 ip = (IPv4) eth.getPayload();
-													IPv4Address srcIp = ip.getSourceAddress();
-													IPv4Address dstIp = ip.getDestinationAddress();
-													mb.setExact(MatchField.IPV4_SRC, srcIp)
-													.setExact(MatchField.IPV4_DST, dstIp)
-													.setExact(MatchField.ETH_TYPE, EthType.IPv4);
-													
-													if (ip.getProtocol().equals(IpProtocol.TCP)) {
-														TCP tcp = (TCP) ip.getPayload();
-														mb.setExact(MatchField.IP_PROTO, IpProtocol.TCP)
-														.setExact(MatchField.TCP_SRC, tcp.getSourcePort())
-														.setExact(MatchField.TCP_DST, tcp.getDestinationPort());
-													} else if (ip.getProtocol().equals(IpProtocol.UDP)) {
-														UDP udp = (UDP) ip.getPayload();
-														mb.setExact(MatchField.IP_PROTO, IpProtocol.UDP)
-														.setExact(MatchField.UDP_SRC, udp.getSourcePort())
-														.setExact(MatchField.UDP_DST, udp.getDestinationPort());
-													}	
-												} else if (eth.getEtherType() == Ethernet.TYPE_ARP) {
-													mb.setExact(MatchField.ETH_TYPE, EthType.ARP);
-												} 
-												
-												routeMatch = mb.build();
-											}
-
-											pushRoute(route, routeMatch, pi, sw.getId(), cookie,
-													cntx, requestFlowRemovedNotifn, false,
-													OFFlowModCommand.ADD);
-										}
+						if (route != null) {
+							if (log.isTraceEnabled()) {
+								log.trace("pushRoute inPort={} route={} " +
+										"destination={}:{}",
+										new Object[] { inPort, route,
+										dstDap.getSwitchDPID(),
+										dstDap.getPort()});
+							}
+							U64 cookie = AppCookie.makeCookie(FORWARDING_APP_ID, 0);
+
+							// The packet in match will only contain the port number.
+							// We need to add in specifics for the hosts we're routing between.
+							Ethernet eth = IFloodlightProviderService.bcStore.get(cntx, IFloodlightProviderService.CONTEXT_PI_PAYLOAD);
+							VlanVid vlan = VlanVid.ofVlan(eth.getVlanID());
+							MacAddress srcMac = eth.getSourceMACAddress();
+							MacAddress dstMac = eth.getDestinationMACAddress();
+
+							// A retentive builder will remember all MatchFields of the parent the builder was generated from
+							// With a normal builder, all parent MatchFields will be lost if any MatchFields are added, mod, del
+							// TODO (This is a bug in Loxigen and the retentive builder is a workaround.)
+							Match.Builder mb = sw.getOFFactory().buildMatch();
+							mb.setExact(MatchField.IN_PORT, inPort)
+							.setExact(MatchField.ETH_SRC, srcMac)
+							.setExact(MatchField.ETH_DST, dstMac);
+
+							if (!vlan.equals(VlanVid.ZERO)) {
+								mb.setExact(MatchField.VLAN_VID, OFVlanVidMatch.ofVlanVid(vlan));
+							}
+
+							// TODO Detect switch type and match to create hardware-implemented flow
+							// TODO Set option in config file to support specific or MAC-only matches
+							if (eth.getEtherType() == Ethernet.TYPE_IPv4) {
+								IPv4 ip = (IPv4) eth.getPayload();
+								IPv4Address srcIp = ip.getSourceAddress();
+								IPv4Address dstIp = ip.getDestinationAddress();
+								mb.setExact(MatchField.IPV4_SRC, srcIp)
+								.setExact(MatchField.IPV4_DST, dstIp)
+								.setExact(MatchField.ETH_TYPE, EthType.IPv4);
+
+								if (ip.getProtocol().equals(IpProtocol.TCP)) {
+									TCP tcp = (TCP) ip.getPayload();
+									mb.setExact(MatchField.IP_PROTO, IpProtocol.TCP)
+									.setExact(MatchField.TCP_SRC, tcp.getSourcePort())
+									.setExact(MatchField.TCP_DST, tcp.getDestinationPort());
+								} else if (ip.getProtocol().equals(IpProtocol.UDP)) {
+									UDP udp = (UDP) ip.getPayload();
+									mb.setExact(MatchField.IP_PROTO, IpProtocol.UDP)
+									.setExact(MatchField.UDP_SRC, udp.getSourcePort())
+									.setExact(MatchField.UDP_DST, udp.getDestinationPort());
+								}	
+							} else if (eth.getEtherType() == Ethernet.TYPE_ARP) {
+								mb.setExact(MatchField.ETH_TYPE, EthType.ARP);
+							}
+
+							pushRoute(route, mb.build(), pi, sw.getId(), cookie,
+									cntx, requestFlowRemovedNotifn, false,
+									OFFlowModCommand.ADD);
+						}
 					}
 					iSrcDaps++;
 					iDstDaps++;
diff --git a/src/main/java/net/floodlightcontroller/testmodule/TestModule.java b/src/main/java/net/floodlightcontroller/testmodule/TestModule.java
index e72a25058..5c2c1b02a 100644
--- a/src/main/java/net/floodlightcontroller/testmodule/TestModule.java
+++ b/src/main/java/net/floodlightcontroller/testmodule/TestModule.java
@@ -43,6 +43,7 @@ import org.projectfloodlight.openflow.protocol.meterband.OFMeterBand;
 import org.projectfloodlight.openflow.protocol.meterband.OFMeterBandDrop;
 import org.projectfloodlight.openflow.protocol.oxm.OFOxm;
 import org.projectfloodlight.openflow.protocol.oxm.OFOxmEthSrc;
+import org.projectfloodlight.openflow.protocol.ver13.OFMeterModCommandSerializerVer13;
 import org.projectfloodlight.openflow.types.ArpOpcode;
 import org.projectfloodlight.openflow.types.DatapathId;
 import org.projectfloodlight.openflow.types.EthType;
@@ -125,7 +126,6 @@ public class TestModule implements IFloodlightModule, IOFSwitchListener {
 	@Override
 	public void switchAdded(DatapathId switchId) {
 		OFFactory factory = switchService.getSwitch(switchId).getOFFactory();
-		
 		/*
 		 * An attempt at meters, but they aren't supported anywhere, yet... 
 		 * OFMeterBand mb = factory.meterBands().buildDrop()
@@ -134,12 +134,13 @@ public class TestModule implements IFloodlightModule, IOFSwitchListener {
 				.build();
 		ArrayList<OFMeterBand> mbl = new ArrayList<OFMeterBand>();
 		mbl.add(mb);
-		
+				
 		OFMeterMod mm = factory.buildMeterMod()
 				.setMeters(mbl)
 				.setMeterId(1)
-				.setCommand(0)
-				.build(); */
+				.setCommand(OFMeterModCommandSerializerVer13.ADD_VAL) 
+				.build(); 
+		// This is a bug. You should be able to directly do OFMeterModCommand.ADD */
 		
 		/*HashSet<OFTableConfig> tblCfg = new HashSet<OFTableConfig>();
 		tblCfg.add(OFTableConfig.TABLE_MISS_CONTROLLER);
-- 
GitLab