[VOL-1035] Flow decomposition tests and code cleanup

Change-Id: Ie739160772e515721ab45a4bcffbb9ce7764b2e3
diff --git a/rw_core/flow_decomposition/flow_decomposer.go b/rw_core/flow_decomposition/flow_decomposer.go
index 54a6761..233465b 100644
--- a/rw_core/flow_decomposition/flow_decomposer.go
+++ b/rw_core/flow_decomposition/flow_decomposer.go
@@ -308,7 +308,16 @@
 
 //frequently used extractors
 
-func GetActions(flow *ofp.OfpFlowStats) []*ofp.OfpAction {
+func excludeAction(action *ofp.OfpAction, exclude ...ofp.OfpActionType) bool {
+	for _, actionToExclude := range exclude {
+		if action.Type == actionToExclude {
+			return true
+		}
+	}
+	return false
+}
+
+func GetActions(flow *ofp.OfpFlowStats, exclude ...ofp.OfpActionType) []*ofp.OfpAction {
 	if flow == nil {
 		return nil
 	}
@@ -318,13 +327,32 @@
 			if instActions == nil {
 				return nil
 			}
-			return instActions.Actions
+			if len(exclude) == 0 {
+				return instActions.Actions
+			} else {
+				filteredAction := make([]*ofp.OfpAction, 0)
+				for _, action := range instActions.Actions {
+					if !excludeAction(action, exclude...) {
+						filteredAction = append(filteredAction, action)
+					}
+				}
+				return filteredAction
+			}
 		}
 	}
 	return nil
 }
 
-func GetOfbFields(flow *ofp.OfpFlowStats) []*ofp.OfpOxmOfbField {
+func excludeOxmOfbField(field *ofp.OfpOxmOfbField, exclude ...ofp.OxmOfbFieldTypes) bool {
+	for _, fieldToExclude := range exclude {
+		if field.Type == fieldToExclude {
+			return true
+		}
+	}
+	return false
+}
+
+func GetOfbFields(flow *ofp.OfpFlowStats, exclude ...ofp.OxmOfbFieldTypes) []*ofp.OfpOxmOfbField {
 	if flow == nil || flow.Match == nil || flow.Match.Type != ofp.OfpMatchType_OFPMT_OXM {
 		return nil
 	}
@@ -334,7 +362,17 @@
 			ofbFields = append(ofbFields, field.GetOfbField())
 		}
 	}
-	return ofbFields
+	if len(exclude) == 0 {
+		return ofbFields
+	} else {
+		filteredFields := make([]*ofp.OfpOxmOfbField, 0)
+		for _, ofbField := range ofbFields {
+			if !excludeOxmOfbField(ofbField, exclude...) {
+				filteredFields = append(filteredFields, ofbField)
+			}
+		}
+		return filteredFields
+	}
 }
 
 func GetOutPort(flow *ofp.OfpFlowStats) uint32 {
@@ -516,6 +554,7 @@
 	}
 	group.Desc = &ofp.OfpGroupDesc{Type: mod.Type, GroupId: mod.GroupId, Buckets: mod.Buckets}
 	group.Stats = &ofp.OfpGroupStats{GroupId: mod.GroupId}
+	//TODO do we need to instantiate bucket bins?
 	return group
 }
 
@@ -635,8 +674,8 @@
 	return FlowStatsEntryFromFlowModMessage(MkSimpleFlowMod(matchFields, fa.Actions, fa.Command, fa.KV))
 }
 
-func MkGroupStat(groupId uint32, buckets []*ofp.OfpBucket, command *ofp.OfpGroupModCommand) *ofp.OfpGroupEntry {
-	return GroupEntryFromGroupMod(MkMulticastGroupMod(groupId, buckets, command))
+func MkGroupStat(ga *fu.GroupArgs) *ofp.OfpGroupEntry {
+	return GroupEntryFromGroupMod(MkMulticastGroupMod(ga.GroupId, ga.Buckets, ga.Command))
 }
 
 type FlowDecomposer struct {
@@ -663,16 +702,22 @@
 	for _, flow := range flows.Items {
 		decomposedRules = fd.decomposeFlow(agent, flow, groupMap)
 		for deviceId, flowAndGroups := range decomposedRules.Rules {
-			fmt.Println("!!!!!", deviceId, flowAndGroups)
-			deviceRules.Rules[deviceId] = fu.NewFlowsAndGroups()
+			deviceRules.CreateEntryIfNotExist(deviceId)
 			deviceRules.Rules[deviceId].AddFrom(flowAndGroups)
 		}
 	}
 	return deviceRules
 }
 
-func (fd *FlowDecomposer) processControllerBoundFlow(agent coreIf.LogicalDeviceAgent, route []graph.RouteHop, inPortNo uint32, outPortNo uint32, flow *ofp.OfpFlowStats, deviceRules *fu.DeviceRules) *fu.DeviceRules {
+//processControllerBoundFlow decomposes trap flows
+func (fd *FlowDecomposer) processControllerBoundFlow(agent coreIf.LogicalDeviceAgent, route []graph.RouteHop,
+	inPortNo uint32, outPortNo uint32, flow *ofp.OfpFlowStats) *fu.DeviceRules {
+
 	log.Debugw("trap-flow", log.Fields{"inPortNo": inPortNo, "outPortNo": outPortNo, "flow": flow})
+	deviceRules := fu.NewDeviceRules()
+
+	egressHop := route[1]
+
 	fg := fu.NewFlowsAndGroups()
 	if agent.GetDeviceGraph().IsRootPort(inPortNo) {
 		log.Debug("trap-nni")
@@ -680,16 +725,12 @@
 		fa = &fu.FlowArgs{
 			KV: fu.OfpFlowModArgs{"priority": uint64(flow.Priority), "cookie": flow.Cookie},
 			MatchFields: []*ofp.OfpOxmOfbField{
-				InPort(route[1].Egress), //egress_hop.egress_port.port_no
+				InPort(egressHop.Egress),
 			},
 			Actions: GetActions(flow),
 		}
 		// Augment the matchfields with the ofpfields from the flow
-		for _, val := range GetOfbFields(flow) {
-			if val.Type != IN_PORT {
-				fa.MatchFields = append(fa.MatchFields, val)
-			}
-		}
+		fa.MatchFields = append(fa.MatchFields, GetOfbFields(flow, IN_PORT)...)
 		fg.AddFlow(MkFlowStat(fa))
 	} else {
 		// Trap flow for UNI port
@@ -698,7 +739,7 @@
 		//inPortNo is 0 for wildcard input case, do not include upstream port for 4000 flow in input
 		var inPorts []uint32
 		if inPortNo == 0 {
-			inPorts = agent.GetWildcardInputPorts(route[1].Egress) // exclude egress_hop.egress_port.port_no
+			inPorts = agent.GetWildcardInputPorts(egressHop.Egress) // exclude egress_hop.egress_port.port_no
 		} else {
 			inPorts = []uint32{inPortNo}
 		}
@@ -708,41 +749,37 @@
 			fa = &fu.FlowArgs{
 				KV: fu.OfpFlowModArgs{"priority": uint64(flow.Priority), "cookie": flow.Cookie},
 				MatchFields: []*ofp.OfpOxmOfbField{
-					InPort(route[1].Ingress), //egress_hop.ingress_port.port_no
+					InPort(egressHop.Ingress),
 					VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | inputPort),
 				},
 				Actions: []*ofp.OfpAction{
 					PushVlan(0x8100),
 					SetField(VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 4000)),
-					Output(route[1].Egress),
+					Output(egressHop.Egress),
 				},
 			}
 			// Augment the matchfields with the ofpfields from the flow
-			for _, val := range GetOfbFields(flow) {
-				if val.Type != IN_PORT && val.Type != VLAN_VID {
-					fa.MatchFields = append(fa.MatchFields, val)
-				}
-			}
+			fa.MatchFields = append(fa.MatchFields, GetOfbFields(flow, IN_PORT, VLAN_VID)...)
 			fg.AddFlow(MkFlowStat(fa))
 
 			// Downstream flow
 			fa = &fu.FlowArgs{
 				KV: fu.OfpFlowModArgs{"priority": uint64(flow.Priority)},
 				MatchFields: []*ofp.OfpOxmOfbField{
-					InPort(route[1].Egress), //egress_hop.ingress_port.port_no
+					InPort(egressHop.Egress),
 					VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 4000),
 					VlanPcp(0),
 					Metadata_ofp(uint64(inputPort)),
 				},
 				Actions: []*ofp.OfpAction{
 					PopVlan(),
-					Output(route[1].Ingress),
+					Output(egressHop.Ingress),
 				},
 			}
 			fg.AddFlow(MkFlowStat(fa))
 		}
 	}
