> On 15/07/2021 22:37, Rosemarie O'Riorden wrote:
> > Deprecate current OVS provided defaults for DPDK socket-mem and
> > socket-limit that are planned to be removed in OVS 2.17. At that point
> > DPDK defaults will be used instead. Warnings have been added to alert
> > users in advance.
> >
> 
> A few very minor things in the series, otherwise they lgtm. UT passing,
> GHA passing. thanks.
> 
> +cc for David Wilder.
> 
> David, please see this patchset and as OVS will in future no longer
> construct the --socket-mem/socket-limit values, you might want to check
> that the DPDK defaults are working ok for PPC. If not, you still have
> time to fix them for DPDK 21.11 before OVS 2.17.
> 


I think this patch should be applied to the master branch today in time for 
branch of the 2.16 release. Patch 2 and 3 should be left to master after 2.16 
branches.

@Kevin Traynor Am I ok to add you ack here?

I can make the minor changes below on commit.

Regards
Ian


> > Signed-off-by: Rosemarie O'Riorden <[email protected]>
> > ---
> > Version 3:
> >  - Fixed typo and edited commit message.
> >
> >  Documentation/intro/install/dpdk.rst |  3 ++-
> >  NEWS                                 |  2 ++
> >  lib/dpdk.c                           | 11 +++++++++++
> >  vswitchd/vswitch.xml                 |  8 ++++++--
> >  4 files changed, 21 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/intro/install/dpdk.rst
> b/Documentation/intro/install/dpdk.rst
> > index 612f2fdbc..d8fa931fa 100644
> > --- a/Documentation/intro/install/dpdk.rst
> > +++ b/Documentation/intro/install/dpdk.rst
> > @@ -291,7 +291,8 @@ listed below. Defaults will be provided for all values
> not explicitly set.
> >  ``dpdk-socket-mem``
> >    Comma separated list of memory to pre-allocate from hugepages on specific
> >    sockets. If not specified, 1024 MB will be set for each numa node by
> > -  default.
> > +  default. This behavior will change with the 2.17 release, with no default
> > +  value from OVS. Instead, DPDK default will be used.
> >
> >  ``dpdk-hugepage-dir``
> >    Directory where hugetlbfs is mounted
> > diff --git a/NEWS b/NEWS
> > index dddd57fc2..126f5a927 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -29,6 +29,8 @@ Post-v2.15.0
> >         Available only if DPDK experimantal APIs enabled during the build.
> >       * Add hardware offload support for VXLAN flows (experimental).
> >         Available only if DPDK experimantal APIs enabled during the build.
> > +     * EAL options --socket-mem and --socket-limit to have default values
> > +       removed with 2.17 release. Logging added to alert users.
> >     - ovsdb-tool:
> >       * New option '--election-timer' to the 'create-cluster' command to 
> > set the
> >         leader election timer during cluster creation.
> > diff --git a/lib/dpdk.c b/lib/dpdk.c
> > index 0c910092c..b70c01cf4 100644
> > --- a/lib/dpdk.c
> > +++ b/lib/dpdk.c
> > @@ -217,6 +217,7 @@ construct_dpdk_mutex_options(const struct smap
> *ovs_other_config,
> >          int found_opts = 0, scan, found_pos = -1;
> >          const char *found_value;
> >          struct dpdk_exclusive_options_map *popt = &excl_opts[i];
> > +        bool using_default = false;
> >
> >          for (scan = 0; scan < MAX_DPDK_EXCL_OPTS
> >                   && popt->ovs_dpdk_options[scan]; ++scan) {
> > @@ -233,6 +234,7 @@ construct_dpdk_mutex_options(const struct smap
> *ovs_other_config,
> >              if (popt->default_option) {
> >                  found_pos = popt->default_option;
> >                  found_value = popt->default_value;
> > +                using_default = true;
> >              } else {
> >                  continue;
> >              }
> > @@ -245,6 +247,12 @@ construct_dpdk_mutex_options(const struct smap
> *ovs_other_config,
> >          }
> >
> >          if (!args_contains(args, popt->eal_dpdk_options[found_pos])) {
> > +            if (using_default) {
> > +                VLOG_INFO("Using default value for '%s'. OVS will no 
> > longer "
> > +                          "provide a default for this argument starting "
> > +                          "from 2.17 release. DPDK defaults will be used "
> > +                          "instead.", popt->eal_dpdk_options[found_pos]);
> > +            }
> >              svec_add(args, popt->eal_dpdk_options[found_pos]);
> >              svec_add(args, found_value);
> >          } else {
> > @@ -482,6 +490,9 @@ dpdk_init__(const struct smap *ovs_other_config)
> >          if (i < args.n - 1) {
> >              svec_add(&args, "--socket-limit");
> >              svec_add(&args, args.names[i + 1]);
> > +            VLOG_INFO("Using default value for '--socket-limit. OVS will 
> > no "
> 
> '--socket-limit is missing the closing apostrophe
> 
> > +                      "longer provide a default for this argument starting 
> > "
> > +                      "from 2.17 release. DPDK defaults will be used 
> > instead.");
> 
> checkpatch is complaining about the last line length
> 
> WARNING: Line is 80 characters long (recommended limit is 79)
> #69 FILE: lib/dpdk.c:495:
>                       "from 2.17 release. DPDK defaults will be used
> instead.");
> 
> 
> >          }
> >      }
> >
> > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> > index 3522b2497..c26ebb796 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -365,8 +365,10 @@
> >            If dpdk-socket-mem and dpdk-alloc-mem are not specified, 
> > dpdk-socket-
> mem
> >            will be used and the default value is 1024 for each numa node. If
> >            dpdk-socket-mem and dpdk-alloc-mem are specified at same time,
> > -          dpdk-socket-mem will be used as default. Changing this value
> > -          requires restarting the daemon.
> > +          dpdk-socket-mem will be used as default. With the 2.17 release,
> > +          dpdk-socket-mem will no longer be used by default. DPDK defaults 
> > will
> > +          be used instead.
> > +          Changing this value requires restarting the daemon.
> >          </p>
> >        </column>
> >
> > @@ -388,6 +390,8 @@
> >            options specified or <code>--legacy-mem</code> provided in
> >            <ref column="other_config" key="dpdk-extra"/>, limits will not be
> >            applied.
> > +          With the 2.17 release, the OVS default value will no longer be
> > +          provided, and DPDK defaults will be used instead.
> >            Changing this value requires restarting the daemon.
> >          </p>
> >        </column>
> >

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to