[PATCH] eal: support including mapped memory in core dump

[PATCH] eal: support including mapped memory in core dump

[Dmitry Kozlyuk]

From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

Commit d72e4042c5eb ("mem: exclude unused memory from core dump")
unconditionally excluded all hugepage memory managed by DPDK.
The rationale was to avoid overly large core dumps
generated from reserved (PROT_NONE) but not mapped memory.
Mapped hugepages, however, may hold data useful for debugging,
even if not being fully used and containing some garbage.
Users may want to include those hugepages in core dump.

Add `--huge-dump` EAL command-line option to include in core dump
all mapped hugepages and also non-hugepage memory
allocated with `--no-huge` (as it substitutes for hugepages).
Using this option requires more disk storage for core dumps,
and also may include sensitive data in core dump,
which is why it must be explicitly enabled and a warning is printed.

Linux requires /proc/self/coredump_filter adjustment
to include hugepages mapped with MAP_SHARED in core dump.
Windows EAL requires no change since it automatically
excludes reserved memory and includes committed memory.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
Lewis, testing on FreeBSD would be appreciated.

 doc/guides/linux_gsg/eal_args.include.rst |  8 +++
 lib/eal/common/eal_common_dynmem.c        |  1 +
 lib/eal/common/eal_common_options.c       |  7 ++
 lib/eal/common/eal_internal_cfg.h         |  2 +
 lib/eal/common/eal_options.h              |  2 +
 lib/eal/freebsd/eal_memory.c              | 19 +++++
 lib/eal/linux/eal_memalloc.c              |  8 +++
 lib/eal/linux/eal_memory.c                | 86 +++++++++++++++++++++--
 8 files changed, 129 insertions(+), 4 deletions(-)

diff --git a/doc/guides/linux_gsg/eal_args.include.rst b/doc/guides/linux_gsg/eal_args.include.rst
index 9cfbf7de84..0cab6b2f16 100644
--- a/doc/guides/linux_gsg/eal_args.include.rst
+++ b/doc/guides/linux_gsg/eal_args.include.rst
@@ -122,6 +122,14 @@ Memory-related options
     to system pthread stack size unless the optional size (in kbytes) is
     specified.
 
+*   ``--huge-dump``
+
+    Include in code dump all hugepages mapped by DPDK either at startup or later,
+    and ordirary pages mapped by DPDK at startup when ``--no-huge`` is given.
+    Hugepages increase core dumps size roughly by the amount of memory used.
+    Dumped memory often contains processed data, which may be sensitive.
+    Primary and secondary processes can use this option independently.
+
 Debugging options
 ~~~~~~~~~~~~~~~~~
 
diff --git a/lib/eal/common/eal_common_dynmem.c b/lib/eal/common/eal_common_dynmem.c
index b4dc231940..e6497307b5 100644
--- a/lib/eal/common/eal_common_dynmem.c
+++ b/lib/eal/common/eal_common_dynmem.c
@@ -7,6 +7,7 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include <rte_errno.h>
 #include <rte_log.h>
 #include <rte_string_fns.h>
 
diff --git a/lib/eal/common/eal_common_options.c b/lib/eal/common/eal_common_options.c
index f1a5e329a5..b02a9894ff 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -105,6 +105,7 @@ eal_long_options[] = {
 	{OPT_NO_TELEMETRY,      0, NULL, OPT_NO_TELEMETRY_NUM     },
 	{OPT_FORCE_MAX_SIMD_BITWIDTH, 1, NULL, OPT_FORCE_MAX_SIMD_BITWIDTH_NUM},
 	{OPT_HUGE_WORKER_STACK, 2, NULL, OPT_HUGE_WORKER_STACK_NUM     },
+	{OPT_HUGE_DUMP,         0, NULL, OPT_HUGE_DUMP_NUM        },
 
 	{0,                     0, NULL, 0                        }
 };
@@ -1942,6 +1943,11 @@ eal_parse_common_option(int opt, const char *optarg,
 			return -1;
 		}
 		break;
+	case OPT_HUGE_DUMP_NUM:
+		conf->huge_dump = 1;
+		EAL_LOG(WARNING, "Using --"OPT_HUGE_DUMP
+			" may result in core dump files which are large or contain sensitive data.");
+		break;
 
 	/* don't know what to do, leave this to caller */
 	default:
