From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5A6C7C47422 for ; Mon, 29 Jan 2024 13:58:20 +0000 (UTC) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2A508402A8; Mon, 29 Jan 2024 14:58:19 +0100 (CET) Received: from szxga07-in.huawei.com (szxga07-in.huawei.com [45.249.212.35]) by mails.dpdk.org (Postfix) with ESMTP id 71180400D5 for ; Mon, 29 Jan 2024 14:58:17 +0100 (CET) Received: from mail.maildlp.com (unknown [172.19.88.214]) by szxga07-in.huawei.com (SkyGuard) with ESMTP id 4TNqbG0M80z1Q89D; Mon, 29 Jan 2024 21:56:26 +0800 (CST) Received: from kwepemm600004.china.huawei.com (unknown [7.193.23.242]) by mail.maildlp.com (Postfix) with ESMTPS id 2D5581A016B; Mon, 29 Jan 2024 21:58:15 +0800 (CST) Received: from [10.67.121.59] (10.67.121.59) by kwepemm600004.china.huawei.com (7.193.23.242) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Mon, 29 Jan 2024 21:58:14 +0800 Message-ID: <993a921d-3191-1b9a-847e-83acde073a9d@huawei.com> Date: Mon, 29 Jan 2024 21:58: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 , CC: , , , Gagandeep Singh , Hemant Agrawal , Simei Su , Qi Zhang , Qiming Yang , Junfeng Guo 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> <3a11b30d-346f-446f-903a-5a56cbae3853@amd.com> <665b0b6e-1ad9-a692-39cb-9e45e6b78b08@huawei.com> <7ecc6f3b-78f8-6421-307d-2c6c484c6109@huawei.com> <5afa7ecf-254f-424b-9a58-1548e8ef270d@amd.com> <50f72945-4f45-4f0f-9c52-a62522e27c26@amd.com> From: "lihuisong (C)" In-Reply-To: <50f72945-4f45-4f0f-9c52-a62522e27c26@amd.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.59] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To kwepemm600004.china.huawei.com (7.193.23.242) 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 在 2024/1/29 19:16, Ferruh Yigit 写道: > On 1/27/2024 1:52 AM, lihuisong (C) wrote: >> 在 2024/1/27 0:54, Ferruh Yigit 写道: >>> 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 Ferruh, >> >> Thanks for taking your time to reply. >> >>> 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. >>> >> correct. >>> 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. >> There are a couple of device which have the same driver on a platform or >> OS. >> But just allow one device to support or be responsible for PTP feature. >> The firmware will report a capability to tell the device if it is >> support PTP. >> But, currently, driver doesn't know how to report user which device >> support PTP feature. >> > Driver can hold a private data that records if PTP supported by the > device or not, and according this value PTP dev_ops can return error or > success. > > This may not be ideal but it lets user to know about the support status, > can this work? I don't think it is user friendly. Users know which port supports the PTP feature only when the API fails to be invoked, right? In addition, this is a common issue for all supported PTP device. So It is better to do this check in PMD. > > >> In addition, many drivers use RTE_LIBRTE_IEEE1588 to control PTP code flow. >> Whether the device supports PTP is irrelevant to this macro. >> > Yes, I guess because both features use same HW, there is confusion there. > >>> 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. >> Yes, we need to consider  carefully. >>> 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. >> Thank you mentioning so many ways. >> For the end of advice, I don't think it is appropriate. >> Because we have to modify dynamically the pointer address of all PTP >> APIs in dev_ops on the above case. >> > I was thinking for the case application distinguish if PTP related > dev_ops set or not, but after your explanation I can see this won't help > for your case. > Because in your case PTP dev_ops implemented but some devices support it > and some don't, and you are looking for a way to distinguish it. Yes > >> How about we use rte_eth_dev_info.dev_capa to report PTP offload? >> This is specifically used to report "Non-offload capabilities" according >> to its document. >> > As mentioned above 'rte_eth_dev_info' is overloaded, I am for being more > cautious to expand it more. > Also I think it is a wider discussion if we want a capability reporting > in ethdev and where it should be. How about we send a RFC patch which use rte_eth_dev_info.dev_capa to report PTP offload and start to disscuss this issue? And add Thomas's patch [1] and this patch. [1]https://patchwork.dpdk.org/project/dpdk/patch/20230203132810.14187-1-thomas@monjalon.net/ > > .