On 11/30/22 20:11, Mike Pattrick wrote:
> On Wed, Nov 30, 2022 at 7:27 AM Ilya Maximets <i.maxim...@ovn.org> wrote:
>>
>> On 11/25/22 18:19, Adrian Moreno wrote:
>>> Hi Mike,
>>>
>>> Sorry it took that long to review this patch.
>>>
>>> On 3/25/22 23:17, Mike Pattrick wrote:
>>>> Add new option --dump-hugepages option in ovs-ctl to enable the addition
>>>> of hugepages in the core dump filter.
>>>>
>>>> Signed-off-by: Mike Pattrick <m...@redhat.com>
>>>> ---
>>>>   NEWS                 |  4 ++++
>>>>   utilities/ovs-ctl.in | 15 +++++++++++----
>>>>   2 files changed, 15 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/NEWS b/NEWS
>>>> index 8fa57836a..7af60dce3 100644
>>>> --- a/NEWS
>>>> +++ b/NEWS
>>>> @@ -3,6 +3,10 @@ Post-v2.17.0
>>>>      - OVSDB:
>>>>        * 'relay' service model now supports transaction history, i.e. 
>>>> honors the
>>>>          'last-txn-id' field in 'monitor_cond_since' requests from clients.
>>>> +   - ovs-ctl:
>>>> +     * New option '--dump-hugepages' to include hugepages in core dumps. 
>>>> This
>>>> +       can assist with postmortem analysis involving DPDK, but may also 
>>>> produce
>>>> +       significantly larger core dump files.
>>>>
>>>
>>> I'm afraid this part needs rebasing.
>>>
>>>>     v2.17.0 - 17 Feb 2022
>>>> diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
>>>> index e6e07f476..8f900314b 100644
>>>> --- a/utilities/ovs-ctl.in
>>>> +++ b/utilities/ovs-ctl.in
>>>> @@ -103,8 +103,13 @@ set_system_ids () {
>>>>       action "Configuring Open vSwitch system IDs" "$@" $extra_ids
>>>>   }
>>>>   -check_force_cores () {
>>>> -    if test X"$FORCE_COREFILES" = Xyes; then
>>>> +check_core_config () {
>>>> +    if test X"$DUMP_HUGEPAGES" = Xyes; then
>>>> +        echo 0x3f > /proc/self/coredump_filter
>>
>> Shouldn't this be 0x7f instead?
>> 0x3f doesn't enable bit #6, which is responsible for dumping
>> shared huge pages.  Or am I missing something?
> 
> That's a good point, the hugepage may or may not be private. I'll send
> in a new one.

OK.  One thing to think about though is that we'll grab
VM memory, I guess, in case we have vhost-user ports.
So, the core dump size can become insanely huge.

The downside of not having them is inability to inspect
virtqueues and stuff in the dump.

> 
> Cheers,
> M
> 
>>
>> Best regards, Ilya Maximets.
>>
>>>> +        if test X"$FORCE_COREFILES" = Xyes; then
>>>> +            ulimit -c unlimited
>>>> +        fi
>>>> +    elif test X"$FORCE_COREFILES" = Xyes; then
>>>>           ulimit -c 67108864
>>>>       fi
>>>>   }
>>>> @@ -116,7 +121,7 @@ del_transient_ports () {
>>>>   }
>>>>     do_start_ovsdb () {
>>>> -    check_force_cores
>>>> +    check_core_config
>>>>         if daemon_is_running ovsdb-server; then
>>>>           log_success_msg "ovsdb-server is already running"
>>>> @@ -193,7 +198,7 @@ add_managers () {
>>>>   }
>>>>     do_start_forwarding () {
>>>> -    check_force_cores
>>>> +    check_core_config
>>>>         insert_mod_if_required || return 1
>>>>   @@ -330,6 +335,7 @@ set_defaults () {
>>>>         DAEMON_CWD=/
>>>>       FORCE_COREFILES=yes
>>>> +    DUMP_HUGEPAGES=no
>>>>       MLOCKALL=yes
>>>>       SELF_CONFINEMENT=yes
>>>>       MONITOR=yes
>>>> @@ -419,6 +425,7 @@ Other important options for "start", "restart" and 
>>>> "force-reload-kmod":
>>>>   Less important options for "start", "restart" and "force-reload-kmod":
>>>>     --daemon-cwd=DIR               set working dir for OVS daemons 
>>>> (default: $DAEMON_CWD)
>>>>     --no-force-corefiles           do not force on core dumps for OVS 
>>>> daemons
>>>> +  --dump-hugepages               include hugepages in coredumps
>>>>     --no-mlockall                  do not lock all of ovs-vswitchd into 
>>>> memory
>>>>     --ovsdb-server-priority=NICE   set ovsdb-server's niceness (default: 
>>>> $OVSDB_SERVER_PRIORITY)
>>>>     --ovsdb-server-options=OPTIONS additional options for ovsdb-server 
>>>> (example: '-vconsole:dbg -vfile:dbg')
>>>>
>>>
>>> Tested locally and verified that with the option hugepages appear in 
>>> coredumps.
>>> Apart from the need to rebase the NEWS, the patch looks good to me.
>>>
>>> Acked-by: Adrian Moreno <amore...@redhat.com>
>>>
>>> --
>>> Adrián Moreno
>>
> 

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

Reply via email to