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]>
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.