ONOS-7898 Action profile group/member refactoring
Also includes:
- New abstract P4Runtime codec implementation. Currently used for action
profile members/groups encoding/deconding, the plan is to handle all
other codecs via this.
- Improved read requests in P4RuntimeClientImpl
- Removed handling of max group size in P4Runtime driver. Instead, added
modified group translator to specify a max group size by using
information from the pipeline model.
Change-Id: I684bae0184d683bb448ba19863c561f9848479d2
diff --git a/core/api/src/main/java/org/onosproject/net/pi/model/PiActionProfileModel.java b/core/api/src/main/java/org/onosproject/net/pi/model/PiActionProfileModel.java
index 66b8c53..0a66ee9 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/model/PiActionProfileModel.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/model/PiActionProfileModel.java
@@ -41,16 +41,28 @@
Collection<PiTableId> tables();
/**
- * Returns true if this action profile implements dynamic selection, false otherwise.
+ * Returns true if this action profile implements dynamic selection, false
+ * otherwise.
*
- * @return true if action profile implements dynamic selection, false otherwise
+ * @return true if action profile implements dynamic selection, false
+ * otherwise
*/
boolean hasSelector();
/**
- * Returns the maximum number of member entries of this action profile.
+ * Returns the maximum number of member entries that this action profile can
+ * hold.
*
* @return maximum number of member entries
*/
- long maxSize();
+ long size();
+
+ /**
+ * Returns the maximum number of members a group of this action profile can
+ * hold. This method is meaningful only if the action profile implements
+ * dynamic selection. 0 signifies that an explicit limit is not set.
+ *
+ * @return maximum number of members a group can hold
+ */
+ int maxGroupSize();
}
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiAction.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiAction.java
index 24655b6..78a62fd 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiAction.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiAction.java
@@ -30,21 +30,24 @@
import static com.google.common.base.Preconditions.checkNotNull;
/**
- * Instance of an action, and its runtime parameters, of a table entry in a protocol-independent pipeline.
+ * Instance of an action, and its runtime parameters, of a table entry in a
+ * protocol-independent pipeline.
*/
@Beta
public final class PiAction implements PiTableAction {
private final PiActionId actionId;
- private final Map<PiActionParamId, PiActionParam> runtimeParams;
+ private final ImmutableMap<PiActionParamId, PiActionParam> runtimeParams;
/**
- * Creates a new action instance for the given action identifier and runtime parameters.
+ * Creates a new action instance for the given action identifier and runtime
+ * parameters.
*
* @param actionId action identifier
* @param runtimeParams list of runtime parameters
*/
- private PiAction(PiActionId actionId, Map<PiActionParamId, PiActionParam> runtimeParams) {
+ private PiAction(PiActionId actionId,
+ Map<PiActionParamId, PiActionParam> runtimeParams) {
this.actionId = actionId;
this.runtimeParams = ImmutableMap.copyOf(runtimeParams);
}
@@ -64,8 +67,8 @@
}
/**
- * Returns all runtime parameters of this action. Return an empty collection if the action doesn't take any runtime
- * parameters.
+ * Returns all runtime parameters of this action. Return an empty collection
+ * if the action doesn't take any runtime parameters.
*
* @return list of byte sequences
*/
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileGroup.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileGroup.java
index 96168b9..f86e70c 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileGroup.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileGroup.java
@@ -19,13 +19,15 @@
import com.google.common.annotations.Beta;
import com.google.common.base.MoreObjects;
import com.google.common.base.Objects;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import org.onosproject.net.pi.model.PiActionProfileId;
import java.util.Collection;
import java.util.Map;
+import java.util.Optional;
+import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
/**
@@ -34,70 +36,96 @@
@Beta
public final class PiActionProfileGroup implements PiEntity {
- private final PiActionProfileGroupId id;
- private final ImmutableSet<PiActionProfileMember> members;
private final PiActionProfileId actionProfileId;
+ private final PiActionProfileGroupId groupId;
+ private final ImmutableMap<PiActionProfileMemberId, WeightedMember> members;
+ private final int maxSize;
- private PiActionProfileGroup(PiActionProfileGroupId id,
- ImmutableSet<PiActionProfileMember> members,
- PiActionProfileId actionProfileId) {
- this.id = id;
+ private PiActionProfileGroup(PiActionProfileGroupId groupId,
+ ImmutableMap<PiActionProfileMemberId, WeightedMember> members,
+ PiActionProfileId actionProfileId,
+ int maxSize) {
+ this.groupId = groupId;
this.members = members;
this.actionProfileId = actionProfileId;
+ this.maxSize = maxSize;
}
/**
- * Returns the identifier of this action profile group.
+ * Returns the ID of this action profile group.
*
- * @return action profile group identifier
+ * @return action profile group ID
*/
public PiActionProfileGroupId id() {
- return id;
+ return groupId;
}
/**
- * Returns the members of this action profile group.
+ * Returns the list of member references of this action profile group.
*
* @return collection of action profile members.
*/
- public Collection<PiActionProfileMember> members() {
- return members;
+ public Collection<WeightedMember> members() {
+ return members.values();
}
/**
- * Gets identifier of the action profile.
+ * Returns the group member identified by the given action profile member
+ * ID, if present.
*
- * @return action profile id
+ * @param memberId action profile member ID
+ * @return optional group member
*/
- public PiActionProfileId actionProfileId() {
+ public Optional<WeightedMember> member(PiActionProfileMemberId memberId) {
+ return Optional.of(members.get(memberId));
+ }
+
+ /**
+ * Returns the maximum number of members that this group can hold. 0
+ * signifies that a limit is not set.
+ *
+ * @return maximum number of members that this group can hold
+ */
+ public int maxSize() {
+ return maxSize;
+ }
+
+ /**
+ * Returns the ID of the action profile where this group belong.
+ *
+ * @return action profile ID
+ */
+ public PiActionProfileId actionProfile() {
return actionProfileId;
}
@Override
- public boolean equals(Object o) {
- if (this == o) {
+ public boolean equals(Object obj) {
+ if (this == obj) {
return true;
}
- if (o == null || !(o instanceof PiActionProfileGroup)) {
+ if (obj == null || getClass() != obj.getClass()) {
return false;
}
- PiActionProfileGroup that = (PiActionProfileGroup) o;
- return Objects.equal(id, that.id) &&
- Objects.equal(members, that.members) &&
- Objects.equal(actionProfileId, that.actionProfileId);
+ final PiActionProfileGroup other = (PiActionProfileGroup) obj;
+ return Objects.equal(this.groupId, other.groupId)
+ && Objects.equal(this.members, other.members)
+ && Objects.equal(this.maxSize, other.maxSize)
+ && Objects.equal(this.actionProfileId, other.actionProfileId);
}
@Override
public int hashCode() {
- return Objects.hashCode(id, members);
+ return Objects.hashCode(groupId, members, maxSize, actionProfileId);
}
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
- .add("groupId", id)
+ .add("actionProfile", actionProfileId)
+ .add("id", groupId)
.add("members", members)
- .add("piActionProfileId", actionProfileId)
+ .add("maxSize", maxSize)
.toString();
}
@@ -120,55 +148,115 @@
*/
public static final class Builder {
- private PiActionProfileGroupId id;
- private Map<PiActionProfileMemberId, PiActionProfileMember> members = Maps.newHashMap();
- private PiActionProfileId piActionProfileId;
+ private PiActionProfileGroupId groupId;
+ private Map<PiActionProfileMemberId, WeightedMember> members = Maps.newHashMap();
+ private PiActionProfileId actionProfileId;
+ private int maxSize;
private Builder() {
// hides constructor.
}
/**
- * Sets the identifier of this action profile group.
+ * Sets the ID of this action profile group.
*
- * @param id action profile group identifier
+ * @param id action profile group ID
* @return this
*/
public Builder withId(PiActionProfileGroupId id) {
- this.id = id;
+ this.groupId = id;
return this;
}
/**
- * Adds one member to this action profile group.
+ * Adds one member to this action profile.
*
- * @param member action profile member
+ * @param member member to add
* @return this
*/
- public Builder addMember(PiActionProfileMember member) {
+ public Builder addMember(WeightedMember member) {
+ checkNotNull(member);
members.put(member.id(), member);
return this;
}
/**
- * Adds many members to this action profile group.
+ * Adds one member to this action profile group with default weight.
*
- * @param members action profile members
+ * @param memberId ID of the action profile member to add
* @return this
*/
- public Builder addMembers(Collection<PiActionProfileMember> members) {
- members.forEach(this::addMember);
+ public Builder addMember(PiActionProfileMemberId memberId) {
+ addMember(new WeightedMember(memberId, WeightedMember.DEFAULT_WEIGHT));
return this;
}
/**
- * Sets the identifier of the action profile.
+ * Adds one member to this action profile group with default weight.
*
- * @param piActionProfileId the identifier of the action profile
+ * @param memberInstance the action profile member instance to add
+ * @return this
+ */
+ public Builder addMember(PiActionProfileMember memberInstance) {
+ addMember(new WeightedMember(memberInstance, WeightedMember.DEFAULT_WEIGHT));
+ return this;
+ }
+
+ /**
+ * Adds all members to this action profile group with default weight.
+ *
+ * @param memberInstances the action profile member instance to add
+ * @return this
+ */
+ public Builder addMembers(Iterable<PiActionProfileMember> memberInstances) {
+ memberInstances.forEach(this::addMember);
+ return this;
+ }
+
+ /**
+ * Adds one member to this action profile group with the given weight.
+ *
+ * @param memberId ID of the action profile member to add
+ * @param weight weight
+ * @return this
+ */
+ public Builder addMember(PiActionProfileMemberId memberId, int weight) {
+ addMember(new WeightedMember(memberId, weight));
+ return this;
+ }
+
+ /**
+ * Adds one member to this action profile group with the given weight.
+ *
+ * @param memberInstance the action profile member instance to add
+ * @param weight weight
+ * @return this
+ */
+ public Builder addMember(PiActionProfileMember memberInstance, int weight) {
+ addMember(new WeightedMember(memberInstance, weight));
+ return this;
+ }
+
+ /**
+ * Sets the ID of the action profile.
+ *
+ * @param piActionProfileId the ID of the action profile
* @return this
*/
public Builder withActionProfileId(PiActionProfileId piActionProfileId) {
- this.piActionProfileId = piActionProfileId;
+ this.actionProfileId = piActionProfileId;
+ return this;
+ }
+
+ /**
+ * Sets the maximum number of members that this group can hold.
+ *
+ * @param maxSize maximum number of members that this group can hold
+ * @return this
+ */
+ public Builder withMaxSize(int maxSize) {
+ checkArgument(maxSize >= 0, "maxSize cannot be negative");
+ this.maxSize = maxSize;
return this;
}
@@ -178,10 +266,119 @@
* @return action profile group
*/
public PiActionProfileGroup build() {
- checkNotNull(id);
- checkNotNull(piActionProfileId);
+ checkNotNull(groupId);
+ checkNotNull(actionProfileId);
+ checkArgument(maxSize == 0 || members.size() <= maxSize,
+ "The number of members cannot exceed maxSize");
+ final boolean validActionProfileId = members.isEmpty() || members.values()
+ .stream().allMatch(m -> m.instance() == null || m.instance()
+ .actionProfile().equals(actionProfileId));
+ checkArgument(
+ validActionProfileId,
+ "The members' action profile ID must match the group one");
return new PiActionProfileGroup(
- id, ImmutableSet.copyOf(members.values()), piActionProfileId);
+ groupId, ImmutableMap.copyOf(members), actionProfileId, maxSize);
+ }
+ }
+
+ /**
+ * Weighted reference to an action profile member as used in an action
+ * profile group.
+ */
+ public static final class WeightedMember {
+
+ public static final int DEFAULT_WEIGHT = 1;
+
+ private final PiActionProfileMemberId memberId;
+ private final int weight;
+ private final PiActionProfileMember memberInstance;
+
+ /**
+ * Creates a new reference for the given action profile member ID and
+ * weight.
+ *
+ * @param memberId action profile member ID
+ * @param weight weight
+ */
+ public WeightedMember(PiActionProfileMemberId memberId, int weight) {
+ checkNotNull(memberId);
+ this.memberId = memberId;
+ this.weight = weight;
+ this.memberInstance = null;
+ }
+
+ /**
+ * Creates a new reference from the given action profile member instance
+ * and weight. This constructor should be used when performing one-shot
+ * group programming (see {@link #instance()}).
+ *
+ * @param memberInstance action profile member instance
+ * @param weight weight
+ */
+ public WeightedMember(PiActionProfileMember memberInstance, int weight) {
+ checkNotNull(memberInstance);
+ this.memberId = memberInstance.id();
+ this.weight = weight;
+ this.memberInstance = memberInstance;
+ }
+
+ /**
+ * Returns the ID of the action profile member.
+ *
+ * @return action profile member ID
+ */
+ public PiActionProfileMemberId id() {
+ return memberId;
+ }
+
+ /**
+ * Returns the weight of this group member.
+ *
+ * @return weight
+ */
+ public int weight() {
+ return weight;
+ }
+
+ /**
+ * If present, returns the instance of the action profile member pointed
+ * by this reference, otherwise returns null. This method is provided as
+ * a convenient way to perform one-shot group programming, and as such
+ * is meaningful only when performing write operations to a device. In
+ * other words, when reading groups from a device only the member
+ * reference should be returned and not the actual instance, hence this
+ * method should return null.
+ *
+ * @return action profile member instance, or null
+ */
+ public PiActionProfileMember instance() {
+ return memberInstance;
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(memberId, weight);
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (obj == null || getClass() != obj.getClass()) {
+ return false;
+ }
+ final WeightedMember other = (WeightedMember) obj;
+ return Objects.equal(this.memberId, other.memberId)
+ && Objects.equal(this.weight, other.weight);
+ }
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("memberId", memberId)
+ .add("weight", weight)
+ .toString();
}
}
}
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileGroupHandle.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileGroupHandle.java
index 42ed272..519a1c4 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileGroupHandle.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileGroupHandle.java
@@ -34,7 +34,7 @@
private PiActionProfileGroupHandle(DeviceId deviceId, PiActionProfileGroup group) {
super(deviceId);
- actionProfileId = group.actionProfileId();
+ actionProfileId = group.actionProfile();
groupId = group.id();
}
@@ -81,7 +81,7 @@
public String toString() {
return MoreObjects.toStringHelper(this)
.add("deviceId", deviceId())
- .add("actionProfileId", actionProfileId)
+ .add("actionProfile", actionProfileId)
.add("groupId", groupId)
.toString();
}
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileMember.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileMember.java
index 92c3562..b5df9d0 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileMember.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileMember.java
@@ -32,19 +32,13 @@
private final PiActionProfileId actionProfileId;
private final PiActionProfileMemberId memberId;
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 PiActionProfileMember(PiActionProfileId actionProfileId,
PiActionProfileMemberId memberId,
- PiAction action,
- int weight) {
+ PiAction action) {
this.actionProfileId = actionProfileId;
this.memberId = memberId;
this.action = action;
- this.weight = weight;
}
/**
@@ -74,15 +68,6 @@
return action;
}
- /**
- * Returns the weight associated to this member.
- *
- * @return weight
- */
- public int weight() {
- return weight;
- }
-
@Override
public PiEntityType piEntityType() {
return PiEntityType.ACTION_PROFILE_MEMBER;
@@ -97,15 +82,14 @@
return false;
}
PiActionProfileMember that = (PiActionProfileMember) o;
- return weight == that.weight &&
- Objects.equal(actionProfileId, that.actionProfileId) &&
+ return Objects.equal(actionProfileId, that.actionProfileId) &&
Objects.equal(memberId, that.memberId) &&
Objects.equal(action, that.action);
}
@Override
public int hashCode() {
- return Objects.hashCode(actionProfileId, memberId, action, weight);
+ return Objects.hashCode(actionProfileId, memberId, action);
}
@Override
@@ -114,7 +98,6 @@
.add("actionProfile", actionProfileId)
.add("id", memberId)
.add("action", action)
- .add("weight", weight)
.toString();
}
@@ -133,9 +116,8 @@
public static final class Builder {
private PiActionProfileId actionProfileId;
- private PiActionProfileMemberId id;
+ private PiActionProfileMemberId memberId;
private PiAction action;
- private int weight;
private Builder() {
// Hides constructor.
@@ -159,7 +141,7 @@
* @return this
*/
public Builder withId(PiActionProfileMemberId id) {
- this.id = id;
+ this.memberId = id;
return this;
}
@@ -175,28 +157,15 @@
}
/**
- * Sets the weight of this member.
- * <p>
- * Default value is 0.
- *
- * @param weight weight
- * @return this
- */
- public Builder withWeight(int weight) {
- this.weight = weight;
- return this;
- }
-
- /**
* Creates a new action profile member.
*
* @return action profile member
*/
public PiActionProfileMember build() {
checkNotNull(actionProfileId);
- checkNotNull(id);
+ checkNotNull(memberId);
checkNotNull(action);
- return new PiActionProfileMember(actionProfileId, id, action, weight);
+ return new PiActionProfileMember(actionProfileId, memberId, action);
}
}
}
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileMemberHandle.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileMemberHandle.java
index 8771650..40cb960 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileMemberHandle.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiActionProfileMemberHandle.java
@@ -24,17 +24,17 @@
import static com.google.common.base.Preconditions.checkNotNull;
/**
- * Global identifier of a PI action profile group member, uniquely defined by a
+ * Global identifier of a PI action profile member, uniquely defined by a
* device ID, action profile ID, and member ID.
*/
public final class PiActionProfileMemberHandle extends PiHandle<PiActionProfileMember> {
- private final PiActionProfileMemberId memberId;
private final PiActionProfileId actionProfileId;
+ private final PiActionProfileMemberId memberId;
private PiActionProfileMemberHandle(DeviceId deviceId,
- PiActionProfileId actionProfileId,
- PiActionProfileMemberId memberId) {
+ PiActionProfileId actionProfileId,
+ PiActionProfileMemberId memberId) {
super(deviceId);
this.actionProfileId = actionProfileId;
this.memberId = memberId;
@@ -119,7 +119,7 @@
public String toString() {
return MoreObjects.toStringHelper(this)
.add("deviceId", deviceId())
- .add("actionProfileId", actionProfileId)
+ .add("actionProfile", actionProfileId)
.add("memberId", memberId)
.toString();
}