| Message ID | 20200610104839.54608-4-harry.van.haaren@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | DPCLS Subtable ISA Optimization | expand |
On Wed, Jun 10, 2020 at 3:47 AM Harry van Haaren <harry.van.haaren@intel.com> wrote: > > This commit introduces a new command, "dpif-netdev/subtable-lookup-get" add prio > which prints the avaiable sutable lookup functions available in this OVS typo available subtable > binary. Example output from the command: > > Available lookup functions (priority : name) > 0 : autovalidator > 1 : generic > > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com> > --- The rest looks good to me. William
> -----Original Message----- > From: William Tu <u9012063@gmail.com> > Sent: Tuesday, June 16, 2020 4:42 PM > To: Van Haaren, Harry <harry.van.haaren@intel.com> > Cc: ovs-dev <ovs-dev@openvswitch.org>; Stokes, Ian <ian.stokes@intel.com>; > Ilya Maximets <i.maximets@ovn.org>; Federico Iezzi <fiezzi@redhat.com> > Subject: Re: [PATCH v3 3/7] dpif-netdev: add subtable-lookup-get command for > usability > > On Wed, Jun 10, 2020 at 3:47 AM Harry van Haaren > <harry.van.haaren@intel.com> wrote: > > > > This commit introduces a new command, "dpif-netdev/subtable-lookup-get" > add prio Hey William, Just reviewing your FB in detail here, today the commands work as follows: - the "set" command is "subtable-lookup-prio-set" - the "get" command is "subtable-lookup-get" // Note : no "prio" here This is done on purpose - the get returns the list of subtable lookup implementations, and is not really technically directly to do with "prio" . It does however, return the priority too... Open to suggestions, would the more similar commands below be better? subtable-lookup-prio-get subtable-lookup-prio-set Regards, -Harry
On Wed, Jun 17, 2020 at 8:50 AM Van Haaren, Harry <harry.van.haaren@intel.com> wrote: > > > -----Original Message----- > > From: William Tu <u9012063@gmail.com> > > Sent: Tuesday, June 16, 2020 4:42 PM > > To: Van Haaren, Harry <harry.van.haaren@intel.com> > > Cc: ovs-dev <ovs-dev@openvswitch.org>; Stokes, Ian <ian.stokes@intel.com>; > > Ilya Maximets <i.maximets@ovn.org>; Federico Iezzi <fiezzi@redhat.com> > > Subject: Re: [PATCH v3 3/7] dpif-netdev: add subtable-lookup-get command for > > usability > > > > On Wed, Jun 10, 2020 at 3:47 AM Harry van Haaren > > <harry.van.haaren@intel.com> wrote: > > > > > > This commit introduces a new command, "dpif-netdev/subtable-lookup-get" > > add prio > > Hey William, > > Just reviewing your FB in detail here, today the commands work as follows: > - the "set" command is "subtable-lookup-prio-set" > - the "get" command is "subtable-lookup-get" // Note : no "prio" here > > This is done on purpose - the get returns the list of subtable lookup implementations, > and is not really technically directly to do with "prio" . It does however, return the priority too... I see, thanks > > Open to suggestions, would the more similar commands below be better? > subtable-lookup-prio-get > subtable-lookup-prio-set I don't have strong opinion, but I prefer subtable-lookup-prio-get subtable-lookup-prio-set btw, when we issue ovs-appctl subtable-lookup-get, does it show "for each subtable", its corresponding lookup function? Because each subtable might not always uses the highest prio lookup function, correct? William
> -----Original Message----- > From: William Tu <u9012063@gmail.com> > Sent: Wednesday, June 17, 2020 4:57 PM > To: Van Haaren, Harry <harry.van.haaren@intel.com> > Cc: ovs-dev <ovs-dev@openvswitch.org>; Stokes, Ian <ian.stokes@intel.com>; > Ilya Maximets <i.maximets@ovn.org>; Federico Iezzi <fiezzi@redhat.com> > Subject: Re: [PATCH v3 3/7] dpif-netdev: add subtable-lookup-get command for > usability > > On Wed, Jun 17, 2020 at 8:50 AM Van Haaren, Harry > <harry.van.haaren@intel.com> wrote: > > > > > -----Original Message----- > > > From: William Tu <u9012063@gmail.com> > > > Sent: Tuesday, June 16, 2020 4:42 PM > > > To: Van Haaren, Harry <harry.van.haaren@intel.com> > > > Cc: ovs-dev <ovs-dev@openvswitch.org>; Stokes, Ian > <ian.stokes@intel.com>; > > > Ilya Maximets <i.maximets@ovn.org>; Federico Iezzi <fiezzi@redhat.com> > > > Subject: Re: [PATCH v3 3/7] dpif-netdev: add subtable-lookup-get command > for > > > usability > > > > > > On Wed, Jun 10, 2020 at 3:47 AM Harry van Haaren > > > <harry.van.haaren@intel.com> wrote: > > > > > > > > This commit introduces a new command, "dpif-netdev/subtable-lookup- > get" > > > add prio > > > > Hey William, > > > > Just reviewing your FB in detail here, today the commands work as follows: > > - the "set" command is "subtable-lookup-prio-set" > > - the "get" command is "subtable-lookup-get" // Note : no "prio" here > > > > This is done on purpose - the get returns the list of subtable lookup > implementations, > > and is not really technically directly to do with "prio" . It does however, return > the priority too... > > I see, thanks > > > > Open to suggestions, would the more similar commands below be better? > > subtable-lookup-prio-get > > subtable-lookup-prio-set > I don't have strong opinion, but I prefer > subtable-lookup-prio-get > subtable-lookup-prio-set Great - 1 opinion is > no opinions, will change to the more similar style. > btw, when we issue ovs-appctl subtable-lookup-get, > does it show "for each subtable", its corresponding lookup function? > Because each subtable might not always uses the highest prio lookup > function, correct? Great question - no - today the get command just returns the available lookup function implementations, and NOT the detailed list of what DPCLS subtable is matched with what implementation. I agree this information may be useful - perhaps we could add a -v version of the get command? Or dump it in another dpif-netdev/* command? For example, the "dump miniflow bits" command as Emma/Ian upstreamed in 2.13 might be a good fit? If OK with you, I'll suggest we keep the functionality in this patchset as is to focus on remaining opens & merge it. I'd be happy to look at improving info/visibility into DPCLS subtable accelerations in a follow-up patchset?
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 30806af16..cd4e1dbb1 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -1264,6 +1264,30 @@ sorted_poll_thread_list(struct dp_netdev *dp, *n = k; } +static void +dpif_netdev_subtable_lookup_get(struct unixctl_conn *conn, int argc OVS_UNUSED, + const char *argv[] OVS_UNUSED, + void *aux OVS_UNUSED) +{ + /* Get a list of all lookup functions */ + struct dpcls_subtable_lookup_info_t *lookup_funcs = NULL; + int32_t count = dpcls_subtable_lookup_info_get(&lookup_funcs); + if (count < 0) { + unixctl_command_reply_error(conn, "error getting lookup names"); + return; + } + + /* Add all lookup functions to reply string */ + struct ds reply = DS_EMPTY_INITIALIZER; + ds_put_cstr(&reply, "Available lookup functions (priority : name)\n"); + for (int i = 0; i < count; i++) { + ds_put_format(&reply, "\t%d : %s\n", lookup_funcs[i].prio, + lookup_funcs[i].name); + } + unixctl_command_reply(conn, ds_cstr(&reply)); + ds_destroy(&reply); +} + static void dpif_netdev_subtable_lookup_set(struct unixctl_conn *conn, int argc, const char *argv[], void *aux OVS_UNUSED) @@ -1528,6 +1552,9 @@ dpif_netdev_init(void) "[lookup_func] [prio] [dp]", 2, 3, dpif_netdev_subtable_lookup_set, NULL); + unixctl_command_register("dpif-netdev/subtable-lookup-get", "", + 0, 0, dpif_netdev_subtable_lookup_get, + NULL); return 0; }
This commit introduces a new command, "dpif-netdev/subtable-lookup-get" which prints the avaiable sutable lookup functions available in this OVS binary. Example output from the command: Available lookup functions (priority : name) 0 : autovalidator 1 : generic Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com> --- lib/dpif-netdev.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)