On 12/6/21 02:49, Mike Pattrick wrote:
> On Fri, Dec 3, 2021 at 8:15 PM Ilya Maximets <i.maxim...@ovn.org> 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 <rorio...@redhat.com>
>>> Signed-off-by: Mike Pattrick <m...@redhat.com>
>>
>> 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 <rte_config.h>]])
>>>      ], [], [[#include <rte_config.h>]])
>>>
>>> +    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 <sys/mman.h>]])
>>> +
>>>      # 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

Reply via email to