[SDFAB-188] Remove buffer drainer from UpfProgrammable
Change-Id: Id10d8b41d203b4af99867d169255a63fe99b25a0
(cherry picked from commit ac94678fb6204b76f44feef3e6098b18c84359bd)
diff --git a/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/DistributedFabricUpfStore.java b/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/DistributedFabricUpfStore.java
index 1f40f3b..02923c2 100644
--- a/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/DistributedFabricUpfStore.java
+++ b/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/DistributedFabricUpfStore.java
@@ -19,13 +19,10 @@
import com.google.common.collect.BiMap;
import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.Maps;
-import org.onlab.packet.Ip4Address;
import org.onlab.util.ImmutableByteSequence;
import org.onlab.util.KryoNamespace;
-import org.onosproject.net.behaviour.upf.PacketDetectionRule;
import org.onosproject.store.serializers.KryoNamespaces;
import org.onosproject.store.service.ConsistentMap;
-import org.onosproject.store.service.DistributedSet;
import org.onosproject.store.service.MapEvent;
import org.onosproject.store.service.MapEventListener;
import org.onosproject.store.service.Serializer;
@@ -38,12 +35,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
-import java.util.Set;
-
-import static com.google.common.base.Preconditions.checkNotNull;
/**
* Distributed implementation of FabricUpfStore.
@@ -58,8 +51,6 @@
protected StorageService storageService;
protected static final String FAR_ID_MAP_NAME = "fabric-upf-far-id";
- protected static final String BUFFER_FAR_ID_SET_NAME = "fabric-upf-buffer-far-id";
- protected static final String FAR_ID_UE_MAP_NAME = "fabric-upf-far-id-ue";
protected static final KryoNamespace.Builder SERIALIZER = KryoNamespace.newBuilder()
.register(KryoNamespaces.API)
.register(UpfRuleIdentifier.class);
@@ -83,9 +74,6 @@
protected Map<Integer, UpfRuleIdentifier> reverseFarIdMap;
private int nextGlobalFarId = 1;
- protected DistributedSet<UpfRuleIdentifier> bufferFarIds;
- protected ConsistentMap<UpfRuleIdentifier, Set<Ip4Address>> farIdToUeAddrs;
-
@Activate
protected void activate() {
// Allow unit test to inject farIdMap here.
@@ -95,17 +83,6 @@
.withRelaxedReadConsistency()
.withSerializer(Serializer.using(SERIALIZER.build()))
.build();
- this.bufferFarIds = storageService.<UpfRuleIdentifier>setBuilder()
- .withName(BUFFER_FAR_ID_SET_NAME)
- .withRelaxedReadConsistency()
- .withSerializer(Serializer.using(SERIALIZER.build()))
- .build().asDistributedSet();
- this.farIdToUeAddrs = storageService.<UpfRuleIdentifier, Set<Ip4Address>>consistentMapBuilder()
- .withName(FAR_ID_UE_MAP_NAME)
- .withRelaxedReadConsistency()
- .withSerializer(Serializer.using(SERIALIZER.build()))
- .build();
-
}
farIdMapListener = new FarIdMapListener();
farIdMap.addListener(farIdMapListener);
@@ -129,8 +106,6 @@
public void reset() {
farIdMap.clear();
reverseFarIdMap.clear();
- bufferFarIds.clear();
- farIdToUeAddrs.clear();
nextGlobalFarId = 0;
}
@@ -171,59 +146,6 @@
return reverseFarIdMap.get(globalFarId);
}
- public void learnFarIdToUeAddrs(PacketDetectionRule pdr) {
- UpfRuleIdentifier ruleId = UpfRuleIdentifier.of(pdr.sessionId(), pdr.farId());
- farIdToUeAddrs.compute(ruleId, (k, set) -> {
- if (set == null) {
- set = new HashSet<>();
- }
- set.add(pdr.ueAddress());
- return set;
- });
- }
-
- @Override
- public boolean isFarIdBuffering(UpfRuleIdentifier farId) {
- checkNotNull(farId);
- return bufferFarIds.contains(farId);
- }
-
- @Override
- public void learBufferingFarId(UpfRuleIdentifier farId) {
- checkNotNull(farId);
- bufferFarIds.add(farId);
- }
-
- @Override
- public void forgetBufferingFarId(UpfRuleIdentifier farId) {
- checkNotNull(farId);
- bufferFarIds.remove(farId);
- }
-
- @Override
- public void forgetUeAddr(Ip4Address ueAddr) {
- farIdToUeAddrs.keySet().forEach(
- farId -> farIdToUeAddrs.computeIfPresent(farId, (farIdz, ueAddrs) -> {
- ueAddrs.remove(ueAddr);
- return ueAddrs;
- }));
- }
-
- @Override
- public Set<Ip4Address> ueAddrsOfFarId(UpfRuleIdentifier farId) {
- return farIdToUeAddrs.getOrDefault(farId, Set.of()).value();
- }
-
- @Override
- public Set<UpfRuleIdentifier> getBufferFarIds() {
- return Set.copyOf(bufferFarIds);
- }
-
- @Override
- public Map<UpfRuleIdentifier, Set<Ip4Address>> getFarIdToUeAddrs() {
- return Map.copyOf(farIdToUeAddrs.asJavaMap());
- }
-
// NOTE: FarIdMapListener is run on the same thread intentionally in order to ensure that
// reverseFarIdMap update always finishes right after farIdMap is updated
private class FarIdMapListener implements MapEventListener<UpfRuleIdentifier, Integer> {
diff --git a/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/FabricUpfProgrammable.java b/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/FabricUpfProgrammable.java
index 77ad732..61345c4 100644
--- a/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/FabricUpfProgrammable.java
+++ b/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/FabricUpfProgrammable.java
@@ -18,14 +18,12 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
-import org.onlab.packet.Ip4Address;
import org.onlab.packet.Ip4Prefix;
import org.onosproject.core.ApplicationId;
import org.onosproject.core.CoreService;
import org.onosproject.drivers.p4runtime.AbstractP4RuntimeHandlerBehaviour;
import org.onosproject.net.PortNumber;
import org.onosproject.net.behaviour.upf.ForwardingActionRule;
-import org.onosproject.net.behaviour.upf.GtpTunnel;
import org.onosproject.net.behaviour.upf.PacketDetectionRule;
import org.onosproject.net.behaviour.upf.PdrStats;
import org.onosproject.net.behaviour.upf.UpfInterface;
@@ -99,15 +97,6 @@
private ApplicationId appId;
- // FIXME: remove, buffer drain should be triggered by Up4Service
- private BufferDrainer bufferDrainer;
-
- // FIXME: dbuf tunnel should be managed by Up4Service
- // Up4Service should be responsible of setting up such tunnel, then transforming FARs for this
- // device accordingly. When the tunnel endpoint change, it should be up to Up4Service to update
- // the FAR on the device.
- private GtpTunnel dbufTunnel;
-
@Override
protected boolean setupBehaviour(String opName) {
if (!super.setupBehaviour(opName)) {
@@ -201,22 +190,6 @@
}
@Override
- public void setBufferDrainer(BufferDrainer drainer) {
- if (!setupBehaviour("setBufferDrainer()")) {
- return;
- }
- this.bufferDrainer = drainer;
- }
-
- @Override
- public void unsetBufferDrainer() {
- if (!setupBehaviour("unsetBufferDrainer()")) {
- return;
- }
- this.bufferDrainer = null;
- }
-
- @Override
public void enablePscEncap(int defaultQfi) throws UpfProgrammableException {
throw new UpfProgrammableException("PSC encap is not supported in fabric-v1model",
UNSUPPORTED_OPERATION);
@@ -244,46 +217,6 @@
}
@Override
- public void setDbufTunnel(Ip4Address switchAddr, Ip4Address dbufAddr) {
- if (!setupBehaviour("setDbufTunnel()")) {
- return;
- }
- this.dbufTunnel = GtpTunnel.builder()
- .setSrc(switchAddr)
- .setDst(dbufAddr)
- .setSrcPort((short) 2152)
- .setTeid(0)
- .build();
- }
-
- @Override
- public void unsetDbufTunnel() {
- if (!setupBehaviour("unsetDbufTunnel()")) {
- return;
- }
- this.dbufTunnel = null;
- }
-
- /**
- * Convert the given buffering FAR to a FAR that tunnels the packet to dbuf.
- *
- * @param far the FAR to convert
- * @return the converted FAR
- */
- private ForwardingActionRule convertToDbufFar(ForwardingActionRule far) {
- if (!far.buffers()) {
- throw new IllegalArgumentException("Converting a non-buffering FAR to a dbuf FAR! This shouldn't happen.");
- }
- return ForwardingActionRule.builder()
- .setFarId(far.farId())
- .withSessionId(far.sessionId())
- .setNotifyFlag(far.notifies())
- .setBufferFlag(true)
- .setTunnel(dbufTunnel)
- .build();
- }
-
- @Override
public void cleanUp() {
if (!setupBehaviour("cleanUp()")) {
return;
@@ -475,11 +408,6 @@
log.info("Installing {}", pdr.toString());
flowRuleService.applyFlowRules(fabricPdr);
log.debug("PDR added with flowID {}", fabricPdr.id().value());
-
- // If the flow rule was applied and the PDR is downlink, add the PDR to the farID->PDR mapping
- if (pdr.matchesUnencapped()) {
- fabricUpfStore.learnFarIdToUeAddrs(pdr);
- }
}
@@ -488,28 +416,10 @@
if (!setupBehaviour("addFar()")) {
return;
}
- UpfRuleIdentifier ruleId = UpfRuleIdentifier.of(far.sessionId(), far.farId());
- if (far.buffers()) {
- // If the far has the buffer flag, modify its tunnel so it directs to dbuf
- far = convertToDbufFar(far);
- fabricUpfStore.learBufferingFarId(ruleId);
- }
FlowRule fabricFar = upfTranslator.farToFabricEntry(far, deviceId, appId, DEFAULT_PRIORITY);
log.info("Installing {}", far.toString());
flowRuleService.applyFlowRules(fabricFar);
log.debug("FAR added with flowID {}", fabricFar.id().value());
- if (!far.buffers() && fabricUpfStore.isFarIdBuffering(ruleId)) {
- // If this FAR does not buffer but used to, then drain the buffer for every UE address
- // that hits this FAR.
- fabricUpfStore.forgetBufferingFarId(ruleId);
- for (var ueAddr : fabricUpfStore.ueAddrsOfFarId(ruleId)) {
- if (bufferDrainer == null) {
- log.warn("Unable to drain downlink buffer for UE {}, bufferDrainer is null", ueAddr);
- } else {
- bufferDrainer.drain(ueAddr);
- }
- }
- }
}
@Override
@@ -620,13 +530,6 @@
}
log.info("Removing {}", pdr.toString());
removeEntry(match, tableId, false);
-
- // Remove the PDR from the farID->PDR mapping
- // This is an inefficient hotfix FIXME: remove UE addrs from the mapping in sublinear time
- if (pdr.matchesUnencapped()) {
- // Should we remove just from the map entry with key == far ID?
- fabricUpfStore.forgetUeAddr(pdr.ueAddress());
- }
}
@Override
diff --git a/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/FabricUpfStore.java b/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/FabricUpfStore.java
index 786ef3c..9b5d853 100644
--- a/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/FabricUpfStore.java
+++ b/pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/upf/FabricUpfStore.java
@@ -16,12 +16,9 @@
package org.onosproject.pipelines.fabric.impl.behaviour.upf;
-import org.onlab.packet.Ip4Address;
import org.onlab.util.ImmutableByteSequence;
-import org.onosproject.net.behaviour.upf.PacketDetectionRule;
import java.util.Map;
-import java.util.Set;
/**
* Stores state required for translation of UPF entities to pipeline-specific ones.
@@ -80,62 +77,6 @@
*
* @param queueId Tofino queue Id
* @return the corresponding scheduling priroity
- */
+ */
String schedulingPriorityOf(int queueId);
-
- /**
- * Stores the mapping between FAR ID and UE address as defined by the given PDR.
- *
- * @param pdr PDR
- */
- void learnFarIdToUeAddrs(PacketDetectionRule pdr);
-
- /**
- * Returns true if the given FAR IDs is known to be a buffering one.
- *
- * @param farId FAR ID
- * @return boolean
- */
- boolean isFarIdBuffering(UpfRuleIdentifier farId);
-
- /**
- * Learns the given FAR ID as being a buffering one.
- *
- * @param farId FAR ID
- */
- void learBufferingFarId(UpfRuleIdentifier farId);
-
- /**
- * Forgets the given FAR ID as being a buffering one.
- *
- * @param farId FAR ID
- */
- void forgetBufferingFarId(UpfRuleIdentifier farId);
-
- /**
- * Returns the set of UE addresses associated with the given FAR ID.
- *
- * @param farId FAR ID
- * @return Set of Ip4Address
- */
- Set<Ip4Address> ueAddrsOfFarId(UpfRuleIdentifier farId);
-
- /**
- * Removes the given UE address from the FAR ID to UE address map.
- * @param ueAddr UE address
- */
- void forgetUeAddr(Ip4Address ueAddr);
-
- /**
- * Returns the set of known buffering FAR IDs.
- * @return set
- */
- Set<UpfRuleIdentifier> getBufferFarIds();
-
- /**
- * Returns the FAR ID to UE addresses map.
- *
- * @return map
- */
- Map<UpfRuleIdentifier, Set<Ip4Address>> getFarIdToUeAddrs();
}