Re: [RFC] ethdev: introduce PTP device capability

["lihuisong (C)" <lihuisong@huawei.com>]

Hi Ferruh,


在 2024/9/23 6:23, Ferruh Yigit 写道:
> On 1/30/2024 3:58 AM, Huisong Li wrote:
>> Currently, the PTP feature is a little messy and has some issue.
>> 1> There is different implementation for PTP. This feature of some
>>     hardware depends on the Rx HW timestamp offload, and use this
>>     offload to detect if enable PTP feature. Others can enable PTP
>>     feature with only ethdev ops.
>> 2> For some drivers, enabling PTP feature also depends on the macro
>>     RTE_LIBRTE_IEEE1588. Actually, whether device support, enable
>>     or disable this feature should not be determined at compilation
>>     time. This has been discussed in thread [1].
>> 3> The user haven't a good way to distinguish which port supports
>>     the PTP feature in the case that a couple of device belong to the
>>     same driver. And PTP related API in ethdev layer doesn't do check
>>     for PTP capability. This has been mentioned and discussed in
>>     thread [2].
>>
>> In the thread [1], there is a conclusion that remove the compiling
>> macro RTE_LIBRTE_IEEE1588. And in the thread [2], there is a common
>> opinion that the RTE_ETH_RX_OFFLOAD_TIMESTAMP cannot be used as the
>> PTP capability.
>>
>> For the above issuse, this patch introduces a PTP capability in
>> rte_eth_dev_info.dev_capa to report PTP capability.
>>
>> Welcome to jumping into discussion.
>>
>> [1] https://patchwork.dpdk.org/project/dpdk/patch/20230203132810.14187-1-thomas@monjalon.net/
>> [2] https://inbox.dpdk.org/dev/20230817084226.55327-1-lihuisong@huawei.com/
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
>>   lib/ethdev/rte_ethdev.h | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>> index efa4f12b2a..4c8bd691bd 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -1613,6 +1613,12 @@ struct rte_eth_conf {
>>   #define RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP         RTE_BIT64(3)
>>   /** Device supports keeping shared flow objects across restart. */
>>   #define RTE_ETH_DEV_CAPA_FLOW_SHARED_OBJECT_KEEP RTE_BIT64(4)
>> +/**
>> + * Device supports PTP feature.
>> + * For some hardware, this feature also need to set the offload
>> + * RTE_ETH_RX_OFFLOAD_TIMESTAMP, please check rte_eth_dev_info.rx_offload_capa.
>> + */
>> +#define RTE_ETH_DEV_CAPA_PTP                     RTE_BIT64(5)
>>   /**@}*/
>>   
>>   /*
> Hi Huisong,
>
> Thanks for the effort, as you mentioned PTP feature can be improved and
> there is a target to remove RTE_LIBRTE_IEEE1588 build time macro.
>
> As far as I remember, one of the main reasons of the RTE_LIBRTE_IEEE1588
> macro is some HW has resource restrictions, when RTE_LIBRTE_IEEE1588 is
> enabled some other features, like vector datapath, are not usable, that
> is why this is a build time selection.
I think the main reason that driver don't support PTP feature in vector 
datapath is for performance.
This can be controled by releated dev_ops API or TIMESTAMP offload and 
no need to use RTE_LIBRTE_IEEE1588 macro, like hns3.
>
> And related to the PTP capability, can you please give some more
> information, what does device having PTP capability exactly means.
Just the device having PTP capability can be used to enable PTP feature.
>
> PTP is protocol and it is implemented in userspace daemon. I guess even
> NIC does not support timestamp offloading, by using time information
> from SW it can still implement PTP, right?

AFAIS, PTP feature implement requires the collaboration of HW and SW, as 
describted by the releated dev_ops API.

> Is PTP offload means, offloading some part of the protocol communication
> withing the device?
Normally, a feature offload means this feature is done in hardware and 
the software doesn't need to something for this.
I reviewed our discussion in [1], we all think it's unreasonable to name 
it "PTP offload".

>
> Btw, the relevant RTE_ETH_RX_OFFLOAD_TIMESTAMP offload is, a little more
> generic HW capability that HW can add timestamp to Rx/Tx packets, which
> can be used for custom diagnostics. HW supporting this offload means
> that HW has some specific clock HW in it.
Yes.
>
> Both having RTE_ETH_RX_OFFLOAD_TIMESTAMP and RTE_ETH_DEV_CAPA_PTP
> capability can be confusing, lets clarify it more.

Let me try to clearify them:
1) RTE_ETH_DEV_CAPA_PTP just means the ethdev support PTP capability because of application no way to know if the device support PTP feature.
2) As you said above, setting RTE_ETH_RX_OFFLOAD_TIMESTAMP offload is not necessary for PTP feature, but also may be for custom diagnostics.
    Some NICs enable PTP feature using only the rte_eth_timesync_xxx API,
    and many NICs also need this offload to fill timestamp in mbuf to cooperate with the implementation of PTP feature.

>
[1] https://inbox.dpdk.org/dev/20230817084226.55327-1-lihuisong@huawei.com/
>
>
> .