[ovs-dev] [PATCH v1] dpdk: Use --in-memory by default.

2021-11-30 Thread Mike Pattrick
If anonymous memory mapping is supported by the kernel, it's better
to run OVS entirely in memory rather than creating shared data
structures. OVS doesn't work in multi-process mode, so there is no need
to litter a filesystem and experience random crashes due to old memory
chunks stored in re-opened files.

When OVS is not running in memory and crashes, it never reaches the
clean up scripts that delete the new files it has created, resulting in
"dirty" memory. OVS will partially overwrite this memory on the next
start-up, but will fail to run again because its filesystem is full of
old memory.

Here is an example of these crashes:
  https://inbox.dpdk.org/dev/20200910162407.12669-1-david.march...@redhat.com/

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1949849
Signed-off-by: Rosemarie O'Riorden 
Signed-off-by: Mike Pattrick 
---
 NEWS | 1 +
 acinclude.m4 | 6 ++
 lib/dpdk.c   | 7 +++
 3 files changed, 14 insertions(+)

diff --git a/NEWS b/NEWS
index dce1aeb2b..f3015c664 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,7 @@ Post-v2.16.0
limiting behavior.
  * Add hardware offload support for matching IPv4/IPv6 frag types
(experimental).
+ * EAL argument --in-memory is applied by default if supported.
- Python:
  * For SSL support, the use of the pyOpenSSL library has been replaced