@@ -2250,6 +2256,7 @@ eal_common_usage(void)
 	       "  --"OPT_IN_MEMORY"   Operate entirely in memory. This will\n"
 	       "                      disable secondary process support\n"
 	       "  --"OPT_BASE_VIRTADDR"     Base virtual address\n"
+	       "  --"OPT_HUGE_DUMP"   Include hugepages in core dump.\n"
 	       "  --"OPT_TELEMETRY"   Enable telemetry support (on by default)\n"
 	       "  --"OPT_NO_TELEMETRY"   Disable telemetry support\n"
 	       "  --"OPT_FORCE_MAX_SIMD_BITWIDTH" Force the max SIMD bitwidth\n"
diff --git a/lib/eal/common/eal_internal_cfg.h b/lib/eal/common/eal_internal_cfg.h
index 167ec501fa..5e03c3952c 100644
--- a/lib/eal/common/eal_internal_cfg.h
+++ b/lib/eal/common/eal_internal_cfg.h
@@ -103,6 +103,8 @@ struct internal_config {
 	struct simd_bitwidth max_simd_bitwidth;
 	/**< max simd bitwidth path to use */
 	size_t huge_worker_stack_size; /**< worker thread stack size */
+	/** True to include mapped hugepages in coredump. */
+	unsigned int huge_dump;
 };
 
 void eal_reset_internal_config(struct internal_config *internal_cfg);
diff --git a/lib/eal/common/eal_options.h b/lib/eal/common/eal_options.h
index 3cc9cb6412..604d39417d 100644
--- a/lib/eal/common/eal_options.h
+++ b/lib/eal/common/eal_options.h
@@ -89,6 +89,8 @@ enum {
 	OPT_FORCE_MAX_SIMD_BITWIDTH_NUM,
 #define OPT_HUGE_WORKER_STACK  "huge-worker-stack"
 	OPT_HUGE_WORKER_STACK_NUM,
+#define OPT_HUGE_DUMP "huge-dump"
+	OPT_HUGE_DUMP_NUM,
 
 	OPT_LONG_MAX_NUM
 };
diff --git a/lib/eal/freebsd/eal_memory.c b/lib/eal/freebsd/eal_memory.c
index a6f3ba226e..cd99af4b0e 100644
--- a/lib/eal/freebsd/eal_memory.c
+++ b/lib/eal/freebsd/eal_memory.c
@@ -94,6 +94,12 @@ rte_eal_hugepage_init(void)
 
 		eal_memseg_list_populate(msl, addr, n_segs);
 
+		if (internal_conf->huge_dump) {
+			if (eal_mem_set_dump(addr, mem_sz, true) < 0)
+				EAL_LOG(WARNING, "Failed to include pages in core dump (address %p, size %zu): %s",
+					addr, mem_sz, rte_strerror(rte_errno));
+		}
+
 		return 0;
 	}
 
@@ -205,6 +211,12 @@ rte_eal_hugepage_init(void)
 
 			rte_fbarray_set_used(arr, ms_idx);
 
+			if (internal_conf->huge_dump) {
+				if (eal_mem_set_dump(addr, page_sz, true) < 0)
+					EAL_LOG(WARNING, "Failed to include hugepage in core dump (address %p, size %zu): %s",
+						addr, page_sz, rte_strerror(rte_errno));
+			}
+
 			EAL_LOG(INFO, "Mapped memory segment %u @ %p: physaddr:0x%"
 					PRIx64", len %zu",
 					seg_idx++, addr, physaddr, page_sz);
