Re: [PATCH net-next v2] net/ionic: avoid OOB TX partner lookup for hwstamp RXQ
"Creeley, Brett" <[email protected]>
| Newsgroups | org.kernel.vger.stable,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
On 8/12/2026 10:52 PM, Shannon Nelson wrote:
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> On 8/12/26 22:13, Anand Khoje wrote:
>> The dedicated hardware timestamp RX queue is allocated with q->index
>> equal to lif->ionic->nrxqs_per_lif. The normal txqcqs array only
>> contains the regular queue pairs, so using that index to set
>> rxq->partner
>> can read one entry past txqcqs[] and then write through the derived
>> pointer.
>> Only link RX/TX partners for normal queue-pair indexes. Leave the
>> hwstamp
>> RX queue unpaired, and make the XDP_TX path abort cleanly if an RX queue
>> has no TX partner.
>>
>> Fixes: 8eeed8373e1c ("ionic: Add XDP_TX support")
>> Signed-off-by: Anand Khoje <[email protected]>
>> Reviewed-by: Si-Wei Liu <[email protected]>
>> ---
>> v2:
>> Fixed the Fixes tag.
>>
>> drivers/net/ethernet/pensando/ionic/ionic_lif.c | 14
>> ++++++++++++++
>> drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 5 ++++-
>> 2 files changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
>> b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
>> index fd3ee9820531..1822361e1070 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
>> @@ -920,9 +920,23 @@ static int ionic_lif_rxq_init(struct ionic_lif
>> *lif, struct ionic_qcq *qcq)
>> };
>> int err;
>>
>> + q->partner = NULL;
>> +
>> + /* Only normal RX queues have matching TX queue partners,
>> + * skip partner pairing for hwstamp RX queue.
>> + */
>> + if (q->index >= lif->nxqs)
>> + goto skip_partner;
>> +
>> + if (WARN_ON_ONCE(!lif->txqcqs ||
>> + q->index >= lif->ionic->ntxqs_per_lif ||
>> + !lif->txqcqs[q->index]))
>> + return -EINVAL;
>
> This driver doesn't have any other WARN type statements, and the WARN*
> is more often frowned upon now that so many places use
> panic_on_warn. The other place where this kind of check is done simply
> prints a dev_err() and returns -ENXIO - see ionic_txrx_enable(). Brett
> or Eric J might have another opinion, but you might stick with that
> practice here.
Thanks for pointing this out Shannon. I agree and would like to keep the
driver consistent regarding not using WARN statements.
Brett
>
>> +
>> q->partner = &lif->txqcqs[q->index]->q;
>> q->partner->partner = q;
>>
>> +skip_partner:
>> if (!lif->xdp_prog ||
>> (lif->xdp_prog->aux && lif->xdp_prog->aux->xdp_has_frags))
>> ctx.cmd.q_init.flags |= cpu_to_le16(IONIC_QINIT_F_SG);
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
>> b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
>> index 301ebee2fdc5..73998d61593 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
>> @@ -543,13 +543,18 @@ static bool ionic_run_xdp(struct ionic_rx_stats
>> *stats,
>> break;
>>
>> case XDP_TX:
>> + txq = rxq->partner;
>> + if (unlikely(!txq)) {
>> + err = -EIO;
>> + break;
>> + }
>> +
>> xdpf = xdp_convert_buff_to_frame(&xdp_buf);
>> if (!xdpf) {
>> err = -ENOSPC;
>> break;
>> }
>>
>> - txq = rxq->partner;
>> nq = netdev_get_tx_queue(netdev, txq->index);
>> __netif_tx_lock(nq, smp_processor_id());
>> txq_trans_cond_update(nq);
>
> Other than the above, this looks fine to me.
> Reviewed-by: Shannon Nelson <[email protected]>
>