From: Wojciech Drewek <wojciech.dre...@intel.com> Date: Wed, 21 Aug 2024 14:15:39 +0200
> From: Jacob Keller <jacob.e.kel...@intel.com> > > Add support for receive timestamps to the Rx hotpath. This support only > works when using the flexible descriptor format, so make sure that we > request this format by default if we have receive timestamp support > available in the PTP capabilities. > > In order to report the timestamps to userspace, we need to perform > timestamp extension. The Rx descriptor does actually contain the "40 > bit" timestamp. However, upper 32 bits which contain nanoseconds are > conveniently stored separately in the descriptor. We could extract the > 32bits and lower 8 bits, then perform a bitwise OR to calculate the > 40bit value. This makes no sense, because the timestamp extension > algorithm would simply discard the lower 8 bits anyways. > > Thus, implement timestamp extension as iavf_ptp_extend_32b_timestamp(), > and extract and forward only the 32bits of nominal nanoseconds. > > Signed-off-by: Jacob Keller <jacob.e.kel...@intel.com> > Reviewed-by: Wojciech Drewek <wojciech.dre...@intel.com> > Reviewed-by: Rahul Rameshbabu <rrameshb...@nvidia.com> > Reviewed-by: Sunil Goutham <sgout...@marvell.com> > Reviewed-by: Simon Horman <ho...@kernel.org> > Signed-off-by: Mateusz Polchlopek <mateusz.polchlo...@intel.com> > Signed-off-by: Wojciech Drewek <wojciech.dre...@intel.com> > --- > drivers/net/ethernet/intel/iavf/iavf_main.c | 11 +++- > drivers/net/ethernet/intel/iavf/iavf_ptp.c | 61 +++++++++++++++++++++ > drivers/net/ethernet/intel/iavf/iavf_ptp.h | 4 ++ > drivers/net/ethernet/intel/iavf/iavf_txrx.c | 45 +++++++++++++++ > drivers/net/ethernet/intel/iavf/iavf_type.h | 1 + > 5 files changed, 121 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c > b/drivers/net/ethernet/intel/iavf/iavf_main.c > index 1458410ca560..ebc01a8d1ac6 100644 > --- a/drivers/net/ethernet/intel/iavf/iavf_main.c > +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c > @@ -730,6 +730,15 @@ static u8 iavf_select_rx_desc_format(const struct > iavf_adapter *adapter) > if (!IAVF_RXDID_ALLOWED(adapter)) > return VIRTCHNL_RXDID_1_32B_BASE; > > + /* Rx timestamping requires the use of flexible NIC descriptors */ > + if (iavf_ptp_cap_supported(adapter, VIRTCHNL_1588_PTP_CAP_RX_TSTAMP)) { > + if (rxdids & BIT(VIRTCHNL_RXDID_2_FLEX_SQ_NIC)) > + return VIRTCHNL_RXDID_2_FLEX_SQ_NIC; > + > + pci_warn(adapter->pdev, > + "Unable to negotiate flexible descriptor format\n"); > + } > + > /* Warn if the PF does not list support for the default legacy > * descriptor format. This shouldn't happen, as this is the format > * used if VIRTCHNL_VF_OFFLOAD_RX_FLEX_DESC is not supported. It is > @@ -737,7 +746,7 @@ static u8 iavf_select_rx_desc_format(const struct > iavf_adapter *adapter) > * support for the format. > */ > if (!(rxdids & VIRTCHNL_RXDID_1_32B_BASE_M)) > - dev_warn(&adapter->pdev->dev, "PF does not list support for > default Rx descriptor format\n"); > + pci_warn(adapter->pdev, "PF does not list support for default > Rx descriptor format\n"); Unrelated (addresses my earlier comments, but not related to this patch). [...] > @@ -1085,6 +1086,49 @@ static void iavf_flex_rx_hash(const struct iavf_ring > *ring, > } > } > > +/** > + * iavf_flex_rx_tstamp - Capture Rx timestamp from the descriptor > + * @rx_ring: descriptor ring > + * @rx_desc: specific descriptor > + * @skb: skb currently being received > + * > + * Read the Rx timestamp value from the descriptor and pass it to the stack. > + * > + * This function only operates on the VIRTCHNL_RXDID_2_FLEX_SQ_NIC flexible > + * descriptor writeback format. > + */ > +static void iavf_flex_rx_tstamp(const struct iavf_ring *rx_ring, > + const struct iavf_rx_desc *rx_desc, > + struct sk_buff *skb) > +{ > + struct iavf_adapter *adapter; > + __le64 qw2 = rx_desc->qw2; > + __le64 qw3 = rx_desc->qw3; You only need both @qw2 and @qw3 once, so I'd avoid creating these variables. But given that these qwords are used in other functions, I'd just pass both qwords as function arguments instead of @rx_desc. > + u32 tstamp; > + u64 ns; > + > + /* Skip processing if timestamps aren't enabled */ > + if (!(rx_ring->flags & IAVF_TXRX_FLAGS_HW_TSTAMP)) > + return; > + > + /* Check if this Rx descriptor has a valid timestamp */ > + if (!le64_get_bits(qw2, IAVF_PTP_40B_TSTAMP_VALID)) > + return; > + > + /* the ts_low field only contains the valid bit and sub-nanosecond > + * precision, so we don't need to extract it. > + */ > + tstamp = le64_get_bits(qw3, IAVF_RXD_FLEX_QW3_TSTAMP_HIGH_M); > + > + adapter = netdev_priv(rx_ring->netdev); > + ns = iavf_ptp_extend_32b_timestamp(adapter->ptp.cached_phc_time, > + tstamp); Hmm, this netdev_priv() + adapter access etc. doesn't look good on hotpath. I think it would be better to cache a pointer to adapter->ptp in each ring structure and update them when it gets changed. > + > + *skb_hwtstamps(skb) = (struct skb_shared_hwtstamps) { > + .hwtstamp = ns_to_ktime(ns), > + }; Thanks, Olek