[dpdk-dev] [PATCH] ethdev: report error on name truncation

[dpdk-dev] [PATCH] ethdev: report error on name truncation

[Nithin Kumar Dabilpuram]

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
 lib/librte_ethdev/rte_ethdev.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 9d5107d..bd45445 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -3588,9 +3588,17 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
 {
 	char z_name[RTE_MEMZONE_NAMESIZE];
 	const struct rte_memzone *mz;
+	int rc;
 
-	snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
-		 dev->data->port_id, queue_id, ring_name);
+	rc = snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
+		      dev->device->driver->name, ring_name,
+		      dev->data->port_id, queue_id);
+
+	if (rc >= RTE_MEMZONE_NAMESIZE) {
+		RTE_LOG(ERR, EAL, "%s(): truncated name\n", __func__);
+		rte_errno = ENAMETOOLONG;
+		return NULL;
+	}
 
 	mz = rte_memzone_lookup(z_name);
 	if (mz)
-- 
2.8.4

Re: [dpdk-dev] [PATCH] ethdev: report error on name truncation

[Andrew Rybchenko]

On 1/7/19 5:40 PM, Nithin Kumar Dabilpuram wrote:
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
>   lib/librte_ethdev/rte_ethdev.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 9d5107d..bd45445 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -3588,9 +3588,17 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
>   {
>   	char z_name[RTE_MEMZONE_NAMESIZE];
>   	const struct rte_memzone *mz;
> +	int rc;
>   
> -	snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
> -		 dev->data->port_id, queue_id, ring_name);
> +	rc = snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
> +		      dev->device->driver->name, ring_name,
> +		      dev->data->port_id, queue_id);
> +
> +	if (rc >= RTE_MEMZONE_NAMESIZE) {
> +		RTE_LOG(ERR, EAL, "%s(): truncated name\n", __func__);
> +		rte_errno = ENAMETOOLONG;
> +		return NULL;
> +	}
>   
>   	mz = rte_memzone_lookup(z_name);
>   	if (mz)

It is good to report an error in the case of name truncation, but the patch
does more. It changes format of the memzone name, adds the driver name
in it (what is bad since testpmd has commands to find the memzone by name
and read descriptors (hack, but sometimes very useful)).
Also I'm not sure about function name in the log message. Other places
do not have it.

Re: [dpdk-dev] [PATCH] ethdev: report error on name truncation

[Thomas Monjalon]

Hi,

Please provide an explanation of the current behaviour.

07/01/2019 15:40, Nithin Kumar Dabilpuram:
> -	snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
> -		 dev->data->port_id, queue_id, ring_name);
> +	rc = snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
> +		      dev->device->driver->name, ring_name,
> +		      dev->data->port_id, queue_id);

You should keep the same name format.
Please check this commit:
	http://git.dpdk.org/dpdk/commit/?id=5e046832

Re: [dpdk-dev] [PATCH] ethdev: report error on name truncation

[Stephen Hemminger]

On Mon, 7 Jan 2019 17:47:08 +0300
Andrew Rybchenko <arybchenko@solarflare.com> wrote:

> On 1/7/19 5:40 PM, Nithin Kumar Dabilpuram wrote:
> > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > ---
> >   lib/librte_ethdev/rte_ethdev.c | 12 ++++++++++--
> >   1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> > index 9d5107d..bd45445 100644
> > --- a/lib/librte_ethdev/rte_ethdev.c
> > +++ b/lib/librte_ethdev/rte_ethdev.c
> > @@ -3588,9 +3588,17 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
> >   {
> >   	char z_name[RTE_MEMZONE_NAMESIZE];
> >   	const struct rte_memzone *mz;
> > +	int rc;
> >   
> > -	snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
> > -		 dev->data->port_id, queue_id, ring_name);
> > +	rc = snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
> > +		      dev->device->driver->name, ring_name,
> > +		      dev->data->port_id, queue_id);
> > +
> > +	if (rc >= RTE_MEMZONE_NAMESIZE) {
> > +		RTE_LOG(ERR, EAL, "%s(): truncated name\n", __func__);
> > +		rte_errno = ENAMETOOLONG;
> > +		return NULL;
> > +	}
> >   
> >   	mz = rte_memzone_lookup(z_name);
> >   	if (mz)  
> 
> It is good to report an error in the case of name truncation, but the patch
> does more. It changes format of the memzone name, adds the driver name
> in it (what is bad since testpmd has commands to find the memzone by name
> and read descriptors (hack, but sometimes very useful)).
> Also I'm not sure about function name in the log message. Other places
> do not have it.
> 

