Re: [PATCH v2 1/2] contigmem: support including mapped buffers in core dump

[Lewis Donzis <lew@perftech.com>]

Is the extra control necessary, i.e., why not just always do this and let the EAL option control whether the pages get dumped?

----- On Oct 25, 2024, at 3:26 PM, Dmitry Kozlyuk dmitry.kozliuk@gmail.com wrote:

> It was impossible to include mapped contigmem buffers in core dump.
> Add hw.contigmem.coredump_enable tunable to control the inclusion.
> 
> Mappings backed by OBJ_FICTITIOUS are excluded from core dump.
> While contigmem is a device and its OBJT_DEVICE mappings get this flag,
> they are actually backed by memory.
> Clear OBJ_FICTITIOUS mapping bit to include them in core dump.
> Do this only if hw.contigmem.coredump_enable=1.
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
> doc/guides/freebsd_gsg/build_dpdk.rst | 27 ++++++++++++++++++++-------
> kernel/freebsd/contigmem/contigmem.c  |  8 ++++++++
> 2 files changed, 28 insertions(+), 7 deletions(-)
> 
> diff --git a/doc/guides/freebsd_gsg/build_dpdk.rst
> b/doc/guides/freebsd_gsg/build_dpdk.rst
> index 86e8e5a805..387ec52820 100644
> --- a/doc/guides/freebsd_gsg/build_dpdk.rst
> +++ b/doc/guides/freebsd_gsg/build_dpdk.rst
> @@ -86,6 +86,22 @@ module loading using::
>     kenv hw.contigmem.num_buffers=n
>     kenv hw.contigmem.buffer_size=m
> 
> +Where n is the number of blocks and m is the size in bytes of each area of
> +contiguous memory.  A default of two buffers of size 1073741824 bytes (1
> Gigabyte)
> +each is set during module load if they are not specified in the environment.
> +
> +Buffers are excluded from core dump by default.
> +Mapped buffers can be included in core dump using the following tunable:
> +
> +    hw.contigmem.coredump_enable=1
> +
> +.. note::
> +
> +    Including contigmem buffers in core dump file increases its size,
> +    which may fill the storage or overload the transport.
> +    Buffers typically hold data processed by the application,
> +    like network packets, which may contain sensitive information.
> +
> The kernel environment variables can also be specified during boot by placing
> the
> following in ``/boot/loader.conf``:
> 
> @@ -93,15 +109,12 @@ following in ``/boot/loader.conf``:
> 
>     hw.contigmem.num_buffers=n
>     hw.contigmem.buffer_size=m
> +    hw.contigmem.coredump_enable=1
> 
> The variables can be inspected using the following command::
> 
>     sysctl -a hw.contigmem
> 
> -Where n is the number of blocks and m is the size in bytes of each area of
> -contiguous memory.  A default of two buffers of size 1073741824 bytes (1
> Gigabyte)
> -each is set during module load if they are not specified in the environment.
> -
> The module can then be loaded using kldload::
> 
>     kldload contigmem
> @@ -115,13 +128,13 @@ up time.  This can be achieved by placing lines similar to
> the following into
> 
>     hw.contigmem.num_buffers=1
>     hw.contigmem.buffer_size=1073741824
> +    hw.contigmem.coredump_enable=1
>     contigmem_load="YES"
> 
> .. note::
> 
> -    The contigmem_load directive should be placed after any definitions of
> -    ``hw.contigmem.num_buffers`` and ``hw.contigmem.buffer_size`` if the
> default values
> -    are not to be used.
> +    The ``contigmem_load`` directive should be placed after any definitions
> +    of ``hw.contigmem.*`` if the default values are not to be used.
> 
> An error such as::
> 
> diff --git a/kernel/freebsd/contigmem/contigmem.c
> b/kernel/freebsd/contigmem/contigmem.c
> index 7dd87599d9..591e46dded 100644
> --- a/kernel/freebsd/contigmem/contigmem.c
> +++ b/kernel/freebsd/contigmem/contigmem.c
> @@ -51,6 +51,7 @@ static d_close_t        contigmem_close;
> 
> static int              contigmem_num_buffers = RTE_CONTIGMEM_DEFAULT_NUM_BUFS;
> static int64_t          contigmem_buffer_size = RTE_CONTIGMEM_DEFAULT_BUF_SIZE;
> +static bool             contigmem_coredump_enable;
> 
> static eventhandler_tag contigmem_eh_tag;
> static struct contigmem_buffer contigmem_buffers[RTE_CONTIGMEM_MAX_NUM_BUFS];
> @@ -59,6 +60,7 @@ static int              contigmem_refcnt;
> 
> TUNABLE_INT("hw.contigmem.num_buffers", &contigmem_num_buffers);
> TUNABLE_QUAD("hw.contigmem.buffer_size", &contigmem_buffer_size);
> +TUNABLE_BOOL("hw.contigmem.coredump_enable", &contigmem_coredump_enable);
> 
> static SYSCTL_NODE(_hw, OID_AUTO, contigmem, CTLFLAG_RD, 0, "contigmem");
> 
> @@ -68,6 +70,8 @@ SYSCTL_QUAD(_hw_contigmem, OID_AUTO, buffer_size, CTLFLAG_RD,
> 	&contigmem_buffer_size, 0, "Size of each contiguous buffer");
> SYSCTL_INT(_hw_contigmem, OID_AUTO, num_references, CTLFLAG_RD,
> 	&contigmem_refcnt, 0, "Number of references to contigmem");
> +SYSCTL_BOOL(_hw_contigmem, OID_AUTO, coredump_enable, CTLFLAG_RD,
> +	&contigmem_coredump_enable, 0, "Include mapped buffers in core dump");
> 
> static SYSCTL_NODE(_hw_contigmem, OID_AUTO, physaddr, CTLFLAG_RD, 0,
> 	"physaddr");
> @@ -357,5 +361,9 @@ contigmem_mmap_single(struct cdev *cdev, vm_ooffset_t
> *offset, vm_size_t size,
> 	*obj = cdev_pager_allocate(vmh, OBJT_DEVICE, &contigmem_cdev_pager_ops,
> 			size, nprot, *offset, curthread->td_ucred);
> 
> +	/* Mappings backed by OBJ_FICTITIOUS are excluded from core dump. */
> +	if (contigmem_coredump_enable)
> +		(*obj)->flags &= ~OBJ_FICTITIOUS;
> +
> 	return 0;
> }
> --
> 2.38.4