> > Replace the TxQ buffer ring with one large pool/array of buffers (only
> > for flow scheduling). The completion tag passed to HW through the
>
> [...]
>
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> > index cdecf558d7ec..25eea632a966 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> > @@ -13,6 +13,7 @@ struct idpf_tx_stash {
> > struct libeth_sqe buf;
> > };
> >
> > +#define idpf_tx_buf_next(buf) (*(u32 *)&(buf)->priv)
>
> Align it to the next line, i.e. 2 tabs instead of 2 spaces.
Will fix in v2
>
> > #define idpf_tx_buf_compl_tag(buf) (*(u32 *)&(buf)->priv)
> > LIBETH_SQE_CHECK_PRIV(u32);
> >
> > @@ -91,7 +92,7 @@ static void idpf_tx_buf_rel_all(struct idpf_tx_queue
> *txq)
> > return;
> >
> > /* Free all the Tx buffer sk_buffs */
> > - for (i = 0; i < txq->desc_count; i++)
> > + for (i = 0; i < txq->buf_pool_size; i++)
> > libeth_tx_complete(&txq->tx_buf[i], &cp);
> >
> > kfree(txq->tx_buf);
> > @@ -205,7 +206,11 @@ static int idpf_tx_buf_alloc_all(struct
> idpf_tx_queue *tx_q)
> > /* Allocate book keeping buffers only. Buffers to be supplied to HW
> > * are allocated by kernel network stack and received as part of skb
> > */
> > - buf_size = sizeof(struct idpf_tx_buf) * tx_q->desc_count;
> > + if (idpf_queue_has(FLOW_SCH_EN, tx_q))
> > + tx_q->buf_pool_size = U16_MAX;
>
> 3.2 Mb per queue... OTOH 1 Rx queue with 512 descriptors eats 2.1 Mb,
> not that bad.
>
> > + else
> > + tx_q->buf_pool_size = tx_q->desc_count;
> > + buf_size = sizeof(struct idpf_tx_buf) * tx_q->buf_pool_size;
>
> array_size() if you really want, but the proper way would be to replace
> the kzalloc() below with kcalloc().
Will fix in v2
>
> > tx_q->tx_buf = kzalloc(buf_size, GFP_KERNEL);
> > if (!tx_q->tx_buf)
> > return -ENOMEM;
>
> [...]
>
> > +static bool idpf_tx_clean_bufs(struct idpf_tx_queue *txq, u16 buf_id,
>
> Just use u32 when it comes to function arguments and onstack variables.
Will fix in v2
>
> > + struct libeth_sq_napi_stats *cleaned,
> > + int budget)
> > {
> > - u16 idx = compl_tag & txq->compl_tag_bufid_m;
> > + u16 idx = buf_id & txq->compl_tag_bufid_m;
> > struct idpf_tx_buf *tx_buf = NULL;
> > struct libeth_cq_pp cp = {
> > .dev = txq->dev,
>
> [...]
>
> > if (idpf_queue_has(FLOW_SCH_EN, tx_q)) {
> > if (unlikely(!idpf_tx_get_free_buf_id(tx_q->refillq,
> > &tx_params.compl_tag)))
> > return idpf_tx_drop_skb(tx_q, skb);
> > + buf_id = tx_params.compl_tag;
>
> So this field in tx_params needs to be renamed as it no longer reflects
> its purpose.
I would prefer to keep the compl_tag name. The params struct represents what
goes into the descriptors and from a descriptor perspective, its purpose has
not changed. The compl_tag must be the same across all descriptors for a given
packet. But we can use multiple buf_ids per packet.
However, I can swap it so we pass &buf_id to idpf_tx_get_free_buf_id and assign
it to tx_params.compl_tag.
>
> >
> > tx_params.dtype = IDPF_TX_DESC_DTYPE_FLEX_FLOW_SCHE;
> > tx_params.eop_cmd = IDPF_TXD_FLEX_FLOW_CMD_EOP;
>
> Thanks,
> Olek
Thanks,
Josh
