Message ID | 20220614115743.1143341-6-emma.finn@intel.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [ovs-dev,v7,01/11] ofproto-dpif: Fix incorrect checksums in input packets | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
ovsrobot/intel-ovs-compilation | success | test: success |
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 <emma.finn@intel.com> > Co-authored-by: Kumar Amber <kumar.amber@intel.com> > Signed-off-by: Kumar Amber <kumar.amber@intel.com> > Acked-by: Harry van Haaren <harry.van.haaren@intel.com> > --- > 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
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", + argv[1]); + unixctl_command_reply_error(conn, ds_cstr(&reply)); + } else { + ds_put_format(&reply, "Action implementation set to %s.\n", argv[1]); + 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