Re: [dpdk-dev] [PATCH] ethdev: decrease log level for successful API

[Andrew Rybchenko <arybchenko@solarflare.com>]

On 02.08.2018 19:14, Thomas Monjalon wrote:
> 02/08/2018 16:52, Kevin Traynor:
>> On 08/02/2018 03:41 PM, Andrew Rybchenko wrote:
>>> On 02.08.2018 17:09, Kevin Traynor wrote:
>>>> On 08/02/2018 02:52 PM, Andrew Rybchenko wrote:
>>>>> On 02.08.2018 16:35, Kevin Traynor wrote:
>>>>>> On 08/02/2018 01:59 PM, Andrew Rybchenko wrote:
>>>>>>> On 02.08.2018 15:33, Kevin Traynor wrote:
>>>>>>>> @@ -1319,5 +1319,5 @@ struct rte_eth_dev *
>>>>>>>>            if (dev->data->dev_started == 0) {
>>>>>>>> -        RTE_ETHDEV_LOG(ERR,
>>>>>>>> +        RTE_ETHDEV_LOG(DEBUG,
>>>>>>>>                  "Device with port_id=%"PRIu16" already stopped\n",
>>>>>>>>                  port_id);
>>>>>>> I would suggest to use WARNING here. Yes, it is not an error since
>>>>>>> nothing bad has
>>>>>>> happened and we handle duplicate stop and duplicate start,
>>>>>>> but I think it is bad that (buggy?) application does it. Making it
>>>>>>> debug
>>>>>>> we simply
>>>>>>> hide it too much.
>>>>>>>
>>>>>> I think an application following the documented API is not bad or
>>>>>> buggy.
>>>>> I've failed to find the place were it is documented that device/queue
>>>>> may be stopped/started twice. Could you point out?
>>>>> Yes, return value 0 means success, but it is a separate thing.
>>>>>
>>>> I was commenting directly on the API and it's documentation e.g. below
>>>> for dev start. I'm not aware of other documentation specifying how it
>>>> can/cannot be called.
>>> I would not say so. "0: Success. Ethernet device started" means that
>>> function managed to do the job and finally the device is started.
>>>
>>> Never-mind it is not that important and already paid to much attention.
>>> I've included in CC other ethdev maintainers (who should be there from
>>> the very beginning). I don't mind if it is acked by other ethdev maintainer
>>> and applied. It is definitely not an error as it is now. Thanks.
>>>
>> Sure, sounds good - I'm glad we agree on something :-) I'll leave it for
>> a day and can re-spin tomorrow.
> I would like to give an opinion, but unfortunately it's hard to decide.
> For sure, it should not be a WARNING level (used for non-fatal errors).
> What else do we have? NOTICE, INFO and DEBUG.
> I think it can be INFO.

So, we have ERR (now), WARNING (my initial suggestion), NOTICE
(by Stephen), INFO (by Thomas) and DEBUG (by Kevin)  :)

If it is not treated as an error in application behaviour, I agree that my
suggestion of WARNING is not suitable. Typically NOTICE is the default
log level and if we consider such behaviour of apps correct, it should
be INFO to be silent by default. I really don't like DEBUG since these
messages are still important and could be simply lost in DEBUG
which could to be very-very verbose.