* [PATCH] check the return value of ndo_select_queue()
@ 2009-11-09 7:17 Changli Gao
2009-11-09 7:26 ` David Miller
0 siblings, 1 reply; 16+ messages in thread
From: Changli Gao @ 2009-11-09 7:17 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, xiaosuo
check the return value of ndo_select_queue()
Check the return value of ndo_select_queue(). If the value isn't smaller
than the real_num_tx_queues, print a warning message, and reset it to zero.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
net/core/dev.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index b8f74cf..edf5ea6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1794,10 +1794,17 @@ static struct netdev_queue *dev_pick_tx(struct net_device *dev,
const struct net_device_ops *ops = dev->netdev_ops;
u16 queue_index = 0;
- if (ops->ndo_select_queue)
+ if (ops->ndo_select_queue) {
queue_index = ops->ndo_select_queue(dev, skb);
- else if (dev->real_num_tx_queues > 1)
+ if (queue_index >= dev->real_num_tx_queues) {
+ printk(KERN_WARNING "%s selects TX queue %d, "
+ "but real number of TX queues is %d\n",
+ dev->name, queue_index, dev->real_num_tx_queues);
+ queue_index = 0;
+ }
+ } else if (dev->real_num_tx_queues > 1) {
queue_index = skb_tx_hash(dev, skb);
+ }
skb_set_queue_mapping(skb, queue_index);
return netdev_get_tx_queue(dev, queue_index);
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] check the return value of ndo_select_queue()
2009-11-09 7:17 [PATCH] check the return value of ndo_select_queue() Changli Gao
@ 2009-11-09 7:26 ` David Miller
2009-11-09 8:44 ` Changli Gao
0 siblings, 1 reply; 16+ messages in thread
From: David Miller @ 2009-11-09 7:26 UTC (permalink / raw)
To: xiaosuo; +Cc: netdev
From: Changli Gao <xiaosuo@gmail.com>
Date: Mon, 09 Nov 2009 15:17:17 +0800
> check the return value of ndo_select_queue()
>
> Check the return value of ndo_select_queue(). If the value isn't smaller
> than the real_num_tx_queues, print a warning message, and reset it to zero.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Make it a WARN() so that it ends up in kerneloops.org
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] check the return value of ndo_select_queue()
2009-11-09 7:26 ` David Miller
@ 2009-11-09 8:44 ` Changli Gao
2009-11-09 9:13 ` David Miller
0 siblings, 1 reply; 16+ messages in thread
From: Changli Gao @ 2009-11-09 8:44 UTC (permalink / raw)
To: David Miller; +Cc: netdev
On Mon, Nov 9, 2009 at 3:26 PM, David Miller <davem@davemloft.net> wrote:
> From: Changli Gao <xiaosuo@gmail.com>
> Date: Mon, 09 Nov 2009 15:17:17 +0800
>
>> check the return value of ndo_select_queue()
>>
>> Check the return value of ndo_select_queue(). If the value isn't smaller
>> than the real_num_tx_queues, print a warning message, and reset it to zero.
>>
>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>
> Make it a WARN() so that it ends up in kerneloops.org
>
Like this?
WARN(1, "%s selects TX queue %d, "
"but real number of TX queues is %d\n",
dev->name, queue_index, dev->real_num_tx_queues);
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] check the return value of ndo_select_queue()
2009-11-09 8:44 ` Changli Gao
@ 2009-11-09 9:13 ` David Miller
0 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2009-11-09 9:13 UTC (permalink / raw)
To: xiaosuo; +Cc: netdev
From: Changli Gao <xiaosuo@gmail.com>
Date: Mon, 9 Nov 2009 16:44:05 +0800
> On Mon, Nov 9, 2009 at 3:26 PM, David Miller <davem@davemloft.net> wrote:
>> From: Changli Gao <xiaosuo@gmail.com>
>> Date: Mon, 09 Nov 2009 15:17:17 +0800
>>
>>> check the return value of ndo_select_queue()
>>>
>>> Check the return value of ndo_select_queue(). If the value isn't smaller
>>> than the real_num_tx_queues, print a warning message, and reset it to zero.
>>>
>>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>>
>> Make it a WARN() so that it ends up in kerneloops.org
>>
> Like this?
> WARN(1, "%s selects TX queue %d, "
> "but real number of TX queues is %d\n",
> dev->name, queue_index, dev->real_num_tx_queues);
Yes, something like that.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] check the return value of ndo_select_queue()
2009-11-14 12:37 ` Eric Dumazet
@ 2009-11-16 6:09 ` David Miller
0 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2009-11-16 6:09 UTC (permalink / raw)
To: eric.dumazet; +Cc: xiaosuo, netdev
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 14 Nov 2009 13:37:26 +0100
> Changli Gao a écrit :
>>
>> It seems better than mime.
>>
>>
>
> Well, it's still your patch :)
Applied, thanks everyone :-)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] check the return value of ndo_select_queue()
2009-11-14 12:14 ` Changli Gao
@ 2009-11-14 12:37 ` Eric Dumazet
2009-11-16 6:09 ` David Miller
0 siblings, 1 reply; 16+ messages in thread
From: Eric Dumazet @ 2009-11-14 12:37 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, netdev
Changli Gao a écrit :
>
> It seems better than mime.
>
>
Well, it's still your patch :)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] check the return value of ndo_select_queue()
2009-11-14 7:54 ` Eric Dumazet
@ 2009-11-14 12:14 ` Changli Gao
2009-11-14 12:37 ` Eric Dumazet
0 siblings, 1 reply; 16+ messages in thread
From: Changli Gao @ 2009-11-14 12:14 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, netdev
On Sat, Nov 14, 2009 at 3:54 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
> Please always use scripts/checkpatch.pl before sending patches
>
> } else if((select_queue = dev->netdev_ops->ndo_select_queue)) {
>
> A space is required after the "if"
>
Thanks.
> It seems to me this patch is not matching its ChangeLog :
>
> You moved the :
>
> if (sk && sk->sk_dst_cache)
> sk_tx_queue_set(sk, queue_index);
Maybe I did more in my patch, but I think these little changes doesn't
need separate patches.
>
> Why not using much readable patch like :
>
>
> [PATCH] check the return value of ndo_select_queue()
>
> check the return value of ndo_select_queue()
>
> Check the return value of ndo_select_queue(). If the value isn't smaller
> than the real_num_tx_queues, print a warning message, and reset it to zero.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ----
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index ad8e320..bbc9a41 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1845,6 +1845,20 @@ u16 skb_tx_hash(const struct net_device *dev, const struct sk_buff *skb)
> }
> EXPORT_SYMBOL(skb_tx_hash);
>
> +static inline u16 dev_cap_txqueue(struct net_device *dev, u16 queue_index)
> +{
> + if (unlikely(queue_index >= dev->real_num_tx_queues)) {
> + if (net_ratelimit()) {
> + WARN(1, "%s selects TX queue %d, but "
> + "real number of TX queues is %d\n",
> + dev->name, queue_index,
> + dev->real_num_tx_queues);
> + }
> + return 0;
> + }
> + return queue_index;
> +}
> +
> static struct netdev_queue *dev_pick_tx(struct net_device *dev,
> struct sk_buff *skb)
> {
> @@ -1858,6 +1872,7 @@ static struct netdev_queue *dev_pick_tx(struct net_device *dev,
>
> if (ops->ndo_select_queue) {
> queue_index = ops->ndo_select_queue(dev, skb);
> + queue_index = dev_cap_txqueue(dev, queue_index);
> } else {
> queue_index = 0;
> if (dev->real_num_tx_queues > 1)
>
>
>
It seems better than mime.
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] check the return value of ndo_select_queue()
2009-11-14 0:50 Changli Gao
@ 2009-11-14 7:54 ` Eric Dumazet
2009-11-14 12:14 ` Changli Gao
0 siblings, 1 reply; 16+ messages in thread
From: Eric Dumazet @ 2009-11-14 7:54 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, netdev
Changli Gao a écrit :
> check the return value of ndo_select_queue()
>
> Check the return value of ndo_select_queue(). If the value isn't smaller
> than the real_num_tx_queues, print a warning message, and reset it to zero.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ----
> net/core/dev.c | 33 ++++++++++++++++++++++++---------
> 1 file changed, 24 insertions(+), 9 deletions(-)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index bf629ac..74a3824 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1849,22 +1849,37 @@ static struct netdev_queue *dev_pick_tx(struct
> net_device *dev,
> {
> u16 queue_index;
> struct sock *sk = skb->sk;
> + unsigned int real_num_tx_queues = dev->real_num_tx_queues;
>
> if (sk_tx_queue_recorded(sk)) {
> queue_index = sk_tx_queue_get(sk);
> + if (unlikely(queue_index >= real_num_tx_queues)) {
> + do {
> + queue_index -= real_num_tx_queues;
> + } while (queue_index >= real_num_tx_queues);
> + sk_tx_queue_set(sk, queue_index);
> + }
> } else {
> - const struct net_device_ops *ops = dev->netdev_ops;
> + u16 (*select_queue)(struct net_device *, struct sk_buff *);
>
> - if (ops->ndo_select_queue) {
> - queue_index = ops->ndo_select_queue(dev, skb);
> - } else {
> + if (real_num_tx_queues == 1) {
> queue_index = 0;
> - if (dev->real_num_tx_queues > 1)
> - queue_index = skb_tx_hash(dev, skb);
> -
> - if (sk && sk->sk_dst_cache)
> - sk_tx_queue_set(sk, queue_index);
> + } else if((select_queue = dev->netdev_ops->ndo_select_queue)) {
> + queue_index = select_queue(dev, skb);
> + if (unlikely(queue_index >= real_num_tx_queues)) {
> + if (net_ratelimit())
> + WARN(1, "%s selects TX queue %d, but "
> + "real number of TX queues is %d\n",
> + dev->name, queue_index,
> + real_num_tx_queues);
> + queue_index = 0;
> + }
> + } else {
> + queue_index = skb_tx_hash(dev, skb);
> }
> +
> + if (sk && sk->sk_dst_cache)
> + sk_tx_queue_set(sk, queue_index);
> }
>
> skb_set_queue_mapping(skb, queue_index);
Please always use scripts/checkpatch.pl before sending patches
} else if((select_queue = dev->netdev_ops->ndo_select_queue)) {
A space is required after the "if"
It seems to me this patch is not matching its ChangeLog :
You moved the :
if (sk && sk->sk_dst_cache)
sk_tx_queue_set(sk, queue_index);
Why not using much readable patch like :
[PATCH] check the return value of ndo_select_queue()
check the return value of ndo_select_queue()
Check the return value of ndo_select_queue(). If the value isn't smaller
than the real_num_tx_queues, print a warning message, and reset it to zero.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
----
diff --git a/net/core/dev.c b/net/core/dev.c
index ad8e320..bbc9a41 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1845,6 +1845,20 @@ u16 skb_tx_hash(const struct net_device *dev, const struct sk_buff *skb)
}
EXPORT_SYMBOL(skb_tx_hash);
+static inline u16 dev_cap_txqueue(struct net_device *dev, u16 queue_index)
+{
+ if (unlikely(queue_index >= dev->real_num_tx_queues)) {
+ if (net_ratelimit()) {
+ WARN(1, "%s selects TX queue %d, but "
+ "real number of TX queues is %d\n",
+ dev->name, queue_index,
+ dev->real_num_tx_queues);
+ }
+ return 0;
+ }
+ return queue_index;
+}
+
static struct netdev_queue *dev_pick_tx(struct net_device *dev,
struct sk_buff *skb)
{
@@ -1858,6 +1872,7 @@ static struct netdev_queue *dev_pick_tx(struct net_device *dev,
if (ops->ndo_select_queue) {
queue_index = ops->ndo_select_queue(dev, skb);
+ queue_index = dev_cap_txqueue(dev, queue_index);
} else {
queue_index = 0;
if (dev->real_num_tx_queues > 1)
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH] check the return value of ndo_select_queue()
@ 2009-11-14 0:50 Changli Gao
2009-11-14 7:54 ` Eric Dumazet
0 siblings, 1 reply; 16+ messages in thread
From: Changli Gao @ 2009-11-14 0:50 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Eric Dumazet, Changli Gao
check the return value of ndo_select_queue()
Check the return value of ndo_select_queue(). If the value isn't smaller
than the real_num_tx_queues, print a warning message, and reset it to zero.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
net/core/dev.c | 33 ++++++++++++++++++++++++---------
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index bf629ac..74a3824 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1849,22 +1849,37 @@ static struct netdev_queue *dev_pick_tx(struct
net_device *dev,
{
u16 queue_index;
struct sock *sk = skb->sk;
+ unsigned int real_num_tx_queues = dev->real_num_tx_queues;
if (sk_tx_queue_recorded(sk)) {
queue_index = sk_tx_queue_get(sk);
+ if (unlikely(queue_index >= real_num_tx_queues)) {
+ do {
+ queue_index -= real_num_tx_queues;
+ } while (queue_index >= real_num_tx_queues);
+ sk_tx_queue_set(sk, queue_index);
+ }
} else {
- const struct net_device_ops *ops = dev->netdev_ops;
+ u16 (*select_queue)(struct net_device *, struct sk_buff *);
- if (ops->ndo_select_queue) {
- queue_index = ops->ndo_select_queue(dev, skb);
- } else {
+ if (real_num_tx_queues == 1) {
queue_index = 0;
- if (dev->real_num_tx_queues > 1)
- queue_index = skb_tx_hash(dev, skb);
-
- if (sk && sk->sk_dst_cache)
- sk_tx_queue_set(sk, queue_index);
+ } else if((select_queue = dev->netdev_ops->ndo_select_queue)) {
+ queue_index = select_queue(dev, skb);
+ if (unlikely(queue_index >= real_num_tx_queues)) {
+ if (net_ratelimit())
+ WARN(1, "%s selects TX queue %d, but "
+ "real number of TX queues is %d\n",
+ dev->name, queue_index,
+ real_num_tx_queues);
+ queue_index = 0;
+ }
+ } else {
+ queue_index = skb_tx_hash(dev, skb);
}
+
+ if (sk && sk->sk_dst_cache)
+ sk_tx_queue_set(sk, queue_index);
}
skb_set_queue_mapping(skb, queue_index);
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] check the return value of ndo_select_queue()
2009-11-10 3:43 ` Eric Dumazet
@ 2009-11-13 22:09 ` David Miller
0 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2009-11-13 22:09 UTC (permalink / raw)
To: eric.dumazet; +Cc: xiaosuo, netdev
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 10 Nov 2009 04:43:33 +0100
> Changli Gao a écrit :
>> check the return value of ndo_select_queue()
>>
>> Check the return value of ndo_select_queue(). If the value isn't smaller
>> than the real_num_tx_queues, print a warning message, and reset it to zero.
>>
>> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
This patch does not apply properly to net-next-2.6, the dev_pick_tx()
function looks a lot different now.
Please respin this patch and resubmit, thank you.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] check the return value of ndo_select_queue()
2009-11-10 1:42 Changli Gao
@ 2009-11-10 3:43 ` Eric Dumazet
2009-11-13 22:09 ` David Miller
0 siblings, 1 reply; 16+ messages in thread
From: Eric Dumazet @ 2009-11-10 3:43 UTC (permalink / raw)
To: xiaosuo; +Cc: David S. Miller, netdev
Changli Gao a écrit :
> check the return value of ndo_select_queue()
>
> Check the return value of ndo_select_queue(). If the value isn't smaller
> than the real_num_tx_queues, print a warning message, and reset it to zero.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] check the return value of ndo_select_queue()
@ 2009-11-10 1:42 Changli Gao
2009-11-10 3:43 ` Eric Dumazet
0 siblings, 1 reply; 16+ messages in thread
From: Changli Gao @ 2009-11-10 1:42 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, xiaosuo, Eric Dumazet
check the return value of ndo_select_queue()
Check the return value of ndo_select_queue(). If the value isn't smaller
than the real_num_tx_queues, print a warning message, and reset it to zero.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
net/core/dev.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index b8f74cf..be081a5 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1791,13 +1791,25 @@ EXPORT_SYMBOL(skb_tx_hash);
static struct netdev_queue *dev_pick_tx(struct net_device *dev,
struct sk_buff *skb)
{
- const struct net_device_ops *ops = dev->netdev_ops;
- u16 queue_index = 0;
-
- if (ops->ndo_select_queue)
- queue_index = ops->ndo_select_queue(dev, skb);
- else if (dev->real_num_tx_queues > 1)
+ u16 queue_index;
+ u16 (*ndo_select_queue)(struct net_device*, struct sk_buff*);
+ unsigned int real_num_tx_queues = dev->real_num_tx_queues;
+
+ if (real_num_tx_queues == 1) {
+ queue_index = 0;
+ } else if ((ndo_select_queue = dev->netdev_ops->ndo_select_queue)) {
+ queue_index = ndo_select_queue(dev, skb);
+ if (unlikely(queue_index >= real_num_tx_queues)) {
+ if (net_ratelimit())
+ WARN(1, "%s selects TX queue %d, "
+ "but real number of TX queues is %d\n",
+ dev->name, queue_index,
+ real_num_tx_queues);
+ queue_index = 0;
+ }
+ } else {
queue_index = skb_tx_hash(dev, skb);
+ }
skb_set_queue_mapping(skb, queue_index);
return netdev_get_tx_queue(dev, queue_index);
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] check the return value of ndo_select_queue()
2009-11-09 10:02 ` Eric Dumazet
@ 2009-11-09 11:29 ` Changli Gao
0 siblings, 0 replies; 16+ messages in thread
From: Changli Gao @ 2009-11-09 11:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, netdev
2009/11/9 Eric Dumazet <eric.dumazet@gmail.com>:
> Eric Dumazet a écrit :
>>
>> Yes, but this is a _very_ unlikely condition (a bug IMHO), so you could use :
>>
>> if (unlikely(queue_index >= dev->real_num_tx_queues)) {
>> ...
>> }
>>
>> (This unlikely() clause is implied in WARN... macros)
>>
>
> Also, you want to trigger this message only once, or ratelimit it at least
>
>
How about this version:
u16 queue_index;
u16 (*ndo_select_queue)(struct net_device*, struct sk_buff*);
unsigned int real_num_tx_queues = dev->real_num_tx_queues;
if (real_num_tx_queues == 1) {
queue_index = 0;
} else if ((ndo_select_queue = dev->netdev_ops->ndo_select_queue)) {
queue_index = ndo_select_queue(dev, skb);
if (unlikely(queue_index >= real_num_tx_queues)) {
if (net_ratelimit())
WARN(1, "%s selects TX queue %d, "
"but real number of TX queues is %d\n",
dev->name, queue_index,
real_num_tx_queues);
queue_index = 0;
}
} else {
queue_index = skb_tx_hash(dev, skb);
}
skb_set_queue_mapping(skb, queue_index);
return netdev_get_tx_queue(dev, queue_index);
--
Regards，
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] check the return value of ndo_select_queue()
2009-11-09 9:50 ` Eric Dumazet
@ 2009-11-09 10:02 ` Eric Dumazet
2009-11-09 11:29 ` Changli Gao
0 siblings, 1 reply; 16+ messages in thread
From: Eric Dumazet @ 2009-11-09 10:02 UTC (permalink / raw)
To: xiaosuo; +Cc: David S. Miller, netdev
Eric Dumazet a écrit :
>
> Yes, but this is a _very_ unlikely condition (a bug IMHO), so you could use :
>
> if (unlikely(queue_index >= dev->real_num_tx_queues)) {
> ...
> }
>
> (This unlikely() clause is implied in WARN... macros)
>
Also, you want to trigger this message only once, or ratelimit it at least
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] check the return value of ndo_select_queue()
2009-11-09 9:34 Changli Gao
@ 2009-11-09 9:50 ` Eric Dumazet
2009-11-09 10:02 ` Eric Dumazet
0 siblings, 1 reply; 16+ messages in thread
From: Eric Dumazet @ 2009-11-09 9:50 UTC (permalink / raw)
To: xiaosuo; +Cc: David S. Miller, netdev
Changli Gao a écrit :
> check the return value of ndo_select_queue()
>
> Check the return value of ndo_select_queue(). If the value isn't smaller
> than the real_num_tx_queues, print a warning message, and reset it to zero.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ----
> net/core/dev.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index b8f74cf..0a6bf2e 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1794,10 +1794,17 @@ static struct netdev_queue *dev_pick_tx(struct net_device *dev,
> const struct net_device_ops *ops = dev->netdev_ops;
> u16 queue_index = 0;
>
> - if (ops->ndo_select_queue)
> + if (ops->ndo_select_queue) {
> queue_index = ops->ndo_select_queue(dev, skb);
> - else if (dev->real_num_tx_queues > 1)
> + if (queue_index >= dev->real_num_tx_queues) {
> + WARN(1, "%s selects TX queue %d, "
> + "but real number of TX queues is %d\n",
> + dev->name, queue_index, dev->real_num_tx_queues);
> + queue_index = 0;
> + }
> + } else if (dev->real_num_tx_queues > 1) {
> queue_index = skb_tx_hash(dev, skb);
> + }
>
> skb_set_queue_mapping(skb, queue_index);
> return netdev_get_tx_queue(dev, queue_index);
Yes, but this is a _very_ unlikely condition (a bug IMHO), so you could use :
if (unlikely(queue_index >= dev->real_num_tx_queues)) {
...
}
(This unlikely() clause is implied in WARN... macros)
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] check the return value of ndo_select_queue()
@ 2009-11-09 9:34 Changli Gao
2009-11-09 9:50 ` Eric Dumazet
0 siblings, 1 reply; 16+ messages in thread
From: Changli Gao @ 2009-11-09 9:34 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, xiaosuo
check the return value of ndo_select_queue()
Check the return value of ndo_select_queue(). If the value isn't smaller
than the real_num_tx_queues, print a warning message, and reset it to zero.
Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
net/core/dev.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index b8f74cf..0a6bf2e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1794,10 +1794,17 @@ static struct netdev_queue *dev_pick_tx(struct net_device *dev,
const struct net_device_ops *ops = dev->netdev_ops;
u16 queue_index = 0;
- if (ops->ndo_select_queue)
+ if (ops->ndo_select_queue) {
queue_index = ops->ndo_select_queue(dev, skb);
- else if (dev->real_num_tx_queues > 1)
+ if (queue_index >= dev->real_num_tx_queues) {
+ WARN(1, "%s selects TX queue %d, "
+ "but real number of TX queues is %d\n",
+ dev->name, queue_index, dev->real_num_tx_queues);
+ queue_index = 0;
+ }
+ } else if (dev->real_num_tx_queues > 1) {
queue_index = skb_tx_hash(dev, skb);
+ }
skb_set_queue_mapping(skb, queue_index);
return netdev_get_tx_queue(dev, queue_index);
^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2009-11-16 6:09 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-09 7:17 [PATCH] check the return value of ndo_select_queue() Changli Gao
2009-11-09 7:26 ` David Miller
2009-11-09 8:44 ` Changli Gao
2009-11-09 9:13 ` David Miller
2009-11-09 9:34 Changli Gao
2009-11-09 9:50 ` Eric Dumazet
2009-11-09 10:02 ` Eric Dumazet
2009-11-09 11:29 ` Changli Gao
2009-11-10 1:42 Changli Gao
2009-11-10 3:43 ` Eric Dumazet
2009-11-13 22:09 ` David Miller
2009-11-14 0:50 Changli Gao
2009-11-14 7:54 ` Eric Dumazet
2009-11-14 12:14 ` Changli Gao
2009-11-14 12:37 ` Eric Dumazet
2009-11-16 6:09 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.