with the native 'ssl' module.
diff --git a/acinclude.m4 b/acinclude.m4
index 8ab690f47..700a0ce15 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -472,6 +472,12 @@ AC_DEFUN([OVS_CHECK_DPDK], [
   ], [[#include ]])
 ], [], [[#include ]])
 
+AC_CHECK_DECL([MAP_HUGE_SHIFT], [
+  AC_DEFINE([DPDK_IN_MEMORY_SUPPORTED], [1], [If MAP_HUGE_SHIFT is
+ defined, anonomous memory mapping is supported by the
+ kernel, and --in-memory can be used.])
+], [], [[#include ]])
+
 # DPDK uses dlopen to load plugins.
 OVS_FIND_DEPENDENCY([dlopen], [dl], [libdl])
 
diff --git a/lib/dpdk.c b/lib/dpdk.c
index b2ef31cd2..6cdd69bd2 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -405,6 +405,13 @@ dpdk_init__(const struct smap *ovs_other_config)
 svec_add(&args, ovs_get_program_name());
 construct_dpdk_args(ovs_other_config, &args);
 
+#ifdef DPDK_IN_MEMORY_SUPPORTED
+if (!args_contains(&args, "--in-memory") &&
+!args_contains(&args, "--legacy-mem")) {
+svec_add(&args, "--in-memory");
+}
+#endif
+
 if (args_contains(&args, "-c") || args_contains(&args, "-l")) {
 auto_determine = false;
 }
-- 
2.27.0

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v1] dpdk: Use --in-memory by default.

2021-11-30 Thread 0-day Robot
Bleep bloop.  Greetings Mike Pattrick, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Rosemarie O'Riorden 
Lines checked: 79, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v1] dpdk: Use --in-memory by default.

2021-12-03 Thread Ilya Maximets
On 11/30/21 22:31, Mike Pattrick wrote:
> If anonymous memory mapping is supported by the kernel, it's better
> to run OVS entirely in memory rather than creating shared data
> structures. OVS doesn't work in multi-process mode, so there is no need
> to litter a filesystem and experience random crashes due to old memory
> chunks stored in re-opened files.
> 
> When OVS is not running in memory and crashes, it never reaches the
> clean up scripts that delete the new files it has created, resulting in
> "dirty" memory. OVS will partially overwrite this memory on the next
> start-up, but will fail to run again because its filesystem is full of
> old memory.
> 
> Here is an example of these crashes:
>   https://inbox.dpdk.org/dev/20200910162407.12669-1-david.march...@redhat.com/
> 
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1949849
> Signed-off-by: Rosemarie O'Riorden 
> Signed-off-by: Mike Pattrick 

Hi, Mike.

I'm not sure why did you re-send this patch.  The original one was fine.
It only needed a minor NEWS rebase.  And you didn't address David's
concerns about the commit message anyway.

The main problem I see here is that you actually hijacked the authorship
of the change.  You need to preserve the original 'From' when you're
re-posting someone else's patches.  Git will add a correct 'From' header
automatically during the 'git format-patch' if the authorship of the
patch is correct in your git tree.

Not a big problem.  But sending this reply as a note for myself and other
maintainers, that the original authorship has to be restored before applying.

And there is no need to add your sign-off if you didn't actually made
any code changes.

And since we're here, it should have been v2.  Just "PATCH" without a
version is equal to "PATCH v1".

Best regards, Ilya Maximets.

> ---
>  NEWS | 1 +
>  acinclude.m4 | 6 ++
>  lib/dpdk.c   | 7 +++
>  3 files changed, 14 insertions(+)
> 
> diff --git a/NEWS b/NEWS
> index dce1aeb2b..f3015c664 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -10,6 +10,7 @@ Post-v2.16.0
> limiting behavior.
>   * Add hardware offload support for matching IPv4/IPv6 frag types
> (experimental).
> + * EAL argument --in-memory is applied by default if supported.
> - Python:
>   * For SSL support, the use of the pyOpenSSL library has been replaced
> with the native 'ssl' module.
> diff --git a/acinclude.m4 b/acinclude.m4
> index 8ab690f47..700a0ce15 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -472,6 +472,12 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>], [[#include ]])
>  ], [], [[#include ]])
>  
> +AC_CHECK_DECL([MAP_HUGE_SHIFT], [
> +  AC_DEFINE([DPDK_IN_MEMORY_SUPPORTED], [1], [If MAP_HUGE_SHIFT is
> + defined, anonomous memory mapping is supported by the

s/anonomous/anonymous/

Just noticed.  Can be fixed on commit.

> + kernel, and --in-memory can be used.])
> +], [], [[#include ]])
> +
>  # DPDK uses dlopen to load plugins.
>  OVS_FIND_DEPENDENCY([dlopen], [dl], [libdl])
>  
> diff --git a/lib/dpdk.c b/lib/dpdk.c
> index b2ef31cd2..6cdd69bd2 100644
> --- a/lib/dpdk.c
> +++ b/lib/dpdk.c
> @@ -405,6 +405,13 @@ dpdk_init__(const struct smap *ovs_other_config)
>  svec_add(&args, ovs_get_program_name());
>  construct_dpdk_args(ovs_other_config, &args);
>  
> +#ifdef DPDK_IN_MEMORY_SUPPORTED
> +if (!args_contains(&args, "--in-memory") &&
> +!args_contains(&args, "--legacy-mem")) {
> +svec_add(&args, "--in-memory");
> +}
> +#endif
> +
>  if (args_contains(&args, "-c") || args_contains(&args, "-l")) {
>  auto_determine = false;
>  }
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v1] dpdk: Use --in-memory by default.

2021-12-05 Thread Mike Pattrick
On Fri, Dec 3, 2021 at 8:15 PM Ilya Maximets  wrote:
>
> On 11/30/21 22:31, Mike Pattrick wrote:
> > If anonymous memory mapping is supported by the kernel, it's better
> > to run OVS entirely in memory rather than creating shared data
> > structures. OVS doesn't work in multi-process mode, so there is no need
> > to litter a filesystem and experience random crashes due to old memory
> > chunks stored in re-opened files.
> >
> > When OVS is not running in memory and crashes, it never reaches the
> > clean up scripts that delete the new files it has created, resulting in
> > "dirty" memory. OVS will partially overwrite this memory on the next
> > start-up, but will fail to run again because its filesystem is full of
> > old memory.
> >
> > Here is an example of these crashes:
> >   
> > https://inbox.dpdk.org/dev/20200910162407.12669-1-david.march...@redhat.com/
> >
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1949849
> > Signed-off-by: Rosemarie O'Riorden 
> > Signed-off-by: Mike Pattrick 
>
> Hi, Mike.
>
> I'm not sure why did you re-send this patch.  The original one was fine.
> It only needed a minor NEWS rebase.  And you didn't address David's
> concerns about the commit message anyway.
>
> The main problem I see here is that you actually hijacked the authorship
> of the change.  You need to preserve the original 'From' when you're
> re-posting someone else's patches.  Git will add a correct 'From' header
> automatically during the 'git format-patch' if the authorship of the
> patch is correct in your git tree.

Hello Ilya,

I'm sorry it came off this way, that wasn't intentional. I resent it
because the original patch wouldn't apply for me with git am. I hadn't
seen any comments to the original patch, or any movement on it in the
past few months.


Cheers,
Michael

> Not a big problem.  But sending this reply as a note for myself and other
> maintainers, that the original authorship has to be restored before applying.
>
> And there is no need to add your sign-off if you didn't actually made
> any code changes.
>
> And since we're here, it should have been v2.  Just "PATCH" without a
> version is equal to "PATCH v1".
>
> Best regards, Ilya Maximets.
>
> > ---
> >  NEWS | 1 +
> >  acinclude.m4 | 6 ++
> >  lib/dpdk.c   | 7 +++
> >  3 files changed, 14 insertions(+)
> >
> > diff --git a/NEWS b/NEWS
> > index dce1aeb2b..f3015c664 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -10,6 +10,7 @@ Post-v2.16.0
> > limiting behavior.
> >   * Add hardware offload support for matching IPv4/IPv6 frag types
> > (experimental).
> > + * EAL argument --in-memory is applied by default if supported.
> > - Python:
> >   * For SSL support, the use of the pyOpenSSL library has been replaced
> > with the native 'ssl' module.
> > diff --git a/acinclude.m4 b/acinclude.m4
> > index 8ab690f47..700a0ce15 100644
> > --- a/acinclude.m4
> > +++ b/acinclude.m4
> > @@ -472,6 +472,12 @@ AC_DEFUN([OVS_CHECK_DPDK], [
> >], [[#include ]])
> >  ], [], [[#include ]])
> >
> > +AC_CHECK_DECL([MAP_HUGE_SHIFT], [
> > +  AC_DEFINE([DPDK_IN_MEMORY_SUPPORTED], [1], [If MAP_HUGE_SHIFT is
> > + defined, anonomous memory mapping is supported by the
>
> s/anonomous/anonymous/
>
> Just noticed.  Can be fixed on commit.
>
> > + kernel, and --in-memory can be used.])
> > +], [], [[#include ]])
> > +
> >  # DPDK uses dlopen to load plugins.
> >  OVS_FIND_DEPENDENCY([dlopen], [dl], [libdl])
> >
> > diff --git a/lib/dpdk.c b/lib/dpdk.c
> > index b2ef31cd2..6cdd69bd2 100644
> > --- a/lib/dpdk.c
> > +++ b/lib/dpdk.c
> > @@ -405,6 +405,13 @@ dpdk_init__(const struct smap *ovs_other_config)
> >  svec_add(&args, ovs_get_program_name());
> >  construct_dpdk_args(ovs_other_config, &args);
> >
> > +#ifdef DPDK_IN_MEMORY_SUPPORTED
> > +if (!args_contains(&args, "--in-memory") &&
> > +!args_contains(&args, "--legacy-mem")) {
> > +svec_add(&args, "--in-memory");
> > +}
> > +#endif
> > +
> >  if (args_contains(&args, "-c") || args_contains(&args, "-l")) {
> >  auto_determine = false;
> >  }
> >
>

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v1] dpdk: Use --in-memory by default.

2021-12-06 Thread Ilya Maximets
On 12/6/21 02:49, Mike Pattrick wrote:
> On Fri, Dec 3, 2021 at 8:15 PM Ilya Maximets  wrote:
>>
>> On 11/30/21 22:31, Mike Pattrick wrote:
>>> If anonymous memory mapping is supported by the kernel, it's better
>>> to run OVS entirely in memory rather than creating shared data
>>> structures. OVS doesn't work in multi-process mode, so there is no need
>>> to litter a filesystem and experience random crashes due to old memory
>>> chunks stored in re-opened files.
>>>
>>> When OVS is not running in memory and crashes, it never reaches the
>>> clean up scripts that delete the new files it has created, resulting in
>>> "dirty" memory. OVS will partially overwrite this memory on the next
>>> start-up, but will fail to run again because its filesystem is full of
>>> old memory.
>>>
>>> Here is an example of these crashes:
>>>   
>>> https://inbox.dpdk.org/dev/20200910162407.12669-1-david.march...@redhat.com/
>>>
>>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1949849
>>> Signed-off-by: Rosemarie O'Riorden 
>>> Signed-off-by: Mike Pattrick 
>>
>> Hi, Mike.
>>
>> I'm not sure why did you re-send this patch.  The original one was fine.
>> It only needed a minor NEWS rebase.  And you didn't address David's
>> concerns about the commit message anyway.
>>
>> The main problem I see here is that you actually hijacked the authorship
>> of the change.  You need to preserve the original 'From' when you're
>> re-posting someone else's patches.  Git will add a correct 'From' header
>> automatically during the 'git format-patch' if the authorship of the
>> patch is correct in your git tree.
> 
> Hello Ilya,
> 
> I'm sorry it came off this way, that wasn't intentional. I resent it
> because the original patch wouldn't apply for me with git am.

No worries.

3-way merge seems to work fine with this patch though, i.e. 'git am -3 ...'.
It will show you all the conflicts to resolve in a usual way, i.e. once
resolved, you can 'git add' fixes and 'git am --continue' to finish applying
the patch.  That is the process I'm normally using while applying patches from
the mailing list/patchwork.

> I hadn't
> seen any comments to the original patch, or any movement on it in the
> past few months.

That's true.  But we talked about this patch a bit on last few public meetings.
So, it's not forgotten.  But I agree, that some activity on a mailing list
would be great to have, so I replied.

> 
> 
> Cheers,
> Michael
> 
>> Not a big problem.  But sending this reply as a note for myself and other
>> maintainers, that the original authorship has to be restored before applying.
>>
>> And there is no need to add your sign-off if you didn't actually made
>> any code changes.
>>
>> And since we're here, it should have been v2.  Just "PATCH" without a
>> version is equal to "PATCH v1".
>>
>> Best regards, Ilya Maximets.
>>
>>> ---
>>>  NEWS | 1 +
>>>  acinclude.m4 | 6 ++
>>>  lib/dpdk.c   | 7 +++
>>>  3 files changed, 14 insertions(+)
>>>
>>> diff --git a/NEWS b/NEWS
>>> index dce1aeb2b..f3015c664 100644
>>> --- a/NEWS
>>> +++ b/NEWS
>>> @@ -10,6 +10,7 @@ Post-v2.16.0
>>> limiting behavior.
>>>   * Add hardware offload support for matching IPv4/IPv6 frag types
>>> (experimental).
>>> + * EAL argument --in-memory is applied by default if supported.
>>> - Python:
>>>   * For SSL support, the use of the pyOpenSSL library has been replaced
>>> with the native 'ssl' module.
>>> diff --git a/acinclude.m4 b/acinclude.m4
>>> index 8ab690f47..700a0ce15 100644
>>> --- a/acinclude.m4
>>> +++ b/acinclude.m4
>>> @@ -472,6 +472,12 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>>>], [[#include ]])
>>>  ], [], [[#include ]])
>>>
>>> +AC_CHECK_DECL([MAP_HUGE_SHIFT], [
>>> +  AC_DEFINE([DPDK_IN_MEMORY_SUPPORTED], [1], [If MAP_HUGE_SHIFT is
>>> + defined, anonomous memory mapping is supported by the
>>
>> s/anonomous/anonymous/
>>
>> Just noticed.  Can be fixed on commit.
>>
>>> + kernel, and --in-memory can be used.])
>>> +], [], [[#include ]])
>>> +
>>>  # DPDK uses dlopen to load plugins.
>>>  OVS_FIND_DEPENDENCY([dlopen], [dl], [libdl])
>>>
>>> diff --git a/lib/dpdk.c b/lib/dpdk.c
>>> index b2ef31cd2..6cdd69bd2 100644
>>> --- a/lib/dpdk.c
>>> +++ b/lib/dpdk.c
>>> @@ -405,6 +405,13 @@ dpdk_init__(const struct smap *ovs_other_config)
>>>  svec_add(&args, ovs_get_program_name());
>>>  construct_dpdk_args(ovs_other_config, &args);
>>>
>>> +#ifdef DPDK_IN_MEMORY_SUPPORTED
>>> +if (!args_contains(&args, "--in-memory") &&
>>> +!args_contains(&args, "--legacy-mem")) {
>>> +svec_add(&args, "--in-memory");
>>> +}
>>> +#endif
>>> +
>>>  if (args_contains(&args, "-c") || args_contains(&args, "-l")) {
>>>  auto_determine = false;
>>>  }
>>>
>>
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev