* [dpdk-dev] [PATCH] lib: rte_*_create gives NULL/EEXIST on duped name
@ 2015-10-20 20:21 Yoni Fogel
2015-10-21 10:59 ` Bruce Richardson
0 siblings, 1 reply; 4+ messages in thread
From: Yoni Fogel @ 2015-10-20 20:21 UTC (permalink / raw)
To: dev
Also fixed a bug in many of them where if the rte_malloc of
the TAILQ fails, then we return a pointer to some arbitrary
existing struct.
Signed-off-by: Yoni Fogel <yrobot@amazon.com>
---
lib/librte_acl/rte_acl.c | 53 +++++++++++++++++++++------------------
lib/librte_hash/rte_cuckoo_hash.c | 6 +++--
lib/librte_hash/rte_fbk_hash.c | 5 +++-
lib/librte_lpm/rte_lpm.c | 5 +++-
lib/librte_lpm/rte_lpm6.c | 5 +++-
5 files changed, 44 insertions(+), 30 deletions(-)
diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
index d60219f..f591556 100644
--- a/lib/librte_acl/rte_acl.c
+++ b/lib/librte_acl/rte_acl.c
@@ -213,37 +213,40 @@ rte_acl_create(const struct rte_acl_param *param)
break;
}
+ ctx = NULL;
+ if (te != NULL) {
+ rte_errno = EEXIST;
+ goto exit;
+ }
+
/* if ACL with such name doesn't exist, then create a new one. */
- if (te == NULL) {
- ctx = NULL;
- te = rte_zmalloc("ACL_TAILQ_ENTRY", sizeof(*te), 0);
+ te = rte_zmalloc("ACL_TAILQ_ENTRY", sizeof(*te), 0);
- if (te == NULL) {
- RTE_LOG(ERR, ACL, "Cannot allocate tailq entry!\n");
- goto exit;
- }
+ if (te == NULL) {
+ RTE_LOG(ERR, ACL, "Cannot allocate tailq entry!\n");
+ goto exit;
+ }
- ctx = rte_zmalloc_socket(name, sz, RTE_CACHE_LINE_SIZE, param->socket_id);
+ ctx = rte_zmalloc_socket(name, sz, RTE_CACHE_LINE_SIZE, param->socket_id);
- if (ctx == NULL) {
- RTE_LOG(ERR, ACL,
- "allocation of %zu bytes on socket %d for %s failed\n",
- sz, param->socket_id, name);
- rte_free(te);
- goto exit;
- }
- /* init new allocated context. */
- ctx->rules = ctx + 1;
- ctx->max_rules = param->max_rule_num;
- ctx->rule_sz = param->rule_size;
- ctx->socket_id = param->socket_id;
- ctx->alg = rte_acl_default_classify;
- snprintf(ctx->name, sizeof(ctx->name), "%s", param->name);
+ if (ctx == NULL) {
+ RTE_LOG(ERR, ACL,
+ "allocation of %zu bytes on socket %d for %s failed\n",
+ sz, param->socket_id, name);
+ rte_free(te);
+ goto exit;
+ }
+ /* init new allocated context. */
+ ctx->rules = ctx + 1;
+ ctx->max_rules = param->max_rule_num;
+ ctx->rule_sz = param->rule_size;
+ ctx->socket_id = param->socket_id;
+ ctx->alg = rte_acl_default_classify;
+ snprintf(ctx->name, sizeof(ctx->name), "%s", param->name);
- te->data = (void *) ctx;
+ te->data = (void *) ctx;
- TAILQ_INSERT_TAIL(acl_list, te, next);
- }
+ TAILQ_INSERT_TAIL(acl_list, te, next);
exit:
rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 7019763..fe5a79e 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -206,8 +206,10 @@ rte_hash_create(const struct rte_hash_parameters *params)
/* Guarantee there's no existing */
h = rte_hash_find_existing(params->name);
- if (h != NULL)
- return h;
+ if (h != NULL) {
+ rte_errno = EEXIST;
+ return NULL;
+ }
te = rte_zmalloc("HASH_TAILQ_ENTRY", sizeof(*te), 0);
if (te == NULL) {
diff --git a/lib/librte_hash/rte_fbk_hash.c b/lib/librte_hash/rte_fbk_hash.c
index 8752a47..55c9f35 100644
--- a/lib/librte_hash/rte_fbk_hash.c
+++ b/lib/librte_hash/rte_fbk_hash.c
@@ -140,8 +140,11 @@ rte_fbk_hash_create(const struct rte_fbk_hash_params *params)
if (strncmp(params->name, ht->name, RTE_FBK_HASH_NAMESIZE) == 0)
break;
}
- if (te != NULL)
+ ht = NULL;
+ if (te != NULL) {
+ rte_errno = EEXIST;
goto exit;
+ }
te = rte_zmalloc("FBK_HASH_TAILQ_ENTRY", sizeof(*te), 0);
if (te == NULL) {
diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
index 163ba3c..ea3cd44 100644
--- a/lib/librte_lpm/rte_lpm.c
+++ b/lib/librte_lpm/rte_lpm.c
@@ -181,8 +181,11 @@ rte_lpm_create(const char *name, int socket_id, int max_rules,
if (strncmp(name, lpm->name, RTE_LPM_NAMESIZE) == 0)
break;
}
- if (te != NULL)
+ lpm = NULL;
+ if (te != NULL) {
+ rte_errno = EEXIST;
goto exit;
+ }
/* allocate tailq entry */
te = rte_zmalloc("LPM_TAILQ_ENTRY", sizeof(*te), 0);
diff --git a/lib/librte_lpm/rte_lpm6.c b/lib/librte_lpm/rte_lpm6.c
index 6c2b293..ff0bd76 100644
--- a/lib/librte_lpm/rte_lpm6.c
+++ b/lib/librte_lpm/rte_lpm6.c
@@ -182,8 +182,11 @@ rte_lpm6_create(const char *name, int socket_id,
if (strncmp(name, lpm->name, RTE_LPM6_NAMESIZE) == 0)
break;
}
- if (te != NULL)
+ lpm = NULL;
+ if (te != NULL) {
+ rte_errno = EEXIST;
goto exit;
+ }
/* allocate tailq entry */
te = rte_zmalloc("LPM6_TAILQ_ENTRY", sizeof(*te), 0);
--
2.6.2
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [dpdk-dev] [PATCH] lib: rte_*_create gives NULL/EEXIST on duped name
2015-10-20 20:21 [dpdk-dev] [PATCH] lib: rte_*_create gives NULL/EEXIST on duped name Yoni Fogel
@ 2015-10-21 10:59 ` Bruce Richardson
2015-10-21 12:04 ` Ananyev, Konstantin
0 siblings, 1 reply; 4+ messages in thread
From: Bruce Richardson @ 2015-10-21 10:59 UTC (permalink / raw)
To: Yoni Fogel; +Cc: dev
On Tue, Oct 20, 2015 at 01:21:37PM -0700, Yoni Fogel wrote:
> Also fixed a bug in many of them where if the rte_malloc of
> the TAILQ fails, then we return a pointer to some arbitrary
> existing struct.
>
> Signed-off-by: Yoni Fogel <yrobot@amazon.com>
Thanks, looks like good fixes. However, I think for fixes like these they are
better as one-fix-per-patch, because in this case we have the title referring
to one fix, while the comment log describes a completely separate fix.
Can you perhaps do a V2 as a two-patch set?
/Bruce
> ---
> lib/librte_acl/rte_acl.c | 53 +++++++++++++++++++++------------------
> lib/librte_hash/rte_cuckoo_hash.c | 6 +++--
> lib/librte_hash/rte_fbk_hash.c | 5 +++-
> lib/librte_lpm/rte_lpm.c | 5 +++-
> lib/librte_lpm/rte_lpm6.c | 5 +++-
> 5 files changed, 44 insertions(+), 30 deletions(-)
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH] lib: rte_*_create gives NULL/EEXIST on duped name
2015-10-21 10:59 ` Bruce Richardson
@ 2015-10-21 12:04 ` Ananyev, Konstantin
0 siblings, 0 replies; 4+ messages in thread
From: Ananyev, Konstantin @ 2015-10-21 12:04 UTC (permalink / raw)
To: Richardson, Bruce, Yoni Fogel; +Cc: dev
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Wednesday, October 21, 2015 11:59 AM
> To: Yoni Fogel
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] lib: rte_*_create gives NULL/EEXIST on duped name
>
> On Tue, Oct 20, 2015 at 01:21:37PM -0700, Yoni Fogel wrote:
> > Also fixed a bug in many of them where if the rte_malloc of
> > the TAILQ fails, then we return a pointer to some arbitrary
> > existing struct.
Another nit - please don't forget to update comments in the .h files, about changed behaviour.
Konstantin
> >
> > Signed-off-by: Yoni Fogel <yrobot@amazon.com>
>
> Thanks, looks like good fixes. However, I think for fixes like these they are
> better as one-fix-per-patch, because in this case we have the title referring
> to one fix, while the comment log describes a completely separate fix.
> Can you perhaps do a V2 as a two-patch set?
>
> /Bruce
>
> > ---
> > lib/librte_acl/rte_acl.c | 53 +++++++++++++++++++++------------------
> > lib/librte_hash/rte_cuckoo_hash.c | 6 +++--
> > lib/librte_hash/rte_fbk_hash.c | 5 +++-
> > lib/librte_lpm/rte_lpm.c | 5 +++-
> > lib/librte_lpm/rte_lpm6.c | 5 +++-
> > 5 files changed, 44 insertions(+), 30 deletions(-)
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
* [dpdk-dev] [PATCH] lib: rte_*_create gives NULL/EEXIST on duped name
@ 2015-09-18 17:52 Yoni Fogel
0 siblings, 0 replies; 4+ messages in thread
From: Yoni Fogel @ 2015-09-18 17:52 UTC (permalink / raw)
To: dev
Also fixed a bug in many of them where if the rte_malloc of
the TAILQ fails, then we return a pointer to some arbitrary
existing struct.
---
lib/librte_acl/rte_acl.c | 53 +++++++++++++++++++++------------------
lib/librte_hash/rte_cuckoo_hash.c | 6 +++--
lib/librte_hash/rte_fbk_hash.c | 5 +++-
lib/librte_lpm/rte_lpm.c | 5 +++-
lib/librte_lpm/rte_lpm6.c | 5 +++-
5 files changed, 44 insertions(+), 30 deletions(-)
diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
index d60219f..f591556 100644
--- a/lib/librte_acl/rte_acl.c
+++ b/lib/librte_acl/rte_acl.c
@@ -213,37 +213,40 @@ rte_acl_create(const struct rte_acl_param *param)
break;
}
+ ctx = NULL;
+ if (te != NULL) {
+ rte_errno = EEXIST;
+ goto exit;
+ }
+
/* if ACL with such name doesn't exist, then create a new one. */
- if (te == NULL) {
- ctx = NULL;
- te = rte_zmalloc("ACL_TAILQ_ENTRY", sizeof(*te), 0);
+ te = rte_zmalloc("ACL_TAILQ_ENTRY", sizeof(*te), 0);
- if (te == NULL) {
- RTE_LOG(ERR, ACL, "Cannot allocate tailq entry!\n");
- goto exit;
- }
+ if (te == NULL) {
+ RTE_LOG(ERR, ACL, "Cannot allocate tailq entry!\n");
+ goto exit;
+ }
- ctx = rte_zmalloc_socket(name, sz, RTE_CACHE_LINE_SIZE, param->socket_id);
+ ctx = rte_zmalloc_socket(name, sz, RTE_CACHE_LINE_SIZE, param->socket_id);
- if (ctx == NULL) {
- RTE_LOG(ERR, ACL,
- "allocation of %zu bytes on socket %d for %s failed\n",
- sz, param->socket_id, name);
- rte_free(te);
- goto exit;
- }
- /* init new allocated context. */
- ctx->rules = ctx + 1;
- ctx->max_rules = param->max_rule_num;
- ctx->rule_sz = param->rule_size;
- ctx->socket_id = param->socket_id;
- ctx->alg = rte_acl_default_classify;
- snprintf(ctx->name, sizeof(ctx->name), "%s", param->name);
+ if (ctx == NULL) {
+ RTE_LOG(ERR, ACL,
+ "allocation of %zu bytes on socket %d for %s failed\n",
+ sz, param->socket_id, name);
+ rte_free(te);
+ goto exit;
+ }
+ /* init new allocated context. */
+ ctx->rules = ctx + 1;
+ ctx->max_rules = param->max_rule_num;
+ ctx->rule_sz = param->rule_size;
+ ctx->socket_id = param->socket_id;
+ ctx->alg = rte_acl_default_classify;
+ snprintf(ctx->name, sizeof(ctx->name), "%s", param->name);
- te->data = (void *) ctx;
+ te->data = (void *) ctx;
- TAILQ_INSERT_TAIL(acl_list, te, next);
- }
+ TAILQ_INSERT_TAIL(acl_list, te, next);
exit:
rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 7019763..fe5a79e 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -206,8 +206,10 @@ rte_hash_create(const struct rte_hash_parameters *params)
/* Guarantee there's no existing */
h = rte_hash_find_existing(params->name);
- if (h != NULL)
- return h;
+ if (h != NULL) {
+ rte_errno = EEXIST;
+ return NULL;
+ }
te = rte_zmalloc("HASH_TAILQ_ENTRY", sizeof(*te), 0);
if (te == NULL) {
diff --git a/lib/librte_hash/rte_fbk_hash.c b/lib/librte_hash/rte_fbk_hash.c
index 8752a47..55c9f35 100644
--- a/lib/librte_hash/rte_fbk_hash.c
+++ b/lib/librte_hash/rte_fbk_hash.c
@@ -140,8 +140,11 @@ rte_fbk_hash_create(const struct rte_fbk_hash_params *params)
if (strncmp(params->name, ht->name, RTE_FBK_HASH_NAMESIZE) == 0)
break;
}
- if (te != NULL)
+ ht = NULL;
+ if (te != NULL) {
+ rte_errno = EEXIST;
goto exit;
+ }
te = rte_zmalloc("FBK_HASH_TAILQ_ENTRY", sizeof(*te), 0);
if (te == NULL) {
diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
index 163ba3c..ea3cd44 100644
--- a/lib/librte_lpm/rte_lpm.c
+++ b/lib/librte_lpm/rte_lpm.c
@@ -181,8 +181,11 @@ rte_lpm_create(const char *name, int socket_id, int max_rules,
if (strncmp(name, lpm->name, RTE_LPM_NAMESIZE) == 0)
break;
}
- if (te != NULL)
+ lpm = NULL;
+ if (te != NULL) {
+ rte_errno = EEXIST;
goto exit;
+ }
/* allocate tailq entry */
te = rte_zmalloc("LPM_TAILQ_ENTRY", sizeof(*te), 0);
diff --git a/lib/librte_lpm/rte_lpm6.c b/lib/librte_lpm/rte_lpm6.c
index 6c2b293..ff0bd76 100644
--- a/lib/librte_lpm/rte_lpm6.c
+++ b/lib/librte_lpm/rte_lpm6.c
@@ -182,8 +182,11 @@ rte_lpm6_create(const char *name, int socket_id,
if (strncmp(name, lpm->name, RTE_LPM6_NAMESIZE) == 0)
break;
}
- if (te != NULL)
+ lpm = NULL;
+ if (te != NULL) {
+ rte_errno = EEXIST;
goto exit;
+ }
/* allocate tailq entry */
te = rte_zmalloc("LPM6_TAILQ_ENTRY", sizeof(*te), 0);
--
2.5.2
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-10-21 12:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-20 20:21 [dpdk-dev] [PATCH] lib: rte_*_create gives NULL/EEXIST on duped name Yoni Fogel
2015-10-21 10:59 ` Bruce Richardson
2015-10-21 12:04 ` Ananyev, Konstantin
-- strict thread matches above, loose matches on Subject: below --
2015-09-18 17:52 Yoni Fogel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).