| Message ID | 20220112094244.81402-5-emma.finn@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Actions Infrastructure + Optimizations | expand |
> 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-get Hi Emma, In general this look straight forward enough. General query, I assume that scalar is always the default used, and that the user does not actually have to set it to use it by default? The only time the user should have to set it to scalar would be if they were using a different implementation and wished to revert to scalar? Thanks Ian > > Added separate test-case for ovs-actions get/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 | 2 ++ > lib/dpif-netdev-unixctl.man | 6 ++++++ > lib/dpif-netdev.c | 39 +++++++++++++++++++++++++++++++++++++ > lib/odp-execute-private.c | 14 +++++++++++++ > lib/odp-execute.h | 3 +++ > tests/pmd.at | 21 ++++++++++++++++++++ > 6 files changed, 85 insertions(+) > > diff --git a/NEWS b/NEWS > index 26be454df..42bb876da 100644 > --- a/NEWS > +++ b/NEWS > @@ -21,6 +21,8 @@ Post-v2.16.0 > * Add support for running threads on cores >= RTE_MAX_LCORE. > * 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. > - Python: > * For SSL support, the use of the pyOpenSSL library has been replaced > with the native 'ssl' module. > diff --git a/lib/dpif-netdev-unixctl.man b/lib/dpif-netdev-unixctl.man > index 8cd847416..500daf4de 100644 > --- a/lib/dpif-netdev-unixctl.man > +++ b/lib/dpif-netdev-unixctl.man > @@ -262,3 +262,9 @@ 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-get\fR > +Lists the actions implementations that are available. > +. > +.IP "\fBdpif-netdev/action-impl-set\fR \fIaction_impl\fR" > +Sets the action to be used to \fIaction_impl\fR. By default "scalar" is used. > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index eada4fcd7..f6cc779ef 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -60,6 +60,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" > @@ -1330,6 +1331,38 @@ 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; > + > + int32_t err = odp_actions_impl_set(argv[1]); > + if (err) { > + ds_put_format(&reply, "action implementation %s not found.\n", > + argv[1]); > + const char *reply_str = ds_cstr(&reply); > + unixctl_command_reply_error(conn, reply_str); > + VLOG_ERR("%s", reply_str); > + ds_destroy(&reply); > + return; > + } > + > + 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_get(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(&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) > @@ -1567,6 +1600,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-get", "", > + 0, 0, action_impl_get, > + NULL); > return 0; > } > > diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c > index a4155b5df..c17882a33 100644 > --- a/lib/odp-execute-private.c > +++ b/lib/odp-execute-private.c > @@ -74,6 +74,20 @@ odp_execute_action_set(const char *name, > return -1; > } > > +void > +odp_execute_action_get(struct ds *string) > +{ > + uint32_t i; > + > + ds_put_cstr(string, "Available Actions implementations:\n"); > + for (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 ? "True" : "False", > + i == active_action_impl_index ? "True" : "False"); > + } > +} > + > void > odp_execute_action_init(void) > { > diff --git a/lib/odp-execute.h b/lib/odp-execute.h > index 6441392b9..4f4cdc4ac 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; > @@ -32,6 +33,8 @@ struct dp_packet_batch; > /* Called once at initialization time. */ > void odp_execute_init(void); > > +/* Runtime update get/set functionality. */ > +int32_t odp_actions_impl_get(struct ds *name); > int32_t odp_actions_impl_set(const char *name); > > typedef void (*odp_execute_cb)(void *dp, struct dp_packet_batch *batch, > diff --git a/tests/pmd.at b/tests/pmd.at > index a2f9d34a2..df0b3b54c 100644 > --- a/tests/pmd.at > +++ b/tests/pmd.at > @@ -1162,3 +1162,24 @@ 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 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. > +]) > + > +AT_CHECK([ovs-vsctl show], [], [stdout]) > +AT_CHECK([ovs-appctl dpif-netdev/action-impl-get | grep "scalar"], [], [dnl > + scalar (available: True, active: True) > +]) > + > +AT_CHECK([ovs-appctl dpif-netdev/action-impl-get | grep "autovalidator"], [], > [dnl > + autovalidator (available: True, active: False) > +]) > + > +OVS_VSWITCHD_STOP > +AT_CLEANUP > \ No newline at end of file > -- > 2.25.1
> -----Original Message----- > From: Stokes, Ian <ian.stokes@intel.com> > Sent: Wednesday 12 January 2022 18:45 > To: Finn, Emma <emma.finn@intel.com>; dev@openvswitch.org; Van Haaren, > Harry <harry.van.haaren@intel.com>; Amber, Kumar > <kumar.amber@intel.com>; i.maximets@ovn.org > Subject: RE: [PATCH v5 4/8] odp-execute: Add command to switch action > implementation. > > > 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-get > > Hi Emma, > > In general this look straight forward enough. General query, I assume that scalar > is always the default used, and that the user does not actually have to set it to > use it by default? > > The only time the user should have to set it to scalar would be if they were using > a different implementation and wished to revert to scalar? > > Thanks > Ian Correct, Scalar implementation is set by default. The only time the user would have to set it to scalar would be if they have already been using a different implementation. Thanks, Emma > > > > Added separate test-case for ovs-actions get/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 | 2 ++ > > lib/dpif-netdev-unixctl.man | 6 ++++++ > > lib/dpif-netdev.c | 39 +++++++++++++++++++++++++++++++++++++ > > lib/odp-execute-private.c | 14 +++++++++++++ > > lib/odp-execute.h | 3 +++ > > tests/pmd.at | 21 ++++++++++++++++++++ > > 6 files changed, 85 insertions(+) > > > > diff --git a/NEWS b/NEWS > > index 26be454df..42bb876da 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -21,6 +21,8 @@ Post-v2.16.0 > > * Add support for running threads on cores >= RTE_MAX_LCORE. > > * 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. > > - Python: > > * For SSL support, the use of the pyOpenSSL library has been replaced > > with the native 'ssl' module. > > diff --git a/lib/dpif-netdev-unixctl.man b/lib/dpif-netdev-unixctl.man > > index 8cd847416..500daf4de 100644 > > --- a/lib/dpif-netdev-unixctl.man > > +++ b/lib/dpif-netdev-unixctl.man > > @@ -262,3 +262,9 @@ 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-get\fR > > +Lists the actions implementations that are available. > > +. > > +.IP "\fBdpif-netdev/action-impl-set\fR \fIaction_impl\fR" > > +Sets the action to be used to \fIaction_impl\fR. By default "scalar" is used. > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index > > eada4fcd7..f6cc779ef 100644 > > --- a/lib/dpif-netdev.c > > +++ b/lib/dpif-netdev.c > > @@ -60,6 +60,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" > > @@ -1330,6 +1331,38 @@ 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; > > + > > + int32_t err = odp_actions_impl_set(argv[1]); > > + if (err) { > > + ds_put_format(&reply, "action implementation %s not found.\n", > > + argv[1]); > > + const char *reply_str = ds_cstr(&reply); > > + unixctl_command_reply_error(conn, reply_str); > > + VLOG_ERR("%s", reply_str); > > + ds_destroy(&reply); > > + return; > > + } > > + > > + 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_get(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(&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) > > @@ -1567,6 +1600,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-get", "", > > + 0, 0, action_impl_get, > > + NULL); > > return 0; > > } > > > > diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c > > index a4155b5df..c17882a33 100644 > > --- a/lib/odp-execute-private.c > > +++ b/lib/odp-execute-private.c > > @@ -74,6 +74,20 @@ odp_execute_action_set(const char *name, > > return -1; > > } > > > > +void > > +odp_execute_action_get(struct ds *string) { > > + uint32_t i; > > + > > + ds_put_cstr(string, "Available Actions implementations:\n"); > > + for (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 ? "True" : "False", > > + i == active_action_impl_index ? "True" : "False"); > > + } > > +} > > + > > void > > odp_execute_action_init(void) > > { > > diff --git a/lib/odp-execute.h b/lib/odp-execute.h index > > 6441392b9..4f4cdc4ac 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; > > @@ -32,6 +33,8 @@ struct dp_packet_batch; > > /* Called once at initialization time. */ void > > odp_execute_init(void); > > > > +/* Runtime update get/set functionality. */ int32_t > > +odp_actions_impl_get(struct ds *name); > > int32_t odp_actions_impl_set(const char *name); > > > > typedef void (*odp_execute_cb)(void *dp, struct dp_packet_batch > > *batch, diff --git a/tests/pmd.at b/tests/pmd.at index > > a2f9d34a2..df0b3b54c 100644 > > --- a/tests/pmd.at > > +++ b/tests/pmd.at > > @@ -1162,3 +1162,24 @@ 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 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. > > +]) > > + > > +AT_CHECK([ovs-vsctl show], [], [stdout]) AT_CHECK([ovs-appctl > > +dpif-netdev/action-impl-get | grep "scalar"], [], [dnl > > + scalar (available: True, active: True) > > +]) > > + > > +AT_CHECK([ovs-appctl dpif-netdev/action-impl-get | grep > > +"autovalidator"], [], > > [dnl > > + autovalidator (available: True, active: False) > > +]) > > + > > +OVS_VSWITCHD_STOP > > +AT_CLEANUP > > \ No newline at end of file > > -- > > 2.25.1 >
diff --git a/NEWS b/NEWS index 26be454df..42bb876da 100644 --- a/NEWS +++ b/NEWS @@ -21,6 +21,8 @@ Post-v2.16.0 * Add support for running threads on cores >= RTE_MAX_LCORE. * 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. - Python: * For SSL support, the use of the pyOpenSSL library has been replaced with the native 'ssl' module. diff --git a/lib/dpif-netdev-unixctl.man b/lib/dpif-netdev-unixctl.man index 8cd847416..500daf4de 100644 --- a/lib/dpif-netdev-unixctl.man +++ b/lib/dpif-netdev-unixctl.man @@ -262,3 +262,9 @@ 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-get\fR +Lists the actions implementations that are available. +. +.IP "\fBdpif-netdev/action-impl-set\fR \fIaction_impl\fR" +Sets the action to be used to \fIaction_impl\fR. By default "scalar" is used. diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index eada4fcd7..f6cc779ef 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -60,6 +60,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" @@ -1330,6 +1331,38 @@ 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; + + int32_t err = odp_actions_impl_set(argv[1]); + if (err) { + ds_put_format(&reply, "action implementation %s not found.\n", + argv[1]); + const char *reply_str = ds_cstr(&reply); + unixctl_command_reply_error(conn, reply_str); + VLOG_ERR("%s", reply_str); + ds_destroy(&reply); + return; + } + + 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_get(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(&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) @@ -1567,6 +1600,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-get", "", + 0, 0, action_impl_get, + NULL); return 0; } diff --git a/lib/odp-execute-private.c b/lib/odp-execute-private.c index a4155b5df..c17882a33 100644 --- a/lib/odp-execute-private.c +++ b/lib/odp-execute-private.c @@ -74,6 +74,20 @@ odp_execute_action_set(const char *name, return -1; } +void +odp_execute_action_get(struct ds *string) +{ + uint32_t i; + + ds_put_cstr(string, "Available Actions implementations:\n"); + for (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 ? "True" : "False", + i == active_action_impl_index ? "True" : "False"); + } +} + void odp_execute_action_init(void) { diff --git a/lib/odp-execute.h b/lib/odp-execute.h index 6441392b9..4f4cdc4ac 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; @@ -32,6 +33,8 @@ struct dp_packet_batch; /* Called once at initialization time. */ void odp_execute_init(void); +/* Runtime update get/set functionality. */ +int32_t odp_actions_impl_get(struct ds *name); int32_t odp_actions_impl_set(const char *name); typedef void (*odp_execute_cb)(void *dp, struct dp_packet_batch *batch, diff --git a/tests/pmd.at b/tests/pmd.at index a2f9d34a2..df0b3b54c 100644 --- a/tests/pmd.at +++ b/tests/pmd.at @@ -1162,3 +1162,24 @@ 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 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. +]) + +AT_CHECK([ovs-vsctl show], [], [stdout]) +AT_CHECK([ovs-appctl dpif-netdev/action-impl-get | grep "scalar"], [], [dnl + scalar (available: True, active: True) +]) + +AT_CHECK([ovs-appctl dpif-netdev/action-impl-get | grep "autovalidator"], [], [dnl + autovalidator (available: True, active: False) +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP \ No newline at end of file