On 14 Jun 2022, at 13:57, Emma Finn wrote:
> This commit adds a new command to allow the user to switch
> the active action implementation at runtime. A probe function
> is executed before switching the implementation, to ensure
> the CPU is capable of running the ISA required.
>
> Usage:
> $ ovs-appctl dpif-netdev/action-impl-set scalar
>
> This commit also adds a new command to retrieve the list of available
> action implementations. This can be used by to check what implementations
> of actions are available and what implementation is active during runtime.
>
> Usage:
> $ ovs-appctl dpif-netdev/action-impl-show
>
> Added separate test-case for ovs-actions show/set commands:
> 1023: PMD - ovs-actions configuration
>
> Signed-off-by: Emma Finn <[email protected]>
> Co-authored-by: Kumar Amber <[email protected]>
> Signed-off-by: Kumar Amber <[email protected]>
> Acked-by: Harry van Haaren <[email protected]>
> ---
> NEWS | 3 +++
> lib/dpif-netdev-unixctl.man | 8 ++++++++
> lib/dpif-netdev.c | 38 +++++++++++++++++++++++++++++++++++++
> lib/odp-execute-private.c | 12 ++++++++++++
> lib/odp-execute-private.h | 3 +++
> lib/odp-execute.h | 2 ++
> tests/pmd.at | 30 +++++++++++++++++++++++++++++
> 7 files changed, 96 insertions(+)
>
> diff --git a/NEWS b/NEWS
> index 3a25f3035..90ceabd63 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -35,6 +35,9 @@ Post-v2.17.0
> - Userspace datapath:
> * Add actions auto-validator function to compare different actions
> implementations against default implementation.
> + * Add command line option to switch between different actions
> + implementations available at run time.
> +
>
>
> v2.17.0 - 17 Feb 2022
> diff --git a/lib/dpif-netdev-unixctl.man b/lib/dpif-netdev-unixctl.man
> index 8cd847416..81ef7d856 100644
> --- a/lib/dpif-netdev-unixctl.man
> +++ b/lib/dpif-netdev-unixctl.man
> @@ -262,3 +262,11 @@ PMDs in the case where no value is specified. By
> default "scalar" is used.
> \fIstudy_cnt\fR defaults to 128 and indicates the number of packets that the
> "study" miniflow implementation must parse before choosing an optimal
> implementation.
> +
> +.IP "\fBdpif-netdev/action-impl-show\fR
> +Lists the actions implementations that are available and highlights the
> +currently enabled one.
> +.
> +.IP "\fBdpif-netdev/action-impl-set\fR \fIaction_impl\fR"
> +Sets the action implementation to any available implementation. By default
> +"scalar" is used.
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 47dd7a1a6..5a35c7ce5 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -63,6 +63,7 @@
> #include "netdev-vport.h"
> #include "netlink.h"
> #include "odp-execute.h"
> +#include "odp-execute-private.h"
> #include "odp-util.h"
> #include "openvswitch/dynamic-string.h"
> #include "openvswitch/list.h"
> @@ -1387,6 +1388,37 @@ error:
> ds_destroy(&reply);
> }
>
> +static void
> +action_impl_set(struct unixctl_conn *conn, int argc OVS_UNUSED,
> + const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
> +{
> + struct ds reply = DS_EMPTY_INITIALIZER;
> +
> + int err = odp_actions_impl_set(argv[1]);
> + if (err) {
> + ds_put_format(&reply,
> + "Error: unknown action implementation, %s,
> specified!\n",
Remove "\n" here.
> + argv[1]);
> + unixctl_command_reply_error(conn, ds_cstr(&reply));
> + } else {
> + ds_put_format(&reply, "Action implementation set to %s.\n", argv[1]);
Remove "\n" here.
> + unixctl_command_reply(conn, ds_cstr(&reply));
> + }
> +
> + ds_destroy(&reply);
> +}
> +
> +static void
> +action_impl_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
> + const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
> +{
> + struct ds reply = DS_EMPTY_INITIALIZER;
> +
> + odp_execute_action_get_info(&reply);
> + unixctl_command_reply(conn, ds_cstr(&reply));
> + ds_destroy(&reply);
> +}
> +
> static void
> dpif_netdev_pmd_rebalance(struct unixctl_conn *conn, int argc,
> const char *argv[], void *aux OVS_UNUSED)
> @@ -1624,6 +1656,12 @@ dpif_netdev_init(void)
> unixctl_command_register("dpif-netdev/miniflow-parser-get", "",
> 0, 0, dpif_miniflow_extract_impl_get,
> NULL);
> + unixctl_command_register("dpif-netdev/action-impl-set", "name",
> + 1, 1, action_impl_set,
> + NULL);
> + unixctl_command_register("dpif-netdev/action-impl-show", "",
> + 0, 0, action_impl_show,
> + NULL);
> return 0;
> }
>
> diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c
> index 267f32c3e..f8d0896b5 100644
> --- a/lib/odp-execute-private.c
> +++ b/lib/odp-execute-private.c
> @@ -68,6 +68,18 @@ odp_execute_action_set(const char *name,
> return -EINVAL;
> }
>
> +void
> +odp_execute_action_get_info(struct ds *string)
> +{
> + ds_put_cstr(string, "Available Actions implementations:\n");
> + for (int i = 0; i < ACTION_IMPL_MAX; i++) {
> + ds_put_format(string, " %s (available: %s, active: %s)\n",
> + action_impls[i].name,
> + action_impls[i].available ? "Yes" : "No",
> + i == active_action_impl_index ? "Yes" : "No");
> + }
> +}
> +
> void
> odp_execute_action_init(void)
> {
> diff --git a/lib/odp-execute-private.h b/lib/odp-execute-private.h
> index d3dc669d1..5322eb8df 100644
> --- a/lib/odp-execute-private.h
> +++ b/lib/odp-execute-private.h
> @@ -85,4 +85,7 @@ int action_autoval_init(struct odp_execute_action_impl
> *self);
> int odp_execute_action_set(const char *name,
> struct odp_execute_action_impl *active);
>
> +void odp_execute_action_get_info(struct ds *name);
> +
> +
> #endif /* ODP_EXTRACT_PRIVATE */
> diff --git a/lib/odp-execute.h b/lib/odp-execute.h
> index 50d47b716..8668ab73f 100644
> --- a/lib/odp-execute.h
> +++ b/lib/odp-execute.h
> @@ -23,6 +23,7 @@
> #include <stdint.h>
> #include "openvswitch/types.h"
>
> +struct ds;
> struct nlattr;
> struct dp_packet;
> struct pkt_metadata;
> @@ -36,6 +37,7 @@ typedef void (*odp_execute_action_cb)(struct
> dp_packet_batch *batch,
> const struct nlattr *action);
>
> int odp_actions_impl_set(const char *name);
> +int odp_actions_impl_get(struct ds *name);
>
> typedef void (*odp_execute_cb)(void *dp, struct dp_packet_batch *batch,
> const struct nlattr *action, bool
> should_steal);
> diff --git a/tests/pmd.at b/tests/pmd.at
> index e6b173dab..ac05f5f7d 100644
> --- a/tests/pmd.at
> +++ b/tests/pmd.at
> @@ -1200,3 +1200,33 @@ ovs-appctl: ovs-vswitchd: server returned an error
>
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> +
> +AT_SETUP([PMD - ovs-actions configuration])
> +OVS_VSWITCHD_START([], [], [], [--dummy-numa 0,0])
> +AT_CHECK([ovs-vsctl add-port br0 p1 -- set Interface p1 type=dummy-pmd])
> +
> +dnl Scalar impl is set by default.
> +AT_CHECK([ovs-vsctl show], [], [stdout])
> +AT_CHECK([ovs-appctl dpif-netdev/action-impl-show | grep "scalar"], [], [dnl
> + scalar (available: Yes, active: Yes)
> +])
> +
> +AT_CHECK([ovs-appctl dpif-netdev/action-impl-show | grep "autovalidator"],
> [], [dnl
> + autovalidator (available: Yes, active: No)
> +])
> +
> +dnl Set the autovalidator impl to active.
> +AT_CHECK([ovs-appctl dpif-netdev/action-impl-set autovalidator], [0], [dnl
> +Action implementation set to autovalidator.
> +])
> +
> +AT_CHECK([ovs-appctl dpif-netdev/action-impl-show | grep "scalar"], [], [dnl
> + scalar (available: Yes, active: No)
> +])
> +
> +AT_CHECK([ovs-appctl dpif-netdev/action-impl-show | grep "autovalidator"],
> [], [dnl
> + autovalidator (available: Yes, active: Yes)
> +])
> +
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> --
> 2.32.0
The above test was failing due to a major issue in the next patch.
Here is the diff required to make these tests work (I've also added the
negative tests I requested in v6):
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 5a35c7ce5..3e688fb8b 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1397,11 +1397,11 @@ action_impl_set(struct unixctl_conn *conn, int argc
OVS_UNUSED,
int err = odp_actions_impl_set(argv[1]);
if (err) {
ds_put_format(&reply,
- "Error: unknown action implementation, %s, specified!\n",
+ "Error: unknown action implementation, %s, specified!",
argv[1]);
unixctl_command_reply_error(conn, ds_cstr(&reply));
} else {
- ds_put_format(&reply, "Action implementation set to %s.\n", argv[1]);
+ ds_put_format(&reply, "Action implementation set to %s.", argv[1]);
unixctl_command_reply(conn, ds_cstr(&reply));
}
diff --git a/tests/pmd.at b/tests/pmd.at
index ac05f5f7d..643fa9285 100644
--- a/tests/pmd.at
+++ b/tests/pmd.at
@@ -1205,6 +1205,11 @@ AT_SETUP([PMD - ovs-actions configuration])
OVS_VSWITCHD_START([], [], [], [--dummy-numa 0,0])
AT_CHECK([ovs-vsctl add-port br0 p1 -- set Interface p1 type=dummy-pmd])
+dnl Set the scalar first, so we always have the scalar impl as Active.
+AT_CHECK([ovs-appctl dpif-netdev/action-impl-set scalar], [0], [dnl
+Action implementation set to scalar.
+])
+
-dnl Scalar impl is set by default.
-AT_CHECK([ovs-vsctl show], [], [stdout])
AT_CHECK([ovs-appctl dpif-netdev/action-impl-show | grep "scalar"], [], [dnl
@@ -1228,5 +1233,11 @@ AT_CHECK([ovs-appctl dpif-netdev/action-impl-show | grep
"autovalidator"], [], [
autovalidator (available: Yes, active: Yes)
])
-OVS_VSWITCHD_STOP
+dnl Set the autovalidator impl to active.
+AT_CHECK([ovs-appctl dpif-netdev/action-impl-set invalid_implementation], [2],
[], [dnl
+Error: unknown action implementation, invalid_implementation, specified!
+ovs-appctl: ovs-vswitchd: server returned an error
+])
+
+OVS_VSWITCHD_STOP(["/Failed setting action implementation to
invalid_implementation/d"])
AT_CLEANUP
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