@@ -232,6 +244,7 @@ static int
 attach_segment(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
 		void *arg)
 {
+	struct internal_config *internal_conf = eal_get_internal_configuration();
 	struct attach_walk_args *wa = arg;
 	void *addr;
 
@@ -245,6 +258,12 @@ attach_segment(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
 		return -1;
 	wa->seg_idx++;
 
+	if (internal_conf->huge_dump) {
+		if (eal_mem_set_dump(addr, ms->len, true) < 0)
+			EAL_LOG(WARNING, "Failed to include hugepage in core dump (address %p, size %zu): %s",
+				addr, ms->len, rte_strerror(rte_errno));
+	}
+
 	return 0;
 }
 
diff --git a/lib/eal/linux/eal_memalloc.c b/lib/eal/linux/eal_memalloc.c
index e354efc95d..611b1c65db 100644
--- a/lib/eal/linux/eal_memalloc.c
+++ b/lib/eal/linux/eal_memalloc.c
@@ -28,6 +28,7 @@
 #include <linux/mman.h> /* for hugetlb-related mmap flags */
 
 #include <rte_common.h>
+#include <rte_errno.h>
 #include <rte_log.h>
 #include <rte_eal.h>
 #include <rte_memory.h>
@@ -688,6 +689,13 @@ alloc_seg(struct rte_memseg *ms, void *addr, int socket_id,
 	ms->socket_id = socket_id;
 	ms->flags = dirty ? RTE_MEMSEG_FLAG_DIRTY : 0;
 
+	if (internal_conf->huge_dump) {
+		if (eal_mem_set_dump(addr, alloc_sz, true) < 0)
+			EAL_LOG(WARNING,
+				"Failed to include hugepage in coredump (address %p, size %zu): %s",
+				addr, alloc_sz, rte_strerror(rte_errno));
+	}
+
 	return 0;
 
 mapped:
diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c
index 45879ca743..c1cb8d71e7 100644
--- a/lib/eal/linux/eal_memory.c
+++ b/lib/eal/linux/eal_memory.c
@@ -659,6 +659,7 @@ unmap_unneeded_hugepages(struct hugepage_file *hugepg_tbl,
 static int
 remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end)
 {
+	const struct internal_config *internal_conf = eal_get_internal_configuration();
 	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
 	struct rte_memseg_list *msl;
 	struct rte_fbarray *arr;
@@ -668,10 +669,9 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end)
 	uint64_t page_sz;
 	size_t memseg_len;
 	int socket_id;
-#ifndef RTE_ARCH_64
-	const struct internal_config *internal_conf =
-		eal_get_internal_configuration();
-#endif
+	void *map_addr;
+	size_t map_len;
+
 	page_sz = hugepages[seg_start].size;
 	socket_id = hugepages[seg_start].socket_id;
 	seg_len = seg_end - seg_start;
@@ -721,6 +721,9 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end)
 		return -1;
 	}
 
+	map_addr = RTE_PTR_ADD(msl->base_va, ms_idx * page_sz);
+	map_len = page_sz * seg_len;
+
 #ifdef RTE_ARCH_PPC_64
 	/* for PPC64 we go through the list backwards */
 	for (cur_page = seg_end - 1; cur_page >= seg_start;
@@ -793,6 +796,14 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end)
 			EAL_LOG(ERR, "Could not store segment fd: %s",
 				rte_strerror(rte_errno));
 	}
+
+	if (internal_conf->huge_dump) {
+		if (eal_mem_set_dump(map_addr, map_len, true) < 0)
+			EAL_LOG(WARNING,
+				"Failed to include hugepages in core dump (address %p, size %zu): %s",
+				map_addr, map_len, rte_strerror(rte_errno));
+	}
+
 	EAL_LOG(DEBUG, "Allocated %" PRIu64 "M on socket %i",
 			(seg_len * page_sz) >> 20, socket_id);
 	return seg_len;
@@ -1241,6 +1252,13 @@ eal_legacy_hugepage_init(void)
 					__func__);
 			goto fail;
 		}
+
+		if (internal_conf->huge_dump) {
+			if (eal_mem_set_dump(prealloc_addr, mem_sz, true) < 0)
+				EAL_LOG(WARNING, "Failed to include pages in core dump (address %p, size %zu): %s",
+					prealloc_addr, mem_sz, rte_strerror(rte_errno));
+		}
+
 		return 0;
 	}
 
