From cc1ac61bcb5d1c6b70fb2bfb23024150ea0b00c7 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Tue, 11 Jan 2022 21:18:50 -0800 Subject: [PATCH 01/33] Add SlicingProviderService interface --- .../tna/slicing/NetcfgSlicingProvider.java | 18 +++++++ .../fabric/tna/slicing/SlicingManager.java | 3 +- .../slicing/api/SlicingProviderService.java | 51 +++++++++++++++++++ .../tna/slicing/api/SlicingService.java | 42 +-------------- 4 files changed, 73 insertions(+), 41 deletions(-) create mode 100644 src/main/java/org/stratumproject/fabric/tna/slicing/NetcfgSlicingProvider.java create mode 100644 src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingProviderService.java diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/NetcfgSlicingProvider.java b/src/main/java/org/stratumproject/fabric/tna/slicing/NetcfgSlicingProvider.java new file mode 100644 index 000000000..a556c578f --- /dev/null +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/NetcfgSlicingProvider.java @@ -0,0 +1,18 @@ +package org.stratumproject.fabric.tna.slicing; + +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Reference; +import org.osgi.service.component.annotations.ReferenceCardinality; +import org.stratumproject.fabric.tna.slicing.api.SlicingProviderService; + +@Component(immediate = true, service = { + NetcfgSlicingProvider.class, +}) +public class NetcfgSlicingProvider { + + @Reference(cardinality = ReferenceCardinality.MANDATORY) + protected SlicingProviderService slicingProviderService; + + // TODO: listen for netcfg, register slices/tcs with slicingProviderService + +} diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java index e765e027a..13a2480dd 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java @@ -47,6 +47,7 @@ import org.stratumproject.fabric.tna.slicing.api.SliceId; import org.stratumproject.fabric.tna.slicing.api.SlicingAdminService; import org.stratumproject.fabric.tna.slicing.api.SlicingException; +import org.stratumproject.fabric.tna.slicing.api.SlicingProviderService; import org.stratumproject.fabric.tna.slicing.api.SlicingService; import org.stratumproject.fabric.tna.slicing.api.TrafficClass; import org.stratumproject.fabric.tna.web.SliceIdCodec; @@ -83,7 +84,7 @@ SlicingService.class, SlicingAdminService.class }) -public class SlicingManager implements SlicingService, SlicingAdminService { +public class SlicingManager implements SlicingService, SlicingProviderService, SlicingAdminService { @Reference(cardinality = ReferenceCardinality.MANDATORY) protected CoreService coreService; diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingProviderService.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingProviderService.java new file mode 100644 index 000000000..3e2139b5c --- /dev/null +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingProviderService.java @@ -0,0 +1,51 @@ +// Copyright 2021-present Open Networking Foundation +// SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 +package org.stratumproject.fabric.tna.slicing.api; + +/** + * Service through which slicing providers can inject information. + */ +public interface SlicingProviderService { + + /** + * Adds a slice with given ID. + * Adding default slice is not allowed. + * + * @param sliceId slice identifier + * @return true if the slice is added successfully. + * @throws SlicingException if an error occurred. + */ + boolean addSlice(SliceId sliceId); + + /** + * Removes a slice with given ID. + * Removing default slice is not allowed. + * + * @param sliceId slice identifier + * @return true if the slice is removed successfully. + * @throws SlicingException if an error occurred. + */ + boolean removeSlice(SliceId sliceId); + + /** + * Adds a traffic class to given slice. + * BEST_EFFORT traffic class must be added first. + * + * @param sliceId slice identifier + * @param tc traffic class config + * @return true if the traffic class is added to given slice successfully. + * @throws SlicingException if an error occurred. + */ + boolean addTrafficClass(SliceId sliceId, TrafficClass tc); + + /** + * Removes a traffic class from given slice. + * BEST_EFFORT traffic class must be removed last. + * + * @param sliceId slice identifier + * @param tc traffic class + * @return true if the traffic class is removed from given slice successfully. + * @throws SlicingException if an error occurred. + */ + boolean removeTrafficClass(SliceId sliceId, TrafficClass tc); +} diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingService.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingService.java index 299265e5c..b402a5579 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingService.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingService.java @@ -11,26 +11,6 @@ */ public interface SlicingService { - /** - * Adds a slice with given ID. - * Adding default slice is not allowed. - * - * @param sliceId slice identifier - * @return true if the slice is added successfully. - * @throws SlicingException if an error occurred. - */ - boolean addSlice(SliceId sliceId); - - /** - * Removes a slice with given ID. - * Removing default slice is not allowed. - * - * @param sliceId slice identifier - * @return true if the slice is removed successfully. - * @throws SlicingException if an error occurred. - */ - boolean removeSlice(SliceId sliceId); - /** * Gets all slice IDs. * @@ -38,26 +18,6 @@ public interface SlicingService { */ Set getSlices(); - /** - * Adds a traffic class to given slice. - * - * @param sliceId slice identifier - * @param tc traffic class - * @return true if the traffic class is added to given slice successfully. - * @throws SlicingException if an error occurred. - */ - boolean addTrafficClass(SliceId sliceId, TrafficClass tc); - - /** - * Removes a traffic class from given slice. - * - * @param sliceId slice identifier - * @param tc traffic class - * @return true if the traffic class is removed from given slice successfully. - * @throws SlicingException if an error occurred. - */ - boolean removeTrafficClass(SliceId sliceId, TrafficClass tc); - /** * Gets all traffic classes in given slice. * @@ -84,6 +44,8 @@ public interface SlicingService { */ TrafficClass getDefaultTrafficClass(SliceId sliceId); + // TODO: get traffic class parameters + /** * Associates flow identified by given selector with given slice ID and traffic class. * From b3a48b24e436655b08db2b9d98f94f99102584ff Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Tue, 11 Jan 2022 21:27:32 -0800 Subject: [PATCH 02/33] Remove slice/tc initialization from FabricUpfProgrammable We wil use netcfg for initialization --- .../behaviour/upf/FabricUpfProgrammable.java | 55 ------------------- 1 file changed, 55 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/behaviour/upf/FabricUpfProgrammable.java b/src/main/java/org/stratumproject/fabric/tna/behaviour/upf/FabricUpfProgrammable.java index 9d96bde73..68a316634 100644 --- a/src/main/java/org/stratumproject/fabric/tna/behaviour/upf/FabricUpfProgrammable.java +++ b/src/main/java/org/stratumproject/fabric/tna/behaviour/upf/FabricUpfProgrammable.java @@ -27,7 +27,6 @@ import org.onosproject.net.flow.DefaultTrafficTreatment; import org.onosproject.net.flow.FlowRule; import org.onosproject.net.flow.FlowRuleService; -import org.onosproject.net.flow.TrafficSelector; import org.onosproject.net.flow.criteria.PiCriterion; import org.onosproject.net.packet.DefaultOutboundPacket; import org.onosproject.net.packet.OutboundPacket; @@ -36,8 +35,6 @@ import org.onosproject.net.pi.model.PiCounterModel; import org.onosproject.net.pi.model.PiTableId; import org.onosproject.net.pi.model.PiTableModel; -import org.onosproject.net.pi.runtime.PiAction; -import org.onosproject.net.pi.runtime.PiActionParam; import org.onosproject.net.pi.runtime.PiCounterCell; import org.onosproject.net.pi.runtime.PiCounterCellHandle; import org.onosproject.net.pi.runtime.PiCounterCellId; @@ -46,13 +43,9 @@ import org.stratumproject.fabric.tna.PipeconfLoader; import org.stratumproject.fabric.tna.behaviour.FabricCapabilities; import org.stratumproject.fabric.tna.slicing.api.SliceId; -import org.stratumproject.fabric.tna.slicing.api.SlicingException; -import org.stratumproject.fabric.tna.slicing.api.SlicingService; -import org.stratumproject.fabric.tna.slicing.api.TrafficClass; import java.nio.ByteBuffer; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.List; import java.util.Map; @@ -61,12 +54,9 @@ import static java.lang.String.format; import static org.onosproject.net.pi.model.PiCounterType.INDIRECT; -import static org.stratumproject.fabric.tna.behaviour.FabricUtils.sliceTcConcat; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.FABRIC_EGRESS_SPGW_EG_TUNNEL_PEERS; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.FABRIC_EGRESS_SPGW_GTPU_ENCAP; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.FABRIC_EGRESS_SPGW_TERMINATIONS_COUNTER; -import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.FABRIC_INGRESS_QOS_QUEUES; -import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.FABRIC_INGRESS_QOS_SET_QUEUE; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.FABRIC_INGRESS_SPGW_DOWNLINK_SESSIONS; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.FABRIC_INGRESS_SPGW_DOWNLINK_TERMINATIONS; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.FABRIC_INGRESS_SPGW_IG_TUNNEL_PEERS; @@ -76,13 +66,11 @@ import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.FABRIC_INGRESS_SPGW_UPLINK_TERMINATIONS; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.HDR_GTPU_IS_VALID; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.HDR_IPV4_DST_ADDR; -import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.HDR_SLICE_TC; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.HDR_TEID; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.HDR_TUNNEL_IPV4_DST; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.HDR_TUN_PEER_ID; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.HDR_UE_ADDR; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.HDR_UE_SESSION_ID; -import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.QID; /** * Implementation of a UPF programmable device behavior. @@ -97,7 +85,6 @@ public class FabricUpfProgrammable extends AbstractP4RuntimeHandlerBehaviour private static final int PRIORITY_LOW = 10; protected FlowRuleService flowRuleService; - protected SlicingService slicingService; protected PacketService packetService; protected FabricUpfTranslator upfTranslator; @@ -130,7 +117,6 @@ protected boolean setupBehaviour(String opName) { } flowRuleService = handler().get(FlowRuleService.class); - slicingService = handler().get(SlicingService.class); packetService = handler().get(PacketService.class); upfTranslator = new FabricUpfTranslator(); final CoreService coreService = handler().get(CoreService.class); @@ -154,47 +140,11 @@ protected boolean setupBehaviour(String opName) { public boolean init() { if (setupBehaviour("init()")) { log.info("UpfProgrammable initialized for appId {} and deviceId {}", appId, deviceId); - try { - /* Add MOBILE slice and BEST_EFFORT tc if needed - and initialize the remaining traffic classes */ - Set tcs = slicingService.getTrafficClasses(SLICE_MOBILE); - if (tcs.isEmpty()) { - slicingService.addSlice(SLICE_MOBILE); - } - Arrays.stream(TrafficClass.values()).forEach(tc -> { - if (tcs.contains(tc) || tc.equals(TrafficClass.BEST_EFFORT) || - tc.equals(TrafficClass.SYSTEM)) { - return; - } - slicingService.addTrafficClass(SLICE_MOBILE, tc); - }); - } catch (SlicingException e) { - log.error("Exception while configuring traffic class for Mobile Slice: {}", e.getMessage()); - return false; - } return true; } return false; } - private FlowRule setQueueFlowRule(int sliceId, int tc, int queueId) { - TrafficSelector trafficSelector = DefaultTrafficSelector.builder() - .matchPi(PiCriterion.builder().matchExact(HDR_SLICE_TC, sliceTcConcat(sliceId, tc)).build()) - .build(); - PiAction action = PiAction.builder() - .withId(FABRIC_INGRESS_QOS_SET_QUEUE) - .withParameter(new PiActionParam(QID, queueId)) - .build(); - - return DefaultFlowRule.builder() - .forDevice(deviceId).fromApp(appId).makePermanent() - .forTable(FABRIC_INGRESS_QOS_QUEUES) - .withSelector(trafficSelector) - .withTreatment(DefaultTrafficTreatment.builder().piTableAction(action).build()) - .withPriority(PRIORITY_LOW) - .build(); - } - @Override public boolean fromThisUpf(FlowRule flowRule) { return flowRule.deviceId().equals(this.deviceId) && @@ -327,11 +277,6 @@ public void cleanUp() { return; } log.info("Clearing all UPF-related table entries."); - // Remove static Queue Configuration - slicingService.removeTrafficClass(SLICE_MOBILE, TrafficClass.ELASTIC); - slicingService.removeTrafficClass(SLICE_MOBILE, TrafficClass.REAL_TIME); - slicingService.removeTrafficClass(SLICE_MOBILE, TrafficClass.CONTROL); - slicingService.removeSlice(SLICE_MOBILE); } @Override From e15995943ac87a7cb456fc992c035a2cd6d73075 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Tue, 11 Jan 2022 21:29:36 -0800 Subject: [PATCH 03/33] Remove slice/tc initialization from FabricUpfProgrammable We wil use netcfg for initialization --- .../fabric/tna/behaviour/upf/FabricUpfProgrammable.java | 5 ----- .../fabric/tna/behaviour/upf/FabricUpfTranslator.java | 4 ++-- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/behaviour/upf/FabricUpfProgrammable.java b/src/main/java/org/stratumproject/fabric/tna/behaviour/upf/FabricUpfProgrammable.java index 68a316634..f91fc5a9b 100644 --- a/src/main/java/org/stratumproject/fabric/tna/behaviour/upf/FabricUpfProgrammable.java +++ b/src/main/java/org/stratumproject/fabric/tna/behaviour/upf/FabricUpfProgrammable.java @@ -42,7 +42,6 @@ import org.slf4j.LoggerFactory; import org.stratumproject.fabric.tna.PipeconfLoader; import org.stratumproject.fabric.tna.behaviour.FabricCapabilities; -import org.stratumproject.fabric.tna.slicing.api.SliceId; import java.nio.ByteBuffer; import java.util.ArrayList; @@ -82,8 +81,6 @@ public class FabricUpfProgrammable extends AbstractP4RuntimeHandlerBehaviour private static final int DEFAULT_PRIORITY = 128; private static final long DEFAULT_P4_DEVICE_ID = 1; - private static final int PRIORITY_LOW = 10; - protected FlowRuleService flowRuleService; protected PacketService packetService; protected FabricUpfTranslator upfTranslator; @@ -98,8 +95,6 @@ public class FabricUpfProgrammable extends AbstractP4RuntimeHandlerBehaviour private ApplicationId appId; - static final SliceId SLICE_MOBILE = SliceId.of(SliceId.MAX); - @Override protected boolean setupBehaviour(String opName) { // Already initialized. diff --git a/src/main/java/org/stratumproject/fabric/tna/behaviour/upf/FabricUpfTranslator.java b/src/main/java/org/stratumproject/fabric/tna/behaviour/upf/FabricUpfTranslator.java index f14c3d980..12cd3e01d 100644 --- a/src/main/java/org/stratumproject/fabric/tna/behaviour/upf/FabricUpfTranslator.java +++ b/src/main/java/org/stratumproject/fabric/tna/behaviour/upf/FabricUpfTranslator.java @@ -75,7 +75,6 @@ import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.TUNNEL_SRC_PORT; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.TUN_DST_ADDR; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.TUN_PEER_ID; -import static org.stratumproject.fabric.tna.behaviour.upf.FabricUpfProgrammable.SLICE_MOBILE; /** * Provides logic to translate UPF entities into pipeline-specific ones and vice-versa. @@ -600,7 +599,8 @@ public FlowRule interfaceToFabricEntry(UpfInterface upfInterface, DeviceId devic .build(); PiAction action = PiAction.builder() .withId(actionId) - .withParameter(new PiActionParam(SLICE_ID, SLICE_MOBILE.id())) + // FIXME: slice id should come from UP4 + .withParameter(new PiActionParam(SLICE_ID, 99)) .build(); return DefaultFlowRule.builder() .forDevice(deviceId).fromApp(appId).makePermanent() From 71482aea0c939b127f1fe0aa41d848b612239c92 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Tue, 11 Jan 2022 21:34:52 -0800 Subject: [PATCH 04/33] Remove REST API for adding/removing slice/tcs --- .../fabric/tna/web/SlicingWebResource.java | 103 +----------------- 1 file changed, 4 insertions(+), 99 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/web/SlicingWebResource.java b/src/main/java/org/stratumproject/fabric/tna/web/SlicingWebResource.java index 5af2ca9da..3ad0fde2f 100644 --- a/src/main/java/org/stratumproject/fabric/tna/web/SlicingWebResource.java +++ b/src/main/java/org/stratumproject/fabric/tna/web/SlicingWebResource.java @@ -5,7 +5,6 @@ import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; - import org.onosproject.net.flow.TrafficSelector; import org.onosproject.rest.AbstractWebResource; import org.slf4j.Logger; @@ -14,19 +13,18 @@ import org.stratumproject.fabric.tna.slicing.api.SlicingService; import org.stratumproject.fabric.tna.slicing.api.TrafficClass; -import java.io.IOException; -import java.io.InputStream; -import java.util.Set; - import javax.ws.rs.Consumes; import javax.ws.rs.DELETE; import javax.ws.rs.GET; +import javax.ws.rs.POST; import javax.ws.rs.Path; import javax.ws.rs.PathParam; -import javax.ws.rs.POST; import javax.ws.rs.Produces; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import java.io.IOException; +import java.io.InputStream; +import java.util.Set; import static org.onlab.util.Tools.readTreeFromStream; @@ -58,51 +56,6 @@ public Response getSlice() { return Response.ok(root).build(); } - /** - * Add a new slice. - * Pre-defined slice IDs: Default Slice = 0, Mobile Traffic Slice = 15 - * - * @param sliceId ID of the slice (DEFAULT_SLICE=0, MOBILE_SLICE=15) - * @return 200 ok or 400 bad request - */ - @POST - @Consumes(MediaType.APPLICATION_JSON) - @Path("slice/{sliceId}") - public Response addSlice(@PathParam("sliceId") int sliceId) { - boolean result = slicingService.addSlice(SliceId.of(sliceId)); - - Response response; - if (result) { - response = Response.ok().build(); - } else { - response = Response.status(400).build(); - } - - return response; - } - - /** - * Remove an existing slice. - * Pre-defined slice IDs: Default Slice = 0, Mobile Traffic Slice = 15 - * - * @param sliceId ID of the slice - * @return 200 ok or 400 bad request - */ - @DELETE - @Path("slice/{sliceId}") - public Response removeSlice(@PathParam("sliceId") int sliceId) { - boolean result = slicingService.removeSlice(SliceId.of(sliceId)); - - Response response; - if (result) { - response = Response.ok().build(); - } else { - response = Response.status(400).build(); - } - - return response; - } - /** * Get all traffic class of a slice. * Pre-defined slice IDs: Default Slice = 0, Mobile Traffic Slice = 15 @@ -128,31 +81,6 @@ public Response getTc(@PathParam("sliceId") int sliceId) { return response; } - /** - * Add a traffic class to a slice. - * Pre-defined slice IDs: Default Slice = 0, Mobile Traffic Slice = 15 - * Traffic class values: CONTROL, REAL_TIME, ELASTIC, BEST_EFFORT - * - * @param sliceId ID of the slice - * @param tc Traffic class to be added to the given slice - * @return 200 ok or 400 bad request - */ - @POST - @Consumes(MediaType.APPLICATION_JSON) - @Path("tc/{sliceId}/{tc}") - public Response addTc(@PathParam("sliceId") int sliceId, @PathParam("tc") String tc) { - boolean result = slicingService.addTrafficClass(SliceId.of(sliceId), TrafficClass.valueOf(tc)); - - Response response; - if (result) { - response = Response.ok().build(); - } else { - response = Response.status(400).build(); - } - - return response; - } - /** * Get default traffic class given a slice. * Pre-defined slice IDs: Default Slice = 0, Mobile Traffic Slice = 15 @@ -201,29 +129,6 @@ public Response setDefaultTc(@PathParam("sliceId") int sliceId, @PathParam("tc") return response; } - /** - * Remove a traffic class from a slice. - * Pre-defined slice IDs: Default Slice = 0, Mobile Traffic Slice = 15 - * - * @param sliceId ID of the slice - * @param tc Traffic class to be removed - * @return 200 ok or 400 bad request - */ - @DELETE - @Path("tc/{sliceId}/{tc}") - public Response removeTc(@PathParam("sliceId") int sliceId, @PathParam("tc") String tc) { - boolean result = slicingService.removeTrafficClass(SliceId.of(sliceId), TrafficClass.valueOf(tc)); - - Response response; - if (result) { - response = Response.ok().build(); - } else { - response = Response.status(400).build(); - } - - return response; - } - /** * Get classifier flows by slice ID and traffic class. * Pre-defined slice IDs: Default Slice = 0, Mobile Traffic Slice = 15 From 22385a1cf8d25057517d84a7d8dbf8872b4d03db Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Tue, 11 Jan 2022 21:38:41 -0800 Subject: [PATCH 05/33] Remove unused CLI commands --- .../tna/slicing/cli/SliceAddCommand.java | 31 ---------------- .../tna/slicing/cli/SliceRemoveCommand.java | 31 ---------------- .../fabric/tna/slicing/cli/TcAddCommand.java | 37 ------------------- .../tna/slicing/cli/TcRemoveCommand.java | 37 ------------------- 4 files changed, 136 deletions(-) delete mode 100644 src/main/java/org/stratumproject/fabric/tna/slicing/cli/SliceAddCommand.java delete mode 100644 src/main/java/org/stratumproject/fabric/tna/slicing/cli/SliceRemoveCommand.java delete mode 100644 src/main/java/org/stratumproject/fabric/tna/slicing/cli/TcAddCommand.java delete mode 100644 src/main/java/org/stratumproject/fabric/tna/slicing/cli/TcRemoveCommand.java diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/SliceAddCommand.java b/src/main/java/org/stratumproject/fabric/tna/slicing/cli/SliceAddCommand.java deleted file mode 100644 index 491694a3e..000000000 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/SliceAddCommand.java +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2021-present Open Networking Foundation -// SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 -package org.stratumproject.fabric.tna.slicing.cli; - -import org.apache.karaf.shell.api.action.Argument; -import org.apache.karaf.shell.api.action.Command; -import org.apache.karaf.shell.api.action.lifecycle.Service; -import org.onosproject.cli.AbstractShellCommand; -import org.stratumproject.fabric.tna.slicing.api.SliceId; -import org.stratumproject.fabric.tna.slicing.api.SlicingService; - -/** - * Add network slice. - */ -@Service -@Command(scope = "fabric-tna", name = "slice-add", description = "Add network slice") -public class SliceAddCommand extends AbstractShellCommand { - @Argument(index = 0, name = "sliceId", - description = "sliceId. Used to identify a slice.", - required = true, multiValued = false) - int sliceId; - - @Override - protected void doExecute() { - SlicingService slicingService = getService(SlicingService.class); - boolean result = slicingService.addSlice(SliceId.of(sliceId)); - if (result) { - print("Slice %s added", sliceId); - } - } -} diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/SliceRemoveCommand.java b/src/main/java/org/stratumproject/fabric/tna/slicing/cli/SliceRemoveCommand.java deleted file mode 100644 index 8b498b524..000000000 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/SliceRemoveCommand.java +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2021-present Open Networking Foundation -// SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 -package org.stratumproject.fabric.tna.slicing.cli; - -import org.apache.karaf.shell.api.action.Argument; -import org.apache.karaf.shell.api.action.Command; -import org.apache.karaf.shell.api.action.lifecycle.Service; -import org.onosproject.cli.AbstractShellCommand; -import org.stratumproject.fabric.tna.slicing.api.SliceId; -import org.stratumproject.fabric.tna.slicing.api.SlicingService; - -/** - * Remove network slice. - */ -@Service -@Command(scope = "fabric-tna", name = "slice-remove", description = "Remove network slice") -public class SliceRemoveCommand extends AbstractShellCommand { - @Argument(index = 0, name = "sliceId", - description = "sliceId. Used to identify a slice.", - required = true, multiValued = false) - int sliceId; - - @Override - protected void doExecute() { - SlicingService slicingService = getService(SlicingService.class); - boolean result = slicingService.removeSlice(SliceId.of(sliceId)); - if (result) { - print("Slice %s removed", sliceId); - } - } -} diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/TcAddCommand.java b/src/main/java/org/stratumproject/fabric/tna/slicing/cli/TcAddCommand.java deleted file mode 100644 index 0e4168606..000000000 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/TcAddCommand.java +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2021-present Open Networking Foundation -// SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 -package org.stratumproject.fabric.tna.slicing.cli; - -import org.apache.karaf.shell.api.action.Argument; -import org.apache.karaf.shell.api.action.Command; -import org.apache.karaf.shell.api.action.lifecycle.Service; -import org.onosproject.cli.AbstractShellCommand; -import org.stratumproject.fabric.tna.slicing.api.SliceId; -import org.stratumproject.fabric.tna.slicing.api.SlicingService; -import org.stratumproject.fabric.tna.slicing.api.TrafficClass; - -/** - * Add traffic class. - */ -@Service -@Command(scope = "fabric-tna", name = "tc-add", description = "Add traffic class") -public class TcAddCommand extends AbstractShellCommand { - @Argument(index = 0, name = "sliceId", - description = "sliceId. Used to identify a slice.", - required = true, multiValued = false) - int sliceId; - - @Argument(index = 1, name = "tc", - description = "Traffic class. Used to classify the traffic.", - required = true, multiValued = false) - String tc; - - @Override - protected void doExecute() { - SlicingService slicingService = getService(SlicingService.class); - boolean result = slicingService.addTrafficClass(SliceId.of(sliceId), TrafficClass.valueOf(tc)); - if (result) { - print("TC %s added to slice %s", tc, sliceId); - } - } -} diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/TcRemoveCommand.java b/src/main/java/org/stratumproject/fabric/tna/slicing/cli/TcRemoveCommand.java deleted file mode 100644 index 0ce6e6582..000000000 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/TcRemoveCommand.java +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2021-present Open Networking Foundation -// SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 -package org.stratumproject.fabric.tna.slicing.cli; - -import org.apache.karaf.shell.api.action.Argument; -import org.apache.karaf.shell.api.action.Command; -import org.apache.karaf.shell.api.action.lifecycle.Service; -import org.onosproject.cli.AbstractShellCommand; -import org.stratumproject.fabric.tna.slicing.api.SliceId; -import org.stratumproject.fabric.tna.slicing.api.SlicingService; -import org.stratumproject.fabric.tna.slicing.api.TrafficClass; - -/** - * Remove traffic class. - */ -@Service -@Command(scope = "fabric-tna", name = "tc-remove", description = "Remove traffic class") -public class TcRemoveCommand extends AbstractShellCommand { - @Argument(index = 0, name = "sliceId", - description = "sliceId. Used to identify a slice.", - required = true, multiValued = false) - int sliceId; - - @Argument(index = 1, name = "tc", - description = "Traffic class. Used to classify the traffic.", - required = true, multiValued = false) - String tc; - - @Override - protected void doExecute() { - SlicingService slicingService = getService(SlicingService.class); - boolean result = slicingService.removeTrafficClass(SliceId.of(sliceId), TrafficClass.valueOf(tc)); - if (result) { - print("TC %s removed from slice %s", tc, sliceId); - } - } -} From a13fa6e006954e75bbd8dd19f6ea574ab1432edb Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Wed, 12 Jan 2022 12:04:23 -0800 Subject: [PATCH 06/33] Revisit TrafficClass and remove hardcoded initialization Sytem is not a traffic class --- .../fabric/tna/behaviour/Constants.java | 11 +------ .../fabric/tna/slicing/SlicingManager.java | 21 +----------- .../fabric/tna/slicing/api/QueueId.java | 6 +--- .../fabric/tna/slicing/api/TrafficClass.java | 33 +++++++++++-------- 4 files changed, 22 insertions(+), 49 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/behaviour/Constants.java b/src/main/java/org/stratumproject/fabric/tna/behaviour/Constants.java index b96177b67..dd87254e5 100644 --- a/src/main/java/org/stratumproject/fabric/tna/behaviour/Constants.java +++ b/src/main/java/org/stratumproject/fabric/tna/behaviour/Constants.java @@ -47,18 +47,9 @@ public final class Constants { public static final int PKT_IN_MIRROR_SESSION_ID = 0x1FF; // Static Queue IDs (should match those in gen-stratum-qos-config.py) + // FIXME: remove hardcoded queue ID, should be derived from netcfg public static final int QUEUE_ID_BEST_EFFORT = 0; public static final int QUEUE_ID_SYSTEM = 1; - public static final int QUEUE_ID_CONTROL = 2; - public static final int QUEUE_ID_FIRST_REAL_TIME = 3; // This will always be 3 - // FIXME: ELASTIC_ID can change and it should be configurable at runtime (i.e., via netcfg?) - public static final int QUEUE_ID_FIRST_ELASTIC = 6; // TODO: this can change - - // Traffic Classes - public static final int TC_BEST_EFFORT = 0; // Also the default TC - public static final int TC_CONTROL = 1; - public static final int TC_REAL_TIME = 2; - public static final int TC_ELASTIC = 3; public static final int DEFAULT_SLICE_ID = 0; diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java index 13a2480dd..15c2fe006 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java @@ -170,23 +170,9 @@ protected void activate() { queueExecutor = Executors.newSingleThreadExecutor(groupedThreads("fabric-tna-queue-event", "%d", log)); queueStore.addListener(queueListener); + // FIXME: should BEST-EFFORT be configured via netcfg? // Shared queues are pre-provisioned and always available queueStore.put(QueueId.BEST_EFFORT, new QueueStoreValue(TrafficClass.BEST_EFFORT, true)); - queueStore.put(QueueId.SYSTEM, new QueueStoreValue(TrafficClass.SYSTEM, true)); - queueStore.put(QueueId.CONTROL, new QueueStoreValue(TrafficClass.CONTROL, true)); - - // FIXME Dedicate queues should be dynamically provisioned via API in the future - // This configuration is based on the util/sample-qos-config.yaml queues configuration - // Max rate = 45 Mbps - queueStore.put(QueueId.of(3), new QueueStoreValue(TrafficClass.REAL_TIME, true)); - // Max rate = 30 Mbps - queueStore.put(QueueId.of(4), new QueueStoreValue(TrafficClass.REAL_TIME, true)); - // Max rate = 25 Mbps - queueStore.put(QueueId.of(5), new QueueStoreValue(TrafficClass.REAL_TIME, true)); - // Min guaranteed rate = 100 Mbps - queueStore.put(QueueId.of(6), new QueueStoreValue(TrafficClass.ELASTIC, true)); - // Min guaranteed rate = 200 Mbps - queueStore.put(QueueId.of(7), new QueueStoreValue(TrafficClass.ELASTIC, true)); classifierFlowStore = storageService.consistentMapBuilder() .withName("fabric-tna-classifier-flow") @@ -293,10 +279,6 @@ public Set getSlices() { @Override public boolean addTrafficClass(SliceId sliceId, TrafficClass tc) { - if (tc == TrafficClass.SYSTEM) { - throw new SlicingException(INVALID, "SYSTEM TC should not be associated with any slice"); - } - StringBuilder errorMessage = new StringBuilder(); SliceStoreKey key = new SliceStoreKey(sliceId, tc); sliceStore.compute(key, (k, v) -> { @@ -507,7 +489,6 @@ private QueueId allocateQueue(TrafficClass tc) { if (queueId.isPresent()) { // Don't mark shared queues as they are always available. if (tc != TrafficClass.BEST_EFFORT && - tc != TrafficClass.SYSTEM && tc != TrafficClass.CONTROL) { queueStore.compute(queueId.get(), (k, v) -> { v.setAvailable(false); diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/QueueId.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/QueueId.java index db164c55c..e360432a6 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/QueueId.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/QueueId.java @@ -6,18 +6,14 @@ import static com.google.common.base.Preconditions.checkArgument; import static org.stratumproject.fabric.tna.behaviour.Constants.QUEUE_ID_BEST_EFFORT; -import static org.stratumproject.fabric.tna.behaviour.Constants.QUEUE_ID_CONTROL; -import static org.stratumproject.fabric.tna.behaviour.Constants.QUEUE_ID_SYSTEM; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.HDR_EGRESS_QID_BITWIDTH; /** - * Queue Identifier. + * Queue identifier. */ public final class QueueId extends Identifier implements Comparable { public static final Integer MAX = 1 << HDR_EGRESS_QID_BITWIDTH - 1; public static final QueueId BEST_EFFORT = QueueId.of(QUEUE_ID_BEST_EFFORT); - public static final QueueId SYSTEM = QueueId.of(QUEUE_ID_SYSTEM); - public static final QueueId CONTROL = QueueId.of(QUEUE_ID_CONTROL); private QueueId(int id) { super(id); diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClass.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClass.java index 9fe2d481d..b1dca67ba 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClass.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClass.java @@ -2,24 +2,29 @@ // SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 package org.stratumproject.fabric.tna.slicing.api; -import static org.stratumproject.fabric.tna.behaviour.Constants.TC_BEST_EFFORT; -import static org.stratumproject.fabric.tna.behaviour.Constants.TC_CONTROL; -import static org.stratumproject.fabric.tna.behaviour.Constants.TC_ELASTIC; -import static org.stratumproject.fabric.tna.behaviour.Constants.TC_REAL_TIME; - /** - * Traffic Class. + * Type of traffic class. */ public enum TrafficClass { - BEST_EFFORT(TC_BEST_EFFORT), - CONTROL(TC_CONTROL), - REAL_TIME(TC_REAL_TIME), - ELASTIC(TC_ELASTIC), - SYSTEM(99); // For reserving SYSTEM queue only. Not used for table programming + BEST_EFFORT(0), + CONTROL(1), + REAL_TIME(2), + ELASTIC(3); + + public final int intValue; - public final int tc; + TrafficClass(int intValue) { + this.intValue = intValue; + } - TrafficClass(int tc) { - this.tc = tc; + /** + * Returns an integer uniquely identifying this traffic class. To be used in + * flow programming, e.g., when writing the TC in packet headers and + * metadata. + * + * @return TC integer value + */ + public int toInt() { + return intValue; } } From c45c3e1ca639c8aa584f7039d07c0efd1d64d058 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Wed, 12 Jan 2022 16:04:43 -0800 Subject: [PATCH 07/33] Introduce TrafficClassConfig class --- .../fabric/tna/slicing/SlicingManager.java | 59 ++++++++------- .../tna/slicing/api/SlicingAdminService.java | 2 +- .../slicing/api/SlicingProviderService.java | 6 +- .../tna/slicing/api/TrafficClassConfig.java | 75 +++++++++++++++++++ 4 files changed, 111 insertions(+), 31 deletions(-) create mode 100644 src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java index 15c2fe006..68db1f8b1 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java @@ -50,15 +50,16 @@ import org.stratumproject.fabric.tna.slicing.api.SlicingProviderService; import org.stratumproject.fabric.tna.slicing.api.SlicingService; import org.stratumproject.fabric.tna.slicing.api.TrafficClass; +import org.stratumproject.fabric.tna.slicing.api.TrafficClassConfig; import org.stratumproject.fabric.tna.web.SliceIdCodec; import org.stratumproject.fabric.tna.web.TrafficClassCodec; import java.util.Comparator; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Optional; import java.util.Set; -import java.util.Map.Entry; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicBoolean; @@ -121,8 +122,8 @@ public class SlicingManager implements SlicingService, SlicingProviderService, S protected ApplicationId appId; - protected ConsistentMap sliceStore; - private MapEventListener sliceListener; + protected ConsistentMap sliceStore; + private MapEventListener sliceListener; private ExecutorService sliceExecutor; protected ConsistentMap queueStore; @@ -148,11 +149,12 @@ protected void activate() { .register(KryoNamespaces.API) .register(SliceId.class) .register(TrafficClass.class) + .register(TrafficClassConfig.class) .register(QueueId.class) .register(SliceStoreKey.class) .register(QueueStoreValue.class); - sliceStore = storageService.consistentMapBuilder() + sliceStore = storageService.consistentMapBuilder() .withName("fabric-tna-slice") .withRelaxedReadConsistency() .withSerializer(Serializer.using(serializer.build())) @@ -193,7 +195,7 @@ protected void activate() { defaultTcStore.addListener(defaultTcListener); // Default slice is pre-provisioned - sliceStore.put(new SliceStoreKey(SliceId.DEFAULT, TrafficClass.BEST_EFFORT), QueueId.BEST_EFFORT); + sliceStore.put(new SliceStoreKey(SliceId.DEFAULT, TrafficClass.BEST_EFFORT), TrafficClassConfig.BEST_EFFORT); defaultTcStore.put(SliceId.DEFAULT, TrafficClass.BEST_EFFORT); deviceListener = new InternalDeviceListener(); @@ -223,6 +225,8 @@ protected void deactivate() { defaultTcStore.destroy(); defaultTcExecutor.shutdown(); + // FIXME: clean up classifier flow rules and store + codecService.unregisterCodec(SliceId.class); codecService.unregisterCodec(TrafficClass.class); @@ -235,7 +239,7 @@ public boolean addSlice(SliceId sliceId) { throw new SlicingException(INVALID, "Adding default slice is not allowed"); } - return addTrafficClass(sliceId, TrafficClass.BEST_EFFORT) && + return addTrafficClass(sliceId, TrafficClassConfig.BEST_EFFORT) && setDefaultTrafficClass(sliceId, TrafficClass.BEST_EFFORT); } @@ -263,9 +267,7 @@ public boolean removeSlice(SliceId sliceId) { tcs.stream() .sorted(Comparator.comparingInt(TrafficClass::ordinal).reversed()) // Remove BEST_EFFORT the last - .forEach(tc -> { - removeTrafficClass(sliceId, tc); - }); + .forEach(tc -> removeTrafficClass(sliceId, tc)); return true; } @@ -278,26 +280,28 @@ public Set getSlices() { } @Override - public boolean addTrafficClass(SliceId sliceId, TrafficClass tc) { + public boolean addTrafficClass(SliceId sliceId, TrafficClassConfig tcConfig) { StringBuilder errorMessage = new StringBuilder(); - SliceStoreKey key = new SliceStoreKey(sliceId, tc); + SliceStoreKey key = new SliceStoreKey(sliceId, tcConfig.trafficClass()); sliceStore.compute(key, (k, v) -> { if (v != null) { - errorMessage.append(String.format("TC %s is already allocated for slice %s", tc, sliceId)); + errorMessage.append(String.format("TC %s is already allocated for slice %s", tcConfig, sliceId)); return v; } - QueueId queueId = allocateQueue(tc); + // FIXME: queue allocation must go, it's now provided by users via netcfg + QueueId queueId = allocateQueue(tcConfig.trafficClass()); if (queueId == null) { - errorMessage.append(String.format("Unable to find available queue for %s", tc)); + errorMessage.append(String.format("Unable to find available queue for %s", tcConfig)); return null; } - log.info("Allocate queue {} for slice {} tc {}", queueId, sliceId, tc); - return queueId; + log.info("Allocate queue {} for slice {} tc {}", queueId, sliceId, tcConfig); + return tcConfig; }); if (errorMessage.length() != 0) { + // FIXME: SlicingProviderException throw new SlicingException(FAILED, errorMessage.toString()); } @@ -327,7 +331,7 @@ public boolean removeTrafficClass(SliceId sliceId, TrafficClass tc) { return v; } - deallocateQueue(v); + deallocateQueue(v.queueId()); log.info("Deallocate queue {} for slice {} tc {}", v, sliceId, tc); return null; }); @@ -350,10 +354,13 @@ public Set getTrafficClasses(SliceId sliceId) { @Override public boolean setDefaultTrafficClass(SliceId sliceId, TrafficClass tc) { StringBuilder errorMessage = new StringBuilder(); + // FIXME: this is ugly and race prone. should we let the data plane + // handle inconsistencies? E.g. if a default TC has not been allocated, + // then the switch should pick best effort. sliceStore.compute(new SliceStoreKey(sliceId, tc), (k, v) -> { if (v == null) { errorMessage.append(String.format("Can't set %s as default TC because it has not" + - " been allocated to slice %s", tc, sliceId)); + " been allocated to slice %s", tc, sliceId)); } else { defaultTcStore.put(sliceId, tc); } @@ -373,7 +380,7 @@ public TrafficClass getDefaultTrafficClass(SliceId sliceId) { } @Override - public Map getSliceStore() { + public Map getSliceStore() { return Map.copyOf(sliceStore.asJavaMap()); } @@ -385,7 +392,7 @@ public boolean addFlow(TrafficSelector selector, SliceId sliceId, TrafficClass t // Accept 5-tuple only if (!fiveTupleOnly(selector)) { throw new SlicingException(UNSUPPORTED, - String.format("Only accept 5-tuple %s", selector.toString())); + String.format("Only accept 5-tuple %s", selector)); } SliceStoreKey value = new SliceStoreKey(sliceId, tc); @@ -650,8 +657,8 @@ private FlowRule buildClassifierFlowRule(DeviceId deviceId, return flowRule; } - private class InternalSliceListener implements MapEventListener { - public void event(MapEvent event) { + private class InternalSliceListener implements MapEventListener { + public void event(MapEvent event) { // Distributed work based on QueueId. Consistent with InternalQueueListener log.info("Processing slice event {}", event); sliceExecutor.submit(() -> { @@ -660,8 +667,8 @@ public void event(MapEvent event) { case UPDATE: if (workPartitionService.isMine(event.newValue().value(), toStringHasher())) { deviceService.getAvailableDevices().forEach(device -> - addQueueTable(device.id(), - event.key().sliceId(), event.key().trafficClass(), event.newValue().value()) + addQueueTable(device.id(), + event.key().sliceId(), event.key().trafficClass(), event.newValue().value().queueId()) ); } break; @@ -669,7 +676,7 @@ public void event(MapEvent event) { if (workPartitionService.isMine(event.oldValue().value(), toStringHasher())) { deviceService.getAvailableDevices().forEach(device -> removeQueueTable(device.id(), - event.key().sliceId(), event.key().trafficClass(), event.oldValue().value()) + event.key().sliceId(), event.key().trafficClass(), event.oldValue().value().queueId()) ); } break; @@ -783,7 +790,7 @@ public void event(DeviceEvent event) { if (workPartitionService.isMine(deviceId, toStringHasher())) { if (deviceService.isAvailable(deviceId)) { sliceStore.forEach(e -> addQueueTable(deviceId, - e.getKey().sliceId(), e.getKey().trafficClass(), e.getValue().value()) + e.getKey().sliceId(), e.getKey().trafficClass(), e.getValue().value().queueId()) ); if (isLeafSwitch(deviceId)) { classifierFlowStore.forEach(e -> addClassifierFlowRule(deviceId, diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingAdminService.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingAdminService.java index c37b38a85..248d319b1 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingAdminService.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingAdminService.java @@ -17,7 +17,7 @@ public interface SlicingAdminService { * * @return map of slice store */ - Map getSliceStore(); + Map getSliceStore(); /** * Reserves a queue for the queue pool of given traffic class. diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingProviderService.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingProviderService.java index 3e2139b5c..beee472e8 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingProviderService.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingProviderService.java @@ -29,18 +29,16 @@ public interface SlicingProviderService { /** * Adds a traffic class to given slice. - * BEST_EFFORT traffic class must be added first. * * @param sliceId slice identifier - * @param tc traffic class config + * @param tcConfig traffic class config * @return true if the traffic class is added to given slice successfully. * @throws SlicingException if an error occurred. */ - boolean addTrafficClass(SliceId sliceId, TrafficClass tc); + boolean addTrafficClass(SliceId sliceId, TrafficClassConfig tcConfig); /** * Removes a traffic class from given slice. - * BEST_EFFORT traffic class must be removed last. * * @param sliceId slice identifier * @param tc traffic class diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java new file mode 100644 index 000000000..25175295c --- /dev/null +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java @@ -0,0 +1,75 @@ +// Copyright $today.year-present Open Networking Foundation +// SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 + +package org.stratumproject.fabric.tna.slicing.api; + +import com.google.common.base.MoreObjects; + +import java.util.Objects; + +/** + * Configuration of a traffic class. + */ +// TODO: complete javadocs +public class TrafficClassConfig { + private static final int UNLIMITED_MAX_RATE = -1; + + public static final TrafficClassConfig BEST_EFFORT = new TrafficClassConfig( + TrafficClass.BEST_EFFORT, QueueId.BEST_EFFORT, UNLIMITED_MAX_RATE, 0); + + private final TrafficClass tc; + private final QueueId qid; + private final int maxRateBps; + private final int gminRateBps; + + public TrafficClassConfig(TrafficClass tc, QueueId qid, int maxRateBps, int gminRateBps) { + this.tc = tc; + this.qid = qid; + this.maxRateBps = maxRateBps; + this.gminRateBps = gminRateBps; + } + + public TrafficClass trafficClass() { + return tc; + } + + public QueueId queueId() { + return qid; + } + + public int getMaxRateBps() { + return maxRateBps; + } + + public int getGminRateBps() { + return gminRateBps; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + TrafficClassConfig that = (TrafficClassConfig) o; + return maxRateBps == that.maxRateBps && + gminRateBps == that.gminRateBps && + tc == that.tc && + Objects.equals(qid, that.qid); + } + + @Override + public int hashCode() { + return Objects.hash(tc, qid, maxRateBps, gminRateBps); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("tc", tc) + .add("qid", qid) + .add("maxRateBps", maxRateBps) + .add("gminRateBps", gminRateBps) + .toString(); + } + + // TODO: builder +} From 7bbf074ad027d2cb8db5bf16ee4783e42e7b3e3e Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Wed, 12 Jan 2022 16:15:36 -0800 Subject: [PATCH 08/33] Remove queue allocation logic --- .../fabric/tna/slicing/SlicingManager.java | 144 +----------------- .../tna/slicing/api/SlicingAdminService.java | 24 --- .../tna/slicing/cli/QueueReleaseCommand.java | 32 ---- .../tna/slicing/cli/QueueReserveCommand.java | 39 ----- .../tna/slicing/cli/QueueStoreGetCommand.java | 26 ---- 5 files changed, 3 insertions(+), 262 deletions(-) delete mode 100644 src/main/java/org/stratumproject/fabric/tna/slicing/cli/QueueReleaseCommand.java delete mode 100644 src/main/java/org/stratumproject/fabric/tna/slicing/cli/QueueReserveCommand.java delete mode 100644 src/main/java/org/stratumproject/fabric/tna/slicing/cli/QueueStoreGetCommand.java diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java index 68db1f8b1..a06d15a9f 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java @@ -62,7 +62,6 @@ import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; import java.util.stream.Collectors; @@ -126,10 +125,6 @@ public class SlicingManager implements SlicingService, SlicingProviderService, S private MapEventListener sliceListener; private ExecutorService sliceExecutor; - protected ConsistentMap queueStore; - private MapEventListener queueListener; - private ExecutorService queueExecutor; - protected ConsistentMap classifierFlowStore; private MapEventListener classifierFlowListener; private ExecutorService classifierFlowExecutor; @@ -163,19 +158,6 @@ protected void activate() { sliceExecutor = Executors.newSingleThreadExecutor(groupedThreads("fabric-tna-slice-event", "%d", log)); sliceStore.addListener(sliceListener); - queueStore = storageService.consistentMapBuilder() - .withName("fabric-tna-queue") - .withRelaxedReadConsistency() - .withSerializer(Serializer.using(serializer.build())) - .build(); - queueListener = new InternalQueueListener(); - queueExecutor = Executors.newSingleThreadExecutor(groupedThreads("fabric-tna-queue-event", "%d", log)); - queueStore.addListener(queueListener); - - // FIXME: should BEST-EFFORT be configured via netcfg? - // Shared queues are pre-provisioned and always available - queueStore.put(QueueId.BEST_EFFORT, new QueueStoreValue(TrafficClass.BEST_EFFORT, true)); - classifierFlowStore = storageService.consistentMapBuilder() .withName("fabric-tna-classifier-flow") .withRelaxedReadConsistency() @@ -214,10 +196,6 @@ protected void deactivate() { sliceStore.destroy(); sliceExecutor.shutdown(); - queueStore.removeListener(queueListener); - queueStore.destroy(); - queueExecutor.shutdown(); - deviceService.removeListener(deviceListener); deviceExecutor.shutdown(); @@ -289,19 +267,12 @@ public boolean addTrafficClass(SliceId sliceId, TrafficClassConfig tcConfig) { return v; } - // FIXME: queue allocation must go, it's now provided by users via netcfg - QueueId queueId = allocateQueue(tcConfig.trafficClass()); - if (queueId == null) { - errorMessage.append(String.format("Unable to find available queue for %s", tcConfig)); - return null; - } - - log.info("Allocate queue {} for slice {} tc {}", queueId, sliceId, tcConfig); + log.info("Added traffic class {} to slice {}: {}", tcConfig.trafficClass(), sliceId, tcConfig); return tcConfig; }); if (errorMessage.length() != 0) { - // FIXME: SlicingProviderException + // FIXME: SlicingProviderException should be checked throw new SlicingException(FAILED, errorMessage.toString()); } @@ -331,8 +302,7 @@ public boolean removeTrafficClass(SliceId sliceId, TrafficClass tc) { return v; } - deallocateQueue(v.queueId()); - log.info("Deallocate queue {} for slice {} tc {}", v, sliceId, tc); + log.info("Removed traffic class {} from slice {}", tc, sliceId); return null; }); @@ -442,90 +412,6 @@ private Set getFlows(SliceId sliceId) { .collect(Collectors.toSet()); } - //FIXME: Apply slicing exception on Queue APIs when dynamic queue config is ready - @Override - public boolean reserveQueue(QueueId queueId, TrafficClass tc) { - AtomicBoolean result = new AtomicBoolean(false); - - queueStore.compute(queueId, (k, v) -> { - if (v != null) { - log.warn("Queue {} has already been allocated to TC {}", k, v); - return v; - } - log.info("Queue {} successfully reserved for TC {}", k, tc); - result.set(true); - return new QueueStoreValue(tc, true); - }); - - return result.get(); - } - - @Override - public boolean releaseQueue(QueueId queueId) { - AtomicBoolean result = new AtomicBoolean(false); - - queueStore.compute(queueId, (k, v) -> { - if (v == null) { - log.warn("Queue {} is not reserved", queueId); - return null; - } - if (!v.available()) { - log.warn("Queue {} in use", queueId); - return v; - } - log.info("Queue {} is released from TC {}", queueId, v.trafficClass()); - result.set(true); - return null; - }); - - return result.get(); - } - - @Override - public Map getQueueStore() { - return Map.copyOf(queueStore.asJavaMap()); - } - - private QueueId allocateQueue(TrafficClass tc) { - Optional queueId = queueStore.stream() - .filter(e -> e.getValue().value().trafficClass() == tc) - .filter(e -> e.getValue().value().available()) - .findFirst() - .map(Map.Entry::getKey); - - if (queueId.isPresent()) { - // Don't mark shared queues as they are always available. - if (tc != TrafficClass.BEST_EFFORT && - tc != TrafficClass.CONTROL) { - queueStore.compute(queueId.get(), (k, v) -> { - v.setAvailable(false); - return v; - }); - } - log.info("Allocated queue {} to TC {}", queueId.get(), tc); - return queueId.get(); - } else { - log.warn("No queue available for TC {}", tc); - return null; - } - } - - private boolean deallocateQueue(QueueId queueId) { - AtomicBoolean result = new AtomicBoolean(false); - - queueStore.compute(queueId, (k, v) -> { - if (v == null) { - log.warn("Queue {} not reserved yet", queueId); - return v; - } - v.setAvailable(true); - result.set(true); - return v; - }); - - return result.get(); - } - private FlowRule buildDefaultTcFlowRule(DeviceId deviceId, SliceId sliceId, TrafficClass tc) { PiCriterion.Builder piCriterionBuilder = PiCriterion.builder() .matchTernary(P4InfoConstants.HDR_SLICE_TC, sliceTcConcat(sliceId.id(), 0x00), 0x3C) @@ -687,30 +573,6 @@ public void event(MapEvent event) { } } - private class InternalQueueListener implements MapEventListener { - public void event(MapEvent event) { - // Distributed work based on QueueId. Consistent with InternalQueueListener - log.info("Processing queue event {}", event); - sliceExecutor.submit(() -> { - switch (event.type()) { - case INSERT: - case UPDATE: - if (workPartitionService.isMine(event.newValue().value(), toStringHasher())) { - // TODO programmatically config queues - } - break; - case REMOVE: - if (workPartitionService.isMine(event.oldValue().value(), toStringHasher())) { - // TODO programmatically config queues - } - break; - default: - break; - } - }); - } - } - private class InternalFlowListener implements MapEventListener { public void event(MapEvent event) { log.info("Processing flow classifier event {}", event); diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingAdminService.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingAdminService.java index 248d319b1..4db0732aa 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingAdminService.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingAdminService.java @@ -18,28 +18,4 @@ public interface SlicingAdminService { * @return map of slice store */ Map getSliceStore(); - - /** - * Reserves a queue for the queue pool of given traffic class. - * - * @param queueId queue identifier - * @param tc traffic class - * @return true if the queue is successfully reserved to the queue pool of given TC - */ - boolean reserveQueue(QueueId queueId, TrafficClass tc); - - /** - * Releases a queue from the queue pool. - * - * @param queueId queue identifier - * @return true if the queue is successfully released from the queue pool of given TC - */ - boolean releaseQueue(QueueId queueId); - - /** - * Gets all entries in the queue store. - * - * @return map of queue store - */ - Map getQueueStore(); } diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/QueueReleaseCommand.java b/src/main/java/org/stratumproject/fabric/tna/slicing/cli/QueueReleaseCommand.java deleted file mode 100644 index 39a2b424b..000000000 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/QueueReleaseCommand.java +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright 2021-present Open Networking Foundation -// SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 -package org.stratumproject.fabric.tna.slicing.cli; - -import org.apache.karaf.shell.api.action.Argument; -import org.apache.karaf.shell.api.action.Command; -import org.apache.karaf.shell.api.action.lifecycle.Service; -import org.onosproject.cli.AbstractShellCommand; -import org.stratumproject.fabric.tna.slicing.api.QueueId; -import org.stratumproject.fabric.tna.slicing.api.SlicingAdminService; -/** - * Reserve queue for TC. - */ -@Service -@Command(scope = "fabric-tna", name = "queue-release", description = "Release queue from TC") -public class QueueReleaseCommand extends AbstractShellCommand { - @Argument(index = 0, name = "queueId", - description = "queueId. Used to identify a queue.", - required = true, multiValued = false) - int queueId; - - @Override - protected void doExecute() { - SlicingAdminService slicingAdminService = getService(SlicingAdminService.class); - boolean result = slicingAdminService.releaseQueue(QueueId.of(queueId)); - if (result) { - print("Queue %s released", queueId); - } else { - print("Failed to release queue %s", queueId); - } - } -} diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/QueueReserveCommand.java b/src/main/java/org/stratumproject/fabric/tna/slicing/cli/QueueReserveCommand.java deleted file mode 100644 index 03d6b1174..000000000 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/QueueReserveCommand.java +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright 2021-present Open Networking Foundation -// SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 -package org.stratumproject.fabric.tna.slicing.cli; - -import org.apache.karaf.shell.api.action.Argument; -import org.apache.karaf.shell.api.action.Command; -import org.apache.karaf.shell.api.action.lifecycle.Service; -import org.onosproject.cli.AbstractShellCommand; -import org.stratumproject.fabric.tna.slicing.api.QueueId; -import org.stratumproject.fabric.tna.slicing.api.SlicingAdminService; -import org.stratumproject.fabric.tna.slicing.api.TrafficClass; - -/** - * Reserve queue for TC. - */ -@Service -@Command(scope = "fabric-tna", name = "queue-reserve", description = "Reserve queue for TC") -public class QueueReserveCommand extends AbstractShellCommand { - @Argument(index = 0, name = "queueId", - description = "queueId. Used to identify a queue.", - required = true, multiValued = false) - int queueId; - - @Argument(index = 1, name = "tc", - description = "Traffic class. Used to classify the traffic.", - required = true, multiValued = false) - String tc; - - @Override - protected void doExecute() { - SlicingAdminService slicingAdminService = getService(SlicingAdminService.class); - boolean result = slicingAdminService.reserveQueue(QueueId.of(queueId), TrafficClass.valueOf(tc)); - if (result) { - print("Queue %s reserved for TC %s", queueId, tc); - } else { - print("Failed to reserve queue %s for TC %s", queueId, tc); - } - } -} diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/QueueStoreGetCommand.java b/src/main/java/org/stratumproject/fabric/tna/slicing/cli/QueueStoreGetCommand.java deleted file mode 100644 index 790579b60..000000000 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/QueueStoreGetCommand.java +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2021-present Open Networking Foundation -// SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 -package org.stratumproject.fabric.tna.slicing.cli; - -import org.apache.karaf.shell.api.action.Command; -import org.apache.karaf.shell.api.action.lifecycle.Service; -import org.onosproject.cli.AbstractShellCommand; -import org.stratumproject.fabric.tna.slicing.api.SlicingAdminService; - -import java.util.Map; - -/** - * Get slice store entries. - */ -@Service -@Command(scope = "fabric-tna", name = "queue-store", description = "Get queue store entries") -public class QueueStoreGetCommand extends AbstractShellCommand { - - @Override - protected void doExecute() { - SlicingAdminService slicingAdminService = getService(SlicingAdminService.class); - slicingAdminService.getQueueStore().entrySet().stream().sorted(Map.Entry.comparingByKey()).forEach(e -> { - print("%s -> %s", e.getKey(), e.getValue()); - }); - } -} From 40608418f1e99861cc6843fa8be45430fecbf179 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Wed, 12 Jan 2022 17:01:04 -0800 Subject: [PATCH 09/33] Comments and renaming on SlicingManager --- .../fabric/tna/slicing/SlicingManager.java | 101 ++++++++++-------- .../tna/slicing/api/SlicingService.java | 6 +- .../tna/slicing/cli/FlowAddCommand.java | 2 +- .../tna/slicing/cli/FlowGetCommand.java | 2 +- .../tna/slicing/cli/FlowRemoveCommand.java | 2 +- .../fabric/tna/web/SlicingWebResource.java | 6 +- .../tna/slicing/SlicingManagerTest.java | 28 ++--- 7 files changed, 78 insertions(+), 69 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java index a06d15a9f..16ba365f2 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java @@ -21,7 +21,6 @@ import org.onosproject.net.flow.TrafficSelector; import org.onosproject.net.flow.criteria.PiCriterion; import org.onosproject.net.intent.WorkPartitionService; -import org.onosproject.net.pi.model.PiPipeconf; import org.onosproject.net.pi.model.PiTableId; import org.onosproject.net.pi.runtime.PiAction; import org.onosproject.net.pi.runtime.PiActionParam; @@ -58,7 +57,6 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; -import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -121,14 +119,17 @@ public class SlicingManager implements SlicingService, SlicingProviderService, S protected ApplicationId appId; + // Stores currently allocated slices and their traffic classes. protected ConsistentMap sliceStore; private MapEventListener sliceListener; private ExecutorService sliceExecutor; + // Stores classifier flows. protected ConsistentMap classifierFlowStore; private MapEventListener classifierFlowListener; private ExecutorService classifierFlowExecutor; + // Stores the default traffic class for each slice. protected ConsistentMap defaultTcStore; private MapEventListener defaultTcListener; private ExecutorService defaultTcExecutor; @@ -155,7 +156,8 @@ protected void activate() { .withSerializer(Serializer.using(serializer.build())) .build(); sliceListener = new InternalSliceListener(); - sliceExecutor = Executors.newSingleThreadExecutor(groupedThreads("fabric-tna-slice-event", "%d", log)); + sliceExecutor = Executors.newSingleThreadExecutor(groupedThreads( + "fabric-tna-slice-event", "%d", log)); sliceStore.addListener(sliceListener); classifierFlowStore = storageService.consistentMapBuilder() @@ -163,8 +165,9 @@ protected void activate() { .withRelaxedReadConsistency() .withSerializer(Serializer.using(serializer.build())) .build(); - classifierFlowListener = new InternalFlowListener(); - classifierFlowExecutor = Executors.newSingleThreadExecutor(groupedThreads("fabric-tna-flow-event", "%d", log)); + classifierFlowListener = new InternalClassifierFlowListener(); + classifierFlowExecutor = Executors.newSingleThreadExecutor(groupedThreads( + "fabric-tna-classifier-flow-event", "%d", log)); classifierFlowStore.addListener(classifierFlowListener); defaultTcStore = storageService.consistentMapBuilder() @@ -173,17 +176,20 @@ protected void activate() { .withSerializer(Serializer.using(serializer.build())) .build(); defaultTcListener = new InternalDefaultTcListener(); - defaultTcExecutor = Executors.newSingleThreadExecutor(groupedThreads("fabric-tna-default-tc-event", "%d", log)); + defaultTcExecutor = Executors.newSingleThreadExecutor(groupedThreads( + "fabric-tna-default-tc-event", "%d", log)); defaultTcStore.addListener(defaultTcListener); - // Default slice is pre-provisioned + // Default slice is pre-provisioned. sliceStore.put(new SliceStoreKey(SliceId.DEFAULT, TrafficClass.BEST_EFFORT), TrafficClassConfig.BEST_EFFORT); defaultTcStore.put(SliceId.DEFAULT, TrafficClass.BEST_EFFORT); deviceListener = new InternalDeviceListener(); - deviceExecutor = Executors.newSingleThreadExecutor(groupedThreads("fabric-tna-device-event", "%d", log)); + deviceExecutor = Executors.newSingleThreadExecutor(groupedThreads( + "fabric-tna-device-event", "%d", log)); deviceService.addListener(deviceListener); + // FIXME: do we still need these? REST APIs for manipulating slices are gone. codecService.registerCodec(SliceId.class, new SliceIdCodec()); codecService.registerCodec(TrafficClass.class, new TrafficClassCodec()); @@ -214,7 +220,7 @@ protected void deactivate() { @Override public boolean addSlice(SliceId sliceId) { if (sliceId.equals(SliceId.DEFAULT)) { - throw new SlicingException(INVALID, "Adding default slice is not allowed"); + throw new SlicingException(INVALID, "Adding the default slice is not allowed"); } return addTrafficClass(sliceId, TrafficClassConfig.BEST_EFFORT) && @@ -224,23 +230,23 @@ public boolean addSlice(SliceId sliceId) { @Override public boolean removeSlice(SliceId sliceId) { if (sliceId.equals(SliceId.DEFAULT)) { - throw new SlicingException(INVALID, "Removing default slice is not allowed"); + throw new SlicingException(INVALID, "Removing the default slice is not allowed"); } Set tcs = getTrafficClasses(sliceId); if (tcs.isEmpty() && !defaultTcStore.containsKey(sliceId)) { - throw new SlicingException(FAILED, String.format("Cannot remove a non-existent slice %s", sliceId)); + throw new SlicingException(FAILED, String.format("Cannot remove non-existent slice %s", sliceId)); } - Set classifierFlows = getFlows(sliceId); + Set classifierFlows = getClassifierFlows(sliceId); if (!classifierFlows.isEmpty()) { throw new SlicingException(FAILED, - String.format("Cannot remove slice %s with %d Flow Classifier Rules", - sliceId, classifierFlows.size())); + String.format("Cannot remove slice %s with %d classifier flow rules", + sliceId, classifierFlows.size())); } - // Ensure to remove the default TC before removing the actual traffic classes + // Remove the default TC before removing the actual traffic classes. defaultTcStore.remove(sliceId); tcs.stream() @@ -281,7 +287,7 @@ public boolean addTrafficClass(SliceId sliceId, TrafficClassConfig tcConfig) { @Override public boolean removeTrafficClass(SliceId sliceId, TrafficClass tc) { - Set classifierFlows = getFlows(sliceId, tc); + Set classifierFlows = getClassifierFlows(sliceId, tc); if (!classifierFlows.isEmpty()) { throw new SlicingException(FAILED, String.format("Cannot remove %s from slice %s with %d Flow Classifier Rules", @@ -355,7 +361,7 @@ public Map getSliceStore() { } @Override - public boolean addFlow(TrafficSelector selector, SliceId sliceId, TrafficClass tc) { + public boolean addClassifierFlow(TrafficSelector selector, SliceId sliceId, TrafficClass tc) { if (selector.equals(DefaultTrafficSelector.emptySelector())) { throw new SlicingException(INVALID, "Empty traffic selector is not allowed"); } @@ -375,13 +381,13 @@ public boolean addFlow(TrafficSelector selector, SliceId sliceId, TrafficClass t } @Override - public boolean removeFlow(TrafficSelector selector, SliceId sliceId, TrafficClass tc) { + public boolean removeClassifierFlow(TrafficSelector selector, SliceId sliceId, TrafficClass tc) { StringBuilder errorMessage = new StringBuilder(); classifierFlowStore.compute(selector, (k, v) -> { if (v == null) { errorMessage.append( - String.format("There is no such Flow Classifier Rule %s for slice %s and TC %s", - selector, sliceId, tc)); + String.format("There is no such Flow Classifier Rule %s for slice %s and TC %s", + selector, sliceId, tc)); return null; } log.info("Removing flow {} from slice {} tc {}", selector, sliceId, tc); @@ -396,7 +402,7 @@ public boolean removeFlow(TrafficSelector selector, SliceId sliceId, TrafficClas } @Override - public Set getFlows(SliceId sliceId, TrafficClass tc) { + public Set getClassifierFlows(SliceId sliceId, TrafficClass tc) { SliceStoreKey value = new SliceStoreKey(sliceId, tc); return classifierFlowStore.entrySet().stream() @@ -405,7 +411,7 @@ public Set getFlows(SliceId sliceId, TrafficClass tc) { .collect(Collectors.toSet()); } - private Set getFlows(SliceId sliceId) { + private Set getClassifierFlows(SliceId sliceId) { return classifierFlowStore.entrySet().stream() .filter(e -> e.getValue().value().sliceId().equals(sliceId)) .map(Entry::getKey) @@ -446,43 +452,41 @@ private void resetDefaultTrafficClass(DeviceId deviceId, SliceId sliceId, Traffi log.info("Remove default TC on {} for slice {}", deviceId, sliceId); } - private void addQueueTable(DeviceId deviceId, SliceId sliceId, TrafficClass tc, QueueId queueId) { - buildFlowRules(deviceId, sliceId, tc, queueId).forEach(f -> flowRuleService.applyFlowRules(f)); + private void addQueuesFlowRules(DeviceId deviceId, SliceId sliceId, TrafficClass tc, QueueId queueId) { + buildQueuesFlowRules(deviceId, sliceId, tc, queueId).forEach(f -> flowRuleService.applyFlowRules(f)); log.info("Add queue table flow on {} for slice {} tc {} queueId {}", deviceId, sliceId, tc, queueId); } - private void removeQueueTable(DeviceId deviceId, SliceId sliceId, TrafficClass tc, QueueId queueId) { - buildFlowRules(deviceId, sliceId, tc, queueId).forEach(f -> flowRuleService.removeFlowRules(f)); + private void removeQueuesFlowRules(DeviceId deviceId, SliceId sliceId, TrafficClass tc, QueueId queueId) { + buildQueuesFlowRules(deviceId, sliceId, tc, queueId).forEach(f -> flowRuleService.removeFlowRules(f)); log.info("Remove queue table flow on {} for slice {} tc {} queueId {}", deviceId, sliceId, tc, queueId); } private FabricCapabilities getCapabilities(DeviceId deviceId) throws RuntimeException { - Optional pipeconf = pipeconfService.getPipeconf(deviceId); - return pipeconf + return pipeconfService.getPipeconf(deviceId) .map(FabricCapabilities::new) - .orElseThrow( - () -> new RuntimeException("Cannot get capabilities for deviceId " + deviceId.toString() - )); + .orElseThrow(() -> new RuntimeException( + "Cannot get capabilities for deviceId " + deviceId.toString())); } - private List buildFlowRules(DeviceId deviceId, SliceId sliceId, TrafficClass tc, QueueId queueId) { + private List buildQueuesFlowRules(DeviceId deviceId, SliceId sliceId, TrafficClass tc, QueueId queueId) { List flowRules = Lists.newArrayList(); if (tc == TrafficClass.CONTROL) { int red = getCapabilities(deviceId).getMeterColor(Color.RED); int green = getCapabilities(deviceId).getMeterColor(Color.GREEN); - flowRules.add(buildFlowRule(deviceId, sliceId, tc, queueId, green)); - flowRules.add(buildFlowRule(deviceId, sliceId, tc, QueueId.BEST_EFFORT, red)); + flowRules.add(buildQueuesFlowRule(deviceId, sliceId, tc, queueId, green)); + flowRules.add(buildQueuesFlowRule(deviceId, sliceId, tc, QueueId.BEST_EFFORT, red)); } else { - flowRules.add(buildFlowRule(deviceId, sliceId, tc, queueId, null)); + flowRules.add(buildQueuesFlowRule(deviceId, sliceId, tc, queueId, null)); } return flowRules; } - private FlowRule buildFlowRule(DeviceId deviceId, - SliceId sliceId, - TrafficClass tc, - QueueId queueId, - Integer color) { + private FlowRule buildQueuesFlowRule(DeviceId deviceId, + SliceId sliceId, + TrafficClass tc, + QueueId queueId, + Integer color) { PiCriterion.Builder piCriterionBuilder = PiCriterion.builder() .matchExact(P4InfoConstants.HDR_SLICE_TC, sliceTcConcat(sliceId.id(), tc.ordinal())); if (color != null) { @@ -545,7 +549,8 @@ private FlowRule buildClassifierFlowRule(DeviceId deviceId, private class InternalSliceListener implements MapEventListener { public void event(MapEvent event) { - // Distributed work based on QueueId. Consistent with InternalQueueListener + // Update queues table on all devices. + // Distribute work based on QueueId. log.info("Processing slice event {}", event); sliceExecutor.submit(() -> { switch (event.type()) { @@ -553,7 +558,7 @@ public void event(MapEvent event) { case UPDATE: if (workPartitionService.isMine(event.newValue().value(), toStringHasher())) { deviceService.getAvailableDevices().forEach(device -> - addQueueTable(device.id(), + addQueuesFlowRules(device.id(), event.key().sliceId(), event.key().trafficClass(), event.newValue().value().queueId()) ); } @@ -561,8 +566,8 @@ public void event(MapEvent event) { case REMOVE: if (workPartitionService.isMine(event.oldValue().value(), toStringHasher())) { deviceService.getAvailableDevices().forEach(device -> - removeQueueTable(device.id(), - event.key().sliceId(), event.key().trafficClass(), event.oldValue().value().queueId()) + removeQueuesFlowRules(device.id(), + event.key().sliceId(), event.key().trafficClass(), event.oldValue().value().queueId()) ); } break; @@ -573,8 +578,9 @@ public void event(MapEvent event) { } } - private class InternalFlowListener implements MapEventListener { + private class InternalClassifierFlowListener implements MapEventListener { public void event(MapEvent event) { + // Update classifier table on devices. log.info("Processing flow classifier event {}", event); classifierFlowExecutor.submit(() -> { switch (event.type()) { @@ -582,6 +588,7 @@ public void event(MapEvent event) { case UPDATE: if (workPartitionService.isMine(event.newValue().value(), toStringHasher())) { deviceService.getAvailableDevices().forEach(device -> { + // Classify traffic only at the edge. We use DSCP for intermediate hops. if (isLeafSwitch(device.id())) { addClassifierFlowRule(device.id(), event.key(), event.newValue().value().sliceId(), event.newValue().value().trafficClass()); @@ -609,6 +616,7 @@ public void event(MapEvent event) { private class InternalDefaultTcListener implements MapEventListener { @Override public void event(MapEvent event) { + // Update default tc tables. log.info("Processing Default TC event {}", event); defaultTcExecutor.submit(() -> { switch (event.type()) { @@ -643,6 +651,7 @@ public void event(MapEvent event) { private class InternalDeviceListener implements DeviceListener { @Override public void event(DeviceEvent event) { + // Provision all tables on device. log.info("Processing device event {}", event); deviceExecutor.submit(() -> { switch (event.type()) { @@ -651,7 +660,7 @@ public void event(DeviceEvent event) { DeviceId deviceId = event.subject().id(); if (workPartitionService.isMine(deviceId, toStringHasher())) { if (deviceService.isAvailable(deviceId)) { - sliceStore.forEach(e -> addQueueTable(deviceId, + sliceStore.forEach(e -> addQueuesFlowRules(deviceId, e.getKey().sliceId(), e.getKey().trafficClass(), e.getValue().value().queueId()) ); if (isLeafSwitch(deviceId)) { diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingService.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingService.java index b402a5579..c0bc4dedb 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingService.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingService.java @@ -55,7 +55,7 @@ public interface SlicingService { * @return true if the flow is associated with given slice/tc successfully. * @throws SlicingException if an error occurred. */ - boolean addFlow(TrafficSelector selector, SliceId sliceId, TrafficClass tc); + boolean addClassifierFlow(TrafficSelector selector, SliceId sliceId, TrafficClass tc); /** * Dissociates flow identified by given selector with given slice ID and traffic class. @@ -66,7 +66,7 @@ public interface SlicingService { * @return true if the flow is dissociate with given slice/tc successfully. * @throws SlicingException if an error occurred. */ - boolean removeFlow(TrafficSelector selector, SliceId sliceId, TrafficClass tc); + boolean removeClassifierFlow(TrafficSelector selector, SliceId sliceId, TrafficClass tc); /** * Gets all flows in given sliceId and traffic class. @@ -75,5 +75,5 @@ public interface SlicingService { * @param tc traffic class * @return set of flow identifiers */ - Set getFlows(SliceId sliceId, TrafficClass tc); + Set getClassifierFlows(SliceId sliceId, TrafficClass tc); } diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/FlowAddCommand.java b/src/main/java/org/stratumproject/fabric/tna/slicing/cli/FlowAddCommand.java index 9ba6896e5..f637d3caf 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/FlowAddCommand.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/cli/FlowAddCommand.java @@ -71,7 +71,7 @@ protected void doExecute() { SlicingService slicingService = getService(SlicingService.class); TrafficSelector selector = parseArguments(); - if (slicingService.addFlow(selector, SliceId.of(sliceId), TrafficClass.valueOf(tc))) { + if (slicingService.addClassifierFlow(selector, SliceId.of(sliceId), TrafficClass.valueOf(tc))) { print("Flow %s added to slice %d tc %s", selector.toString(), sliceId, tc); } } diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/FlowGetCommand.java b/src/main/java/org/stratumproject/fabric/tna/slicing/cli/FlowGetCommand.java index ace961708..c052488ab 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/FlowGetCommand.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/cli/FlowGetCommand.java @@ -30,6 +30,6 @@ public class FlowGetCommand extends AbstractShellCommand { @Override protected void doExecute() { SlicingService slicingService = getService(SlicingService.class); - print(slicingService.getFlows(SliceId.of(sliceId), TrafficClass.valueOf(tc)).toString()); + print(slicingService.getClassifierFlows(SliceId.of(sliceId), TrafficClass.valueOf(tc)).toString()); } } diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/FlowRemoveCommand.java b/src/main/java/org/stratumproject/fabric/tna/slicing/cli/FlowRemoveCommand.java index def9d70d8..a63150d86 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/cli/FlowRemoveCommand.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/cli/FlowRemoveCommand.java @@ -71,7 +71,7 @@ protected void doExecute() { SlicingService slicingService = getService(SlicingService.class); TrafficSelector selector = parseArguments(); - if (slicingService.removeFlow(selector, SliceId.of(sliceId), TrafficClass.valueOf(tc))) { + if (slicingService.removeClassifierFlow(selector, SliceId.of(sliceId), TrafficClass.valueOf(tc))) { print("Flow %s removed from slice %d tc %s", selector.toString(), sliceId, tc); } } diff --git a/src/main/java/org/stratumproject/fabric/tna/web/SlicingWebResource.java b/src/main/java/org/stratumproject/fabric/tna/web/SlicingWebResource.java index 3ad0fde2f..76553b2cd 100644 --- a/src/main/java/org/stratumproject/fabric/tna/web/SlicingWebResource.java +++ b/src/main/java/org/stratumproject/fabric/tna/web/SlicingWebResource.java @@ -141,7 +141,7 @@ public Response setDefaultTc(@PathParam("sliceId") int sliceId, @PathParam("tc") @GET @Path("flow/{sliceId}/{tc}") public Response getFlow(@PathParam("sliceId") int sliceId, @PathParam("tc") String tc) { - Set result = slicingService.getFlows(SliceId.of(sliceId), TrafficClass.valueOf(tc)); + Set result = slicingService.getClassifierFlows(SliceId.of(sliceId), TrafficClass.valueOf(tc)); ObjectNode root = mapper().createObjectNode(); ArrayNode array = root.putArray("TrafficSelector"); @@ -182,7 +182,7 @@ public Response addFlow(@PathParam("sliceId") int sliceId, @PathParam("tc") Stri try { ObjectNode jsonTree = readTreeFromStream(mapper(), input); TrafficSelector selector = codec(TrafficSelector.class).decode(jsonTree, this); - result = slicingService.addFlow(selector, SliceId.of(sliceId), TrafficClass.valueOf(tc)); + result = slicingService.addClassifierFlow(selector, SliceId.of(sliceId), TrafficClass.valueOf(tc)); } catch (IOException ex) { throw new IllegalArgumentException(ex); } @@ -215,7 +215,7 @@ public Response removeFlow(@PathParam("sliceId") int sliceId, @PathParam("tc") S try { ObjectNode jsonTree = readTreeFromStream(mapper(), input); TrafficSelector selector = codec(TrafficSelector.class).decode(jsonTree, this); - result = slicingService.removeFlow(selector, SliceId.of(sliceId), TrafficClass.valueOf(tc)); + result = slicingService.removeClassifierFlow(selector, SliceId.of(sliceId), TrafficClass.valueOf(tc)); } catch (IOException ex) { throw new IllegalArgumentException(ex); } diff --git a/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java b/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java index ec7b1e939..23c39e852 100644 --- a/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java +++ b/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java @@ -386,16 +386,16 @@ public void testAddFlowClassifier() { manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.REAL_TIME); // Normal - manager.addFlow(selector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); - assertEquals(1, manager.getFlows(SLICE_IDS.get(1), TrafficClass.REAL_TIME).size()); - assertTrue(manager.getFlows(SLICE_IDS.get(1), TrafficClass.REAL_TIME).contains(selector)); + manager.addClassifierFlow(selector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); + assertEquals(1, manager.getClassifierFlows(SLICE_IDS.get(1), TrafficClass.REAL_TIME).size()); + assertTrue(manager.getClassifierFlows(SLICE_IDS.get(1), TrafficClass.REAL_TIME).contains(selector)); } @Test public void testAddFlowClassifierException1() { exceptionRule.expect(SlicingException.class); exceptionRule.expectMessage("Empty traffic selector is not allowed"); - manager.addFlow(DefaultTrafficSelector.builder().build(), + manager.addClassifierFlow(DefaultTrafficSelector.builder().build(), SLICE_IDS.get(1), TrafficClass.REAL_TIME); } @@ -406,7 +406,7 @@ public void testAddFlowClassifierException2() { exceptionRule.expect(SlicingException.class); exceptionRule.expectMessage("Only accept 5-tuple DefaultTrafficSelector{criteria=[ETH_DST:01:00:5E:00:00:00]}"); - manager.addFlow(wrongSelector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); + manager.addClassifierFlow(wrongSelector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); } @Test @@ -415,12 +415,12 @@ public void testRemoveFlowClassifier() { TrafficSelector selector = DefaultTrafficSelector.builder().matchUdpDst(TpPort.tpPort(100)).build(); manager.addSlice(SLICE_IDS.get(1)); manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.REAL_TIME); - manager.addFlow(selector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); + manager.addClassifierFlow(selector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); // Normal - assertEquals(1, manager.getFlows(SLICE_IDS.get(1), TrafficClass.REAL_TIME).size()); - manager.removeFlow(selector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); - assertEquals(0, manager.getFlows(SLICE_IDS.get(1), TrafficClass.REAL_TIME).size()); + assertEquals(1, manager.getClassifierFlows(SLICE_IDS.get(1), TrafficClass.REAL_TIME).size()); + manager.removeClassifierFlow(selector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); + assertEquals(0, manager.getClassifierFlows(SLICE_IDS.get(1), TrafficClass.REAL_TIME).size()); } @Test @@ -431,7 +431,7 @@ public void testRemoveFlowClassifierException() { exceptionRule.expect(SlicingException.class); exceptionRule.expectMessage("There is no such Flow Classifier Rule " + "DefaultTrafficSelector{criteria=[TCP_DST:100]} for slice 1 and TC REAL_TIME"); - manager.removeFlow(wrongSelector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); + manager.removeClassifierFlow(wrongSelector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); } @Test @@ -440,7 +440,7 @@ public void testRemoveSliceAndTcWithFlowClassifierException1() { TrafficSelector selector = DefaultTrafficSelector.builder().matchUdpDst(TpPort.tpPort(100)).build(); manager.addSlice(SLICE_IDS.get(1)); manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.REAL_TIME); - manager.addFlow(selector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); + manager.addClassifierFlow(selector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); // Fail to remove Slice and TC when Flow Classifier exceptionRule.expect(SlicingException.class); @@ -454,7 +454,7 @@ public void testRemoveSliceAndTcWithFlowClassifierException2() { TrafficSelector selector = DefaultTrafficSelector.builder().matchUdpDst(TpPort.tpPort(100)).build(); manager.addSlice(SLICE_IDS.get(1)); manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.REAL_TIME); - manager.addFlow(selector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); + manager.addClassifierFlow(selector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); // Fail to remove Slice and TC when Flow Classifier exceptionRule.expect(SlicingException.class); @@ -592,7 +592,7 @@ public void testFlowListener() { // Adding mock rule to slice 1 BE capturedAddedFlowRules.reset(); - manager.addFlow(selector, SLICE_IDS.get(1), TrafficClass.BEST_EFFORT); + manager.addClassifierFlow(selector, SLICE_IDS.get(1), TrafficClass.BEST_EFFORT); assertAfter(50, () -> { assertEquals(1 * numDevices, capturedAddedFlowRules.getValues().size()); assertTrue(capturedAddedFlowRules.getValues() @@ -601,7 +601,7 @@ public void testFlowListener() { // Removing mock rule from slice 1 BE capturedRemovedFlowRules.reset(); - manager.removeFlow(selector, SLICE_IDS.get(1), TrafficClass.BEST_EFFORT); + manager.removeClassifierFlow(selector, SLICE_IDS.get(1), TrafficClass.BEST_EFFORT); assertAfter(50, () -> { assertEquals(1 * numDevices, capturedRemovedFlowRules.getValues().size()); assertTrue(capturedRemovedFlowRules.getValues() From e0a8eadb7254ec7832ed1a8754e888e1d5f79e6b Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Wed, 12 Jan 2022 19:25:55 -0800 Subject: [PATCH 10/33] Fix meter color int value --- .../tna/behaviour/FabricCapabilities.java | 8 ++++---- .../fabric/tna/slicing/SlicingManager.java | 7 ++++--- .../slicing/api/{Color.java => MeterColor.java} | 17 +++++++++++++---- 3 files changed, 21 insertions(+), 11 deletions(-) rename src/main/java/org/stratumproject/fabric/tna/slicing/api/{Color.java => MeterColor.java} (62%) diff --git a/src/main/java/org/stratumproject/fabric/tna/behaviour/FabricCapabilities.java b/src/main/java/org/stratumproject/fabric/tna/behaviour/FabricCapabilities.java index f81351299..b9bea2c30 100644 --- a/src/main/java/org/stratumproject/fabric/tna/behaviour/FabricCapabilities.java +++ b/src/main/java/org/stratumproject/fabric/tna/behaviour/FabricCapabilities.java @@ -5,7 +5,7 @@ import org.onosproject.net.pi.model.PiPipeconf; import org.slf4j.Logger; -import org.stratumproject.fabric.tna.slicing.api.Color; +import org.stratumproject.fabric.tna.slicing.api.MeterColor; import java.io.BufferedReader; import java.io.IOException; @@ -70,11 +70,11 @@ public boolean isArchTna() { .orElse(false); } - public int getMeterColor(Color color) { - if (isArchV1model() && color == Color.RED) { + public int getMeterColor(MeterColor color) { + if (isArchV1model() && color == MeterColor.RED) { return BMV2_COLOR_RED; } else { - return color.ordinal(); + return color.toInt(); } } diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java index 16ba365f2..049d66ea1 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java @@ -41,7 +41,7 @@ import org.slf4j.Logger; import org.stratumproject.fabric.tna.behaviour.FabricCapabilities; import org.stratumproject.fabric.tna.behaviour.P4InfoConstants; -import org.stratumproject.fabric.tna.slicing.api.Color; +import org.stratumproject.fabric.tna.slicing.api.MeterColor; import org.stratumproject.fabric.tna.slicing.api.QueueId; import org.stratumproject.fabric.tna.slicing.api.SliceId; import org.stratumproject.fabric.tna.slicing.api.SlicingAdminService; @@ -63,6 +63,7 @@ import java.util.function.Function; import java.util.stream.Collectors; +import static java.lang.String.format; import static org.onlab.util.Tools.groupedThreads; import static org.slf4j.LoggerFactory.getLogger; import static org.stratumproject.fabric.tna.behaviour.FabricUtils.fiveTupleOnly; @@ -472,8 +473,8 @@ private FabricCapabilities getCapabilities(DeviceId deviceId) throws RuntimeExce private List buildQueuesFlowRules(DeviceId deviceId, SliceId sliceId, TrafficClass tc, QueueId queueId) { List flowRules = Lists.newArrayList(); if (tc == TrafficClass.CONTROL) { - int red = getCapabilities(deviceId).getMeterColor(Color.RED); - int green = getCapabilities(deviceId).getMeterColor(Color.GREEN); + int red = getCapabilities(deviceId).getMeterColor(MeterColor.RED); + int green = getCapabilities(deviceId).getMeterColor(MeterColor.GREEN); flowRules.add(buildQueuesFlowRule(deviceId, sliceId, tc, queueId, green)); flowRules.add(buildQueuesFlowRule(deviceId, sliceId, tc, QueueId.BEST_EFFORT, red)); } else { diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/Color.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/MeterColor.java similarity index 62% rename from src/main/java/org/stratumproject/fabric/tna/slicing/api/Color.java rename to src/main/java/org/stratumproject/fabric/tna/slicing/api/MeterColor.java index d603cac98..4927227e1 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/Color.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/MeterColor.java @@ -9,14 +9,23 @@ /** * Meter bucket color. */ -public enum Color { +public enum MeterColor { GREEN(COLOR_GREEN), YELLOW(COLOR_YELLOW), RED(COLOR_RED); - public final int color; + public final int intValue; - Color(int color) { - this.color = color; + MeterColor(int intValue) { + this.intValue = intValue; + } + + /** + * Returns the integer value of this color to be used for flow programming. + * + * @return integer value + */ + public int toInt() { + return intValue; } } From 2adadd31e9c1cb01015487f1e79a622ff150233e Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Wed, 12 Jan 2022 19:27:18 -0800 Subject: [PATCH 11/33] Less ambiguous handling of BE --- .../fabric/tna/slicing/SlicingManager.java | 70 ++++++++++++++----- 1 file changed, 52 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java index 049d66ea1..d5ddc52a4 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java @@ -224,7 +224,11 @@ public boolean addSlice(SliceId sliceId) { throw new SlicingException(INVALID, "Adding the default slice is not allowed"); } - return addTrafficClass(sliceId, TrafficClassConfig.BEST_EFFORT) && + if (sliceExists(sliceId)) { + throw new SlicingException(FAILED, format("Slice %s already exists", sliceId)); + } + + return addTrafficClassInternal(true, sliceId, TrafficClassConfig.BEST_EFFORT) && setDefaultTrafficClass(sliceId, TrafficClass.BEST_EFFORT); } @@ -237,26 +241,28 @@ public boolean removeSlice(SliceId sliceId) { Set tcs = getTrafficClasses(sliceId); if (tcs.isEmpty() && !defaultTcStore.containsKey(sliceId)) { - throw new SlicingException(FAILED, String.format("Cannot remove non-existent slice %s", sliceId)); + throw new SlicingException(FAILED, format("Cannot remove non-existent slice %s", sliceId)); } Set classifierFlows = getClassifierFlows(sliceId); if (!classifierFlows.isEmpty()) { throw new SlicingException(FAILED, - String.format("Cannot remove slice %s with %d classifier flow rules", + format("Cannot remove slice %s with %d classifier flow rules", sliceId, classifierFlows.size())); } // Remove the default TC before removing the actual traffic classes. defaultTcStore.remove(sliceId); - tcs.stream() - .sorted(Comparator.comparingInt(TrafficClass::ordinal).reversed()) // Remove BEST_EFFORT the last - .forEach(tc -> removeTrafficClass(sliceId, tc)); + tcs.forEach(tc -> removeTrafficClassInternal(true, sliceId, tc)); return true; } + private boolean sliceExists(SliceId sliceId) { + return !getTrafficClasses(sliceId).isEmpty(); + } + @Override public Set getSlices() { return sliceStore.keySet().stream() @@ -266,11 +272,21 @@ public Set getSlices() { @Override public boolean addTrafficClass(SliceId sliceId, TrafficClassConfig tcConfig) { + return addTrafficClassInternal(false, sliceId, tcConfig); + } + + private boolean addTrafficClassInternal(boolean addSlice, SliceId sliceId, TrafficClassConfig tcConfig) { + if (!addSlice && !sliceExists(sliceId)) { + throw new SlicingException(INVALID, format( + "Cannot add traffic class to non-existent slice %s", sliceId)); + } + StringBuilder errorMessage = new StringBuilder(); SliceStoreKey key = new SliceStoreKey(sliceId, tcConfig.trafficClass()); sliceStore.compute(key, (k, v) -> { if (v != null) { - errorMessage.append(String.format("TC %s is already allocated for slice %s", tcConfig, sliceId)); + errorMessage.append(format("TC %s is already allocated for slice %s", + tcConfig.trafficClass(), sliceId)); return v; } @@ -288,24 +304,42 @@ public boolean addTrafficClass(SliceId sliceId, TrafficClassConfig tcConfig) { @Override public boolean removeTrafficClass(SliceId sliceId, TrafficClass tc) { + return removeTrafficClassInternal(false, sliceId, tc); + } + + private boolean removeTrafficClassInternal(boolean removeSlice, SliceId sliceId, TrafficClass tc) { + if (!sliceExists(sliceId)) { + throw new SlicingException(INVALID, format( + "Cannot remove a traffic class from non-existent slice %s", sliceId)); + } + + if (!removeSlice && tc == TrafficClass.BEST_EFFORT) { + throw new SlicingException(INVALID, + "Cannot remove BEST_EFFORT traffic class from any slice"); + } + Set classifierFlows = getClassifierFlows(sliceId, tc); if (!classifierFlows.isEmpty()) { throw new SlicingException(FAILED, - String.format("Cannot remove %s from slice %s with %d Flow Classifier Rules", - tc, sliceId, classifierFlows.size())); + format("Cannot remove %s from slice %s with %d classifier flow rules", + tc, sliceId, classifierFlows.size())); } StringBuilder errorMessage = new StringBuilder(); SliceStoreKey key = new SliceStoreKey(sliceId, tc); sliceStore.compute(key, (k, v) -> { if (v == null) { - errorMessage.append(String.format("TC %s has not been allocated to slice %s", tc, sliceId)); + errorMessage.append(format( + "Traffic class %s has not been allocated for slice %s", + tc, sliceId)); return null; } // Ensure the TC is not being used as Default TC if (tc == getDefaultTrafficClass(sliceId)) { - errorMessage.append(String.format("Can't remove %s from slice %s while it is being used as Default TC", - tc, sliceId)); + errorMessage.append(format( + "Cannot remove %s from slice %s while it is being used " + + "as the default traffic class", + tc, sliceId)); return v; } @@ -336,7 +370,7 @@ public boolean setDefaultTrafficClass(SliceId sliceId, TrafficClass tc) { // then the switch should pick best effort. sliceStore.compute(new SliceStoreKey(sliceId, tc), (k, v) -> { if (v == null) { - errorMessage.append(String.format("Can't set %s as default TC because it has not" + + errorMessage.append(format("Cannot set %s as the default traffic class because it has not" + " been allocated to slice %s", tc, sliceId)); } else { defaultTcStore.put(sliceId, tc); @@ -369,7 +403,7 @@ public boolean addClassifierFlow(TrafficSelector selector, SliceId sliceId, Traf // Accept 5-tuple only if (!fiveTupleOnly(selector)) { throw new SlicingException(UNSUPPORTED, - String.format("Only accept 5-tuple %s", selector)); + "Selector can only express a match on the L3-L4 5-tuple fields"); } SliceStoreKey value = new SliceStoreKey(sliceId, tc); @@ -387,7 +421,7 @@ public boolean removeClassifierFlow(TrafficSelector selector, SliceId sliceId, T classifierFlowStore.compute(selector, (k, v) -> { if (v == null) { errorMessage.append( - String.format("There is no such Flow Classifier Rule %s for slice %s and TC %s", + format("There is no such Flow Classifier Rule %s for slice %s and TC %s", selector, sliceId, tc)); return null; } @@ -426,7 +460,7 @@ private FlowRule buildDefaultTcFlowRule(DeviceId deviceId, SliceId sliceId, Traf PiAction.Builder piTableActionBuilder = PiAction.builder() .withId(P4InfoConstants.FABRIC_INGRESS_QOS_SET_DEFAULT_TC) - .withParameter(new PiActionParam(P4InfoConstants.TC, tc.ordinal())); + .withParameter(new PiActionParam(P4InfoConstants.TC, tc.toInt())); FlowRule flowRule = DefaultFlowRule.builder() .forDevice(deviceId) @@ -489,7 +523,7 @@ private FlowRule buildQueuesFlowRule(DeviceId deviceId, QueueId queueId, Integer color) { PiCriterion.Builder piCriterionBuilder = PiCriterion.builder() - .matchExact(P4InfoConstants.HDR_SLICE_TC, sliceTcConcat(sliceId.id(), tc.ordinal())); + .matchExact(P4InfoConstants.HDR_SLICE_TC, sliceTcConcat(sliceId.id(), tc.toInt())); if (color != null) { piCriterionBuilder.matchTernary(HDR_COLOR, color, 1 << HDR_COLOR_BITWIDTH - 1); } @@ -532,7 +566,7 @@ private FlowRule buildClassifierFlowRule(DeviceId deviceId, PiAction.Builder piTableActionBuilder = PiAction.builder() .withId(P4InfoConstants.FABRIC_INGRESS_SLICE_TC_CLASSIFIER_SET_SLICE_ID_TC) .withParameters(Set.of(new PiActionParam(P4InfoConstants.SLICE_ID, sliceId.id()), - new PiActionParam(P4InfoConstants.TC, tc.ordinal()))); + new PiActionParam(P4InfoConstants.TC, tc.toInt()))); FlowRule flowRule = DefaultFlowRule.builder() .forDevice(deviceId) From 025409ed62118d92ab1d451a4a61be55f05076ec Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Wed, 12 Jan 2022 19:27:29 -0800 Subject: [PATCH 12/33] Fix tests --- .../tna/slicing/SlicingManagerTest.java | 455 +++++++----------- 1 file changed, 171 insertions(+), 284 deletions(-) diff --git a/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java b/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java index 23c39e852..395596697 100644 --- a/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java +++ b/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java @@ -2,11 +2,6 @@ // SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 package org.stratumproject.fabric.tna.slicing; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.Optional; -import java.util.Set; import org.easymock.Capture; import org.easymock.CaptureType; @@ -34,7 +29,6 @@ import org.onosproject.net.flow.TrafficSelector; import org.onosproject.net.flow.criteria.PiCriterion; import org.onosproject.net.intent.WorkPartitionService; -import org.onosproject.net.pi.model.PiPipeconf; import org.onosproject.net.pi.model.PiPipeconfId; import org.onosproject.net.pi.model.PiTableId; import org.onosproject.net.pi.runtime.PiAction; @@ -42,21 +36,29 @@ import org.onosproject.net.pi.service.PiPipeconfService; import org.onosproject.segmentrouting.config.SegmentRoutingDeviceConfig; import org.onosproject.store.service.StorageService; -import org.stratumproject.fabric.tna.behaviour.FabricCapabilities; import org.stratumproject.fabric.tna.behaviour.P4InfoConstants; import org.stratumproject.fabric.tna.behaviour.upf.MockPiPipelineModel; -import org.stratumproject.fabric.tna.slicing.api.Color; import org.stratumproject.fabric.tna.slicing.api.QueueId; import org.stratumproject.fabric.tna.slicing.api.SliceId; import org.stratumproject.fabric.tna.slicing.api.SlicingException; import org.stratumproject.fabric.tna.slicing.api.TrafficClass; +import org.stratumproject.fabric.tna.slicing.api.TrafficClassConfig; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.Optional; +import java.util.Set; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.onlab.junit.TestTools.assertAfter; +import static org.stratumproject.fabric.tna.behaviour.Constants.COLOR_GREEN; +import static org.stratumproject.fabric.tna.behaviour.Constants.COLOR_RED; import static org.stratumproject.fabric.tna.behaviour.Constants.TNA; -import static org.stratumproject.fabric.tna.behaviour.Constants.V1MODEL; import static org.stratumproject.fabric.tna.behaviour.FabricUtils.sliceTcConcat; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.FABRIC_INGRESS_QOS_DEFAULT_TC; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.FABRIC_INGRESS_QOS_QUEUES; @@ -68,15 +70,26 @@ public class SlicingManagerTest { private final SlicingManager manager = new SlicingManager(); private static final ApplicationId APP_ID = - new DefaultApplicationId(0, "org.stratumproject.fabric.tna.slicing.test"); + new DefaultApplicationId(0, "org.stratumproject.fabric.tna.slicing.test"); private static final ArrayList SLICE_IDS = new ArrayList<>(); private static final ArrayList DEVICES = new ArrayList<>(); private static final int QOS_FLOW_PRIORITY = 10; private static final int DEFAULT_TC_PRIORITY = 10; private static final int CLASSIFIER_FLOW_PRIORITY = 0; - private static final DeviceId DID = DeviceId.deviceId("device:s1"); - private static final DeviceId DID_BMV2 = DeviceId.deviceId("device:s2"); + private static final DeviceId DEVICE_ID = DeviceId.deviceId("device:s1"); + + private static final QueueId QUEUE_ID_CONTROL = QueueId.of(1); + private static final QueueId QUEUE_ID_REAL_TIME = QueueId.of(2); + private static final QueueId QUEUE_ID_ELASTIC = QueueId.of(3); + + + private static final TrafficClassConfig TC_CONFIG_CONTROL = new TrafficClassConfig( + TrafficClass.CONTROL, QUEUE_ID_CONTROL, 0, 0); + private static final TrafficClassConfig TC_CONFIG_REAL_TIME = new TrafficClassConfig( + TrafficClass.REAL_TIME, QUEUE_ID_REAL_TIME, 0, 0); + private static final TrafficClassConfig TC_CONFIG_ELASTIC = new TrafficClassConfig( + TrafficClass.ELASTIC, QUEUE_ID_ELASTIC, 0, 0); private final CoreService coreService = EasyMock.createMock(CoreService.class); private final StorageService storageService = EasyMock.createMock(StorageService.class); @@ -89,15 +102,6 @@ public class SlicingManagerTest { private final Capture capturedAddedFlowRules = Capture.newInstance(CaptureType.ALL); private final Capture capturedRemovedFlowRules = Capture.newInstance(CaptureType.ALL); - private FabricCapabilities getCapabilities(DeviceId deviceId) throws RuntimeException { - Optional pipeconf = pipeconfService.getPipeconf(deviceId); - return pipeconf - .map(FabricCapabilities::new) - .orElseThrow( - () -> new RuntimeException("Cannot get capabilities for device " + deviceId.toString() - )); - } - @Rule public ExpectedException exceptionRule = ExpectedException.none(); @@ -111,23 +115,15 @@ public void setup() { SLICE_IDS.add(SliceId.of(4)); DEVICES.clear(); - DEVICES.add(new MockDevice(DID, null)); - DEVICES.add(new MockDevice(DID_BMV2, null)); - - String bmv2PipeconfId = "org.stratumproject.fabric.bmv2"; - String tmPipeconfId = "org.stratumproject.fabric.montara_sde_9_5_0"; - MockPiPipelineModel bmv2PipelineModel = - new MockPiPipelineModel(Collections.EMPTY_LIST, - Collections.EMPTY_LIST, - V1MODEL); - MockPiPipelineModel tmPipelineModel = - new MockPiPipelineModel(Collections.EMPTY_LIST, - Collections.EMPTY_LIST, - TNA); - MockPipeconf bmv2MockPipeconf = - new MockPipeconf(new PiPipeconfId(bmv2PipeconfId), bmv2PipelineModel); - MockPipeconf tmMockPipeconf = - new MockPipeconf(new PiPipeconfId(tmPipeconfId), tmPipelineModel); + DEVICES.add(new MockDevice(DEVICE_ID, null)); + + String pipeconfId = "mock_pipeconf"; + MockPiPipelineModel pipelineModel = + new MockPiPipelineModel(Collections.emptyList(), + Collections.emptyList(), + TNA); + MockPipeconf mockPipeconf = + new MockPipeconf(new PiPipeconfId(pipeconfId), pipelineModel); manager.appId = APP_ID; manager.coreService = coreService; @@ -140,20 +136,18 @@ public void setup() { manager.pipeconfService = pipeconfService; EasyMock.expect(coreService.registerApplication(EasyMock.anyObject())).andReturn(APP_ID); - EasyMock.expect(storageService.consistentMapBuilder()).andReturn( - new MockConsistentMap.Builder()); - EasyMock.expect(storageService.consistentMapBuilder()).andReturn( - new MockConsistentMap.Builder()); + EasyMock.expect(storageService.consistentMapBuilder()).andReturn( + new MockConsistentMap.Builder<>()); EasyMock.expect(storageService.consistentMapBuilder()).andReturn( - new MockConsistentMap.Builder()); + new MockConsistentMap.Builder<>()); EasyMock.expect(storageService.consistentMapBuilder()).andReturn( - new MockConsistentMap.Builder()); + new MockConsistentMap.Builder<>()); EasyMock.expect(workPartitionService.isMine( - EasyMock.anyObject(), EasyMock.anyObject())).andReturn(true).anyTimes(); + EasyMock.anyObject(), EasyMock.anyObject())).andReturn(true).anyTimes(); EasyMock.expect(deviceService.getAvailableDevices()).andReturn(DEVICES).anyTimes(); EasyMock.expect(nwCfgService.getConfig(EasyMock.anyObject(), EasyMock.eq(SegmentRoutingDeviceConfig.class))) - .andReturn( - new SegmentRoutingDeviceConfig() { + .andReturn( + new SegmentRoutingDeviceConfig() { @Override public Boolean isEdgeRouter() { return true; @@ -168,8 +162,7 @@ public Boolean isEdgeRouter() { EasyMock.expectLastCall().anyTimes(); codecService.registerCodec(EasyMock.anyObject(), EasyMock.anyObject()); EasyMock.expectLastCall().times(2); - EasyMock.expect(pipeconfService.getPipeconf(DID_BMV2)).andReturn(Optional.of(bmv2MockPipeconf)).anyTimes(); - EasyMock.expect(pipeconfService.getPipeconf(DID)).andReturn(Optional.of(tmMockPipeconf)).anyTimes(); + EasyMock.expect(pipeconfService.getPipeconf(DEVICE_ID)).andReturn(Optional.of(mockPipeconf)).anyTimes(); EasyMock.replay(coreService, storageService, workPartitionService, deviceService, flowRuleService, codecService, nwCfgService, pipeconfService); @@ -182,11 +175,11 @@ public Boolean isEdgeRouter() { @Test public void testAddSlice() { - // Preparation + // Default slice is automatically pre-provisioned. Set expectedSliceIds = new HashSet<>(); expectedSliceIds.add(SLICE_IDS.get(0)); - // Normal + // Test adding new slice. expectedSliceIds.add(SLICE_IDS.get(1)); assertTrue(manager.addSlice(SLICE_IDS.get(1))); assertEquals(expectedSliceIds, manager.getSlices()); @@ -195,115 +188,124 @@ public void testAddSlice() { assertTrue(manager.addSlice(SLICE_IDS.get(2))); assertEquals(expectedSliceIds, manager.getSlices()); + Set expectedTrafficClasses = new HashSet<>(); + expectedTrafficClasses.add(TrafficClass.BEST_EFFORT); + assertEquals(expectedTrafficClasses, manager.getTrafficClasses(SLICE_IDS.get(0))); + assertEquals(expectedTrafficClasses, manager.getTrafficClasses(SLICE_IDS.get(1))); + assertEquals(expectedTrafficClasses, manager.getTrafficClasses(SLICE_IDS.get(2))); + assertEquals(TrafficClass.BEST_EFFORT, manager.getDefaultTrafficClass(SLICE_IDS.get(0))); assertEquals(TrafficClass.BEST_EFFORT, manager.getDefaultTrafficClass(SLICE_IDS.get(1))); assertEquals(TrafficClass.BEST_EFFORT, manager.getDefaultTrafficClass(SLICE_IDS.get(2))); } @Test - public void testAddSliceException1() { + public void testAddSliceExceptionDefaultSlice() { exceptionRule.expect(SlicingException.class); - exceptionRule.expectMessage("Adding default slice is not allowed"); + exceptionRule.expectMessage("Adding the default slice is not allowed"); manager.addSlice(SLICE_IDS.get(0)); } @Test - public void testAddSliceException2() { - // Preparation + public void testAddSliceExceptionAlreadyExists() { manager.addSlice(SLICE_IDS.get(1)); exceptionRule.expect(SlicingException.class); - exceptionRule.expectMessage("TC BEST_EFFORT is already allocated for slice 1"); + exceptionRule.expectMessage("Slice 1 already exists"); manager.addSlice(SLICE_IDS.get(1)); } @Test public void testRemoveSlice() { - // Preparation Set expectedSliceIds = new HashSet<>(); expectedSliceIds.add(SLICE_IDS.get(0)); expectedSliceIds.add(SLICE_IDS.get(1)); expectedSliceIds.add(SLICE_IDS.get(2)); manager.addSlice(SLICE_IDS.get(1)); manager.addSlice(SLICE_IDS.get(2)); - manager.addTrafficClass(SLICE_IDS.get(2), TrafficClass.CONTROL); - // Normal expectedSliceIds.remove(SLICE_IDS.get(1)); assertTrue(manager.removeSlice(SLICE_IDS.get(1))); assertEquals(expectedSliceIds, manager.getSlices()); + assertEquals(1, manager.getTrafficClasses(SLICE_IDS.get(0)).size()); + assertNotNull(manager.getDefaultTrafficClass(SLICE_IDS.get(0))); + assertEquals(0, manager.getTrafficClasses(SLICE_IDS.get(1)).size()); + assertNull(manager.getDefaultTrafficClass(SLICE_IDS.get(1))); + assertEquals(1, manager.getTrafficClasses(SLICE_IDS.get(2)).size()); + assertNotNull(manager.getDefaultTrafficClass(SLICE_IDS.get(2))); + expectedSliceIds.remove(SLICE_IDS.get(2)); assertTrue(manager.removeSlice(SLICE_IDS.get(2))); assertEquals(expectedSliceIds, manager.getSlices()); + + assertEquals(0, manager.getTrafficClasses(SLICE_IDS.get(2)).size()); + assertNull(manager.getDefaultTrafficClass(SLICE_IDS.get(2))); } @Test - public void testRemoveSliceException1() { + public void testRemoveSliceExceptionDefault() { exceptionRule.expect(SlicingException.class); - exceptionRule.expectMessage("Removing default slice is not allowed"); + exceptionRule.expectMessage("Removing the default slice is not allowed"); manager.removeSlice(SLICE_IDS.get(0)); } @Test - public void testRemoveSliceException2() { + public void testRemoveSliceExceptionNonExistent() { exceptionRule.expect(SlicingException.class); - exceptionRule.expectMessage("Cannot remove a non-existent slice 1"); + exceptionRule.expectMessage("Cannot remove non-existent slice 1"); manager.removeSlice(SLICE_IDS.get(1)); } @Test public void testAddTrafficClass() { - // Preparation Set expectedTcs = new HashSet<>(); expectedTcs.add(TrafficClass.BEST_EFFORT); manager.addSlice(SLICE_IDS.get(1)); // Normal expectedTcs.add(TrafficClass.CONTROL); - assertTrue(manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL)); + assertTrue(manager.addTrafficClass(SLICE_IDS.get(1), TC_CONFIG_CONTROL)); assertEquals(expectedTcs, manager.getTrafficClasses(SLICE_IDS.get(1))); expectedTcs.add(TrafficClass.REAL_TIME); - assertTrue(manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.REAL_TIME)); + assertTrue(manager.addTrafficClass(SLICE_IDS.get(1), TC_CONFIG_REAL_TIME)); assertEquals(expectedTcs, manager.getTrafficClasses(SLICE_IDS.get(1))); expectedTcs.add(TrafficClass.ELASTIC); - assertTrue(manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.ELASTIC)); + assertTrue(manager.addTrafficClass(SLICE_IDS.get(1), TC_CONFIG_ELASTIC)); assertEquals(expectedTcs, manager.getTrafficClasses(SLICE_IDS.get(1))); + } - // Add BE to non-existent slice is equivalent to add slice - expectedTcs = new HashSet<>(); - expectedTcs.add(TrafficClass.BEST_EFFORT); - assertTrue(manager.addTrafficClass(SLICE_IDS.get(2), TrafficClass.BEST_EFFORT)); - assertEquals(expectedTcs, manager.getTrafficClasses(SLICE_IDS.get(2))); + @Test + public void testAddTrafficClassExceptionNonExistentSlice() { + exceptionRule.expect(SlicingException.class); + exceptionRule.expectMessage("Cannot add traffic class to non-existent slice 1"); + manager.addTrafficClass(SLICE_IDS.get(1), TrafficClassConfig.BEST_EFFORT); } @Test - public void testAddTrafficClassException() { - // Preparation + public void testAddTrafficClassExceptionAlreadyExists() { manager.addSlice(SLICE_IDS.get(1)); - manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL); + manager.addTrafficClass(SLICE_IDS.get(1), TC_CONFIG_CONTROL); exceptionRule.expect(SlicingException.class); exceptionRule.expectMessage("TC CONTROL is already allocated for slice 1"); - manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL); + manager.addTrafficClass(SLICE_IDS.get(1), TC_CONFIG_CONTROL); } @Test public void testRemoveTrafficClass() { - // Preparation Set expectedTcs = new HashSet<>(); expectedTcs.add(TrafficClass.BEST_EFFORT); expectedTcs.add(TrafficClass.CONTROL); expectedTcs.add(TrafficClass.REAL_TIME); expectedTcs.add(TrafficClass.ELASTIC); manager.addSlice(SLICE_IDS.get(1)); - manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL); - manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.REAL_TIME); - manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.ELASTIC); + manager.addTrafficClass(SLICE_IDS.get(1), TC_CONFIG_CONTROL); + manager.addTrafficClass(SLICE_IDS.get(1), TC_CONFIG_REAL_TIME); + manager.addTrafficClass(SLICE_IDS.get(1), TC_CONFIG_ELASTIC); - // Normal expectedTcs.remove(TrafficClass.CONTROL); assertTrue(manager.removeTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL)); assertEquals(expectedTcs, manager.getTrafficClasses(SLICE_IDS.get(1))); @@ -318,220 +320,142 @@ public void testRemoveTrafficClass() { } @Test - public void testRemoveTrafficClassException1() { - // Preparation - manager.addSlice(SLICE_IDS.get(1)); - manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL); - + public void testRemoveTrafficClassExceptionNonExistentSlice() { exceptionRule.expect(SlicingException.class); - exceptionRule.expectMessage("Can't remove BEST_EFFORT from slice 1 while it is being used as Default TC"); + exceptionRule.expectMessage("Cannot remove a traffic class from non-existent slice 1"); manager.removeTrafficClass(SLICE_IDS.get(1), TrafficClass.BEST_EFFORT); } @Test - public void testRemoveTrafficClassException2() { - // Preparation + public void testRemoveTrafficClassExceptionBestEffort() { manager.addSlice(SLICE_IDS.get(1)); + manager.addTrafficClass(SLICE_IDS.get(1), TC_CONFIG_CONTROL); exceptionRule.expect(SlicingException.class); - exceptionRule.expectMessage("TC CONTROL has not been allocated to slice 1"); - manager.removeTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL); + exceptionRule.expectMessage("Cannot remove BEST_EFFORT traffic class from any slice"); + manager.removeTrafficClass(SLICE_IDS.get(1), TrafficClass.BEST_EFFORT); } @Test - public void testRemoveTrafficClassException3() { + public void testRemoveTrafficClassExceptionNotAllocated() { + manager.addSlice(SLICE_IDS.get(1)); + exceptionRule.expect(SlicingException.class); - exceptionRule.expectMessage("Can't remove BEST_EFFORT from slice 0 while it is being used as Default TC"); - manager.removeTrafficClass(SLICE_IDS.get(0), TrafficClass.BEST_EFFORT); + exceptionRule.expectMessage("Traffic class CONTROL has not been allocated for slice 1"); + manager.removeTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL); } @Test public void testSetDefaultTrafficClass() { - // Preparation manager.addSlice(SLICE_IDS.get(1)); assertEquals(TrafficClass.BEST_EFFORT, manager.getDefaultTrafficClass(SLICE_IDS.get(1))); - manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL); + manager.addTrafficClass(SLICE_IDS.get(1), TC_CONFIG_CONTROL); manager.setDefaultTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL); assertEquals(TrafficClass.CONTROL, manager.getDefaultTrafficClass(SLICE_IDS.get(1))); } @Test - public void testSetDefaultTrafficClassException() { - // Preparation + public void testSetDefaultTrafficClassExceptionNotAllocated() { manager.addSlice(SLICE_IDS.get(1)); exceptionRule.expect(SlicingException.class); - exceptionRule.expectMessage("Can't set CONTROL as default TC because it has not been allocated to slice 1"); + exceptionRule.expectMessage("Cannot set CONTROL as the default traffic class because it has not been allocated to slice 1"); manager.setDefaultTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL); } @Test - public void testRemoveDefaultTrafficClassException() { - // Preparation - manager.addSlice(SLICE_IDS.get(1)); - manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL); - manager.setDefaultTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL); + public void testRemoveTrafficClassExceptionDefaultTc() { + manager.addTrafficClass(SLICE_IDS.get(0), TC_CONFIG_CONTROL); + manager.setDefaultTrafficClass(SLICE_IDS.get(0), TrafficClass.CONTROL); exceptionRule.expect(SlicingException.class); - exceptionRule.expectMessage("Can't remove CONTROL from slice 1 while it is being used as Default TC"); - manager.removeTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL); + exceptionRule.expectMessage("Cannot remove CONTROL from slice 0 while it is " + + "being used as the default traffic class"); + manager.removeTrafficClass(SLICE_IDS.get(0), TrafficClass.CONTROL); } @Test - public void testAddFlowClassifier() { - // Preparation - TrafficSelector selector = DefaultTrafficSelector.builder().matchUdpDst(TpPort.tpPort(100)).build(); + public void testAddClassifierFlow() { manager.addSlice(SLICE_IDS.get(1)); - manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.REAL_TIME); + manager.addTrafficClass(SLICE_IDS.get(1), TC_CONFIG_REAL_TIME); - // Normal + TrafficSelector selector = DefaultTrafficSelector.builder().matchUdpDst(TpPort.tpPort(100)).build(); manager.addClassifierFlow(selector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); assertEquals(1, manager.getClassifierFlows(SLICE_IDS.get(1), TrafficClass.REAL_TIME).size()); assertTrue(manager.getClassifierFlows(SLICE_IDS.get(1), TrafficClass.REAL_TIME).contains(selector)); } @Test - public void testAddFlowClassifierException1() { + public void testAddClassifierFlowExceptionEmptySelector() { exceptionRule.expect(SlicingException.class); exceptionRule.expectMessage("Empty traffic selector is not allowed"); manager.addClassifierFlow(DefaultTrafficSelector.builder().build(), - SLICE_IDS.get(1), TrafficClass.REAL_TIME); + SLICE_IDS.get(1), TrafficClass.REAL_TIME); } @Test - public void testAddFlowClassifierException2() { - // Preparation + public void testAddClassifierFlowExceptionWrongSelector() { TrafficSelector wrongSelector = DefaultTrafficSelector.builder().matchEthDst(MacAddress.IPV4_MULTICAST).build(); - exceptionRule.expect(SlicingException.class); - exceptionRule.expectMessage("Only accept 5-tuple DefaultTrafficSelector{criteria=[ETH_DST:01:00:5E:00:00:00]}"); + exceptionRule.expectMessage("Selector can only express a match on the L3-L4 5-tuple fields"); manager.addClassifierFlow(wrongSelector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); } @Test - public void testRemoveFlowClassifier() { - // Preparation + public void testRemoveClassifierFlow() { TrafficSelector selector = DefaultTrafficSelector.builder().matchUdpDst(TpPort.tpPort(100)).build(); manager.addSlice(SLICE_IDS.get(1)); - manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.REAL_TIME); + manager.addTrafficClass(SLICE_IDS.get(1), TC_CONFIG_REAL_TIME); manager.addClassifierFlow(selector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); - // Normal assertEquals(1, manager.getClassifierFlows(SLICE_IDS.get(1), TrafficClass.REAL_TIME).size()); manager.removeClassifierFlow(selector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); assertEquals(0, manager.getClassifierFlows(SLICE_IDS.get(1), TrafficClass.REAL_TIME).size()); } @Test - public void testRemoveFlowClassifierException() { + public void testRemoveClassifierFlowException() { // Preparation TrafficSelector wrongSelector = DefaultTrafficSelector.builder().matchTcpDst(TpPort.tpPort(100)).build(); exceptionRule.expect(SlicingException.class); exceptionRule.expectMessage("There is no such Flow Classifier Rule " + - "DefaultTrafficSelector{criteria=[TCP_DST:100]} for slice 1 and TC REAL_TIME"); + "DefaultTrafficSelector{criteria=[TCP_DST:100]} for slice 1 and TC REAL_TIME"); manager.removeClassifierFlow(wrongSelector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); } @Test - public void testRemoveSliceAndTcWithFlowClassifierException1() { - // Preparation + public void testRemoveTcWithClassifierFlowException() { TrafficSelector selector = DefaultTrafficSelector.builder().matchUdpDst(TpPort.tpPort(100)).build(); manager.addSlice(SLICE_IDS.get(1)); - manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.REAL_TIME); + manager.addTrafficClass(SLICE_IDS.get(1), TC_CONFIG_REAL_TIME); manager.addClassifierFlow(selector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); - // Fail to remove Slice and TC when Flow Classifier exceptionRule.expect(SlicingException.class); - exceptionRule.expectMessage("Cannot remove REAL_TIME from slice 1 with 1 Flow Classifier Rules"); + exceptionRule.expectMessage("Cannot remove REAL_TIME from slice 1 with 1 classifier flow rules"); assertFalse(manager.removeTrafficClass(SLICE_IDS.get(1), TrafficClass.REAL_TIME)); } @Test - public void testRemoveSliceAndTcWithFlowClassifierException2() { - // Preparation + public void testRemoveSliceWithClassifierFlowException() { TrafficSelector selector = DefaultTrafficSelector.builder().matchUdpDst(TpPort.tpPort(100)).build(); manager.addSlice(SLICE_IDS.get(1)); - manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.REAL_TIME); + manager.addTrafficClass(SLICE_IDS.get(1), TC_CONFIG_REAL_TIME); manager.addClassifierFlow(selector, SLICE_IDS.get(1), TrafficClass.REAL_TIME); - // Fail to remove Slice and TC when Flow Classifier exceptionRule.expect(SlicingException.class); - exceptionRule.expectMessage("Cannot remove slice 1 with 1 Flow Classifier Rules"); + exceptionRule.expectMessage("Cannot remove slice 1 with 1 classifier flow rules"); assertFalse(manager.removeSlice(SLICE_IDS.get(1))); } - @Test - public void testQueue() { - // Preparation - manager.queueStore.put(QueueId.of(4), new QueueStoreValue(TrafficClass.REAL_TIME, true)); - manager.queueStore.put(QueueId.of(7), new QueueStoreValue(TrafficClass.ELASTIC, true)); - SliceStoreKey key; - for (int i = 1; i <= 4; i++) { - manager.addSlice(SLICE_IDS.get(i)); - manager.addTrafficClass(SLICE_IDS.get(i), TrafficClass.CONTROL); - // The following actions may throw a slicing exception - // indicate that there is no available queue - // We skip the exception here because we are not interested - // in it (We are testing the assigned queue id). - try { - manager.addTrafficClass(SLICE_IDS.get(i), TrafficClass.REAL_TIME); - } catch (SlicingException e) { } - try { - manager.addTrafficClass(SLICE_IDS.get(i), TrafficClass.ELASTIC); - } catch (SlicingException e) { } - } - - // All BE should point to same queue - for (int i = 0; i <= 4; i++) { - key = new SliceStoreKey(SLICE_IDS.get(i), TrafficClass.BEST_EFFORT); - assertEquals(QueueId.BEST_EFFORT, manager.getSliceStore().get(key)); - } - - // All Control should point to same queue - for (int i = 1; i <= 4; i++) { - key = new SliceStoreKey(SLICE_IDS.get(i), TrafficClass.CONTROL); - assertEquals(QueueId.CONTROL, manager.getSliceStore().get(key)); - } - - // No slice should point to SYSTEM queue - assertFalse(manager.getSliceStore().entrySet().stream().anyMatch(e -> e.getValue().equals(QueueId.SYSTEM))); - - // Each REAL TIME class should point to single queue - // or point to nothing if there is no available queue - key = new SliceStoreKey(SLICE_IDS.get(1), TrafficClass.REAL_TIME); - assertEquals(QueueId.of(3), manager.getSliceStore().get(key)); - key = new SliceStoreKey(SLICE_IDS.get(2), TrafficClass.REAL_TIME); - assertEquals(QueueId.of(4), manager.getSliceStore().get(key)); - key = new SliceStoreKey(SLICE_IDS.get(4), TrafficClass.REAL_TIME); - assertEquals(null, manager.getSliceStore().get(key)); - - // Each ELASTIC class should point to single queue - // or point to nothing if there is no available queue - key = new SliceStoreKey(SLICE_IDS.get(1), TrafficClass.ELASTIC); - assertEquals(QueueId.of(6), manager.getSliceStore().get(key)); - key = new SliceStoreKey(SLICE_IDS.get(2), TrafficClass.ELASTIC); - assertEquals(QueueId.of(7), manager.getSliceStore().get(key)); - key = new SliceStoreKey(SLICE_IDS.get(3), TrafficClass.ELASTIC); - assertEquals(null, manager.getSliceStore().get(key)); - - // After removing a REAL TIME or ELASTIC class, a queue should be released - // and ready to be allocated - manager.removeTrafficClass(SLICE_IDS.get(2), TrafficClass.ELASTIC); - manager.addTrafficClass(SLICE_IDS.get(3), TrafficClass.ELASTIC); - key = new SliceStoreKey(SLICE_IDS.get(3), TrafficClass.ELASTIC); - assertEquals(QueueId.of(7), manager.getSliceStore().get(key)); - } @Test public void testSliceListener() { - FlowRule slice1BE1 = buildSlice1BE1(); - FlowRule defaulTc1BE = buildDefaultTc1Be(); - FlowRule slice1Control1 = buildSlice1Control1(DID); - FlowRule slice1Control2 = buildSlice1Control2(DID, false); - FlowRule bmv2Slice1Control1 = buildSlice1Control1(DID_BMV2); - FlowRule bmv2Slice1Control2 = buildSlice1Control2(DID_BMV2, true); + FlowRule queuesFlowRuleSlice1BestEffort = buildQueuesFlowRuleSlice1BestEffort(); + FlowRule defaultTcFlowRuleSlice1BestEffort = buildDefaultTcFlowRuleSlice1BestEffort(); + FlowRule queuesFlowRuleSlice1ControlGreen = buildQueuesFlowRuleSlice1ControlGreen(); + FlowRule queuesFlowRuleSlice1ControlRed = buildQueuesFlowRuleSlice1ControlRed(); int numDevices = DEVICES.size(); flowsPreCheck(); @@ -541,24 +465,20 @@ public void testSliceListener() { assertAfter(50, () -> { assertEquals(3 * numDevices, capturedAddedFlowRules.getValues().size()); assertEquals(2, capturedAddedFlowRules.getValues().stream() - .filter(flowRule -> flowRule.exactMatch(slice1BE1)).count()); + .filter(flowRule -> flowRule.exactMatch(queuesFlowRuleSlice1BestEffort)).count()); assertEquals(1, capturedAddedFlowRules.getValues().stream() - .filter(flowRule -> flowRule.exactMatch(defaulTc1BE)).count()); + .filter(flowRule -> flowRule.exactMatch(defaultTcFlowRuleSlice1BestEffort)).count()); }); // Adding Control class to slice 1 capturedAddedFlowRules.reset(); - manager.addTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL); + manager.addTrafficClass(SLICE_IDS.get(1), TC_CONFIG_CONTROL); assertAfter(50, () -> { assertEquals(2 * numDevices, capturedAddedFlowRules.getValues().size()); assertTrue(capturedAddedFlowRules.getValues() - .stream().anyMatch(f -> f.exactMatch(slice1Control1))); - assertTrue(capturedAddedFlowRules.getValues() - .stream().anyMatch(f -> f.exactMatch(slice1Control2))); + .stream().anyMatch(f -> f.exactMatch(queuesFlowRuleSlice1ControlGreen))); assertTrue(capturedAddedFlowRules.getValues() - .stream().anyMatch(f -> f.exactMatch(bmv2Slice1Control1))); - assertTrue(capturedAddedFlowRules.getValues() - .stream().anyMatch(f -> f.exactMatch(bmv2Slice1Control2))); + .stream().anyMatch(f -> f.exactMatch(queuesFlowRuleSlice1ControlRed))); }); // Removing Control class from slice 1 @@ -567,60 +487,55 @@ public void testSliceListener() { assertAfter(50, () -> { assertEquals(2 * numDevices, capturedRemovedFlowRules.getValues().size()); assertTrue(capturedRemovedFlowRules.getValues() - .stream().anyMatch(f -> f.exactMatch(slice1Control1))); - assertTrue(capturedRemovedFlowRules.getValues() - .stream().anyMatch(f -> f.exactMatch(slice1Control2))); - assertTrue(capturedRemovedFlowRules.getValues() - .stream().anyMatch(f -> f.exactMatch(bmv2Slice1Control1))); + .stream().anyMatch(f -> f.exactMatch(queuesFlowRuleSlice1ControlGreen))); assertTrue(capturedRemovedFlowRules.getValues() - .stream().anyMatch(f -> f.exactMatch(bmv2Slice1Control2))); + .stream().anyMatch(f -> f.exactMatch(queuesFlowRuleSlice1ControlRed))); }); } @Test - public void testFlowListener() { - FlowRule mock = build5Tuple(); + public void testClassifierFlowListener() { TrafficSelector selector = DefaultTrafficSelector.builder() - .matchIPSrc(IpPrefix.valueOf("10.20.30.1/32")) - .matchIPDst(IpPrefix.valueOf("10.20.30.2/32")) - .matchIPProtocol((byte) 0x06) - .matchTcpSrc(TpPort.tpPort(80)) - .matchTcpDst(TpPort.tpPort(1234)) - .build(); + .matchIPSrc(IpPrefix.valueOf("10.20.30.1/32")) + .matchIPDst(IpPrefix.valueOf("10.20.30.2/32")) + .matchIPProtocol((byte) 0x06) + .matchTcpSrc(TpPort.tpPort(80)) + .matchTcpDst(TpPort.tpPort(1234)) + .build(); + FlowRule expectedFlowRule = buildClassifierFlowRule(SLICE_IDS.get(1), TrafficClass.BEST_EFFORT, selector); int numDevices = DEVICES.size(); flowsPreCheck(); - // Adding mock rule to slice 1 BE + // Adding flow to slice 1 capturedAddedFlowRules.reset(); manager.addClassifierFlow(selector, SLICE_IDS.get(1), TrafficClass.BEST_EFFORT); assertAfter(50, () -> { - assertEquals(1 * numDevices, capturedAddedFlowRules.getValues().size()); + assertEquals(numDevices, capturedAddedFlowRules.getValues().size()); assertTrue(capturedAddedFlowRules.getValues() - .stream().anyMatch(f -> f.exactMatch(mock))); + .stream().anyMatch(f -> f.exactMatch(expectedFlowRule))); }); - // Removing mock rule from slice 1 BE + // Removing flow from slice 1 capturedRemovedFlowRules.reset(); manager.removeClassifierFlow(selector, SLICE_IDS.get(1), TrafficClass.BEST_EFFORT); assertAfter(50, () -> { - assertEquals(1 * numDevices, capturedRemovedFlowRules.getValues().size()); + assertEquals(numDevices, capturedRemovedFlowRules.getValues().size()); assertTrue(capturedRemovedFlowRules.getValues() - .stream().anyMatch(f -> f.exactMatch(mock))); + .stream().anyMatch(f -> f.exactMatch(expectedFlowRule))); }); } - private FlowRule buildSlice1BE1() { - // Hard coded parameters + private FlowRule buildQueuesFlowRuleSlice1BestEffort() { PiCriterion.Builder piCriterionBuilder = PiCriterion.builder() .matchExact(P4InfoConstants.HDR_SLICE_TC, - sliceTcConcat(SLICE_IDS.get(1).id(), TrafficClass.BEST_EFFORT.ordinal())); + sliceTcConcat(SLICE_IDS.get(1).id(), TrafficClass.BEST_EFFORT.toInt())); PiAction.Builder piTableActionBuilder = PiAction.builder() .withId(P4InfoConstants.FABRIC_INGRESS_QOS_SET_QUEUE) .withParameter(new PiActionParam(P4InfoConstants.QID, QueueId.BEST_EFFORT.id())); - FlowRule flowRule = DefaultFlowRule.builder() - .forDevice(DID) + return DefaultFlowRule.builder() + .forDevice(DEVICE_ID) .forTable(PiTableId.of(FABRIC_INGRESS_QOS_QUEUES.id())) .fromApp(APP_ID) .withPriority(QOS_FLOW_PRIORITY) @@ -628,12 +543,9 @@ private FlowRule buildSlice1BE1() { .withTreatment(DefaultTrafficTreatment.builder().piTableAction(piTableActionBuilder.build()).build()) .makePermanent() .build(); - - return flowRule; } - private FlowRule buildDefaultTc1Be() { - // Hard coded parameters + private FlowRule buildDefaultTcFlowRuleSlice1BestEffort() { PiCriterion piCriterion = PiCriterion.builder() .matchTernary(P4InfoConstants.HDR_SLICE_TC, sliceTcConcat(SLICE_IDS.get(1).id(), 0), 0x3C) .matchExact(P4InfoConstants.HDR_TC_UNKNOWN, 1) @@ -642,8 +554,9 @@ private FlowRule buildDefaultTc1Be() { .withId(P4InfoConstants.FABRIC_INGRESS_QOS_SET_DEFAULT_TC) .withParameter(new PiActionParam(P4InfoConstants.TC, QueueId.BEST_EFFORT.id())) .build(); - FlowRule flowRule = DefaultFlowRule.builder() - .forDevice(DID) + + return DefaultFlowRule.builder() + .forDevice(DEVICE_ID) .forTable(FABRIC_INGRESS_QOS_DEFAULT_TC) .fromApp(APP_ID) .withPriority(DEFAULT_TC_PRIORITY) @@ -651,25 +564,20 @@ private FlowRule buildDefaultTc1Be() { .withTreatment(DefaultTrafficTreatment.builder().piTableAction(piTableAction).build()) .makePermanent() .build(); - - return flowRule; } - private FlowRule buildSlice1Control1(DeviceId deviceId) { - // Hard coded parameters - int colorGreen = getCapabilities(deviceId).getMeterColor(Color.GREEN); - + private FlowRule buildQueuesFlowRuleSlice1ControlGreen() { PiCriterion.Builder piCriterionBuilder = PiCriterion.builder() .matchExact(P4InfoConstants.HDR_SLICE_TC, - sliceTcConcat(SLICE_IDS.get(1).id(), TrafficClass.CONTROL.ordinal())) - .matchTernary(HDR_COLOR, colorGreen, 1 << HDR_COLOR_BITWIDTH - 1); + sliceTcConcat(SLICE_IDS.get(1).id(), TrafficClass.CONTROL.toInt())) + .matchTernary(HDR_COLOR, COLOR_GREEN, 1 << HDR_COLOR_BITWIDTH - 1); PiAction.Builder piTableActionBuilder = PiAction.builder() .withId(P4InfoConstants.FABRIC_INGRESS_QOS_SET_QUEUE) - .withParameter(new PiActionParam(P4InfoConstants.QID, QueueId.CONTROL.id())); + .withParameter(new PiActionParam(P4InfoConstants.QID, QUEUE_ID_CONTROL.id())); - FlowRule flowRule = DefaultFlowRule.builder() - .forDevice(deviceId) + return DefaultFlowRule.builder() + .forDevice(SlicingManagerTest.DEVICE_ID) .forTable(PiTableId.of(FABRIC_INGRESS_QOS_QUEUES.id())) .fromApp(APP_ID) .withPriority(QOS_FLOW_PRIORITY) @@ -677,26 +585,20 @@ private FlowRule buildSlice1Control1(DeviceId deviceId) { .withTreatment(DefaultTrafficTreatment.builder().piTableAction(piTableActionBuilder.build()).build()) .makePermanent() .build(); - - return flowRule; } - private FlowRule buildSlice1Control2(DeviceId deviceId, boolean isBmv2) { - // Hard coded parameters - - int colorRed = getCapabilities(deviceId).getMeterColor(Color.RED); - + private FlowRule buildQueuesFlowRuleSlice1ControlRed() { PiCriterion.Builder piCriterionBuilder = PiCriterion.builder() .matchExact(P4InfoConstants.HDR_SLICE_TC, - sliceTcConcat(SLICE_IDS.get(1).id(), TrafficClass.CONTROL.ordinal())) - .matchTernary(HDR_COLOR, colorRed, 1 << HDR_COLOR_BITWIDTH - 1); + sliceTcConcat(SLICE_IDS.get(1).id(), TrafficClass.CONTROL.toInt())) + .matchTernary(HDR_COLOR, COLOR_RED, 1 << HDR_COLOR_BITWIDTH - 1); PiAction.Builder piTableActionBuilder = PiAction.builder() .withId(P4InfoConstants.FABRIC_INGRESS_QOS_SET_QUEUE) .withParameter(new PiActionParam(P4InfoConstants.QID, QueueId.BEST_EFFORT.id())); - FlowRule flowRule = DefaultFlowRule.builder() - .forDevice(deviceId) + return DefaultFlowRule.builder() + .forDevice(SlicingManagerTest.DEVICE_ID) .forTable(PiTableId.of(FABRIC_INGRESS_QOS_QUEUES.id())) .fromApp(APP_ID) .withPriority(QOS_FLOW_PRIORITY) @@ -704,31 +606,16 @@ private FlowRule buildSlice1Control2(DeviceId deviceId, boolean isBmv2) { .withTreatment(DefaultTrafficTreatment.builder().piTableAction(piTableActionBuilder.build()).build()) .makePermanent() .build(); - - return flowRule; - } - - private FlowRule build5Tuple() { - // Hard coded parameters - TrafficSelector selector = DefaultTrafficSelector.builder() - .matchIPSrc(IpPrefix.valueOf("10.20.30.1/32")) - .matchIPDst(IpPrefix.valueOf("10.20.30.2/32")) - .matchIPProtocol((byte) 0x06) - .matchTcpSrc(TpPort.tpPort(80)) - .matchTcpDst(TpPort.tpPort(1234)) - .build(); - - return buildClassifierFromSelector(SLICE_IDS.get(1), TrafficClass.BEST_EFFORT, selector); } - private FlowRule buildClassifierFromSelector(SliceId sliceId, TrafficClass tc, TrafficSelector selector) { + private FlowRule buildClassifierFlowRule(SliceId sliceId, TrafficClass tc, TrafficSelector selector) { PiAction.Builder piTableActionBuilder = PiAction.builder() .withId(P4InfoConstants.FABRIC_INGRESS_SLICE_TC_CLASSIFIER_SET_SLICE_ID_TC) .withParameters(Set.of(new PiActionParam(P4InfoConstants.SLICE_ID, sliceId.id()), - new PiActionParam(P4InfoConstants.TC, tc.ordinal()))); + new PiActionParam(P4InfoConstants.TC, tc.toInt()))); return DefaultFlowRule.builder() - .forDevice(DID) + .forDevice(DEVICE_ID) .forTable(P4InfoConstants.FABRIC_INGRESS_SLICE_TC_CLASSIFIER_CLASSIFIER) .fromApp(APP_ID) .withPriority(CLASSIFIER_FLOW_PRIORITY) @@ -739,10 +626,10 @@ private FlowRule buildClassifierFromSelector(SliceId sliceId, TrafficClass tc, T } private void flowsPreCheck() { - // We install init flows during start up stage - // So we do a quick check here as a precondition - // e.g. sliceExecutor installs slice flows on activation stage, if the tests start immediately, - // the captured flows may include slice flows (unexpceted). + // We install init flows during start up stage So we do a quick check + // here as a precondition e.g. sliceExecutor installs slice flows on + // activation stage, if the tests start immediately, the captured flows + // may include slice flows (unexpceted). assertAfter(100, 250, () -> { // Number of init flows are variant // For now we install 2 flows for each device From b5a2d1d331ec5fe723b8ca61fefe7e687ba394c9 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Thu, 13 Jan 2022 10:13:02 -0800 Subject: [PATCH 13/33] Remove leftover QueueStoreValue --- .../fabric/tna/slicing/QueueStoreValue.java | 65 ------------------- .../fabric/tna/slicing/SlicingManager.java | 3 +- .../tna/slicing/api/SlicingAdminService.java | 1 - 3 files changed, 1 insertion(+), 68 deletions(-) delete mode 100644 src/main/java/org/stratumproject/fabric/tna/slicing/QueueStoreValue.java diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/QueueStoreValue.java b/src/main/java/org/stratumproject/fabric/tna/slicing/QueueStoreValue.java deleted file mode 100644 index 802dbeb4b..000000000 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/QueueStoreValue.java +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright 2021-present Open Networking Foundation -// SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 -package org.stratumproject.fabric.tna.slicing; - -import com.google.common.base.MoreObjects; -import org.stratumproject.fabric.tna.slicing.api.TrafficClass; - -import java.util.Objects; - -/** - * Value of queue store. - */ -public class QueueStoreValue { - private TrafficClass tc; - private boolean available; - - public QueueStoreValue(TrafficClass tc, boolean available) { - this.tc = tc; - this.available = available; - } - - public TrafficClass trafficClass() { - return this.tc; - } - - public QueueStoreValue setTrafficClass(TrafficClass tc) { - this.tc = tc; - return this; - } - - public boolean available() { - return this.available; - } - - public QueueStoreValue setAvailable(boolean available) { - this.available = available; - return this; - } - - @Override - public boolean equals(final Object obj) { - if (this == obj) { - return true; - } - if (!(obj instanceof QueueStoreValue)) { - return false; - } - final QueueStoreValue other = (QueueStoreValue) obj; - return this.tc == other.tc && - this.available == other.available; - } - - @Override - public int hashCode() { - return Objects.hash(tc, available); - } - - @Override - public String toString() { - return MoreObjects.toStringHelper(getClass()) - .add("trafficClass", tc) - .add("available", available) - .toString(); - } -} diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java index d5ddc52a4..77353a87f 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java @@ -148,8 +148,7 @@ protected void activate() { .register(TrafficClass.class) .register(TrafficClassConfig.class) .register(QueueId.class) - .register(SliceStoreKey.class) - .register(QueueStoreValue.class); + .register(SliceStoreKey.class); sliceStore = storageService.consistentMapBuilder() .withName("fabric-tna-slice") diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingAdminService.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingAdminService.java index 4db0732aa..4ded7fef1 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingAdminService.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingAdminService.java @@ -2,7 +2,6 @@ // SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 package org.stratumproject.fabric.tna.slicing.api; -import org.stratumproject.fabric.tna.slicing.QueueStoreValue; import org.stratumproject.fabric.tna.slicing.SliceStoreKey; import java.util.Map; From e3be92f82d34f2f70a0c2c20ea95c08127e483a5 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Thu, 13 Jan 2022 10:13:13 -0800 Subject: [PATCH 14/33] Javadoc --- .../tna/slicing/api/TrafficClassConfig.java | 54 +++++++++++++++++-- 1 file changed, 50 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java index 25175295c..4dee11b97 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java @@ -10,9 +10,8 @@ /** * Configuration of a traffic class. */ -// TODO: complete javadocs public class TrafficClassConfig { - private static final int UNLIMITED_MAX_RATE = -1; + private static final int UNLIMITED_MAX_RATE = Integer.MAX_VALUE; public static final TrafficClassConfig BEST_EFFORT = new TrafficClassConfig( TrafficClass.BEST_EFFORT, QueueId.BEST_EFFORT, UNLIMITED_MAX_RATE, 0); @@ -22,6 +21,14 @@ public class TrafficClassConfig { private final int maxRateBps; private final int gminRateBps; + /** + * Creates a new traffic class config. + * + * @param tc traffic class enum value + * @param qid queue ID + * @param maxRateBps maximum bitrate in bps + * @param gminRateBps guaranteed minimum rate in bps + */ public TrafficClassConfig(TrafficClass tc, QueueId qid, int maxRateBps, int gminRateBps) { this.tc = tc; this.qid = qid; @@ -29,26 +36,65 @@ public TrafficClassConfig(TrafficClass tc, QueueId qid, int maxRateBps, int gmin this.gminRateBps = gminRateBps; } + /** + * Returns the traffic class. + * + * @return traffic class + */ public TrafficClass trafficClass() { return tc; } + /** + * Returns the queue ID associated to this traffic class. + * + * @return queue ID + */ public QueueId queueId() { return qid; } + /** + * Returns the maximum bitrate in bps. Sources sending at rates higher than + * this might observe their traffic being shaped or policed. Meaningful only + * if {@link #isMaxRateUnlimited()} is false. + * + * @return maximum bitrate in bps + */ public int getMaxRateBps() { return maxRateBps; } + /** + * Returns true if this traffic class is not rate limited, false otherwise. + * + * @return true if rate is unlimited + */ + public boolean isMaxRateUnlimited() { + return maxRateBps == UNLIMITED_MAX_RATE; + } + + /** + * Returns the guaranteed minimum bitrate in bps. Sources sending at rates + * lower than this value can expect their traffic to be serviced without + * limitations (drops or delay) at all times, even during congestion. {@code + * getGminRateBps() == 0} signifies that data plane devices will not provide + * any bandwidth guarantees. + * + * @return guaranteed minimum bitrate in bps + */ public int getGminRateBps() { return gminRateBps; } @Override public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } TrafficClassConfig that = (TrafficClassConfig) o; return maxRateBps == that.maxRateBps && gminRateBps == that.gminRateBps && From 0952439b518009765583e3a9d8929aaec5518601 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Thu, 13 Jan 2022 10:15:55 -0800 Subject: [PATCH 15/33] Wordsmithing --- .../fabric/tna/slicing/api/TrafficClassConfig.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java index 4dee11b97..6e9636b35 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java @@ -8,11 +8,13 @@ import java.util.Objects; /** - * Configuration of a traffic class. + * Describes the configuration of a traffic class within a slice. */ public class TrafficClassConfig { + private static final int UNLIMITED_MAX_RATE = Integer.MAX_VALUE; + // Common to all slices. No bandwidth guarantees or limitations. public static final TrafficClassConfig BEST_EFFORT = new TrafficClassConfig( TrafficClass.BEST_EFFORT, QueueId.BEST_EFFORT, UNLIMITED_MAX_RATE, 0); From 9bb7335943bf6710d7d5f4d136b659ad7f027296 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Thu, 13 Jan 2022 18:28:17 -0800 Subject: [PATCH 16/33] Improvements to TrafficClassConfig --- .../tna/slicing/api/TrafficClassConfig.java | 38 +++++++++++++------ .../tna/slicing/SlicingManagerTest.java | 6 +-- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java index 6e9636b35..f5a8eb3ed 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java @@ -4,24 +4,24 @@ package org.stratumproject.fabric.tna.slicing.api; import com.google.common.base.MoreObjects; - -import java.util.Objects; +import com.google.common.base.Objects; /** * Describes the configuration of a traffic class within a slice. */ public class TrafficClassConfig { - private static final int UNLIMITED_MAX_RATE = Integer.MAX_VALUE; + public static final long UNLIMITED_BPS = Long.MAX_VALUE; // Common to all slices. No bandwidth guarantees or limitations. public static final TrafficClassConfig BEST_EFFORT = new TrafficClassConfig( - TrafficClass.BEST_EFFORT, QueueId.BEST_EFFORT, UNLIMITED_MAX_RATE, 0); + TrafficClass.BEST_EFFORT, QueueId.BEST_EFFORT, UNLIMITED_BPS, 0, false); private final TrafficClass tc; private final QueueId qid; - private final int maxRateBps; - private final int gminRateBps; + private final long maxRateBps; + private final long gminRateBps; + private final boolean isSystemTc; /** * Creates a new traffic class config. @@ -30,12 +30,15 @@ public class TrafficClassConfig { * @param qid queue ID * @param maxRateBps maximum bitrate in bps * @param gminRateBps guaranteed minimum rate in bps + * @param isSystemTc whether this traffic class is to be used for system traffic */ - public TrafficClassConfig(TrafficClass tc, QueueId qid, int maxRateBps, int gminRateBps) { + public TrafficClassConfig(TrafficClass tc, QueueId qid, long maxRateBps, + long gminRateBps, boolean isSystemTc) { this.tc = tc; this.qid = qid; this.maxRateBps = maxRateBps; this.gminRateBps = gminRateBps; + this.isSystemTc = isSystemTc; } /** @@ -63,7 +66,7 @@ public QueueId queueId() { * * @return maximum bitrate in bps */ - public int getMaxRateBps() { + public long getMaxRateBps() { return maxRateBps; } @@ -73,7 +76,7 @@ public int getMaxRateBps() { * @return true if rate is unlimited */ public boolean isMaxRateUnlimited() { - return maxRateBps == UNLIMITED_MAX_RATE; + return maxRateBps == UNLIMITED_BPS; } /** @@ -85,10 +88,19 @@ public boolean isMaxRateUnlimited() { * * @return guaranteed minimum bitrate in bps */ - public int getGminRateBps() { + public long getGminRateBps() { return gminRateBps; } + /** + * Returns true if this class is expected to carry system traffic. + * + * @return true if this is the system traffic class + */ + public boolean isSystemTc() { + return isSystemTc; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -100,13 +112,14 @@ public boolean equals(Object o) { TrafficClassConfig that = (TrafficClassConfig) o; return maxRateBps == that.maxRateBps && gminRateBps == that.gminRateBps && + isSystemTc == that.isSystemTc && tc == that.tc && - Objects.equals(qid, that.qid); + Objects.equal(qid, that.qid); } @Override public int hashCode() { - return Objects.hash(tc, qid, maxRateBps, gminRateBps); + return Objects.hashCode(tc, qid, maxRateBps, gminRateBps, isSystemTc); } @Override @@ -116,6 +129,7 @@ public String toString() { .add("qid", qid) .add("maxRateBps", maxRateBps) .add("gminRateBps", gminRateBps) + .add("isSystemTc", isSystemTc) .toString(); } diff --git a/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java b/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java index 395596697..0265fe425 100644 --- a/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java +++ b/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java @@ -85,11 +85,11 @@ public class SlicingManagerTest { private static final TrafficClassConfig TC_CONFIG_CONTROL = new TrafficClassConfig( - TrafficClass.CONTROL, QUEUE_ID_CONTROL, 0, 0); + TrafficClass.CONTROL, QUEUE_ID_CONTROL, 0, 0, false); private static final TrafficClassConfig TC_CONFIG_REAL_TIME = new TrafficClassConfig( - TrafficClass.REAL_TIME, QUEUE_ID_REAL_TIME, 0, 0); + TrafficClass.REAL_TIME, QUEUE_ID_REAL_TIME, 0, 0, false); private static final TrafficClassConfig TC_CONFIG_ELASTIC = new TrafficClassConfig( - TrafficClass.ELASTIC, QUEUE_ID_ELASTIC, 0, 0); + TrafficClass.ELASTIC, QUEUE_ID_ELASTIC, 0, 0, false); private final CoreService coreService = EasyMock.createMock(CoreService.class); private final StorageService storageService = EasyMock.createMock(StorageService.class); From 1f0bc37f3e8ff305c59cf0ed271dba977ea8cdbd Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Thu, 13 Jan 2022 18:30:54 -0800 Subject: [PATCH 17/33] Rename config class to description to disambiguate from netcfg --- .../fabric/tna/slicing/SlicingManager.java | 25 +++++++++---------- .../tna/slicing/api/SlicingAdminService.java | 2 +- .../slicing/api/SlicingProviderService.java | 2 +- ...nfig.java => TrafficClassDescription.java} | 14 +++++------ .../tna/slicing/SlicingManagerTest.java | 12 ++++----- 5 files changed, 27 insertions(+), 28 deletions(-) rename src/main/java/org/stratumproject/fabric/tna/slicing/api/{TrafficClassConfig.java => TrafficClassDescription.java} (88%) diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java index 77353a87f..37555808f 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java @@ -49,11 +49,10 @@ import org.stratumproject.fabric.tna.slicing.api.SlicingProviderService; import org.stratumproject.fabric.tna.slicing.api.SlicingService; import org.stratumproject.fabric.tna.slicing.api.TrafficClass; -import org.stratumproject.fabric.tna.slicing.api.TrafficClassConfig; +import org.stratumproject.fabric.tna.slicing.api.TrafficClassDescription; import org.stratumproject.fabric.tna.web.SliceIdCodec; import org.stratumproject.fabric.tna.web.TrafficClassCodec; -import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -121,8 +120,8 @@ public class SlicingManager implements SlicingService, SlicingProviderService, S protected ApplicationId appId; // Stores currently allocated slices and their traffic classes. - protected ConsistentMap sliceStore; - private MapEventListener sliceListener; + protected ConsistentMap sliceStore; + private MapEventListener sliceListener; private ExecutorService sliceExecutor; // Stores classifier flows. @@ -146,11 +145,11 @@ protected void activate() { .register(KryoNamespaces.API) .register(SliceId.class) .register(TrafficClass.class) - .register(TrafficClassConfig.class) + .register(TrafficClassDescription.class) .register(QueueId.class) .register(SliceStoreKey.class); - sliceStore = storageService.consistentMapBuilder() + sliceStore = storageService.consistentMapBuilder() .withName("fabric-tna-slice") .withRelaxedReadConsistency() .withSerializer(Serializer.using(serializer.build())) @@ -181,7 +180,7 @@ protected void activate() { defaultTcStore.addListener(defaultTcListener); // Default slice is pre-provisioned. - sliceStore.put(new SliceStoreKey(SliceId.DEFAULT, TrafficClass.BEST_EFFORT), TrafficClassConfig.BEST_EFFORT); + sliceStore.put(new SliceStoreKey(SliceId.DEFAULT, TrafficClass.BEST_EFFORT), TrafficClassDescription.BEST_EFFORT); defaultTcStore.put(SliceId.DEFAULT, TrafficClass.BEST_EFFORT); deviceListener = new InternalDeviceListener(); @@ -227,7 +226,7 @@ public boolean addSlice(SliceId sliceId) { throw new SlicingException(FAILED, format("Slice %s already exists", sliceId)); } - return addTrafficClassInternal(true, sliceId, TrafficClassConfig.BEST_EFFORT) && + return addTrafficClassInternal(true, sliceId, TrafficClassDescription.BEST_EFFORT) && setDefaultTrafficClass(sliceId, TrafficClass.BEST_EFFORT); } @@ -270,11 +269,11 @@ public Set getSlices() { } @Override - public boolean addTrafficClass(SliceId sliceId, TrafficClassConfig tcConfig) { + public boolean addTrafficClass(SliceId sliceId, TrafficClassDescription tcConfig) { return addTrafficClassInternal(false, sliceId, tcConfig); } - private boolean addTrafficClassInternal(boolean addSlice, SliceId sliceId, TrafficClassConfig tcConfig) { + private boolean addTrafficClassInternal(boolean addSlice, SliceId sliceId, TrafficClassDescription tcConfig) { if (!addSlice && !sliceExists(sliceId)) { throw new SlicingException(INVALID, format( "Cannot add traffic class to non-existent slice %s", sliceId)); @@ -390,7 +389,7 @@ public TrafficClass getDefaultTrafficClass(SliceId sliceId) { } @Override - public Map getSliceStore() { + public Map getSliceStore() { return Map.copyOf(sliceStore.asJavaMap()); } @@ -581,8 +580,8 @@ private FlowRule buildClassifierFlowRule(DeviceId deviceId, return flowRule; } - private class InternalSliceListener implements MapEventListener { - public void event(MapEvent event) { + private class InternalSliceListener implements MapEventListener { + public void event(MapEvent event) { // Update queues table on all devices. // Distribute work based on QueueId. log.info("Processing slice event {}", event); diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingAdminService.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingAdminService.java index 4ded7fef1..755d2c1ba 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingAdminService.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingAdminService.java @@ -16,5 +16,5 @@ public interface SlicingAdminService { * * @return map of slice store */ - Map getSliceStore(); + Map getSliceStore(); } diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingProviderService.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingProviderService.java index beee472e8..e9b77b0f6 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingProviderService.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingProviderService.java @@ -35,7 +35,7 @@ public interface SlicingProviderService { * @return true if the traffic class is added to given slice successfully. * @throws SlicingException if an error occurred. */ - boolean addTrafficClass(SliceId sliceId, TrafficClassConfig tcConfig); + boolean addTrafficClass(SliceId sliceId, TrafficClassDescription tcConfig); /** * Removes a traffic class from given slice. diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassDescription.java similarity index 88% rename from src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java rename to src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassDescription.java index f5a8eb3ed..0b9df5e7d 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassConfig.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassDescription.java @@ -7,14 +7,14 @@ import com.google.common.base.Objects; /** - * Describes the configuration of a traffic class within a slice. + * Describes an instance of a traffic class within a slice. */ -public class TrafficClassConfig { +public class TrafficClassDescription { public static final long UNLIMITED_BPS = Long.MAX_VALUE; // Common to all slices. No bandwidth guarantees or limitations. - public static final TrafficClassConfig BEST_EFFORT = new TrafficClassConfig( + public static final TrafficClassDescription BEST_EFFORT = new TrafficClassDescription( TrafficClass.BEST_EFFORT, QueueId.BEST_EFFORT, UNLIMITED_BPS, 0, false); private final TrafficClass tc; @@ -24,7 +24,7 @@ public class TrafficClassConfig { private final boolean isSystemTc; /** - * Creates a new traffic class config. + * Creates a new traffic class description. * * @param tc traffic class enum value * @param qid queue ID @@ -32,8 +32,8 @@ public class TrafficClassConfig { * @param gminRateBps guaranteed minimum rate in bps * @param isSystemTc whether this traffic class is to be used for system traffic */ - public TrafficClassConfig(TrafficClass tc, QueueId qid, long maxRateBps, - long gminRateBps, boolean isSystemTc) { + public TrafficClassDescription(TrafficClass tc, QueueId qid, long maxRateBps, + long gminRateBps, boolean isSystemTc) { this.tc = tc; this.qid = qid; this.maxRateBps = maxRateBps; @@ -109,7 +109,7 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) { return false; } - TrafficClassConfig that = (TrafficClassConfig) o; + TrafficClassDescription that = (TrafficClassDescription) o; return maxRateBps == that.maxRateBps && gminRateBps == that.gminRateBps && isSystemTc == that.isSystemTc && diff --git a/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java b/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java index 0265fe425..29ae44289 100644 --- a/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java +++ b/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java @@ -42,7 +42,7 @@ import org.stratumproject.fabric.tna.slicing.api.SliceId; import org.stratumproject.fabric.tna.slicing.api.SlicingException; import org.stratumproject.fabric.tna.slicing.api.TrafficClass; -import org.stratumproject.fabric.tna.slicing.api.TrafficClassConfig; +import org.stratumproject.fabric.tna.slicing.api.TrafficClassDescription; import java.util.ArrayList; import java.util.Collections; @@ -84,11 +84,11 @@ public class SlicingManagerTest { private static final QueueId QUEUE_ID_ELASTIC = QueueId.of(3); - private static final TrafficClassConfig TC_CONFIG_CONTROL = new TrafficClassConfig( + private static final TrafficClassDescription TC_CONFIG_CONTROL = new TrafficClassDescription( TrafficClass.CONTROL, QUEUE_ID_CONTROL, 0, 0, false); - private static final TrafficClassConfig TC_CONFIG_REAL_TIME = new TrafficClassConfig( + private static final TrafficClassDescription TC_CONFIG_REAL_TIME = new TrafficClassDescription( TrafficClass.REAL_TIME, QUEUE_ID_REAL_TIME, 0, 0, false); - private static final TrafficClassConfig TC_CONFIG_ELASTIC = new TrafficClassConfig( + private static final TrafficClassDescription TC_CONFIG_ELASTIC = new TrafficClassDescription( TrafficClass.ELASTIC, QUEUE_ID_ELASTIC, 0, 0, false); private final CoreService coreService = EasyMock.createMock(CoreService.class); @@ -136,7 +136,7 @@ public void setup() { manager.pipeconfService = pipeconfService; EasyMock.expect(coreService.registerApplication(EasyMock.anyObject())).andReturn(APP_ID); - EasyMock.expect(storageService.consistentMapBuilder()).andReturn( + EasyMock.expect(storageService.consistentMapBuilder()).andReturn( new MockConsistentMap.Builder<>()); EasyMock.expect(storageService.consistentMapBuilder()).andReturn( new MockConsistentMap.Builder<>()); @@ -281,7 +281,7 @@ public void testAddTrafficClass() { public void testAddTrafficClassExceptionNonExistentSlice() { exceptionRule.expect(SlicingException.class); exceptionRule.expectMessage("Cannot add traffic class to non-existent slice 1"); - manager.addTrafficClass(SLICE_IDS.get(1), TrafficClassConfig.BEST_EFFORT); + manager.addTrafficClass(SLICE_IDS.get(1), TrafficClassDescription.BEST_EFFORT); } @Test From 24a62d822a3a8074ec5f511ff786f23d812d2b2e Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Thu, 13 Jan 2022 18:32:30 -0800 Subject: [PATCH 18/33] First stab at config class --- .../fabric/tna/slicing/api/QueueId.java | 3 +- .../tna/slicing/api/SliceDescription.java | 74 ++++++ .../fabric/tna/slicing/api/SlicingConfig.java | 216 ++++++++++++++++++ 3 files changed, 292 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/stratumproject/fabric/tna/slicing/api/SliceDescription.java create mode 100644 src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/QueueId.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/QueueId.java index e360432a6..9f34c5831 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/QueueId.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/QueueId.java @@ -12,6 +12,7 @@ * Queue identifier. */ public final class QueueId extends Identifier implements Comparable { + public static final Integer MIN = 0; public static final Integer MAX = 1 << HDR_EGRESS_QID_BITWIDTH - 1; public static final QueueId BEST_EFFORT = QueueId.of(QUEUE_ID_BEST_EFFORT); @@ -27,7 +28,7 @@ private QueueId(int id) { * @throws IllegalArgumentException if given id is invalid */ public static QueueId of(int id) { - checkArgument(id >= 0 && id <= MAX, "Invalid id %s. Valid range is from %s to %s", id, 0, MAX); + checkArgument(id >= MIN && id <= MAX, "Invalid id %s. Valid range is from %s to %s", id, 0, MAX); return new QueueId(id); } diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SliceDescription.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SliceDescription.java new file mode 100644 index 000000000..daef19fb3 --- /dev/null +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SliceDescription.java @@ -0,0 +1,74 @@ +// Copyright $today.year-present Open Networking Foundation +// SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 + +package org.stratumproject.fabric.tna.slicing.api; + +import com.google.common.base.MoreObjects; +import com.google.common.base.Objects; +import com.google.common.collect.ImmutableMap; + +import java.util.Collection; +import java.util.Map; + +/** + * Describes a slice. + */ +// TODO: javadoc +public class SliceDescription { + + private final SliceId id; + private final String name; + private final ImmutableMap tcConfigs; + + public SliceDescription(SliceId id, String name, Map tcConfigs) { + this.id = id; + this.tcConfigs = ImmutableMap.copyOf(tcConfigs); + this.name = name; + } + + public SliceId id() { + return id; + } + + public Collection tcConfigs() { + return tcConfigs.values(); + } + + public TrafficClassDescription tcConfig(TrafficClass tc) { + return tcConfigs.get(tc); + } + + public String name() { + return name; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + SliceDescription that = (SliceDescription) o; + return Objects.equal(id, that.id) && + Objects.equal(name, that.name) && + Objects.equal(tcConfigs, that.tcConfigs); + } + + @Override + public int hashCode() { + return Objects.hashCode(id, name, tcConfigs); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("id", id) + .add("name", name) + .add("tcConfigs", tcConfigs) + .toString(); + } + + // TODO: builder +} diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java new file mode 100644 index 000000000..10113cab8 --- /dev/null +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java @@ -0,0 +1,216 @@ +// Copyright 2021-present Open Networking Foundation +// SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 + +package org.stratumproject.fabric.tna.slicing.api; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; +import org.onosproject.core.ApplicationId; +import org.onosproject.net.config.Config; +import org.onosproject.net.config.ConfigException; +import org.onosproject.net.config.InvalidFieldException; + +import java.util.Collection; +import java.util.List; +import java.util.Map; + +import static java.lang.String.format; + +/** + * Configuration for slicing. + *

+ * Example: + * 

+ * {
+ *   "apps": {
+ *     "org.stratumproject.fabric-tna": {
+ *       "slicing": {
+ *         "bestEffortQueueId": 0,
+ *         "slices": {
+ *           "0": {
+ *             "name": "Default",
+ *             "tcs": {
+ *               "REAL_TIME": {
+ *                 "queueId": 1,
+ *                 "isSystemTc": true
+ *               }
+ *             }
+ *           },
+ *           "1": {
+ *             "name": "P4-UPF",
+ *             "tcs": {
+ *               "CONTROL": {
+ *                 "queueId": 2,
+ *                 "maxRateBps": "2000000"
+ *               },
+ *               "REAL_TIME": {
+ *                 "queueId": 3,
+ *                 "maxRateBps": "50000000"
+ *               },
+ *               "ELASTIC": {
+ *                 "queueId": 4,
+ *                 "gminRateBps": "10000000"
+ *               }
+ *             }
+ *           },
+ *           "3": {
+ *             "name": "BESS-UPF",
+ *             "tcs": {
+ *               "ELASTIC": {
+ *                 "queueId": 5
+ *               }
+ *             }
+ *           }
+ *         }
+ *       }
+ *     }
+ *   }
+ * }
+ *
+ */ +// TODO: javadoc +public class SlicingConfig extends Config { + private static final String SLICES = "slices"; + private static final String BEST_EFFORT_QUEUE_ID = "bestEffortQueueId"; + private static final String TCS = "tcs"; + private static final String NAME = "name"; + private static final String QUEUE_ID = "queueId"; + private static final String IS_SYSTEM_TC = "isSystemTc"; + private static final String MAX_RATE_BPS = "maxRateBps"; + private static final String GMIN_RATE_BPS = "gminRateBps"; + + private static final int DEFAULT_BEST_EFFORT_QUEUE_ID = QueueId.BEST_EFFORT.id(); + private static final long DEFAULT_MAX_RATE_BPS = TrafficClassDescription.UNLIMITED_BPS; + private static final long DEFAULT_GMIN_RATE_BPS = 0; + private static final boolean DEFAULT_IS_SYSTEM_TC = false; + + @Override + public boolean isValid() { + if (!(hasOnlyFields(object, BEST_EFFORT_QUEUE_ID, SLICES) && + isIntegralNumber(object, BEST_EFFORT_QUEUE_ID, FieldPresence.OPTIONAL, + (long) QueueId.MIN, (long) QueueId.MAX))) { + return false; + } + + for (JsonNode sliceNode : object.path(SLICES)) { + if (!(hasOnlyFields((ObjectNode) sliceNode, NAME, TCS) && + hasFields((ObjectNode) sliceNode, NAME) && + isString((ObjectNode) sliceNode, NAME, FieldPresence.MANDATORY))) { + return false; + } + + for (JsonNode tcNode : sliceNode.path(TCS)) { + if (!(hasOnlyFields((ObjectNode) tcNode, QUEUE_ID, MAX_RATE_BPS, GMIN_RATE_BPS, IS_SYSTEM_TC) && + hasFields((ObjectNode) tcNode, QUEUE_ID) && + isIntegralNumber((ObjectNode) tcNode, QUEUE_ID, FieldPresence.MANDATORY, QueueId.MIN, QueueId.MAX) && + isIntegralNumber((ObjectNode) tcNode, MAX_RATE_BPS, FieldPresence.OPTIONAL, 0, TrafficClassDescription.UNLIMITED_BPS)) && + isIntegralNumber((ObjectNode) tcNode, GMIN_RATE_BPS, FieldPresence.OPTIONAL, 0, TrafficClassDescription.UNLIMITED_BPS) && + isBoolean((ObjectNode) tcNode, IS_SYSTEM_TC, FieldPresence.OPTIONAL)) { + return false; + } + } + + try { + var slices = slices(); + if (slices.isEmpty()) { + throw new InvalidFieldException(SLICES, "At least one slice should be specified"); + } + + var systemTcsCount = 0; + for (SliceDescription sliceConfig : slices) { + for (TrafficClassDescription tcConfig : sliceConfig.tcConfigs()) { + if (tcConfig.isSystemTc()) { + systemTcsCount++; + } + } + } + if (systemTcsCount == 0) { + throw new InvalidFieldException(SLICES, format( + "At least one traffic class should be set as the system one (%s=true)", + IS_SYSTEM_TC)); + } + if (systemTcsCount > 1) { + throw new InvalidFieldException(SLICES, + "Too many traffic classes are set as the system one, only one is allowed"); + } + } catch (ConfigException e) { + throw new InvalidFieldException(SLICES, e); + } + } + + return true; + } + + public QueueId bestEffortQueueId() { + return QueueId.of(object.path(BEST_EFFORT_QUEUE_ID) + .asInt(DEFAULT_BEST_EFFORT_QUEUE_ID)); + } + + public Collection slices() throws ConfigException { + List sliceConfigs = Lists.newArrayList(); + var jsonSlices = object.path(SLICES).fields(); + while (jsonSlices.hasNext()) { + var jsonSlice = jsonSlices.next(); + var sliceId = SliceId.of(Integer.parseInt(jsonSlice.getKey())); + var sliceConfig = slice(sliceId); + if (sliceConfig != null) { + sliceConfigs.add(sliceConfig); + } + } + return sliceConfigs; + } + + public SliceDescription slice(SliceId sliceId) throws ConfigException { + var sliceNode = object.path(SLICES).path(sliceId.toString()); + if (sliceNode.isMissingNode()) { + return null; + } + + var name = sliceNode.path(NAME).asText(); + if (name.isEmpty()) { + throw new ConfigException(format( + "Slice %s must have a valid name", sliceId)); + } + + Map tcConfigs = Maps.newHashMap(); + var tcConfigFields = sliceNode.path(TCS).fields(); + while (tcConfigFields.hasNext()) { + var tcConfigField = tcConfigFields.next(); + var tcName = tcConfigField.getKey(); + TrafficClass tc; + try { + tc = TrafficClass.valueOf(tcName); + } catch (IllegalArgumentException e) { + throw new ConfigException(format( + "%s is not a valid traffic class for slice %s", tcName, sliceId), e); + } + tcConfigs.put(tc, tcConfig(sliceId, tc)); + } + + return new SliceDescription(sliceId, name, tcConfigs); + } + + public TrafficClassDescription tcConfig(SliceId sliceId, TrafficClass tc) throws ConfigException { + var tcNode = object.path(SLICES).path(sliceId.toString()).path(tc.name()); + if (tcNode.isMissingNode()) { + return null; + } + + QueueId queueId; + try { + queueId = QueueId.of(tcNode.path(QUEUE_ID).asInt()); + } catch (IllegalArgumentException e) { + throw new ConfigException(format( + "'%s' is not a valid queue ID for traffic class %s of slice %s", + tcNode.path(QUEUE_ID).asText(), tc, sliceId), e); + } + + var maxRateBps = tcNode.path(MAX_RATE_BPS).asLong(DEFAULT_MAX_RATE_BPS); + var gminRateBps = tcNode.path(GMIN_RATE_BPS).asLong(DEFAULT_GMIN_RATE_BPS); + var isSystemTc = tcNode.path(IS_SYSTEM_TC).asBoolean(DEFAULT_IS_SYSTEM_TC); + + return new TrafficClassDescription(tc, queueId, maxRateBps, gminRateBps, isSystemTc); + } +} From 804cc1a0aebad3c5d8ccc268db82de8e6c4b4636 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Thu, 13 Jan 2022 19:25:55 -0800 Subject: [PATCH 19/33] First stab at tests for SlicingTests --- .../fabric/tna/slicing/api/SlicingConfig.java | 10 +- .../slicing/api/TrafficClassDescription.java | 4 +- .../tna/slicing/api/SlicingConfigTest.java | 92 +++++++++++++++++++ .../fabric/tna/utils/TestUtils.java | 16 ++++ 4 files changed, 115 insertions(+), 7 deletions(-) create mode 100644 src/test/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfigTest.java diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java index 10113cab8..ceff93605 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java @@ -154,10 +154,7 @@ public Collection slices() throws ConfigException { while (jsonSlices.hasNext()) { var jsonSlice = jsonSlices.next(); var sliceId = SliceId.of(Integer.parseInt(jsonSlice.getKey())); - var sliceConfig = slice(sliceId); - if (sliceConfig != null) { - sliceConfigs.add(sliceConfig); - } + sliceConfigs.add(slice(sliceId)); } return sliceConfigs; } @@ -193,7 +190,10 @@ public SliceDescription slice(SliceId sliceId) throws ConfigException { } public TrafficClassDescription tcConfig(SliceId sliceId, TrafficClass tc) throws ConfigException { - var tcNode = object.path(SLICES).path(sliceId.toString()).path(tc.name()); + var tcNode = object.path(SLICES) + .path(sliceId.toString()) + .path(TCS) + .path(tc.name()); if (tcNode.isMissingNode()) { return null; } diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassDescription.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassDescription.java index 0b9df5e7d..a520010f1 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassDescription.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassDescription.java @@ -66,7 +66,7 @@ public QueueId queueId() { * * @return maximum bitrate in bps */ - public long getMaxRateBps() { + public long maxRateBps() { return maxRateBps; } @@ -88,7 +88,7 @@ public boolean isMaxRateUnlimited() { * * @return guaranteed minimum bitrate in bps */ - public long getGminRateBps() { + public long gminRateBps() { return gminRateBps; } diff --git a/src/test/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfigTest.java b/src/test/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfigTest.java new file mode 100644 index 000000000..98858ccbc --- /dev/null +++ b/src/test/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfigTest.java @@ -0,0 +1,92 @@ +// Copyright $today.year-present Open Networking Foundation +// SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 + +package org.stratumproject.fabric.tna.slicing.api; + +import org.junit.Test; +import org.onosproject.TestApplicationId; +import org.onosproject.core.ApplicationId; +import org.stratumproject.fabric.tna.utils.TestUtils; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +/** + * Tests for SlicingConfig. + */ +public class SlicingConfigTest { + + private static final String APP_NAME = "foobar"; + private static final ApplicationId APP_ID = new TestApplicationId(APP_NAME); + + @Test + public void testConstruction() throws Exception { + SlicingConfig config = TestUtils.getSlicingConfig(APP_ID, "/slicing.json"); + + assertTrue(config.isValid()); + + assertEquals(QueueId.of(0), config.bestEffortQueueId()); + + assertEquals(3, config.slices().size()); + assertNotNull(config.slice(SliceId.of(0))); + assertNotNull(config.slice(SliceId.of(1))); + assertNotNull(config.slice(SliceId.of(2))); + assertNull(config.slice(SliceId.of(3))); + + SliceDescription sliceDescr; + TrafficClassDescription tcDescr; + + sliceDescr = config.slice(SliceId.of(0)); + assertEquals(SliceId.of(0), sliceDescr.id()); + assertEquals("Default", sliceDescr.name()); + assertEquals(1, sliceDescr.tcConfigs().size()); + + tcDescr = sliceDescr.tcConfig(TrafficClass.REAL_TIME); + assertNotNull(tcDescr); + assertEquals(QueueId.of(1), tcDescr.queueId()); + assertEquals(TrafficClassDescription.UNLIMITED_BPS, tcDescr.maxRateBps()); + assertEquals(0, tcDescr.gminRateBps()); + assertTrue(tcDescr.isSystemTc()); + + sliceDescr = config.slice(SliceId.of(1)); + assertEquals(SliceId.of(1), sliceDescr.id()); + assertEquals("P4-UPF", sliceDescr.name()); + assertEquals(3, sliceDescr.tcConfigs().size()); + + tcDescr = sliceDescr.tcConfig(TrafficClass.CONTROL); + assertNotNull(tcDescr); + assertEquals(QueueId.of(2), tcDescr.queueId()); + assertEquals(2000000, tcDescr.maxRateBps()); + assertEquals(0, tcDescr.gminRateBps()); + assertFalse(tcDescr.isSystemTc()); + + tcDescr = sliceDescr.tcConfig(TrafficClass.REAL_TIME); + assertNotNull(tcDescr); + assertEquals(QueueId.of(3), tcDescr.queueId()); + assertEquals(50000000, tcDescr.maxRateBps()); + assertEquals(0, tcDescr.gminRateBps()); + assertFalse(tcDescr.isSystemTc()); + + tcDescr = sliceDescr.tcConfig(TrafficClass.ELASTIC); + assertNotNull(tcDescr); + assertEquals(QueueId.of(4), tcDescr.queueId()); + assertEquals(TrafficClassDescription.UNLIMITED_BPS, tcDescr.maxRateBps()); + assertEquals(10000000, tcDescr.gminRateBps()); + assertFalse(tcDescr.isSystemTc()); + + sliceDescr = config.slice(SliceId.of(2)); + assertEquals(SliceId.of(2), sliceDescr.id()); + assertEquals("BESS-UPF", sliceDescr.name()); + assertEquals(1, sliceDescr.tcConfigs().size()); + + tcDescr = sliceDescr.tcConfig(TrafficClass.ELASTIC); + assertNotNull(tcDescr); + assertEquals(QueueId.of(5), tcDescr.queueId()); + assertEquals(TrafficClassDescription.UNLIMITED_BPS, tcDescr.maxRateBps()); + assertEquals(0, tcDescr.gminRateBps()); + assertFalse(tcDescr.isSystemTc()); + } +} \ No newline at end of file diff --git a/src/test/java/org/stratumproject/fabric/tna/utils/TestUtils.java b/src/test/java/org/stratumproject/fabric/tna/utils/TestUtils.java index 352689cab..aeec34269 100644 --- a/src/test/java/org/stratumproject/fabric/tna/utils/TestUtils.java +++ b/src/test/java/org/stratumproject/fabric/tna/utils/TestUtils.java @@ -14,6 +14,7 @@ import org.onosproject.segmentrouting.config.SegmentRoutingDeviceConfig; import org.stratumproject.fabric.tna.inbandtelemetry.IntReportConfig; +import org.stratumproject.fabric.tna.slicing.api.SlicingConfig; import static org.junit.Assert.fail; @@ -23,6 +24,7 @@ public final class TestUtils { private static final String INT_REPORT_CONFIG_KEY = "report"; private static final String SR_CONFIG_KEY = "segmentrouting"; + private static final String SLICING_CONFIG_KEY = "slicing"; private TestUtils() { } @@ -54,4 +56,18 @@ public static IntReportConfig getIntReportConfig(ApplicationId appId, String fil } return config; } + + public static SlicingConfig getSlicingConfig(ApplicationId appId, String filename) { + SlicingConfig config = new SlicingConfig(); + InputStream jsonStream = TestUtils.class.getResourceAsStream(filename); + ObjectMapper mapper = new ObjectMapper(); + JsonNode jsonNode; + try { + jsonNode = mapper.readTree(jsonStream); + config.init(appId, SLICING_CONFIG_KEY, jsonNode, mapper, c -> { }); + } catch (Exception e) { + fail("Got error when reading file " + filename + " : " + e.getMessage()); + } + return config; + } } From 0bfa478fa04bd84431e7a58c89c13ffecc564ea5 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Thu, 13 Jan 2022 19:35:18 -0800 Subject: [PATCH 20/33] Forgot to check in the test JSON --- src/test/resources/slicing.json | 39 +++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 src/test/resources/slicing.json diff --git a/src/test/resources/slicing.json b/src/test/resources/slicing.json new file mode 100644 index 000000000..cf3d6138c --- /dev/null +++ b/src/test/resources/slicing.json @@ -0,0 +1,39 @@ +{ + "bestEffortQueueId": 0, + "slices": { + "0": { + "name": "Default", + "tcs": { + "REAL_TIME": { + "queueId": 1, + "isSystemTc": true + } + } + }, + "1": { + "name": "P4-UPF", + "tcs": { + "CONTROL": { + "queueId": 2, + "maxRateBps": "2000000" + }, + "REAL_TIME": { + "queueId": 3, + "maxRateBps": "50000000" + }, + "ELASTIC": { + "queueId": 4, + "gminRateBps": "10000000" + } + } + }, + "2": { + "name": "BESS-UPF", + "tcs": { + "ELASTIC": { + "queueId": 5 + } + } + } + } +} \ No newline at end of file From 9f8d376199d39956bf8e5056c154712b852d23fc Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Mon, 17 Jan 2022 21:56:56 -0800 Subject: [PATCH 21/33] Consistently call it tcDescription instead of tcConfig --- .../fabric/tna/slicing/SlicingManager.java | 23 ++++++++------- .../tna/slicing/api/SliceDescription.java | 20 ++++++------- .../fabric/tna/slicing/api/SlicingConfig.java | 29 ++++++++++--------- .../slicing/api/SlicingProviderService.java | 6 ++-- .../tna/slicing/SlicingManagerTest.java | 3 +- .../tna/slicing/api/SlicingConfigTest.java | 16 +++++----- 6 files changed, 52 insertions(+), 45 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java index 37555808f..5ca32b8f6 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java @@ -180,7 +180,8 @@ protected void activate() { defaultTcStore.addListener(defaultTcListener); // Default slice is pre-provisioned. - sliceStore.put(new SliceStoreKey(SliceId.DEFAULT, TrafficClass.BEST_EFFORT), TrafficClassDescription.BEST_EFFORT); + sliceStore.put(new SliceStoreKey(SliceId.DEFAULT, TrafficClass.BEST_EFFORT), + TrafficClassDescription.BEST_EFFORT); defaultTcStore.put(SliceId.DEFAULT, TrafficClass.BEST_EFFORT); deviceListener = new InternalDeviceListener(); @@ -269,27 +270,27 @@ public Set getSlices() { } @Override - public boolean addTrafficClass(SliceId sliceId, TrafficClassDescription tcConfig) { - return addTrafficClassInternal(false, sliceId, tcConfig); + public boolean addTrafficClass(SliceId sliceId, TrafficClassDescription tcDescription) { + return addTrafficClassInternal(false, sliceId, tcDescription); } - private boolean addTrafficClassInternal(boolean addSlice, SliceId sliceId, TrafficClassDescription tcConfig) { + private boolean addTrafficClassInternal(boolean addSlice, SliceId sliceId, TrafficClassDescription tcDescription) { if (!addSlice && !sliceExists(sliceId)) { throw new SlicingException(INVALID, format( "Cannot add traffic class to non-existent slice %s", sliceId)); } StringBuilder errorMessage = new StringBuilder(); - SliceStoreKey key = new SliceStoreKey(sliceId, tcConfig.trafficClass()); + SliceStoreKey key = new SliceStoreKey(sliceId, tcDescription.trafficClass()); sliceStore.compute(key, (k, v) -> { if (v != null) { errorMessage.append(format("TC %s is already allocated for slice %s", - tcConfig.trafficClass(), sliceId)); + tcDescription.trafficClass(), sliceId)); return v; } - log.info("Added traffic class {} to slice {}: {}", tcConfig.trafficClass(), sliceId, tcConfig); - return tcConfig; + log.info("Added traffic class {} to slice {}: {}", tcDescription.trafficClass(), sliceId, tcDescription); + return tcDescription; }); if (errorMessage.length() != 0) { @@ -592,7 +593,8 @@ public void event(MapEvent event) { if (workPartitionService.isMine(event.newValue().value(), toStringHasher())) { deviceService.getAvailableDevices().forEach(device -> addQueuesFlowRules(device.id(), - event.key().sliceId(), event.key().trafficClass(), event.newValue().value().queueId()) + event.key().sliceId(), event.key().trafficClass(), + event.newValue().value().queueId()) ); } break; @@ -600,7 +602,8 @@ public void event(MapEvent event) { if (workPartitionService.isMine(event.oldValue().value(), toStringHasher())) { deviceService.getAvailableDevices().forEach(device -> removeQueuesFlowRules(device.id(), - event.key().sliceId(), event.key().trafficClass(), event.oldValue().value().queueId()) + event.key().sliceId(), event.key().trafficClass(), + event.oldValue().value().queueId()) ); } break; diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SliceDescription.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SliceDescription.java index daef19fb3..cc04a79b2 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SliceDescription.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SliceDescription.java @@ -18,11 +18,11 @@ public class SliceDescription { private final SliceId id; private final String name; - private final ImmutableMap tcConfigs; + private final ImmutableMap tcDescriptions; - public SliceDescription(SliceId id, String name, Map tcConfigs) { + public SliceDescription(SliceId id, String name, Map tcDescriptions) { this.id = id; - this.tcConfigs = ImmutableMap.copyOf(tcConfigs); + this.tcDescriptions = ImmutableMap.copyOf(tcDescriptions); this.name = name; } @@ -30,12 +30,12 @@ public SliceId id() { return id; } - public Collection tcConfigs() { - return tcConfigs.values(); + public Collection tcDescriptions() { + return tcDescriptions.values(); } - public TrafficClassDescription tcConfig(TrafficClass tc) { - return tcConfigs.get(tc); + public TrafficClassDescription tcDescription(TrafficClass tc) { + return tcDescriptions.get(tc); } public String name() { @@ -53,12 +53,12 @@ public boolean equals(Object o) { SliceDescription that = (SliceDescription) o; return Objects.equal(id, that.id) && Objects.equal(name, that.name) && - Objects.equal(tcConfigs, that.tcConfigs); + Objects.equal(tcDescriptions, that.tcDescriptions); } @Override public int hashCode() { - return Objects.hashCode(id, name, tcConfigs); + return Objects.hashCode(id, name, tcDescriptions); } @Override @@ -66,7 +66,7 @@ public String toString() { return MoreObjects.toStringHelper(this) .add("id", id) .add("name", name) - .add("tcConfigs", tcConfigs) + .add("tcs", tcDescriptions) .toString(); } diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java index ceff93605..771653dfa 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java @@ -104,9 +104,12 @@ public boolean isValid() { for (JsonNode tcNode : sliceNode.path(TCS)) { if (!(hasOnlyFields((ObjectNode) tcNode, QUEUE_ID, MAX_RATE_BPS, GMIN_RATE_BPS, IS_SYSTEM_TC) && hasFields((ObjectNode) tcNode, QUEUE_ID) && - isIntegralNumber((ObjectNode) tcNode, QUEUE_ID, FieldPresence.MANDATORY, QueueId.MIN, QueueId.MAX) && - isIntegralNumber((ObjectNode) tcNode, MAX_RATE_BPS, FieldPresence.OPTIONAL, 0, TrafficClassDescription.UNLIMITED_BPS)) && - isIntegralNumber((ObjectNode) tcNode, GMIN_RATE_BPS, FieldPresence.OPTIONAL, 0, TrafficClassDescription.UNLIMITED_BPS) && + isIntegralNumber((ObjectNode) tcNode, QUEUE_ID, FieldPresence.MANDATORY, + QueueId.MIN, QueueId.MAX) && + isIntegralNumber((ObjectNode) tcNode, MAX_RATE_BPS, FieldPresence.OPTIONAL, 0, + TrafficClassDescription.UNLIMITED_BPS)) && + isIntegralNumber((ObjectNode) tcNode, GMIN_RATE_BPS, FieldPresence.OPTIONAL, + 0, TrafficClassDescription.UNLIMITED_BPS) && isBoolean((ObjectNode) tcNode, IS_SYSTEM_TC, FieldPresence.OPTIONAL)) { return false; } @@ -120,8 +123,8 @@ public boolean isValid() { var systemTcsCount = 0; for (SliceDescription sliceConfig : slices) { - for (TrafficClassDescription tcConfig : sliceConfig.tcConfigs()) { - if (tcConfig.isSystemTc()) { + for (TrafficClassDescription tcDescription : sliceConfig.tcDescriptions()) { + if (tcDescription.isSystemTc()) { systemTcsCount++; } } @@ -171,11 +174,11 @@ public SliceDescription slice(SliceId sliceId) throws ConfigException { "Slice %s must have a valid name", sliceId)); } - Map tcConfigs = Maps.newHashMap(); - var tcConfigFields = sliceNode.path(TCS).fields(); - while (tcConfigFields.hasNext()) { - var tcConfigField = tcConfigFields.next(); - var tcName = tcConfigField.getKey(); + Map tcDescriptions = Maps.newHashMap(); + var tcDescriptionFields = sliceNode.path(TCS).fields(); + while (tcDescriptionFields.hasNext()) { + var tcDescriptionField = tcDescriptionFields.next(); + var tcName = tcDescriptionField.getKey(); TrafficClass tc; try { tc = TrafficClass.valueOf(tcName); @@ -183,13 +186,13 @@ public SliceDescription slice(SliceId sliceId) throws ConfigException { throw new ConfigException(format( "%s is not a valid traffic class for slice %s", tcName, sliceId), e); } - tcConfigs.put(tc, tcConfig(sliceId, tc)); + tcDescriptions.put(tc, tcDescription(sliceId, tc)); } - return new SliceDescription(sliceId, name, tcConfigs); + return new SliceDescription(sliceId, name, tcDescriptions); } - public TrafficClassDescription tcConfig(SliceId sliceId, TrafficClass tc) throws ConfigException { + public TrafficClassDescription tcDescription(SliceId sliceId, TrafficClass tc) throws ConfigException { var tcNode = object.path(SLICES) .path(sliceId.toString()) .path(TCS) diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingProviderService.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingProviderService.java index e9b77b0f6..6052f0b36 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingProviderService.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingProviderService.java @@ -28,14 +28,14 @@ public interface SlicingProviderService { boolean removeSlice(SliceId sliceId); /** - * Adds a traffic class to given slice. + * Adds a traffic class to the given slice. * * @param sliceId slice identifier - * @param tcConfig traffic class config + * @param tcDescription traffic class config * @return true if the traffic class is added to given slice successfully. * @throws SlicingException if an error occurred. */ - boolean addTrafficClass(SliceId sliceId, TrafficClassDescription tcConfig); + boolean addTrafficClass(SliceId sliceId, TrafficClassDescription tcDescription); /** * Removes a traffic class from given slice. diff --git a/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java b/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java index 29ae44289..49ad8fafc 100644 --- a/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java +++ b/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java @@ -360,7 +360,8 @@ public void testSetDefaultTrafficClassExceptionNotAllocated() { manager.addSlice(SLICE_IDS.get(1)); exceptionRule.expect(SlicingException.class); - exceptionRule.expectMessage("Cannot set CONTROL as the default traffic class because it has not been allocated to slice 1"); + exceptionRule.expectMessage("Cannot set CONTROL as the default traffic " + + "class because it has not been allocated to slice 1"); manager.setDefaultTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL); } diff --git a/src/test/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfigTest.java b/src/test/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfigTest.java index 98858ccbc..0003ab67f 100644 --- a/src/test/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfigTest.java +++ b/src/test/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfigTest.java @@ -42,9 +42,9 @@ public void testConstruction() throws Exception { sliceDescr = config.slice(SliceId.of(0)); assertEquals(SliceId.of(0), sliceDescr.id()); assertEquals("Default", sliceDescr.name()); - assertEquals(1, sliceDescr.tcConfigs().size()); + assertEquals(1, sliceDescr.tcDescriptions().size()); - tcDescr = sliceDescr.tcConfig(TrafficClass.REAL_TIME); + tcDescr = sliceDescr.tcDescription(TrafficClass.REAL_TIME); assertNotNull(tcDescr); assertEquals(QueueId.of(1), tcDescr.queueId()); assertEquals(TrafficClassDescription.UNLIMITED_BPS, tcDescr.maxRateBps()); @@ -54,23 +54,23 @@ public void testConstruction() throws Exception { sliceDescr = config.slice(SliceId.of(1)); assertEquals(SliceId.of(1), sliceDescr.id()); assertEquals("P4-UPF", sliceDescr.name()); - assertEquals(3, sliceDescr.tcConfigs().size()); + assertEquals(3, sliceDescr.tcDescriptions().size()); - tcDescr = sliceDescr.tcConfig(TrafficClass.CONTROL); + tcDescr = sliceDescr.tcDescription(TrafficClass.CONTROL); assertNotNull(tcDescr); assertEquals(QueueId.of(2), tcDescr.queueId()); assertEquals(2000000, tcDescr.maxRateBps()); assertEquals(0, tcDescr.gminRateBps()); assertFalse(tcDescr.isSystemTc()); - tcDescr = sliceDescr.tcConfig(TrafficClass.REAL_TIME); + tcDescr = sliceDescr.tcDescription(TrafficClass.REAL_TIME); assertNotNull(tcDescr); assertEquals(QueueId.of(3), tcDescr.queueId()); assertEquals(50000000, tcDescr.maxRateBps()); assertEquals(0, tcDescr.gminRateBps()); assertFalse(tcDescr.isSystemTc()); - tcDescr = sliceDescr.tcConfig(TrafficClass.ELASTIC); + tcDescr = sliceDescr.tcDescription(TrafficClass.ELASTIC); assertNotNull(tcDescr); assertEquals(QueueId.of(4), tcDescr.queueId()); assertEquals(TrafficClassDescription.UNLIMITED_BPS, tcDescr.maxRateBps()); @@ -80,9 +80,9 @@ public void testConstruction() throws Exception { sliceDescr = config.slice(SliceId.of(2)); assertEquals(SliceId.of(2), sliceDescr.id()); assertEquals("BESS-UPF", sliceDescr.name()); - assertEquals(1, sliceDescr.tcConfigs().size()); + assertEquals(1, sliceDescr.tcDescriptions().size()); - tcDescr = sliceDescr.tcConfig(TrafficClass.ELASTIC); + tcDescr = sliceDescr.tcDescription(TrafficClass.ELASTIC); assertNotNull(tcDescr); assertEquals(QueueId.of(5), tcDescr.queueId()); assertEquals(TrafficClassDescription.UNLIMITED_BPS, tcDescr.maxRateBps()); From 2072395194269bb261afea373a5645d5e193ee7c Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Wed, 19 Jan 2022 17:50:07 -0800 Subject: [PATCH 22/33] Remove ability to configure best effort queue ID For now, we can safely assume it will always be 0 --- .../fabric/tna/behaviour/Constants.java | 1 - .../fabric/tna/slicing/api/QueueId.java | 10 +++++----- .../fabric/tna/slicing/api/SlicingConfig.java | 15 ++++----------- .../fabric/tna/slicing/api/SlicingConfigTest.java | 2 -- src/test/resources/slicing.json | 1 - 5 files changed, 9 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/behaviour/Constants.java b/src/main/java/org/stratumproject/fabric/tna/behaviour/Constants.java index a56db49a5..d0f2a556a 100644 --- a/src/main/java/org/stratumproject/fabric/tna/behaviour/Constants.java +++ b/src/main/java/org/stratumproject/fabric/tna/behaviour/Constants.java @@ -53,7 +53,6 @@ public final class Constants { // Static Queue IDs (should match those in gen-stratum-qos-config.py) // FIXME: remove hardcoded queue ID, should be derived from netcfg - public static final int QUEUE_ID_BEST_EFFORT = 0; public static final int QUEUE_ID_SYSTEM = 1; public static final int DEFAULT_SLICE_ID = 0; diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/QueueId.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/QueueId.java index 9f34c5831..167a32bfc 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/QueueId.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/QueueId.java @@ -5,16 +5,16 @@ import org.onlab.util.Identifier; import static com.google.common.base.Preconditions.checkArgument; -import static org.stratumproject.fabric.tna.behaviour.Constants.QUEUE_ID_BEST_EFFORT; import static org.stratumproject.fabric.tna.behaviour.P4InfoConstants.HDR_EGRESS_QID_BITWIDTH; /** * Queue identifier. */ public final class QueueId extends Identifier implements Comparable { - public static final Integer MIN = 0; - public static final Integer MAX = 1 << HDR_EGRESS_QID_BITWIDTH - 1; - public static final QueueId BEST_EFFORT = QueueId.of(QUEUE_ID_BEST_EFFORT); + public static final QueueId ZERO = new QueueId(0); + public static final QueueId BEST_EFFORT = ZERO; + public static final int MIN = 1; + public static final int MAX = (1 << HDR_EGRESS_QID_BITWIDTH) - 1; private QueueId(int id) { super(id); @@ -28,7 +28,7 @@ private QueueId(int id) { * @throws IllegalArgumentException if given id is invalid */ public static QueueId of(int id) { - checkArgument(id >= MIN && id <= MAX, "Invalid id %s. Valid range is from %s to %s", id, 0, MAX); + checkArgument(id >= MIN && id <= MAX, "Invalid id %s. Valid range is from %s to %s", id, MIN, MAX); return new QueueId(id); } diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java index 771653dfa..d3ff40964 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java @@ -27,7 +27,6 @@ * "apps": { * "org.stratumproject.fabric-tna": { * "slicing": { - * "bestEffortQueueId": 0, * "slices": { * "0": { * "name": "Default", @@ -73,7 +72,6 @@ // TODO: javadoc public class SlicingConfig extends Config { private static final String SLICES = "slices"; - private static final String BEST_EFFORT_QUEUE_ID = "bestEffortQueueId"; private static final String TCS = "tcs"; private static final String NAME = "name"; private static final String QUEUE_ID = "queueId"; @@ -81,16 +79,13 @@ public class SlicingConfig extends Config { private static final String MAX_RATE_BPS = "maxRateBps"; private static final String GMIN_RATE_BPS = "gminRateBps"; - private static final int DEFAULT_BEST_EFFORT_QUEUE_ID = QueueId.BEST_EFFORT.id(); private static final long DEFAULT_MAX_RATE_BPS = TrafficClassDescription.UNLIMITED_BPS; private static final long DEFAULT_GMIN_RATE_BPS = 0; private static final boolean DEFAULT_IS_SYSTEM_TC = false; @Override public boolean isValid() { - if (!(hasOnlyFields(object, BEST_EFFORT_QUEUE_ID, SLICES) && - isIntegralNumber(object, BEST_EFFORT_QUEUE_ID, FieldPresence.OPTIONAL, - (long) QueueId.MIN, (long) QueueId.MAX))) { + if (!(hasOnlyFields(object, SLICES))) { return false; } @@ -146,11 +141,6 @@ public boolean isValid() { return true; } - public QueueId bestEffortQueueId() { - return QueueId.of(object.path(BEST_EFFORT_QUEUE_ID) - .asInt(DEFAULT_BEST_EFFORT_QUEUE_ID)); - } - public Collection slices() throws ConfigException { List sliceConfigs = Lists.newArrayList(); var jsonSlices = object.path(SLICES).fields(); @@ -186,6 +176,9 @@ public SliceDescription slice(SliceId sliceId) throws ConfigException { throw new ConfigException(format( "%s is not a valid traffic class for slice %s", tcName, sliceId), e); } + if (tc.equals(TrafficClass.BEST_EFFORT)) { + throw new ConfigException("BEST_EFFORT is implicit for all slices and cannot be configured"); + } tcDescriptions.put(tc, tcDescription(sliceId, tc)); } diff --git a/src/test/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfigTest.java b/src/test/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfigTest.java index 0003ab67f..8bc8c3bee 100644 --- a/src/test/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfigTest.java +++ b/src/test/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfigTest.java @@ -28,8 +28,6 @@ public void testConstruction() throws Exception { assertTrue(config.isValid()); - assertEquals(QueueId.of(0), config.bestEffortQueueId()); - assertEquals(3, config.slices().size()); assertNotNull(config.slice(SliceId.of(0))); assertNotNull(config.slice(SliceId.of(1))); diff --git a/src/test/resources/slicing.json b/src/test/resources/slicing.json index cf3d6138c..fed36338d 100644 --- a/src/test/resources/slicing.json +++ b/src/test/resources/slicing.json @@ -1,5 +1,4 @@ { - "bestEffortQueueId": 0, "slices": { "0": { "name": "Default", From f9869bd98534d76b9307ff6bc2d5b24cbe643536 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Wed, 19 Jan 2022 18:07:04 -0800 Subject: [PATCH 23/33] Use default for mobile slice --- .../fabric/tna/behaviour/upf/FabricUpfTranslator.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/behaviour/upf/FabricUpfTranslator.java b/src/main/java/org/stratumproject/fabric/tna/behaviour/upf/FabricUpfTranslator.java index 12cd3e01d..17d38c5d5 100644 --- a/src/main/java/org/stratumproject/fabric/tna/behaviour/upf/FabricUpfTranslator.java +++ b/src/main/java/org/stratumproject/fabric/tna/behaviour/upf/FabricUpfTranslator.java @@ -25,6 +25,7 @@ import org.onosproject.net.pi.runtime.PiAction; import org.onosproject.net.pi.runtime.PiActionParam; import org.onosproject.net.pi.runtime.PiTableAction; +import org.stratumproject.fabric.tna.slicing.api.SliceId; import java.util.ArrayList; import java.util.Arrays; @@ -599,8 +600,9 @@ public FlowRule interfaceToFabricEntry(UpfInterface upfInterface, DeviceId devic .build(); PiAction action = PiAction.builder() .withId(actionId) - // FIXME: slice id should come from UP4 - .withParameter(new PiActionParam(SLICE_ID, 99)) + // FIXME: Use a different slice id, provided by UP4 + // This will likely require updating UpfInterface. + .withParameter(new PiActionParam(SLICE_ID, SliceId.DEFAULT.id())) .build(); return DefaultFlowRule.builder() .forDevice(deviceId).fromApp(appId).makePermanent() From b7843c4abb284dfd409aee6e4057ce9028445c5e Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Wed, 19 Jan 2022 18:29:19 -0800 Subject: [PATCH 24/33] clean up rest API docs --- .../stratumproject/fabric/tna/web/SlicingWebResource.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/web/SlicingWebResource.java b/src/main/java/org/stratumproject/fabric/tna/web/SlicingWebResource.java index 76553b2cd..66beb7266 100644 --- a/src/main/java/org/stratumproject/fabric/tna/web/SlicingWebResource.java +++ b/src/main/java/org/stratumproject/fabric/tna/web/SlicingWebResource.java @@ -39,7 +39,6 @@ public class SlicingWebResource extends AbstractWebResource { /** * Get all slices currently programmed. - * Pre-defined slice IDs: Default Slice = 0, Mobile Traffic Slice = 15 * * @return 200 ok and a collection of Slice IDs */ @@ -58,7 +57,6 @@ public Response getSlice() { /** * Get all traffic class of a slice. - * Pre-defined slice IDs: Default Slice = 0, Mobile Traffic Slice = 15 * * @param sliceId ID of the slice * @return 200 ok and a collection of traffic class or 404 not found if the result is empty @@ -83,7 +81,6 @@ public Response getTc(@PathParam("sliceId") int sliceId) { /** * Get default traffic class given a slice. - * Pre-defined slice IDs: Default Slice = 0, Mobile Traffic Slice = 15 * * @param sliceId ID of the slice * @return 200 ok the default traffic class or 404 not found if the result is empty @@ -105,7 +102,6 @@ public Response getDefaultTc(@PathParam("sliceId") int sliceId) { /** * Set the default traffic class for a slice. - * Pre-defined slice IDs: Default Slice = 0, Mobile Traffic Slice = 15 * Traffic class values: CONTROL, REAL_TIME, ELASTIC, BEST_EFFORT * * @param sliceId ID of the slice @@ -131,7 +127,6 @@ public Response setDefaultTc(@PathParam("sliceId") int sliceId, @PathParam("tc") /** * Get classifier flows by slice ID and traffic class. - * Pre-defined slice IDs: Default Slice = 0, Mobile Traffic Slice = 15 * Traffic class values: CONTROL, REAL_TIME, ELASTIC, BEST_EFFORT * * @param sliceId ID of the slice @@ -152,7 +147,6 @@ public Response getFlow(@PathParam("sliceId") int sliceId, @PathParam("tc") Stri /** * Push a classifier flow to classify traffic as part of the given slice and traffic class. - * Pre-defined slice IDs: Default Slice = 0, Mobile Traffic Slice = 15 * Traffic class values: CONTROL, REAL_TIME, ELASTIC, BEST_EFFORT * Traffic Selector Example: * { @@ -198,7 +192,6 @@ public Response addFlow(@PathParam("sliceId") int sliceId, @PathParam("tc") Stri /** * Remove a classifier flow for the given slice and traffic class. - * Pre-defined slice IDs: Default Slice = 0, Mobile Traffic Slice = 15 * Traffic class values: CONTROL, REAL_TIME, ELASTIC, BEST_EFFORT * * @param sliceId ID of slice From c436d0d9f16663652bfd580fda278ce6a1eda287 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Wed, 19 Jan 2022 19:23:45 -0800 Subject: [PATCH 25/33] address review comments --- .../fabric/tna/slicing/SlicingManager.java | 36 ++++++++----------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java index 37555808f..bc1f1a190 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java @@ -180,7 +180,8 @@ protected void activate() { defaultTcStore.addListener(defaultTcListener); // Default slice is pre-provisioned. - sliceStore.put(new SliceStoreKey(SliceId.DEFAULT, TrafficClass.BEST_EFFORT), TrafficClassDescription.BEST_EFFORT); + sliceStore.put(new SliceStoreKey(SliceId.DEFAULT, TrafficClass.BEST_EFFORT), + TrafficClassDescription.BEST_EFFORT); defaultTcStore.put(SliceId.DEFAULT, TrafficClass.BEST_EFFORT); deviceListener = new InternalDeviceListener(); @@ -188,7 +189,6 @@ protected void activate() { "fabric-tna-device-event", "%d", log)); deviceService.addListener(deviceListener); - // FIXME: do we still need these? REST APIs for manipulating slices are gone. codecService.registerCodec(SliceId.class, new SliceIdCodec()); codecService.registerCodec(TrafficClass.class, new TrafficClassCodec()); @@ -362,22 +362,14 @@ public Set getTrafficClasses(SliceId sliceId) { @Override public boolean setDefaultTrafficClass(SliceId sliceId, TrafficClass tc) { - StringBuilder errorMessage = new StringBuilder(); - // FIXME: this is ugly and race prone. should we let the data plane - // handle inconsistencies? E.g. if a default TC has not been allocated, - // then the switch should pick best effort. - sliceStore.compute(new SliceStoreKey(sliceId, tc), (k, v) -> { - if (v == null) { - errorMessage.append(format("Cannot set %s as the default traffic class because it has not" + - " been allocated to slice %s", tc, sliceId)); - } else { - defaultTcStore.put(sliceId, tc); - } - return v; - }); - - if (errorMessage.length() != 0) { - throw new SlicingException(FAILED, errorMessage.toString()); + defaultTcStore.put(sliceId, tc); + + boolean exists = sliceStore.containsKey(new SliceStoreKey(sliceId, tc)); + if (!exists) { + log.warn("Default traffic class {} has not been allocated yet to slice {}, " + + "devices might forward packets as BEST_EFFORT until the " + + "traffic class is allocated", + tc, sliceId); } return true; @@ -591,16 +583,16 @@ public void event(MapEvent event) { case UPDATE: if (workPartitionService.isMine(event.newValue().value(), toStringHasher())) { deviceService.getAvailableDevices().forEach(device -> - addQueuesFlowRules(device.id(), - event.key().sliceId(), event.key().trafficClass(), event.newValue().value().queueId()) + addQueuesFlowRules(device.id(), event.key().sliceId(), + event.key().trafficClass(), event.newValue().value().queueId()) ); } break; case REMOVE: if (workPartitionService.isMine(event.oldValue().value(), toStringHasher())) { deviceService.getAvailableDevices().forEach(device -> - removeQueuesFlowRules(device.id(), - event.key().sliceId(), event.key().trafficClass(), event.oldValue().value().queueId()) + removeQueuesFlowRules(device.id(), event.key().sliceId(), + event.key().trafficClass(), event.oldValue().value().queueId()) ); } break; From dd216f453dc648b9c56bb298883b6dfc8dd386d1 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Wed, 19 Jan 2022 19:23:54 -0800 Subject: [PATCH 26/33] Fix test and checkstyles --- .../fabric/tna/slicing/SlicingManagerTest.java | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java b/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java index 29ae44289..cd7069b3a 100644 --- a/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java +++ b/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java @@ -355,15 +355,6 @@ public void testSetDefaultTrafficClass() { assertEquals(TrafficClass.CONTROL, manager.getDefaultTrafficClass(SLICE_IDS.get(1))); } - @Test - public void testSetDefaultTrafficClassExceptionNotAllocated() { - manager.addSlice(SLICE_IDS.get(1)); - - exceptionRule.expect(SlicingException.class); - exceptionRule.expectMessage("Cannot set CONTROL as the default traffic class because it has not been allocated to slice 1"); - manager.setDefaultTrafficClass(SLICE_IDS.get(1), TrafficClass.CONTROL); - } - @Test public void testRemoveTrafficClassExceptionDefaultTc() { manager.addTrafficClass(SLICE_IDS.get(0), TC_CONFIG_CONTROL); @@ -463,10 +454,10 @@ public void testSliceListener() { capturedAddedFlowRules.reset(); manager.addSlice(SLICE_IDS.get(1)); assertAfter(50, () -> { - assertEquals(3 * numDevices, capturedAddedFlowRules.getValues().size()); - assertEquals(2, capturedAddedFlowRules.getValues().stream() + assertEquals(2 * numDevices, capturedAddedFlowRules.getValues().size()); + assertEquals(numDevices, capturedAddedFlowRules.getValues().stream() .filter(flowRule -> flowRule.exactMatch(queuesFlowRuleSlice1BestEffort)).count()); - assertEquals(1, capturedAddedFlowRules.getValues().stream() + assertEquals(numDevices, capturedAddedFlowRules.getValues().stream() .filter(flowRule -> flowRule.exactMatch(defaultTcFlowRuleSlice1BestEffort)).count()); }); @@ -631,7 +622,7 @@ private void flowsPreCheck() { // activation stage, if the tests start immediately, the captured flows // may include slice flows (unexpceted). assertAfter(100, 250, () -> { - // Number of init flows are variant + // Number of init flows is variable. // For now we install 2 flows for each device assertEquals(2 * DEVICES.size(), capturedAddedFlowRules.getValues().size()); }); From e156a87ca016f888c34dd4b007c1efcc767a0a7c Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Thu, 20 Jan 2022 16:43:39 -0800 Subject: [PATCH 27/33] Move distributed store destroy to preDeactivate hook --- .../fabric/tna/slicing/SlicingManager.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java index bc1f1a190..00d41c0ea 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/SlicingManager.java @@ -139,7 +139,7 @@ public class SlicingManager implements SlicingService, SlicingProviderService, S @Activate protected void activate() { - appId = coreService.registerApplication(APP_NAME); + appId = coreService.registerApplication(APP_NAME, this::preDeactivate); KryoNamespace.Builder serializer = KryoNamespace.newBuilder() .register(KryoNamespaces.API) @@ -195,17 +195,21 @@ protected void activate() { log.info("Started"); } - @Deactivate - protected void deactivate() { + // Called only when we intentionally deactivate the app. + protected void preDeactivate() { sliceStore.removeListener(sliceListener); sliceStore.destroy(); - sliceExecutor.shutdown(); deviceService.removeListener(deviceListener); - deviceExecutor.shutdown(); defaultTcStore.removeListener(defaultTcListener); defaultTcStore.destroy(); + } + + @Deactivate + protected void deactivate() { + sliceExecutor.shutdown(); + deviceExecutor.shutdown(); defaultTcExecutor.shutdown(); // FIXME: clean up classifier flow rules and store From 52e0d255d44d21c0de820c5bf2785da31dabf1fc Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Thu, 20 Jan 2022 17:41:51 -0800 Subject: [PATCH 28/33] Update default tc javadoc --- .../fabric/tna/slicing/api/SlicingService.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingService.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingService.java index c0bc4dedb..af7796956 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingService.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingService.java @@ -27,8 +27,10 @@ public interface SlicingService { Set getTrafficClasses(SliceId sliceId); /** - * Sets a default traffic class that is applied to all unclassified traffic of a slice. - * The given traffic class must be already part of the slice, otherwise this will fail. + * Sets a default traffic class that is applied to all unclassified traffic + * of a slice. The implementation does not attempt to verify that the given + * traffic class has been already allocated for the given slice. If not + * allocated, the device is expected to forward traffic as best effort. * * @param sliceId slice identifier * @param tc traffic class From 42fc8ecc7a8ed0b06704e0cc934151868fe2b298 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Thu, 20 Jan 2022 23:12:47 -0800 Subject: [PATCH 29/33] javadoc --- .../tna/slicing/api/SliceDescription.java | 50 ++++++++++++++----- .../fabric/tna/slicing/api/SlicingConfig.java | 31 ++++++++++-- 2 files changed, 65 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SliceDescription.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SliceDescription.java index cc04a79b2..fa4eae734 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SliceDescription.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SliceDescription.java @@ -1,4 +1,4 @@ -// Copyright $today.year-present Open Networking Foundation +// Copyright 2022-present Open Networking Foundation // SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0 package org.stratumproject.fabric.tna.slicing.api; @@ -13,31 +13,59 @@ /** * Describes a slice. */ -// TODO: javadoc public class SliceDescription { private final SliceId id; private final String name; - private final ImmutableMap tcDescriptions; + private final ImmutableMap tcDescrs; - public SliceDescription(SliceId id, String name, Map tcDescriptions) { + /** + * Creates a new slice description. + * + * @param id slice ID + * @param name name + * @param tcDescrs traffic class descriptions + */ + public SliceDescription(SliceId id, String name, + Map tcDescrs) { this.id = id; - this.tcDescriptions = ImmutableMap.copyOf(tcDescriptions); + this.tcDescrs = ImmutableMap.copyOf(tcDescrs); this.name = name; } + /** + * Returns the slice ID. + * + * @return slice ID + */ public SliceId id() { return id; } + /** + * Returns the descriptions of the traffic classes within this slice. + * + * @return traffic class descriptions + */ public Collection tcDescriptions() { - return tcDescriptions.values(); + return tcDescrs.values(); } + /** + * Returns the description for the given traffic class. + * + * @param tc traffic class + * @return traffic class description + */ public TrafficClassDescription tcDescription(TrafficClass tc) { - return tcDescriptions.get(tc); + return tcDescrs.get(tc); } + /** + * Returns the name of this slice. + * + * @return slice name + */ public String name() { return name; } @@ -53,12 +81,12 @@ public boolean equals(Object o) { SliceDescription that = (SliceDescription) o; return Objects.equal(id, that.id) && Objects.equal(name, that.name) && - Objects.equal(tcDescriptions, that.tcDescriptions); + Objects.equal(tcDescrs, that.tcDescrs); } @Override public int hashCode() { - return Objects.hashCode(id, name, tcDescriptions); + return Objects.hashCode(id, name, tcDescrs); } @Override @@ -66,9 +94,7 @@ public String toString() { return MoreObjects.toStringHelper(this) .add("id", id) .add("name", name) - .add("tcs", tcDescriptions) + .add("tcs", tcDescrs) .toString(); } - - // TODO: builder } diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java index d3ff40964..a1abce4b3 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java @@ -28,7 +28,7 @@ * "org.stratumproject.fabric-tna": { * "slicing": { * "slices": { - * "0": { + * 0: { * "name": "Default", * "tcs": { * "REAL_TIME": { @@ -37,7 +37,7 @@ * } * } * }, - * "1": { + * 1: { * "name": "P4-UPF", * "tcs": { * "CONTROL": { @@ -54,7 +54,7 @@ * } * } * }, - * "3": { + * 2: { * "name": "BESS-UPF", * "tcs": { * "ELASTIC": { @@ -69,8 +69,8 @@ * } * */ -// TODO: javadoc public class SlicingConfig extends Config { + private static final String SLICES = "slices"; private static final String TCS = "tcs"; private static final String NAME = "name"; @@ -141,6 +141,12 @@ public boolean isValid() { return true; } + /** + * Returns the collection of slice descriptions defined in this config. + * + * @return collection of slice descriptions + * @throws ConfigException if the config is invalid + */ public Collection slices() throws ConfigException { List sliceConfigs = Lists.newArrayList(); var jsonSlices = object.path(SLICES).fields(); @@ -152,6 +158,14 @@ public Collection slices() throws ConfigException { return sliceConfigs; } + /** + * Returns the description of the specific slice with the given ID, or null + * if such slice is not defined in this config. + * + * @param sliceId slice ID + * @return slice description + * @throws ConfigException if the config is invalid + */ public SliceDescription slice(SliceId sliceId) throws ConfigException { var sliceNode = object.path(SLICES).path(sliceId.toString()); if (sliceNode.isMissingNode()) { @@ -185,6 +199,15 @@ public SliceDescription slice(SliceId sliceId) throws ConfigException { return new SliceDescription(sliceId, name, tcDescriptions); } + /** + * Returns the description of the given traffic class whitin the given + * slice, or null if missing from this config. + * + * @param sliceId slice ID + * @param tc traffic class + * @return traffic class description + * @throws ConfigException if the config is invalid + */ public TrafficClassDescription tcDescription(SliceId sliceId, TrafficClass tc) throws ConfigException { var tcNode = object.path(SLICES) .path(sliceId.toString()) From 00f6468c2bc2a5350ccbf39eb124d9e87edec28c Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Thu, 20 Jan 2022 23:13:45 -0800 Subject: [PATCH 30/33] review comments --- .../fabric/tna/slicing/api/SlicingConfig.java | 58 +++++++++++-------- .../slicing/api/TrafficClassDescription.java | 2 - 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java index a1abce4b3..fb815f652 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java @@ -109,33 +109,33 @@ public boolean isValid() { return false; } } + } - try { - var slices = slices(); - if (slices.isEmpty()) { - throw new InvalidFieldException(SLICES, "At least one slice should be specified"); - } + try { + var slices = slices(); + if (slices.isEmpty()) { + throw new InvalidFieldException(SLICES, "At least one slice should be specified"); + } - var systemTcsCount = 0; - for (SliceDescription sliceConfig : slices) { - for (TrafficClassDescription tcDescription : sliceConfig.tcDescriptions()) { - if (tcDescription.isSystemTc()) { - systemTcsCount++; - } + var systemTcsCount = 0; + for (SliceDescription sliceConfig : slices) { + for (TrafficClassDescription tcDescription : sliceConfig.tcDescriptions()) { + if (tcDescription.isSystemTc()) { + systemTcsCount++; } } - if (systemTcsCount == 0) { - throw new InvalidFieldException(SLICES, format( - "At least one traffic class should be set as the system one (%s=true)", - IS_SYSTEM_TC)); - } - if (systemTcsCount > 1) { - throw new InvalidFieldException(SLICES, - "Too many traffic classes are set as the system one, only one is allowed"); - } - } catch (ConfigException e) { - throw new InvalidFieldException(SLICES, e); } + if (systemTcsCount == 0) { + throw new InvalidFieldException(SLICES, format( + "At least one traffic class should be set as the system one (%s=true)", + IS_SYSTEM_TC)); + } + if (systemTcsCount > 1) { + throw new InvalidFieldException(SLICES, + "Too many traffic classes are set as the system one, only one is allowed"); + } + } catch (ConfigException e) { + throw new InvalidFieldException(SLICES, e); } return true; @@ -152,7 +152,15 @@ public Collection slices() throws ConfigException { var jsonSlices = object.path(SLICES).fields(); while (jsonSlices.hasNext()) { var jsonSlice = jsonSlices.next(); - var sliceId = SliceId.of(Integer.parseInt(jsonSlice.getKey())); + SliceId sliceId; + try { + sliceId = SliceId.of(Integer.parseInt(jsonSlice.getKey())); + } catch (IllegalArgumentException e) { + // This is catching also NumberFormatException (subclass of + // IllegalArgumentException) thrown by parseInt. + throw new ConfigException(format( + "\"%s\" is not a valid slice ID", jsonSlice.getKey()), e); + } sliceConfigs.add(slice(sliceId)); } return sliceConfigs; @@ -188,7 +196,7 @@ public SliceDescription slice(SliceId sliceId) throws ConfigException { tc = TrafficClass.valueOf(tcName); } catch (IllegalArgumentException e) { throw new ConfigException(format( - "%s is not a valid traffic class for slice %s", tcName, sliceId), e); + "\"%s\" is not a valid traffic class for slice %s", tcName, sliceId), e); } if (tc.equals(TrafficClass.BEST_EFFORT)) { throw new ConfigException("BEST_EFFORT is implicit for all slices and cannot be configured"); @@ -222,7 +230,7 @@ public TrafficClassDescription tcDescription(SliceId sliceId, TrafficClass tc) t queueId = QueueId.of(tcNode.path(QUEUE_ID).asInt()); } catch (IllegalArgumentException e) { throw new ConfigException(format( - "'%s' is not a valid queue ID for traffic class %s of slice %s", + "\"%s\" is not a valid queue ID for traffic class %s of slice %s", tcNode.path(QUEUE_ID).asText(), tc, sliceId), e); } diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassDescription.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassDescription.java index a520010f1..4dbabb042 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassDescription.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/TrafficClassDescription.java @@ -132,6 +132,4 @@ public String toString() { .add("isSystemTc", isSystemTc) .toString(); } - - // TODO: builder } From c5c58e043a6b1150064104c40868ae2bad43acf3 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Thu, 20 Jan 2022 23:13:53 -0800 Subject: [PATCH 31/33] tests --- .../tna/slicing/api/SlicingConfigTest.java | 82 ++++++++++++++++++- .../slicing-invalid-best-effort.json | 12 +++ src/test/resources/slicing-invalid-empty.json | 4 + .../slicing-invalid-no-system-tc.json | 22 +++++ .../slicing-invalid-queue-id-missing.json | 11 +++ .../resources/slicing-invalid-queue-id.json | 12 +++ .../resources/slicing-invalid-slice-id.json | 12 +++ .../slicing-invalid-too-many-system-tcs.json | 23 ++++++ .../slicing-invalid-traffic-class.json | 12 +++ src/test/resources/slicing.json | 2 +- 10 files changed, 190 insertions(+), 2 deletions(-) create mode 100644 src/test/resources/slicing-invalid-best-effort.json create mode 100644 src/test/resources/slicing-invalid-empty.json create mode 100644 src/test/resources/slicing-invalid-no-system-tc.json create mode 100644 src/test/resources/slicing-invalid-queue-id-missing.json create mode 100644 src/test/resources/slicing-invalid-queue-id.json create mode 100644 src/test/resources/slicing-invalid-slice-id.json create mode 100644 src/test/resources/slicing-invalid-too-many-system-tcs.json create mode 100644 src/test/resources/slicing-invalid-traffic-class.json diff --git a/src/test/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfigTest.java b/src/test/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfigTest.java index 8bc8c3bee..ada247bfe 100644 --- a/src/test/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfigTest.java +++ b/src/test/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfigTest.java @@ -3,9 +3,13 @@ package org.stratumproject.fabric.tna.slicing.api; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.onosproject.TestApplicationId; import org.onosproject.core.ApplicationId; +import org.onosproject.net.config.ConfigException; +import org.onosproject.net.config.InvalidFieldException; import org.stratumproject.fabric.tna.utils.TestUtils; import static org.junit.Assert.assertEquals; @@ -22,6 +26,9 @@ public class SlicingConfigTest { private static final String APP_NAME = "foobar"; private static final ApplicationId APP_ID = new TestApplicationId(APP_NAME); + @Rule + public ExpectedException exceptionRule = ExpectedException.none(); + @Test public void testConstruction() throws Exception { SlicingConfig config = TestUtils.getSlicingConfig(APP_ID, "/slicing.json"); @@ -87,4 +94,77 @@ public void testConstruction() throws Exception { assertEquals(0, tcDescr.gminRateBps()); assertFalse(tcDescr.isSystemTc()); } -} \ No newline at end of file + + @Test + public void testInvalidEmpty() { + SlicingConfig config = TestUtils.getSlicingConfig(APP_ID, "/slicing-invalid-empty.json"); + exceptionRule.expect(InvalidFieldException.class); + exceptionRule.expectMessage("At least one slice should be specified"); + config.isValid(); + } + + + @Test + public void testInvalidMissingSystemTc() { + SlicingConfig config = TestUtils.getSlicingConfig(APP_ID, "/slicing-invalid-no-system-tc.json"); + exceptionRule.expect(InvalidFieldException.class); + exceptionRule.expectMessage("At least one traffic class should be set as the system one"); + config.isValid(); + } + + @Test + public void testInvalidTooManySystemTcs() { + SlicingConfig config = TestUtils.getSlicingConfig(APP_ID, "/slicing-invalid-too-many-system-tcs.json"); + exceptionRule.expect(InvalidFieldException.class); + exceptionRule.expectMessage("Too many traffic classes are set as the system one"); + config.isValid(); + } + + @Test + public void testInvalidTrafficClass() { + SlicingConfig config = TestUtils.getSlicingConfig(APP_ID, "/slicing-invalid-traffic-class.json"); + exceptionRule.expect(InvalidFieldException.class); + exceptionRule.expectMessage("not a valid traffic class"); + config.isValid(); + } + + @Test + public void testInvalidSliceId() { + SlicingConfig config = TestUtils.getSlicingConfig(APP_ID, "/slicing-invalid-slice-id.json"); + exceptionRule.expect(InvalidFieldException.class); + exceptionRule.expectMessage("is not a valid slice ID"); + config.isValid(); + } + + @Test + public void testInvalidBestEffortQueueId() { + SlicingConfig config = TestUtils.getSlicingConfig(APP_ID, "/slicing-invalid-best-effort.json"); + exceptionRule.expect(InvalidFieldException.class); + exceptionRule.expectMessage("Field must be greater than 1"); + config.isValid(); + } + + @Test + public void testInvalidBestEffortTcName() throws ConfigException { + SlicingConfig config = TestUtils.getSlicingConfig(APP_ID, "/slicing-invalid-best-effort.json"); + exceptionRule.expect(ConfigException.class); + exceptionRule.expectMessage("BEST_EFFORT is implicit for all slices and cannot be configured"); + config.slice(SliceId.of(0)); + } + + @Test + public void testInvalidQueueId() throws ConfigException { + SlicingConfig config = TestUtils.getSlicingConfig(APP_ID, "/slicing-invalid-queue-id.json"); + exceptionRule.expect(ConfigException.class); + exceptionRule.expectMessage("is not a valid queue ID"); + config.slice(SliceId.of(0)); + } + + @Test + public void testInvalidQueueIdMissing() throws ConfigException { + SlicingConfig config = TestUtils.getSlicingConfig(APP_ID, "/slicing-invalid-queue-id-missing.json"); + exceptionRule.expect(InvalidFieldException.class); + exceptionRule.expectMessage("Field \"queueId\" is invalid: Mandatory field is not present"); + config.isValid(); + } +} diff --git a/src/test/resources/slicing-invalid-best-effort.json b/src/test/resources/slicing-invalid-best-effort.json new file mode 100644 index 000000000..1e082bb60 --- /dev/null +++ b/src/test/resources/slicing-invalid-best-effort.json @@ -0,0 +1,12 @@ +{ + "slices": { + "0": { + "name": "Default", + "tcs": { + "BEST_EFFORT": { + "queueId": 0 + } + } + } + } +} diff --git a/src/test/resources/slicing-invalid-empty.json b/src/test/resources/slicing-invalid-empty.json new file mode 100644 index 000000000..ee1bd2c29 --- /dev/null +++ b/src/test/resources/slicing-invalid-empty.json @@ -0,0 +1,4 @@ +{ + "slices": { + } +} diff --git a/src/test/resources/slicing-invalid-no-system-tc.json b/src/test/resources/slicing-invalid-no-system-tc.json new file mode 100644 index 000000000..9d254b663 --- /dev/null +++ b/src/test/resources/slicing-invalid-no-system-tc.json @@ -0,0 +1,22 @@ +{ + "slices": { + "0": { + "name": "Default", + "tcs": { + "REAL_TIME": { + "queueId": 1, + "isSystemTc": false + } + } + }, + "1": { + "name": "P4-UPF", + "tcs": { + "CONTROL": { + "queueId": 2, + "maxRateBps": "2000000" + } + } + } + } +} diff --git a/src/test/resources/slicing-invalid-queue-id-missing.json b/src/test/resources/slicing-invalid-queue-id-missing.json new file mode 100644 index 000000000..ec8ed6faa --- /dev/null +++ b/src/test/resources/slicing-invalid-queue-id-missing.json @@ -0,0 +1,11 @@ +{ + "slices": { + "0": { + "name": "Default", + "tcs": { + "REAL_TIME": { + } + } + } + } +} diff --git a/src/test/resources/slicing-invalid-queue-id.json b/src/test/resources/slicing-invalid-queue-id.json new file mode 100644 index 000000000..32a97d1b9 --- /dev/null +++ b/src/test/resources/slicing-invalid-queue-id.json @@ -0,0 +1,12 @@ +{ + "slices": { + "0": { + "name": "Default", + "tcs": { + "REAL_TIME": { + "queueId": "foo" + } + } + } + } +} diff --git a/src/test/resources/slicing-invalid-slice-id.json b/src/test/resources/slicing-invalid-slice-id.json new file mode 100644 index 000000000..31998915d --- /dev/null +++ b/src/test/resources/slicing-invalid-slice-id.json @@ -0,0 +1,12 @@ +{ + "slices": { + "foo": { + "name": "Default", + "tcs": { + "REAL_TIME": { + "queueId": 1 + } + } + } + } +} diff --git a/src/test/resources/slicing-invalid-too-many-system-tcs.json b/src/test/resources/slicing-invalid-too-many-system-tcs.json new file mode 100644 index 000000000..1dde8ec21 --- /dev/null +++ b/src/test/resources/slicing-invalid-too-many-system-tcs.json @@ -0,0 +1,23 @@ +{ + "slices": { + "0": { + "name": "Default", + "tcs": { + "REAL_TIME": { + "queueId": 1, + "isSystemTc": true + } + } + }, + "1": { + "name": "P4-UPF", + "tcs": { + "CONTROL": { + "queueId": 2, + "maxRateBps": "2000000", + "isSystemTc": true + } + } + } + } +} diff --git a/src/test/resources/slicing-invalid-traffic-class.json b/src/test/resources/slicing-invalid-traffic-class.json new file mode 100644 index 000000000..9e5d5210e --- /dev/null +++ b/src/test/resources/slicing-invalid-traffic-class.json @@ -0,0 +1,12 @@ +{ + "slices": { + "0": { + "name": "Default", + "tcs": { + "FOO": { + "queueId": "1" + } + } + } + } +} diff --git a/src/test/resources/slicing.json b/src/test/resources/slicing.json index fed36338d..9a38e464e 100644 --- a/src/test/resources/slicing.json +++ b/src/test/resources/slicing.json @@ -35,4 +35,4 @@ } } } -} \ No newline at end of file +} From 3309d3dcdbdafc3fc71bd4cc59387187ec233a66 Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Thu, 20 Jan 2022 23:24:41 -0800 Subject: [PATCH 32/33] Fix tests --- .../stratumproject/fabric/tna/slicing/SlicingManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java b/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java index cd7069b3a..12e751940 100644 --- a/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java +++ b/src/test/java/org/stratumproject/fabric/tna/slicing/SlicingManagerTest.java @@ -135,7 +135,7 @@ public void setup() { manager.networkCfgService = nwCfgService; manager.pipeconfService = pipeconfService; - EasyMock.expect(coreService.registerApplication(EasyMock.anyObject())).andReturn(APP_ID); + EasyMock.expect(coreService.registerApplication(EasyMock.anyObject(), EasyMock.anyObject())).andReturn(APP_ID); EasyMock.expect(storageService.consistentMapBuilder()).andReturn( new MockConsistentMap.Builder<>()); EasyMock.expect(storageService.consistentMapBuilder()).andReturn( From 3f0c995d5cd9da7b07970d2b2c5bbce0228fb0ba Mon Sep 17 00:00:00 2001 From: Carmelo Cascone Date: Thu, 20 Jan 2022 23:29:24 -0800 Subject: [PATCH 33/33] Restore double quotes in json example --- .../fabric/tna/slicing/api/SlicingConfig.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java index fb815f652..5a3e1123e 100644 --- a/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java +++ b/src/main/java/org/stratumproject/fabric/tna/slicing/api/SlicingConfig.java @@ -28,7 +28,7 @@ * "org.stratumproject.fabric-tna": { * "slicing": { * "slices": { - * 0: { + * "0": { * "name": "Default", * "tcs": { * "REAL_TIME": { @@ -37,7 +37,7 @@ * } * } * }, - * 1: { + * "1": { * "name": "P4-UPF", * "tcs": { * "CONTROL": { @@ -54,7 +54,7 @@ * } * } * }, - * 2: { + * "2": { * "name": "BESS-UPF", * "tcs": { * "ELASTIC": {