mempool: check for invalid args on creation
Checks
Commit Message
Currently, a mempool can be created if the number of
objects is zero or the size of these is zero.
In these scenarios, rte_mempool_create should return NULL,
as the mempool created is useless.
Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
lib/librte_mempool/rte_mempool.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
Comments
On 17.07.2018 13:37, Pablo de Lara wrote:
> Currently, a mempool can be created if the number of
> objects is zero or the size of these is zero.
> In these scenarios, rte_mempool_create should return NULL,
> as the mempool created is useless.
>
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
> lib/librte_mempool/rte_mempool.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
> index 8c8b9f809..8c9573f1a 100644
> --- a/lib/librte_mempool/rte_mempool.c
> +++ b/lib/librte_mempool/rte_mempool.c
> @@ -916,6 +916,18 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
>
> mempool_list = RTE_TAILQ_CAST(rte_mempool_tailq.head, rte_mempool_list);
>
> + /* asked for zero items */
> + if (n == 0) {
> + rte_errno = EINVAL;
> + return NULL;
> + }
I agree which the check since attempt to populate it will most likely
fail with -ENOSPC.
> +
> + /* asked for zero-sized elements */
> + if (elt_size == 0) {
> + rte_errno = EINVAL;
> + return NULL;
> + }
> +
I'm not sure about this one. I could imagine the case when mempool
elements are
used just as unique markers. So, I'm not sure that we should restrict
such usage.
> /* asked cache too big */
> if (cache_size > RTE_MEMPOOL_CACHE_MAX_SIZE ||
> CALC_CACHE_FLUSHTHRESH(cache_size) > n) {
Hi Andrew,
> -----Original Message-----
> From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
> Sent: Friday, July 20, 2018 4:49 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> olivier.matz@6wind.com
> Cc: dev@dpdk.org
> Subject: Re: [PATCH] mempool: check for invalid args on creation
>
> On 17.07.2018 13:37, Pablo de Lara wrote:
> > Currently, a mempool can be created if the number of objects is zero
> > or the size of these is zero.
> > In these scenarios, rte_mempool_create should return NULL, as the
> > mempool created is useless.
> >
> > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > ---
> > lib/librte_mempool/rte_mempool.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/lib/librte_mempool/rte_mempool.c
> > b/lib/librte_mempool/rte_mempool.c
> > index 8c8b9f809..8c9573f1a 100644
> > --- a/lib/librte_mempool/rte_mempool.c
> > +++ b/lib/librte_mempool/rte_mempool.c
> > @@ -916,6 +916,18 @@ rte_mempool_create_empty(const char *name,
> > unsigned n, unsigned elt_size,
> >
> > mempool_list = RTE_TAILQ_CAST(rte_mempool_tailq.head,
> > rte_mempool_list);
> >
> > + /* asked for zero items */
> > + if (n == 0) {
> > + rte_errno = EINVAL;
> > + return NULL;
> > + }
>
> I agree which the check since attempt to populate it will most likely fail with -
> ENOSPC.
>
> > +
> > + /* asked for zero-sized elements */
> > + if (elt_size == 0) {
> > + rte_errno = EINVAL;
> > + return NULL;
> > + }
> > +
>
> I'm not sure about this one. I could imagine the case when mempool elements
> are used just as unique markers. So, I'm not sure that we should restrict such
> usage.
I can drop this one and send a v2 on the first check. Is that OK?
Thanks!
Pablo
On 01.08.2018 21:16, De Lara Guarch, Pablo wrote:
> Hi Andrew,
>
>> -----Original Message-----
>> From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
>> Sent: Friday, July 20, 2018 4:49 PM
>> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
>> olivier.matz@6wind.com
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH] mempool: check for invalid args on creation
>>
>> On 17.07.2018 13:37, Pablo de Lara wrote:
>>> Currently, a mempool can be created if the number of objects is zero
>>> or the size of these is zero.
>>> In these scenarios, rte_mempool_create should return NULL, as the
>>> mempool created is useless.
>>>
>>> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
>>> ---
>>> lib/librte_mempool/rte_mempool.c | 12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>>
>>> diff --git a/lib/librte_mempool/rte_mempool.c
>>> b/lib/librte_mempool/rte_mempool.c
>>> index 8c8b9f809..8c9573f1a 100644
>>> --- a/lib/librte_mempool/rte_mempool.c
>>> +++ b/lib/librte_mempool/rte_mempool.c
>>> @@ -916,6 +916,18 @@ rte_mempool_create_empty(const char *name,
>>> unsigned n, unsigned elt_size,
>>>
>>> mempool_list = RTE_TAILQ_CAST(rte_mempool_tailq.head,
>>> rte_mempool_list);
>>>
>>> + /* asked for zero items */
>>> + if (n == 0) {
>>> + rte_errno = EINVAL;
>>> + return NULL;
>>> + }
>> I agree which the check since attempt to populate it will most likely fail with -
>> ENOSPC.
>>
>>> +
>>> + /* asked for zero-sized elements */
>>> + if (elt_size == 0) {
>>> + rte_errno = EINVAL;
>>> + return NULL;
>>> + }
>>> +
>> I'm not sure about this one. I could imagine the case when mempool elements
>> are used just as unique markers. So, I'm not sure that we should restrict such
>> usage.
> I can drop this one and send a v2 on the first check. Is that OK?
Yes. Thanks.
Andrew.
@@ -916,6 +916,18 @@ rte_mempool_create_empty(const char *name, unsigned n, unsigned elt_size,
mempool_list = RTE_TAILQ_CAST(rte_mempool_tailq.head, rte_mempool_list);
+ /* asked for zero items */
+ if (n == 0) {
+ rte_errno = EINVAL;
+ return NULL;
+ }
+
+ /* asked for zero-sized elements */
+ if (elt_size == 0) {
+ rte_errno = EINVAL;
+ return NULL;
+ }
+
/* asked cache too big */
if (cache_size > RTE_MEMPOOL_CACHE_MAX_SIZE ||
CALC_CACHE_FLUSHTHRESH(cache_size) > n) {