@@ -1518,6 +1536,7 @@ getFileSize(int fd)
 static int
 eal_legacy_hugepage_attach(void)
 {
+	struct internal_config *internal_conf = eal_get_internal_configuration();
 	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
 	struct hugepage_file *hp = NULL;
 	unsigned int num_hp = 0;
@@ -1616,6 +1635,12 @@ eal_legacy_hugepage_attach(void)
 		if (eal_memalloc_set_seg_fd(msl_idx, ms_idx, fd) < 0)
 			EAL_LOG(ERR, "Could not store segment fd: %s",
 				rte_strerror(rte_errno));
+
+		if (internal_conf->huge_dump) {
+			if (eal_mem_set_dump(map_addr, map_sz, true) < 0)
+				EAL_LOG(WARNING, "Failed to include hugepage in core dump (address %p, size %zu): %s",
+					map_addr, map_sz, rte_strerror(rte_errno));
+		}
 	}
 	/* unmap the hugepage config file, since we are done using it */
 	munmap(hp, size);
@@ -1650,12 +1675,65 @@ eal_hugepage_attach(void)
 	return 0;
 }
 
+static int
+enable_shared_hugepage_coredump(void)
+{
+	const char *path = "/proc/self/coredump_filter";
+	const unsigned long shared_hugepage_flag = RTE_BIT64(6);
+
+	FILE *f;
+	uint64_t coredump_filter;
+
+	f = fopen(path, "r");
+	if (f == NULL) {
+		rte_errno = errno;
+		EAL_LOG(ERR, "Failed to open %s for reading: %s", path, strerror(errno));
+		return -1;
+	}
+
+	if (fscanf(f, "%"SCNx64, &coredump_filter) != 1) {
+		rte_errno = errno;
+		EAL_LOG(ERR, "Failed to parse %s: %s", path, strerror(errno));
+		fclose(f);
+		return -1;
+	}
+
+	fclose(f);
+
+	if (coredump_filter & shared_hugepage_flag)
+		return 0;
+
+	f = fopen(path, "w");
+	if (f == NULL) {
+		rte_errno = errno;
+		EAL_LOG(ERR, "Failed to open %s for writing: %s", path, strerror(errno));
+		return -1;
+	}
+
+	coredump_filter |= shared_hugepage_flag;
+	if (fprintf(f, "%"PRIx64, coredump_filter) <= 0) {
+		rte_errno = EIO;
+		EAL_LOG(ERR, "Failed to write %"PRIx64" to %s", coredump_filter, path);
+		fclose(f);
+		return -1;
+	}
+
+	fclose(f);
+	return 0;
+}
+
 int
 rte_eal_hugepage_init(void)
 {
 	const struct internal_config *internal_conf =
 		eal_get_internal_configuration();
 
+	if (internal_conf->huge_dump && enable_shared_hugepage_coredump() < 0) {
+		EAL_LOG(ERR, "Failed to enable shared hugepage core dump: %s",
+			rte_strerror(rte_errno));
+		return -1;
+	}
+
 	return internal_conf->legacy_mem ?
 			eal_legacy_hugepage_init() :
 			eal_dynmem_hugepage_init();
-- 
2.38.4

Re: [PATCH] eal: support including mapped memory in core dump

[Stephen Hemminger]

On Thu, 24 Oct 2024 02:18:59 +0300
Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:

> From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> 
> Commit d72e4042c5eb ("mem: exclude unused memory from core dump")
> unconditionally excluded all hugepage memory managed by DPDK.
> The rationale was to avoid overly large core dumps
> generated from reserved (PROT_NONE) but not mapped memory.
> Mapped hugepages, however, may hold data useful for debugging,
> even if not being fully used and containing some garbage.
> Users may want to include those hugepages in core dump.
> 
> Add `--huge-dump` EAL command-line option to include in core dump
> all mapped hugepages and also non-hugepage memory
> allocated with `--no-huge` (as it substitutes for hugepages).
> Using this option requires more disk storage for core dumps,
> and also may include sensitive data in core dump,
> which is why it must be explicitly enabled and a warning is printed.
> 
> Linux requires /proc/self/coredump_filter adjustment
> to include hugepages mapped with MAP_SHARED in core dump.
> Windows EAL requires no change since it automatically
> excludes reserved memory and includes committed memory.
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

Has build errors.

Re: [PATCH] eal: support including mapped memory in core dump

[Stephen Hemminger]

On Thu, 24 Oct 2024 02:18:59 +0300
Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:

> --- a/lib/eal/common/eal_internal_cfg.h
> +++ b/lib/eal/common/eal_internal_cfg.h
> @@ -103,6 +103,8 @@ struct internal_config {
>  	struct simd_bitwidth max_simd_bitwidth;
>  	/**< max simd bitwidth path to use */
>  	size_t huge_worker_stack_size; /**< worker thread stack size */
> +	/** True to include mapped hugepages in coredump. */
> +	unsigned int huge_dump;

No need to waste 4 bytes for a flag.
Find an existing one byte hole and use that.

struct internal_config {
	volatile size_t            memory;               /*     0     8 */
	volatile unsigned int      force_nchannel;       /*     8     4 */
	volatile unsigned int      force_nrank;          /*    12     4 */
	volatile unsigned int      no_hugetlbfs;         /*    16     4 */
	struct hugepage_file_discipline hugepage_file;   /*    20     2 */

	/* XXX 2 bytes hole, try to pack */

	volatile unsigned int      no_pci;               /*    24     4 */
	volatile unsigned int      no_hpet;              /*    28     4 */
	volatile unsigned int      vmware_tsc_map;       /*    32     4 */
	volatile unsigned int      no_shconf;            /*    36     4 */
	volatile unsigned int      in_memory;            /*    40     4 */
	volatile unsigned int      create_uio_dev;       /*    44     4 */
	volatile enum rte_proc_type_t  process_type;     /*    48     4 */
	volatile unsigned int      force_sockets;        /*    52     4 */
	volatile volatile uint64_t   socket_mem;         /*    56   256 */
	/* --- cacheline 4 boundary (256 bytes) was 56 bytes ago --- */
	volatile unsigned int      force_socket_limits;  /*   312     4 */

	/* XXX 4 bytes hole, try to pack */

	/* --- cacheline 5 boundary (320 bytes) --- */
	volatile volatile uint64_t   socket_limit;       /*   320   256 */
	/* --- cacheline 9 boundary (576 bytes) --- */
	uintptr_t                  base_virtaddr;        /*   576     8 */
	volatile unsigned int      legacy_mem;           /*   584     4 */
	volatile unsigned int      match_allocations;    /*   588     4 */
	volatile unsigned int      single_file_segments; /*   592     4 */
	volatile int               syslog_facility;      /*   596     4 */
	volatile enum rte_intr_mode  vfio_intr_mode;     /*   600     4 */
	rte_uuid_t                 vfio_vf_token;        /*   604    16 */

	/* XXX 4 bytes hole, try to pack */

	char *                     hugefile_prefix;      /*   624     8 */
	char *                     hugepage_dir;         /*   632     8 */
	/* --- cacheline 10 boundary (640 bytes) --- */
	char *                     user_mbuf_pool_ops_name; /*   640     8 */
	unsigned int               num_hugepage_sizes;   /*   648     4 */

	/* XXX 4 bytes hole, try to pack */

	struct hugepage_info       hugepage_info[3];     /*   656 12720 */
	/* --- cacheline 209 boundary (13376 bytes) --- */
	enum rte_iova_mode         iova_mode;            /* 13376     4 */

	/* XXX 4 bytes hole, try to pack */

	rte_cpuset_t               ctrl_cpuset;          /* 13384   128 */
	/* --- cacheline 211 boundary (13504 bytes) was 8 bytes ago --- */
	volatile unsigned int      init_complete;        /* 13512     4 */
	unsigned int               no_telemetry;         /* 13516     4 */
	struct simd_bitwidth       max_simd_bitwidth;    /* 13520     4 */

	/* XXX last struct has 1 hole */
	/* XXX 4 bytes hole, try to pack */

	size_t                     huge_worker_stack_size; /* 13528     8 */

	/* size: 13536, cachelines: 212, members: 34 */
	/* sum members: 13514, holes: 6, sum holes: 22 */
	/* member types with holes: 1, total: 1 */
	/* last cacheline: 32 bytes */
};

RE: [PATCH] eal: support including mapped memory in core dump

[Morten Brørup]

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Thursday, 24 October 2024 04.31
> 
> On Thu, 24 Oct 2024 02:18:59 +0300
> Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
> 
> > --- a/lib/eal/common/eal_internal_cfg.h
> > +++ b/lib/eal/common/eal_internal_cfg.h
> > @@ -103,6 +103,8 @@ struct internal_config {
> >  	struct simd_bitwidth max_simd_bitwidth;
> >  	/**< max simd bitwidth path to use */
> >  	size_t huge_worker_stack_size; /**< worker thread stack size */
> > +	/** True to include mapped hugepages in coredump. */
> > +	unsigned int huge_dump;
> 
> No need to waste 4 bytes for a flag.
> Find an existing one byte hole and use that.

Disagree with Stephen on this one.
Using unsigned int for Booleans is the convention in this structure.
Please following the convention and stick with unsigned int.

De-bloating this structure belongs in a separate patch series.
I have no clue how "hot" the structure is, so don't know if it is worth the effort.

> 
> struct internal_config {
> 	volatile size_t            memory;               /*     0     8
> */
> 	volatile unsigned int      force_nchannel;       /*     8     4
> */
> 	volatile unsigned int      force_nrank;          /*    12     4
> */
> 	volatile unsigned int      no_hugetlbfs;         /*    16     4
> */
> 	struct hugepage_file_discipline hugepage_file;   /*    20     2
> */
> 
> 	/* XXX 2 bytes hole, try to pack */
> 
> 	volatile unsigned int      no_pci;               /*    24     4
> */
> 	volatile unsigned int      no_hpet;              /*    28     4
> */
> 	volatile unsigned int      vmware_tsc_map;       /*    32     4
> */
> 	volatile unsigned int      no_shconf;            /*    36     4
> */
> 	volatile unsigned int      in_memory;            /*    40     4
> */
> 	volatile unsigned int      create_uio_dev;       /*    44     4
> */
> 	volatile enum rte_proc_type_t  process_type;     /*    48     4
> */
> 	volatile unsigned int      force_sockets;        /*    52     4
> */
> 	volatile volatile uint64_t   socket_mem;         /*    56   256
> */
> 	/* --- cacheline 4 boundary (256 bytes) was 56 bytes ago --- */
> 	volatile unsigned int      force_socket_limits;  /*   312     4
> */
> 
> 	/* XXX 4 bytes hole, try to pack */
> 
> 	/* --- cacheline 5 boundary (320 bytes) --- */
> 	volatile volatile uint64_t   socket_limit;       /*   320   256
> */
> 	/* --- cacheline 9 boundary (576 bytes) --- */
> 	uintptr_t                  base_virtaddr;        /*   576     8
> */
> 	volatile unsigned int      legacy_mem;           /*   584     4
> */
> 	volatile unsigned int      match_allocations;    /*   588     4
> */
> 	volatile unsigned int      single_file_segments; /*   592     4
> */
> 	volatile int               syslog_facility;      /*   596     4
> */
> 	volatile enum rte_intr_mode  vfio_intr_mode;     /*   600     4
> */
> 	rte_uuid_t                 vfio_vf_token;        /*   604    16
> */
> 
> 	/* XXX 4 bytes hole, try to pack */
> 
> 	char *                     hugefile_prefix;      /*   624     8
> */
> 	char *                     hugepage_dir;         /*   632     8
> */
> 	/* --- cacheline 10 boundary (640 bytes) --- */
> 	char *                     user_mbuf_pool_ops_name; /*   640
> 8 */
> 	unsigned int               num_hugepage_sizes;   /*   648     4
> */
> 
> 	/* XXX 4 bytes hole, try to pack */
> 
> 	struct hugepage_info       hugepage_info[3];     /*   656 12720
> */
> 	/* --- cacheline 209 boundary (13376 bytes) --- */
> 	enum rte_iova_mode         iova_mode;            /* 13376     4
> */
> 
> 	/* XXX 4 bytes hole, try to pack */
> 
> 	rte_cpuset_t               ctrl_cpuset;          /* 13384   128
> */
> 	/* --- cacheline 211 boundary (13504 bytes) was 8 bytes ago ---
> */
> 	volatile unsigned int      init_complete;        /* 13512     4
> */
> 	unsigned int               no_telemetry;         /* 13516     4
> */
> 	struct simd_bitwidth       max_simd_bitwidth;    /* 13520     4
> */
> 
> 	/* XXX last struct has 1 hole */
> 	/* XXX 4 bytes hole, try to pack */
> 
> 	size_t                     huge_worker_stack_size; /* 13528     8
> */
> 
> 	/* size: 13536, cachelines: 212, members: 34 */
> 	/* sum members: 13514, holes: 6, sum holes: 22 */
> 	/* member types with holes: 1, total: 1 */
> 	/* last cacheline: 32 bytes */
> };

RE: [PATCH] eal: support including mapped memory in core dump

[Morten Brørup]

> From: Dmitry Kozlyuk [mailto:dmitry.kozliuk@gmail.com]
> Sent: Thursday, 24 October 2024 01.19
> 
> Commit d72e4042c5eb ("mem: exclude unused memory from core dump")
> unconditionally excluded all hugepage memory managed by DPDK.
> The rationale was to avoid overly large core dumps
> generated from reserved (PROT_NONE) but not mapped memory.
> Mapped hugepages, however, may hold data useful for debugging,
> even if not being fully used and containing some garbage.
> Users may want to include those hugepages in core dump.
> 
> Add `--huge-dump` EAL command-line option to include in core dump
> all mapped hugepages and also non-hugepage memory
> allocated with `--no-huge` (as it substitutes for hugepages).

I agree with the behavior of also dumping --no-huge mapped.
Perhaps because of this, the option should be named differently; something like --dump-verbose, --dump-all, --dump-mapped.

> Using this option requires more disk storage for core dumps,
> and also may include sensitive data in core dump,
> which is why it must be explicitly enabled and a warning is printed.
> 
> Linux requires /proc/self/coredump_filter adjustment
> to include hugepages mapped with MAP_SHARED in core dump.
> Windows EAL requires no change since it automatically
> excludes reserved memory and includes committed memory.

The above O/S specific information should also be included as notes in the documentation.

> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
> Lewis, testing on FreeBSD would be appreciated.

If there are any notes about FreeBSD, like the notes about Linux and Windows above, it should also be included in the documentation.

> --- a/doc/guides/linux_gsg/eal_args.include.rst
> +++ b/doc/guides/linux_gsg/eal_args.include.rst
> @@ -122,6 +122,14 @@ Memory-related options
>      to system pthread stack size unless the optional size (in kbytes)
> is
>      specified.
> 
> +*   ``--huge-dump``
> +
> +    Include in code dump all hugepages mapped by DPDK either at
> startup or later,
> +    and ordirary pages mapped by DPDK at startup when ``--no-huge`` is
> given.
> +    Hugepages increase core dumps size roughly by the amount of memory
> used.
> +    Dumped memory often contains processed data, which may be
> sensitive.
> +    Primary and secondary processes can use this option independently.
> +

With O/S specific notes added to the documentation,
Acked-by: Morten Brørup <mb@smartsharesystems.com>

Re: [PATCH] eal: support including mapped memory in core dump

[Dmitry Kozlyuk]

2024-10-24 09:22 (UTC+0200), Morten Brørup:
> > From: Dmitry Kozlyuk [mailto:dmitry.kozliuk@gmail.com]
> > Sent: Thursday, 24 October 2024 01.19
[...]
> > Add `--huge-dump` EAL command-line option to include in core dump
> > all mapped hugepages and also non-hugepage memory
> > allocated with `--no-huge` (as it substitutes for hugepages).  
> 
> I agree with the behavior of also dumping --no-huge mapped.
> Perhaps because of this, the option should be named differently; something
> like --dump-verbose, --dump-all, --dump-mapped. 

Or one may interpret "huge dump" as "the dump will be huge" :)
I'll use "--dump-mapped" as the most precise, thanks.

> > Linux requires /proc/self/coredump_filter adjustment
> > to include hugepages mapped with MAP_SHARED in core dump.
> > Windows EAL requires no change since it automatically
> > excludes reserved memory and includes committed memory.  
> 
> The above O/S specific information should also be included as notes in the documentation.
[...]
> If there are any notes about FreeBSD, like the notes about Linux and
> Windows above, it should also be included in the documentation.

"Linux requires..." here explains what DPDK code in the patch is doing
and "Windows EAL requires no change" explains why there are no code changes.
No OS-specific user action is required for --huge-dump to work.
I'll rephrase that comment and add a release note about the new option,
but I think documentation already contains everything users need.