Various fixes in preparation of Stratum demo at ONS 2019

- Do not read counters with table entries for Barefoot drivers
- If driver behavior setup fails, log which operation we are aborting
- Remove unnecessary setup steps in Stratum-related drivers
- Always get clients by their key in gRPC-based drivers
- Log when P4Runtime group operation fails because of missing group in
store
- Fix polling of table entry counters for P4Runtime driver

Change-Id: Ic9bf19b76d8cb5a191aec24852af4410fea8b998
This commit is contained in:
Carmelo Cascone 2019-04-08 14:50:52 -07:00
parent b48533e3f7
commit c32976e7d5
16 changed files with 114 additions and 140 deletions

View File

@ -19,6 +19,7 @@
swVersion="1.0" extends="p4runtime">
<behaviour api="org.onosproject.net.behaviour.PiPipelineProgrammable"
impl="org.onosproject.drivers.barefoot.TofinoPipelineProgrammable"/>
<property name="tableReadCountersWithTableEntries">false</property>
<property name="tableDeleteBeforeUpdate">true</property>
</driver>
@ -27,8 +28,9 @@
<behaviour api="org.onosproject.net.behaviour.PiPipelineProgrammable"
impl="org.onosproject.drivers.barefoot.TofinoPipelineProgrammable"/>
<!-- The current version of p4lang/PI used in Stratum does not
support reading default table entries -->
support reading default table entries -->
<property name="supportDefaultTableEntry">false</property>
<property name="tableReadCountersWithTableEntries">false</property>
</driver>
</drivers>

View File

