From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 08F40433A8; Thu, 23 Nov 2023 12:40:20 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CE02840A73; Thu, 23 Nov 2023 12:40:19 +0100 (CET) Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by mails.dpdk.org (Postfix) with ESMTP id 10439402BD for ; Thu, 23 Nov 2023 12:40:17 +0100 (CET) Received: from kwepemm000004.china.huawei.com (unknown [172.30.72.57]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4SbbdZ0ZKqzMnKQ; Thu, 23 Nov 2023 19:35:30 +0800 (CST) Received: from [10.67.121.59] (10.67.121.59) by kwepemm000004.china.huawei.com (7.193.23.18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Thu, 23 Nov 2023 19:40:14 +0800 Message-ID: <4ca421eb-3a0f-d629-801f-1b99a02a1387@huawei.com> Date: Thu, 23 Nov 2023 19:40:13 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 Subject: Re: [PATCH v3 0/2] ethdev: add the check for PTP capability To: Ferruh Yigit , Hemant Agrawal , "dev@dpdk.org" , Gagandeep Singh CC: "thomas@monjalon.net" , "andrew.rybchenko@oktetlabs.ru" , "liuyonglong@huawei.com" References: <20220628133959.21381-1-liudongdong3@huawei.com> <20230817084226.55327-1-lihuisong@huawei.com> <1834a6a9-ef92-4a67-a987-490151cf5380@amd.com> <242e8583-969e-d8ca-2dd4-80e8cf73a662@huawei.com> <0d7f429c-8862-4f16-b7e5-46d69581f54f@amd.com> <15eacaf1-b2a3-3542-395e-4a6d8b6f5426@huawei.com> From: "lihuisong (C)" In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.59] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To kwepemm000004.china.huawei.com (7.193.23.18) X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 在 2023/11/2 7:39, Ferruh Yigit 写道: > On 10/20/2023 4:58 AM, lihuisong (C) wrote: >> 在 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. >> > There is already a patch by Thomas to remove RTE_LIBRTE_IEEE1588 [1], > agree that this functionality needs some attention. > > Removing RTE_LIBRTE_IEEE1588 impact drivers, that is what holding us back. +1 remove the compile macro RTE_LIBRTE_IEEE1588. And hns3 had beed removed it. > > > [1] > https://patchwork.dpdk.org/project/dpdk/patch/20230203132810.14187-1-thomas@monjalon.net/ > >> If there is a capability, it will be perfect, no matter whether it is >> TIMESTAMP offload. >> What do you think, Ferruh? >> > Difficulty is to know when to enable HW timestamp, and for some drivers > this may change the descriptor format (to include timestamp), so driver > should set correct datapath functions for this case. Yes, to get Rx timestamp of PTP packet from descriptor for many NIC. > > We know when a HW timer is required, it is required for PTP protocol and > required for TIMESTAMP offload. TIMESTAMP offload may be unnecessary for some NIC which don't get Rx timestamp from descriptor(But, IMO, like this hardware is very rare.). > > What do you think to dynamically enable it for PTP when > 'rte_eth_timesync_enable()' API called, and for TIMESTAMP offload when > the offload is enabled. Agree above. At least, this can make sure all NIC can enable PTP feature. > If this works, now new configuration item or offload is required, what > do you think? The new capability item is required to know if the port support PTP feature. so application can enable/disable PTP based on this capability. > >>>>> 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(-) >>>>>>> >>>>>> . > .