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 0EF11CD4933 for ; Thu, 21 Sep 2023 11:59:19 +0000 (UTC) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2E559402E2; Thu, 21 Sep 2023 13:59:18 +0200 (CEST) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id E016D402D2 for ; Thu, 21 Sep 2023 13:59:16 +0200 (CEST) Received: from kwepemm000004.china.huawei.com (unknown [172.30.72.57]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4Rrv3g0zmvzNmdb; Thu, 21 Sep 2023 19:55:27 +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; Thu, 21 Sep 2023 19:59:13 +0800 Message-ID: Date: Thu, 21 Sep 2023 19:59:12 +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 , , Gagandeep Singh , Hemant Agrawal , Qiming Yang , Qi Zhang , Simei Su , Junfeng Guo CC: , , 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: <0d7f429c-8862-4f16-b7e5-46d69581f54f@amd.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.59] X-ClientProxiedBy: dggems703-chm.china.huawei.com (10.3.19.180) 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 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. If TIMESTAMP offload is not for PTP, I don't know what the point of this offload independent existence is. > >>> 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. understand. > >>>> 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. >> 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? *-->igc code:* Having following codes in igc_recv_scattered_pkts():         if (rxq->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) {             uint32_t *ts = rte_pktmbuf_mtod_offset(first_seg,                     uint32_t *, -IGC_TS_HDR_LEN);             rxq->rx_timestamp = (uint64_t)ts[3] * NSEC_PER_SEC +                     ts[2];             rxm->timesync = rxq->queue_id;         } Note：this rxm->timesync will be used in timesync_read_rx_timestamp() *-->ice code:* #ifndef RTE_LIBRTE_ICE_16BYTE_RX_DESC         if (ice_timestamp_dynflag > 0 &&             (rxq->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP)) {             rxq->time_high =                rte_le_to_cpu_32(rxd.wb.flex_ts.ts_high);             if (unlikely(is_tsinit)) {                 ts_ns = ice_tstamp_convert_32b_64b(hw, ad, 1, rxq->time_high);                 rxq->hw_time_low = (uint32_t)ts_ns;                 rxq->hw_time_high = (uint32_t)(ts_ns >> 32);                 is_tsinit = false;             } else {                 if (rxq->time_high < rxq->hw_time_low)                     rxq->hw_time_high += 1;                 ts_ns = (uint64_t)rxq->hw_time_high << 32 | rxq->time_high;                 rxq->hw_time_low = rxq->time_high;             }             rxq->hw_time_update = rte_get_timer_cycles() /                          (rte_get_timer_hz() / 1000);             *RTE_MBUF_DYNFIELD(rxm,                        (ice_timestamp_dynfield_offset),                        rte_mbuf_timestamp_t *) = ts_ns;             pkt_flags |= ice_timestamp_dynflag;         }         if (ad->ptp_ena && ((rxm->packet_type & RTE_PTYPE_L2_MASK) ==             RTE_PTYPE_L2_ETHER_TIMESYNC)) {             rxq->time_high =                rte_le_to_cpu_32(rxd.wb.flex_ts.ts_high);             rxm->timesync = rxq->queue_id;             pkt_flags |= RTE_MBUF_F_RX_IEEE1588_PTP;         } #endif Note: rxm->timesync and rxq->time_high will be used in timesync_read_rx_timestamp() > > >>> 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(-) >>>> >>> . > .