Improved consistency for BMv2 flow rules handling
Change-Id: I3a4798af3f35f135e8162385a1bf7fc059028307
diff --git a/drivers/bmv2/src/main/java/org/onosproject/drivers/bmv2/Bmv2FlowRuleProgrammable.java b/drivers/bmv2/src/main/java/org/onosproject/drivers/bmv2/Bmv2FlowRuleProgrammable.java
index 27c3fc2..403f68c 100644
--- a/drivers/bmv2/src/main/java/org/onosproject/drivers/bmv2/Bmv2FlowRuleProgrammable.java
+++ b/drivers/bmv2/src/main/java/org/onosproject/drivers/bmv2/Bmv2FlowRuleProgrammable.java
@@ -24,6 +24,7 @@
import org.onosproject.bmv2.api.context.Bmv2FlowRuleTranslator;
import org.onosproject.bmv2.api.context.Bmv2FlowRuleTranslatorException;
import org.onosproject.bmv2.api.context.Bmv2Interpreter;
+import org.onosproject.bmv2.api.context.Bmv2TableModel;
import org.onosproject.bmv2.api.runtime.Bmv2DeviceAgent;
import org.onosproject.bmv2.api.runtime.Bmv2FlowRuleWrapper;
import org.onosproject.bmv2.api.runtime.Bmv2MatchKey;
@@ -112,10 +113,10 @@
List<FlowEntry> entryList = Lists.newArrayList();
- configuration.tables().forEach(table -> {
+ for (Bmv2TableModel table : configuration.tables()) {
// For each table in the configuration AND exposed by the interpreter.
if (!interpreter.tableIdMap().inverse().containsKey(table.name())) {
- return;
+ continue; // next table
}
List<Bmv2ParsedTableEntry> installedEntries;
@@ -123,33 +124,34 @@
installedEntries = deviceAgent.getTableEntries(table.name());
} catch (Bmv2RuntimeException e) {
log.warn("Failed to get table entries of table {} of {}: {}", table.name(), deviceId, e.explain());
- return;
+ continue; // next table
}
- installedEntries.forEach(parsedEntry -> {
- Bmv2TableEntryReference entryRef = new Bmv2TableEntryReference(deviceId,
- table.name(),
+ for (Bmv2ParsedTableEntry parsedEntry : installedEntries) {
+ Bmv2TableEntryReference entryRef = new Bmv2TableEntryReference(deviceId, table.name(),
parsedEntry.matchKey());
- ENTRY_LOCKS.compute(entryRef, (key, value) -> {
- Bmv2FlowRuleWrapper frWrapper = tableEntryService.lookupEntryReference(entryRef);
+ ENTRY_LOCKS.putIfAbsent(entryRef, true);
+ synchronized (ENTRY_LOCKS.get(entryRef)) {
+
+ Bmv2FlowRuleWrapper frWrapper = tableEntryService.lookup(entryRef);
if (frWrapper == null) {
log.warn("missing reference from table entry service, BUG? " +
"deviceId={}, tableName={}, matchKey={}",
deviceId, table.name(), entryRef.matchKey());
- return null;
+ continue; // next entry
}
long remoteEntryId = parsedEntry.entryId();
long localEntryId = frWrapper.entryId();
if (remoteEntryId != localEntryId) {
- log.warn("getFlowEntries(): inconsistent entry id! BUG? Updating it... remote={}, local={}",
+ log.debug("getFlowEntries(): inconsistent entry id! BUG? Updating it... remote={}, local={}",
remoteEntryId, localEntryId);
frWrapper = new Bmv2FlowRuleWrapper(frWrapper.rule(), remoteEntryId,
frWrapper.creationDate());
- tableEntryService.bindEntryReference(entryRef, frWrapper);
+ tableEntryService.bind(entryRef, frWrapper);
}
long bytes = 0L;
@@ -171,11 +173,9 @@
FlowEntry entry = new DefaultFlowEntry(frWrapper.rule(), ADDED, frWrapper.lifeInSeconds(),
packets, bytes);
entryList.add(entry);
- return true;
- });
-
- });
- });
+ }
+ }
+ }
return Collections.unmodifiableCollection(entryList);
}
@@ -226,19 +226,16 @@
bmv2Entry = translator.translate(rule, context);
} catch (Bmv2FlowRuleTranslatorException e) {
log.warn("Unable to translate flow rule: {} - {}", e.getMessage(), rule);
- continue;
+ continue; // next rule
}
String tableName = bmv2Entry.tableName();
Bmv2TableEntryReference entryRef = new Bmv2TableEntryReference(deviceId, tableName, bmv2Entry.matchKey());
- /*
- From here on threads are synchronized over entryKey, i.e. serialize operations
- over the same matchKey of a specific table and device.
- */
- ENTRY_LOCKS.compute(entryRef, (key, value) -> {
+ ENTRY_LOCKS.putIfAbsent(entryRef, true);
+ synchronized (ENTRY_LOCKS.get(entryRef)) {
// Get from store
- Bmv2FlowRuleWrapper frWrapper = tableEntryService.lookupEntryReference(entryRef);
+ Bmv2FlowRuleWrapper frWrapper = tableEntryService.lookup(entryRef);
try {
if (operation == Operation.APPLY) {
// Apply entry
@@ -269,15 +266,14 @@
} catch (Bmv2RuntimeException e) {
log.warn("Unable to {} flow rule: {}", operation.name(), e.explain());
}
- // Update binding in table entry service.
+
+ // Update entryRef binding in table entry service.
if (frWrapper != null) {
- tableEntryService.bindEntryReference(entryRef, frWrapper);
- return true;
+ tableEntryService.bind(entryRef, frWrapper);
} else {
- tableEntryService.unbindEntryReference(entryRef);
- return null;
+ tableEntryService.unbind(entryRef);
}
- });
+ }
}
return processedFlowRules;
@@ -287,7 +283,7 @@
try {
return agent.addTableEntry(entry);
} catch (Bmv2RuntimeException e) {
- if (e.getCode() != TABLE_DUPLICATE_ENTRY) {
+ if (e.getCode().equals(TABLE_DUPLICATE_ENTRY)) {
forceRemove(agent, entry.tableName(), entry.matchKey());
return agent.addTableEntry(entry);
} else {
@@ -301,7 +297,7 @@
try {
agent.deleteTableEntry(tableName, entryId);
} catch (Bmv2RuntimeException e) {
- if (e.getCode() == TABLE_INVALID_HANDLE || e.getCode() == TABLE_EXPIRED_HANDLE) {
+ if (e.getCode().equals(TABLE_INVALID_HANDLE) || e.getCode().equals(TABLE_EXPIRED_HANDLE)) {
// entry is not there with the declared ID, try with a forced remove.
forceRemove(agent, tableName, matchKey);
} else {
diff --git a/protocols/bmv2/api/src/main/java/org/onosproject/bmv2/api/service/Bmv2TableEntryService.java b/protocols/bmv2/api/src/main/java/org/onosproject/bmv2/api/service/Bmv2TableEntryService.java
index cd25ee5..5c5ee38 100644
--- a/protocols/bmv2/api/src/main/java/org/onosproject/bmv2/api/service/Bmv2TableEntryService.java
+++ b/protocols/bmv2/api/src/main/java/org/onosproject/bmv2/api/service/Bmv2TableEntryService.java
@@ -21,6 +21,7 @@
import org.onosproject.bmv2.api.context.Bmv2FlowRuleTranslator;
import org.onosproject.bmv2.api.runtime.Bmv2FlowRuleWrapper;
import org.onosproject.bmv2.api.runtime.Bmv2TableEntryReference;
+import org.onosproject.net.DeviceId;
/**
* A service for managing BMv2 table entries.
@@ -41,7 +42,7 @@
* @param entryRef a table entry reference
* @param rule a BMv2 flow rule wrapper
*/
- void bindEntryReference(Bmv2TableEntryReference entryRef, Bmv2FlowRuleWrapper rule);
+ void bind(Bmv2TableEntryReference entryRef, Bmv2FlowRuleWrapper rule);
/**
* Returns the ONOS flow rule associated with the given BMv2 table entry reference, or null if there's no such a
@@ -50,12 +51,19 @@
* @param entryRef a table entry reference
* @return a BMv2 flow rule wrapper
*/
- Bmv2FlowRuleWrapper lookupEntryReference(Bmv2TableEntryReference entryRef);
+ Bmv2FlowRuleWrapper lookup(Bmv2TableEntryReference entryRef);
/**
* Removes any flow rule previously bound with a given BMv2 table entry reference.
*
* @param entryRef a table entry reference
*/
- void unbindEntryReference(Bmv2TableEntryReference entryRef);
+ void unbind(Bmv2TableEntryReference entryRef);
+
+ /**
+ * Removes all bindings for a given device.
+ *
+ * @param deviceId a device ID
+ */
+ void unbindAll(DeviceId deviceId);
}
diff --git a/protocols/bmv2/ctl/src/main/java/org/onosproject/bmv2/ctl/Bmv2TableEntryServiceImpl.java b/protocols/bmv2/ctl/src/main/java/org/onosproject/bmv2/ctl/Bmv2TableEntryServiceImpl.java
index 2afb99f..38f627d 100644
--- a/protocols/bmv2/ctl/src/main/java/org/onosproject/bmv2/ctl/Bmv2TableEntryServiceImpl.java
+++ b/protocols/bmv2/ctl/src/main/java/org/onosproject/bmv2/ctl/Bmv2TableEntryServiceImpl.java
@@ -34,6 +34,7 @@
import org.onosproject.bmv2.api.service.Bmv2Controller;
import org.onosproject.bmv2.api.service.Bmv2DeviceContextService;
import org.onosproject.bmv2.api.service.Bmv2TableEntryService;
+import org.onosproject.net.DeviceId;
import org.onosproject.store.serializers.KryoNamespaces;
import org.onosproject.store.service.EventuallyConsistentMap;
import org.onosproject.store.service.StorageService;
@@ -99,21 +100,29 @@
}
@Override
- public Bmv2FlowRuleWrapper lookupEntryReference(Bmv2TableEntryReference entryRef) {
+ public Bmv2FlowRuleWrapper lookup(Bmv2TableEntryReference entryRef) {
checkNotNull(entryRef, "table entry reference cannot be null");
return flowRules.get(entryRef);
}
@Override
- public void bindEntryReference(Bmv2TableEntryReference entryRef, Bmv2FlowRuleWrapper rule) {
+ public void bind(Bmv2TableEntryReference entryRef, Bmv2FlowRuleWrapper rule) {
checkNotNull(entryRef, "table entry reference cannot be null");
checkNotNull(rule, "bmv2 flow rule cannot be null");
flowRules.put(entryRef, rule);
}
@Override
- public void unbindEntryReference(Bmv2TableEntryReference entryRef) {
+ public void unbind(Bmv2TableEntryReference entryRef) {
checkNotNull(entryRef, "table entry reference cannot be null");
flowRules.remove(entryRef);
}
+
+ @Override
+ public void unbindAll(DeviceId deviceId) {
+ flowRules.keySet()
+ .stream()
+ .filter(entryRef -> entryRef.deviceId().equals(deviceId))
+ .forEach(flowRules::remove);
+ }
}
diff --git a/providers/bmv2/device/src/main/java/org/onosproject/provider/bmv2/device/impl/Bmv2DeviceProvider.java b/providers/bmv2/device/src/main/java/org/onosproject/provider/bmv2/device/impl/Bmv2DeviceProvider.java
index ebdcbcb..1a9d4a0 100644
--- a/providers/bmv2/device/src/main/java/org/onosproject/provider/bmv2/device/impl/Bmv2DeviceProvider.java
+++ b/providers/bmv2/device/src/main/java/org/onosproject/provider/bmv2/device/impl/Bmv2DeviceProvider.java
@@ -29,6 +29,7 @@
import org.onosproject.bmv2.api.service.Bmv2Controller;
import org.onosproject.bmv2.api.service.Bmv2DeviceContextService;
import org.onosproject.bmv2.api.service.Bmv2DeviceListener;
+import org.onosproject.bmv2.api.service.Bmv2TableEntryService;
import org.onosproject.common.net.AbstractDeviceProvider;
import org.onosproject.core.ApplicationId;
import org.onosproject.core.CoreService;
@@ -107,6 +108,9 @@
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected Bmv2DeviceContextService contextService;
+ @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+ protected Bmv2TableEntryService tableEntryService;
+
private ApplicationId appId;
/**
@@ -228,6 +232,8 @@
private void resetDeviceState(DeviceId did) {
try {
controller.getAgent(did).resetState();
+ // Tables emptied. Reset all bindings.
+ tableEntryService.unbindAll(did);
} catch (Bmv2RuntimeException e) {
log.warn("Unable to reset {}: {}", did, e.toString());
}