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

[Andrew Rybchenko <arybchenko@solarflare.com>]

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.