[v2] net/ionic: avoid OOB TX partner lookup for hwstamp RXQ

[PATCH net-next v2] net/ionic: avoid OOB TX partner lookup for hwstamp RXQ

Posted by Anand Khoje 1 month, 2 weeks ago

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 <anand.a.khoje@oracle.com>
Reviewed-by: Si-Wei Liu <si-wei.liu@oracle.com>
---
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;
+
 	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);
-- 
2.47.3

Re: [PATCH net-next v2] net/ionic: avoid OOB TX partner lookup for hwstamp RXQ

Posted by Shannon Nelson 1 month, 2 weeks ago

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 <anand.a.khoje@oracle.com>
> Reviewed-by: Si-Wei Liu <si-wei.liu@oracle.com>
> ---
> 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.

> +
>   	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 <sln@onemain.com>

Re: [PATCH net-next v2] net/ionic: avoid OOB TX partner lookup for hwstamp RXQ

Posted by Creeley, Brett 1 month, 1 week ago


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 <anand.a.khoje@oracle.com>
>> Reviewed-by: Si-Wei Liu <si-wei.liu@oracle.com>
>> ---
>> 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 <sln@onemain.com>
>