Maybe MEMZONE_NAMESIZE should be big enough that this should never happen?
The size is arbitrary anyway.

Re: [dpdk-dev] [PATCH] ethdev: report error on name truncation

[Andrew Rybchenko]

On 1/7/19 6:53 PM, Stephen Hemminger wrote:
> On Mon, 7 Jan 2019 17:47:08 +0300
> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
>
>> On 1/7/19 5:40 PM, Nithin Kumar Dabilpuram wrote:
>>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>> ---
>>>    lib/librte_ethdev/rte_ethdev.c | 12 ++++++++++--
>>>    1 file changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>>> index 9d5107d..bd45445 100644
>>> --- a/lib/librte_ethdev/rte_ethdev.c
>>> +++ b/lib/librte_ethdev/rte_ethdev.c
>>> @@ -3588,9 +3588,17 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
>>>    {
>>>    	char z_name[RTE_MEMZONE_NAMESIZE];
>>>    	const struct rte_memzone *mz;
>>> +	int rc;
>>>    
>>> -	snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
>>> -		 dev->data->port_id, queue_id, ring_name);
>>> +	rc = snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
>>> +		      dev->device->driver->name, ring_name,
>>> +		      dev->data->port_id, queue_id);
>>> +
>>> +	if (rc >= RTE_MEMZONE_NAMESIZE) {
>>> +		RTE_LOG(ERR, EAL, "%s(): truncated name\n", __func__);
>>> +		rte_errno = ENAMETOOLONG;
>>> +		return NULL;
>>> +	}
>>>    
>>>    	mz = rte_memzone_lookup(z_name);
>>>    	if (mz)
>> It is good to report an error in the case of name truncation, but the patch
>> does more. It changes format of the memzone name, adds the driver name
>> in it (what is bad since testpmd has commands to find the memzone by name
>> and read descriptors (hack, but sometimes very useful)).
>> Also I'm not sure about function name in the log message. Other places
>> do not have it.
>>
> Maybe MEMZONE_NAMESIZE should be big enough that this should never happen?
> The size is arbitrary anyway.

ring_name is provided by caller, so we can't guarantee it locally.

Re: [dpdk-dev] [EXT] Re: [PATCH] ethdev: report error on name truncation

[Nithin Kumar Dabilpuram]

Hi Thomas,

Sorry, missed to notice that format changed by your commit recently got overridden 
during some of our internal branch merging. I'll send a v2 with same format.

--
Thanks
Nithin



From: Thomas Monjalon <thomas@monjalon.net>
Sent: Monday, January 7, 2019 8:20 PM
To: Nithin Kumar Dabilpuram
Cc: Ferruh Yigit; Andrew Rybchenko; dev@dpdk.org; Jerin Jacob Kollanukkaran
Subject: [EXT] Re: [PATCH] ethdev: report error on name truncation
  

External Email

----------------------------------------------------------------------
Hi,

Please provide an explanation of the current behaviour.

07/01/2019 15:40, Nithin Kumar Dabilpuram:
> -     snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
> -              dev->data->port_id, queue_id, ring_name);
> +     rc = snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",
> +                   dev->device->driver->name, ring_name,
> +                   dev->data->port_id, queue_id);

You should keep the same name format.
Please check this commit:
        http://git.dpdk.org/dpdk/commit/?id=5e046832


    

Re: [dpdk-dev] [EXT] Re: [PATCH] ethdev: report error on name truncation

[Nithin Kumar Dabilpuram]

Sure. Format change was unintentional and happened during internal merge, I’ll fix that and remove function name and send V2.

--
Thanks
Nithin

From: Andrew Rybchenko <arybchenko@solarflare.com>
Sent: Monday, January 7, 2019 8:17 PM
To: Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>; Thomas Monjalon <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@intel.com>; Andrew Rybchenko <arybchenko@solarflare.com>
Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>
Subject: [EXT] Re: [dpdk-dev] [PATCH] ethdev: report error on name truncation

External Email
________________________________
On 1/7/19 5:40 PM, Nithin Kumar Dabilpuram wrote:

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com><mailto:ndabilpuram@marvell.com>

---

 lib/librte_ethdev/rte_ethdev.c | 12 ++++++++++--

 1 file changed, 10 insertions(+), 2 deletions(-)



diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c

index 9d5107d..bd45445 100644

--- a/lib/librte_ethdev/rte_ethdev.c

+++ b/lib/librte_ethdev/rte_ethdev.c

