[next,S77-V3,08/10] i40evf: fix ring to vector mapping

Submitted by alice michael on July 14, 2017, 1:27 p.m.

Details

Message ID 20170714132709.52657-8-alice.michael@intel.com
State Under Review
Delegated to: Jeff Kirsher
Headers show

Commit Message

alice michael July 14, 2017, 1:27 p.m.
From: Alan Brady <alan.brady@intel.com>

The current implementation for mapping queues to vectors is broken
because it attempts to map each tx and rx ring to its own vector,
however we use combined queues so we should actually be mapping the
tx/rx rings together on one vector.

Also in the current implementation, in the case where we have more
queues than vectors, we attempt to group the queues together into
'chunks' and map each 'chunk' of queues to a vector.  Chunking them
together would be more ideal if, and only if, we only had RSS because of
the way the hashing algorithm works but in the case of a future patch
that enables VF ADq, round robin assignment is better and still works
with RSS.

This patch resolves both those issues and simplifies the code needed to
accomplish this.  Instead of treating the case where we have more queues
than vectors as special, if we notice our vector index is greater than
vectors, reset the vector index to zero and continue mapping.  This
should ensure that in both cases, whether we have enough vectors for
each queue or not, the queues get appropriately mapped.

Testing-hints:
    Make sure traffic continues to be received and features like RSS
    continue to work as expected.

Signed-off-by: Alan Brady <alan.brady@intel.com>
---
 drivers/net/ethernet/intel/i40evf/i40evf_main.c | 48 ++++++-------------------
 1 file changed, 10 insertions(+), 38 deletions(-)

Comments

Bowers, AndrewX July 21, 2017, 8:47 p.m.
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On
> Behalf Of Alice Michael
> Sent: Friday, July 14, 2017 6:27 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan@lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S77-V3 08/10] i40evf: fix ring to vector
> mapping
> 
> From: Alan Brady <alan.brady@intel.com>
> 
> The current implementation for mapping queues to vectors is broken
> because it attempts to map each tx and rx ring to its own vector, however we
> use combined queues so we should actually be mapping the tx/rx rings
> together on one vector.
> 
> Also in the current implementation, in the case where we have more queues
> than vectors, we attempt to group the queues together into 'chunks' and
> map each 'chunk' of queues to a vector.  Chunking them together would be
> more ideal if, and only if, we only had RSS because of the way the hashing
> algorithm works but in the case of a future patch that enables VF ADq, round
> robin assignment is better and still works with RSS.
> 
> This patch resolves both those issues and simplifies the code needed to
> accomplish this.  Instead of treating the case where we have more queues
> than vectors as special, if we notice our vector index is greater than vectors,
> reset the vector index to zero and continue mapping.  This should ensure
> that in both cases, whether we have enough vectors for each queue or not,
> the queues get appropriately mapped.
> 
> Testing-hints:
>     Make sure traffic continues to be received and features like RSS
>     continue to work as expected.
> 
> Signed-off-by: Alan Brady <alan.brady@intel.com>
> ---
>  drivers/net/ethernet/intel/i40evf/i40evf_main.c | 48 ++++++------------------
> -
>  1 file changed, 10 insertions(+), 38 deletions(-)


Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

Patch hide | download patch | download mbox

diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index d8e17c2..551fba6 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -432,52 +432,24 @@  i40evf_map_vector_to_txq(struct i40evf_adapter *adapter, int v_idx, int t_idx)
  **/
 static int i40evf_map_rings_to_vectors(struct i40evf_adapter *adapter)
 {
+	int rings_remaining = adapter->num_active_queues;
+	int ridx = 0, vidx = 0;
 	int q_vectors;
-	int v_start = 0;
-	int rxr_idx = 0, txr_idx = 0;
-	int rxr_remaining = adapter->num_active_queues;
-	int txr_remaining = adapter->num_active_queues;
-	int i, j;
-	int rqpv, tqpv;
 	int err = 0;
 
 	q_vectors = adapter->num_msix_vectors - NONQ_VECS;
 
-	/* The ideal configuration...
-	 * We have enough vectors to map one per queue.
-	 */
-	if (q_vectors >= (rxr_remaining * 2)) {
-		for (; rxr_idx < rxr_remaining; v_start++, rxr_idx++)
-			i40evf_map_vector_to_rxq(adapter, v_start, rxr_idx);
+	for (; ridx < rings_remaining; ridx++) {
+		i40evf_map_vector_to_rxq(adapter, vidx, ridx);
+		i40evf_map_vector_to_txq(adapter, vidx, ridx);
 
-		for (; txr_idx < txr_remaining; v_start++, txr_idx++)
-			i40evf_map_vector_to_txq(adapter, v_start, txr_idx);
-		goto out;
-	}
-
-	/* If we don't have enough vectors for a 1-to-1
-	 * mapping, we'll have to group them so there are
-	 * multiple queues per vector.
-	 * Re-adjusting *qpv takes care of the remainder.
-	 */
-	for (i = v_start; i < q_vectors; i++) {
-		rqpv = DIV_ROUND_UP(rxr_remaining, q_vectors - i);
-		for (j = 0; j < rqpv; j++) {
-			i40evf_map_vector_to_rxq(adapter, i, rxr_idx);
-			rxr_idx++;
-			rxr_remaining--;
-		}
-	}
-	for (i = v_start; i < q_vectors; i++) {
-		tqpv = DIV_ROUND_UP(txr_remaining, q_vectors - i);
-		for (j = 0; j < tqpv; j++) {
-			i40evf_map_vector_to_txq(adapter, i, txr_idx);
-			txr_idx++;
-			txr_remaining--;
-		}
+		/* In the case where we have more queues than vectors, continue
+		 * round-robin on vectors until all queues are mapped.
+		 */
+		if (++vidx >= q_vectors)
+			vidx = 0;
 	}
 
-out:
 	adapter->aq_required |= I40EVF_FLAG_AQ_MAP_VECTORS;
 
 	return err;