From patchwork Sat Nov 14 07:54:04 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 38422 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id 28777B7B61 for ; Sat, 14 Nov 2009 18:54:26 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753928AbZKNHyF (ORCPT ); Sat, 14 Nov 2009 02:54:05 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753921AbZKNHyF (ORCPT ); Sat, 14 Nov 2009 02:54:05 -0500 Received: from gw1.cosmosbay.com ([212.99.114.194]:44276 "EHLO gw1.cosmosbay.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753693AbZKNHyD (ORCPT ); Sat, 14 Nov 2009 02:54:03 -0500 Received: from [127.0.0.1] (localhost [127.0.0.1]) by gw1.cosmosbay.com (8.13.7/8.13.7) with ESMTP id nAE7s50n007583; Sat, 14 Nov 2009 08:54:05 +0100 Message-ID: <4AFE621C.1020004@gmail.com> Date: Sat, 14 Nov 2009 08:54:04 +0100 From: Eric Dumazet User-Agent: Thunderbird 2.0.0.23 (Windows/20090812) MIME-Version: 1.0 To: Changli Gao CC: "David S. Miller" , netdev@vger.kernel.org Subject: Re: [PATCH] check the return value of ndo_select_queue() References: <412e6f7f0911131650j6d13f95bt20aa607f9f44c3af@mail.gmail.com> In-Reply-To: <412e6f7f0911131650j6d13f95bt20aa607f9f44c3af@mail.gmail.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-1.6 (gw1.cosmosbay.com [0.0.0.0]); Sat, 14 Nov 2009 08:54:05 +0100 (CET) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 > ---- > 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 Signed-off-by: Eric Dumazet ---- --- 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 --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)