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;
>  

Reply via email to