From 3ef744bf98aa67613f3cd2d5228a9dc3015ddee1 Mon Sep 17 00:00:00 2001 From: pierventre Date: Mon, 21 Mar 2022 22:51:26 +0100 Subject: [PATCH] [SDFAB-1136] Remove meter mirror There is no valid reason to have it in the p4rtmeterprogrammable: - some devices cannot guarantee r/w symmetry - meters can be only modified - removal is a modify with default values Change-Id: I6d859f2d65195f3e7068390fee5e3a943972cac5 --- .../p4runtime/P4RuntimeMeterProgrammable.java | 46 ++----------------- .../DistributedP4RuntimeMeterMirror.java | 41 ----------------- 2 files changed, 4 insertions(+), 83 deletions(-) delete mode 100644 drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/mirror/DistributedP4RuntimeMeterMirror.java 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 8bb4fc7ad4..2ebf7b2120 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 @@ -19,8 +19,6 @@ package org.onosproject.drivers.p4runtime; 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.DefaultMeter; @@ -63,7 +61,6 @@ public class P4RuntimeMeterProgrammable extends AbstractP4RuntimeHandlerBehaviou private static final Striped WRITE_LOCKS = Striped.lock(30); private PiMeterTranslator translator; - private P4RuntimeMeterMirror meterMirror; private PiPipelineModel pipelineModel; @Override @@ -73,7 +70,6 @@ public class P4RuntimeMeterProgrammable extends AbstractP4RuntimeHandlerBehaviou } translator = translationService.meterTranslator(); - meterMirror = handler().get(P4RuntimeMeterMirror.class); pipelineModel = pipeconf.pipelineModel(); return true; } @@ -120,13 +116,10 @@ public class P4RuntimeMeterProgrammable extends AbstractP4RuntimeHandlerBehaviou return false; } - WriteRequest request = client.write(p4DeviceId, pipeconf); - appendEntryToWriteRequestOrSkip(request, handle, piMeterCellConfig); + WriteRequest request = client.write(p4DeviceId, pipeconf) + .entity(piMeterCellConfig, UpdateType.MODIFY); if (!request.pendingUpdates().isEmpty()) { result = request.submitSync().isSuccess(); - if (result) { - meterMirror.applyWriteRequest(request); - } } } finally { WRITE_LOCKS.get(deviceId).unlock(); @@ -154,8 +147,6 @@ public class P4RuntimeMeterProgrammable extends AbstractP4RuntimeHandlerBehaviou .filter(piMeterCellConfig -> !piMeterCellConfig.isDefaultConfig()) .collect(Collectors.toList()); - meterMirror.sync(deviceId, piMeterCellConfigs); - if (piMeterCellConfigs.isEmpty()) { return CompletableFuture.completedFuture(Collections.emptyList()); } @@ -179,11 +170,8 @@ public class P4RuntimeMeterProgrammable extends AbstractP4RuntimeHandlerBehaviou // Reset all inconsistent meter cells to the default config 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)); - } - + inconsistentOrDefaultCells.forEach(cellId -> + request.entity(PiMeterCellConfig.reset(cellId), UpdateType.MODIFY)); request.submit().whenComplete((response, ex) -> { if (ex != null) { log.error("Exception resetting inconsistent meter entries", ex); @@ -191,7 +179,6 @@ public class P4RuntimeMeterProgrammable extends AbstractP4RuntimeHandlerBehaviou log.debug("Successfully removed {} out of {} inconsistent meter entries", response.success().size(), response.all().size()); } - response.success().forEach(entity -> meterMirror.remove((PiMeterCellHandle) entity.handle())); }); } @@ -215,7 +202,6 @@ public class P4RuntimeMeterProgrammable extends AbstractP4RuntimeHandlerBehaviou private Meter forgeMeter(PiMeterCellConfig config, PiMeterCellHandle handle) { final Optional> translatedEntity = translator.lookup(handle); - final TimedEntry timedEntry = meterMirror.get(handle); // A meter cell config might not be present in the translation store if it // is default configuration. @@ -239,13 +225,6 @@ public class P4RuntimeMeterProgrammable extends AbstractP4RuntimeHandlerBehaviou return null; } - // Big problems in the mirror ? This could happen in case of issues with - // the eventual consistent maps used in the AbstractDistributedP4RuntimeMirror - if (timedEntry == null) { - log.warn("Meter entry handle not found in device mirror: {}", handle); - return null; - } - Meter original = translatedEntity.get().original(); // Forge a meter with MeterCellId, Bands and DeviceId using the original value. DefaultMeter meter = (DefaultMeter) DefaultMeter.builder() @@ -257,23 +236,6 @@ public class P4RuntimeMeterProgrammable extends AbstractP4RuntimeHandlerBehaviou return meter; } - private boolean appendEntryToWriteRequestOrSkip( - final WriteRequest writeRequest, - final PiMeterCellHandle handle, - PiMeterCellConfig configToModify) { - - final TimedEntry 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; - } - /** * Returns true if the given PiMeterCellConfigs are similar enough to be deemed equal * for reconciliation purposes. This is required to handle read/write asymmetry in devices diff --git a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/mirror/DistributedP4RuntimeMeterMirror.java b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/mirror/DistributedP4RuntimeMeterMirror.java deleted file mode 100644 index 48546fc74b..0000000000 --- a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/mirror/DistributedP4RuntimeMeterMirror.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright 2017-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.drivers.p4runtime.mirror; - -import org.onosproject.net.pi.runtime.PiEntityType; -import org.onosproject.net.pi.runtime.PiMeterCellConfig; -import org.onosproject.net.pi.runtime.PiMeterCellHandle; -import org.osgi.service.component.annotations.Component; - -/** - * Distributed implementation of a P4Runtime meter mirror. - */ -@Component(immediate = true, service = P4RuntimeMeterMirror.class) -public final class DistributedP4RuntimeMeterMirror - extends AbstractDistributedP4RuntimeMirror - - implements P4RuntimeMeterMirror { - - public DistributedP4RuntimeMeterMirror() { - super(PiEntityType.METER_CELL_CONFIG); - } - - @Override - protected String mapSimpleName() { - return PiEntityType.METER_CELL_CONFIG.name().toLowerCase(); - } -}