Re: [ovs-dev] [PATCH v1] dpdk: Use --in-memory by default.
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(, ovs_get_program_name());
>>> construct_dpdk_args(ovs_other_config, );
>>>
>>> +#ifdef DPDK_IN_MEMORY_SUPPORTED
>>> +if (!args_contains(, "--in-memory") &&
>>> +!args_contains(, "--legacy-mem")) {
>>> +svec_add(, "--in-memory");
>>> +}
>>> +#endif
>>> +
>>> if (args_contains(, "-c") || args_contains(, "-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.
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(, ovs_get_program_name());
> > construct_dpdk_args(ovs_other_config, );
> >
> > +#ifdef DPDK_IN_MEMORY_SUPPORTED
> > +if (!args_contains(, "--in-memory") &&
> > +!args_contains(, "--legacy-mem")) {
> > +svec_add(, "--in-memory");
> > +}
> > +#endif
> > +
> > if (args_contains(, "-c") || args_contains(, "-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.
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(, ovs_get_program_name());
> construct_dpdk_args(ovs_other_config, );
>
> +#ifdef DPDK_IN_MEMORY_SUPPORTED
> +if (!args_contains(, "--in-memory") &&
> +!args_contains(, "--legacy-mem")) {
> +svec_add(, "--in-memory");
> +}
> +#endif
> +
> if (args_contains(, "-c") || args_contains(, "-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.
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
