* [PATCH 0/2] net/iavf: handle PF not enabling Rx timestamps properly
@ 2025-11-13 21:33 Jacob Keller
2025-11-13 21:33 ` [PATCH 1/2] net/iavf: check if PF actually indicates Rx timestamps Jacob Keller
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jacob Keller @ 2025-11-13 21:33 UTC (permalink / raw)
To: dev; +Cc: Paul Greenwalt, Vladimir Medvedkin, Jacob Keller
In certain cases, the PF for the iavf driver may support
VIRTCHNL_VF_CAP_PTP but will not enable Rx timestamps. If this occurs, the
iavf driver will attempt to continue anyways. When that happens, the
resulting timestamps will appear to function initially. Upon closer
inspection it becomes clear that the timestamps are invalid and bogus.
First, when reporting a timestamp, always check the validity bit instead of
blindly reporting it. This avoids extending an invalid (likely zero'd)
timestamp value from the Rx descriptor.
Second, don't enable the RTE_ETH_RX_OFFLOAD_TIMESTAMP capability if the PF
doesn't indicate we have support. This will prevent applications from
trying to timestamp when it is not properly enabled.
Typically, this should not happen, as the PFs which support
VIRTCHNL_VF_CAP_PTP should support Rx timestamping. However, we recently
discovered a flaw in some implementations of the
VIRTCHNL_OP_1588_PTP_GET_CAPS command. The layout of the capabilities
structure is incorrect, with the caps member placed at a different byte
offset than the expected structure layout. This is the case with the
upstream Linux ice PF driver.
This results in the PF rejecting the request for Rx timestamping, and
leaving timestamping disabled. A proper fix for this situation is
difficult. If we merely changed the structure layout in DPDK, then it would
stop being compatible with other host OSes and with other implementations
of the Linux ice PF. If we changed the layout upstream, it would break
compatibility with the upstream iAVF VF driver.
A proper fix to resolve this will take some time, as we will likely have to
introduce a new flag or ops capability across many drivers. In the mean
time, we should at least make sure the DPDK driver stops reporting bogus
timestamps in such a setup.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
Jacob Keller (2):
net/iavf: check if PF actually indicates Rx timestamps
net/iavf: check Rx timestamp validity bit
drivers/net/intel/iavf/iavf_rxtx.h | 3 +++
drivers/net/intel/iavf/iavf_ethdev.c | 3 ++-
drivers/net/intel/iavf/iavf_rxtx.c | 9 ++++++---
3 files changed, 11 insertions(+), 4 deletions(-)
---
base-commit: be0112878b4fc3a1b918370ed2fe615c52039d77
change-id: 20251113-jk-dpdk-iavf-rx-timestamping-improvements-a06d21385371
Best regards,
--
Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] net/iavf: check if PF actually indicates Rx timestamps
2025-11-13 21:33 [PATCH 0/2] net/iavf: handle PF not enabling Rx timestamps properly Jacob Keller
@ 2025-11-13 21:33 ` Jacob Keller
2025-11-17 11:37 ` Bruce Richardson
2025-11-13 21:33 ` [PATCH 2/2] net/iavf: check Rx timestamp validity bit Jacob Keller
2025-11-17 11:49 ` [PATCH 0/2] net/iavf: handle PF not enabling Rx timestamps properly Bruce Richardson
2 siblings, 1 reply; 6+ messages in thread
From: Jacob Keller @ 2025-11-13 21:33 UTC (permalink / raw)
To: dev; +Cc: Paul Greenwalt, Vladimir Medvedkin, Jacob Keller
The iavf driver has support for hardware Rx timestamps since commit h
b5cd735132f6 ("net/iavf: enable Rx timestamp on flex descriptor").
To enable this, the VF must first negotiate PTP capabilities with the PF
by sending the VIRTCHNL_OP_1588_PTP_GET_CAPS command, with the requested
capabilities. The PF will respond with the actually supported subset of
capabilities.
The PF may not actually enable Rx timestamping, even if it reports the
overall PTP capability support. If this happens, the iavf driver logic
will incorrectly report that Rx timestamps can be enabled despite being
rejected by the PF.
This is unlikely in practice, as most PFs which support the
VIRTCHNL_VF_CAP_PTP will support Rx timestamping. However, there are
some cases where this may not be true.
Check that the PF actually reports the Rx timestamping capability
instead of assuming it is enabled. Doing so prevents the DPDK
application from attempting to enable Rx timestamps when they won't
actually be enabled.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/intel/iavf/iavf_ethdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index 15e49fe24814..3ef766de4704 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -1177,7 +1177,8 @@ iavf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_CRC)
dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_KEEP_CRC;
- if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_CAP_PTP)
+ if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_CAP_PTP &&
+ vf->ptp_caps & VIRTCHNL_1588_PTP_CAP_RX_TSTAMP)
dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_TIMESTAMP;
if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_VLAN_V2 &&
--
2.51.0.rc1.197.g6d975e95c9d7
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] net/iavf: check Rx timestamp validity bit
2025-11-13 21:33 [PATCH 0/2] net/iavf: handle PF not enabling Rx timestamps properly Jacob Keller
2025-11-13 21:33 ` [PATCH 1/2] net/iavf: check if PF actually indicates Rx timestamps Jacob Keller
@ 2025-11-13 21:33 ` Jacob Keller
2025-11-17 11:45 ` Bruce Richardson
2025-11-17 11:49 ` [PATCH 0/2] net/iavf: handle PF not enabling Rx timestamps properly Bruce Richardson
2 siblings, 1 reply; 6+ messages in thread
From: Jacob Keller @ 2025-11-13 21:33 UTC (permalink / raw)
To: dev; +Cc: Paul Greenwalt, Vladimir Medvedkin, Jacob Keller
When reporting an Rx timestamp from the receive descriptor, the iavf
driver does not check the validity bit in the time_stamp_low field. In
the event that hardware does not capture a receive timestamp for any
reason, this valid bit is unset, and the timestamp value in the
descriptor is zeroed out.
The iavf driver ignores this and passes the zero value into the
iavf_tstamp_convert_32b_64b function regardless, and proceeds to treat
the result as a valid timestamp.
Instead of reporting a zero timestamp which users can clearly interpret
as invalid, the raw 0 value from the descriptor is "extended" to the
64-bit timestamp. This results in values which are not immediately
obvious as invalid to users:
timestamp 1760629088881475583
timestamp 1760629088881475583
timestamp 1760629088881475583
First, if the value is printed in base 10 it is not immediately obvious
that the lower 32 bits are zero. Second, multiple packets in sequence
will receive the same "timestamp".
This occurs because of the timestamp extension logic. The receive
descriptor timestamps are 40 bits, with 32 bits of nanosecond precision,
7 bits of subnanosecond precision, and 1 validity bit. The
sub-nanosecond precision bits are discarded. To obtain a 64-bit
timestamp, the upper 32 bits are calculated from the lower 32-bits and a
snapshot of the PHC timer that is captured recently (within ~2 seconds
of the packet timestamp). This enables reporting proper full 64-bit
timestamps without needing to store all 64 bits in the receive
descriptor.
However, when timestamps are not working properly, the raw 'zero' value
is extended regardless of whether hardware indicated it was a valid
timestamp. As a result, users can see what appear at a glance as valid
timestamps. However, they will not match the packet reception time, and
will only update when the upper bits would roll over. This occurs every
2^32 seconds, or approximately once every 4 seconds.
Instead of reporting bogus extended timestamp values which could confuse
user applications, check the validity bit and only report a timestamp of
the valid bit is set. This matches the implementation used in the Linux
PF driver.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/intel/iavf/iavf_rxtx.h | 3 +++
drivers/net/intel/iavf/iavf_rxtx.c | 9 ++++++---
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/intel/iavf/iavf_rxtx.h b/drivers/net/intel/iavf/iavf_rxtx.h
index 5c9339b99f9a..8efb3bd04ee5 100644
--- a/drivers/net/intel/iavf/iavf_rxtx.h
+++ b/drivers/net/intel/iavf/iavf_rxtx.h
@@ -504,6 +504,9 @@ enum iavf_tx_ctx_desc_tunnel_l4_tunnel_type {
/* for iavf_32b_rx_flex_desc.pkt_len member */
#define IAVF_RX_FLX_DESC_PKT_LEN_M (0x3FFF) /* 14-bits */
+/* Valid indicator bit for the time_stamp_low field */
+#define IAVF_RX_FLX_DESC_TS_VALID (0x1UL)
+
int iavf_dev_rx_queue_setup(struct rte_eth_dev *dev,
uint16_t queue_idx,
uint16_t nb_desc,
diff --git a/drivers/net/intel/iavf/iavf_rxtx.c b/drivers/net/intel/iavf/iavf_rxtx.c
index ea49059f83a1..d8662fd81533 100644
--- a/drivers/net/intel/iavf/iavf_rxtx.c
+++ b/drivers/net/intel/iavf/iavf_rxtx.c
@@ -1582,7 +1582,8 @@ iavf_recv_pkts_flex_rxd(void *rx_queue,
rxd_to_pkt_fields_ops[rxq->rxdid](rxq, rxm, &rxd);
pkt_flags = iavf_flex_rxd_error_to_pkt_flags(rx_stat_err0);
- if (iavf_timestamp_dynflag > 0) {
+ if (iavf_timestamp_dynflag > 0 &&
+ rxd.wb.time_stamp_low & IAVF_RX_FLX_DESC_TS_VALID) {
ts_ns = iavf_tstamp_convert_32b_64b(rxq->phc_time,
rte_le_to_cpu_32(rxd.wb.flex_ts.ts_high));
@@ -1751,7 +1752,8 @@ iavf_recv_scattered_pkts_flex_rxd(void *rx_queue, struct rte_mbuf **rx_pkts,
rxd_to_pkt_fields_ops[rxq->rxdid](rxq, first_seg, &rxd);
pkt_flags = iavf_flex_rxd_error_to_pkt_flags(rx_stat_err0);
- if (iavf_timestamp_dynflag > 0) {
+ if (iavf_timestamp_dynflag > 0 &&
+ rxd.wb.time_stamp_low & IAVF_RX_FLX_DESC_TS_VALID) {
ts_ns = iavf_tstamp_convert_32b_64b(rxq->phc_time,
rte_le_to_cpu_32(rxd.wb.flex_ts.ts_high));
@@ -2036,7 +2038,8 @@ iavf_rx_scan_hw_ring_flex_rxd(struct ci_rx_queue *rxq,
stat_err0 = rte_le_to_cpu_16(rxdp[j].wb.status_error0);
pkt_flags = iavf_flex_rxd_error_to_pkt_flags(stat_err0);
- if (iavf_timestamp_dynflag > 0) {
+ if (iavf_timestamp_dynflag > 0 &&
+ rxdp[j].wb.time_stamp_low & IAVF_RX_FLX_DESC_TS_VALID) {
ts_ns = iavf_tstamp_convert_32b_64b(rxq->phc_time,
rte_le_to_cpu_32(rxdp[j].wb.flex_ts.ts_high));
--
2.51.0.rc1.197.g6d975e95c9d7
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] net/iavf: check if PF actually indicates Rx timestamps
2025-11-13 21:33 ` [PATCH 1/2] net/iavf: check if PF actually indicates Rx timestamps Jacob Keller
@ 2025-11-17 11:37 ` Bruce Richardson
0 siblings, 0 replies; 6+ messages in thread
From: Bruce Richardson @ 2025-11-17 11:37 UTC (permalink / raw)
To: Jacob Keller; +Cc: dev, Paul Greenwalt, Vladimir Medvedkin
On Thu, Nov 13, 2025 at 01:33:44PM -0800, Jacob Keller wrote:
> The iavf driver has support for hardware Rx timestamps since commit h
> b5cd735132f6 ("net/iavf: enable Rx timestamp on flex descriptor").
>
> To enable this, the VF must first negotiate PTP capabilities with the PF
> by sending the VIRTCHNL_OP_1588_PTP_GET_CAPS command, with the requested
> capabilities. The PF will respond with the actually supported subset of
> capabilities.
>
> The PF may not actually enable Rx timestamping, even if it reports the
> overall PTP capability support. If this happens, the iavf driver logic
> will incorrectly report that Rx timestamps can be enabled despite being
> rejected by the PF.
>
> This is unlikely in practice, as most PFs which support the
> VIRTCHNL_VF_CAP_PTP will support Rx timestamping. However, there are
> some cases where this may not be true.
>
> Check that the PF actually reports the Rx timestamping capability
> instead of assuming it is enabled. Doing so prevents the DPDK
> application from attempting to enable Rx timestamps when they won't
> actually be enabled.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
Fixes: b5cd735132f6 ("net/iavf: enable Rx timestamp on flex descriptor")
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] net/iavf: check Rx timestamp validity bit
2025-11-13 21:33 ` [PATCH 2/2] net/iavf: check Rx timestamp validity bit Jacob Keller
@ 2025-11-17 11:45 ` Bruce Richardson
0 siblings, 0 replies; 6+ messages in thread
From: Bruce Richardson @ 2025-11-17 11:45 UTC (permalink / raw)
To: Jacob Keller; +Cc: dev, Paul Greenwalt, Vladimir Medvedkin
On Thu, Nov 13, 2025 at 01:33:45PM -0800, Jacob Keller wrote:
> When reporting an Rx timestamp from the receive descriptor, the iavf
> driver does not check the validity bit in the time_stamp_low field. In
> the event that hardware does not capture a receive timestamp for any
> reason, this valid bit is unset, and the timestamp value in the
> descriptor is zeroed out.
>
> The iavf driver ignores this and passes the zero value into the
> iavf_tstamp_convert_32b_64b function regardless, and proceeds to treat
> the result as a valid timestamp.
>
> Instead of reporting a zero timestamp which users can clearly interpret
> as invalid, the raw 0 value from the descriptor is "extended" to the
> 64-bit timestamp. This results in values which are not immediately
> obvious as invalid to users:
>
> timestamp 1760629088881475583
> timestamp 1760629088881475583
> timestamp 1760629088881475583
>
> First, if the value is printed in base 10 it is not immediately obvious
> that the lower 32 bits are zero. Second, multiple packets in sequence
> will receive the same "timestamp".
>
> This occurs because of the timestamp extension logic. The receive
> descriptor timestamps are 40 bits, with 32 bits of nanosecond precision,
> 7 bits of subnanosecond precision, and 1 validity bit. The
> sub-nanosecond precision bits are discarded. To obtain a 64-bit
> timestamp, the upper 32 bits are calculated from the lower 32-bits and a
> snapshot of the PHC timer that is captured recently (within ~2 seconds
> of the packet timestamp). This enables reporting proper full 64-bit
> timestamps without needing to store all 64 bits in the receive
> descriptor.
>
> However, when timestamps are not working properly, the raw 'zero' value
> is extended regardless of whether hardware indicated it was a valid
> timestamp. As a result, users can see what appear at a glance as valid
> timestamps. However, they will not match the packet reception time, and
> will only update when the upper bits would roll over. This occurs every
> 2^32 seconds, or approximately once every 4 seconds.
>
> Instead of reporting bogus extended timestamp values which could confuse
> user applications, check the validity bit and only report a timestamp of
> the valid bit is set. This matches the implementation used in the Linux
> PF driver.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
Fixes: b5cd735132f6 ("net/iavf: enable Rx timestamp on flex descriptor")
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] net/iavf: handle PF not enabling Rx timestamps properly
2025-11-13 21:33 [PATCH 0/2] net/iavf: handle PF not enabling Rx timestamps properly Jacob Keller
2025-11-13 21:33 ` [PATCH 1/2] net/iavf: check if PF actually indicates Rx timestamps Jacob Keller
2025-11-13 21:33 ` [PATCH 2/2] net/iavf: check Rx timestamp validity bit Jacob Keller
@ 2025-11-17 11:49 ` Bruce Richardson
2 siblings, 0 replies; 6+ messages in thread
From: Bruce Richardson @ 2025-11-17 11:49 UTC (permalink / raw)
To: Jacob Keller; +Cc: dev, Paul Greenwalt, Vladimir Medvedkin
On Thu, Nov 13, 2025 at 01:33:43PM -0800, Jacob Keller wrote:
> In certain cases, the PF for the iavf driver may support
> VIRTCHNL_VF_CAP_PTP but will not enable Rx timestamps. If this occurs, the
> iavf driver will attempt to continue anyways. When that happens, the
> resulting timestamps will appear to function initially. Upon closer
> inspection it becomes clear that the timestamps are invalid and bogus.
>
> First, when reporting a timestamp, always check the validity bit instead of
> blindly reporting it. This avoids extending an invalid (likely zero'd)
> timestamp value from the Rx descriptor.
>
> Second, don't enable the RTE_ETH_RX_OFFLOAD_TIMESTAMP capability if the PF
> doesn't indicate we have support. This will prevent applications from
> trying to timestamp when it is not properly enabled.
>
> Typically, this should not happen, as the PFs which support
> VIRTCHNL_VF_CAP_PTP should support Rx timestamping. However, we recently
> discovered a flaw in some implementations of the
> VIRTCHNL_OP_1588_PTP_GET_CAPS command. The layout of the capabilities
> structure is incorrect, with the caps member placed at a different byte
> offset than the expected structure layout. This is the case with the
> upstream Linux ice PF driver.
>
> This results in the PF rejecting the request for Rx timestamping, and
> leaving timestamping disabled. A proper fix for this situation is
> difficult. If we merely changed the structure layout in DPDK, then it would
> stop being compatible with other host OSes and with other implementations
> of the Linux ice PF. If we changed the layout upstream, it would break
> compatibility with the upstream iAVF VF driver.
>
> A proper fix to resolve this will take some time, as we will likely have to
> introduce a new flag or ops capability across many drivers. In the mean
> time, we should at least make sure the DPDK driver stops reporting bogus
> timestamps in such a setup.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> Jacob Keller (2):
> net/iavf: check if PF actually indicates Rx timestamps
> net/iavf: check Rx timestamp validity bit
>
Series applied to dpdk-next-net-intel.
Thanks,
/Bruce
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-11-17 11:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-13 21:33 [PATCH 0/2] net/iavf: handle PF not enabling Rx timestamps properly Jacob Keller
2025-11-13 21:33 ` [PATCH 1/2] net/iavf: check if PF actually indicates Rx timestamps Jacob Keller
2025-11-17 11:37 ` Bruce Richardson
2025-11-13 21:33 ` [PATCH 2/2] net/iavf: check Rx timestamp validity bit Jacob Keller
2025-11-17 11:45 ` Bruce Richardson
2025-11-17 11:49 ` [PATCH 0/2] net/iavf: handle PF not enabling Rx timestamps properly Bruce Richardson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).