홍길동 / onos

Go to a project
Toggle navigation Toggle navigation pinning
Carmelo Cascone
Committed by Jonathan Hart
ee4cd7e4 1 parent 9e39e31c

Improved consistency for BMv2 flow rules handling 

Change-Id: I3a4798af3f35f135e8162385a1bf7fc059028307
Inline Side-by-side
Showing 4 changed files with 56 additions and 37 deletions
......@@ -24,6 +24,7 @@ import org.onosproject.bmv2.api.context.Bmv2DeviceContext;
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 @@ public class Bmv2FlowRuleProgrammable extends AbstractHandlerBehaviour implement
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 @@ public class Bmv2FlowRuleProgrammable extends AbstractHandlerBehaviour implement
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 @@ public class Bmv2FlowRuleProgrammable extends AbstractHandlerBehaviour implement
FlowEntry entry = new DefaultFlowEntry(frWrapper.rule(), ADDED, frWrapper.lifeInSeconds(),
packets, bytes);
entryList.add(entry);
return true;
});
});
});
}
}
}
return Collections.unmodifiableCollection(entryList);
}
......@@ -226,19 +226,16 @@ public class Bmv2FlowRuleProgrammable extends AbstractHandlerBehaviour implement
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 @@ public class Bmv2FlowRuleProgrammable extends AbstractHandlerBehaviour implement
} 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 @@ public class Bmv2FlowRuleProgrammable extends AbstractHandlerBehaviour implement
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 @@ public class Bmv2FlowRuleProgrammable extends AbstractHandlerBehaviour implement
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 {
......
......@@ -21,6 +21,7 @@ import com.google.common.annotations.Beta;
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 @@ public interface Bmv2TableEntryService {
* @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 @@ public interface Bmv2TableEntryService {
* @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);
}
......
......@@ -34,6 +34,7 @@ import org.onosproject.bmv2.api.runtime.Bmv2ValidMatchParam;
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 @@ public class Bmv2TableEntryServiceImpl implements Bmv2TableEntryService {
}
@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);
}
}
......
......@@ -29,6 +29,7 @@ import org.onosproject.bmv2.api.runtime.Bmv2RuntimeException;
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 @@ public class Bmv2DeviceProvider extends AbstractDeviceProvider {
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected Bmv2DeviceContextService contextService;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected Bmv2TableEntryService tableEntryService;
private ApplicationId appId;
/**
......@@ -228,6 +232,8 @@ public class Bmv2DeviceProvider extends AbstractDeviceProvider {
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());
}
......