diff mbox series

[ovs-dev,1/2] dpdk: Allow configuration of max memory zones.

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

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Roi Dayan Jan. 20, 2025, 10:12 a.m. UTC
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.

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(+)

Comments

Kevin Traynor Jan. 22, 2025, 12:26 p.m. UTC | #1
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>
Roi Dayan Jan. 28, 2025, 11:38 a.m. UTC | #2
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>
>
Kevin Traynor Jan. 30, 2025, 11:33 a.m. UTC | #3
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 mbox series

Patch

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>