[SDFAB-557] Fix max meter in ONOS
Change-Id: I17f1b760fb4c16e3c0daa0ed0a4e54009b5e9aaf
diff --git a/core/api/src/main/java/org/onosproject/net/meter/DefaultMeterFeatures.java b/core/api/src/main/java/org/onosproject/net/meter/DefaultMeterFeatures.java
index 3806250..b9a4368 100644
--- a/core/api/src/main/java/org/onosproject/net/meter/DefaultMeterFeatures.java
+++ b/core/api/src/main/java/org/onosproject/net/meter/DefaultMeterFeatures.java
@@ -66,8 +66,11 @@
@Override
public long maxMeter() {
- // For OpenFlow meter, return end index as maxMeter
- return scope.isGlobal() ? endIndex + 1 : endIndex - startIndex + 1;
+ long maxMeter = 0;
+ if (startIndex != -1 && endIndex != -1) {
+ maxMeter = endIndex - startIndex + 1;
+ }
+ return maxMeter;
}
@Override
@@ -243,7 +246,7 @@
// 0, for the rest (A P4RT meter)
if (mmeter != 0L && starti == -1L && endi == -1L) {
starti = mscope.isGlobal() ? 1 : 0;
- endi = mmeter - 1;
+ endi = mscope.isGlobal() ? mmeter : mmeter - 1;
}
// If one of the index is unset/unvalid value, treated as no meter features
if (starti <= -1 || endi <= -1) {
diff --git a/core/api/src/test/java/org/onosproject/net/meter/DefaultMeterFeaturesTest.java b/core/api/src/test/java/org/onosproject/net/meter/DefaultMeterFeaturesTest.java
new file mode 100644
index 0000000..5cd0d1a
--- /dev/null
+++ b/core/api/src/test/java/org/onosproject/net/meter/DefaultMeterFeaturesTest.java
@@ -0,0 +1,99 @@
+/*
+ * 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.meter;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.onosproject.net.DeviceId;
+
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * DefaultMeterFeatures Tests.
+ */
+public class DefaultMeterFeaturesTest {
+
+ private MeterFeatures mf;
+ private DeviceId did = DeviceId.deviceId("foo:foo");
+ private short band = 2;
+ private short color = 3;
+ private Set<Band.Type> types;
+ private Set<Meter.Unit> units;
+ private MeterScope globalScope = MeterScope.globalScope();
+ private MeterScope fooScope = MeterScope.of("foo");
+
+ @Before
+ public void setup() {
+ types = Set.of(Band.Type.DROP);
+ units = Set.of(Meter.Unit.KB_PER_SEC);
+ }
+
+ @Test
+ public void testZeroMaxMeter() {
+ mf = DefaultMeterFeatures.builder()
+ .forDevice(did)
+ .withMaxMeters(0L)
+ .withScope(globalScope)
+ .withMaxBands(band)
+ .withMaxColors(color)
+ .withBandTypes(types)
+ .withUnits(units)
+ .hasBurst(true)
+ .hasStats(true).build();
+
+ assertEquals(-1, mf.startIndex());
+ assertEquals(-1, mf.endIndex());
+ assertEquals(0L, mf.maxMeter());
+ }
+
+ @Test
+ public void testOfMaxMeter() {
+ mf = DefaultMeterFeatures.builder()
+ .forDevice(did)
+ .withMaxMeters(1024L)
+ .withScope(globalScope)
+ .withMaxBands(band)
+ .withMaxColors(color)
+ .withBandTypes(types)
+ .withUnits(units)
+ .hasBurst(true)
+ .hasStats(true).build();
+
+ assertEquals(1L, mf.startIndex());
+ assertEquals(1024L, mf.endIndex());
+ assertEquals(1024L, mf.maxMeter());
+ }
+
+ @Test
+ public void testNonOfMaxMeter() {
+ mf = DefaultMeterFeatures.builder()
+ .forDevice(did)
+ .withMaxMeters(1024L)
+ .withScope(fooScope)
+ .withMaxBands(band)
+ .withMaxColors(color)
+ .withBandTypes(types)
+ .withUnits(units)
+ .hasBurst(true)
+ .hasStats(true).build();
+
+ assertEquals(0L, mf.startIndex());
+ assertEquals(1023L, mf.endIndex());
+ assertEquals(1024L, mf.maxMeter());
+ }
+}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/meter/impl/DistributedMeterStore.java b/core/store/dist/src/main/java/org/onosproject/store/meter/impl/DistributedMeterStore.java
index 7d8c7a5..5b9252d 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/meter/impl/DistributedMeterStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/meter/impl/DistributedMeterStore.java
@@ -627,9 +627,9 @@
if (maxMeters == 0L) {
return null;
} else {
- // OpenFlow meter index starts from 1, ends with max-1
+ // OpenFlow meter index starts from 1, ends with max
startIndex = 1L;
- endIndex = maxMeters - 1;
+ endIndex = maxMeters;
}
}
// Get a new value
diff --git a/core/store/dist/src/test/java/org/onosproject/store/meter/impl/DistributedMeterStoreTest.java b/core/store/dist/src/test/java/org/onosproject/store/meter/impl/DistributedMeterStoreTest.java
index 37ff7b4..5268527 100644
--- a/core/store/dist/src/test/java/org/onosproject/store/meter/impl/DistributedMeterStoreTest.java
+++ b/core/store/dist/src/test/java/org/onosproject/store/meter/impl/DistributedMeterStoreTest.java
@@ -330,6 +330,8 @@
assertThat(mid1, is(meterStore.allocateMeterId(did1)));
// Reserve id 2
assertThat(mid2, is(meterStore.allocateMeterId(did1)));
+ // Reserve id 3
+ assertThat(mid3, is(meterStore.allocateMeterId(did1)));
// Max meter error
assertNull(meterStore.allocateMeterId(did1));
}
diff --git a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeMeterProgrammable.java b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeMeterProgrammable.java
index 5165783..b903fa3 100644
--- a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeMeterProgrammable.java
+++ b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeMeterProgrammable.java
@@ -314,9 +314,13 @@
// The scope value will be PiMeterId
.withScope(MeterScope.of(piMeterModel.id().id()))
.withMaxBands(PI_METER_MAX_BAND)
- .withMaxColors(PI_METER_MAX_COLOR)
- .withStartIndex(PI_METER_START_INDEX)
- .withEndIndex(piMeterModel.size() - 1);
+ .withMaxColors(PI_METER_MAX_COLOR);
+
+ // We extract the number of supported meters.
+ if (piMeterModel.size() > 0) {
+ builder.withStartIndex(PI_METER_START_INDEX)
+ .withEndIndex(piMeterModel.size() - 1);
+ }
// p4rt meters support MARK_YELLOW (committed config) and
// MARK_RED (peak config) band types.
diff --git a/providers/openflow/meter/src/main/java/org/onosproject/provider/of/meter/util/MeterFeaturesBuilder.java b/providers/openflow/meter/src/main/java/org/onosproject/provider/of/meter/util/MeterFeaturesBuilder.java
index 4fc61aa..9e7963d 100644
--- a/providers/openflow/meter/src/main/java/org/onosproject/provider/of/meter/util/MeterFeaturesBuilder.java
+++ b/providers/openflow/meter/src/main/java/org/onosproject/provider/of/meter/util/MeterFeaturesBuilder.java
@@ -73,9 +73,15 @@
MeterFeatures.Builder builder = DefaultMeterFeatures.builder()
.forDevice(deviceId)
.withMaxBands(ofMeterFeatures.getMaxBands())
- .withMaxColors(ofMeterFeatures.getMaxColor())
- .withStartIndex(OF_METER_START_INDEX)
- .withEndIndex(ofMeterFeatures.getMaxMeter());
+ .withMaxColors(ofMeterFeatures.getMaxColor());
+
+ /*
+ * We extract the number of supported meters.
+ */
+ if (ofMeterFeatures.getMaxMeter() > 0) {
+ builder.withStartIndex(OF_METER_START_INDEX)
+ .withEndIndex(ofMeterFeatures.getMaxMeter());
+ }
/*
* We extract the supported band types.
*/