All of lore.kernel.org
 help / color / mirror / Atom feed
From: huangdengdui <huangdengdui@huawei.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: Ferruh Yigit <ferruh.yigit@amd.com>, <dev@dpdk.org>,
	<aman.deep.singh@intel.com>, <liuyonglong@huawei.com>,
	David Marchand <david.marchand@redhat.com>,
	Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH] app/testpmd: handle IEEE1588 init fail
Date: Wed, 17 Apr 2024 17:26:44 +0800	[thread overview]
Message-ID: <b06b205c-e342-4bff-8d48-ccb8f4fa0758@huawei.com> (raw)
In-Reply-To: <20240408195023.5be4fed5@hermes.local>



On 2024/4/9 10:50, Stephen Hemminger wrote:
> On Tue, 9 Apr 2024 10:06:01 +0800
> huangdengdui <huangdengdui@huawei.com> wrote:
> 
>> On 2024/4/8 16:45, Ferruh Yigit wrote:
>>> On 4/8/2024 6:52 AM, huangdengdui wrote:  
>>>>
>>>>
>>>> On 2024/4/6 0:44, Stephen Hemminger wrote:  
>>>>> On Sat, 30 Mar 2024 15:44:09 +0800
>>>>> Dengdui Huang <huangdengdui@huawei.com> wrote:
>>>>>  
>>>>>> When the port's timestamping function failed to initialize
>>>>>> (for example, the device does not support PTP), the packets
>>>>>> received by the hardware do not contain the timestamp.
>>>>>> In this case, IEEE1588 packet forwarding should not start.
>>>>>> This patch fix it.
>>>>>>
>>>>>> Plus, adding a failure message when failed to disable PTP.
>>>>>>
>>>>>> Fixes: a78040c990cb ("app/testpmd: update forward engine beginning")
>>>>>> Cc: stable@dpdk.org
>>>>>>
>>>>>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>  
>>>>>
>>>>> Noticed that ieee1588 part is printing errors to stdout,
>>>>> but other parts of test-pmd are using stderr or TEST_PMD_LOG.
>>>>>
>>>>> It would be good to decide on one good way to handle this
>>>>> across all of testpmd.  
>>>>
>>>> Yeah, it's a bit of a mess. Is it better to use TEST_PMD_LOG?
>>>> But this is a test app, and modifying it seems unnecessary.
>>>> What should we do next?
>>>>  
>>>
>>> 'TESTPMD_LOG' exists and used in a few places, but still most of the
>>> logging done with 'printf/fprintf'.
>>>
>>> Agree to have an agreement what to use, document it, and stick to it.
>>>
>>>
>>> For some cases, like 'usage()' output (testpmd supported parameters), or
>>> cmdline prompt we always want to have output, so 'printf' suits well.
>>>
>>> Not sure where 'TESTPMD_LOG' is needed and what is the benefit. I don't
>>> remember many cases that I want to refine testpmd app level output.
>>> Maybe a case can be packet verbose output, but it also has its specific
>>> command to control it.
>>>
>>> So should we continue to 'printf/fprintf', is there any disadvantage to
>>> do so?  
>> OK, 'printf/fprintf' is really necessary. Am I to understand it as follows?
>>
>> 'TESTPMD_LOG' is more suitable for printing app runtime context logs,
>> such as initialization logs and uninstallation logs.
>>
>> 'printf' is suitable for normal interaction with the user, such as
>> show port info <port_id>
>>
>> When should we print to stderr?
> 
> Any Unix convention is that any error message should go to stderr.
> For test-pmd, using TESTPMD_LOG has benefits and problems.
> The benefit is following a convention across all the startup and error
> codes. And with the color log patches, errors are highlighted.
> 
> But the current way rte_log works, the testpmd stuff ends up going
> to syslog/journal which is not necessary and overly chatty.

That seems hard to unify. I don't have a better idea at the moment.
Do you have a better suggestion?

      reply	other threads:[~2024-04-17  9:26 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-30  7:44 [PATCH] app/testpmd: handle IEEE1588 init fail Dengdui Huang
2024-04-05 15:36 ` Singh, Aman Deep
2024-07-05 11:27   ` Ferruh Yigit
2024-04-05 16:44 ` Stephen Hemminger
2024-04-08  5:52   ` huangdengdui
2024-04-08  8:45     ` Ferruh Yigit
2024-04-09  2:06       ` huangdengdui
2024-04-09  2:50         ` Stephen Hemminger
2024-04-17  9:26           ` huangdengdui [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b06b205c-e342-4bff-8d48-ccb8f4fa0758@huawei.com \
    --to=huangdengdui@huawei.com \
    --cc=aman.deep.singh@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@amd.com \
    --cc=liuyonglong@huawei.com \
    --cc=stephen@networkplumber.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.