Message ID | 20250120101210.1216196-1-roid@nvidia.com |
---|---|
State | Rejected |
Delegated to: | Kevin Traynor |
Headers | show |
Series | [ovs-dev,1/2] dpdk: Allow configuration of max memory zones. | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 20/01/2025 10:12, Roi Dayan via dev wrote: > From: Eli Britstein <elibr@nvidia.com> > > ovs-vsctl set o . other_config:dpdk-max-memzones=XXX. > This configuration requires restart in order to take effect. > Hi Roi/Eli, Before we add a low level configuration knob - why is it needed, is the default in DPDK not suitable for some cases ? Looking from OVS user perspective, they would need some documentation on: - how they will know if they need to use this - how to calculate a suitable value It would be ok to point to DPDK for further details, but OVS user will need some starting points. Otherwise you will have people trying random numbers if they encounter any memory issues. thanks, Kevin. > Signed-off-by: Eli Britstein <elibr@nvidia.com> > Acked-by: Roi Dayan <roid@nvidia.com> > --- > NEWS | 2 ++ > lib/dpdk.c | 26 ++++++++++++++++++++++++++ > vswitchd/vswitch.xml | 12 ++++++++++++ > 3 files changed, 40 insertions(+) > > diff --git a/NEWS b/NEWS > index d59692d8b305..e232d35067ed 100644 > --- a/NEWS > +++ b/NEWS > @@ -89,6 +89,8 @@ v3.4.0 - 15 Aug 2024 > * Link status changes are now handled via interrupt mode if the DPDK > driver supports it. It is possible to revert to polling mode by setting > per interface 'options:dpdk-lsc-interrupt' to 'false'. > + * New configuration knob 'other_config:dpdk-max-memzones' to set dpdk > + max memory zones. > - Python: > * Added custom transaction support to the Idl via add_op(). > * Added support for different output formats like 'json' to Python's > diff --git a/lib/dpdk.c b/lib/dpdk.c > index b7516257c5e4..de729bedd9da 100644 > --- a/lib/dpdk.c > +++ b/lib/dpdk.c > @@ -310,6 +310,28 @@ malloc_dump_stats_wrapper(FILE *stream) > rte_malloc_dump_stats(stream, NULL); > } > > +#ifdef ALLOW_EXPERIMENTAL_API rte_memzone_max_set() is no longer experimental https://git.dpdk.org/dpdk-stable/tree/lib/eal/version.map#n229 > +static void > +dpdk_init_max_memzones(const struct smap *ovs_other_config) > +{ > + uint32_t max_memzones; > + int rv; > + > + max_memzones = smap_get_uint(ovs_other_config, "dpdk-max-memzones", 0); > + > + if (!max_memzones) { > + return; > + } > + > + rv = rte_memzone_max_set(max_memzones); > + if (rv) { > + VLOG_WARN("Failed to set max memzones to %"PRIu32, max_memzones); > + } else { > + VLOG_INFO("Setting max memzones to %"PRIu32, max_memzones); > + } > +} > +#endif > + > static bool > dpdk_init__(const struct smap *ovs_other_config) > { > @@ -342,6 +364,10 @@ dpdk_init__(const struct smap *ovs_other_config) > auto_determine = false; > } > > +#ifdef ALLOW_EXPERIMENTAL_API > + dpdk_init_max_memzones(ovs_other_config); > +#endif > + > /** > * NOTE: This is an unsophisticated mechanism for determining the DPDK > * main core. > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml > index 275bcbec0b5a..38259e251c9d 100644 > --- a/vswitchd/vswitch.xml > +++ b/vswitchd/vswitch.xml > @@ -425,6 +425,18 @@ > </p> > </column> > > + <column name="other_config" key="dpdk-max-memzones" > + type='{"type": "integer"}'> > + <p> > + Specifies the maximum number of memzones that can be created in > + DPDK. > + </p> > + <p> > + The default is empty, keeping DPDK's default. Changing this value > + requires restarting the daemon. > + </p> > + </column> > + > <column name="other_config" key="dpdk-extra" > type='{"type": "string"}'> > <p>
On 22/01/2025 14:26, Kevin Traynor wrote: > On 20/01/2025 10:12, Roi Dayan via dev wrote: >> From: Eli Britstein <elibr@nvidia.com> >> >> ovs-vsctl set o . other_config:dpdk-max-memzones=XXX. >> This configuration requires restart in order to take effect. >> > > Hi Roi/Eli, > > Before we add a low level configuration knob - why is it needed, is the > default in DPDK not suitable for some cases ? > > Looking from OVS user perspective, they would need some documentation on: > - how they will know if they need to use this > - how to calculate a suitable value > > It would be ok to point to DPDK for further details, but OVS user will > need some starting points. Otherwise you will have people trying random > numbers if they encounter any memory issues. > > thanks, > Kevin. Hi Kevin, Thanks for the review. It's like you said really. In some scenarios, and with custom code we tested, we found the need to increase the default for more dpdk memory zones. That said, I'm not 100% sure if we still need to change it but we thought that an option to be able to modify the default might be useful for the future. I'm also not able to point to DPDK for further details as I couldn't find a specific help page for this besides a man page that just say the function exists. So if it's not helpful as it is we can skip it for now. If someone can review the second, and not related, patch in this series or if no comments at all then I'll resend it alone at a later time. Thanks, Roi > >> Signed-off-by: Eli Britstein <elibr@nvidia.com> >> Acked-by: Roi Dayan <roid@nvidia.com> >> --- >> NEWS | 2 ++ >> lib/dpdk.c | 26 ++++++++++++++++++++++++++ >> vswitchd/vswitch.xml | 12 ++++++++++++ >> 3 files changed, 40 insertions(+) >> >> diff --git a/NEWS b/NEWS >> index d59692d8b305..e232d35067ed 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -89,6 +89,8 @@ v3.4.0 - 15 Aug 2024 >> * Link status changes are now handled via interrupt mode if the DPDK >> driver supports it. It is possible to revert to polling mode by setting >> per interface 'options:dpdk-lsc-interrupt' to 'false'. >> + * New configuration knob 'other_config:dpdk-max-memzones' to set dpdk >> + max memory zones. >> - Python: >> * Added custom transaction support to the Idl via add_op(). >> * Added support for different output formats like 'json' to Python's >> diff --git a/lib/dpdk.c b/lib/dpdk.c >> index b7516257c5e4..de729bedd9da 100644 >> --- a/lib/dpdk.c >> +++ b/lib/dpdk.c >> @@ -310,6 +310,28 @@ malloc_dump_stats_wrapper(FILE *stream) >> rte_malloc_dump_stats(stream, NULL); >> } >> >> +#ifdef ALLOW_EXPERIMENTAL_API > > rte_memzone_max_set() is no longer experimental > https://git.dpdk.org/dpdk-stable/tree/lib/eal/version.map#n229 > >> +static void >> +dpdk_init_max_memzones(const struct smap *ovs_other_config) >> +{ >> + uint32_t max_memzones; >> + int rv; >> + >> + max_memzones = smap_get_uint(ovs_other_config, "dpdk-max-memzones", 0); >> + >> + if (!max_memzones) { >> + return; >> + } >> + >> + rv = rte_memzone_max_set(max_memzones); >> + if (rv) { >> + VLOG_WARN("Failed to set max memzones to %"PRIu32, max_memzones); >> + } else { >> + VLOG_INFO("Setting max memzones to %"PRIu32, max_memzones); >> + } >> +} >> +#endif >> + >> static bool >> dpdk_init__(const struct smap *ovs_other_config) >> { >> @@ -342,6 +364,10 @@ dpdk_init__(const struct smap *ovs_other_config) >> auto_determine = false; >> } >> >> +#ifdef ALLOW_EXPERIMENTAL_API >> + dpdk_init_max_memzones(ovs_other_config); >> +#endif >> + >> /** >> * NOTE: This is an unsophisticated mechanism for determining the DPDK >> * main core. >> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml >> index 275bcbec0b5a..38259e251c9d 100644 >> --- a/vswitchd/vswitch.xml >> +++ b/vswitchd/vswitch.xml >> @@ -425,6 +425,18 @@ >> </p> >> </column> >> >> + <column name="other_config" key="dpdk-max-memzones" >> + type='{"type": "integer"}'> >> + <p> >> + Specifies the maximum number of memzones that can be created in >> + DPDK. >> + </p> >> + <p> >> + The default is empty, keeping DPDK's default. Changing this value >> + requires restarting the daemon. >> + </p> >> + </column> >> + >> <column name="other_config" key="dpdk-extra" >> type='{"type": "string"}'> >> <p> >
On 28/01/2025 11:38, Roi Dayan wrote: > > > On 22/01/2025 14:26, Kevin Traynor wrote: >> On 20/01/2025 10:12, Roi Dayan via dev wrote: >>> From: Eli Britstein <elibr@nvidia.com> >>> >>> ovs-vsctl set o . other_config:dpdk-max-memzones=XXX. >>> This configuration requires restart in order to take effect. >>> >> >> Hi Roi/Eli, >> >> Before we add a low level configuration knob - why is it needed, is the >> default in DPDK not suitable for some cases ? >> >> Looking from OVS user perspective, they would need some documentation on: >> - how they will know if they need to use this >> - how to calculate a suitable value >> >> It would be ok to point to DPDK for further details, but OVS user will >> need some starting points. Otherwise you will have people trying random >> numbers if they encounter any memory issues. >> >> thanks, >> Kevin. > > Hi Kevin, > > Thanks for the review. It's like you said really. In some scenarios, and > with custom code we tested, we found the need to increase the default for > more dpdk memory zones. That said, I'm not 100% sure if we still need to > change it but we thought that an option to be able to modify the default > might be useful for the future. > I'm also not able to point to DPDK for further details as I couldn't find > a specific help page for this besides a man page that just say the function > exists. > > So if it's not helpful as it is we can skip it for now. > Hi Roi, ok, let's drop it for now. We can always revisit in the future if the need arises. > If someone can review the second, and not related, patch in this > series or if no comments at all then I'll resend it alone at a > later time. > Yes, we'll review that. No need to resend unless there is rework required. thanks, Kevin. > Thanks, > Roi > > > >> >>> Signed-off-by: Eli Britstein <elibr@nvidia.com> >>> Acked-by: Roi Dayan <roid@nvidia.com> >>> --- >>> NEWS | 2 ++ >>> lib/dpdk.c | 26 ++++++++++++++++++++++++++ >>> vswitchd/vswitch.xml | 12 ++++++++++++ >>> 3 files changed, 40 insertions(+) >>> >>> diff --git a/NEWS b/NEWS >>> index d59692d8b305..e232d35067ed 100644 >>> --- a/NEWS >>> +++ b/NEWS >>> @@ -89,6 +89,8 @@ v3.4.0 - 15 Aug 2024 >>> * Link status changes are now handled via interrupt mode if the DPDK >>> driver supports it. It is possible to revert to polling mode by setting >>> per interface 'options:dpdk-lsc-interrupt' to 'false'. >>> + * New configuration knob 'other_config:dpdk-max-memzones' to set dpdk >>> + max memory zones. >>> - Python: >>> * Added custom transaction support to the Idl via add_op(). >>> * Added support for different output formats like 'json' to Python's >>> diff --git a/lib/dpdk.c b/lib/dpdk.c >>> index b7516257c5e4..de729bedd9da 100644 >>> --- a/lib/dpdk.c >>> +++ b/lib/dpdk.c >>> @@ -310,6 +310,28 @@ malloc_dump_stats_wrapper(FILE *stream) >>> rte_malloc_dump_stats(stream, NULL); >>> } >>> >>> +#ifdef ALLOW_EXPERIMENTAL_API >> >> rte_memzone_max_set() is no longer experimental >> https://git.dpdk.org/dpdk-stable/tree/lib/eal/version.map#n229 >> >>> +static void >>> +dpdk_init_max_memzones(const struct smap *ovs_other_config) >>> +{ >>> + uint32_t max_memzones; >>> + int rv; >>> + >>> + max_memzones = smap_get_uint(ovs_other_config, "dpdk-max-memzones", 0); >>> + >>> + if (!max_memzones) { >>> + return; >>> + } >>> + >>> + rv = rte_memzone_max_set(max_memzones); >>> + if (rv) { >>> + VLOG_WARN("Failed to set max memzones to %"PRIu32, max_memzones); >>> + } else { >>> + VLOG_INFO("Setting max memzones to %"PRIu32, max_memzones); >>> + } >>> +} >>> +#endif >>> + >>> static bool >>> dpdk_init__(const struct smap *ovs_other_config) >>> { >>> @@ -342,6 +364,10 @@ dpdk_init__(const struct smap *ovs_other_config) >>> auto_determine = false; >>> } >>> >>> +#ifdef ALLOW_EXPERIMENTAL_API >>> + dpdk_init_max_memzones(ovs_other_config); >>> +#endif >>> + >>> /** >>> * NOTE: This is an unsophisticated mechanism for determining the DPDK >>> * main core. >>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml >>> index 275bcbec0b5a..38259e251c9d 100644 >>> --- a/vswitchd/vswitch.xml >>> +++ b/vswitchd/vswitch.xml >>> @@ -425,6 +425,18 @@ >>> </p> >>> </column> >>> >>> + <column name="other_config" key="dpdk-max-memzones" >>> + type='{"type": "integer"}'> >>> + <p> >>> + Specifies the maximum number of memzones that can be created in >>> + DPDK. >>> + </p> >>> + <p> >>> + The default is empty, keeping DPDK's default. Changing this value >>> + requires restarting the daemon. >>> + </p> >>> + </column> >>> + >>> <column name="other_config" key="dpdk-extra" >>> type='{"type": "string"}'> >>> <p> >> >
diff --git a/NEWS b/NEWS index d59692d8b305..e232d35067ed 100644 --- a/NEWS +++ b/NEWS @@ -89,6 +89,8 @@ v3.4.0 - 15 Aug 2024 * Link status changes are now handled via interrupt mode if the DPDK driver supports it. It is possible to revert to polling mode by setting per interface 'options:dpdk-lsc-interrupt' to 'false'. + * New configuration knob 'other_config:dpdk-max-memzones' to set dpdk + max memory zones. - Python: * Added custom transaction support to the Idl via add_op(). * Added support for different output formats like 'json' to Python's diff --git a/lib/dpdk.c b/lib/dpdk.c index b7516257c5e4..de729bedd9da 100644 --- a/lib/dpdk.c +++ b/lib/dpdk.c @@ -310,6 +310,28 @@ malloc_dump_stats_wrapper(FILE *stream) rte_malloc_dump_stats(stream, NULL); } +#ifdef ALLOW_EXPERIMENTAL_API +static void +dpdk_init_max_memzones(const struct smap *ovs_other_config) +{ + uint32_t max_memzones; + int rv; + + max_memzones = smap_get_uint(ovs_other_config, "dpdk-max-memzones", 0); + + if (!max_memzones) { + return; + } + + rv = rte_memzone_max_set(max_memzones); + if (rv) { + VLOG_WARN("Failed to set max memzones to %"PRIu32, max_memzones); + } else { + VLOG_INFO("Setting max memzones to %"PRIu32, max_memzones); + } +} +#endif + static bool dpdk_init__(const struct smap *ovs_other_config) { @@ -342,6 +364,10 @@ dpdk_init__(const struct smap *ovs_other_config) auto_determine = false; } +#ifdef ALLOW_EXPERIMENTAL_API + dpdk_init_max_memzones(ovs_other_config); +#endif + /** * NOTE: This is an unsophisticated mechanism for determining the DPDK * main core. diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index 275bcbec0b5a..38259e251c9d 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -425,6 +425,18 @@ </p> </column> + <column name="other_config" key="dpdk-max-memzones" + type='{"type": "integer"}'> + <p> + Specifies the maximum number of memzones that can be created in + DPDK. + </p> + <p> + The default is empty, keeping DPDK's default. Changing this value + requires restarting the daemon. + </p> + </column> + <column name="other_config" key="dpdk-extra" type='{"type": "string"}'> <p>