[SDFAB-500][SDFAB-527] Meters cleanup and leftovers
- Improve ONOS cli enabling CRUD of p4rt trtcm
- Improve ONOS rest enabling CRUD of p4rt trtcm
- Improve MeterService with scope defined reads and integrate in cli/rest
- Add support along the stack for BYTE_PER_SEC unit
- Add support along the stack for COMMITTED and PEAK bands
- Fix several bugs in ONOS cli/rest interfaces
- Improve REST codecs
- Fix NPE in MeterDriverProvider
- Improve PiMeterTransalation by enforcing trtcm config
- Implement explicit translation of the bands
- Fix ONOS reconciliation by removing from the mirror the wrong configs
- Remove unnecessary checks in MeterEntryCodec
- Update unit tests
It will follow a 2nd patch to complete SDFAB-527
Change-Id: I855235b17f60cb1d39f5b9a042c1015105a8a269
diff --git a/core/api/src/main/java/org/onosproject/net/meter/Band.java b/core/api/src/main/java/org/onosproject/net/meter/Band.java
index 9f9479c..befbead 100644
--- a/core/api/src/main/java/org/onosproject/net/meter/Band.java
+++ b/core/api/src/main/java/org/onosproject/net/meter/Band.java
@@ -51,8 +51,27 @@
* Defines a meter band with no action, used to mark
* packets internally in the pipeline, i.e. without
* modifying the packet headers.
+ *
+ * @deprecated in onos-2.5, replace by MARK_YELLOW and MARK_RED
*/
+ @Deprecated
NONE,
+
+ /**
+ * Defines a meter band for the configuration of the committed
+ * rate AND the committed burst size. Used in conjunction with MARK_RED
+ * to implement a srTCM or trTCM, see RFCs 2697 and 2698 respectively.
+ */
+ MARK_YELLOW,
+
+ /**
+ * Defines a meter band for the configuration of the peak rate
+ * AND the peak burst size OR the excess burst size. When used to
+ * configure a srTCM excess rate must be 0. Used in conjunction with
+ * MARK_YELLOW to implement a srTCM or trTCM, see RFCs 2697 and 2698
+ * respectively.
+ */
+ MARK_RED,
}
/**
diff --git a/core/api/src/main/java/org/onosproject/net/meter/DefaultMeterRequest.java b/core/api/src/main/java/org/onosproject/net/meter/DefaultMeterRequest.java
index 0dbd4f7..617151d 100644
--- a/core/api/src/main/java/org/onosproject/net/meter/DefaultMeterRequest.java
+++ b/core/api/src/main/java/org/onosproject/net/meter/DefaultMeterRequest.java
@@ -33,8 +33,6 @@
*/
public final class DefaultMeterRequest extends AbstractAnnotated implements MeterRequest {
-
-
private final ApplicationId appId;
private final Meter.Unit unit;
private final boolean burst;
@@ -122,6 +120,8 @@
}
public static final class Builder implements MeterRequest.Builder {
+ // Relevant only for delete
+ private static final Band DUMMY_BAND = new DefaultBand(Band.Type.DROP, 0L, 0L, (short) 0);
private ApplicationId appId;
private Meter.Unit unit = Meter.Unit.KB_PER_SEC;
@@ -196,6 +196,11 @@
@Override
public MeterRequest remove() {
+ // Allow to create the removal request without specifying bands
+ if (bands == null || bands.isEmpty()) {
+ bands = ImmutableSet.of(DUMMY_BAND);
+ }
+
validate();
return new DefaultMeterRequest(deviceId, appId, unit, burst, bands, context,
Type.REMOVE, scope, desiredIndex, annotations);
diff --git a/core/api/src/main/java/org/onosproject/net/meter/MeterService.java b/core/api/src/main/java/org/onosproject/net/meter/MeterService.java
index 5b3ca3b..2d37f87 100644
--- a/core/api/src/main/java/org/onosproject/net/meter/MeterService.java
+++ b/core/api/src/main/java/org/onosproject/net/meter/MeterService.java
@@ -91,6 +91,15 @@
Collection<Meter> getMeters(DeviceId deviceId);
/**
+ * Fetches the meters by the device id and scope.
+ *
+ * @param deviceId a device id
+ * @param scope meters scope
+ * @return a collection of meters
+ */
+ Collection<Meter> getMeters(DeviceId deviceId, MeterScope scope);
+
+ /**
* Allocates a new meter id in the system.
*
* @param deviceId the device id
diff --git a/core/api/src/main/java/org/onosproject/net/meter/MeterStore.java b/core/api/src/main/java/org/onosproject/net/meter/MeterStore.java
index 7f4de4b..4abf233 100644
--- a/core/api/src/main/java/org/onosproject/net/meter/MeterStore.java
+++ b/core/api/src/main/java/org/onosproject/net/meter/MeterStore.java
@@ -118,6 +118,17 @@
Collection<Meter> getAllMeters(DeviceId deviceId);
/**
+ * Returns all meters stored in the store for a
+ * precise device and scope.
+ *
+ * @param deviceId a device id
+ * @param scope meters scope
+ * @return an immutable copy of the meters stored for a given device
+ * withing a given scope
+ */
+ Collection<Meter> getAllMeters(DeviceId deviceId, MeterScope scope);
+
+ /**
* Update the store by deleting the failed meter.
* Notifies the delegate that the meter failed to allow it
* to nofity the app.
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterBand.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterBand.java
index b1dbcf5..ea8c34d 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterBand.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterBand.java
@@ -27,21 +27,33 @@
*/
@Beta
public final class PiMeterBand {
+ private final PiMeterBandType type;
private final long rate;
private final long burst;
/**
* Creates a band with rate and burst.
*
+ * @param type type of this band
* @param rate rate of this band
* @param burst burst of this band
*/
- public PiMeterBand(long rate, long burst) {
+ public PiMeterBand(PiMeterBandType type, long rate, long burst) {
+ this.type = type;
this.rate = rate;
this.burst = burst;
}
/**
+ * Returns the type of this band.
+ *
+ * @return type of this band
+ */
+ public PiMeterBandType type() {
+ return type;
+ }
+
+ /**
* Returns the rate of this band.
*
* @return rate of this band
@@ -61,7 +73,7 @@
@Override
public int hashCode() {
- return Objects.hash(rate, burst);
+ return Objects.hash(type, rate, burst);
}
@Override
@@ -71,7 +83,8 @@
}
if (obj instanceof PiMeterBand) {
PiMeterBand that = (PiMeterBand) obj;
- return Objects.equals(rate, that.rate) &&
+ return Objects.equals(type, that.type) &&
+ Objects.equals(rate, that.rate) &&
Objects.equals(burst, that.burst);
}
@@ -80,6 +93,7 @@
public String toString() {
return toStringHelper(this)
+ .add("type", type)
.add("rate", rate)
.add("burst", burst).toString();
}
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterBandType.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterBandType.java
new file mode 100644
index 0000000..ee15a57
--- /dev/null
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterBandType.java
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2021-present Open Networking Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.onosproject.net.pi.runtime;
+
+import com.google.common.annotations.Beta;
+
+/**
+ * Type of meter band of a protocol-independent pipeline.
+ */
+@Beta
+public enum PiMeterBandType {
+ /**
+ * Committed band config.
+ */
+ COMMITTED("committed"),
+
+ /**
+ * Peak band config.
+ */
+ PEAK("peak");
+
+ private final String humanReadableName;
+
+ PiMeterBandType(String humanReadableName) {
+ this.humanReadableName = humanReadableName;
+ }
+
+ /**
+ * Returns a human readable representation of this PI meter band type (useful
+ * for logging).
+ *
+ * @return string
+ */
+ public String humanReadableName() {
+ return humanReadableName;
+ }
+}
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterCellConfig.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterCellConfig.java
index d3da340..ecbfe07 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterCellConfig.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterCellConfig.java
@@ -19,13 +19,12 @@
import com.google.common.annotations.Beta;
import com.google.common.base.MoreObjects;
import com.google.common.base.Objects;
-import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
import org.onosproject.net.DeviceId;
-import java.util.ArrayList;
-import java.util.Collection;
import java.util.Collections;
-import java.util.List;
+import java.util.Map;
import static com.google.common.base.Preconditions.checkNotNull;
@@ -36,7 +35,7 @@
public final class PiMeterCellConfig implements PiEntity {
private final PiMeterCellId cellId;
- private final ImmutableList<PiMeterBand> piMeterBands;
+ private final ImmutableMap<PiMeterBandType, PiMeterBand> piMeterBands;
/**
* Creates a new meter cell configuration for the given cell identifier and meter bands.
@@ -44,9 +43,9 @@
* @param cellId meter cell identifier
* @param piMeterBands meter bands
*/
- private PiMeterCellConfig(PiMeterCellId cellId, Collection<PiMeterBand> piMeterBands) {
+ private PiMeterCellConfig(PiMeterCellId cellId, Map<PiMeterBandType, PiMeterBand> piMeterBands) {
this.cellId = cellId;
- this.piMeterBands = ImmutableList.copyOf(piMeterBands);
+ this.piMeterBands = ImmutableMap.copyOf(piMeterBands);
}
/**
@@ -59,11 +58,11 @@
}
/**
- * Returns the collection of bands of this cell.
+ * Returns the map of bands of this cell.
*
* @return meter bands
*/
- public Collection<PiMeterBand> meterBands() {
+ public Map<PiMeterBandType, PiMeterBand> meterBands() {
return piMeterBands;
}
@@ -88,6 +87,24 @@
}
/**
+ * Returns the committed configuration if present.
+ *
+ * @return the committed band. Null otherwise
+ */
+ public PiMeterBand committedBand() {
+ return piMeterBands.get(PiMeterBandType.COMMITTED);
+ }
+
+ /**
+ * Returns the peak configuration if present.
+ *
+ * @return the peak band. Null otherwise
+ */
+ public PiMeterBand peakBand() {
+ return piMeterBands.get(PiMeterBandType.PEAK);
+ }
+
+ /**
* Returns a PiMeterCellConfig with no bands.
* Used to reset a PI meter cell.
*
@@ -95,7 +112,7 @@
* @return a PiMeterCellConfig with no bands
*/
public static PiMeterCellConfig reset(PiMeterCellId piMeterCellId) {
- return new PiMeterCellConfig(piMeterCellId, Collections.emptyList());
+ return new PiMeterCellConfig(piMeterCellId, Collections.emptyMap());
}
@Override
@@ -118,8 +135,7 @@
}
PiMeterCellConfig that = (PiMeterCellConfig) o;
- return piMeterBands.containsAll((that.piMeterBands)) &&
- piMeterBands.size() == that.piMeterBands.size() &&
+ return Objects.equal(piMeterBands, that.piMeterBands) &&
Objects.equal(cellId, that.cellId);
}
@@ -146,8 +162,8 @@
}
public static final class Builder {
- private PiMeterCellId cellId;
- private List<PiMeterBand> bands = new ArrayList<>();
+ private PiMeterCellId cellId;
+ private Map<PiMeterBandType, PiMeterBand> bands = Maps.newHashMap();
private Builder() {
@@ -173,7 +189,31 @@
* @return this
*/
public PiMeterCellConfig.Builder withMeterBand(PiMeterBand band) {
- this.bands.add(band);
+ this.bands.put(band.type(), band);
+ return this;
+ }
+
+ /**
+ * Sets the committed band of this meter.
+ *
+ * @param rate committed rate
+ * @param burst committed burst
+ * @return this
+ */
+ public PiMeterCellConfig.Builder withCommittedBand(long rate, long burst) {
+ this.bands.put(PiMeterBandType.COMMITTED, new PiMeterBand(PiMeterBandType.COMMITTED, rate, burst));
+ return this;
+ }
+
+ /**
+ * Sets the peak band of this meter.
+ *
+ * @param rate peak rate
+ * @param burst peak burst
+ * @return this
+ */
+ public PiMeterCellConfig.Builder withPeakBand(long rate, long burst) {
+ this.bands.put(PiMeterBandType.PEAK, new PiMeterBand(PiMeterBandType.PEAK, rate, burst));
return this;
}
diff --git a/core/api/src/test/java/org/onosproject/net/meter/MeterServiceAdapter.java b/core/api/src/test/java/org/onosproject/net/meter/MeterServiceAdapter.java
index 2a298b4..86795e9 100644
--- a/core/api/src/test/java/org/onosproject/net/meter/MeterServiceAdapter.java
+++ b/core/api/src/test/java/org/onosproject/net/meter/MeterServiceAdapter.java
@@ -61,6 +61,11 @@
}
@Override
+ public Collection<Meter> getMeters(DeviceId deviceId, MeterScope scope) {
+ return null;
+ }
+
+ @Override
public MeterId allocateMeterId(DeviceId deviceId) {
return null;
}