From: Bruce Richardson <bruce.richardson@intel.com>
To: Soumyadeep Hore <soumyadeep.hore@intel.com>
Cc: <manoj.kumar.subbarao@intel.com>, <aman.deep.singh@intel.com>,
<dev@dpdk.org>, <rajesh3.kumar@intel.com>
Subject: Re: [PATCH v3 1/2] net/iavf: remove PHC polling from Rx datapath
Date: Wed, 8 Apr 2026 17:27:29 +0100 [thread overview]
Message-ID: <adaB8ar4Kb5lzpAD@bricha3-mobl1.ger.corp.intel.com> (raw)
In-Reply-To: <20260406212208.1562899-2-soumyadeep.hore@intel.com>
On Mon, Apr 06, 2026 at 05:22:07PM -0400, Soumyadeep Hore wrote:
> Remove periodic PHC read/update checks from scalar and vector flex
> RX paths, keeping timestamp conversion based on queue PHC state.
>
> This avoids hot-path PHC polling overhead and preserves the latency
> fix for RX timestamp-enabled traffic.
>
> Signed-off-by: Soumyadeep Hore <soumyadeep.hore@intel.com>
> ---
> drivers/net/intel/iavf/iavf_rxtx.c | 34 -------------------
> drivers/net/intel/iavf/iavf_rxtx_vec_avx2.c | 24 ++-----------
> drivers/net/intel/iavf/iavf_rxtx_vec_avx512.c | 26 ++------------
> 3 files changed, 6 insertions(+), 78 deletions(-)
>
With all the code deletions, does the feature still work after this patch?
If not, I will probably need to squash patches 1 & 2 together on apply so
that the feature is not broken in the middle of the set.
Also, patches are missing fixes lines and the reference to the relevant
bugzilla [1].
Thanks,
/Bruce
[1] https://bugs.dpdk.org/show_bug.cgi?id=1898
> diff --git a/drivers/net/intel/iavf/iavf_rxtx.c b/drivers/net/intel/iavf/iavf_rxtx.c
> index 4ff6c18dc4..fabccc89bf 100644
> --- a/drivers/net/intel/iavf/iavf_rxtx.c
> +++ b/drivers/net/intel/iavf/iavf_rxtx.c
> @@ -1507,16 +1507,6 @@ iavf_recv_pkts_flex_rxd(void *rx_queue,
> rx_ring = rxq->rx_flex_ring;
> ptype_tbl = rxq->iavf_vsi->adapter->ptype_tbl;
>
> - if (rxq->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) {
> - uint64_t sw_cur_time = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
> -
> - if (sw_cur_time - rxq->hw_time_update > 4) {
> - if (iavf_get_phc_time(rxq))
> - PMD_DRV_LOG(ERR, "get physical time failed");
> - rxq->hw_time_update = sw_cur_time;
> - }
> - }
> -
> while (nb_rx < nb_pkts) {
> rxdp = &rx_ring[rx_id];
> rx_stat_err0 = rte_le_to_cpu_16(rxdp->wb.status_error0);
> @@ -1585,7 +1575,6 @@ iavf_recv_pkts_flex_rxd(void *rx_queue,
> rte_le_to_cpu_32(rxd.wb.flex_ts.ts_high));
>
> rxq->phc_time = ts_ns;
> - rxq->hw_time_update = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
>
> *RTE_MBUF_DYNFIELD(rxm,
> iavf_timestamp_dynfield_offset,
> @@ -1627,16 +1616,6 @@ iavf_recv_scattered_pkts_flex_rxd(void *rx_queue, struct rte_mbuf **rx_pkts,
> volatile union ci_rx_flex_desc *rxdp;
> const uint32_t *ptype_tbl = rxq->iavf_vsi->adapter->ptype_tbl;
>
> - if (rxq->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) {
> - uint64_t sw_cur_time = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
> -
> - if (sw_cur_time - rxq->hw_time_update > 4) {
> - if (iavf_get_phc_time(rxq))
> - PMD_DRV_LOG(ERR, "get physical time failed");
> - rxq->hw_time_update = sw_cur_time;
> - }
> - }
> -
> while (nb_rx < nb_pkts) {
> rxdp = &rx_ring[rx_id];
> rx_stat_err0 = rte_le_to_cpu_16(rxdp->wb.status_error0);
> @@ -1755,7 +1734,6 @@ iavf_recv_scattered_pkts_flex_rxd(void *rx_queue, struct rte_mbuf **rx_pkts,
> rte_le_to_cpu_32(rxd.wb.flex_ts.ts_high));
>
> rxq->phc_time = ts_ns;
> - rxq->hw_time_update = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
>
> *RTE_MBUF_DYNFIELD(first_seg,
> iavf_timestamp_dynfield_offset,
> @@ -1969,16 +1947,6 @@ iavf_rx_scan_hw_ring_flex_rxd(struct ci_rx_queue *rxq,
> if (!(stat_err0 & (1 << IAVF_RX_FLEX_DESC_STATUS0_DD_S)))
> return 0;
>
> - if (rxq->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) {
> - uint64_t sw_cur_time = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
> -
> - if (sw_cur_time - rxq->hw_time_update > 4) {
> - if (iavf_get_phc_time(rxq))
> - PMD_DRV_LOG(ERR, "get physical time failed");
> - rxq->hw_time_update = sw_cur_time;
> - }
> - }
> -
> /* Scan LOOK_AHEAD descriptors at a time to determine which
> * descriptors reference packets that are ready to be received.
> */
> @@ -2041,8 +2009,6 @@ iavf_rx_scan_hw_ring_flex_rxd(struct ci_rx_queue *rxq,
> rte_le_to_cpu_32(rxdp[j].wb.flex_ts.ts_high));
>
> rxq->phc_time = ts_ns;
> - rxq->hw_time_update = rte_get_timer_cycles() /
> - (rte_get_timer_hz() / 1000);
>
> *RTE_MBUF_DYNFIELD(mb,
> iavf_timestamp_dynfield_offset,
> diff --git a/drivers/net/intel/iavf/iavf_rxtx_vec_avx2.c b/drivers/net/intel/iavf/iavf_rxtx_vec_avx2.c
> index db0462f0f5..c91123ead4 100644
> --- a/drivers/net/intel/iavf/iavf_rxtx_vec_avx2.c
> +++ b/drivers/net/intel/iavf/iavf_rxtx_vec_avx2.c
> @@ -514,19 +514,8 @@ _iavf_recv_raw_pkts_vec_avx2_flex_rxd(struct ci_rx_queue *rxq,
> if (!(rxdp->wb.status_error0 &
> rte_cpu_to_le_32(1 << IAVF_RX_FLEX_DESC_STATUS0_DD_S)))
> return 0;
> - bool is_tsinit = false;
> uint8_t inflection_point = 0;
> __m256i hw_low_last = _mm256_set_epi32(0, 0, 0, 0, 0, 0, 0, rxq->phc_time);
> - if (rxq->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) {
> - uint64_t sw_cur_time = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
> -
> - if (unlikely(sw_cur_time - rxq->hw_time_update > 4)) {
> - hw_low_last = _mm256_setzero_si256();
> - is_tsinit = 1;
> - } else {
> - hw_low_last = _mm256_set_epi32(0, 0, 0, 0, 0, 0, 0, rxq->phc_time);
> - }
> - }
>
> /* constants used in processing loop */
> const __m256i crc_adjust =
> @@ -1152,14 +1141,9 @@ _iavf_recv_raw_pkts_vec_avx2_flex_rxd(struct ci_rx_queue *rxq,
> *RTE_MBUF_DYNFIELD(rx_pkts[i + 7],
> iavf_timestamp_dynfield_offset, uint32_t *) = _mm256_extract_epi32(ts_low1, 7);
>
> - if (unlikely(is_tsinit)) {
> - uint32_t in_timestamp;
> - if (iavf_get_phc_time(rxq))
> - PMD_DRV_LOG(ERR, "get physical time failed");
> - in_timestamp = *RTE_MBUF_DYNFIELD(rx_pkts[i + 0],
> - iavf_timestamp_dynfield_offset, uint32_t *);
> - rxq->phc_time = iavf_tstamp_convert_32b_64b(rxq->phc_time, in_timestamp);
> - }
> + rxq->phc_time = iavf_tstamp_convert_32b_64b(rxq->phc_time,
> + *RTE_MBUF_DYNFIELD(rx_pkts[i + 0],
> + iavf_timestamp_dynfield_offset, uint32_t *));
>
> *RTE_MBUF_DYNFIELD(rx_pkts[i + 0],
> iavf_timestamp_dynfield_offset + 4, uint32_t *) = (uint32_t)(rxq->phc_time >> 32);
> @@ -1388,8 +1372,6 @@ _iavf_recv_raw_pkts_vec_avx2_flex_rxd(struct ci_rx_queue *rxq,
> PMD_DRV_LOG(ERR, "invalid inflection point for rx timestamp");
> break;
> }
> -
> - rxq->hw_time_update = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
> }
> if (burst != IAVF_VPMD_DESCS_PER_LOOP_WIDE)
> break;
> diff --git a/drivers/net/intel/iavf/iavf_rxtx_vec_avx512.c b/drivers/net/intel/iavf/iavf_rxtx_vec_avx512.c
> index 4e8bf94fa0..a7c0a02eba 100644
> --- a/drivers/net/intel/iavf/iavf_rxtx_vec_avx512.c
> +++ b/drivers/net/intel/iavf/iavf_rxtx_vec_avx512.c
> @@ -615,19 +615,7 @@ _iavf_recv_raw_pkts_vec_avx512_flex_rxd(struct ci_rx_queue *rxq,
>
> #ifdef IAVF_RX_TS_OFFLOAD
> uint8_t inflection_point = 0;
> - bool is_tsinit = false;
> __m256i hw_low_last = _mm256_set_epi32(0, 0, 0, 0, 0, 0, 0, (uint32_t)rxq->phc_time);
> -
> - if (rxq->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) {
> - uint64_t sw_cur_time = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
> -
> - if (unlikely(sw_cur_time - rxq->hw_time_update > 4)) {
> - hw_low_last = _mm256_setzero_si256();
> - is_tsinit = 1;
> - } else {
> - hw_low_last = _mm256_set_epi32(0, 0, 0, 0, 0, 0, 0, (uint32_t)rxq->phc_time);
> - }
> - }
> #endif
>
> /* constants used in processing loop */
> @@ -1343,15 +1331,9 @@ _iavf_recv_raw_pkts_vec_avx512_flex_rxd(struct ci_rx_queue *rxq,
> *RTE_MBUF_DYNFIELD(rx_pkts[i + 7],
> iavf_timestamp_dynfield_offset, uint32_t *) = _mm256_extract_epi32(ts_low1, 7);
>
> - if (unlikely(is_tsinit)) {
> - uint32_t in_timestamp;
> -
> - if (iavf_get_phc_time(rxq))
> - PMD_DRV_LOG(ERR, "get physical time failed");
> - in_timestamp = *RTE_MBUF_DYNFIELD(rx_pkts[i + 0],
> - iavf_timestamp_dynfield_offset, uint32_t *);
> - rxq->phc_time = iavf_tstamp_convert_32b_64b(rxq->phc_time, in_timestamp);
> - }
> + rxq->phc_time = iavf_tstamp_convert_32b_64b(rxq->phc_time,
> + *RTE_MBUF_DYNFIELD(rx_pkts[i + 0],
> + iavf_timestamp_dynfield_offset, uint32_t *));
>
> *RTE_MBUF_DYNFIELD(rx_pkts[i + 0],
> iavf_timestamp_dynfield_offset + 4, uint32_t *) = (uint32_t)(rxq->phc_time >> 32);
> @@ -1584,8 +1566,6 @@ _iavf_recv_raw_pkts_vec_avx512_flex_rxd(struct ci_rx_queue *rxq,
> PMD_DRV_LOG(ERR, "invalid inflection point for rx timestamp");
> break;
> }
> -
> - rxq->hw_time_update = rte_get_timer_cycles() / (rte_get_timer_hz() / 1000);
> }
> #endif
> if (burst != IAVF_VPMD_DESCS_PER_LOOP_WIDE)
> --
> 2.47.1
>
next prev parent reply other threads:[~2026-04-08 16:27 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-02 15:21 [PATCH v1 0/2] Update Rx Timestamp in IAVF PMD Soumyadeep Hore
2026-04-02 15:21 ` [PATCH v1 1/2] net/iavf: remove PHC polling from Rx datapath Soumyadeep Hore
2026-04-02 15:46 ` [PATCH v2 0/2] Update Rx Timestamp in IAVF PMD Soumyadeep Hore
2026-04-02 15:46 ` [PATCH v2 1/2] net/iavf: remove PHC polling from Rx datapath Soumyadeep Hore
2026-04-06 21:22 ` [PATCH v3 0/2] Update Rx Timestamp in IAVF PMD Soumyadeep Hore
2026-04-06 21:22 ` [PATCH v3 1/2] net/iavf: remove PHC polling from Rx datapath Soumyadeep Hore
2026-04-08 16:27 ` Bruce Richardson [this message]
2026-04-06 21:22 ` [PATCH v3 2/2] net/iavf: reuse device alarm for PHC sync Soumyadeep Hore
2026-04-02 15:46 ` [PATCH v2 " Soumyadeep Hore
2026-04-02 15:48 ` [PATCH v2 0/2] Update Rx Timestamp in IAVF PMD Soumyadeep Hore
2026-04-02 15:48 ` [PATCH v2 1/2] net/iavf: remove PHC polling from Rx datapath Soumyadeep Hore
2026-04-02 15:48 ` [PATCH v2 2/2] net/iavf: reuse device alarm for PHC sync Soumyadeep Hore
2026-04-02 15:21 ` [PATCH v1 " Soumyadeep Hore
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=adaB8ar4Kb5lzpAD@bricha3-mobl1.ger.corp.intel.com \
--to=bruce.richardson@intel.com \
--cc=aman.deep.singh@intel.com \
--cc=dev@dpdk.org \
--cc=manoj.kumar.subbarao@intel.com \
--cc=rajesh3.kumar@intel.com \
--cc=soumyadeep.hore@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox