Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/11525/?format=api
{ "id": 11525, "url": "http://patches.dpdk.org/api/patches/11525/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/patch/1458044745-32764-1-git-send-email-olivier.matz@6wind.com/", "project": { "id": 1, "url": "http://patches.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": "<1458044745-32764-1-git-send-email-olivier.matz@6wind.com>", "list_archive_url": "https://inbox.dpdk.org/dev/1458044745-32764-1-git-send-email-olivier.matz@6wind.com", "date": "2016-03-15T12:25:45", "name": "[dpdk-dev,RFC] hash/lpm: return NULL if the object exists", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": true, "hash": "a9afc0b07b8e8ab4bc7fd98ee141dc002a694347", "submitter": { "id": 8, "url": "http://patches.dpdk.org/api/people/8/?format=api", "name": "Olivier Matz", "email": "olivier.matz@6wind.com" }, "delegate": null, "mbox": "http://patches.dpdk.org/project/dpdk/patch/1458044745-32764-1-git-send-email-olivier.matz@6wind.com/mbox/", "series": [], "comments": "http://patches.dpdk.org/api/patches/11525/comments/", "check": "pending", "checks": "http://patches.dpdk.org/api/patches/11525/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [IPv6:::1])\n\tby dpdk.org (Postfix) with ESMTP id CB3775584;\n\tTue, 15 Mar 2016 13:25:57 +0100 (CET)", "from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com\n\t[62.23.145.76]) by dpdk.org (Postfix) with ESMTP id 6D13C2C1A\n\tfor <dev@dpdk.org>; Tue, 15 Mar 2016 13:25:57 +0100 (CET)", "from glumotte.dev.6wind.com (unknown [10.16.0.195])\n\tby proxy.6wind.com (Postfix) with ESMTP id 7B8CB24ACF\n\tfor <dev@dpdk.org>; Tue, 15 Mar 2016 13:25:14 +0100 (CET)" ], "From": "Olivier Matz <olivier.matz@6wind.com>", "To": "dev@dpdk.org", "Date": "Tue, 15 Mar 2016 13:25:45 +0100", "Message-Id": "<1458044745-32764-1-git-send-email-olivier.matz@6wind.com>", "X-Mailer": "git-send-email 2.1.4", "Subject": "[dpdk-dev] [RFC] hash/lpm: return NULL if the object exists", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "patches and discussions about DPDK <dev.dpdk.org>", "List-Unsubscribe": "<http://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<http://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "content": "Seen by trying to fix the func_reentrancy autotest. The test\nwas doing the following on several cores in parallel:\n\n name = \"common_name\";\n do several times {\n obj = allocate_an_object(name) // obj = ring, mempool, hash, lpm, ...\n if (obj == NULL && lookup(name) == NULL)\n return TEST_FAIL;\n }\n\nIssues:\n\n1/ rings, mempools, hashs API are not coherent\n rings and mempool return NULL if the object does not exist\n hash and lpm return an object that was allocated allocated if\n it already was allocated\n\n2/ The hash/lpm API looks dangerous: when an object is returned,\n the user does not know if it should be freed or not (no refcnt)\n\n3/ There are some possible race conditions in cuckoo_hash as the\n lock is not held in rte_hash_create(). We could find some cases\n where NULL is returned when the object already exists (ex: when\n rte_ring_create() fails).\n\nThis patch tries to rationalize the APIs of lpm and hash.\n\nSigned-off-by: Olivier Matz <olivier.matz@6wind.com>\n---\n app/test/test_func_reentrancy.c | 31 +++++++++++++++++++++----------\n app/test/test_lpm6.c | 2 +-\n lib/librte_hash/rte_cuckoo_hash.c | 2 +-\n lib/librte_hash/rte_fbk_hash.c | 4 +++-\n lib/librte_lpm/rte_lpm.c | 8 ++++++--\n lib/librte_lpm/rte_lpm6.c | 4 +++-\n 6 files changed, 35 insertions(+), 16 deletions(-)", "diff": "diff --git a/app/test/test_func_reentrancy.c b/app/test/test_func_reentrancy.c\nindex 5d09296..300a3bc 100644\n--- a/app/test/test_func_reentrancy.c\n+++ b/app/test/test_func_reentrancy.c\n@@ -83,6 +83,7 @@ typedef void (*case_clean_t)(unsigned lcore_id);\n \n #define MAX_LCORES\tRTE_MAX_MEMZONE / (MAX_ITER_TIMES * 4U)\n \n+static rte_atomic32_t obj_count = RTE_ATOMIC32_INIT(0);\n static rte_atomic32_t synchro = RTE_ATOMIC32_INIT(0);\n \n #define WAIT_SYNCHRO_FOR_SLAVES() do{ \\\n@@ -100,6 +101,7 @@ test_eal_init_once(__attribute__((unused)) void *arg)\n \n \tWAIT_SYNCHRO_FOR_SLAVES();\n \n+\trte_atomic32_set(&obj_count, 1); /* silent the check in the caller */\n \tif (rte_eal_init(0, NULL) != -1)\n \t\treturn -1;\n \n@@ -122,8 +124,8 @@ ring_create_lookup(__attribute__((unused)) void *arg)\n \t/* create the same ring simultaneously on all threads */\n \tfor (i = 0; i < MAX_ITER_TIMES; i++) {\n \t\trp = rte_ring_create(\"fr_test_once\", 4096, SOCKET_ID_ANY, 0);\n-\t\tif ((NULL == rp) && (rte_ring_lookup(\"fr_test_once\") == NULL))\n-\t\t\treturn -1;\n+\t\tif (rp != NULL)\n+\t\t\trte_atomic32_inc(&obj_count);\n \t}\n \n \t/* create/lookup new ring several times */\n@@ -172,8 +174,8 @@ mempool_create_lookup(__attribute__((unused)) void *arg)\n \t\t\t\t\tNULL, NULL,\n \t\t\t\t\tmy_obj_init, NULL,\n \t\t\t\t\tSOCKET_ID_ANY, 0);\n-\t\tif ((NULL == mp) && (rte_mempool_lookup(\"fr_test_once\") == NULL))\n-\t\t\treturn -1;\n+\t\tif (mp != NULL)\n+\t\t\trte_atomic32_inc(&obj_count);\n \t}\n \n \t/* create/lookup new ring several times */\n@@ -238,8 +240,8 @@ hash_create_free(__attribute__((unused)) void *arg)\n \thash_params.name = \"fr_test_once\";\n \tfor (i = 0; i < MAX_ITER_TIMES; i++) {\n \t\thandle = rte_hash_create(&hash_params);\n-\t\tif ((NULL == handle) && (rte_hash_find_existing(\"fr_test_once\") == NULL))\n-\t\t\treturn -1;\n+\t\tif (handle != NULL)\n+\t\t\trte_atomic32_inc(&obj_count);\n \t}\n \n \t/* create mutiple times simultaneously */\n@@ -306,8 +308,8 @@ fbk_create_free(__attribute__((unused)) void *arg)\n \tfbk_params.name = \"fr_test_once\";\n \tfor (i = 0; i < MAX_ITER_TIMES; i++) {\n \t\thandle = rte_fbk_hash_create(&fbk_params);\n-\t\tif ((NULL == handle) && (rte_fbk_hash_find_existing(\"fr_test_once\") == NULL))\n-\t\t\treturn -1;\n+\t\tif (handle != NULL)\n+\t\t\trte_atomic32_inc(&obj_count);\n \t}\n \n \t/* create mutiple fbk tables simultaneously */\n@@ -372,8 +374,8 @@ lpm_create_free(__attribute__((unused)) void *arg)\n \t/* create the same lpm simultaneously on all threads */\n \tfor (i = 0; i < MAX_ITER_TIMES; i++) {\n \t\tlpm = rte_lpm_create(\"fr_test_once\", SOCKET_ID_ANY, &config);\n-\t\tif ((NULL == lpm) && (rte_lpm_find_existing(\"fr_test_once\") == NULL))\n-\t\t\treturn -1;\n+\t\tif (lpm != NULL)\n+\t\t\trte_atomic32_inc(&obj_count);\n \t}\n \n \t/* create mutiple fbk tables simultaneously */\n@@ -432,10 +434,12 @@ launch_test(struct test_case *pt_case)\n \tunsigned lcore_id;\n \tunsigned cores_save = rte_lcore_count();\n \tunsigned cores = RTE_MIN(cores_save, MAX_LCORES);\n+\tunsigned count;\n \n \tif (pt_case->func == NULL)\n \t\treturn -1;\n \n+\trte_atomic32_set(&obj_count, 0);\n \trte_atomic32_set(&synchro, 0);\n \n \tRTE_LCORE_FOREACH_SLAVE(lcore_id) {\n@@ -462,6 +466,13 @@ launch_test(struct test_case *pt_case)\n \t\t\tpt_case->clean(lcore_id);\n \t}\n \n+\tcount = rte_atomic32_read(&obj_count);\n+\tif (count != 1) {\n+\t\tprintf(\"%s: common object allocated %d times (should be 1)\\n\",\n+\t\t\tpt_case->name, count);\n+\t\tret = -1;\n+\t}\n+\n \treturn ret;\n }\n \ndiff --git a/app/test/test_lpm6.c b/app/test/test_lpm6.c\nindex 1f88d7a..b464342 100644\n--- a/app/test/test_lpm6.c\n+++ b/app/test/test_lpm6.c\n@@ -222,7 +222,7 @@ test1(void)\n \n \t/* rte_lpm6_create: lpm name == LPM2 */\n \tlpm3 = rte_lpm6_create(\"LPM1\", SOCKET_ID_ANY, &config);\n-\tTEST_LPM_ASSERT(lpm3 == lpm1);\n+\tTEST_LPM_ASSERT(lpm3 == NULL);\n \n \trte_lpm6_free(lpm1);\n \trte_lpm6_free(lpm2);\ndiff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c\nindex 71b5b76..857e152 100644\n--- a/lib/librte_hash/rte_cuckoo_hash.c\n+++ b/lib/librte_hash/rte_cuckoo_hash.c\n@@ -231,7 +231,7 @@ rte_hash_create(const struct rte_hash_parameters *params)\n \t/* Guarantee there's no existing */\n \th = rte_hash_find_existing(params->name);\n \tif (h != NULL)\n-\t\treturn h;\n+\t\treturn NULL;\n \n \tte = rte_zmalloc(\"HASH_TAILQ_ENTRY\", sizeof(*te), 0);\n \tif (te == NULL) {\ndiff --git a/lib/librte_hash/rte_fbk_hash.c b/lib/librte_hash/rte_fbk_hash.c\nindex 8752a47..ea854f1 100644\n--- a/lib/librte_hash/rte_fbk_hash.c\n+++ b/lib/librte_hash/rte_fbk_hash.c\n@@ -140,8 +140,10 @@ rte_fbk_hash_create(const struct rte_fbk_hash_params *params)\n \t\tif (strncmp(params->name, ht->name, RTE_FBK_HASH_NAMESIZE) == 0)\n \t\t\tbreak;\n \t}\n-\tif (te != NULL)\n+\tif (te != NULL) {\n+\t\tht = NULL;\n \t\tgoto exit;\n+\t}\n \n \tte = rte_zmalloc(\"FBK_HASH_TAILQ_ENTRY\", sizeof(*te), 0);\n \tif (te == NULL) {\ndiff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c\nindex ccaaa2a..dd62f9b 100644\n--- a/lib/librte_lpm/rte_lpm.c\n+++ b/lib/librte_lpm/rte_lpm.c\n@@ -209,8 +209,10 @@ rte_lpm_create_v20(const char *name, int socket_id, int max_rules,\n \t\tif (strncmp(name, lpm->name, RTE_LPM_NAMESIZE) == 0)\n \t\t\tbreak;\n \t}\n-\tif (te != NULL)\n+\tif (te != NULL) {\n+\t\tlpm = NULL;\n \t\tgoto exit;\n+\t}\n \n \t/* allocate tailq entry */\n \tte = rte_zmalloc(\"LPM_TAILQ_ENTRY\", sizeof(*te), 0);\n@@ -280,8 +282,10 @@ rte_lpm_create_v1604(const char *name, int socket_id,\n \t\tif (strncmp(name, lpm->name, RTE_LPM_NAMESIZE) == 0)\n \t\t\tbreak;\n \t}\n-\tif (te != NULL)\n+\tif (te != NULL) {\n+\t\tlpm = NULL;\n \t\tgoto exit;\n+\t}\n \n \t/* allocate tailq entry */\n \tte = rte_zmalloc(\"LPM_TAILQ_ENTRY\", sizeof(*te), 0);\ndiff --git a/lib/librte_lpm/rte_lpm6.c b/lib/librte_lpm/rte_lpm6.c\nindex 6c2b293..4e9f2d0 100644\n--- a/lib/librte_lpm/rte_lpm6.c\n+++ b/lib/librte_lpm/rte_lpm6.c\n@@ -182,8 +182,10 @@ rte_lpm6_create(const char *name, int socket_id,\n \t\tif (strncmp(name, lpm->name, RTE_LPM6_NAMESIZE) == 0)\n \t\t\tbreak;\n \t}\n-\tif (te != NULL)\n+\tif (te != NULL) {\n+\t\tlpm = NULL;\n \t\tgoto exit;\n+\t}\n \n \t/* allocate tailq entry */\n \tte = rte_zmalloc(\"LPM6_TAILQ_ENTRY\", sizeof(*te), 0);\n", "prefixes": [ "dpdk-dev", "RFC" ] }