[P4Runtime] Make sure write requests contain the codec failure reason
The submit() method of p4 WriteRequestImpl is returning an empty
response if there are no update messages. However, one of the reasons
for updates to be empty is the fact the catch block in
appendToRequestMsg is reached due to some invalid usage of the p4runtime
contract. In such situations, not only the user doesn't know why the
request is failing (absence of logging in ONOS) but the responseBuilder
which contains the failure is also not propagated. As a result, a future
call to P4RuntimeWriteClient.WriteResponse.isSuccess() will return
true (as if the request actually succeeded) and .all() will also not
contain the failedResponse appended during the CodecException. Added a
test to illustrate the issue.
Change-Id: I0acfd3b34b3ed1db2d91f91fed08f9d00800dda4
(cherry picked from commit 89e3d23d2b865d1f94c3f5d31c792ffe33d01311)
diff --git a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/client/WriteRequestImpl.java b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/client/WriteRequestImpl.java
index 2201948..45e2c75 100644
--- a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/client/WriteRequestImpl.java
+++ b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/client/WriteRequestImpl.java
@@ -169,7 +169,7 @@
client.deviceId(), writeRequest.getUpdatesCount());
if (writeRequest.getUpdatesCount() == 0) {
// No need to ask the server.
- return completedFuture(WriteResponseImpl.EMPTY);
+ return completedFuture(responseBuilder.buildAsIs());
}
final CompletableFuture<P4RuntimeWriteClient.WriteResponse> future =
new CompletableFuture<>();
@@ -243,6 +243,7 @@
piEntity == null ? handle : piEntity);
}
} catch (CodecException e) {
+ log.error("Failed to add {} to write request for {}: {}", updateType, handle.deviceId(), e.getMessage());
responseBuilder.addFailedResponse(
handle, piEntity, updateType, e.getMessage(),
P4RuntimeWriteClient.EntityUpdateStatus.CODEC_ERROR);
diff --git a/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/P4RuntimeGroupTest.java b/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/P4RuntimeGroupTest.java
index 146f102..b7e806b 100644
--- a/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/P4RuntimeGroupTest.java
+++ b/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/P4RuntimeGroupTest.java
@@ -45,6 +45,7 @@
import org.onosproject.net.pi.runtime.PiActionProfileGroupId;
import org.onosproject.net.pi.runtime.PiActionProfileMember;
import org.onosproject.net.pi.runtime.PiActionProfileMemberId;
+import org.onosproject.p4runtime.api.P4RuntimeWriteClient;
import org.onosproject.p4runtime.ctl.client.P4RuntimeClientImpl;
import org.onosproject.p4runtime.ctl.controller.P4RuntimeControllerImpl;
import p4.v1.P4RuntimeOuterClass.ActionProfileGroup;
@@ -168,6 +169,27 @@
}
@Test
+ public void testInvalidPiActionProfileMember() {
+ PiActionParam param = new PiActionParam(PORT_PARAM_ID, "invalidString");
+ PiAction piAction = PiAction.builder()
+ .withId(EGRESS_PORT_ACTION_ID)
+ .withParameter(param).build();
+ PiActionProfileMember actionProfileMember = PiActionProfileMember.builder()
+ .forActionProfile(ACT_PROF_ID)
+ .withAction(piAction)
+ .withId(PiActionProfileMemberId.of(BASE_MEM_ID + 1))
+ .build();
+ P4RuntimeWriteClient.WriteRequest writeRequest = client.write(P4_DEVICE_ID, PIPECONF);
+ writeRequest.insert(actionProfileMember);
+ P4RuntimeWriteClient.WriteResponse response = writeRequest.submitSync();
+
+ assertEquals(false, response.isSuccess());
+ assertEquals(1, response.all().size());
+ assertEquals("Wrong size for param 'port' of action 'set_egress_port', expected 2 bytes, but found 13",
+ response.all().iterator().next().explanation());
+ }
+
+ @Test
public void testInsertPiActionProfileGroup() throws Exception {
CompletableFuture<Void> complete = p4RuntimeServerImpl.expectRequests(1);
client.write(P4_DEVICE_ID, PIPECONF).insert(GROUP).submitSync();