Re: [dpdk-dev] [PATCH] net/ice: enable Rx timestamp on Flex Descriptor

["Zhang, Qi Z" <qi.z.zhang@intel.com>]

> -----Original Message-----
> From: Su, Simei <simei.su@intel.com>
> Sent: Saturday, September 18, 2021 2:57 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>; Su, Simei
> <simei.su@intel.com>
> Subject: [PATCH] net/ice: enable Rx timestamp on Flex Descriptor
> 
> Use the dynamic mbuf to register timestamp field and flag.
> The ice has the feature to dump Rx timestamp value into dynamic mbuf field
> by flex descriptor. This feature is turned on by dev config
> "enable-rx-timestamp". Currently, it's only supported under scalar path.
> 
> Signed-off-by: Simei Su <simei.su@intel.com>
> ---
>  doc/guides/rel_notes/release_21_11.rst |   1 +
>  drivers/net/ice/ice_ethdev.c           |   6 +-
>  drivers/net/ice/ice_rxtx.c             | 107
> +++++++++++++++++++++++++++++++++
>  drivers/net/ice/ice_rxtx.h             |   6 ++
>  drivers/net/ice/ice_rxtx_vec_common.h  |   3 +
>  5 files changed, 122 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/guides/rel_notes/release_21_11.rst
> b/doc/guides/rel_notes/release_21_11.rst
> index dc44739..1b9dac6 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -70,6 +70,7 @@ New Features
>  * **Updated Intel ice driver.**
> 
>    Added 1PPS out support by a devargs.
> +  * Added Rx timstamp support by dynamic mbuf on Flex Descriptor.

How about just "added DEV_RX_OFFLOAD_TIMESTAMP support"

> 
> 
>  Removed Items
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c index
> 76dcabf..06adf43 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -31,6 +31,9 @@
>  #define ICE_HW_DEBUG_MASK_ARG     "hw_debug_mask"
>  #define ICE_ONE_PPS_OUT_ARG       "pps_out"
> 
> +uint64_t ice_timestamp_dynflag;
> +int ice_timestamp_dynfield_offset = -1;
> +
>  static const char * const ice_valid_args[] = {
>  	ICE_SAFE_MODE_SUPPORT_ARG,
>  	ICE_PIPELINE_MODE_SUPPORT_ARG,
> @@ -3672,7 +3675,8 @@ ice_dev_info_get(struct rte_eth_dev *dev, struct
> rte_eth_dev_info *dev_info)
>  			DEV_RX_OFFLOAD_QINQ_STRIP |
>  			DEV_RX_OFFLOAD_OUTER_IPV4_CKSUM |
>  			DEV_RX_OFFLOAD_VLAN_EXTEND |
> -			DEV_RX_OFFLOAD_RSS_HASH;
> +			DEV_RX_OFFLOAD_RSS_HASH |
> +			DEV_RX_OFFLOAD_TIMESTAMP;
>  		dev_info->tx_offload_capa |=
>  			DEV_TX_OFFLOAD_QINQ_INSERT |
>  			DEV_TX_OFFLOAD_IPV4_CKSUM |
> diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c index
> 5d7ab4f..717d3f0 100644
> --- a/drivers/net/ice/ice_rxtx.c
> +++ b/drivers/net/ice/ice_rxtx.c
> @@ -302,6 +302,18 @@ ice_program_hw_rx_queue(struct ice_rx_queue
> *rxq)
>  		}
>  	}
> 
> +	if (rxq->offloads & DEV_RX_OFFLOAD_TIMESTAMP) {
> +		/* Register mbuf field and flag for Rx timestamp */
> +		err = rte_mbuf_dyn_rx_timestamp_register(
> +				&ice_timestamp_dynfield_offset,
> +				&ice_timestamp_dynflag);
> +		if (err != 0) {
> +			PMD_INIT_LOG(ERR,
> +				"Cannot register mbuf field/flag for timestamp");
> +			return -EINVAL;
> +		}
> +	}
> +
>  	memset(&rx_ctx, 0, sizeof(rx_ctx));
> 
>  	rx_ctx.base = rxq->rx_ring_dma / ICE_QUEUE_BASE_ADDR_UNIT; @@
> -354,6 +366,9 @@ ice_program_hw_rx_queue(struct ice_rx_queue *rxq)
>  	regval |= (0x03 << QRXFLXP_CNTXT_RXDID_PRIO_S) &
>  		QRXFLXP_CNTXT_RXDID_PRIO_M;
> 
> +	if (rxq->offloads & DEV_RX_OFFLOAD_TIMESTAMP)
> +		regval |= QRXFLXP_CNTXT_TS_M;
> +
>  	ICE_WRITE_REG(hw, QRXFLXP_CNTXT(rxq->reg_idx), regval);
> 
>  	err = ice_clear_rxq_ctx(hw, rxq->reg_idx); @@ -1530,6 +1545,45 @@
> ice_rxd_to_vlan_tci(struct rte_mbuf *mb, volatile union ice_rx_flex_desc
> *rxdp)
>  		   mb->vlan_tci, mb->vlan_tci_outer);  }
> 
> +uint64_t
> +ice_read_time(struct ice_hw *hw)
> +{
> +	uint32_t hi, lo, lo2;
> +	uint64_t time;
> +
> +	lo = ICE_READ_REG(hw, GLTSYN_TIME_L(0));
> +	hi = ICE_READ_REG(hw, GLTSYN_TIME_H(0));
> +	lo2 = ICE_READ_REG(hw, GLTSYN_TIME_L(0));
> +
> +	if (lo2 < lo) {
> +		lo = ICE_READ_REG(hw, GLTSYN_TIME_L(0));
> +		hi = ICE_READ_REG(hw, GLTSYN_TIME_H(0));
> +	}
> +
> +	time = ((uint64_t)hi << 32) | lo;
> +
> +	return time;
> +}
> +
> +uint64_t
> +ice_tstamp_convert_32b_64b(uint64_t time, uint64_t timestamp) {
> +	const uint64_t mask = 0xFFFFFFFF;
> +	uint32_t delta;
> +	uint64_t ns;
> +
> +	delta = (timestamp - (uint32_t)(time & mask));
> +
> +	if (delta > (mask / 2)) {
> +		delta = ((uint32_t)(time & mask) - timestamp);
> +		ns = time - delta;
> +	} else {
> +		ns = time + delta;
> +	}
> +
> +	return ns;
> +}

