[PATCH v3 0/2] ethdev: add the check for PTP capability

[lihuisong (C)]

在 2023/9/21 19:17, Hemant Agrawal 写道:
> HI Ferruh,
>
>> 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"?
>>
>>>> PTP is a protocol for time sync.
>>>> Rx TIMESTAMP offload is to ask HW to add timestamp to mbuf.
>>> Yes.
>>> But a lot of PMDs actually depand on HW to report Rx timestamp
>>> releated information because of reading Rx timestamp of PTP SYNC
>>> packet in read_rx_timestamp API.
>>>
>> HW support may be required for PTP but this doesn't mean timestamp
>> offload is used.
>>>>> And then about four years later, ptpclient enable Rx timestamp
>>>>> offload because some PMDs require this offload to enable. Please see
>>>>> commit 7a04a4f67dca ("examples/ptpclient: enable Rx timestamp
>> offload").
>>>> dpaa2 seems using TIMESTAMP offload and PTP together, hence they
>>>> updated ptpclient sample to set TIMESTAMP offload.
> [Hemant] In case of dpaa2, we need to enable HW timestamp for PTP. In the current dpaa2 driver
> If the code is compiled with, RTE_LIBRTE_IEEE1588, we are enabling the HW timestamp
> Otherwise, we are only enabling it when the TIMESTAMP offload is selected.
>
> We added patch in ptpclient earlier to pass the timestamp offload, however later we also updated the driver to do it by default.
>
It is a little mess for PTP and RTE_LIBRTE_IEEE1588 to use.
Actually, whether PTP code is compiled should not depended on this macro 
RTE_LIBRTE_IEEE1588.
If there is a capability, it will be perfect, no matter whether it is 
TIMESTAMP offload.
What do you think, Ferruh?
>>> There are many PMDs doing like this, such as ice, igc, cnxk, dpaa2,
>>> hns3 and so on.
>>>
>> Can you please point the ice & igc code, cc'ing their maintainers, we can look
>> together?
>>
>>
>>>> We need to clarify dpaa2 usage.
>>>>
>>>>> By all the records, this is more like a process of perfecting PTP
>>>>> feature.
>>>>> Not all network adaptors support PTP feature. So adding the check
>>>>> for PTP capability in ethdev layer is necessary.
>>>>>
>>>> Nope, as PTP (IEEE1588/802.1AS) implemented as dev_ops, and ops
>>>> already checked, so no additional check is needed.
>>> But only having dev_ops about PTP doesn't satisfy the use of this feature.
>>> For example,
>>> there are serveal network ports belonged to a driver on one OS, and
>>> only one port support PTP function.
>>> So driver needs one *PTP* offload.
>>>> We just need to clarify TIMESTAMP offload and PTP usage and find out
>>>> what is causing confusion.
>>> Yes it is a little bit confusion.
>>> There are two kinds of implementation:
>>> A： ixgbe and txgbe (it seems that their HW is similar) don't need
>>> TIMESTAMP offload，and only use dev_ops to finish PTP feature.
>>> B:  saving "Rx timestamp related information" from Rx description when
>>> receive PTP SYNC packet and
>>>      report it in read_rx_timestamp API.
>>> For case B, most of driver use TIMESTAMP offload to decide if driver
>>> save "Rx timestamp related information.
>>> What do you think about this, Ferruh?
>>>> I would be great if you can help on clarification, and update
>>>> documentation or API comments, or what ever required, for this.
>>> ok
>>>>> ---
>>>>> v3:
>>>>>    - patch [2/3] for hns3 has been applied and so remove it.
>>>>>    - ops pointer check is closer to usage.
>>>>>
>>>>> Huisong Li (2):
>>>>>     examples/ptpclient: add the check for PTP capability
>>>>>     ethdev: add the check for the valitity of timestamp offload
>>>>>
>>>>>    examples/ptpclient/ptpclient.c |  5 +++
>>>>>    lib/ethdev/rte_ethdev.c        | 57
>>>>> +++++++++++++++++++++++++++++++++-
>>>>>    2 files changed, 61 insertions(+), 1 deletion(-)
>>>>>
>>>> .