diff mbox

pktgen: fix multiple queue warning

Message ID 20081028193042.22781.15774.stgit@gitlost.lost
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Kirsher, Jeffrey T Oct. 28, 2008, 7:30 p.m. UTC 
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(-)


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

Comments

Robert Olsson Oct. 28, 2008, 8:10 p.m. UTC | #1 
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
--
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
David Miller Oct. 28, 2008, 8:22 p.m. UTC | #2 
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.
--
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
David Miller Oct. 28, 2008, 8:22 p.m. UTC | #3 
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. :-/
--
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
Robert Olsson Nov. 8, 2008, 11:29 a.m. UTC | #4 
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
 
 
--
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
David Miller Nov. 17, 2008, 7:30 a.m. UTC | #5 
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.
--
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
diff mbox

Patch

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