@@ -3588,9 +3588,17 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,

 {

        char z_name[RTE_MEMZONE_NAMESIZE];

        const struct rte_memzone *mz;

+       int rc;



-       snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",

-                dev->data->port_id, queue_id, ring_name);

+       rc = snprintf(z_name, sizeof(z_name), "%s_%s_%d_%d",

+                     dev->device->driver->name, ring_name,

+                     dev->data->port_id, queue_id);

+

+       if (rc >= RTE_MEMZONE_NAMESIZE) {

+               RTE_LOG(ERR, EAL, "%s(): truncated name\n", __func__);

+               rte_errno = ENAMETOOLONG;

+               return NULL;

+       }



        mz = rte_memzone_lookup(z_name);

        if (mz)

It is good to report an error in the case of name truncation, but the patch
does more. It changes format of the memzone name, adds the driver name
in it (what is bad since testpmd has commands to find the memzone by name
and read descriptors (hack, but sometimes very useful)).
Also I'm not sure about function name in the log message. Other places
do not have it.

[dpdk-dev] [PATCH v2] ethdev: report error on name truncation

[Nithin Kumar Dabilpuram]

Currently this api doesn't report error if name is
truncated and so user is not sure about uniqueness
of name. This change reports error to help user.

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---

v2:
Fix issue caused by rebase and also fix log message

 lib/librte_ethdev/rte_ethdev.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 9d5107d..47d4f4a 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -3588,9 +3588,16 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
 {
 	char z_name[RTE_MEMZONE_NAMESIZE];
 	const struct rte_memzone *mz;
+	int rc;
 
-	snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
-		 dev->data->port_id, queue_id, ring_name);
+	rc = snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
+		      dev->data->port_id, queue_id, ring_name);
+
+	if (rc >= RTE_MEMZONE_NAMESIZE) {
+		RTE_ETHDEV_LOG(ERR, "truncated name");
+		rte_errno = ENAMETOOLONG;
+		return NULL;
+	}
 
 	mz = rte_memzone_lookup(z_name);
 	if (mz)
-- 
2.8.4

Re: [dpdk-dev] [PATCH v2] ethdev: report error on name truncation

[Wiles, Keith]

> On Jan 13, 2019, at 9:38 AM, Nithin Kumar Dabilpuram <ndabilpuram@marvell.com> wrote:
> 
> Currently this api doesn't report error if name is
> truncated and so user is not sure about uniqueness
> of name. This change reports error to help user.
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
> 
> v2:
> Fix issue caused by rebase and also fix log message
> 
> lib/librte_ethdev/rte_ethdev.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 9d5107d..47d4f4a 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -3588,9 +3588,16 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
> {
> 	char z_name[RTE_MEMZONE_NAMESIZE];
> 	const struct rte_memzone *mz;
> +	int rc;
> 
> -	snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
> -		 dev->data->port_id, queue_id, ring_name);
> +	rc = snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
> +		      dev->data->port_id, queue_id, ring_name);
> +
> +	if (rc >= RTE_MEMZONE_NAMESIZE) {
> +		RTE_ETHDEV_LOG(ERR, "truncated name");
> +		rte_errno = ENAMETOOLONG;
> +		return NULL;
> +	}

I we are already returning an error here should the RTE_LOG be DEBUG and not ERR. Of course this does mean we would have to check return codes :-)
> 
> 	mz = rte_memzone_lookup(z_name);
> 	if (mz)
> -- 
> 2.8.4
> 

Regards,
Keith

Re: [dpdk-dev] [PATCH v2] ethdev: report error on name truncation

[Thomas Monjalon]

13/01/2019 20:28, Wiles, Keith:
> > On Jan 13, 2019, at 9:38 AM, Nithin Kumar Dabilpuram <ndabilpuram@marvell.com> wrote:
> > --- a/lib/librte_ethdev/rte_ethdev.c
> > +++ b/lib/librte_ethdev/rte_ethdev.c
> > +	if (rc >= RTE_MEMZONE_NAMESIZE) {
> > +		RTE_ETHDEV_LOG(ERR, "truncated name");
> > +		rte_errno = ENAMETOOLONG;
> > +		return NULL;
> > +	}
> 
> I we are already returning an error here should the RTE_LOG be DEBUG
> and not ERR.
> Of course this does mean we would have to check return codes :-)

In the general case, we should always log the errors as RTE_LOG_ERR,
no matter it is handled and logged again at an upper level.
Don't you think so?

Re: [dpdk-dev] [PATCH v2] ethdev: report error on name truncation

[Wiles, Keith]

