RE: [PATCH v2] net/netvsc: fix number Tx queues > Rx queues

[Long Li <longli@microsoft.com>]

> a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c index
> 9bf1ec5509..c0aaeaa972 100644
> --- a/drivers/net/netvsc/hn_rxtx.c
> +++ b/drivers/net/netvsc/hn_rxtx.c
> @@ -243,6 +243,7 @@ hn_dev_tx_queue_setup(struct rte_eth_dev *dev,  {
>  	struct hn_data *hv = dev->data->dev_private;
>  	struct hn_tx_queue *txq;
> +	struct hn_rx_queue *rxq;
>  	char name[RTE_MEMPOOL_NAMESIZE];
>  	uint32_t tx_free_thresh;
>  	int err = -ENOMEM;
> @@ -301,6 +302,22 @@ hn_dev_tx_queue_setup(struct rte_eth_dev *dev,
>  		goto error;
>  	}
> 
> +	/*
> +	 * If there are more Tx queues than Rx queues, allocate rx_queues
> +	 * with event buffer so that Tx completion messages can still be
> +	 * received
> +	 */
> +	if (queue_idx >= dev->data->nb_rx_queues) {
> +		rxq = hn_rx_queue_alloc(hv, queue_idx, socket_id);

Need to check if rxq is NULL.

> +		/*
> +		 * Don't allocate mbuf pool or rx ring.  RSS is always configured
> +		 * to ensure packets aren't received by this Rx queue.
> +		 */
> +		rxq->mb_pool = NULL;
> +		rxq->rx_ring = NULL;
> +		dev->data->rx_queues[queue_idx] = rxq;
> +	}
> +
>  	txq->agg_szmax  = RTE_MIN(hv->chim_szmax, hv->rndis_agg_size);
>  	txq->agg_pktmax = hv->rndis_agg_pkts;
>  	txq->agg_align  = hv->rndis_agg_align; @@ -354,6 +371,17 @@ static
> void hn_txd_put(struct hn_tx_queue *txq, struct hn_txdesc *txd)
>  	rte_mempool_put(txq->txdesc_pool, txd);  }
> 
> +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);
> +}
> +
>  void
>  hn_dev_tx_queue_release(struct rte_eth_dev *dev, uint16_t qid)  { @@ -364,6
> +392,13 @@ hn_dev_tx_queue_release(struct rte_eth_dev *dev, uint16_t qid)
>  	if (!txq)
>  		return;
> 
> +	/*
> +	 * Free any Rx queues allocated for a Tx queue without a corresponding
> +	 * Rx queue
> +	 */
> +	if (qid >= dev->data->nb_rx_queues)
> +		hn_rx_queue_free_common(dev->data->rx_queues[qid]);
> +
>  	rte_mempool_free(txq->txdesc_pool);
> 
>  	rte_memzone_free(txq->tx_rndis_mz);
> @@ -942,6 +977,13 @@ hn_dev_rx_queue_setup(struct rte_eth_dev *dev,
>  	if (queue_idx == 0) {
>  		rxq = hv->primary;
>  	} else {
> +		/*
> +		 * If the number of Tx queues was previously greater than
> +		 * the number of Rx queues, we may already have allocated
> +		 * an rxq. If so, free it now before allocating a new one.
> +		 */
> +		hn_rx_queue_free_common(dev->data-
> >rx_queues[queue_idx]);

This logic seems strange. How about check if rxq is already allocated. If not, allocate it.

Something like:

if (!dev->data->rx_queues[queue_idx])
	rxq = hn_rx_queue_alloc(hv, queue_idx, socket_id);



Thanks,

Long