More robust P4Runtime group handling
This patch solves the PENDING_UPDATE and PENDING_ADD_RETRY issue
observed on the ONS EU topology.
The P4Runtime action profile group handling has been re-implemented to
be robust against inconsistencies of the device mirror, which is now
periodically synchronized with the device state. Similarly, we implement
a routine in the P4RuntimeClient to cleanup unused action profile
members.
This patch includes also:
- Refactor PI handle classes to allow creating handles without the
entity instance
- Use list instead of collections in P4RuntimeClient methods, as order
of updates sent and/or entities received from the device is important
Change-Id: I2e7964ce90f43d66680131b47ab52aca32ab55d2
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionGroupHandle.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionGroupHandle.java
index 4c87f1f..6969714 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionGroupHandle.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionGroupHandle.java
@@ -20,6 +20,7 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Objects;
import org.onosproject.net.DeviceId;
+import org.onosproject.net.pi.model.PiActionProfileId;
/**
* Global identifier of a PI action group applied to a device, uniquely defined
@@ -28,8 +29,13 @@
@Beta
public final class PiActionGroupHandle extends PiHandle<PiActionGroup> {
+ private final PiActionProfileId actionProfileId;
+ private final PiActionGroupId groupId;
+
private PiActionGroupHandle(DeviceId deviceId, PiActionGroup group) {
- super(deviceId, group);
+ super(deviceId);
+ actionProfileId = group.actionProfileId();
+ groupId = group.id();
}
/**
@@ -45,10 +51,15 @@
}
@Override
+ public PiEntityType entityType() {
+ return PiEntityType.GROUP;
+ }
+
+ @Override
public int hashCode() {
return Objects.hashCode(deviceId(),
- piEntity().actionProfileId(),
- piEntity().id());
+ actionProfileId,
+ groupId);
}
@Override
@@ -61,17 +72,17 @@
}
PiActionGroupHandle that = (PiActionGroupHandle) o;
return Objects.equal(deviceId(), that.deviceId()) &&
- Objects.equal(piEntity().actionProfileId(),
- that.piEntity().actionProfileId()) &&
- Objects.equal(piEntity().id(), that.piEntity().id());
+ Objects.equal(actionProfileId,
+ that.actionProfileId) &&
+ Objects.equal(groupId, that.groupId);
}
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("deviceId", deviceId())
- .add("actionProfileId", piEntity().actionProfileId())
- .add("groupId", piEntity().id())
+ .add("actionProfileId", actionProfileId)
+ .add("groupId", groupId)
.toString();
}
}
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionGroupMember.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionGroupMember.java
index 1c5ecb3..690d118 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionGroupMember.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionGroupMember.java
@@ -19,6 +19,7 @@
import com.google.common.annotations.Beta;
import com.google.common.base.MoreObjects;
import com.google.common.base.Objects;
+import org.onosproject.net.pi.model.PiActionProfileId;
import static com.google.common.base.Preconditions.checkNotNull;
@@ -28,11 +29,18 @@
@Beta
public final class PiActionGroupMember implements PiEntity {
+ private final PiActionProfileId actionProfileId;
private final PiActionGroupMemberId id;
private final PiAction action;
+ // FIXME: in P4Runtime weight is an attribute of the member reference in a
+ // group. Either remove it from this class or define the containing group
+ // ID.
private final int weight;
- private PiActionGroupMember(PiActionGroupMemberId id, PiAction action, int weight) {
+ private PiActionGroupMember(
+ PiActionProfileId actionProfileId, PiActionGroupMemberId id,
+ PiAction action, int weight) {
+ this.actionProfileId = actionProfileId;
this.id = id;
this.action = action;
this.weight = weight;
@@ -48,6 +56,15 @@
}
/**
+ * Returns the identifier of the action profile.
+ *
+ * @return action profile identifier
+ */
+ public PiActionProfileId actionProfile() {
+ return actionProfileId;
+ }
+
+ /**
* Returns the action associated to this member.
*
* @return action
@@ -80,18 +97,20 @@
}
PiActionGroupMember that = (PiActionGroupMember) o;
return weight == that.weight &&
+ Objects.equal(actionProfileId, that.actionProfileId) &&
Objects.equal(id, that.id) &&
Objects.equal(action, that.action);
}
@Override
public int hashCode() {
- return Objects.hashCode(id, action, weight);
+ return Objects.hashCode(actionProfileId, id, action, weight);
}
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
+ .add("actionProfile", actionProfileId)
.add("id", id)
.add("action", action)
.add("weight", weight)
@@ -112,6 +131,7 @@
*/
public static final class Builder {
+ private PiActionProfileId actionProfileId;
private PiActionGroupMemberId id;
private PiAction action;
private int weight;
@@ -121,6 +141,17 @@
}
/**
+ * Sets the action profile identifier of this member.
+ *
+ * @param actionProfileId action profile identifier
+ * @return this
+ */
+ public Builder forActionProfile(PiActionProfileId actionProfileId) {
+ this.actionProfileId = actionProfileId;
+ return this;
+ }
+
+ /**
* Sets the identifier of this member.
*
* @param id member identifier
@@ -161,9 +192,10 @@
* @return action group member
*/
public PiActionGroupMember build() {
+ checkNotNull(actionProfileId);
checkNotNull(id);
checkNotNull(action);
- return new PiActionGroupMember(id, action, weight);
+ return new PiActionGroupMember(actionProfileId, id, action, weight);
}
}
}
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionGroupMemberHandle.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionGroupMemberHandle.java
new file mode 100644
index 0000000..9ef8a31
--- /dev/null
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionGroupMemberHandle.java
@@ -0,0 +1,126 @@
+/*
+ * Copyright 2018-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.net.pi.runtime;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Objects;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.pi.model.PiActionProfileId;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+/**
+ * Global identifier of a PI action profile group member, uniquely defined by a
+ * device ID, action profile ID, and member ID.
+ */
+public final class PiActionGroupMemberHandle extends PiHandle<PiActionGroupMember> {
+
+ private final PiActionGroupMemberId memberId;
+ private final PiActionProfileId actionProfileId;
+
+ private PiActionGroupMemberHandle(DeviceId deviceId,
+ PiActionProfileId actionProfileId,
+ PiActionGroupMemberId memberId) {
+ super(deviceId);
+ this.actionProfileId = actionProfileId;
+ this.memberId = memberId;
+ }
+
+ /**
+ * Creates a new handle for the given device ID, action profile ID, and
+ * member ID.
+ *
+ * @param deviceId device ID
+ * @param actionProfileId action profile ID
+ * @param memberId member ID
+ * @return action profile group member handle
+ */
+ public static PiActionGroupMemberHandle of(
+ DeviceId deviceId,
+ PiActionProfileId actionProfileId,
+ PiActionGroupMemberId memberId) {
+ return new PiActionGroupMemberHandle(
+ deviceId, actionProfileId, memberId);
+ }
+
+ /**
+ * Creates a new handle for the given device ID, and action profile group
+ * member instance.
+ *
+ * @param deviceId device ID
+ * @param member member instance
+ * @return action profile group member handle
+ */
+ public static PiActionGroupMemberHandle of(
+ DeviceId deviceId,
+ PiActionGroupMember member) {
+ checkNotNull(member);
+ return new PiActionGroupMemberHandle(
+ deviceId, member.actionProfile(), member.id());
+ }
+
+ /**
+ * Returns the member ID of this handle.
+ *
+ * @return member ID
+ */
+ public PiActionGroupMemberId memberId() {
+ return memberId;
+ }
+
+ /**
+ * Returns the action profile ID of this handle.
+ *
+ * @return action profile ID
+ */
+ public PiActionProfileId actionProfileId() {
+ return actionProfileId;
+ }
+
+ @Override
+ public PiEntityType entityType() {
+ return PiEntityType.GROUP_MEMBER;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(deviceId(), actionProfileId, memberId);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (obj == null || getClass() != obj.getClass()) {
+ return false;
+ }
+ final PiActionGroupMemberHandle other = (PiActionGroupMemberHandle) obj;
+ return Objects.equal(this.deviceId(), other.deviceId())
+ && Objects.equal(this.actionProfileId, other.actionProfileId)
+ && Objects.equal(this.memberId, other.memberId);
+ }
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("deviceId", deviceId())
+ .add("actionProfileId", actionProfileId)
+ .add("memberId", memberId)
+ .toString();
+ }
+}
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiHandle.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiHandle.java
index e8e70d1..eb74288 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiHandle.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiHandle.java
@@ -29,11 +29,9 @@
public abstract class PiHandle<E extends PiEntity> {
private final DeviceId deviceId;
- private final E piEntity;
- protected PiHandle(DeviceId deviceId, E piEntity) {
+ protected PiHandle(DeviceId deviceId) {
this.deviceId = checkNotNull(deviceId);
- this.piEntity = checkNotNull(piEntity);
}
/**
@@ -50,18 +48,7 @@
*
* @return PI entity type
*/
- public final PiEntityType entityType() {
- return piEntity.piEntityType();
- }
-
- /**
- * The entity to which this handle is associated.
- *
- * @return PI entity
- */
- public final E piEntity() {
- return piEntity;
- }
+ public abstract PiEntityType entityType();
@Override
public abstract int hashCode();
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterHandle.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterHandle.java
index ad2af9d..4baa6fa 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterHandle.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterHandle.java
@@ -21,33 +21,56 @@
import com.google.common.base.Objects;
import org.onosproject.net.DeviceId;
+import static com.google.common.base.Preconditions.checkNotNull;
+
/**
- * Global identifier of a PI meter cell configuration applied to a device, uniquely defined
- * by a device ID and meter cell ID.
+ * Global identifier of a PI meter cell configuration applied to a device,
+ * uniquely defined by a device ID and meter cell ID.
*/
@Beta
public final class PiMeterHandle extends PiHandle<PiMeterCellConfig> {
- private PiMeterHandle(DeviceId deviceId, PiMeterCellConfig meterCellConfig) {
- super(deviceId, meterCellConfig);
+ private final PiMeterCellId cellId;
+
+ private PiMeterHandle(DeviceId deviceId, PiMeterCellId meterCellId) {
+ super(deviceId);
+ this.cellId = meterCellId;
}
/**
- * Creates a new handle for the given device ID and PI meter cell configuration.
+ * Creates a new handle for the given device ID and PI meter cell ID.
*
- * @param deviceId device ID
+ * @param deviceId device ID
+ * @param meterCellId meter cell ID
+ * @return PI meter handle
+ */
+ public static PiMeterHandle of(DeviceId deviceId,
+ PiMeterCellId meterCellId) {
+ return new PiMeterHandle(deviceId, meterCellId);
+ }
+
+ /**
+ * Creates a new handle for the given device ID and PI meter cell
+ * configuration.
+ *
+ * @param deviceId device ID
* @param meterCellConfig meter config
* @return PI meter handle
*/
public static PiMeterHandle of(DeviceId deviceId,
PiMeterCellConfig meterCellConfig) {
- return new PiMeterHandle(deviceId, meterCellConfig);
+ checkNotNull(meterCellConfig);
+ return new PiMeterHandle(deviceId, meterCellConfig.cellId());
+ }
+
+ @Override
+ public PiEntityType entityType() {
+ return PiEntityType.METER_CELL_CONFIG;
}
@Override
public int hashCode() {
- return Objects.hashCode(deviceId(),
- piEntity().cellId());
+ return Objects.hashCode(deviceId(), cellId);
}
@Override
@@ -60,15 +83,14 @@
}
PiMeterHandle that = (PiMeterHandle) o;
return Objects.equal(deviceId(), that.deviceId()) &&
- Objects.equal(piEntity().cellId(),
- that.piEntity().cellId());
+ Objects.equal(cellId, that.cellId);
}
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("deviceId", deviceId())
- .add("meterCellId", piEntity().cellId())
+ .add("meterCellId", cellId)
.toString();
}
-}
\ No newline at end of file
+}
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMulticastGroupEntryHandle.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMulticastGroupEntryHandle.java
index f9b1170..65a3f28 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMulticastGroupEntryHandle.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMulticastGroupEntryHandle.java
@@ -21,6 +21,8 @@
import com.google.common.base.Objects;
import org.onosproject.net.DeviceId;
+import static com.google.common.base.Preconditions.checkNotNull;
+
/**
* Global identifier of a PI multicast group entry applied to the packet
* replication engine of a device, uniquely defined by a device ID, and group
@@ -29,8 +31,23 @@
@Beta
public final class PiMulticastGroupEntryHandle extends PiHandle<PiMulticastGroupEntry> {
- private PiMulticastGroupEntryHandle(DeviceId deviceId, PiMulticastGroupEntry entry) {
- super(deviceId, entry);
+ private final long groupId;
+
+ private PiMulticastGroupEntryHandle(DeviceId deviceId, long groupId) {
+ super(deviceId);
+ this.groupId = groupId;
+ }
+
+ /**
+ * Creates a new handle for the given device ID and PI multicast group ID.
+ *
+ * @param deviceId device ID
+ * @param groupId multicast group ID
+ * @return PI multicast group entry handle
+ */
+ public static PiMulticastGroupEntryHandle of(DeviceId deviceId,
+ long groupId) {
+ return new PiMulticastGroupEntryHandle(deviceId, groupId);
}
/**
@@ -43,12 +60,18 @@
*/
public static PiMulticastGroupEntryHandle of(DeviceId deviceId,
PiMulticastGroupEntry entry) {
- return new PiMulticastGroupEntryHandle(deviceId, entry);
+ checkNotNull(entry);
+ return new PiMulticastGroupEntryHandle(deviceId, entry.groupId());
+ }
+
+ @Override
+ public PiEntityType entityType() {
+ return PiEntityType.PRE_MULTICAST_GROUP_ENTRY;
}
@Override
public int hashCode() {
- return Objects.hashCode(deviceId(), piEntity().groupId());
+ return Objects.hashCode(deviceId(), groupId);
}
@Override
@@ -61,14 +84,14 @@
}
PiMulticastGroupEntryHandle that = (PiMulticastGroupEntryHandle) o;
return Objects.equal(deviceId(), that.deviceId()) &&
- Objects.equal(piEntity().groupId(), that.piEntity().groupId());
+ Objects.equal(groupId, that.groupId);
}
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("deviceId", deviceId())
- .add("groupId", piEntity().groupId())
+ .add("groupId", groupId)
.toString();
}
}
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiTableEntry.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiTableEntry.java
index 04b2528..f8a1460 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiTableEntry.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiTableEntry.java
@@ -135,6 +135,7 @@
}
PiTableEntry that = (PiTableEntry) o;
return priority == that.priority &&
+ cookie == that.cookie &&
Double.compare(that.timeout, timeout) == 0 &&
Objects.equal(tableId, that.tableId) &&
Objects.equal(matchKey, that.matchKey) &&
@@ -145,7 +146,7 @@
@Override
public int hashCode() {
return Objects.hashCode(tableId, matchKey, isDefaultAction, tableAction,
- priority, timeout);
+ priority, cookie, timeout);
}
@Override
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiTableEntryHandle.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiTableEntryHandle.java
index 7eeb7f6..2b210a1 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiTableEntryHandle.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiTableEntryHandle.java
@@ -20,6 +20,9 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Objects;
import org.onosproject.net.DeviceId;
+import org.onosproject.net.pi.model.PiTableId;
+
+import static com.google.common.base.Preconditions.checkNotNull;
/**
* Global identifier of a PI table entry applied on a device, uniquely defined
@@ -28,8 +31,28 @@
@Beta
public final class PiTableEntryHandle extends PiHandle<PiTableEntry> {
- private PiTableEntryHandle(DeviceId deviceId, PiTableEntry entry) {
- super(deviceId, entry);
+ private final PiTableId tableId;
+ private final PiMatchKey matchKey;
+
+ private PiTableEntryHandle(DeviceId deviceId, PiTableId tableId, PiMatchKey matchKey) {
+ super(deviceId);
+ this.tableId = tableId;
+ this.matchKey = matchKey;
+ }
+
+ /**
+ * Creates a new handle for the given device ID, PI table ID, and match
+ * key.
+ *
+ * @param deviceId device ID
+ * @param tableId table ID
+ * @param matchKey match key
+ * @return PI table entry handle
+ */
+ public static PiTableEntryHandle of(DeviceId deviceId, PiTableId tableId, PiMatchKey matchKey) {
+ checkNotNull(tableId);
+ checkNotNull(matchKey);
+ return new PiTableEntryHandle(deviceId, tableId, matchKey);
}
/**
@@ -40,14 +63,18 @@
* @return PI table entry handle
*/
public static PiTableEntryHandle of(DeviceId deviceId, PiTableEntry entry) {
- return new PiTableEntryHandle(deviceId, entry);
+ checkNotNull(entry);
+ return PiTableEntryHandle.of(deviceId, entry.table(), entry.matchKey());
+ }
+
+ @Override
+ public PiEntityType entityType() {
+ return PiEntityType.TABLE_ENTRY;
}
@Override
public int hashCode() {
- return Objects.hashCode(deviceId(),
- piEntity().table(),
- piEntity().matchKey());
+ return Objects.hashCode(deviceId(), tableId, matchKey);
}
@Override
@@ -60,18 +87,16 @@
}
final PiTableEntryHandle other = (PiTableEntryHandle) obj;
return Objects.equal(this.deviceId(), other.deviceId())
- && Objects.equal(this.piEntity().table(),
- other.piEntity().table())
- && Objects.equal(this.piEntity().matchKey(),
- other.piEntity().matchKey());
+ && Objects.equal(this.tableId, other.tableId)
+ && Objects.equal(this.matchKey, other.matchKey);
}
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
.add("deviceId", deviceId())
- .add("tableId", piEntity().table())
- .add("matchKey", piEntity().matchKey())
+ .add("tableId", tableId)
+ .add("matchKey", matchKey)
.toString();
}
}