[PATCH] pktgen: fix multiple queue warning

[PATCH] pktgen: fix multiple queue warning

[Jeff Kirsher]

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

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 <jesse.brandeburg@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
CC: Robert Olsson <robert.olsson@its.uu.se>
--
---

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

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 99f656d..a47f5ba 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -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;
 	}
 

[PATCH] pktgen: fix multiple queue warning

[Robert Olsson]

Hi, 

Looks correct to me. Can test to test tomorrow.

Signed-off-by: Robert Olsson <robert.olsson@its.uu.se>

Cheers.
				--ro


Jeff Kirsher writes:
 > From: Jesse Brandeburg <jesse.brandeburg@intel.com>
 > 
 > 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 <jesse.brandeburg@intel.com>
 > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
 > CC: Robert Olsson <robert.olsson@its.uu.se>
 > --
 > ---
 > 
 >  net/core/pktgen.c |   27 +++++++++++++--------------
 >  1 files changed, 13 insertions(+), 14 deletions(-)
 > 
 > diff --git a/net/core/pktgen.c b/net/core/pktgen.c
 > index 99f656d..a47f5ba 100644
 > --- a/net/core/pktgen.c
 > +++ b/net/core/pktgen.c
 > @@ -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 majordomo@vger.kernel.org
 > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pktgen: fix multiple queue warning

[David Miller]

From: Robert Olsson <robert@robur.slu.se>
Date: Tue, 28 Oct 2008 21:10:21 +0100

> Looks correct to me. Can test to test tomorrow.
> 
> Signed-off-by: Robert Olsson <robert.olsson@its.uu.se>

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

Re: [PATCH] pktgen: fix multiple queue warning

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Tue, 28 Oct 2008 13:22:18 -0700 (PDT)

> From: Robert Olsson <robert@robur.slu.se>
> Date: Tue, 28 Oct 2008 21:10:21 +0100
> 
> > Looks correct to me. Can test to test tomorrow.
> > 
> > Signed-off-by: Robert Olsson <robert.olsson@its.uu.se>
> 
> I've applied Jesse's patch to net-2.6, thanks everyone.

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

[PATCH] pktgen: fix multiple queue warning

[Robert Olsson]

Jeff Kirsher writes:


 > -	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
 
 

Re: [PATCH] pktgen: fix multiple queue warning

[David Miller]

From: Robert Olsson <robert@robur.slu.se>
Date: Sat, 8 Nov 2008 12:29:32 +0100

> Jeff Kirsher writes:
> 
>  > -	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.
> 
> 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.

Re: [PATCH] pktgen: fix multiple queue warning

[Robert Olsson]

David Miller writes:

 > >  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 <robert.olsson@its.uu.se>


diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index a47f5ba..8997e91 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -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

Re: [PATCH] pktgen: fix multiple queue warning

[David Miller]

From: Robert Olsson <robert@robur.slu.se>
Date: Mon, 17 Nov 2008 09:59:03 +0100

> 
> David Miller writes:
> 
>  > >  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.
> 
> Signed-off-by: Robert Olsson <robert.olsson@its.uu.se>

Applied, thanks Robert.