Refactor channel and mastership handling in P4Runtime
This (big) change aims at solving the issue observed with mastership flapping
and device connection/disconnection with P4Runtime.
Channel handling is now based on the underlying gRPC channel state. Before,
channel events (open/close/error) were generated as a consequence of P4Runtime
StreamChannel events, making device availability dependent on mastership. Now
Stream Channel events only affect mastership (MASTER/STANDBY or NONE when the
SteamChannel RPC is not active).
Mastership handling has been refactored to generate P4Runtime election IDs that
are compatible with the mastership preference decided by the MastershipService.
GeneralDeviceProvider has been re-implemented to support in-order
device event processing and to reduce implementation complexity. Stats polling
has been moved to a separate component, and netcfg handling updated to only
depend on BasicDeviceConfig, augmented with a pipeconf field, and re-using the
managementAddress field to set the gRPC server endpoints (e.g.
grpc://myswitch.local:50051). Before it was depending on 3 different config
classes, making hard to detect changes.
Finally, this change affects some core interfaces:
- Adds a method to DeviceProvider and DeviceHandshaker to check for device
availability, making the meaning of availability device-specific. This is needed
in cases where the device manager needs to change the availability state of a
device (as in change #20842)
- Support device providers not capable of reconciling mastership role responses
with requests (like P4Runtime).
- Clarify the meaning of "connection" in the DeviceConnect behavior.
- Allows driver-based providers to check devices for reachability and
availability without probing the device via the network.
Change-Id: I7ff30d29f5d02ad938e3171536e54ae2916629a2
diff --git a/core/api/src/main/java/org/onosproject/net/device/DeviceAgentEvent.java b/core/api/src/main/java/org/onosproject/net/device/DeviceAgentEvent.java
index d2520b0..b214070 100644
--- a/core/api/src/main/java/org/onosproject/net/device/DeviceAgentEvent.java
+++ b/core/api/src/main/java/org/onosproject/net/device/DeviceAgentEvent.java
@@ -54,19 +54,19 @@
ROLE_MASTER,
/**
- * Signifies that the agent has standby/slave mastership role.
+ * Signifies that the agent has acquired standby/slave mastership role.
*/
ROLE_STANDBY,
/**
- * Signifies that the agent cannot acquire any valid mastership role for
+ * Signifies that the agent doesn't have any valid mastership role for
* the device.
*/
ROLE_NONE,
/**
- * Signifies that the agent cannot perform operations on the device
- * because its role is not master.
+ * Signifies that the agent tried to perform some operations on the
+ * device that requires master role.
*/
NOT_MASTER,
@@ -81,15 +81,4 @@
public DeviceAgentEvent(Type type, DeviceId deviceId) {
super(type, deviceId);
}
-
- /**
- * Creates a new device agent event for the given type, device ID and time.
- *
- * @param type event type
- * @param deviceId device ID
- * @param time occurrence time
- */
- public DeviceAgentEvent(Type type, DeviceId deviceId, long time) {
- super(type, deviceId, time);
- }
}
diff --git a/core/api/src/main/java/org/onosproject/net/device/DeviceHandshaker.java b/core/api/src/main/java/org/onosproject/net/device/DeviceHandshaker.java
index bceaeff..8a941ca 100644
--- a/core/api/src/main/java/org/onosproject/net/device/DeviceHandshaker.java
+++ b/core/api/src/main/java/org/onosproject/net/device/DeviceHandshaker.java
@@ -31,13 +31,63 @@
public interface DeviceHandshaker extends DeviceConnect {
/**
- * Checks the reachability (connectivity) of a device. Reachability, unlike
- * availability, denotes whether THIS particular node can send messages and
- * receive replies from the specified device.
+ * Returns true if this node is presumed to be able to send messages and
+ * receive replies from the device.
+ * <p>
+ * The implementation should not make any attempt at actively probing the
+ * device over the network, as such it should not block execution. Instead,
+ * it should return a result based solely on internal state (e.g. socket
+ * state). If it returns true, then this node is expected to communicate
+ * with the server successfully. In other words, if any message would be
+ * sent to the device immediately after this method is called and returns
+ * true, then such message is expected, but NOT guaranteed, to reach the
+ * device. If false, it means communication with the device is unlikely to
+ * happen soon.
+ * <p>
+ * Some implementations might require a connection to be created via {@link
+ * #connect()} before checking for reachability. Similarly, after invoking
+ * {@link #disconnect()}, this method might always return false.
*
- * @return CompletableFuture eventually true if reachable, false otherwise
+ * @return true if the device is deemed reachable, false otherwise
*/
- CompletableFuture<Boolean> isReachable();
+ boolean isReachable();
+
+ /**
+ * Similar to {@link #isReachable()}, but performs probing of the device
+ * over the network. This method should be called if {@link #isReachable()}
+ * returns false and the caller wants to be sure this is not a transient
+ * failure state by actively probing the device.
+ *
+ * @return completable future eventually true if device responded to probe,
+ * false otherwise
+ */
+ CompletableFuture<Boolean> probeReachability();
+
+ /**
+ * Checks the availability of the device. Availability denotes whether the
+ * device is reachable and able to perform its functions as expected (e.g.,
+ * forward traffic). Similar to {@link #isReachable()}, implementations are
+ * not allowed to probe the device over the network, but the result should
+ * be based solely on internal state.
+ * <p>
+ * Implementation of this method is optional. If not supported, an exception
+ * should be thrown.
+ *
+ * @return true if the device is deemed available, false otherwise
+ * @throws UnsupportedOperationException if this method is not supported and
+ * {@link #probeAvailability()} should
+ * be used instead.
+ */
+ boolean isAvailable();
+
+ /**
+ * Similar to {@link #isAvailable()} but allows probing the device over the
+ * network. Differently from {@link #isAvailable()}, implementation of this
+ * method is mandatory.
+ *
+ * @return completable future eventually true if available, false otherwise
+ */
+ CompletableFuture<Boolean> probeAvailability();
/**
* Notifies the device a mastership role change as decided by the core. The
@@ -45,10 +95,48 @@
* signaling the mastership role accepted by the device.
*
* @param newRole new mastership role
+ * @throws UnsupportedOperationException if the device does not support
+ * mastership handling
*/
void roleChanged(MastershipRole newRole);
/**
+ * Notifies the device of a mastership role change as decided by the core.
+ * Differently from {@link #roleChanged(MastershipRole)}, the role is
+ * described by the given preference value, where {@code preference = 0}
+ * signifies {@link MastershipRole#MASTER} role and {@code preference > 0}
+ * signifies {@link MastershipRole#STANDBY}. Smaller preference values
+ * indicates higher mastership priority for different nodes.
+ * <p>
+ * This method does not permit notifying role {@link MastershipRole#NONE},
+ * in which case {@link #roleChanged(MastershipRole)} should be used
+ * instead.
+ * <p>
+ * Term is a monotonically increasing number, increased by one every time a
+ * new master is elected.
+ * <p>
+ * The implementation of this method should trigger a {@link
+ * DeviceAgentEvent} signaling the mastership role accepted by the device.
+ *
+ * @param preference preference value, where 0 signifies {@link
+ * MastershipRole#MASTER} and all other values {@link
+ * MastershipRole#STANDBY}
+ * @param term term number
+ * @throws UnsupportedOperationException if the device does not support
+ * mastership handling, or if it does
+ * not support setting preference-based
+ * mastership, and {@link #roleChanged(MastershipRole)}
+ * should be used instead
+ */
+ default void roleChanged(int preference, long term) {
+ if (preference == 0) {
+ roleChanged(MastershipRole.MASTER);
+ } else {
+ roleChanged(MastershipRole.STANDBY);
+ }
+ }
+
+ /**
* Returns the last known mastership role agreed by the device for this
* node.
*
diff --git a/core/api/src/main/java/org/onosproject/net/device/DeviceProvider.java b/core/api/src/main/java/org/onosproject/net/device/DeviceProvider.java
index ebdf7b1..771a8e4 100644
--- a/core/api/src/main/java/org/onosproject/net/device/DeviceProvider.java
+++ b/core/api/src/main/java/org/onosproject/net/device/DeviceProvider.java
@@ -41,8 +41,8 @@
* Notifies the provider of a mastership role change for the specified
* device as decided by the core.
*
- * @param deviceId device identifier
- * @param newRole newly determined mastership role
+ * @param deviceId device identifier
+ * @param newRole newly determined mastership role
*/
void roleChanged(DeviceId deviceId, MastershipRole newRole);
@@ -50,18 +50,40 @@
* Checks the reachability (connectivity) of a device from this provider.
* Reachability, unlike availability, denotes whether THIS particular node
* can send messages and receive replies from the specified device.
+ * <p>
+ * Implementations are encouraged to check for reachability by using only
+ * internal provider state, i.e., without blocking execution.
*
- * @param deviceId device identifier
+ * @param deviceId device identifier
* @return true if reachable, false otherwise
*/
boolean isReachable(DeviceId deviceId);
/**
- * Administratively enables or disables a port.
+ * Checks the availability of the device from the provider perspective.
+ * Availability denotes whether the device is reachable by
+ * this node and able to perform its functions as expected (e.g., forward
+ * traffic).
+ *
+ * <p>
+ * Implementations are encouraged to check for availability by using only
+ * internal provider state, i.e., without blocking execution.
*
* @param deviceId device identifier
+ * @return completable future eventually true if available, false otherwise
+ */
+ default boolean isAvailable(DeviceId deviceId) {
+ // For most implementations such as OpenFlow, reachability is equivalent
+ // to availability.
+ return isReachable(deviceId);
+ }
+
+ /**
+ * Administratively enables or disables a port.
+ *
+ * @param deviceId device identifier
* @param portNumber port number
- * @param enable true if port is to be enabled, false to disable
+ * @param enable true if port is to be enabled, false to disable
*/
void changePortState(DeviceId deviceId, PortNumber portNumber,
boolean enable);
diff --git a/core/api/src/main/java/org/onosproject/net/device/DeviceProviderService.java b/core/api/src/main/java/org/onosproject/net/device/DeviceProviderService.java
index 80996eb..b00fa63 100644
--- a/core/api/src/main/java/org/onosproject/net/device/DeviceProviderService.java
+++ b/core/api/src/main/java/org/onosproject/net/device/DeviceProviderService.java
@@ -74,15 +74,30 @@
void portStatusChanged(DeviceId deviceId, PortDescription portDescription);
/**
- * Notifies the core about the result of a RoleRequest sent to a device.
+ * Notifies the core about the result of a role request sent to a device.
+ * This method assumes that the provider knows the original role that was
+ * requested for a given response, if that is not the case, and only the
+ * response is known to the provider, then {@link #receivedRoleReply(DeviceId,
+ * MastershipRole)} should be used instead.
*
- * @param deviceId identity of the device
+ * @param deviceId identity of the device
* @param requested mastership role that was requested by the node
- * @param response mastership role the switch accepted
+ * @param response mastership role the switch accepted
*/
void receivedRoleReply(DeviceId deviceId, MastershipRole requested, MastershipRole response);
/**
+ * Notifies the core about a mastership role reported by the given device
+ * for this node.
+ *
+ * @param deviceId identity of the device
+ * @param response mastership role the switch accepted
+ */
+ default void receivedRoleReply(DeviceId deviceId, MastershipRole response) {
+ receivedRoleReply(deviceId, null, response);
+ }
+
+ /**
* Updates statistics about all ports of a device.
*
* @param deviceId identity of the device