pktgen: fix multiple queue warning

PATCH New Jeff Kirsher 3 participants 8 messages Oct 28, 2008 – Nov 19, 2008

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Jeff Kirsher

author

2008-10-28 19:31 UTC #0 lore raw

PATCH 1/1 pktgen: fix multiple queue warning

From: Jesse Brandeburg <[email protected]>

when testing the new pktgen module with multiple queues and ixgbe with:
pgset "flag QUEUE_MAP_CPU"

I found that I was getting errors in dmesg like:
pktgen: WARNING: QUEUE_MAP_CPU disabled because CPU count (8) exceeds number <4>pktgen: WARNING: of tx queues (8) on eth15

you'll note, 8 really doesn't exceed 8.

This patch seemed to fix the logic errors and also the attempts at limiting line length in printk (which didn't work anyway)

Signed-off-by Jesse Brandeburg <[email protected]>

Signed-off-by Jeff Kirsher <[email protected]>

CC Robert Olsson <[email protected]>

--
---

 net/core/pktgen.c |   27 +++++++++++++--------------
 1 files changed, 13 insertions(+), 14 deletions(-)

net/core/pktgen.c +13 ~~-14~~

index 99f656d..a47f5ba 100644
@@ -1973,28 +1973,27 @@ static void pktgen_setup_inject(struct pktgen_dev *pkt_dev)
 
 	/* make sure that we don't pick a non-existing transmit queue */
 	ntxq = pkt_dev->odev->real_num_tx_queues;
-	if (ntxq <= num_online_cpus() && (pkt_dev->flags & F_QUEUE_MAP_CPU)) {
+	if (ntxq > num_online_cpus() && (pkt_dev->flags & F_QUEUE_MAP_CPU)) {
 		printk(KERN_WARNING "pktgen: WARNING: QUEUE_MAP_CPU "
-		       "disabled because CPU count (%d) exceeds number ",
-		       num_online_cpus());
-		printk(KERN_WARNING "pktgen: WARNING: of tx queues "
-		       "(%d) on %s \n", ntxq, pkt_dev->odev->name);
+		       "disabled because CPU count (%d) exceeds number "
+		       "of tx queues (%d) on %s\n", num_online_cpus(), ntxq,
+		       pkt_dev->odev->name);
 		pkt_dev->flags &= ~F_QUEUE_MAP_CPU;
 	}
 	if (ntxq <= pkt_dev->queue_map_min) {
 		printk(KERN_WARNING "pktgen: WARNING: Requested "
-		       "queue_map_min (%d) exceeds number of tx\n",
-		       pkt_dev->queue_map_min);
-		printk(KERN_WARNING "pktgen: WARNING: queues (%d) on "
-		       "%s, resetting\n", ntxq, pkt_dev->odev->name);
+		       "queue_map_min (zero-based) (%d) exceeds valid range "
+		       "[0 - %d] for (%d) queues on %s, resetting\n",
+		       pkt_dev->queue_map_min, (ntxq ?: 1)- 1, ntxq,
+		       pkt_dev->odev->name);
 		pkt_dev->queue_map_min = ntxq - 1;
 	}
-	if (ntxq <= pkt_dev->queue_map_max) {
+	if (pkt_dev->queue_map_max >= ntxq) {
 		printk(KERN_WARNING "pktgen: WARNING: Requested "
-		       "queue_map_max (%d) exceeds number of tx\n",
-		       pkt_dev->queue_map_max);
-		printk(KERN_WARNING "pktgen: WARNING: queues (%d) on "
-		       "%s, resetting\n", ntxq, pkt_dev->odev->name);
+		       "queue_map_max (zero-based) (%d) exceeds valid range "
+		       "[0 - %d] for (%d) queues on %s, resetting\n",
+		       pkt_dev->queue_map_max, (ntxq ?: 1)- 1, ntxq,
+		       pkt_dev->odev->name);
 		pkt_dev->queue_map_max = ntxq - 1;
 	}

Robert Olsson

2008-10-28 20:10 UTC #1 lore raw

PATCH 1/1 pktgen: fix multiple queue warning

Hi,

Looks correct to me. Can test to test tomorrow.

Signed-off-by Robert Olsson <[email protected]>

Cheers.

				--ro

Quoted 74 lines · Jeff Kirsher

From: Jesse Brandeburg <[email protected]>

when testing the new pktgen module with multiple queues and ixgbe with:
pgset "flag QUEUE_MAP_CPU"

I found that I was getting errors in dmesg like:
pktgen: WARNING: QUEUE_MAP_CPU disabled because CPU count (8) exceeds number <4>pktgen: WARNING: of tx queues (8) on eth15

you'll note, 8 really doesn't exceed 8.

