From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id 32AFA1B4B6 for ; Thu, 2 Aug 2018 15:52:14 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id A9590B40060; Thu, 2 Aug 2018 13:52:12 +0000 (UTC) Received: from [192.168.1.16] (85.187.13.33) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Thu, 2 Aug 2018 14:52:07 +0100 To: Kevin Traynor , References: <1533213223-24028-1-git-send-email-ktraynor@redhat.com> <661cb701-42a8-1416-1e78-8fa316f0300a@solarflare.com> From: Andrew Rybchenko Message-ID: <7f50a75f-d585-d8fd-5b31-783a5c4a37bc@solarflare.com> Date: Thu, 2 Aug 2018 16:52:02 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Originating-IP: [85.187.13.33] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24006.003 X-TM-AS-Result: No-11.467700-8.000000-10 X-TMASE-MatchedRID: HXSqh3WYKfsOwH4pD14DsPHkpkyUphL9IaLR+2xKRDKjD9XxfoENjnkD xC8oBMkzMJPHfuZG0eOri0qvV9KSBCxpw1FoxdYV4pdq9sdj8LXyoRTj7qeEaNVTZaI6TuNovph VFiQ+RgJL1JirBB3uqGZUyDrAiJrGOsXhsCoCbSb0hv/rD7WVZETrIRzbtv0Gnp9KgXcu34z13O PkzR0M6TEEIfCM+GrRTJgPbXJoOuv2ckw6NiGelEKcYi5Qw/RVfS0Ip2eEHny+qryzYw2E8LLn+ 0Vm71Lcq7rFUcuGp/EgBwKKRHe+r/I+7rIAcKFHVv/B8y1+yLYvB/DFhdmbXmLxkXk1u28w3R08 mnQA2L8= X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--11.467700-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24006.003 X-MDID: 1533217933-X43OmZHuOxRA Subject: Re: [dpdk-dev] [PATCH] ethdev: decrease log level for successful API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Aug 2018 13:52:14 -0000 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: >>> Change log level of messages from ERR back to DEBUG where >>> existing API indicates that the condition is a success. >>> >>> This means applications using the API in its current form will >>> not get new ERR logs. >>> >>> Fixes: bea1e0c70cfc ("ethdev: convert static log type usage to dynamic") >>> >>> Signed-off-by: Kevin Traynor >>> --- >>> lib/librte_ethdev/rte_ethdev.c | 12 ++++++------ >>> 1 file changed, 6 insertions(+), 6 deletions(-) >>> >>> diff --git a/lib/librte_ethdev/rte_ethdev.c >>> b/lib/librte_ethdev/rte_ethdev.c >>> index c7ab157..16656d2 100644 >>> --- a/lib/librte_ethdev/rte_ethdev.c >>> +++ b/lib/librte_ethdev/rte_ethdev.c >>> @@ -797,5 +797,5 @@ struct rte_eth_dev * >>> if (dev->data->rx_queue_state[rx_queue_id] != >>> RTE_ETH_QUEUE_STATE_STOPPED) { >>> - RTE_ETHDEV_LOG(ERR, >>> + RTE_ETHDEV_LOG(DEBUG, >>> "Queue %"PRIu16" of device with port_id=%"PRIu16" >>> already started\n", >>> rx_queue_id, port_id); >>> @@ -824,5 +824,5 @@ struct rte_eth_dev * >>> if (dev->data->rx_queue_state[rx_queue_id] == >>> RTE_ETH_QUEUE_STATE_STOPPED) { >>> - RTE_ETHDEV_LOG(ERR, >>> + RTE_ETHDEV_LOG(DEBUG, >>> "Queue %"PRIu16" of device with port_id=%"PRIu16" >>> already stopped\n", >>> rx_queue_id, port_id); >>> @@ -857,5 +857,5 @@ struct rte_eth_dev * >>> if (dev->data->tx_queue_state[tx_queue_id] != >>> RTE_ETH_QUEUE_STATE_STOPPED) { >>> - RTE_ETHDEV_LOG(ERR, >>> + RTE_ETHDEV_LOG(DEBUG, >>> "Queue %"PRIu16" of device with port_id=%"PRIu16" >>> already started\n", >>> tx_queue_id, port_id); >>> @@ -882,5 +882,5 @@ struct rte_eth_dev * >>> if (dev->data->tx_queue_state[tx_queue_id] == >>> RTE_ETH_QUEUE_STATE_STOPPED) { >>> - RTE_ETHDEV_LOG(ERR, >>> + RTE_ETHDEV_LOG(DEBUG, >>> "Queue %"PRIu16" of device with port_id=%"PRIu16" >>> already stopped\n", >>> tx_queue_id, port_id); >>> @@ -1287,5 +1287,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 started\n", >>> port_id); >>> @@ -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. > An application may not maintain some state and always call a stop before > a start etc. in line with the API. > > I don't think an API success condition should be changed from a debug > message to anything else. Otherwise, it is trying to flag a warning > about the application which is following the API! It and will just spook > people when they see a new warning.