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 2D7AC431B5; Fri, 20 Oct 2023 05:58:21 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id DF52A402B8; Fri, 20 Oct 2023 05:58:20 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id 64CF2402B7 for ; Fri, 20 Oct 2023 05:58:19 +0200 (CEST) Received: from kwepemm000004.china.huawei.com (unknown [172.30.72.53]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4SBW085Cs5zvQ56; Fri, 20 Oct 2023 11:53:28 +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.31; Fri, 20 Oct 2023 11:58:14 +0800 Message-ID: <15eacaf1-b2a3-3542-395e-4a6d8b6f5426@huawei.com> Date: Fri, 20 Oct 2023 11:58:14 +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: Hemant Agrawal , Ferruh Yigit , "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> 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: dggems701-chm.china.huawei.com (10.3.19.178) 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/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(-) >>>>> >>>> .