@ -20,7 +20,6 @@ import com.google.common.base.Strings;
import org.onosproject.gnmi.api.GnmiClient;
import org.onosproject.gnmi.api.GnmiClientKey;
import org.onosproject.gnmi.api.GnmiController;
import org.onosproject.net.Device;
import org.onosproject.net.DeviceId;
import org.onosproject.net.config.NetworkConfigService;
import org.onosproject.net.config.basics.BasicDeviceConfig;
@ -40,18 +39,14 @@ public class AbstractGnmiHandlerBehaviour extends AbstractHandlerBehaviour {
protected final Logger log = LoggerFactory.getLogger(getClass());
protected DeviceId deviceId;
protected DeviceService deviceService;
protected Device device;
protected GnmiController controller;
protected GnmiClient client;
protected boolean setupBehaviour() {
protected boolean setupBehaviour(String opName) {
deviceId = handler().data().deviceId();
deviceService = handler().get(DeviceService.class);
controller = handler().get(GnmiController.class);
client = controller.getClient(deviceId);
client = getClientByKey();
if (client == null) {
log.warn("Unable to find client for {}, aborting operation", deviceId);
log.warn("Missing client for {}, aborting {}", deviceId, opName);
return false;
}

View File

@ -76,7 +76,7 @@ public class OpenConfigGnmiDeviceDescriptionDiscovery
@Override
public List<PortDescription> discoverPortDetails() {
if (!setupBehaviour()) {
if (!setupBehaviour("discoverPortDetails()")) {
return Collections.emptyList();
}
log.debug("Discovering port details on device {}", handler().data().deviceId());

View File

@ -50,7 +50,7 @@ public class OpenConfigGnmiPortStatisticsDiscovery extends AbstractGnmiHandlerBe
@Override
public Collection<PortStatistics> discoverPortStatistics() {
if (!setupBehaviour()) {
if (!setupBehaviour("discoverPortStatistics()")) {
return Collections.emptyList();
}

View File

@ -20,11 +20,9 @@ import com.google.common.base.Strings;
import org.onosproject.gnoi.api.GnoiClient;
import org.onosproject.gnoi.api.GnoiClientKey;
import org.onosproject.gnoi.api.GnoiController;
import org.onosproject.net.Device;
import org.onosproject.net.DeviceId;
import org.onosproject.net.config.NetworkConfigService;
import org.onosproject.net.config.basics.BasicDeviceConfig;
import org.onosproject.net.device.DeviceService;
import org.onosproject.net.driver.AbstractHandlerBehaviour;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -39,19 +37,14 @@ public class AbstractGnoiHandlerBehaviour extends AbstractHandlerBehaviour {
protected final Logger log = LoggerFactory.getLogger(getClass());
protected DeviceId deviceId;
protected DeviceService deviceService;
protected Device device;
protected GnoiController controller;
protected GnoiClient client;
protected boolean setupBehaviour() {
protected boolean setupBehaviour(String opName) {
deviceId = handler().data().deviceId();
deviceService = handler().get(DeviceService.class);
controller = handler().get(GnoiController.class);
client = controller.getClient(deviceId);
client = getClientByKey();
if (client == null) {
log.warn("Unable to find client for {}, aborting operation", deviceId);
log.warn("Missing client for {}, aborting {}", deviceId, opName);
return false;
}

View File

@ -16,17 +16,18 @@
package org.onosproject.drivers.gnoi;
import gnoi.system.SystemOuterClass.RebootMethod;
import gnoi.system.SystemOuterClass.RebootRequest;
import gnoi.system.SystemOuterClass.RebootResponse;
import gnoi.system.SystemOuterClass.RebootMethod;
import org.onosproject.net.behaviour.BasicSystemOperations;
import java.util.concurrent.CompletableFuture;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.concurrent.CompletableFuture;
/**
* Implementation of the BasicSystemOperations behavior for gNOI-enabled devices.
* Implementation of the BasicSystemOperations behavior for gNOI-enabled
* devices.
*/
public class GnoiBasicSystemOperationsImpl
extends AbstractGnoiHandlerBehaviour implements BasicSystemOperations {
@ -36,12 +37,13 @@ public class GnoiBasicSystemOperationsImpl
@Override
public CompletableFuture<Boolean> reboot() {
if (!setupBehaviour()) {
if (!setupBehaviour("reboot()")) {
return CompletableFuture.completedFuture(false);
}
final RebootRequest.Builder requestMsg = RebootRequest.newBuilder().setMethod(RebootMethod.COLD);
final CompletableFuture<Boolean> future = client.reboot(requestMsg.build())
return client.reboot(requestMsg.build())
.handle((response, error) -> {
if (error == null) {
log.debug("gNOI reboot() for device {} returned {}", deviceId, response);
@ -51,16 +53,14 @@ public class GnoiBasicSystemOperationsImpl
return false;
}
});
return future;
}
@Override
public CompletableFuture<Long> time() {
if (!setupBehaviour()) {
if (!setupBehaviour("time()")) {
return CompletableFuture.completedFuture(0L);
}
final CompletableFuture<Long> future = client.time()
return client.time()
.handle((response, error) -> {
if (error == null) {
log.debug("gNOI time() for device {} returned {}", deviceId.uri(), response.getTime());
@ -70,7 +70,5 @@ public class GnoiBasicSystemOperationsImpl
return 0L;
}
});
return future;
}
}

View File

@ -47,43 +47,29 @@ public class AbstractP4RuntimeHandlerBehaviour extends AbstractHandlerBehaviour
// Initialized by setupBehaviour()
protected DeviceId deviceId;
protected DeviceService deviceService;
protected Device device;
protected P4RuntimeController controller;
protected PiPipeconf pipeconf;
protected P4RuntimeClient client;
protected PiTranslationService translationService;
/**
* Initializes this behaviour attributes. Returns true if the operation was
* successful, false otherwise. This method assumes that the P4Runtime
* controller already has a client for this device and that the device has
* been created in the core.
* successful, false otherwise.
*
* @param opName name of the operation
* @return true if successful, false otherwise
*/
protected boolean setupBehaviour() {
protected boolean setupBehaviour(String opName) {
deviceId = handler().data().deviceId();
deviceService = handler().get(DeviceService.class);
device = deviceService.getDevice(deviceId);
if (device == null) {
log.warn("Unable to find device with id {}", deviceId);
return false;
}
controller = handler().get(P4RuntimeController.class);
client = controller.getClient(deviceId);
client = getClientByKey();
if (client == null) {
log.warn("Unable to find client for {}", deviceId);
log.warn("Missing client for {}, aborting {}", deviceId, opName);
return false;
}
PiPipeconfService piPipeconfService = handler().get(PiPipeconfService.class);
if (!piPipeconfService.getPipeconf(deviceId).isPresent()) {
log.warn("Unable to get assigned pipeconf for {} (mapping " +
"missing in PiPipeconfService)",
deviceId);
log.warn("Missing pipeconf for {}, cannot perform {}", deviceId, opName);
return false;
}
pipeconf = piPipeconfService.getPipeconf(deviceId).get();
@ -100,6 +86,11 @@ public class AbstractP4RuntimeHandlerBehaviour extends AbstractHandlerBehaviour
* @return interpreter or null
*/
PiPipelineInterpreter getInterpreter() {
final Device device = handler().get(DeviceService.class).getDevice(deviceId);
if (device == null) {
log.warn("Unable to find device {}, cannot get interpreter", deviceId);
return null;
}
if (!device.is(PiPipelineInterpreter.class)) {
log.warn("Unable to get interpreter for {}, missing behaviour",
deviceId);

View File

@ -81,8 +81,8 @@ public class P4RuntimeActionGroupProgrammable
private PiGroupTranslator groupTranslator;
@Override
protected boolean setupBehaviour() {
if (!super.setupBehaviour()) {
protected boolean setupBehaviour(String opName) {
if (!super.setupBehaviour(opName)) {
return false;
}
groupMirror = this.handler().get(P4RuntimeActionProfileGroupMirror.class);
@ -95,7 +95,7 @@ public class P4RuntimeActionGroupProgrammable
@Override
public void performGroupOperation(DeviceId deviceId,
GroupOperations groupOps) {
if (!setupBehaviour()) {
if (!setupBehaviour("performGroupOperation()")) {
return;
}
@ -107,8 +107,8 @@ public class P4RuntimeActionGroupProgrammable
// GroupDescription.
Group groupOnStore = groupStore.getGroup(deviceId, op.groupId());
if (groupOnStore == null) {
log.warn("Unable to find group {} in store, aborting {} operation",
op.groupId(), op.opType());
log.warn("Unable to find group {} in store, aborting {} operation [{}]",
op.groupId(), op.opType(), op);
return;
}
GroupDescription groupDesc = new DefaultGroupDescription(
@ -121,7 +121,7 @@ public class P4RuntimeActionGroupProgrammable
@Override
public Collection<Group> getGroups() {
if (!setupBehaviour()) {
if (!setupBehaviour("getGroups()")) {
return Collections.emptyList();
}

View File

@ -108,9 +108,9 @@ public class P4RuntimeFlowRuleProgrammable
private PiFlowRuleTranslator translator;
@Override
protected boolean setupBehaviour() {
protected boolean setupBehaviour(String opName) {
if (!super.setupBehaviour()) {
if (!super.setupBehaviour(opName)) {
return false;
}
@ -123,7 +123,7 @@ public class P4RuntimeFlowRuleProgrammable
@Override
public Collection<FlowEntry> getFlowEntries() {
if (!setupBehaviour()) {
if (!setupBehaviour("getFlowEntries()")) {
return Collections.emptyList();
}
@ -149,12 +149,13 @@ public class P4RuntimeFlowRuleProgrammable
return Collections.emptyList();
}
final Map<PiTableEntry, PiCounterCellData> counterCellMap =
final Map<PiTableEntryHandle, PiCounterCellData> counterCellMap =
readEntryCounters(deviceEntries);
// Forge flow entries with counter values.
for (PiTableEntry entry : deviceEntries) {
final PiTableEntryHandle handle = entry.handle(deviceId);
final FlowEntry flowEntry = forgeFlowEntry(
entry, counterCellMap.get(entry));
entry, handle, counterCellMap.get(handle));
if (flowEntry == null) {
// Entry is on device but unknown to translation service or
// device mirror. Inconsistent. Mark for removal.
@ -228,8 +229,8 @@ public class P4RuntimeFlowRuleProgrammable
}
private FlowEntry forgeFlowEntry(PiTableEntry entry,
PiTableEntryHandle handle,
PiCounterCellData cellData) {
final PiTableEntryHandle handle = entry.handle(deviceId);
final Optional<PiTranslatedEntity<FlowRule, PiTableEntry>>
translatedEntity = translator.lookup(handle);
final TimedEntry<PiTableEntry> timedEntry = tableMirror.get(handle);
@ -262,14 +263,14 @@ public class P4RuntimeFlowRuleProgrammable
private Collection<FlowEntry> getFlowEntriesFromMirror() {
return tableMirror.getAll(deviceId).stream()
.map(timedEntry -> forgeFlowEntry(
timedEntry.entry(), null))
timedEntry.entry(), timedEntry.entry().handle(deviceId), null))
.filter(Objects::nonNull)
.collect(Collectors.toList());
}
private Collection<FlowRule> processFlowRules(Collection<FlowRule> rules,
Operation driverOperation) {
if (!setupBehaviour() || rules.isEmpty()) {
if (!setupBehaviour("processFlowRules()") || rules.isEmpty()) {
return Collections.emptyList();
}
// Created batched write request.
@ -459,52 +460,38 @@ public class P4RuntimeFlowRuleProgrammable
originalDefaultEntry.action().equals(entry.action());
}
private Map<PiTableEntry, PiCounterCellData> readEntryCounters(
private Map<PiTableEntryHandle, PiCounterCellData> readEntryCounters(
Collection<PiTableEntry> tableEntries) {
if (!driverBoolProperty(SUPPORT_TABLE_COUNTERS,
DEFAULT_SUPPORT_TABLE_COUNTERS)
|| tableEntries.isEmpty()) {
return Collections.emptyMap();
}
final Map<PiTableEntry, PiCounterCellData> cellDataMap = Maps.newHashMap();
// We expect the server to return table entries with counter data (if
// the table supports counter). Here we extract such counter data and we
// determine if there are missing counter cells (if, for example, the
// serves does not support returning counter data with table entries)
final Set<PiHandle> missingCellHandles = tableEntries.stream()
.map(t -> {
if (t.counter() != null) {
// Counter data found in table entry.
cellDataMap.put(t, t.counter());
return null;
} else {
return t;
}
})
.filter(Objects::nonNull)
// Ignore for default entries and for tables without counters.
.filter(e -> !e.isDefaultAction())
.filter(e -> tableHasCounter(e.table()))
.map(PiCounterCellId::ofDirect)
.map(id -> PiCounterCellHandle.of(deviceId, id))
.collect(Collectors.toSet());
// We might be sending a large read request (for thousands or more
// of counter cell handles). We request the driver to vet this
// operation via driver property.
if (!missingCellHandles.isEmpty()
&& !driverBoolProperty(READ_COUNTERS_WITH_TABLE_ENTRIES,
DEFAULT_READ_COUNTERS_WITH_TABLE_ENTRIES)) {
client.read(pipeconf)
.handles(missingCellHandles)
if (driverBoolProperty(READ_COUNTERS_WITH_TABLE_ENTRIES,
DEFAULT_READ_COUNTERS_WITH_TABLE_ENTRIES)) {
return tableEntries.stream()
.filter(t -> t.counter() != null)
.collect(Collectors.toMap(
t -> t.handle(deviceId), PiTableEntry::counter));
} else {
final Set<PiHandle> cellHandles = tableEntries.stream()
.filter(e -> !e.isDefaultAction())
.filter(e -> tableHasCounter(e.table()))
.map(PiCounterCellId::ofDirect)
.map(id -> PiCounterCellHandle.of(deviceId, id))
.collect(Collectors.toSet());
// FIXME: We might be sending a very large read request...
return client.read(pipeconf)
.handles(cellHandles)
.submitSync()
.all(PiCounterCell.class).stream()
.filter(c -> c.cellId().counterType().equals(PiCounterType.DIRECT))
.forEach(c -> cellDataMap.put(c.cellId().tableEntry(), c.data()));
.collect(Collectors.toMap(
c -> c.cellId().tableEntry().handle(deviceId),
PiCounterCell::data));
}
return cellDataMap;
}
private boolean tableHasCounter(PiTableId tableId) {

View File

@ -120,12 +120,12 @@ public class P4RuntimeHandshaker extends AbstractP4RuntimeHandlerBehaviour imple
@Override
public void roleChanged(MastershipRole newRole) {
if (!setupBehaviour("roleChanged()")) {
return;
}
if (newRole.equals(MastershipRole.NONE)) {
final P4RuntimeClient client = getClientByKey();
if (client != null) {
log.info("Notified role NONE, closing session...");
client.closeSession();
}
log.info("Notified role NONE, closing session...");
client.closeSession();
} else {
throw new UnsupportedOperationException(
"Use preference-based way for setting MASTER or STANDBY roles");
@ -134,18 +134,19 @@ public class P4RuntimeHandshaker extends AbstractP4RuntimeHandlerBehaviour imple
@Override
public void roleChanged(int preference, long term) {
if (setupBehaviour()) {
final int clusterSize = handler().get(ClusterService.class)
.getNodes().size();
if (clusterSize > MAX_CLUSTER_SIZE) {
throw new IllegalStateException(
"Cluster too big! Maz size supported is " + MAX_CLUSTER_SIZE);
}
BigInteger electionId = BigInteger.valueOf(term)
.multiply(BigInteger.valueOf(MAX_CLUSTER_SIZE))
.subtract(BigInteger.valueOf(preference));
client.setMastership(preference == 0, electionId);
if (!setupBehaviour("roleChanged()")) {
return;
}
final int clusterSize = handler().get(ClusterService.class)
.getNodes().size();
if (clusterSize > MAX_CLUSTER_SIZE) {
throw new IllegalStateException(
"Cluster too big! Maz size supported is " + MAX_CLUSTER_SIZE);
}
BigInteger electionId = BigInteger.valueOf(term)
.multiply(BigInteger.valueOf(MAX_CLUSTER_SIZE))
.subtract(BigInteger.valueOf(preference));
client.setMastership(preference == 0, electionId);
}
@Override

View File

@ -66,8 +66,8 @@ public class P4RuntimeMeterProgrammable extends AbstractP4RuntimeHandlerBehaviou
private PiPipelineModel pipelineModel;
@Override
protected boolean setupBehaviour() {
if (!super.setupBehaviour()) {
protected boolean setupBehaviour(String opName) {
if (!super.setupBehaviour(opName)) {
return false;
}
@ -80,7 +80,7 @@ public class P4RuntimeMeterProgrammable extends AbstractP4RuntimeHandlerBehaviou
@Override
public CompletableFuture<Boolean> performMeterOperation(MeterOperation meterOp) {
if (!setupBehaviour()) {
if (!setupBehaviour("performMeterOperation()")) {
return CompletableFuture.completedFuture(false);
}
@ -118,7 +118,7 @@ public class P4RuntimeMeterProgrammable extends AbstractP4RuntimeHandlerBehaviou
@Override
public CompletableFuture<Collection<Meter>> getMeters() {
if (!setupBehaviour()) {
if (!setupBehaviour("getMeters()")) {
return CompletableFuture.completedFuture(Collections.emptyList());
}

View File

@ -62,8 +62,8 @@ public class P4RuntimeMulticastGroupProgrammable
private PiMulticastGroupTranslator mcGroupTranslator;
@Override
protected boolean setupBehaviour() {
if (!super.setupBehaviour()) {
protected boolean setupBehaviour(String opName) {
if (!super.setupBehaviour(opName)) {
return false;
}
mcGroupMirror = this.handler().get(P4RuntimeMulticastGroupMirror.class);
@ -74,20 +74,25 @@ public class P4RuntimeMulticastGroupProgrammable
@Override
public void performGroupOperation(DeviceId deviceId, GroupOperations groupOps) {
if (!setupBehaviour()) {
if (!setupBehaviour("performGroupOperation()")) {
return;
}
groupOps.operations().stream()
.filter(op -> op.groupType().equals(GroupDescription.Type.ALL))
.forEach(op -> {
final Group group = groupStore.getGroup(deviceId, op.groupId());
if (group == null) {
log.warn("Unable to find group {} in store, aborting {} operation [{}]",
op.groupId(), op.opType(), op);
return;
}
processMcGroupOp(group, op.opType());
});
}
@Override
public Collection<Group> getGroups() {
if (!setupBehaviour()) {
if (!setupBehaviour("getGroups()")) {
return Collections.emptyList();
}
return ImmutableList.copyOf(getMcGroups());

View File

@ -33,7 +33,7 @@ public class P4RuntimePacketProgrammable
@Override
public void emit(OutboundPacket packet) {
if (!this.setupBehaviour()) {
if (!this.setupBehaviour("emit()")) {
return;
}

View File

@ -17,23 +17,23 @@ package org.onosproject.drivers.p4runtime;
import org.onosproject.net.DeviceId;
import org.onosproject.net.behaviour.TableStatisticsDiscovery;
import org.onosproject.net.flow.FlowRuleService;
import org.onosproject.net.flow.FlowEntry;
import org.onosproject.net.flow.TableId;
import org.onosproject.net.flow.IndexTableId;
import org.onosproject.net.flow.TableStatisticsEntry;
import org.onosproject.net.flow.DefaultTableStatisticsEntry;
import org.onosproject.net.pi.model.PiPipelineModel;
import org.onosproject.net.flow.FlowEntry;
import org.onosproject.net.flow.FlowRuleService;
import org.onosproject.net.flow.IndexTableId;
import org.onosproject.net.flow.TableId;
import org.onosproject.net.flow.TableStatisticsEntry;
import org.onosproject.net.pi.model.PiPipelineInterpreter;
import org.onosproject.net.pi.model.PiPipelineModel;
import org.onosproject.net.pi.model.PiTableId;
import org.onosproject.net.pi.model.PiTableModel;
import java.util.List;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Map;
import java.util.HashMap;
import java.util.Iterator;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import static com.google.common.collect.Lists.newArrayList;
@ -45,7 +45,7 @@ public class P4RuntimeTableStatisticsDiscovery extends AbstractP4RuntimeHandlerB
@Override
public List<TableStatisticsEntry> getTableStatistics() {
if (!setupBehaviour()) {
if (!setupBehaviour("getTableStatistics()")) {
return Collections.emptyList();
}
FlowRuleService flowService = handler().get(FlowRuleService.class);

View File

@ -66,8 +66,8 @@ public class StratumDeviceDescriptionDiscovery
.set(AnnotationKeys.PROTOCOL, format(
"%s, %s, %s",
p4Descr.annotations().value(AnnotationKeys.PROTOCOL),
gnoiDescr.annotations().value(AnnotationKeys.PROTOCOL),
gnmiDescr.annotations().value(AnnotationKeys.PROTOCOL)))
gnmiDescr.annotations().value(AnnotationKeys.PROTOCOL),
gnoiDescr.annotations().value(AnnotationKeys.PROTOCOL)))
.build());
}

View File

@ -152,7 +152,9 @@ public final class TableEntryCodec
.build());
// Counter.
piTableEntryBuilder.withCounterCellData(decodeCounter(message.getCounterData()));
if (message.hasCounterData()) {
piTableEntryBuilder.withCounterCellData(decodeCounter(message.getCounterData()));
}
return piTableEntryBuilder.build();
}