On Wed, Aug 18, 2021 at 10:16 PM Stephen Hemminger
[off-list ref] wrote:

On Wed, 18 Aug 2021 15:07:25 +0530
Jerin Jacob [off-list ref] wrote:

quoted

On Tue, Aug 17, 2021 at 9:22 PM Stephen Hemminger
[off-list ref] wrote:

quoted

On Tue, 17 Aug 2021 20:57:50 +0530
Jerin Jacob [off-list ref] wrote:

quoted

On Tue, Aug 17, 2021 at 8:39 PM Stephen Hemminger
[off-list ref] wrote:

quoted

On Tue, 17 Aug 2021 13:08:46 +0530
Jerin Jacob [off-list ref] wrote:

quoted

On Tue, Aug 17, 2021 at 9:23 AM Stephen Hemminger
[off-list ref] wrote:

quoted

On Tue, 17 Aug 2021 08:57:18 +0530
[off-list ref] wrote:

quoted

From: Jerin Jacob <redacted>

Introducing oops handling API with following specification
and enable stub implementation for Linux and FreeBSD.

On rte_eal_init() invocation, the EAL library installs the
oops handler for the essential signals.
The rte_oops_signals_enabled() API provides the list
of signals the library installed by the EAL.

This is a big change, and many applications already handle these
signals themselves. Therefore adding this needs to be opt-in
and not enabled by default.

In order to avoid every application explicitly register this
sighandler and to cater to the
co-existing application-specific signal-hander usage.
The following design has been chosen. (It is mentioned in the commit log,
I will describe here for more clarity)

Case 1:
a) The application installs the signal handler prior to rte_eal_init().
b) Implementation stores the application-specific signal and replace a
signal handler as oops eal handler
c) when application/DPDK get the segfault, the default EAL oops
handler gets invoked
d) Then it dumps the EAL specific message, it calls the
application-specific signal handler
installed in step 1 by application. This avoids breaking any contract
with the application.
i.e Behavior is the same current EAL now.
That is the reason for not using SA_RESETHAND(which call SIG_DFL after
eal oops handler instead
application-specific handler)

Case 2:
a) The application install the signal handler after rte_eal_init(),
b) EAL hander get replaced with application handle then the application can call
rte_oops_decode() to decode.

In order to cater the above use case, rte_oops_signals_enabled() and
rte_oops_decode()
provided.

Here we are not breaking any contract with the application.
Do you have concerns about this design?

In our application as a service it is important not to do any backtrace
in production. We rely on other infrastructure to process coredumps.

Other infrastructure will work. For example, If we are using standard coredump
using linux infra. In Current implementation,
- EAL handler dump the DPDK OOPS like kernel on stderr
- Implementation calls SIG_DFL in eal oops handler
- The above step creates the coredump or re-directs any other
infrastructure you are using for coredump.

quoted

This should be controlled enabled by a command line argument.

If we allow other infrastructure coredump to work as-is, why
enable/disable required from eal?

The addition of DPDK OOPS adds additional steps which make all
faults be identified as the oops code.

Since we are using SA_ONSTACK it is not losing the original segfault
info.

I verified like this, Please find below the steps.

0) Enable coredump infra in Linux using coredumpctl or so
1) Apply this series
2) Apply for the following patch to create a segfault from the library.
This will test, segfault caught by eal and forward to default Linux singal
handler.

[main]dell[dpdk.org] $ git diff

diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 3438a96b75..b935c32c98 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c

@@ -1338,6 +1338,8 @@ rte_eal_init(int argc, char **argv)

        eal_mcfg_complete();

+       /* Generate a segfault */
+       *(volatile int *)0x05 = 0;
        return fctret;

 }

3)Build
meson --buildtype debug build
ninja -C build

4) Run
$ ./build/app/test/dpdk-test --no-huge  -c 0x2

Please find oops dump[1] and gdb core dump backtrace[2].
Gdb core dump trace preserves the original segfault cause and trace.

Any other concerns?

Your new oops handling duplicates existing code in our application
(and I know others that do this as well). The problem is that an
application may do this before calling rte_eal_init and your new
code will break that.

Not sure what it breaks, Could you elaborate on this? Your app signal
handler will be called with the original signal
the info it is registered before rte_eal_init().

We can have an additional API to disable the oops prints if you
insist. (Though I don't the know use case
where someone needs this other than someone don't want to see/log this
print). If that is rational,
I can add API to disable oops print it. I prefer to install it by
default as it won't break anything and it helps
to not add oops API in existing apps i.e without calling any
additional features in all existing applications.

Therefore my recommendation is that the new oops handling needs
to be not a built in feature of EAL.