Above two helper functions can be merged into one or be wrapped by a new function. 
and all functions can be defined as a static inline function in ice_rxtx.h.

> +
>  #define ICE_LOOK_AHEAD 8
>  #if (ICE_LOOK_AHEAD != 8)
>  #error "PMD ICE: ICE_LOOK_AHEAD must be 8\n"
> @@ -1546,6 +1600,9 @@ ice_rx_scan_hw_ring(struct ice_rx_queue *rxq)
>  	int32_t i, j, nb_rx = 0;
>  	uint64_t pkt_flags = 0;
>  	uint32_t *ptype_tbl = rxq->vsi->adapter->ptype_tbl;
> +	struct ice_vsi *vsi = rxq->vsi;
> +	struct ice_hw *hw = ICE_VSI_TO_HW(vsi);
> +	uint64_t time, ts_ns;
> 
>  	rxdp = &rxq->rx_ring[rxq->rx_tail];
>  	rxep = &rxq->sw_ring[rxq->rx_tail];
> @@ -1589,6 +1646,20 @@ ice_rx_scan_hw_ring(struct ice_rx_queue *rxq)
>  			ice_rxd_to_vlan_tci(mb, &rxdp[j]);
>  			rxq->rxd_to_pkt_fields(rxq, mb, &rxdp[j]);
> 
> +			if (rxq->offloads & DEV_RX_OFFLOAD_TIMESTAMP) {
> +				rxq->time_high =
> +				   rte_le_to_cpu_32(rxdp[j].wb.flex_ts.ts_high);
> +				time = ice_read_time(hw);
> +				ts_ns = ice_tstamp_convert_32b_64b(time,
> +								rxq->time_high);
> +				if (ice_timestamp_dynflag > 0) {
> +					*RTE_MBUF_DYNFIELD(mb,
> +						ice_timestamp_dynfield_offset,
> +						rte_mbuf_timestamp_t *) = ts_ns;
> +					mb->ol_flags |= ice_timestamp_dynflag;
> +				}
> +			}
> +
>  			mb->ol_flags |= pkt_flags;
>  		}
> 
> @@ -1772,6 +1843,9 @@ ice_recv_scattered_pkts(void *rx_queue,
>  	uint64_t dma_addr;
>  	uint64_t pkt_flags;
>  	uint32_t *ptype_tbl = rxq->vsi->adapter->ptype_tbl;
> +	struct ice_vsi *vsi = rxq->vsi;
> +	struct ice_hw *hw = ICE_VSI_TO_HW(vsi);
> +	uint64_t time, ts_ns;
> 
>  	while (nb_rx < nb_pkts) {
>  		rxdp = &rx_ring[rx_id];
> @@ -1882,6 +1956,21 @@ ice_recv_scattered_pkts(void *rx_queue,
>  		ice_rxd_to_vlan_tci(first_seg, &rxd);
>  		rxq->rxd_to_pkt_fields(rxq, first_seg, &rxd);
>  		pkt_flags = ice_rxd_error_to_pkt_flags(rx_stat_err0);
> +
> +		if (rxq->offloads & DEV_RX_OFFLOAD_TIMESTAMP) {
> +			rxq->time_high =
> +			   rte_le_to_cpu_32(rxd.wb.flex_ts.ts_high);
> +			time = ice_read_time(hw);
> +			ts_ns = ice_tstamp_convert_32b_64b(time,
> +							rxq->time_high);
> +			if (ice_timestamp_dynflag > 0) {
> +				*RTE_MBUF_DYNFIELD(first_seg,
> +					ice_timestamp_dynfield_offset,
> +					rte_mbuf_timestamp_t *) = ts_ns;
> +				first_seg->ol_flags |= ice_timestamp_dynflag;
> +			}
> +		}
> +
>  		first_seg->ol_flags |= pkt_flags;
>  		/* Prefetch data of first segment, if configured to do so. */
>  		rte_prefetch0(RTE_PTR_ADD(first_seg->buf_addr,
> @@ -2237,6 +2326,9 @@ ice_recv_pkts(void *rx_queue,
>  	uint64_t dma_addr;
>  	uint64_t pkt_flags;
>  	uint32_t *ptype_tbl = rxq->vsi->adapter->ptype_tbl;
> +	struct ice_vsi *vsi = rxq->vsi;
> +	struct ice_hw *hw = ICE_VSI_TO_HW(vsi);
> +	uint64_t time, ts_ns;
> 
>  	while (nb_rx < nb_pkts) {
>  		rxdp = &rx_ring[rx_id];
> @@ -2288,6 +2380,21 @@ ice_recv_pkts(void *rx_queue,
>  		ice_rxd_to_vlan_tci(rxm, &rxd);
>  		rxq->rxd_to_pkt_fields(rxq, rxm, &rxd);
>  		pkt_flags = ice_rxd_error_to_pkt_flags(rx_stat_err0);
> +
> +		if (rxq->offloads & DEV_RX_OFFLOAD_TIMESTAMP) {
> +			rxq->time_high =
> +			   rte_le_to_cpu_32(rxd.wb.flex_ts.ts_high);
> +			time = ice_read_time(hw);
> +			ts_ns = ice_tstamp_convert_32b_64b(time,
> +							rxq->time_high);
> +			if (ice_timestamp_dynflag > 0) {
> +				*RTE_MBUF_DYNFIELD(rxm,
> +					ice_timestamp_dynfield_offset,
> +					rte_mbuf_timestamp_t *) = ts_ns;
> +				rxm->ol_flags |= ice_timestamp_dynflag;
> +			}
> +		}
> +
>  		rxm->ol_flags |= pkt_flags;
>  		/* copy old mbuf to rx_pkts */
>  		rx_pkts[nb_rx++] = rxm;
> diff --git a/drivers/net/ice/ice_rxtx.h b/drivers/net/ice/ice_rxtx.h index
> b10db08..b3dc80b 100644
> --- a/drivers/net/ice/ice_rxtx.h
> +++ b/drivers/net/ice/ice_rxtx.h
> @@ -40,6 +40,9 @@
> 
>  #define ICE_RXDID_COMMS_OVS	22
> 
> +extern uint64_t ice_timestamp_dynflag;
> +extern int ice_timestamp_dynfield_offset;
> +
>  typedef void (*ice_rx_release_mbufs_t)(struct ice_rx_queue *rxq);  typedef
> void (*ice_tx_release_mbufs_t)(struct ice_tx_queue *txq);  typedef void
> (*ice_rxd_to_pkt_fields_t)(struct ice_rx_queue *rxq, @@ -89,6 +92,7 @@
> struct ice_rx_queue {
>  	ice_rxd_to_pkt_fields_t rxd_to_pkt_fields; /* handle FlexiMD by RXDID */
>  	ice_rx_release_mbufs_t rx_rel_mbufs;
>  	uint64_t offloads;
> +	uint32_t time_high;

Why we need this field?
I saw the only place it is referenced is to set value with rte_le_to_cpu_32(rxdp[j].wb.flex_ts.ts_high and parse it to ice_tstamp_convert_32b_64b

>