Re: [Intel-wired-lan] [PATCH iwl-next v6 2/2] ice: add E830 Earliest TxTime First Offload support

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

On Mon, Aug 11, 2025 at 04:44:06AM -0400, Paul Greenwalt wrote:
> E830 supports Earliest TxTime First (ETF) hardware offload, which is
> configured via the ETF Qdisc on a per-queue basis (see tc-etf(8)). ETF
> introduces a new Tx flow mechanism that utilizes a timestamp ring
> (tstamp_ring) alongside the standard Tx ring. This timestamp ring is
> used to indicate when hardware will transmit a packet. Tx Time is
> supported on the first 2048 Tx queues of the device, and the NVM image
> limits the maximum number of Tx queues to 2048 for the device.
> 
> The allocation and initialization of the timestamp ring occur when the
> feature is enabled on a specific Tx queue via tc-etf. The requested Tx
> Time queue index cannot be greater than the number of Tx queues
> (vsi->num_txq).
> 
> To support ETF, the following flags and bitmap are introduced:
> 
>  - ICE_F_TXTIME: Device feature flag set for E830 NICs, indicating ETF
>    support.
>  - txtime_txqs: PF-level bitmap set when ETF is enabled and cleared
>    when disabled for a specific Tx queue. It is used by
>    ice_is_txtime_ena() to check if ETF is allocated and configured on
>    any Tx queue, which is checked during Tx ring allocation.
>  - ICE_TX_FLAGS_TXTIME: Per Tx ring flag set when ETF is allocated and
>    configured for a specific Tx queue. It determines ETF status during
>    packet transmission and is checked by ice_is_txtime_ena() to verify
>    if ETF is enabled on any Tx queue.
> 
> Due to a hardware issue that can result in a malicious driver detection
> event, additional timestamp descriptors are required when wrapping
> around the timestamp ring. Up to 64 additional timestamp descriptors
> are reserved, reducing the available Tx descriptors.
> 
> To accommodate this, ICE_MAX_NUM_DESC_BY_MAC is introduced, defining:
> 
>  - E830: Maximum Tx descriptor count of 8096 (8K - 32 - 64 for timestamp
>    fetch descriptors).
>  - E810 and E82X: Maximum Tx descriptor count of 8160 (8K - 32).
> 
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Co-developed-by: Alice Michael <alice.michael@intel.com>
> Signed-off-by: Alice Michael <alice.michael@intel.com>
> Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice.h          |  33 ++-
>  .../net/ethernet/intel/ice/ice_adminq_cmd.h   |  35 +++
>  drivers/net/ethernet/intel/ice/ice_base.c     | 245 +++++++++++++++---
>  drivers/net/ethernet/intel/ice/ice_base.h     |   1 +
>  drivers/net/ethernet/intel/ice/ice_common.c   |  78 ++++++
>  drivers/net/ethernet/intel/ice/ice_common.h   |   6 +
>  drivers/net/ethernet/intel/ice/ice_ethtool.c  |  14 +-
>  .../net/ethernet/intel/ice/ice_hw_autogen.h   |   3 +
>  .../net/ethernet/intel/ice/ice_lan_tx_rx.h    |  41 +++
>  drivers/net/ethernet/intel/ice/ice_lib.c      |   1 +
>  drivers/net/ethernet/intel/ice/ice_main.c     | 113 +++++++-
>  drivers/net/ethernet/intel/ice/ice_txrx.c     | 171 +++++++++++-
>  drivers/net/ethernet/intel/ice/ice_txrx.h     |  28 +-
>  drivers/net/ethernet/intel/ice/ice_txrx_lib.h |  14 +
>  drivers/net/ethernet/intel/ice/ice_virtchnl.c |   2 +-
>  15 files changed, 730 insertions(+), 55 deletions(-)
> 

(...)

