On Tue, Oct 13, 2026 at 3:44 AM Tathagat Priyadarshi
<[email protected]> wrote:
>
> The DPDK GVE driver previously tracked transmitted packet count (re_cnt)
> at the end of each burst rather than tracking descriptor intervals per
> packet against GVE_TX_MIN_RE_INTERVAL (32).
>
> In multi-segment and TSO transmissions, each packet consumes multiple
> descriptors. The descriptor ring could exhaust before 32 packets were
> sent, resulting in zero descriptors requesting report_event and
> preventing the hardware DMA engine from returning GVE_COMPL_TYPE_DQO_DESC
> completions. Without these completions, txq->nb_free was never
> replenished, permanently stalling the TX queue.
>
> Align with the Linux kernel driver (gve_tx_update_tail()) by tracking
> descriptor indices with last_re_idx:
> - Replace re_cnt with last_re_idx in struct gve_tx_queue.
> - Reset last_re_idx = 0 in gve_reset_tx_ring_state_dqo().
> - On each packet transmission, check descriptor interval
>   (last_desc_idx - last_re_idx) & mask >= GVE_TX_MIN_RE_INTERVAL.
>   When met, set report_event on the packet's last descriptor and
>   update last_re_idx.
> - Ring the transmit doorbell immediately upon setting report_event to
>   kick hardware processing early and reduce device fetch latency during
>   bursts of multi-descriptor TSO packets, amortizing MMIO writes over
>   RE_INTERVAL descriptors.
> - Defensively pop completion tags before writing TSO context descriptors
>   to avoid partial descriptor ring updates if completion tags are
>   exhausted.
>
> Fixes: 4022f9999f56 ("net/gve: support basic Tx data path for DQO")
> Fixes: 403c671a46b6 ("net/gve: support TSO in DQO RDA")
> Cc: [email protected]
>
> Signed-off-by: Tathagat Priyadarshi <[email protected]>
> ---
>  drivers/net/gve/gve_ethdev.h |  2 +-
>  drivers/net/gve/gve_tx_dqo.c | 46 ++++++++++++++++++++----------------
>  2 files changed, 27 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
> index c9a176ff17..2033f49444 100644
> --- a/drivers/net/gve/gve_ethdev.h
> +++ b/drivers/net/gve/gve_ethdev.h
> @@ -193,8 +193,8 @@ struct gve_tx_queue {
>         uint16_t sw_size;
>         uint8_t cur_gen_bit;
>         uint32_t last_desc_cleaned;
> +       uint32_t last_re_idx;
>         void **txqs;
> -       uint16_t re_cnt;
>
>         /* Only valid for DQO_RDA queue format */
>         struct gve_tx_queue *complq;
> diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c
> index c864d722d9..0839179d7f 100644
> --- a/drivers/net/gve/gve_tx_dqo.c
> +++ b/drivers/net/gve/gve_tx_dqo.c
> @@ -250,21 +250,23 @@ gve_fill_tx_pkt_desc(struct gve_tx_queue *txq, uint16_t 
> *tx_id,
>  uint16_t
>  gve_tx_burst_dqo(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
>  {
> +       volatile union gve_tx_desc_dqo *txr, *txd;
>         struct gve_tx_queue *txq = tx_queue;
> -       volatile union gve_tx_desc_dqo *txr;
> -       volatile union gve_tx_desc_dqo *txd;
> -       uint16_t mask = txq->nb_tx_desc - 1;
> -       struct gve_tx_pkt *pkts;
> +       uint32_t last_re_interval;
>         struct rte_mbuf *tx_pkt;
> -       uint16_t compl_tag;
> -       const char *reason;
> +       struct gve_tx_pkt *pkts;
> +       uint32_t last_desc_idx;
>         uint16_t nb_tx = 0;
>         uint64_t bytes = 0;
> +       const char *reason;
> +       uint16_t compl_tag;
>         uint64_t ol_flags;
>         uint16_t nb_descs;
> -       bool csum, tso;
>         uint16_t tx_id;
> +       bool csum, tso;
> +       uint16_t mask;

It's kind of hard to tell from first glance, given how many
declarations are here and how many are changing, but are all of these
changes necessary for a fix? Things like moving txr, txd to the same
line or separating the declaration and definition of mask, even though
txq is already defined in an earlier line anyway seem unnecessary at
first glance. There might be other such changes, but I stopped there,
since those are in the first 5 lines of delta. Let's try to keep the
fix focused, only making changes that are actually necessary.

>
> +       mask = txq->nb_tx_desc - 1;
>         pkts = txq->pkt_ring_dqo;
>         txr = txq->tx_ring;
>
> @@ -306,15 +308,15 @@ gve_tx_burst_dqo(void *tx_queue, struct rte_mbuf 
> **tx_pkts, uint16_t nb_pkts)
>                         break;
>                 }
>
> +               if (!gve_free_compl_tags_pop(txq, &compl_tag))
> +                       break;
> +
>                 if (tso) {
>                         txd = &txr[tx_id];
>                         gve_tx_fill_seg_desc_dqo(txd, tx_pkt);
>                         tx_id = (tx_id + 1) & mask;
>                 }
>
> -               if (!gve_free_compl_tags_pop(txq, &compl_tag))
> -                       break;
> -
>                 pkts[compl_tag].mbuf = tx_pkt;
>                 while (tx_pkt) {
>                         /* Skip writing descriptors if mbuf has no data. */
> @@ -331,23 +333,26 @@ gve_tx_burst_dqo(void *tx_queue, struct rte_mbuf 
> **tx_pkts, uint16_t nb_pkts)
>                 txd = &txr[(tx_id - 1) & mask];
>                 txd->pkt.end_of_packet = 1;
>
> +               last_desc_idx = (tx_id - 1) & mask;
> +               last_re_interval = (last_desc_idx - txq->last_re_idx) & mask;
> +               if (unlikely(last_re_interval >= GVE_TX_MIN_RE_INTERVAL)) {
> +                       txd->pkt.report_event = true;
> +                       txq->last_re_idx = last_desc_idx;
> +                       rte_write32(tx_id, txq->qtx_tail);
> +                       txq->tx_tail = tx_id;
> +               }
> +
>                 txq->nb_free -= nb_descs;
>                 txq->nb_used += nb_descs;
>         }
>
> -       /* update the tail pointer if any packets were processed */
> -       if (nb_tx > 0) {
> -               /* Request a descriptor completion on the last descriptor */
> -               txq->re_cnt += nb_tx;
> -               if (txq->re_cnt >= GVE_TX_MIN_RE_INTERVAL) {
> -                       txd = &txr[(tx_id - 1) & mask];
> -                       txd->pkt.report_event = true;
> -                       txq->re_cnt = 0;
> -               }
> -
> +       /* update the tail pointer if any uncommitted descriptors remain */
> +       if (txq->tx_tail != tx_id) {
>                 rte_write32(tx_id, txq->qtx_tail);
>                 txq->tx_tail = tx_id;
> +       }
>
> +       if (nb_tx > 0) {
>                 txq->stats.packets += nb_tx;
>                 txq->stats.bytes += bytes;
>                 txq->stats.errors += nb_pkts - nb_tx;
> @@ -445,6 +450,7 @@ gve_reset_tx_ring_state_dqo(struct gve_tx_queue *txq)
>         txq->nb_used = 0;
>
>         txq->last_desc_cleaned = 0;
> +       txq->last_re_idx = 0;
>         txq->nb_free = txq->nb_tx_desc - 1;
>
>         txq->complq_tail = 0;
> --
> 2.43.0
>

Reply via email to