> On Jan 13, 2019, at 2:02 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 13/01/2019 20:28, Wiles, Keith:
>>> On Jan 13, 2019, at 9:38 AM, Nithin Kumar Dabilpuram <ndabilpuram@marvell.com> wrote:
>>> --- a/lib/librte_ethdev/rte_ethdev.c
>>> +++ b/lib/librte_ethdev/rte_ethdev.c
>>> +	if (rc >= RTE_MEMZONE_NAMESIZE) {
>>> +		RTE_ETHDEV_LOG(ERR, "truncated name");
>>> +		rte_errno = ENAMETOOLONG;
>>> +		return NULL;
>>> +	}
>> 
>> I we are already returning an error here should the RTE_LOG be DEBUG
>> and not ERR.
>> Of course this does mean we would have to check return codes :-)
> 
> In the general case, we should always log the errors as RTE_LOG_ERR,
> no matter it is handled and logged again at an upper level.
> Don't you think so?

My only concern is cluttering up the console output and developers should be checking return codes, which I know we do not do sometimes in DPDK.
I think we need to do some cleaning up of DPDK and test return codes or make the function return void, but that is a different problem then this one.

If we are fine with this type of log style then we can leave it. To me is just seem redundant if we are returning a code the calling function should report the error. In some cases we will get two or more messages about the same problem depending on the call path.
> 
> 

Regards,
Keith

Re: [dpdk-dev] [PATCH v2] ethdev: report error on name truncation

[Thomas Monjalon]

13/01/2019 21:19, Wiles, Keith:
> > On Jan 13, 2019, at 2:02 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> > 13/01/2019 20:28, Wiles, Keith:
> >>> On Jan 13, 2019, at 9:38 AM, Nithin Kumar Dabilpuram <ndabilpuram@marvell.com> wrote:
> >>> --- a/lib/librte_ethdev/rte_ethdev.c
> >>> +++ b/lib/librte_ethdev/rte_ethdev.c
> >>> +	if (rc >= RTE_MEMZONE_NAMESIZE) {
> >>> +		RTE_ETHDEV_LOG(ERR, "truncated name");
> >>> +		rte_errno = ENAMETOOLONG;
> >>> +		return NULL;
> >>> +	}
> >> 
> >> I we are already returning an error here should the RTE_LOG be DEBUG
> >> and not ERR.
> >> Of course this does mean we would have to check return codes :-)
> > 
> > In the general case, we should always log the errors as RTE_LOG_ERR,
> > no matter it is handled and logged again at an upper level.
> > Don't you think so?
> 
> My only concern is cluttering up the console output and developers should be checking return codes, which I know we do not do sometimes in DPDK.
> I think we need to do some cleaning up of DPDK and test return codes or make the function return void, but that is a different problem then this one.
> 
> If we are fine with this type of log style then we can leave it. To me is just seem redundant if we are returning a code the calling function should report the error. In some cases we will get two or more messages about the same problem depending on the call path.

The log can be different at different levels.
The deepest log will give details, while other logs will give context.

The more error logs we have, the better it may be for the user.
I understand the concern about keeping logs clean, but for errors,
I don't see possible redundancy as a spam.

Anyone else think differently?

Re: [dpdk-dev] [PATCH v2] ethdev: report error on name truncation

[Andrew Rybchenko]

On 1/14/19 12:21 AM, Thomas Monjalon wrote:
> 13/01/2019 21:19, Wiles, Keith:
>>> On Jan 13, 2019, at 2:02 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>> 13/01/2019 20:28, Wiles, Keith:
>>>>> On Jan 13, 2019, at 9:38 AM, Nithin Kumar Dabilpuram <ndabilpuram@marvell.com> wrote:
>>>>> --- a/lib/librte_ethdev/rte_ethdev.c
>>>>> +++ b/lib/librte_ethdev/rte_ethdev.c
>>>>> +	if (rc >= RTE_MEMZONE_NAMESIZE) {
>>>>> +		RTE_ETHDEV_LOG(ERR, "truncated name");
>>>>> +		rte_errno = ENAMETOOLONG;
>>>>> +		return NULL;
>>>>> +	}
>>>> I we are already returning an error here should the RTE_LOG be DEBUG
>>>> and not ERR.
>>>> Of course this does mean we would have to check return codes :-)
>>> In the general case, we should always log the errors as RTE_LOG_ERR,
>>> no matter it is handled and logged again at an upper level.
>>> Don't you think so?
>> My only concern is cluttering up the console output and developers should be checking return codes, which I know we do not do sometimes in DPDK.
>> I think we need to do some cleaning up of DPDK and test return codes or make the function return void, but that is a different problem then this one.
>>
>> If we are fine with this type of log style then we can leave it. To me is just seem redundant if we are returning a code the calling function should report the error. In some cases we will get two or more messages about the same problem depending on the call path.
> The log can be different at different levels.
> The deepest log will give details, while other logs will give context.
>
> The more error logs we have, the better it may be for the user.
> I understand the concern about keeping logs clean, but for errors,
> I don't see possible redundancy as a spam.
>
> Anyone else think differently?