>  
> +/**
> + * ice_cfg_txtime - configure Tx Time for the Tx ring
> + * @tx_ring: pointer to the Tx ring structure
> + *
> + * Return: 0 on success, negative value on failure.
> + */
> +static int ice_cfg_txtime(struct ice_tx_ring *tx_ring)
> +{
> +	int err, timeout = 50;
> +	struct ice_vsi *vsi;
> +	struct device *dev;
> +	struct ice_pf *pf;
> +	u32 queue;
> +
> +	if (!tx_ring)
> +		return -EINVAL;
> +
> +	vsi = tx_ring->vsi;
> +	pf = vsi->back;
> +	while (test_and_set_bit(ICE_CFG_BUSY, pf->state)) {
> +		timeout--;
> +		if (!timeout)
> +			return -EBUSY;
> +		usleep_range(1000, 2000);
> +	}
> +
> +	queue = tx_ring->q_index;
> +	dev = ice_pf_to_dev(pf);
> +	err = ice_qp_dis(vsi, queue);
> +	if (err) {
> +		dev_err(dev, "Failed to disable Tx queue %d for TxTime configuration\n",
> +			queue);
> +		goto exit;

nit: in this case you leave queue pair in limbo state. i would be trying
to bring it up regardless whether disable succeeded.

> +	}
> +
> +	err = ice_qp_ena(vsi, queue);
> +	if (err)
> +		dev_err(dev, "Failed to enable Tx queue %d for TxTime configuration\n",
> +			queue);
> +
> +exit:
> +	clear_bit(ICE_CFG_BUSY, pf->state);
> +	return err;
> +}
> +

(...)

>  
>  dma_error:
> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.h b/drivers/net/ethernet/intel/ice/ice_txrx.h
> index 2fd8e78178a2..be74851eadd4 100644
> --- a/drivers/net/ethernet/intel/ice/ice_txrx.h
> +++ b/drivers/net/ethernet/intel/ice/ice_txrx.h
> @@ -310,6 +310,12 @@ enum ice_dynamic_itr {
>  #define ICE_TX_LEGACY	1
>  
>  /* descriptor ring, associated with a VSI */
> +struct ice_tstamp_ring {
> +	struct ice_tx_ring *tx_ring;	/* Backreference to associated Tx ring */
> +	dma_addr_t dma;			/* physical address of ring */
> +	struct rcu_head rcu;            /* to avoid race on free */
> +} ____cacheline_internodealigned_in_smp;
> +
>  struct ice_rx_ring {
>  	/* CL1 - 1st cacheline starts here */
>  	void *desc;			/* Descriptor ring memory */
> @@ -387,11 +393,22 @@ struct ice_tx_ring {
>  	struct xsk_buff_pool *xsk_pool;
>  	u16 next_to_use;
>  	u16 next_to_clean;
> +	u16 tstamp_next_to_use;		/* Time stamp ring next to use */
> +	u16 tstamp_count;		/* Time stamp ring descriptors count */
> +	u8 __iomem *tstamp_tail;	/* Time stamp ring tail pointer */
> +	void *tstamp_desc;		/* Time stamp descriptor ring memory */

we spoke about putting these onto ice_tstamp_ring...if i am reading this
right you want to avoid touching ice_tstamp_ring in hot-path - is that
correct?

if that ring is etf-enabled then does it ever process the normal tx
traffic? what i'm trying to ask is whether you considered putting these
members onto union with next_to_use, count and *desc.

how different is layout of ice_tx_ring after your change?

rest of the code looks good to me now, however i still would like to
clarify things mentioned above.

>  	u16 q_handle;			/* Queue handle per TC */
>  	u16 reg_idx;			/* HW register index of the ring */
>  	u16 count;			/* Number of descriptors */
>  	u16 q_index;			/* Queue number of ring */
>  	u16 xdp_tx_active;
> +	u16 quanta_prof_id;
> +	u8 dcb_tc;			/* Traffic class of ring */
> +#define ICE_TX_FLAGS_RING_XDP		BIT(0)
> +#define ICE_TX_FLAGS_RING_VLAN_L2TAG1	BIT(1)
> +#define ICE_TX_FLAGS_RING_VLAN_L2TAG2	BIT(2)
> +#define ICE_TX_FLAGS_TXTIME		BIT(3)
> +	u8 flags;
>  	/* stats structs */
>  	struct ice_ring_stats *ring_stats;
>  	/* CL3 - 3rd cacheline starts here */
> @@ -401,13 +418,7 @@ struct ice_tx_ring {
>  	struct ice_ptp_tx *tx_tstamps;
>  	spinlock_t tx_lock;
>  	u32 txq_teid;			/* Added Tx queue TEID */
> -	/* CL4 - 4th cacheline starts here */
> -#define ICE_TX_FLAGS_RING_XDP		BIT(0)
> -#define ICE_TX_FLAGS_RING_VLAN_L2TAG1	BIT(1)
> -#define ICE_TX_FLAGS_RING_VLAN_L2TAG2	BIT(2)
> -	u8 flags;
> -	u8 dcb_tc;			/* Traffic class of ring */
> -	u16 quanta_prof_id;
> +	struct ice_tstamp_ring *tstamp_ring;
>  } ____cacheline_internodealigned_in_smp;
>