Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/87109/?format=api
{ "id": 87109, "url": "http://patchwork.dpdk.org/api/patches/87109/?format=api", "web_url": "http://patchwork.dpdk.org/project/dpdk/patch/eb996f70519934e11d12f84adec3b4974d75a820.1611335831.git.anatoly.burakov@intel.com/", "project": { "id": 1, "url": "http://patchwork.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<eb996f70519934e11d12f84adec3b4974d75a820.1611335831.git.anatoly.burakov@intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/eb996f70519934e11d12f84adec3b4974d75a820.1611335831.git.anatoly.burakov@intel.com", "date": "2021-01-22T17:17:15", "name": "[v3] mem: improve parameter checking on memory hotplug", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "e134d6907c34e7adf94912c8cdb8d5cc4605e118", "submitter": { "id": 4, "url": "http://patchwork.dpdk.org/api/people/4/?format=api", "name": "Burakov, Anatoly", "email": "anatoly.burakov@intel.com" }, "delegate": { "id": 1, "url": "http://patchwork.dpdk.org/api/users/1/?format=api", "username": "tmonjalo", "first_name": "Thomas", "last_name": "Monjalon", "email": "thomas@monjalon.net" }, "mbox": "http://patchwork.dpdk.org/project/dpdk/patch/eb996f70519934e11d12f84adec3b4974d75a820.1611335831.git.anatoly.burakov@intel.com/mbox/", "series": [ { "id": 14909, "url": "http://patchwork.dpdk.org/api/series/14909/?format=api", "web_url": "http://patchwork.dpdk.org/project/dpdk/list/?series=14909", "date": "2021-01-22T17:17:15", "name": "[v3] mem: improve parameter checking on memory hotplug", "version": 3, "mbox": "http://patchwork.dpdk.org/series/14909/mbox/" } ], "comments": "http://patchwork.dpdk.org/api/patches/87109/comments/", "check": "warning", "checks": "http://patchwork.dpdk.org/api/patches/87109/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@inbox.dpdk.org", "Delivered-To": "patchwork@inbox.dpdk.org", "Received": [ "from mails.dpdk.org (mails.dpdk.org [217.70.189.124])\n\tby inbox.dpdk.org (Postfix) with ESMTP id D1C60A0A0A;\n\tFri, 22 Jan 2021 18:17:19 +0100 (CET)", "from [217.70.189.124] (localhost [127.0.0.1])\n\tby mails.dpdk.org (Postfix) with ESMTP id 58DBA141047;\n\tFri, 22 Jan 2021 18:17:19 +0100 (CET)", "from mga17.intel.com (mga17.intel.com [192.55.52.151])\n by mails.dpdk.org (Postfix) with ESMTP id 9747114103D\n for <dev@dpdk.org>; Fri, 22 Jan 2021 18:17:17 +0100 (CET)", "from fmsmga004.fm.intel.com ([10.253.24.48])\n by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n 22 Jan 2021 09:17:16 -0800", "from silpixa00399498.ir.intel.com (HELO\n silpixa00399498.ger.corp.intel.com) ([10.237.222.179])\n by fmsmga004.fm.intel.com with ESMTP; 22 Jan 2021 09:17:16 -0800" ], "IronPort-SDR": [ "\n 17GYvnmT/BTZZgCwL5lyX/HJHG2kT9Xvg5/urA2RDbuEmQzKWEnp5c5P06KD820u1QLgp2BCw8\n qiOcll4twtTA==", "\n enKRPkroE76P1B769pHuKO2YA+ZOzr3P9Ijm3cTD9NNwaAQELCDNvtSI+xyY1UiUlxlUJUCxlQ\n FNEu8n3RExOA==" ], "X-IronPort-AV": [ "E=McAfee;i=\"6000,8403,9872\"; a=\"159251155\"", "E=Sophos;i=\"5.79,367,1602572400\"; d=\"scan'208\";a=\"159251155\"", "E=Sophos;i=\"5.79,367,1602572400\"; d=\"scan'208\";a=\"400734244\"" ], "X-ExtLoop1": "1", "From": "Anatoly Burakov <anatoly.burakov@intel.com>", "To": "dev@dpdk.org", "Date": "Fri, 22 Jan 2021 17:17:15 +0000", "Message-Id": "\n <eb996f70519934e11d12f84adec3b4974d75a820.1611335831.git.anatoly.burakov@intel.com>", "X-Mailer": "git-send-email 2.25.1", "In-Reply-To": "\n <20ad523bdee024979dfc74cc32c8c02954cd1d4c.1610984505.git.anatoly.burakov@intel.com>", "References": "\n <20ad523bdee024979dfc74cc32c8c02954cd1d4c.1610984505.git.anatoly.burakov@intel.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[dpdk-dev] [PATCH v3] mem: improve parameter checking on memory\n hotplug", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n <mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://mails.dpdk.org/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n <mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "content": "Currently, we don't check anything that comes in through memory hotplug\nsubsystem using the IPC, because we always assume the data is correct.\nThis is okay as anyone having access to the IPC socket would also have\nrights to crash the DPDK process through other means, but it's still a\ngood practice to do parameter checking, so fix the code to do that.\n\nSigned-off-by: Anatoly Burakov <anatoly.burakov@intel.com>\n---\n\nNotes:\n v3:\n - Fixed typos\n \n v2:\n - Fixed 32-bit compile issues\n\n lib/librte_eal/common/malloc_heap.c | 3 +-\n lib/librte_eal/common/malloc_mp.c | 70 +++++++++++++++++++++++++----\n lib/librte_eal/common/malloc_mp.h | 4 +-\n 3 files changed, 66 insertions(+), 11 deletions(-)", "diff": "diff --git a/lib/librte_eal/common/malloc_heap.c b/lib/librte_eal/common/malloc_heap.c\nindex 5a09247a65..ee400f38ec 100644\n--- a/lib/librte_eal/common/malloc_heap.c\n+++ b/lib/librte_eal/common/malloc_heap.c\n@@ -460,6 +460,7 @@ try_expand_heap_secondary(struct malloc_heap *heap, uint64_t pg_sz,\n \t\tsize_t elt_size, int socket, unsigned int flags, size_t align,\n \t\tsize_t bound, bool contig)\n {\n+\tstruct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;\n \tstruct malloc_mp_req req;\n \tint req_result;\n \n@@ -473,7 +474,7 @@ try_expand_heap_secondary(struct malloc_heap *heap, uint64_t pg_sz,\n \treq.alloc_req.elt_size = elt_size;\n \treq.alloc_req.page_sz = pg_sz;\n \treq.alloc_req.socket = socket;\n-\treq.alloc_req.heap = heap; /* it's in shared memory */\n+\treq.alloc_req.malloc_heap_idx = heap - mcfg->malloc_heaps;\n \n \treq_result = request_to_primary(&req);\n \ndiff --git a/lib/librte_eal/common/malloc_mp.c b/lib/librte_eal/common/malloc_mp.c\nindex 1f212f8349..0b19d4d5fb 100644\n--- a/lib/librte_eal/common/malloc_mp.c\n+++ b/lib/librte_eal/common/malloc_mp.c\n@@ -11,6 +11,7 @@\n \n #include \"eal_memalloc.h\"\n #include \"eal_memcfg.h\"\n+#include \"eal_private.h\"\n \n #include \"malloc_elem.h\"\n #include \"malloc_mp.h\"\n@@ -175,10 +176,49 @@ handle_sync(const struct rte_mp_msg *msg, const void *peer)\n \treturn 0;\n }\n \n+static int\n+handle_free_request(const struct malloc_mp_req *m)\n+{\n+\tconst struct rte_memseg_list *msl;\n+\tvoid *start, *end;\n+\tsize_t len;\n+\n+\tlen = m->free_req.len;\n+\tstart = m->free_req.addr;\n+\tend = RTE_PTR_ADD(start, len - 1);\n+\n+\t/* check if the requested memory actually exists */\n+\tmsl = rte_mem_virt2memseg_list(start);\n+\tif (msl == NULL) {\n+\t\tRTE_LOG(ERR, EAL, \"Requested to free unknown memory\\n\");\n+\t\treturn -1;\n+\t}\n+\n+\t/* check if end is within the same memory region */\n+\tif (rte_mem_virt2memseg_list(end) != msl) {\n+\t\tRTE_LOG(ERR, EAL, \"Requested to free memory spanning multiple regions\\n\");\n+\t\treturn -1;\n+\t}\n+\n+\t/* we're supposed to only free memory that's not external */\n+\tif (msl->external) {\n+\t\tRTE_LOG(ERR, EAL, \"Requested to free external memory\\n\");\n+\t\treturn -1;\n+\t}\n+\n+\t/* now that we've validated the request, announce it */\n+\teal_memalloc_mem_event_notify(RTE_MEM_EVENT_FREE,\n+\t\t\tm->free_req.addr, m->free_req.len);\n+\n+\t/* now, do the actual freeing */\n+\treturn malloc_heap_free_pages(m->free_req.addr, m->free_req.len);\n+}\n+\n static int\n handle_alloc_request(const struct malloc_mp_req *m,\n \t\tstruct mp_request *req)\n {\n+\tstruct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;\n \tconst struct malloc_req_alloc *ar = &m->alloc_req;\n \tstruct malloc_heap *heap;\n \tstruct malloc_elem *elem;\n@@ -187,17 +227,35 @@ handle_alloc_request(const struct malloc_mp_req *m,\n \tint n_segs;\n \tvoid *map_addr;\n \n+\t/* this is checked by the API, but we need to prevent divide by zero */\n+\tif (ar->page_sz == 0 || !rte_is_power_of_2(ar->page_sz)) {\n+\t\tRTE_LOG(ERR, EAL, \"Attempting to allocate with invalid page size\\n\");\n+\t\treturn -1;\n+\t}\n+\n+\t/* heap idx is index into the heap array, not socket ID */\n+\tif (ar->malloc_heap_idx >= RTE_MAX_HEAPS) {\n+\t\tRTE_LOG(ERR, EAL, \"Attempting to allocate from invalid heap\\n\");\n+\t\treturn -1;\n+\t}\n+\n+\theap = &mcfg->malloc_heaps[ar->malloc_heap_idx];\n+\n+\t/* for allocations, we must only use internal heaps */\n+\tif (rte_malloc_heap_socket_is_external(heap->socket_id)) {\n+\t\tRTE_LOG(ERR, EAL, \"Attempting to allocate from external heap\\n\");\n+\t\treturn -1;\n+\t}\n+\n \talloc_sz = RTE_ALIGN_CEIL(ar->align + ar->elt_size +\n \t\t\tMALLOC_ELEM_TRAILER_LEN, ar->page_sz);\n \tn_segs = alloc_sz / ar->page_sz;\n \n-\theap = ar->heap;\n-\n \t/* we can't know in advance how many pages we'll need, so we malloc */\n \tms = malloc(sizeof(*ms) * n_segs);\n \tif (ms == NULL) {\n \t\tRTE_LOG(ERR, EAL, \"Couldn't allocate memory for request state\\n\");\n-\t\tgoto fail;\n+\t\treturn -1;\n \t}\n \tmemset(ms, 0, sizeof(*ms) * n_segs);\n \n@@ -261,11 +319,7 @@ handle_request(const struct rte_mp_msg *msg, const void *peer __rte_unused)\n \tif (m->t == REQ_TYPE_ALLOC) {\n \t\tret = handle_alloc_request(m, entry);\n \t} else if (m->t == REQ_TYPE_FREE) {\n-\t\teal_memalloc_mem_event_notify(RTE_MEM_EVENT_FREE,\n-\t\t\t\tm->free_req.addr, m->free_req.len);\n-\n-\t\tret = malloc_heap_free_pages(m->free_req.addr,\n-\t\t\t\tm->free_req.len);\n+\t\tret = handle_free_request(m);\n \t} else {\n \t\tRTE_LOG(ERR, EAL, \"Unexpected request from secondary\\n\");\n \t\tgoto fail;\ndiff --git a/lib/librte_eal/common/malloc_mp.h b/lib/librte_eal/common/malloc_mp.h\nindex 2b86b76f68..0095062b72 100644\n--- a/lib/librte_eal/common/malloc_mp.h\n+++ b/lib/librte_eal/common/malloc_mp.h\n@@ -30,7 +30,7 @@ enum malloc_req_result {\n };\n \n struct malloc_req_alloc {\n-\tstruct malloc_heap *heap;\n+\tuint32_t malloc_heap_idx;\n \tuint64_t page_sz;\n \tsize_t elt_size;\n \tint socket;\n@@ -46,7 +46,7 @@ struct malloc_req_free {\n \t\tvoid *addr;\n \t\tuint64_t addr_64;\n \t};\n-\tuint64_t len;\n+\tsize_t len;\n };\n \n struct malloc_mp_req {\n", "prefixes": [ "v3" ] }