This patch seemed to fix the logic errors and also the attempts at limiting line length in printk (which didn't work anyway)

Signed-off-by Jesse Brandeburg <[email protected]>

Signed-off-by Jeff Kirsher <[email protected]>

CC Robert Olsson <[email protected]>

--
---

 net/core/pktgen.c |   27 +++++++++++++--------------
 1 files changed, 13 insertions(+), 14 deletions(-)

net/core/pktgen.c +13 ~~-15~~

index 99f656d..a47f5ba 100644
@@ -1973,28 +1973,27 @@ static void pktgen_setup_inject(struct pktgen_dev *pkt_dev)
 
 	/* make sure that we don't pick a non-existing transmit queue */
 	ntxq = pkt_dev->odev->real_num_tx_queues;
-	if (ntxq <= num_online_cpus() && (pkt_dev->flags & F_QUEUE_MAP_CPU)) {
+	if (ntxq > num_online_cpus() && (pkt_dev->flags & F_QUEUE_MAP_CPU)) {
 		printk(KERN_WARNING "pktgen: WARNING: QUEUE_MAP_CPU "
-		       "disabled because CPU count (%d) exceeds number ",
-		       num_online_cpus());
-		printk(KERN_WARNING "pktgen: WARNING: of tx queues "
-		       "(%d) on %s \n", ntxq, pkt_dev->odev->name);
+		       "disabled because CPU count (%d) exceeds number "
+		       "of tx queues (%d) on %s\n", num_online_cpus(), ntxq,
+		       pkt_dev->odev->name);
 		pkt_dev->flags &= ~F_QUEUE_MAP_CPU;
 	}
 	if (ntxq <= pkt_dev->queue_map_min) {
 		printk(KERN_WARNING "pktgen: WARNING: Requested "
-		       "queue_map_min (%d) exceeds number of tx\n",
-		       pkt_dev->queue_map_min);
-		printk(KERN_WARNING "pktgen: WARNING: queues (%d) on "
-		       "%s, resetting\n", ntxq, pkt_dev->odev->name);
+		       "queue_map_min (zero-based) (%d) exceeds valid range "
+		       "[0 - %d] for (%d) queues on %s, resetting\n",
+		       pkt_dev->queue_map_min, (ntxq ?: 1)- 1, ntxq,
+		       pkt_dev->odev->name);
 		pkt_dev->queue_map_min = ntxq - 1;
 	}
-	if (ntxq <= pkt_dev->queue_map_max) {
+	if (pkt_dev->queue_map_max >= ntxq) {
 		printk(KERN_WARNING "pktgen: WARNING: Requested "
-		       "queue_map_max (%d) exceeds number of tx\n",
-		       pkt_dev->queue_map_max);
-		printk(KERN_WARNING "pktgen: WARNING: queues (%d) on "
-		       "%s, resetting\n", ntxq, pkt_dev->odev->name);
+		       "queue_map_max (zero-based) (%d) exceeds valid range "
+		       "[0 - %d] for (%d) queues on %s, resetting\n",
+		       pkt_dev->queue_map_max, (ntxq ?: 1)- 1, ntxq,
+		       pkt_dev->odev->name);
 		pkt_dev->queue_map_max = ntxq - 1;
 	}
 
 
--

To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

David Miller

maintainer Applied

2008-10-28 20:22 UTC #2 lore raw

From: Robert Olsson <[email protected]>
Date: Tue, 28 Oct 2008 21:10:21 +0100

Quoted 3 lines

Looks correct to me. Can test to test tomorrow.
Signed-off-by Robert Olsson <[email protected]>

I've applied Jesse's patch to net-2.6, thanks everyone.

David Miller

maintainer

2008-10-28 20:23 UTC #3 lore raw

From: David Miller <[email protected]>
Date: Tue, 28 Oct 2008 13:22:18 -0700 (PDT)

Quoted 8 lines

From: Robert Olsson <[email protected]>
Date: Tue, 28 Oct 2008 21:10:21 +0100
Quoted 3 lines
Looks correct to me. Can test to test tomorrow.
Signed-off-by Robert Olsson <[email protected]>
I've applied Jesse's patch to net-2.6, thanks everyone.

Sorry, I meant "Jeff" of course. :-/

Robert Olsson

2008-11-08 11:29 UTC #4 lore raw

PATCH 1/1 pktgen: fix multiple queue warning

In reply to Jeff Kirsher

net/core/pktgen.c static void pktgen_setup_inject(struct pktgen_dev *pkt_dev) · line 1981

Quoted 7 lines · Jeff Kirsher

net/core/pktgen.c +1 -5

