[ovs-dev] [v13 06/12] dpif-netdev: Add command to get dpif implementations.
Ferriter, Cian
cian.ferriter at intel.com
Thu Jun 24 11:54:18 UTC 2021
Hi Flavio,
Thanks for the review. My responses are inline.
Cian
> -----Original Message-----
> From: Flavio Leitner <fbl at sysclose.org>
> Sent: Wednesday 23 June 2021 20:21
> To: Ferriter, Cian <cian.ferriter at intel.com>
> Cc: ovs-dev at openvswitch.org; i.maximets at ovn.org
> Subject: Re: [ovs-dev] [v13 06/12] dpif-netdev: Add command to get dpif implementations.
>
> On Thu, Jun 17, 2021 at 05:18:19PM +0100, Cian Ferriter wrote:
> > From: Harry van Haaren <harry.van.haaren at intel.com>
> >
> > This commit adds a new command to retrieve the list of available
> > DPIF implementations. This can be used by to check what implementations
> > of the DPIF are available in any given OVS binary.
> >
> > Usage:
> > $ ovs-appctl dpif-netdev/dpif-get
>
> I didn't mention this in the dpif-set but it would be great
> to have a more targeted command name, like dpif-impl-{get,set}.
>
I'll rename the commands to the above format, thanks for the suggestion.
> >
> > Signed-off-by: Harry van Haaren <harry.van.haaren at intel.com>
> >
> > ---
> >
> > v13:
> > - Add NEWS item about DPIF get and set commands here rather than in a
> > later commit.
> > - Add documentation items about DPIF set commands here rather than in a
> > later commit.
> > ---
> > Documentation/topics/dpdk/bridge.rst | 8 ++++++++
> > NEWS | 1 +
> > lib/dpif-netdev-private-dpif.c | 8 ++++++++
> > lib/dpif-netdev-private-dpif.h | 6 ++++++
> > lib/dpif-netdev-unixctl.man | 3 +++
> > lib/dpif-netdev.c | 24 ++++++++++++++++++++++++
> > 6 files changed, 50 insertions(+)
> >
> > diff --git a/Documentation/topics/dpdk/bridge.rst b/Documentation/topics/dpdk/bridge.rst
> > index fafa8c821..f59e26cbe 100644
> > --- a/Documentation/topics/dpdk/bridge.rst
> > +++ b/Documentation/topics/dpdk/bridge.rst
> > @@ -226,6 +226,14 @@ stats associated with the datapath.
> > Just like with the SIMD DPCLS feature above, SIMD can be applied to the DPIF to
> > improve performance.
> >
> > +OVS provides multiple implementations of the DPIF. The available
> > +implementations can be listed with the following command ::
> > +
> > + $ ovs-appctl dpif-netdev/dpif-get
> > + Available DPIF implementations:
> > + dpif_scalar
> > + dpif_avx512
> > +
> > By default, dpif_scalar is used. The DPIF implementation can be selected by
> > name ::
> >
> > diff --git a/NEWS b/NEWS
> > index 6a4a7b76d..c47ab349e 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -12,6 +12,7 @@ Post-v2.15.0
> > * Refactor lib/dpif-netdev.c to multiple header files.
> > * Add avx512 implementation of dpif which can process non recirculated
> > packets. It supports partial HWOL, EMC, SMC and DPCLS lookups.
> > + * Add commands to get and set the dpif implementations.
> > - ovs-ctl:
> > * New option '--no-record-hostname' to disable hostname configuration
> > in ovsdb on startup.
> > diff --git a/lib/dpif-netdev-private-dpif.c b/lib/dpif-netdev-private-dpif.c
> > index d829a7ee5..3649e775d 100644
> > --- a/lib/dpif-netdev-private-dpif.c
> > +++ b/lib/dpif-netdev-private-dpif.c
> > @@ -73,6 +73,14 @@ dp_netdev_impl_set_default(dp_netdev_input_func func)
> > default_dpif_func = func;
> > }
> >
> > +uint32_t
> > +dp_netdev_impl_get(const struct dpif_netdev_impl_info_t **out_impls)
> > +{
> > + ovs_assert(out_impls);
> > + *out_impls = dpif_impls;
> > + return ARRAY_SIZE(dpif_impls);
> > +}
> > +
>
> This could receive struct ds and fill with the internal details
> to keep internal details in private-dpif.c
>
>
Yes, hiding the details is a good suggestion. I'll fix this.
> > /* This function checks all available DPIF implementations, and selects the
> > * returns the function pointer to the one requested by "name".
> > */
> > diff --git a/lib/dpif-netdev-private-dpif.h b/lib/dpif-netdev-private-dpif.h
> > index a6db3c7f2..717e9e2f9 100644
> > --- a/lib/dpif-netdev-private-dpif.h
> > +++ b/lib/dpif-netdev-private-dpif.h
> > @@ -48,6 +48,12 @@ struct dpif_netdev_impl_info_t {
> > const char *name;
> > };
> >
> > +/* This function returns all available implementations to the caller. The
> > + * quantity of implementations is returned by the int return value.
> > + */
> > +uint32_t
> > +dp_netdev_impl_get(const struct dpif_netdev_impl_info_t **out_impls);
> > +
> > /* This function checks all available DPIF implementations, and selects the
> > * returns the function pointer to the one requested by "name".
> > */
> > diff --git a/lib/dpif-netdev-unixctl.man b/lib/dpif-netdev-unixctl.man
> > index b348940b0..534823879 100644
> > --- a/lib/dpif-netdev-unixctl.man
> > +++ b/lib/dpif-netdev-unixctl.man
> > @@ -227,5 +227,8 @@ When this is the case, the above command prints the load-balancing information
> > of the bonds configured in datapath \fIdp\fR showing the interface associated
> > with each bucket (hash).
> > .
> > +.IP "\fBdpif-netdev/dpif-get\fR
> > +Lists the DPIF implementations that are available.
> > +.
> > .IP "\fBdpif-netdev/dpif-set\fR \fIdpif_impl\fR"
> > Sets the DPIF to be used to \fIdpif_impl\fR. By default "dpif_scalar" is used.
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 9c234ef3d..59a44a848 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -991,6 +991,27 @@ dpif_netdev_subtable_lookup_set(struct unixctl_conn *conn, int argc,
> > ds_destroy(&reply);
> > }
> >
> > +static void
> > +dpif_netdev_impl_get(struct unixctl_conn *conn, int argc OVS_UNUSED,
> > + const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
> > +{
> > + const struct dpif_netdev_impl_info_t *dpif_impls;
>
> then here you initialize 'reply', call dp_netdev_impl_get() and
> reply if it succeed or report an error.
>
> Does that make sense?
>
It makes sense. I'll do that. Thanks for the suggestion!
> Thanks,
> fbl
>
> > + uint32_t count = dp_netdev_impl_get(&dpif_impls);
> > + if (count == 0) {
> > + unixctl_command_reply_error(conn, "error getting dpif names");
> > + return;
> > + }
> > +
> > + /* Add all dpif functions to reply string. */
> > + struct ds reply = DS_EMPTY_INITIALIZER;
> > + ds_put_cstr(&reply, "Available DPIF implementations:\n");
> > + for (uint32_t i = 0; i < count; i++) {
> > + ds_put_format(&reply, " %s\n", dpif_impls[i].name);
> > + }
> > + unixctl_command_reply(conn, ds_cstr(&reply));
> > + ds_destroy(&reply);
> > +}
> > +
> > static void
> > dpif_netdev_impl_set(struct unixctl_conn *conn, int argc,
> > const char *argv[], void *aux OVS_UNUSED)
> > @@ -1292,6 +1313,9 @@ dpif_netdev_init(void)
> > "dpif_implementation_name [dp]",
> > 1, 2, dpif_netdev_impl_set,
> > NULL);
> > + unixctl_command_register("dpif-netdev/dpif-get", "",
> > + 0, 0, dpif_netdev_impl_get,
> > + NULL);
> > return 0;
> > }
> >
> > --
> > 2.32.0
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> --
> fbl
More information about the dev
mailing list