On Wed, Nov 30, 2022 at 7:27 AM Ilya Maximets <[email protected]> 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 <[email protected]>
> >> ---
> >> 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.
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 <[email protected]>
> >
> > --
> > Adrián Moreno
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
