Don't use the Match from the Firewall in Forwarding if a rule allowed the...

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).

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++;

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);