-	deviceRules.AddFlowsAndGroup(route[1].DeviceID, fg)
+	deviceRules.AddFlowsAndGroup(egressHop.DeviceID, fg)
 	return deviceRules
 }
 
@@ -750,8 +787,15 @@
 // upstream needs to get Q-in-Q treatment and that this is expressed via two flow rules, the first using the
 // goto-statement. We also assume that the inner tag is applied at the ONU, while the outer tag is
 // applied at the OLT
-func (fd *FlowDecomposer) processUpstreamNonControllerBoundFlow(agent coreIf.LogicalDeviceAgent, route []graph.RouteHop, inPortNo uint32, outPortNo uint32, flow *ofp.OfpFlowStats, deviceRules *fu.DeviceRules) *fu.DeviceRules {
+func (fd *FlowDecomposer) processUpstreamNonControllerBoundFlow(agent coreIf.LogicalDeviceAgent,
+	route []graph.RouteHop, inPortNo uint32, outPortNo uint32, flow *ofp.OfpFlowStats) *fu.DeviceRules {
+
 	log.Debugw("upstream-non-controller-bound-flow", log.Fields{"inPortNo": inPortNo, "outPortNo": outPortNo})
+	deviceRules := fu.NewDeviceRules()
+
+	ingressHop := route[0]
+	egressHop := route[1]
+
 	if HasNextTable(flow) {
 		log.Debugw("has-next-table", log.Fields{"table_id": flow.TableId})
 		if outPortNo != 0 {
@@ -761,21 +805,19 @@
 		fa = &fu.FlowArgs{
 			KV: fu.OfpFlowModArgs{"priority": uint64(flow.Priority), "cookie": flow.Cookie},
 			MatchFields: []*ofp.OfpOxmOfbField{
-				InPort(route[0].Ingress), //ingress_hop.ingress_port.port_no
+				InPort(ingressHop.Ingress),
 			},
 			Actions: GetActions(flow),
 		}
 		// Augment the matchfields with the ofpfields from the flow
-		for _, val := range GetOfbFields(flow) {
-			if val.Type != IN_PORT {
-				fa.MatchFields = append(fa.MatchFields, val)
-			}
-		}
-		// Agument the Actions
-		fa.Actions = append(fa.Actions, Output(route[0].Egress))
+		fa.MatchFields = append(fa.MatchFields, GetOfbFields(flow, IN_PORT)...)
+
+		// Augment the Actions
+		fa.Actions = append(fa.Actions, Output(ingressHop.Egress))
+
 		fg := fu.NewFlowsAndGroups()
 		fg.AddFlow(MkFlowStat(fa))
-		deviceRules.AddFlowsAndGroup(route[0].DeviceID, fg)
+		deviceRules.AddFlowsAndGroup(ingressHop.DeviceID, fg)
 	} else {
 		var actions []ofp.OfpActionType
 		var isOutputTypeInActions bool
@@ -791,41 +833,33 @@
 			fa = &fu.FlowArgs{
 				KV: fu.OfpFlowModArgs{"priority": uint64(flow.Priority), "cookie": flow.Cookie},
 				MatchFields: []*ofp.OfpOxmOfbField{
-					InPort(route[0].Ingress), //ingress_hop.ingress_port.port_no
+					InPort(ingressHop.Ingress),
 				},
 				Actions: []*ofp.OfpAction{
-					Output(route[0].Egress),
+					Output(ingressHop.Egress),
 				},
 			}
 			// Augment the matchfields with the ofpfields from the flow
-			for _, val := range GetOfbFields(flow) {
-				if val.Type != IN_PORT {
-					fa.MatchFields = append(fa.MatchFields, val)
-				}
-			}
+			fa.MatchFields = append(fa.MatchFields, GetOfbFields(flow, IN_PORT)...)
 			fg := fu.NewFlowsAndGroups()
 			fg.AddFlow(MkFlowStat(fa))
-			deviceRules.AddFlowsAndGroup(route[0].DeviceID, fg)
+			deviceRules.AddFlowsAndGroup(ingressHop.DeviceID, fg)
 
 			// parent device flow
 			fa = &fu.FlowArgs{
 				KV: fu.OfpFlowModArgs{"priority": uint64(flow.Priority), "cookie": flow.Cookie},
 				MatchFields: []*ofp.OfpOxmOfbField{
-					InPort(route[1].Ingress), //egress_hop.ingress_port.port_no
+					InPort(egressHop.Ingress), //egress_hop.ingress_port.port_no
 				},
 				Actions: []*ofp.OfpAction{
-					Output(route[1].Egress),
+					Output(egressHop.Egress),
 				},
 			}
 			// Augment the matchfields with the ofpfields from the flow
-			for _, val := range GetOfbFields(flow) {
-				if val.Type != IN_PORT {
-					fa.MatchFields = append(fa.MatchFields, val)
-				}
-			}
+			fa.MatchFields = append(fa.MatchFields, GetOfbFields(flow, IN_PORT)...)
 			fg = fu.NewFlowsAndGroups()
 			fg.AddFlow(MkFlowStat(fa))
-			deviceRules.AddFlowsAndGroup(route[1].DeviceID, fg)
+			deviceRules.AddFlowsAndGroup(egressHop.DeviceID, fg)
 		} else {
 			if outPortNo == 0 {
 				log.Warnw("outPort-should-be-specified", log.Fields{"outPortNo": outPortNo})
@@ -834,38 +868,38 @@
 			fa = &fu.FlowArgs{
 				KV: fu.OfpFlowModArgs{"priority": uint64(flow.Priority), "cookie": flow.Cookie},
 				MatchFields: []*ofp.OfpOxmOfbField{
-					InPort(route[1].Ingress), //egress_hop.ingress_port.port_no
+					InPort(egressHop.Ingress),
 				},
 			}
 			// Augment the matchfields with the ofpfields from the flow
-			for _, val := range GetOfbFields(flow) {
-				if val.Type != IN_PORT {
-					fa.MatchFields = append(fa.MatchFields, val)
-				}
-			}
-			// Augment the Actions
-			updatedAction := make([]*ofp.OfpAction, 0)
-			for _, action := range GetActions(flow) {
-				if action.Type != OUTPUT {
-					updatedAction = append(updatedAction, action)
-				}
-			}
-			updatedAction = append(updatedAction, Output(route[1].Egress))
-			fa.Actions = updatedAction
+			fa.MatchFields = append(fa.MatchFields, GetOfbFields(flow, IN_PORT)...)
+
+			//Augment the actions
+			filteredAction := GetActions(flow, OUTPUT)
+			filteredAction = append(filteredAction, Output(egressHop.Egress))
+			fa.Actions = filteredAction
+
 			fg := fu.NewFlowsAndGroups()
 			fg.AddFlow(MkFlowStat(fa))
-			deviceRules.AddFlowsAndGroup(route[1].DeviceID, fg)
+			deviceRules.AddFlowsAndGroup(egressHop.DeviceID, fg)
 		}
 	}
 	return deviceRules
 }
 
-func (fd *FlowDecomposer) processDownstreamFlowWithNextTable(agent coreIf.LogicalDeviceAgent, route []graph.RouteHop, inPortNo uint32, outPortNo uint32, flow *ofp.OfpFlowStats, deviceRules *fu.DeviceRules) *fu.DeviceRules {
+// processDownstreamFlowWithNextTable decomposes downstream flows containing next table ID instructions
+func (fd *FlowDecomposer) processDownstreamFlowWithNextTable(agent coreIf.LogicalDeviceAgent, route []graph.RouteHop,
+	inPortNo uint32, outPortNo uint32, flow *ofp.OfpFlowStats) *fu.DeviceRules {
+
 	log.Debugw("downstream-flow-with-next-table", log.Fields{"inPortNo": inPortNo, "outPortNo": outPortNo})
+	deviceRules := fu.NewDeviceRules()
+
 	if outPortNo != 0 {
 		log.Warnw("outPort-should-not-be-specified", log.Fields{"outPortNo": outPortNo})
 	}
 	ingressHop := route[0]
+	egressHop := route[1]
+
 	if GetMetaData(flow) != 0 {
 		log.Debugw("creating-metadata-flow", log.Fields{"flow": flow})
 		portNumber := uint32(GetPortNumberFromMetadata(flow))
@@ -877,7 +911,7 @@
 				//	TODO: Delete flow
 				return deviceRules
 			case 2:
-				log.Debugw("route-found", log.Fields{"ingressHop": route[0], "egressHop": route[1]})
+				log.Debugw("route-found", log.Fields{"ingressHop": ingressHop, "egressHop": egressHop})
 				break
 			default:
 				log.Errorw("invalid-route-length", log.Fields{"routeLen": len(route)})
@@ -895,19 +929,17 @@
 		fa = &fu.FlowArgs{
 			KV: fu.OfpFlowModArgs{"priority": uint64(flow.Priority), "cookie": flow.Cookie},
 			MatchFields: []*ofp.OfpOxmOfbField{
-				InPort(ingressHop.Ingress), //ingress_hop.ingress_port.port_no
+				InPort(ingressHop.Ingress),
 				Metadata_ofp(innerTag),
 			},
 			Actions: GetActions(flow),
 		}
 		// Augment the matchfields with the ofpfields from the flow
-		for _, val := range GetOfbFields(flow) {
-			if val.Type != IN_PORT && val.Type != METADATA {
-				fa.MatchFields = append(fa.MatchFields, val)
-			}
-		}
-		// Agument the Actions
+		fa.MatchFields = append(fa.MatchFields, GetOfbFields(flow, IN_PORT, METADATA)...)
+
+		// Augment the Actions
 		fa.Actions = append(fa.Actions, Output(ingressHop.Egress))
+
 		fg := fu.NewFlowsAndGroups()
 		fg.AddFlow(MkFlowStat(fa))
 		deviceRules.AddFlowsAndGroup(ingressHop.DeviceID, fg)
@@ -917,18 +949,16 @@
 		fa = &fu.FlowArgs{
 			KV: fu.OfpFlowModArgs{"priority": uint64(flow.Priority), "cookie": flow.Cookie},
 			MatchFields: []*ofp.OfpOxmOfbField{
-				InPort(ingressHop.Ingress), //ingress_hop.ingress_port.port_no
+				InPort(ingressHop.Ingress),
 			},
 			Actions: GetActions(flow),
 		}
 		// Augment the matchfields with the ofpfields from the flow
-		for _, val := range GetOfbFields(flow) {
-			if val.Type != IN_PORT {
-				fa.MatchFields = append(fa.MatchFields, val)
-			}
-		}
-		// Agument the Actions
+		fa.MatchFields = append(fa.MatchFields, GetOfbFields(flow, IN_PORT)...)
+
+		// Augment the Actions
 		fa.Actions = append(fa.Actions, Output(ingressHop.Egress))
+
 		fg := fu.NewFlowsAndGroups()
 		fg.AddFlow(MkFlowStat(fa))
 		deviceRules.AddFlowsAndGroup(ingressHop.DeviceID, fg)
@@ -936,8 +966,16 @@
 	return deviceRules
 }
 
-func (fd *FlowDecomposer) processUnicastFlow(agent coreIf.LogicalDeviceAgent, route []graph.RouteHop, inPortNo uint32, outPortNo uint32, flow *ofp.OfpFlowStats, deviceRules *fu.DeviceRules) *fu.DeviceRules {
+// processUnicastFlow decomposes unicast flows
+func (fd *FlowDecomposer) processUnicastFlow(agent coreIf.LogicalDeviceAgent, route []graph.RouteHop,
+	inPortNo uint32, outPortNo uint32, flow *ofp.OfpFlowStats) *fu.DeviceRules {
+
 	log.Debugw("unicast-flow", log.Fields{"inPortNo": inPortNo, "outPortNo": outPortNo})
+	deviceRules := fu.NewDeviceRules()
+
+	ingressHop := route[0]
+	egressHop := route[1]
+
 	var actions []ofp.OfpActionType
 	var isOutputTypeInActions bool
 	for _, action := range GetActions(flow) {
@@ -952,73 +990,65 @@
 		fa = &fu.FlowArgs{
 			KV: fu.OfpFlowModArgs{"priority": uint64(flow.Priority), "cookie": flow.Cookie},
 			MatchFields: []*ofp.OfpOxmOfbField{
-				InPort(route[0].Ingress), //ingress_hop.ingress_port.port_no
+				InPort(ingressHop.Ingress),
 			},
 			Actions: []*ofp.OfpAction{
-				Output(route[0].Egress),
+				Output(ingressHop.Egress),
 			},
 		}
 		// Augment the matchfields with the ofpfields from the flow
-		for _, val := range GetOfbFields(flow) {
-			if val.Type != IN_PORT {
-				fa.MatchFields = append(fa.MatchFields, val)
-			}
-		}
+		fa.MatchFields = append(fa.MatchFields, GetOfbFields(flow, IN_PORT)...)
+
 		fg := fu.NewFlowsAndGroups()
 		fg.AddFlow(MkFlowStat(fa))
-		deviceRules.AddFlowsAndGroup(route[0].DeviceID, fg)
+		deviceRules.AddFlowsAndGroup(ingressHop.DeviceID, fg)
 
 		// Child device flow
 		fa = &fu.FlowArgs{
 			KV: fu.OfpFlowModArgs{"priority": uint64(flow.Priority), "cookie": flow.Cookie},
 			MatchFields: []*ofp.OfpOxmOfbField{
-				InPort(route[1].Ingress), //egress_hop.ingress_port.port_no
+				InPort(egressHop.Ingress),
 			},
 			Actions: []*ofp.OfpAction{
-				Output(route[1].Egress),
+				Output(egressHop.Egress),
 			},
 		}
 		// Augment the matchfields with the ofpfields from the flow
-		for _, val := range GetOfbFields(flow) {
-			if val.Type != IN_PORT {
-				fa.MatchFields = append(fa.MatchFields, val)
-			}
-		}
+		fa.MatchFields = append(fa.MatchFields, GetOfbFields(flow, IN_PORT)...)
+
 		fg = fu.NewFlowsAndGroups()
 		fg.AddFlow(MkFlowStat(fa))
-		deviceRules.AddFlowsAndGroup(route[1].DeviceID, fg)
+		deviceRules.AddFlowsAndGroup(egressHop.DeviceID, fg)
 	} else {
 		var fa *fu.FlowArgs
 		fa = &fu.FlowArgs{
 			KV: fu.OfpFlowModArgs{"priority": uint64(flow.Priority), "cookie": flow.Cookie},
 			MatchFields: []*ofp.OfpOxmOfbField{
-				InPort(route[1].Ingress), //egress_hop.ingress_port.port_no
+				InPort(egressHop.Ingress),
 			},
 		}
 		// Augment the matchfields with the ofpfields from the flow
-		for _, val := range GetOfbFields(flow) {
-			if val.Type != IN_PORT {
-				fa.MatchFields = append(fa.MatchFields, val)
-			}
-		}
+		fa.MatchFields = append(fa.MatchFields, GetOfbFields(flow, IN_PORT)...)
+
 		// Augment the Actions
-		updatedAction := make([]*ofp.OfpAction, 0)
-		for _, action := range GetActions(flow) {
-			if action.Type != OUTPUT {
-				updatedAction = append(updatedAction, action)
-			}
-		}
-		updatedAction = append(updatedAction, Output(route[1].Egress))
-		fa.Actions = updatedAction
+		filteredAction := GetActions(flow, OUTPUT)
+		filteredAction = append(filteredAction, Output(egressHop.Egress))
+		fa.Actions = filteredAction
+
 		fg := fu.NewFlowsAndGroups()
 		fg.AddFlow(MkFlowStat(fa))
-		deviceRules.AddFlowsAndGroup(route[1].DeviceID, fg)
+		deviceRules.AddFlowsAndGroup(egressHop.DeviceID, fg)
 	}
 	return deviceRules
 }
 
-func (fd *FlowDecomposer) processMulticastFlow(agent coreIf.LogicalDeviceAgent, route []graph.RouteHop, inPortNo uint32, outPortNo uint32, flow *ofp.OfpFlowStats, deviceRules *fu.DeviceRules, grpId uint32, groupMap map[uint32]*ofp.OfpGroupEntry) *fu.DeviceRules {
+// processMulticastFlow decompose multicast flows
+func (fd *FlowDecomposer) processMulticastFlow(agent coreIf.LogicalDeviceAgent, route []graph.RouteHop,
+	inPortNo uint32, outPortNo uint32, flow *ofp.OfpFlowStats, grpId uint32,
+	groupMap map[uint32]*ofp.OfpGroupEntry) *fu.DeviceRules {
+
 	log.Debugw("multicast-flow", log.Fields{"inPortNo": inPortNo, "outPortNo": outPortNo})
+	deviceRules := fu.NewDeviceRules()
 
 	//having no Group yet is the same as having a Group with no buckets
 	var grp *ofp.OfpGroupEntry
@@ -1048,14 +1078,18 @@
 			//	TODO: Delete flow
 			return deviceRules
 		case 2:
-			log.Debugw("route-found", log.Fields{"ingressHop": route[0], "egressHop": route[1]})
+			log.Debugw("route-found", log.Fields{"ingressHop": route2[0], "egressHop": route2[1]})
 			break
 		default:
 			log.Errorw("invalid-route-length", log.Fields{"routeLen": len(route)})
 			return deviceRules
 		}
 
-		if route[0].Ingress != route2[0].Ingress {
+		ingressHop := route[0]
+		ingressHop2 := route2[0]
+		egressHop := route2[1]
+
+		if ingressHop.Ingress != ingressHop2.Ingress {
 			log.Errorw("mc-ingress-hop-hop2-mismatch", log.Fields{"inPortNo": inPortNo, "outPortNo": outPortNo, "comment": "ignoring flow"})
 			return deviceRules
 		}
@@ -1064,53 +1098,47 @@
 		fa = &fu.FlowArgs{
 			KV: fu.OfpFlowModArgs{"priority": uint64(flow.Priority), "cookie": flow.Cookie},
 			MatchFields: []*ofp.OfpOxmOfbField{
-				InPort(route[0].Ingress), //ingress_hop.ingress_port.port_no
+				InPort(ingressHop.Ingress),
 			},
 		}
 		// Augment the matchfields with the ofpfields from the flow
-		for _, val := range GetOfbFields(flow) {
-			if val.Type != IN_PORT {
-				fa.MatchFields = append(fa.MatchFields, val)
-			}
-		}
+		fa.MatchFields = append(fa.MatchFields, GetOfbFields(flow, IN_PORT)...)
+
 		// Augment the Actions
-		updatedAction := make([]*ofp.OfpAction, 0)
-		for _, action := range GetActions(flow) {
-			if action.Type != GROUP {
-				updatedAction = append(updatedAction, action)
-			}
-		}
-		updatedAction = append(updatedAction, PopVlan())
-		updatedAction = append(updatedAction, Output(route[1].Ingress))
-		fa.Actions = updatedAction
+		filteredAction := GetActions(flow, GROUP)
+		filteredAction = append(filteredAction, PopVlan())
+		filteredAction = append(filteredAction, Output(route2[1].Ingress))
+		fa.Actions = filteredAction
+
 		fg := fu.NewFlowsAndGroups()
 		fg.AddFlow(MkFlowStat(fa))
-		deviceRules.AddFlowsAndGroup(route[0].DeviceID, fg)
+		deviceRules.AddFlowsAndGroup(ingressHop.DeviceID, fg)
 
 		// Set the child device flow
 		fa = &fu.FlowArgs{
 			KV: fu.OfpFlowModArgs{"priority": uint64(flow.Priority), "cookie": flow.Cookie},
 			MatchFields: []*ofp.OfpOxmOfbField{
-				InPort(route[1].Ingress), //egress_hop.ingress_port.port_no
+				InPort(egressHop.Ingress),
 			},
 		}
 		// Augment the matchfields with the ofpfields from the flow
-		for _, val := range GetOfbFields(flow) {
-			if val.Type != IN_PORT && val.Type != VLAN_VID && val.Type != VLAN_PCP {
-				fa.MatchFields = append(fa.MatchFields, val)
-			}
-		}
+		fa.MatchFields = append(fa.MatchFields, GetOfbFields(flow, IN_PORT, VLAN_VID, VLAN_PCP)...)
+
 		// Augment the Actions
-		otherActions = append(otherActions, Output(route[1].Egress))
+		otherActions = append(otherActions, Output(egressHop.Egress))
 		fa.Actions = otherActions
+
 		fg = fu.NewFlowsAndGroups()
 		fg.AddFlow(MkFlowStat(fa))
-		deviceRules.AddFlowsAndGroup(route[1].DeviceID, fg)
+		deviceRules.AddFlowsAndGroup(egressHop.DeviceID, fg)
 	}
 	return deviceRules
 }
 
-func (fd *FlowDecomposer) decomposeFlow(agent coreIf.LogicalDeviceAgent, flow *ofp.OfpFlowStats, groupMap map[uint32]*ofp.OfpGroupEntry) *fu.DeviceRules {
+// decomposeFlow decomposes a flow for a logical device into flows for each physical device
+func (fd *FlowDecomposer) decomposeFlow(agent coreIf.LogicalDeviceAgent, flow *ofp.OfpFlowStats,
+	groupMap map[uint32]*ofp.OfpGroupEntry) *fu.DeviceRules {
+
 	inPortNo := GetInPort(flow)
 	outPortNo := GetOutPort(flow)
 
@@ -1131,32 +1159,27 @@
 	}
 
 	var ingressDevice *voltha.Device
-	//var egressDevice *voltha.Device
 	var err error
 	if ingressDevice, err = fd.deviceMgr.GetDevice(route[0].DeviceID); err != nil {
 		log.Errorw("ingress-device-not-found", log.Fields{"deviceId": route[0].DeviceID})
 		return deviceRules
 	}
-	//if egressDevice, err = fd.deviceMgr.getDevice(route[1].DeviceID); err != nil {
-	//	log.Errorw("egress-device-not-found", log.Fields{"deviceId": route[1].DeviceID})
-	//	return deviceRules
-	//}
 
 	isDownstream := ingressDevice.Root
 	isUpstream := !isDownstream
 
 	// Process controller bound flow
 	if outPortNo != 0 && (outPortNo&0x7fffffff) == uint32(ofp.OfpPortNo_OFPP_CONTROLLER) {
-		deviceRules = fd.processControllerBoundFlow(agent, route, inPortNo, outPortNo, flow, deviceRules)
+		deviceRules = fd.processControllerBoundFlow(agent, route, inPortNo, outPortNo, flow)
 	} else {
 		if isUpstream {
-			deviceRules = fd.processUpstreamNonControllerBoundFlow(agent, route, inPortNo, outPortNo, flow, deviceRules)
+			deviceRules = fd.processUpstreamNonControllerBoundFlow(agent, route, inPortNo, outPortNo, flow)
 		} else if HasNextTable(flow) {
-			deviceRules = fd.processDownstreamFlowWithNextTable(agent, route, inPortNo, outPortNo, flow, deviceRules)
+			deviceRules = fd.processDownstreamFlowWithNextTable(agent, route, inPortNo, outPortNo, flow)
 		} else if outPortNo != 0 { // Unicast
-			deviceRules = fd.processUnicastFlow(agent, route, inPortNo, outPortNo, flow, deviceRules)
+			deviceRules = fd.processUnicastFlow(agent, route, inPortNo, outPortNo, flow)
 		} else if grpId := GetGroup(flow); grpId != 0 { //Multicast
-			deviceRules = fd.processMulticastFlow(agent, route, inPortNo, outPortNo, flow, deviceRules, grpId, groupMap)
+			deviceRules = fd.processMulticastFlow(agent, route, inPortNo, outPortNo, flow, grpId, groupMap)
 		}
 	}
 	return deviceRules
diff --git a/rw_core/flow_decomposition/flow_decomposer_test.go b/rw_core/flow_decomposition/flow_decomposer_test.go
index b2ba777..6c7d7fa 100644
--- a/rw_core/flow_decomposition/flow_decomposer_test.go
+++ b/rw_core/flow_decomposition/flow_decomposer_test.go
@@ -27,15 +27,10 @@
 	"testing"
 )
 
-const (
-	maxOnuOnPort4 int = 1
-	maxOnuOnPort5 int = 1
-)
-
 func init() {
-	log.AddPackage(log.JSON, log.DebugLevel, nil)
+	log.AddPackage(log.JSON, log.WarnLevel, nil)
 	log.UpdateAllLoggers(log.Fields{"instanceId": "flow-descomposition"})
-	log.SetAllLogLevel(log.DebugLevel)
+	log.SetAllLogLevel(log.WarnLevel)
 }
 
 type testDeviceManager struct {
@@ -50,8 +45,8 @@
 		Root:     true,
 		ParentId: "logical_device",
 		Ports: []*voltha.Port{
-			&voltha.Port{PortNo: 1, Label: "pon"},
-			&voltha.Port{PortNo: 2, Label: "nni"},
+			{PortNo: 1, Label: "pon"},
+			{PortNo: 2, Label: "nni"},
 		},
 	}
 	tdm.devices["onu1"] = &voltha.Device{
@@ -59,8 +54,8 @@
 		Root:     false,
 		ParentId: "olt",
 		Ports: []*voltha.Port{
-			&voltha.Port{PortNo: 1, Label: "pon"},
-			&voltha.Port{PortNo: 2, Label: "uni"},
+			{PortNo: 1, Label: "pon"},
+			{PortNo: 2, Label: "uni"},
 		},
 	}
 	tdm.devices["onu2"] = &voltha.Device{
@@ -68,8 +63,8 @@
 		Root:     false,
 		ParentId: "olt",
 		Ports: []*voltha.Port{
-			&voltha.Port{PortNo: 1, Label: "pon"},
-			&voltha.Port{PortNo: 2, Label: "uni"},
+			{PortNo: 1, Label: "pon"},
+			{PortNo: 2, Label: "uni"},
 		},
 	}
 	tdm.devices["onu3"] = &voltha.Device{
@@ -77,8 +72,8 @@
 		Root:     false,
 		ParentId: "olt",
 		Ports: []*voltha.Port{
-			&voltha.Port{PortNo: 1, Label: "pon"},
-			&voltha.Port{PortNo: 2, Label: "uni"},
+			{PortNo: 1, Label: "pon"},
+			{PortNo: 2, Label: "uni"},
 		},
 	}
 	tdm.devices["onu4"] = &voltha.Device{
@@ -86,8 +81,8 @@
 		Root:     false,
 		ParentId: "olt",
 		Ports: []*voltha.Port{
-			&voltha.Port{PortNo: 1, Label: "pon"},
-			&voltha.Port{PortNo: 2, Label: "uni"},
+			{PortNo: 1, Label: "pon"},
+			{PortNo: 2, Label: "uni"},
 		},
 	}
 	return &tdm
@@ -97,7 +92,7 @@
 	if d, ok := tdm.devices[deviceId]; ok {
 		return d, nil
 	}
-	return nil, errors.New("Absent")
+	return nil, errors.New("ABSENT.")
 }
 
 type testFlowDecomposer struct {
@@ -127,12 +122,12 @@
 	//DOWNSTREAM ROUTES
 
 	tfd.routes[graph.OFPortLink{Ingress: 10, Egress: 1}] = []graph.RouteHop{
-		graph.RouteHop{
+		{
 			DeviceID: "olt",
 			Ingress:  tfd.dMgr.devices["olt"].Ports[1].PortNo,
 			Egress:   tfd.dMgr.devices["olt"].Ports[0].PortNo,
 		},
-		graph.RouteHop{
+		{
 			DeviceID: "onu1",
 			Ingress:  tfd.dMgr.devices["onu1"].Ports[0].PortNo,
 			Egress:   tfd.dMgr.devices["onu1"].Ports[1].PortNo,
@@ -140,36 +135,36 @@
 	}
 
 	tfd.routes[graph.OFPortLink{Ingress: 10, Egress: 2}] = []graph.RouteHop{
-		graph.RouteHop{
+		{
 			DeviceID: "olt",
 			Ingress:  tfd.dMgr.devices["olt"].Ports[1].PortNo,
 			Egress:   tfd.dMgr.devices["olt"].Ports[0].PortNo,
 		},
-		graph.RouteHop{
+		{
 			DeviceID: "onu2",
 			Ingress:  tfd.dMgr.devices["onu2"].Ports[0].PortNo,
 			Egress:   tfd.dMgr.devices["onu2"].Ports[1].PortNo,
 		},
 	}
 	tfd.routes[graph.OFPortLink{Ingress: 10, Egress: 3}] = []graph.RouteHop{
-		graph.RouteHop{
+		{
 			DeviceID: "olt",
 			Ingress:  tfd.dMgr.devices["olt"].Ports[1].PortNo,
 			Egress:   tfd.dMgr.devices["olt"].Ports[0].PortNo,
 		},
-		graph.RouteHop{
+		{
 			DeviceID: "onu3",
 			Ingress:  tfd.dMgr.devices["onu3"].Ports[0].PortNo,
 			Egress:   tfd.dMgr.devices["onu3"].Ports[1].PortNo,
 		},
 	}
 	tfd.routes[graph.OFPortLink{Ingress: 10, Egress: 4}] = []graph.RouteHop{
-		graph.RouteHop{
+		{
 			DeviceID: "olt",
 			Ingress:  tfd.dMgr.devices["olt"].Ports[1].PortNo,
 			Egress:   tfd.dMgr.devices["olt"].Ports[0].PortNo,
 		},
-		graph.RouteHop{
+		{
 			DeviceID: "onu4",
 			Ingress:  tfd.dMgr.devices["onu4"].Ports[0].PortNo,
 			Egress:   tfd.dMgr.devices["onu4"].Ports[1].PortNo,
@@ -179,48 +174,48 @@
 	//UPSTREAM DATA PLANE
 
 	tfd.routes[graph.OFPortLink{Ingress: 1, Egress: 10}] = []graph.RouteHop{
-		graph.RouteHop{
+		{
 			DeviceID: "onu1",
 			Ingress:  tfd.dMgr.devices["onu1"].Ports[1].PortNo,
 			Egress:   tfd.dMgr.devices["onu1"].Ports[0].PortNo,
 		},
-		graph.RouteHop{
+		{
 			DeviceID: "olt",
 			Ingress:  tfd.dMgr.devices["olt"].Ports[0].PortNo,
 			Egress:   tfd.dMgr.devices["olt"].Ports[1].PortNo,
 		},
 	}
 	tfd.routes[graph.OFPortLink{Ingress: 2, Egress: 10}] = []graph.RouteHop{
-		graph.RouteHop{
+		{
 			DeviceID: "onu2",
 			Ingress:  tfd.dMgr.devices["onu2"].Ports[1].PortNo,
 			Egress:   tfd.dMgr.devices["onu2"].Ports[0].PortNo,
 		},
-		graph.RouteHop{
+		{
 			DeviceID: "olt",
 			Ingress:  tfd.dMgr.devices["olt"].Ports[0].PortNo,
 			Egress:   tfd.dMgr.devices["olt"].Ports[1].PortNo,
 		},
 	}
 	tfd.routes[graph.OFPortLink{Ingress: 3, Egress: 10}] = []graph.RouteHop{
-		graph.RouteHop{
+		{
 			DeviceID: "onu3",
 			Ingress:  tfd.dMgr.devices["onu3"].Ports[1].PortNo,
 			Egress:   tfd.dMgr.devices["onu3"].Ports[0].PortNo,
 		},
-		graph.RouteHop{
+		{
 			DeviceID: "olt",
 			Ingress:  tfd.dMgr.devices["olt"].Ports[0].PortNo,
 			Egress:   tfd.dMgr.devices["olt"].Ports[1].PortNo,
 		},
 	}
 	tfd.routes[graph.OFPortLink{Ingress: 4, Egress: 10}] = []graph.RouteHop{
-		graph.RouteHop{
+		{
 			DeviceID: "onu4",
 			Ingress:  tfd.dMgr.devices["onu4"].Ports[1].PortNo,
 			Egress:   tfd.dMgr.devices["onu4"].Ports[0].PortNo,
 		},
-		graph.RouteHop{
+		{
 			DeviceID: "olt",
 			Ingress:  tfd.dMgr.devices["olt"].Ports[0].PortNo,
 			Egress:   tfd.dMgr.devices["olt"].Ports[1].PortNo,
@@ -231,48 +226,48 @@
 
 	// openflow port 0 means absence of a port - go/protobuf interpretation
 	tfd.routes[graph.OFPortLink{Ingress: 1, Egress: 0}] = []graph.RouteHop{
-		graph.RouteHop{
+		{
 			DeviceID: "onu1",
 			Ingress:  tfd.dMgr.devices["onu1"].Ports[1].PortNo,
 			Egress:   tfd.dMgr.devices["onu1"].Ports[0].PortNo,
 		},
-		graph.RouteHop{
+		{
 			DeviceID: "olt",
 			Ingress:  tfd.dMgr.devices["olt"].Ports[0].PortNo,
 			Egress:   tfd.dMgr.devices["olt"].Ports[1].PortNo,
 		},
 	}
 	tfd.routes[graph.OFPortLink{Ingress: 2, Egress: 0}] = []graph.RouteHop{
-		graph.RouteHop{
+		{
 			DeviceID: "onu2",
 			Ingress:  tfd.dMgr.devices["onu2"].Ports[1].PortNo,
 			Egress:   tfd.dMgr.devices["onu2"].Ports[0].PortNo,
 		},
-		graph.RouteHop{
+		{
 			DeviceID: "olt",
 			Ingress:  tfd.dMgr.devices["olt"].Ports[0].PortNo,
 			Egress:   tfd.dMgr.devices["olt"].Ports[1].PortNo,
 		},
 	}
 	tfd.routes[graph.OFPortLink{Ingress: 3, Egress: 0}] = []graph.RouteHop{
-		graph.RouteHop{
+		{
 			DeviceID: "onu3",
 			Ingress:  tfd.dMgr.devices["onu3"].Ports[1].PortNo,
 			Egress:   tfd.dMgr.devices["onu3"].Ports[0].PortNo,
 		},
-		graph.RouteHop{
+		{
 			DeviceID: "olt",
 			Ingress:  tfd.dMgr.devices["olt"].Ports[0].PortNo,
 			Egress:   tfd.dMgr.devices["olt"].Ports[1].PortNo,
 		},
 	}
 	tfd.routes[graph.OFPortLink{Ingress: 4, Egress: 0}] = []graph.RouteHop{
-		graph.RouteHop{
+		{
 			DeviceID: "onu4",
 			Ingress:  tfd.dMgr.devices["onu4"].Ports[1].PortNo,
 			Egress:   tfd.dMgr.devices["onu4"].Ports[0].PortNo,
 		},
-		graph.RouteHop{
+		{
 			DeviceID: "olt",
 			Ingress:  tfd.dMgr.devices["olt"].Ports[0].PortNo,
 			Egress:   tfd.dMgr.devices["olt"].Ports[1].PortNo,
@@ -282,17 +277,17 @@
 	// DOWNSTREAM NEXT TABLE BASED
 
 	tfd.routes[graph.OFPortLink{Ingress: 10, Egress: 0}] = []graph.RouteHop{
-		graph.RouteHop{
+		{
 			DeviceID: "olt",
 			Ingress:  tfd.dMgr.devices["olt"].Ports[1].PortNo,
 			Egress:   tfd.dMgr.devices["olt"].Ports[0].PortNo,
 		},
-		graph.RouteHop{}, // 2nd hop is not known yet
+		{}, // 2nd hop is not known yet
 	}
 
 	tfd.routes[graph.OFPortLink{Ingress: 0, Egress: 10}] = []graph.RouteHop{
-		graph.RouteHop{}, // 1st hop is wildcard
-		graph.RouteHop{
+		{}, // 1st hop is wildcard
+		{
 			DeviceID: "olt",
 			Ingress:  tfd.dMgr.devices["olt"].Ports[0].PortNo,
 			Egress:   tfd.dMgr.devices["olt"].Ports[1].PortNo,
@@ -395,7 +390,7 @@
 	if len(excludePort) == 1 {
 		exclPort = excludePort[0]
 	}
-	for portno, _ := range tfd.logicalPorts {
+	for portno := range tfd.logicalPorts {
 		if portno != exclPort {
 			lPorts = append(lPorts, portno)
 		}
@@ -445,9 +440,9 @@
 	groups := ofp.FlowGroups{}
 	tfd := newTestFlowDecomposer(newTestDeviceManager())
 
-	device_rules := tfd.fd.DecomposeRules(tfd, flows, groups)
-	onu1FlowAndGroup := device_rules.Rules["onu1"]
-	oltFlowAndGroup := device_rules.Rules["olt"]
+	deviceRules := tfd.fd.DecomposeRules(tfd, flows, groups)
+	onu1FlowAndGroup := deviceRules.Rules["onu1"]
+	oltFlowAndGroup := deviceRules.Rules["olt"]
 	assert.Equal(t, 1, onu1FlowAndGroup.Flows.Len())
 	assert.Equal(t, 0, onu1FlowAndGroup.Groups.Len())
 	assert.Equal(t, 2, oltFlowAndGroup.Flows.Len())
@@ -525,9 +520,9 @@
 	groups := ofp.FlowGroups{}
 	tfd := newTestFlowDecomposer(newTestDeviceManager())
 
-	device_rules := tfd.fd.DecomposeRules(tfd, flows, groups)
-	onu1FlowAndGroup := device_rules.Rules["onu1"]
-	oltFlowAndGroup := device_rules.Rules["olt"]
+	deviceRules := tfd.fd.DecomposeRules(tfd, flows, groups)
+	onu1FlowAndGroup := deviceRules.Rules["onu1"]
+	oltFlowAndGroup := deviceRules.Rules["olt"]
 	assert.Equal(t, 1, onu1FlowAndGroup.Flows.Len())
 	assert.Equal(t, 0, onu1FlowAndGroup.Groups.Len())
 	assert.Equal(t, 2, oltFlowAndGroup.Flows.Len())
@@ -586,80 +581,227 @@
 	assert.Equal(t, expectedOltFlow.String(), derivedFlow.String())
 }
 
-//func TestUnicastUpstreamRuleDecomposition(t *testing.T) {
-//
-//	var fa *fu.FlowArgs
-//	fa = &fu.FlowArgs{
-//		KV: fu.OfpFlowModArgs{"priority": 500, "table_id":1},
-//		MatchFields: []*ofp.OfpOxmOfbField{
-//			InPort(1),
-//			VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0),
-//			VlanPcp(0),
-//		},
-//		Actions: []*ofp.OfpAction{
-//			SetField(VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 101)),
-//		},
-//	}
-//
-//	var fa2 *fu.FlowArgs
-//	fa2 = &fu.FlowArgs{
-//		KV: fu.OfpFlowModArgs{"priority": 500},
-//		MatchFields: []*ofp.OfpOxmOfbField{
-//			InPort(1),
-//			VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 101),
-//			VlanPcp(0),
-//		},
-//		Actions: []*ofp.OfpAction{
-//			PushVlan(0x8100),
-//			SetField(VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 1000)),
-//			SetField(VlanPcp(0)),
-//			Output(10),
-//		},
-//	}
-//
-//	flows := ofp.Flows{Items:[]*ofp.OfpFlowStats{MkFlowStat(fa), MkFlowStat(fa2)}}
-//	groups := ofp.FlowGroups{}
-//	tfd := newTestFlowDecomposer(newTestDeviceManager())
-//
-//	device_rules := tfd.fd.DecomposeRules(tfd, flows, groups)
-//	onu1FlowAndGroup := device_rules.Rules["onu1"]
-//	oltFlowAndGroup := device_rules.Rules["olt"]
-//	assert.Equal(t, 2, onu1FlowAndGroup.Flows.Len())
-//	assert.Equal(t, 0, onu1FlowAndGroup.Groups.Len())
-//	assert.Equal(t, 1, oltFlowAndGroup.Flows.Len())
-//	assert.Equal(t, 0, oltFlowAndGroup.Groups.Len())
-//
-//	fa = &fu.FlowArgs{
-//		KV: fu.OfpFlowModArgs{"priority": 500},
-//		MatchFields: []*ofp.OfpOxmOfbField{
-//			InPort(2),
-//			VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0),
-//			VlanPcp(0),
-//		},
-//		Actions: []*ofp.OfpAction{
-//			SetField(VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 101)),
-//			Output(1),
-//		},
-//	}
-//	expectedOnu1Flow := MkFlowStat(fa)
-//	derivedFlow := onu1FlowAndGroup.GetFlow(1)
-//	assert.Equal(t, expectedOnu1Flow.String(), derivedFlow.String())
-//
-//	fa = &fu.FlowArgs{
-//		KV: fu.OfpFlowModArgs{"priority": 500},
-//		MatchFields: []*ofp.OfpOxmOfbField{
-//			InPort(1),
-//			VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 101),
-//			VlanPcp(0),
-//		},
-//		Actions: []*ofp.OfpAction{
-//			PushVlan(0x8100),
-//			SetField(VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 1000)),
-//			SetField(VlanPcp(0)),
-//			Output(2),
-//		},
-//	}
-//	expectedOltFlow := MkFlowStat(fa)
-//	derivedFlow = oltFlowAndGroup.GetFlow(0)
-//	assert.Equal(t, expectedOltFlow.String(), derivedFlow.String())
-//}
+func TestUnicastUpstreamRuleDecomposition(t *testing.T) {
+
+	var fa *fu.FlowArgs
+	fa = &fu.FlowArgs{
+		KV: fu.OfpFlowModArgs{"priority": 500, "table_id": 1},
+		MatchFields: []*ofp.OfpOxmOfbField{
+			InPort(1),
+			VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0),
+			VlanPcp(0),
+		},
+		Actions: []*ofp.OfpAction{
+			SetField(VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 101)),
+		},
+	}
+
+	var fa2 *fu.FlowArgs
+	fa2 = &fu.FlowArgs{
+		KV: fu.OfpFlowModArgs{"priority": 500},
+		MatchFields: []*ofp.OfpOxmOfbField{
+			InPort(1),
+			VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 101),
+			VlanPcp(0),
+		},
+		Actions: []*ofp.OfpAction{
+			PushVlan(0x8100),
+			SetField(VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 1000)),
+			SetField(VlanPcp(0)),
+			Output(10),
+		},
+	}
+
+	flows := ofp.Flows{Items: []*ofp.OfpFlowStats{MkFlowStat(fa), MkFlowStat(fa2)}}
+	groups := ofp.FlowGroups{}
+	tfd := newTestFlowDecomposer(newTestDeviceManager())
+
+	deviceRules := tfd.fd.DecomposeRules(tfd, flows, groups)
+	onu1FlowAndGroup := deviceRules.Rules["onu1"]
+	oltFlowAndGroup := deviceRules.Rules["olt"]
+	assert.Equal(t, 2, onu1FlowAndGroup.Flows.Len())
+	assert.Equal(t, 0, onu1FlowAndGroup.Groups.Len())
+	assert.Equal(t, 1, oltFlowAndGroup.Flows.Len())
+	assert.Equal(t, 0, oltFlowAndGroup.Groups.Len())
+
+	fa = &fu.FlowArgs{
+		KV: fu.OfpFlowModArgs{"priority": 500},
+		MatchFields: []*ofp.OfpOxmOfbField{
+			InPort(2),
+			VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0),
+			VlanPcp(0),
+		},
+		Actions: []*ofp.OfpAction{
+			SetField(VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 101)),
+			Output(1),
+		},
+	}
+	expectedOnu1Flow := MkFlowStat(fa)
+	derivedFlow := onu1FlowAndGroup.GetFlow(1)
+	assert.Equal(t, expectedOnu1Flow.String(), derivedFlow.String())
+
+	fa = &fu.FlowArgs{
+		KV: fu.OfpFlowModArgs{"priority": 500},
+		MatchFields: []*ofp.OfpOxmOfbField{
+			InPort(1),
+			VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 101),
+			VlanPcp(0),
+		},
+		Actions: []*ofp.OfpAction{
+			PushVlan(0x8100),
+			SetField(VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 1000)),
+			SetField(VlanPcp(0)),
+			Output(2),
+		},
+	}
+	expectedOltFlow := MkFlowStat(fa)
+	derivedFlow = oltFlowAndGroup.GetFlow(0)
+	assert.Equal(t, expectedOltFlow.String(), derivedFlow.String())
+}
+
+func TestUnicastDownstreamRuleDecomposition(t *testing.T) {
+	var fa1 *fu.FlowArgs
+	fa1 = &fu.FlowArgs{
+		KV: fu.OfpFlowModArgs{"priority": 500, "table_id": 1},
+		MatchFields: []*ofp.OfpOxmOfbField{
+			InPort(10),
+			Metadata_ofp((1000 << 32) | 1),
+			VlanPcp(0),
+		},
+		Actions: []*ofp.OfpAction{
+			PopVlan(),
+		},
+	}
+
+	var fa2 *fu.FlowArgs
+	fa2 = &fu.FlowArgs{
+		KV: fu.OfpFlowModArgs{"priority": 500},
+		MatchFields: []*ofp.OfpOxmOfbField{
+			InPort(10),
+			VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 101),
+			VlanPcp(0),
+		},
+		Actions: []*ofp.OfpAction{
+			SetField(VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0)),
+			Output(1),
+		},
+	}
+
+	flows := ofp.Flows{Items: []*ofp.OfpFlowStats{MkFlowStat(fa1), MkFlowStat(fa2)}}
+	groups := ofp.FlowGroups{}
+	tfd := newTestFlowDecomposer(newTestDeviceManager())
+
+	deviceRules := tfd.fd.DecomposeRules(tfd, flows, groups)
+	onu1FlowAndGroup := deviceRules.Rules["onu1"]
+	oltFlowAndGroup := deviceRules.Rules["olt"]
+	assert.Equal(t, 2, onu1FlowAndGroup.Flows.Len())
+	assert.Equal(t, 0, onu1FlowAndGroup.Groups.Len())
+	assert.Equal(t, 1, oltFlowAndGroup.Flows.Len())
+	assert.Equal(t, 0, oltFlowAndGroup.Groups.Len())
+
+	fa1 = &fu.FlowArgs{
+		KV: fu.OfpFlowModArgs{"priority": 500},
+		MatchFields: []*ofp.OfpOxmOfbField{
+			InPort(2),
+			Metadata_ofp(1000),
+			VlanPcp(0),
+		},
+		Actions: []*ofp.OfpAction{
+			PopVlan(),
+			Output(1),
+		},
+	}
+	expectedOltFlow := MkFlowStat(fa1)
+	derivedFlow := oltFlowAndGroup.GetFlow(0)
+	assert.Equal(t, expectedOltFlow.String(), derivedFlow.String())
+
+	fa1 = &fu.FlowArgs{
+		KV: fu.OfpFlowModArgs{"priority": 500},
+		MatchFields: []*ofp.OfpOxmOfbField{
+			InPort(1),
+			VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 101),
+			VlanPcp(0),
+		},
+		Actions: []*ofp.OfpAction{
+			SetField(VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 0)),
+			Output(2),
+		},
+	}
+	expectedOnu1Flow := MkFlowStat(fa1)
+	derivedFlow = onu1FlowAndGroup.GetFlow(1)
+	assert.Equal(t, expectedOnu1Flow.String(), derivedFlow.String())
+}
+
+func TestMulticastDownstreamRuleDecomposition(t *testing.T) {
+	var fa *fu.FlowArgs
+	fa = &fu.FlowArgs{
+		KV: fu.OfpFlowModArgs{"priority": 500},
+		MatchFields: []*ofp.OfpOxmOfbField{
+			InPort(10),
+			VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 170),
+			VlanPcp(0),
+			EthType(0x800),
+			Ipv4Dst(0xe00a0a0a),
+		},
+		Actions: []*ofp.OfpAction{
+			Group(10),
+		},
+	}
+
+	var ga *fu.GroupArgs
+	ga = &fu.GroupArgs{
+		GroupId: 10,
+		Buckets: []*ofp.OfpBucket{
+			{Actions: []*ofp.OfpAction{
+				PopVlan(),
+				Output(1),
+			},
+			},
+		},
+	}
+
+	flows := ofp.Flows{Items: []*ofp.OfpFlowStats{MkFlowStat(fa)}}
+	groups := ofp.FlowGroups{Items: []*ofp.OfpGroupEntry{MkGroupStat(ga)}}
+	tfd := newTestFlowDecomposer(newTestDeviceManager())
+
+	deviceRules := tfd.fd.DecomposeRules(tfd, flows, groups)
+	onu1FlowAndGroup := deviceRules.Rules["onu1"]
+	oltFlowAndGroup := deviceRules.Rules["olt"]
+	assert.Equal(t, 2, onu1FlowAndGroup.Flows.Len())
+	assert.Equal(t, 0, onu1FlowAndGroup.Groups.Len())
+	assert.Equal(t, 1, oltFlowAndGroup.Flows.Len())
+	assert.Equal(t, 0, oltFlowAndGroup.Groups.Len())
+
+	fa = &fu.FlowArgs{
+		KV: fu.OfpFlowModArgs{"priority": 500},
+		MatchFields: []*ofp.OfpOxmOfbField{
+			InPort(2),
+			VlanVid(uint32(ofp.OfpVlanId_OFPVID_PRESENT) | 170),
+			VlanPcp(0),
+			EthType(0x800),
+			Ipv4Dst(0xe00a0a0a),
+		},
+		Actions: []*ofp.OfpAction{
+			PopVlan(),
+			Output(1),
+		},
+	}
+	expectedOltFlow := MkFlowStat(fa)
+	derivedFlow := oltFlowAndGroup.GetFlow(0)
+	assert.Equal(t, expectedOltFlow.String(), derivedFlow.String())
+
+	fa = &fu.FlowArgs{
+		KV: fu.OfpFlowModArgs{"priority": 500},
+		MatchFields: []*ofp.OfpOxmOfbField{
+			InPort(1),
+			EthType(0x800),
+			Ipv4Dst(0xe00a0a0a),
+		},
+		Actions: []*ofp.OfpAction{
+			Output(2),
+		},
+	}
+	expectedOnu1Flow := MkFlowStat(fa)
+	derivedFlow = onu1FlowAndGroup.GetFlow(1)
+	assert.Equal(t, expectedOnu1Flow.String(), derivedFlow.String())
+}