-	if (ntxq <= num_online_cpus() && (pkt_dev->flags & F_QUEUE_MAP_CPU)) {
+	if (ntxq > num_online_cpus() && (pkt_dev->flags & F_QUEUE_MAP_CPU)) {
 		printk(KERN_WARNING "pktgen: WARNING: QUEUE_MAP_CPU "
-		       "disabled because CPU count (%d) exceeds number ",
-		       num_online_cpus());
-		printk(KERN_WARNING "pktgen: WARNING: of tx queues "
-		       "(%d) on %s \n", ntxq, pkt_dev->odev->name);

Jeff,

This triggers with the niu driver which has 12 TX queues on our 8 CPU-core system. Might be best to make sure that pkt_dev->cur_queue_map never exceeds ntxq.

pkt_dev->cur_queue_map = pkt_dev->cur_queue_map % pkt_dev->odev->real_num_tx_queues;

Cheers.

					--ro

David Miller

maintainer

2008-11-17 07:30 UTC #5 lore raw

From: Robert Olsson <[email protected]>
Date: Sat, 8 Nov 2008 12:29:32 +0100

Quoted 14 lines

net/core/pktgen.c static void pktgen_setup_inject(struct pktgen_dev *pkt_dev) · line 1981
Quoted 7 lines · Jeff Kirsher
net/core/pktgen.c +1 -5
-	if (ntxq <= num_online_cpus() && (pkt_dev->flags & F_QUEUE_MAP_CPU)) {
+	if (ntxq > num_online_cpus() && (pkt_dev->flags & F_QUEUE_MAP_CPU)) {
 		printk(KERN_WARNING "pktgen: WARNING: QUEUE_MAP_CPU "
-		       "disabled because CPU count (%d) exceeds number ",
-		       num_online_cpus());
-		printk(KERN_WARNING "pktgen: WARNING: of tx queues "
-		       "(%d) on %s \n", ntxq, pkt_dev->odev->name);
This triggers with the niu driver which has 12 TX queues on our 8 CPU-core system. Might be best to make sure that pkt_dev->cur_queue_map never exceeds ntxq.

Quoted 1 line

pkt_dev->cur_queue_map = pkt_dev->cur_queue_map % pkt_dev->odev->real_num_tx_queues;

Can someone cook up a patch to fix this?

Thanks.

Robert Olsson

2008-11-17 08:59 UTC #6 lore raw

Quoted 3 lines · David Miller

Quoted 1 line
This triggers with the niu driver which has 12 TX queues on our 8 CPU-core
Can someone cook up a patch to fix this?

Here is one suggestion...

As number of TX queues in unrelated to number of CPU's we remove this test and just make sure nxtq never gets exceeded.

Cheers.

					--ro

Signed-off-by Robert Olsson <[email protected]>

net/core/pktgen.c +2 -7

index a47f5ba..8997e91 100644
@@ -1973,13 +1973,7 @@ static void pktgen_setup_inject(struct pktgen_dev *pkt_dev)
 
 	/* make sure that we don't pick a non-existing transmit queue */
 	ntxq = pkt_dev->odev->real_num_tx_queues;
-	if (ntxq > num_online_cpus() && (pkt_dev->flags & F_QUEUE_MAP_CPU)) {
-		printk(KERN_WARNING "pktgen: WARNING: QUEUE_MAP_CPU "
-		       "disabled because CPU count (%d) exceeds number "
-		       "of tx queues (%d) on %s\n", num_online_cpus(), ntxq,
-		       pkt_dev->odev->name);
-		pkt_dev->flags &= ~F_QUEUE_MAP_CPU;
-	}
+
 	if (ntxq <= pkt_dev->queue_map_min) {
 		printk(KERN_WARNING "pktgen: WARNING: Requested "
 		       "queue_map_min (zero-based) (%d) exceeds valid range "

@@ -2202,6 +2196,7 @@ static void set_cur_queue_map(struct pktgen_dev *pkt_dev)
 		}
 		pkt_dev->cur_queue_map = t;
 	}
+	pkt_dev->cur_queue_map  = pkt_dev->cur_queue_map % pkt_dev->odev->real_num_tx_queues;
 }
 
 /* Increment/randomize headers according to flags and current values

David Miller

maintainer Applied

2008-11-19 22:09 UTC #7 lore raw

From: Robert Olsson <[email protected]>
Date: Mon, 17 Nov 2008 09:59:03 +0100

Quoted 5 lines

Quoted 1 line · David Miller
Quoted 1 line
This triggers with the niu driver which has 12 TX queues on our 8 CPU-core

Quoted 8 lines

Quoted 1 line
Can someone cook up a patch to fix this?
Here is one suggestion...
As number of TX queues in unrelated to number of CPU's we remove this test and just make sure nxtq never gets exceeded.
Signed-off-by Robert Olsson <[email protected]>

Applied, thanks Robert.