From: Ferruh Yigit <ferruh.yigit@amd.com>
To: "lihuisong (C)" <lihuisong@huawei.com>, dev@dpdk.org
Cc: thomas@monjalon.net, andrew.rybchenko@oktetlabs.ru,
liuyonglong@huawei.com, Gagandeep Singh <g.singh@nxp.com>,
Hemant Agrawal <hemant.agrawal@nxp.com>,
Simei Su <simei.su@intel.com>, Qi Zhang <qi.z.zhang@intel.com>,
Qiming Yang <qiming.yang@intel.com>,
Junfeng Guo <junfeng.guo@intel.com>
Subject: Re: [PATCH v3 0/2] ethdev: add the check for PTP capability
Date: Fri, 26 Jan 2024 16:54:30 +0000 [thread overview]
Message-ID: <5afa7ecf-254f-424b-9a58-1548e8ef270d@amd.com> (raw)
In-Reply-To: <7ecc6f3b-78f8-6421-307d-2c6c484c6109@huawei.com>
On 1/11/2024 6:25 AM, lihuisong (C) wrote:
> Hi Ferruh,
>
> 在 2023/11/23 19:56, lihuisong (C) 写道:
>>
>> 在 2023/11/2 7:39, Ferruh Yigit 写道:
>>> timesync_read_rx_timestamp
>>> On 9/21/2023 12:59 PM, lihuisong (C) wrote:
>>>> add ice & igc maintainers
>>>>
>>>> 在 2023/9/21 19:06, Ferruh Yigit 写道:
>>>>> On 9/21/2023 11:02 AM, lihuisong (C) wrote:
>>>>>> Hi Ferruh,
>>>>>>
>>>>>> Sorry for my delay reply because of taking a look at all PMDs
>>>>>> implementation.
>>>>>>
>>>>>>
>>>>>> 在 2023/9/16 1:46, Ferruh Yigit 写道:
>>>>>>> On 8/17/2023 9:42 AM, Huisong Li wrote:
>>>>>>>> From the first version of ptpclient, it seems that this example
>>>>>>>> assume that
>>>>>>>> the PMDs support the PTP feature and enable PTP by default.
>>>>>>>> Please see
>>>>>>>> commit ab129e9065a5 ("examples/ptpclient: add minimal PTP client")
>>>>>>>> which are introduced in 2015.
>>>>>>>>
>>>>>>>> And two years later, Rx HW timestamp offload was introduced to
>>>>>>>> enable or
>>>>>>>> disable PTP feature in HW via rte_eth_rxmode. Please see
>>>>>>>> commit 42ffc45aa340 ("ethdev: add Rx HW timestamp capability").
>>>>>>>>
>>>>>>> Hi Huisong,
>>>>>>>
>>>>>>> As far as I know this offload is not for PTP.
>>>>>>> PTP and TIMESTAMP are different.
>>>>>> If TIMESTAMP offload cannot stand for PTP, we may need to add one new
>>>>>> offlaod for PTP.
>>>>>>
>>>>> Can you please detail what is "PTP offload"?
>>>>>
>>>> It indicates whether the device supports PTP or enable PTP feature.
>>>>
>>> We have 'rte_eth_timesync_enable()' and 'rte_eth_timesync_disable()'
>>> APIs to control PTP support.
>> No, this is just to control it.
>> we still need to like a device capablity to report application if the
>> port support to call this API, right?
>>>
>>> But when mention from "offload", it is something device itself does.
>>>
>>> PTP is a protocol (IEEE 1588), and used to synchronize clocks.
>>> What I get is protocol can be parsed by networking stack and it can be
>>> used by application to synchronize clock.
>>>
>>> When you are refer to "PTP offload", does it mean device (NIC)
>>> understands the protocol and parse it to synchronize device clock with
>>> other devices?
>> Good point. PTP offload is unreasonable.
>> But the capablity is required indeed.
>> What do you think of introducing a RTE_ETH_DEV_PTP in
>> dev->data->dev_flags for PTP feature?
>
> Can you take a look at this discussion line again?
>
> Let's introduce a RTE_ETH_DEV_PTP in dev->data->dev_flags to reveal if
> the device support PTP feature.
>
Hi Huisong,
First let me try to summarize the discussion since it has been some time.
HW timer block can be used for Rx timestamp offload
(RTE_ETH_RX_OFFLOAD_TIMESTAMP) and/or PTP support, but they are two
different things.
This patch uses 'RTE_ETH_RX_OFFLOAD_TIMESTAMP' capability for PTP
support, which is wrong.
After we agreed on above, your next question is to use 'dev_flag' to
report PTP capability.
First, can you please describe what is the motivation to learn if HW
supports PTP or now, what is the benefit of knowing this.
If we agree that there is a need to know the PTP capability, question is
where to report this capability.
Suggested 'dev_flags' is used for various things, some are internal
flags and some are status, I don't think overloading this variable is
not good idea.
Other option is an update 'rte_eth_dev_info_get()' for it but it has the
same problem, this function is already overloaded with many different
things.
We can have an API just to get PTP capability, but this will require a
new dev_ops, this can be an option but my concern is having a capability
dev_ops for each feature create a mess in dev_ops.
Perhaps we can have single get_capability() API, and it gets features as
flags to return if that feature is supported or not, but this requires a
wider discussion.
Instead can we deduce the capability from PTP relevant dev_ops, if they
are implemented we can say it is supported? This doesn't require new
support.
next prev parent reply other threads:[~2024-01-26 16:54 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-28 13:39 [PATCH 0/3] some bugfixes for PTP Dongdong Liu
2022-06-28 13:39 ` [PATCH 1/3] examples/ptpclient: add the check for PTP capability Dongdong Liu
2022-06-28 13:39 ` [PATCH 2/3] net/hns3: fix fail to receive PTP packet Dongdong Liu
2022-06-28 13:39 ` [PATCH 3/3] ethdev: add the check for the valitity of timestamp offload Dongdong Liu
2022-07-02 8:17 ` [PATCH v2 0/3] some bugfixes for PTP Dongdong Liu
2022-07-02 8:17 ` [PATCH v2 1/3] examples/ptpclient: add the check for PTP capability Dongdong Liu
2022-07-02 8:17 ` [PATCH v2 2/3] net/hns3: fix fail to receive PTP packet Dongdong Liu
2022-07-02 8:17 ` [PATCH v2 3/3] ethdev: add the check for the valitity of timestamp offload Dongdong Liu
2022-07-06 14:57 ` Andrew Rybchenko
2022-07-07 2:05 ` lihuisong (C)
2023-08-17 8:42 ` [PATCH v3 0/2] ethdev: add the check for PTP capability Huisong Li
2023-08-17 8:42 ` [PATCH v3 1/2] examples/ptpclient: " Huisong Li
2023-09-15 17:29 ` Ferruh Yigit
2023-09-21 9:18 ` lihuisong (C)
2023-09-21 11:02 ` Ferruh Yigit
2023-09-21 11:22 ` Hemant Agrawal
2023-10-20 4:05 ` lihuisong (C)
2023-09-21 11:36 ` lihuisong (C)
2023-08-17 8:42 ` [PATCH v3 2/2] ethdev: add the check for the valitity of timestamp offload Huisong Li
2023-09-15 17:46 ` [PATCH v3 0/2] ethdev: add the check for PTP capability Ferruh Yigit
2023-09-21 10:02 ` lihuisong (C)
2023-09-21 11:06 ` Ferruh Yigit
2023-09-21 11:17 ` Hemant Agrawal
2023-10-20 3:58 ` lihuisong (C)
2023-11-01 23:39 ` Ferruh Yigit
2023-11-23 11:40 ` lihuisong (C)
2023-11-01 23:39 ` Ferruh Yigit
2023-09-21 11:59 ` lihuisong (C)
2023-11-01 23:39 ` Ferruh Yigit
2023-11-23 11:56 ` lihuisong (C)
2024-01-11 6:25 ` lihuisong (C)
2024-01-26 16:54 ` Ferruh Yigit [this message]
2024-01-27 1:52 ` lihuisong (C)
2024-01-29 11:16 ` Ferruh Yigit
2024-01-29 13:58 ` lihuisong (C)
2024-01-29 15:00 ` Ferruh Yigit
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=5afa7ecf-254f-424b-9a58-1548e8ef270d@amd.com \
--to=ferruh.yigit@amd.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=dev@dpdk.org \
--cc=g.singh@nxp.com \
--cc=hemant.agrawal@nxp.com \
--cc=junfeng.guo@intel.com \
--cc=lihuisong@huawei.com \
--cc=liuyonglong@huawei.com \
--cc=qi.z.zhang@intel.com \
--cc=qiming.yang@intel.com \
--cc=simei.su@intel.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).