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 <[email protected]>
> Co-developed-by: Alice Michael <[email protected]>
> Signed-off-by: Alice Michael <[email protected]>
> Signed-off-by: Paul Greenwalt <[email protected]>
> ---
> 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;
>