[SDFAB-354] Improve P4RTMeterProgrammable
Change-Id: I65a325f90a49853c6c4a1cfb8212a016a8ec2b2d
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 e62482b..d3da340 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
@@ -69,19 +69,21 @@
/**
* Check if the config represents a modify operation.
+ * Or it is a non-default config read from south bound.
*
* @return true if there are exactly 2 bands
*/
- public boolean isModify() {
+ public boolean isModifyConfig() {
return piMeterBands.size() == 2;
}
/**
* Check if the config represents a reset operation.
+ * Or it is a default config read from south bound.
*
- * @return true if there is no band.
+ * @return true if there is no band
*/
- public boolean isReset() {
+ public boolean isDefaultConfig() {
return piMeterBands.isEmpty();
}
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 caaa751..a7dbadd 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
@@ -16,11 +16,11 @@
package org.onosproject.drivers.p4runtime;
-import com.google.common.cache.CacheBuilder;
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
+import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
+import com.google.common.util.concurrent.Striped;
import org.onosproject.drivers.p4runtime.mirror.P4RuntimeMeterMirror;
+import org.onosproject.drivers.p4runtime.mirror.TimedEntry;
import org.onosproject.net.DeviceId;
import org.onosproject.net.meter.Band;
import org.onosproject.net.meter.DefaultBand;
@@ -39,38 +39,33 @@
import org.onosproject.net.pi.runtime.PiMeterCellHandle;
import org.onosproject.net.pi.runtime.PiMeterCellId;
import org.onosproject.net.pi.service.PiMeterTranslator;
+import org.onosproject.net.pi.service.PiTranslatedEntity;
import org.onosproject.net.pi.service.PiTranslationException;
+import org.onosproject.p4runtime.api.P4RuntimeWriteClient.WriteRequest;
+import org.onosproject.p4runtime.api.P4RuntimeWriteClient.WriteResponse;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
+import java.util.List;
+import java.util.Optional;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
import java.util.stream.Collectors;
import static com.google.common.base.Preconditions.checkNotNull;
import static org.onosproject.net.meter.MeterOperation.Type.ADD;
import static org.onosproject.net.meter.MeterOperation.Type.MODIFY;
import static org.onosproject.net.meter.MeterOperation.Type.REMOVE;
+import static org.onosproject.p4runtime.api.P4RuntimeWriteClient.UpdateType;
/**
* Implementation of MeterProgrammable behaviour for P4Runtime.
*/
public class P4RuntimeMeterProgrammable extends AbstractP4RuntimeHandlerBehaviour implements MeterProgrammable {
- private static final int METER_LOCK_EXPIRE_TIME_IN_MIN = 10;
- private static final LoadingCache<PiMeterCellHandle, Lock>
- ENTRY_LOCKS = CacheBuilder.newBuilder()
- .expireAfterAccess(METER_LOCK_EXPIRE_TIME_IN_MIN, TimeUnit.MINUTES)
- .build(new CacheLoader<PiMeterCellHandle, Lock>() {
- @Override
- public Lock load(PiMeterCellHandle handle) {
- return new ReentrantLock();
- }
- });
+ private static final Striped<Lock> WRITE_LOCKS = Striped.lock(30);
private PiMeterTranslator translator;
private P4RuntimeMeterMirror meterMirror;
@@ -100,37 +95,47 @@
private boolean processMeterOp(MeterOperation meterOp) {
PiMeterCellConfig piMeterCellConfig;
- switch (meterOp.type()) {
- case ADD:
- case MODIFY:
- // Create a config for modify operation
- try {
- piMeterCellConfig = translator.translate(meterOp.meter(), pipeconf);
- } catch (PiTranslationException e) {
- log.warn("Unable translate meter, aborting meter operation {}: {}", meterOp.type(), e.getMessage());
- log.debug("exception", e);
+ final PiMeterCellHandle handle = PiMeterCellHandle.of(deviceId,
+ (PiMeterCellId) meterOp.meter().meterCellId());
+ boolean result = true;
+ WRITE_LOCKS.get(deviceId).lock();
+ try {
+ switch (meterOp.type()) {
+ case ADD:
+ case MODIFY:
+ // Create a config for modify operation
+ try {
+ piMeterCellConfig = translator.translate(meterOp.meter(), pipeconf);
+ } catch (PiTranslationException e) {
+ log.warn("Unable translate meter, aborting meter operation {}: {}",
+ meterOp.type(), e.getMessage());
+ log.debug("exception", e);
+ return false;
+ }
+ translator.learn(handle, new PiTranslatedEntity<>(meterOp.meter(), piMeterCellConfig, handle));
+ break;
+ case REMOVE:
+ // Create a empty config for reset operation
+ PiMeterCellId piMeterCellId = (PiMeterCellId) meterOp.meter().meterCellId();
+ piMeterCellConfig = PiMeterCellConfig.reset(piMeterCellId);
+ translator.forget(handle);
+ break;
+ default:
+ log.warn("Meter Operation type {} not supported", meterOp.type());
return false;
+ }
+
+ WriteRequest request = client.write(p4DeviceId, pipeconf);
+ appendEntryToWriteRequestOrSkip(request, handle, piMeterCellConfig);
+ if (!request.pendingUpdates().isEmpty()) {
+ result = request.submitSync().isSuccess();
+ if (result) {
+ meterMirror.applyWriteRequest(request);
}
- break;
- case REMOVE:
- // Create a empty config for reset operation
- PiMeterCellId piMeterCellId = (PiMeterCellId) meterOp.meter().meterCellId();
- piMeterCellConfig = PiMeterCellConfig.reset(piMeterCellId);
- break;
- default:
- log.warn("Meter Operation type {} not supported", meterOp.type());
- return false;
+ }
+ } finally {
+ WRITE_LOCKS.get(deviceId).unlock();
}
-
- final PiMeterCellHandle handle = PiMeterCellHandle.of(deviceId, piMeterCellConfig);
- ENTRY_LOCKS.getUnchecked(handle).lock();
- final boolean result = client.write(p4DeviceId, pipeconf)
- .modify(piMeterCellConfig).submitSync().isSuccess();
- if (result) {
- meterMirror.put(handle, piMeterCellConfig);
- }
- ENTRY_LOCKS.getUnchecked(handle).unlock();
-
return result;
}
@@ -151,19 +156,38 @@
piMeterCellConfigs = client.read(p4DeviceId, pipeconf)
.meterCells(meterIds).submitSync().all(PiMeterCellConfig.class);
- Collection<Meter> meters = piMeterCellConfigs.stream()
- .map(p -> {
- DefaultMeter meter = (DefaultMeter) DefaultMeter.builder()
- .withBands(p.meterBands().stream().map(b -> DefaultBand.builder()
- .withRate(b.rate())
- .burstSize(b.burst())
- .ofType(Band.Type.NONE)
- .build()).collect(Collectors.toList()))
- .withCellId(p.cellId()).build();
- meter.setState(MeterState.ADDED);
- return meter;
- })
- .collect(Collectors.toList());
+ meterMirror.sync(deviceId, piMeterCellConfigs);
+
+ if (piMeterCellConfigs.isEmpty()) {
+ return CompletableFuture.completedFuture(Collections.emptyList());
+ }
+
+ List<PiMeterCellId> inconsistentOrDefaultCells = Lists.newArrayList();
+ List<Meter> meters = Lists.newArrayList();
+
+ // Check the consistency of meter config
+ for (PiMeterCellConfig config : piMeterCellConfigs) {
+ PiMeterCellHandle handle = PiMeterCellHandle.of(deviceId, config);
+ DefaultMeter meter = (DefaultMeter) forgeMeter(config, handle);
+ if (meter == null) {
+ // A default config cannot be used to forge meter
+ // because meter has at least 1 band while default config has no band
+ inconsistentOrDefaultCells.add(config.cellId());
+ } else {
+ meters.add(meter);
+ }
+ }
+
+ // Reset all inconsistent meter cells to default state
+ if (!inconsistentOrDefaultCells.isEmpty()) {
+ WriteRequest request = client.write(p4DeviceId, pipeconf);
+ for (PiMeterCellId cellId : inconsistentOrDefaultCells) {
+ PiMeterCellHandle handle = PiMeterCellHandle.of(deviceId, cellId);
+ appendEntryToWriteRequestOrSkip(request, handle, PiMeterCellConfig.reset(cellId));
+ }
+ WriteResponse response = request.submitSync();
+ meterMirror.applyWriteResponse(response);
+ }
return CompletableFuture.completedFuture(meters);
}
@@ -182,6 +206,67 @@
return CompletableFuture.completedFuture(meterFeatures);
}
+ private Meter forgeMeter(PiMeterCellConfig config, PiMeterCellHandle handle) {
+ final Optional<PiTranslatedEntity<Meter, PiMeterCellConfig>>
+ translatedEntity = translator.lookup(handle);
+ final TimedEntry<PiMeterCellConfig> timedEntry = meterMirror.get(handle);
+
+ // A meter cell config might not be present in the translation store if it
+ // is default configuration.
+ if (translatedEntity.isEmpty()) {
+ if (!config.isDefaultConfig()) {
+ log.warn("Meter Cell Config obtained from device {} is different from " +
+ "one in in translation store: device={}, store=Default", deviceId, config);
+ } else {
+ log.debug("Configs obtained from device: {} and present in the store are default, " +
+ "skipping the forge section");
+ }
+ return null;
+ }
+ // The config is not consistent
+ if (!translatedEntity.get().translated().equals(config)) {
+ log.warn("Meter Cell Config obtained from device {} is different from " +
+ "one in in translation store: device={}, store={}",
+ deviceId, config, translatedEntity.get().translated());
+ return null;
+ }
+ if (timedEntry == null) {
+ log.warn("Meter entry handle not found in device mirror: {}", handle);
+ return null;
+ }
+
+ // Forge a meter with MeterCellId, Bands and DeviceId
+ // Other values are not required because we cannot retrieve them from the south
+ DefaultMeter meter = (DefaultMeter) DefaultMeter.builder()
+ .withBands(config.meterBands().stream().map(b -> DefaultBand.builder()
+ .withRate(b.rate())
+ .burstSize(b.burst())
+ .ofType(Band.Type.NONE)
+ .build()).collect(Collectors.toList()))
+ .withCellId(config.cellId())
+ .forDevice(deviceId)
+ .build();
+ meter.setState(MeterState.ADDED);
+ return meter;
+ }
+
+ private boolean appendEntryToWriteRequestOrSkip(
+ final WriteRequest writeRequest,
+ final PiMeterCellHandle handle,
+ PiMeterCellConfig configToModify) {
+
+ final TimedEntry<PiMeterCellConfig> configOnDevice = meterMirror.get(handle);
+
+ if (configOnDevice != null && configOnDevice.entry().equals(configToModify)) {
+ log.debug("Ignoring re-apply of existing entry: {}", configToModify);
+ return true;
+ }
+
+ writeRequest.entity(configToModify, UpdateType.MODIFY);
+
+ return false;
+ }
+
/**
* P4 meter features builder.
*/
diff --git a/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/MeterEntryCodec.java b/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/MeterEntryCodec.java
index 2da12d1..f5e0f41 100644
--- a/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/MeterEntryCodec.java
+++ b/protocols/p4runtime/utils/src/main/java/org/onosproject/p4runtime/ctl/codec/MeterEntryCodec.java
@@ -34,12 +34,12 @@
static P4RuntimeOuterClass.MeterConfig getP4Config(PiMeterCellConfig piConfig)
throws CodecException {
- // A reset config has no band
- if (piConfig.isReset()) {
+ // The config has no band, we don't have to create a P4RT meter config
+ if (piConfig.isDefaultConfig()) {
return null;
}
- // A modify config has exactly 2 bands
- if (!piConfig.isModify()) {
+ // If it is not a reset operation, the config must be a modify config and has exactly 2 bands
+ if (!piConfig.isModifyConfig()) {
throw new CodecException("Number of meter bands should be 2 (Modify) or 0 (Reset)");
}
final PiMeterBand[] bands = piConfig.meterBands().toArray(new PiMeterBand[0]);