Error logs are bad if an API assumes retries to be used to resolve it
(if buffer is insufficient, increase buffer and retry). But it is not 
the case
here. Theoretically it is possible here (provide shorter name), but very
unlikely. So, I'm OK with the added error log.

Re: [dpdk-dev] [PATCH v2] ethdev: report error on name truncation

[Thomas Monjalon]

13/01/2019 16:38, Nithin Kumar Dabilpuram:
> Currently this api doesn't report error if name is
> truncated and so user is not sure about uniqueness
> of name. This change reports error to help user.
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
> +	if (rc >= RTE_MEMZONE_NAMESIZE) {
> +		RTE_ETHDEV_LOG(ERR, "truncated name");

It would be better understandable from an user perspective by saying
"ring name too long".
And it would be even better with \n at the end ;)

> +		rte_errno = ENAMETOOLONG;
> +		return NULL;
> +	}

[dpdk-dev] [PATCH v3] ethdev: report error on name truncation

[Nithin Kumar Dabilpuram]

Currently this api doesn't report error if name is
truncated and so user is not sure about uniqueness
of name. This change reports error to help user.

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
v3:
Change log message
v2:
Fix issue caused by rebase and also fix log message

 lib/librte_ethdev/rte_ethdev.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 9d5107d..474f305 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -3588,9 +3588,16 @@ rte_eth_dma_zone_reserve(const struct rte_eth_dev *dev, const char *ring_name,
 {
 	char z_name[RTE_MEMZONE_NAMESIZE];
 	const struct rte_memzone *mz;
+	int rc;
 
-	snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
-		 dev->data->port_id, queue_id, ring_name);
+	rc = snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
+		      dev->data->port_id, queue_id, ring_name);
+
+	if (rc >= RTE_MEMZONE_NAMESIZE) {
+		RTE_ETHDEV_LOG(ERR, "ring name too long\n");
+		rte_errno = ENAMETOOLONG;
+		return NULL;
+	}
 
 	mz = rte_memzone_lookup(z_name);
 	if (mz)
-- 
2.8.4

Re: [dpdk-dev] [PATCH v3] ethdev: report error on name truncation

[Thomas Monjalon]

17/01/2019 15:13, Nithin Kumar Dabilpuram:
> Currently this api doesn't report error if name is
> truncated and so user is not sure about uniqueness
> of name. This change reports error to help user.
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
> +	rc = snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
> +		      dev->data->port_id, queue_id, ring_name);
> +
> +	if (rc >= RTE_MEMZONE_NAMESIZE) {
> +		RTE_ETHDEV_LOG(ERR, "ring name too long\n");
> +		rte_errno = ENAMETOOLONG;
> +		return NULL;
> +	}

Usually we don't insert a blank line before a test of a return value.
It's really a nitpick, so Ferruh, it's up to you to keep it or not when applying.

Acked-by: Thomas Monjalon <thomas@monjalon.net>

Re: [dpdk-dev] [PATCH v3] ethdev: report error on name truncation

[Ferruh Yigit]

On 1/17/2019 3:38 PM, Thomas Monjalon wrote:
> 17/01/2019 15:13, Nithin Kumar Dabilpuram:
>> Currently this api doesn't report error if name is
>> truncated and so user is not sure about uniqueness
>> of name. This change reports error to help user.
>>
>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
>> ---
>> +	rc = snprintf(z_name, sizeof(z_name), "eth_p%d_q%d_%s",
>> +		      dev->data->port_id, queue_id, ring_name);
>> +
>> +	if (rc >= RTE_MEMZONE_NAMESIZE) {
>> +		RTE_ETHDEV_LOG(ERR, "ring name too long\n");
>> +		rte_errno = ENAMETOOLONG;
>> +		return NULL;
>> +	}
> 
> Usually we don't insert a blank line before a test of a return value.
> It's really a nitpick, so Ferruh, it's up to you to keep it or not when applying.
> 
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> 

Applied to dpdk-next-net/master, thanks.

(Blank line removed while merging)