net/netvsc: fix number Tx queues > Rx queues
PATCH v5 1/1 net/netvsc: fix number Tx queues > Rx queues
From: Alan Elder <[email protected]>
The previous code allowed the number of Tx queues to be set higher than the number of Rx queues. If a packet was sent on a Tx queue with index
Quoted 1 line
= number Rx queues there was a segfault due to accessing beyond the end
of the dev->data->rx_queues[] array.
This commit fixes the issue by creating an Rx queue for every Tx queue meaning that an event buffer is allocated to handle receiving Tx completion messages.
mbuf pool and Rx ring are not allocated for these additional Rx queues and RSS configuration ensures that no packets are received on them.
---
v5:
* Resend/fixed up the last verison of the patch garbled in patchwork
v4:
* Include segfault core stack in commit message
v3:
* Handle case of Rx queue creation failure in hn_dev_tx_queue_setup.
* Re-use rx queue if it has already been allocated.
* Don't allocate an mbuf if pool is NULL. This avoids segfault if RSS configuration is incorrect.
v2:
* Remove function declaration for static non-member function
drivers/net/netvsc/hn_ethdev.c | 9 +++++ drivers/net/netvsc/hn_rxtx.c | 68 +++++++++++++++++++++++++++++----- 2 files changed, 68 insertions(+), 9 deletions(-)
drivers/net/netvsc/hn_ethdev.c +9 -0
@@ -313,6 +313,15 @@ static int hn_rss_reta_update(struct rte_eth_dev *dev,if (reta_conf[idx].mask & mask) hv->rss_ind[i] = reta_conf[idx].reta[shift]; + +/* +* Ensure we don't allow config that directs traffic to an Rx +* queue that we aren't going to poll +*/ +if (hv->rss_ind[i] >= dev->data->nb_rx_queues) { +PMD_DRV_LOG (ERR ,"RSS distributing traffic to invalid Rx queue" ); +return -EINVAL ; + } } err =hn_rndis_conf_rss (hv,NDIS_RSS_FLAG_DISABLE );
drivers/net/netvsc/hn_rxtx.c +59 -9
130 lines not on this page · raw message
Signature · 1 line
2.25.1
drivers/net/netvsc/hn_rxtx.c static void hn_reset_txagg(struct hn_tx_queue *txq) · line 233
Quoted 9 lines · On Thu, 17 Oct 2024 12:20:29 -0700 [email protected] wrote:
drivers/net/netvsc/hn_rxtx.c +9
-0+static void +hn_rx_queue_free_common (struct hn_rx_queue *rxq) { +if (!rxq) +return ; + +rte_free (rxq->rxbuf_info); +rte_free (rxq->event_buf); +rte_free (rxq); +}
Minor nit, DPDK style is for the bracket on the next line. Checkpatch will complain about this.
Quoted 5 lines
Subject: Re: [Patch v5] net/netvsc: fix number Tx queues > Rx queues
On Thu, 17 Oct 2024 12:20:29 -0700
[email protected] wrote:
Quoted 12 lines
drivers/net/netvsc/hn_rxtx.c static void hn_reset_txagg(struct hn_tx_queue *txq) · line 233
Quoted 9 lines
drivers/net/netvsc/hn_rxtx.c +9
-0+static void +hn_rx_queue_free_common (struct hn_rx_queue *rxq) { +if (!rxq) +return ; + +rte_free (rxq->rxbuf_info); +rte_free (rxq->event_buf); +rte_free (rxq); +}Minor nit, DPDK style is for the bracket on the next line. Checkpatch will complain about this.
Is it okay to take the patch as is, or should I send a v6 to have it fixed?
Quoted 18 lines · On Thu, 17 Oct 2024 22:38:49 +0000 Long Li <[email protected]> wrote:
Quoted 5 lines
Subject: Re: [Patch v5] net/netvsc: fix number Tx queues > Rx queues
On Thu, 17 Oct 2024 12:20:29 -0700
[email protected] wrote:Quoted 12 lines
drivers/net/netvsc/hn_rxtx.c static void hn_reset_txagg(struct hn_tx_queue *txq) · line 233
Quoted 9 lines
drivers/net/netvsc/hn_rxtx.c +9
-0+static void +hn_rx_queue_free_common (struct hn_rx_queue *rxq) { +if (!rxq) +return ; + +rte_free (rxq->rxbuf_info); +rte_free (rxq->event_buf); +rte_free (rxq); +}Minor nit, DPDK style is for the bracket on the next line. Checkpatch will complain about this.
Quoted 1 line
Is it okay to take the patch as is, or should I send a v6 to have it fixed?
It is ok as is, but followup to fix the minor stuff like this would be good.
Quoted 21 lines · Stephen Hemminger
Quoted 18 lines · On Thu, 17 Oct 2024 22:38:49 +0000 Long Li <[email protected]> wrote:
Quoted 17 lines
Subject: Re: [Patch v5] net/netvsc: fix number Tx queues > Rx queues
drivers/net/netvsc/hn_rxtx.c static void hn_reset_txagg(struct hn_tx_queue *txq) · line 233
Quoted 9 lines · On Thu, 17 Oct 2024 12:20:29 -0700 [email protected] wrote:
drivers/net/netvsc/hn_rxtx.c +9
-0+static void +hn_rx_queue_free_common (struct hn_rx_queue *rxq) { +if (!rxq) +return ; + +rte_free (rxq->rxbuf_info); +rte_free (rxq->event_buf); +rte_free (rxq); +}Minor nit, DPDK style is for the bracket on the next line. Checkpatch will complain about this.
Quoted 4 lines
Quoted 1 line
Is it okay to take the patch as is, or should I send a v6 to have it fixed?
It is ok as is, but followup to fix the minor stuff like this would be good.
I will fix this one while merging.
Recheck-request: intel-Functional, intel-Testing
Ferruh Yigit
AppliedQuoted 14 lines · On 10/17/2024 8:20 PM, [email protected] wrote:
From: Alan Elder <[email protected]>
The previous code allowed the number of Tx queues to be set higher than the number of Rx queues. If a packet was sent on a Tx queue with index
Quoted 1 line
= number Rx queues there was a segfault due to accessing beyond the end
of the dev->data->rx_queues[] array.
This commit fixes the issue by creating an Rx queue for every Tx queue meaning that an event buffer is allocated to handle receiving Tx completion messages.
mbuf pool and Rx ring are not allocated for these additional Rx queues and RSS configuration ensures that no packets are received on them.
Quoted 7 lines
Fixes 4e9c73e96e83 ("net/netvsc: add Hyper-V network device")Signed-off-by Alan Elder <[email protected]>Signed-off-by Long Li <[email protected]>
Applied to dpdk-next-net/main, thanks.
Checkpatch warning fixed while merging.