From patchwork Tue Jun 8 14:18:41 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 54984 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.180.67]) by ozlabs.org (Postfix) with ESMTP id E91E71007D5 for ; Wed, 9 Jun 2010 00:18:53 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755364Ab0FHOSs (ORCPT ); Tue, 8 Jun 2010 10:18:48 -0400 Received: from mail-fx0-f46.google.com ([209.85.161.46]:40878 "EHLO mail-fx0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755354Ab0FHOSr (ORCPT ); Tue, 8 Jun 2010 10:18:47 -0400 Received: by fxm8 with SMTP id 8so2921730fxm.19 for ; Tue, 08 Jun 2010 07:18:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:subject:from:to:cc :in-reply-to:references:content-type:date:message-id:mime-version :x-mailer:content-transfer-encoding; bh=kURdmSmSEoj7w3So+bZTuhrJL9qw9Y4yIiIb1h+CjvU=; b=Vg68bJu0ewBey+sOgRLMFKFVb1LuZfvP5na0O9XZOb2MyvnbjB3CnaTqaUZnGM+tQ4 TsxG8DU0OACCq6HVPB8Kdk5t99rufNvT+rNyU6hGPPs4DKIUkN/8G7h59jVMx+LWuPTD d/pzOGMd3iPirU471FL++8KIzQivSyY1eRUUQ= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=subject:from:to:cc:in-reply-to:references:content-type:date :message-id:mime-version:x-mailer:content-transfer-encoding; b=qEJXvhI8pCVP9xTWqLTZ0UsyTPhOgNHGvAIHuOPs05quvFOtpJ0w7Bc9GyPTkCuTK0 gYSMaIf6PboIPgDzI37ZNMNWxTejXRe8y11Zq907QTwnHrQ6PhNbGT6FkkfjeVaEby+2 WDrL1QoG3HHiFCUYhdcwL29MDegzFrZGanBmg= Received: by 10.223.45.200 with SMTP id g8mr16555950faf.67.1276006724868; Tue, 08 Jun 2010 07:18:44 -0700 (PDT) Received: from [127.0.0.1] ([85.17.35.125]) by mx.google.com with ESMTPS id 13sm25670200fad.7.2010.06.08.07.18.42 (version=SSLv3 cipher=RC4-MD5); Tue, 08 Jun 2010 07:18:43 -0700 (PDT) Subject: Re: RFS seems to have incompatiblities with bridged vlans From: Eric Dumazet To: John Fastabend Cc: Stephen Hemminger , Peter Lieven , "davem@davemloft.net" , "netdev@vger.kernel.org" In-Reply-To: <4C0D7312.1020300@intel.com> References: <62C40338-045A-417E-9B90-E59A320E1343@dlh.net> <20100607145910.2458ac87@nehalam> <4C0D7312.1020300@intel.com> Date: Tue, 08 Jun 2010 16:18:41 +0200 Message-ID: <1276006721.2486.141.camel@edumazet-laptop> Mime-Version: 1.0 X-Mailer: Evolution 2.28.3 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Le lundi 07 juin 2010 à 15:30 -0700, John Fastabend a écrit : > There is always a possibility that the underlying device sets the > queue_mapping to be greater then num_cpus. Also I suspect the same > issue exists with bonding devices. Maybe something like the following > is worth while? compile tested only, > > [PATCH] 8021q: vlan reassigns dev without check queue_mapping > > recv path reassigns skb->dev without sanity checking the > queue_mapping field. This can result in the queue_mapping > field being set incorrectly if the new dev supports less > queues then the underlying device. > > This patch just resets the queue_mapping to 0 which should > resolve this issue? Any thoughts? > > The same issue could happen on bonding devices as well. > > Signed-off-by: John Fastabend > --- > > net/8021q/vlan_core.c | 6 ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c > index bd537fc..ad309f8 100644 > --- a/net/8021q/vlan_core.c > +++ b/net/8021q/vlan_core.c > @@ -21,6 +21,9 @@ int __vlan_hwaccel_rx(struct sk_buff *skb, struct > vlan_group *grp, > if (!skb->dev) > goto drop; > > + if (unlikely(skb->queue_mapping >= skb->dev->real_num_tx_queues)) > + skb_set_queue_mapping(skb, 0); > + > return (polling ? netif_receive_skb(skb) : netif_rx(skb)); > > drop: > @@ -93,6 +96,9 @@ vlan_gro_common(struct napi_struct *napi, struct > vlan_group *grp, > if (!skb->dev) > goto drop; > > + if (unlikely(skb->queue_mapping >= skb->dev->real_num_tx_queues)) > + skb_set_queue_mapping(skb, 0); > + > for (p = napi->gro_list; p; p = p->next) { > NAPI_GRO_CB(p)->same_flow = > p->dev == skb->dev && !compare_ether_header( > -- Only a workaround, added in hot path in a otherwise 'good' driver (multiqueue enabled and ready) eth0 -------> bond / bridge ---------> vlan.id (nbtxq=8) (ntxbq=1) (nbtxq=X) X is capped to 1 because of bond/bridge, while bond has no "queue" (LLTX driver) Solutions : 1) queue_mapping could be silently tested in get_rps_cpu()... --- 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 6f330ce..3a3f7f6 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -2272,14 +2272,11 @@ static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb, if (skb_rx_queue_recorded(skb)) { u16 index = skb_get_rx_queue(skb); - if (unlikely(index >= dev->num_rx_queues)) { - if (net_ratelimit()) { - pr_warning("%s received packet on queue " - "%u, but number of RX queues is %u\n", - dev->name, index, dev->num_rx_queues); - } - goto done; - } + if (WARN_ONCE(index >= dev->num_rx_queues, + KERN_WARNING "%s received packet on queue %u, " + "but number of RX queues is %u\n", + dev->name, index, dev->num_rx_queues)) + index %= dev->num_rx_queues; rxqueue = dev->_rx + index; } else rxqueue = dev->_rx; 2) bond/bridge should setup more queues, just in case. We probably need to be able to make things more dynamic, (propagate nbtxq between layers) but not for 2.6.35 diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 5e12462..ce813dd 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -5012,8 +5012,8 @@ int bond_create(struct net *net, const char *name) rtnl_lock(); - bond_dev = alloc_netdev(sizeof(struct bonding), name ? name : "", - bond_setup); + bond_dev = alloc_netdev_mq(sizeof(struct bonding), name ? name : "", + bond_setup, max(64, nr_cpu_ids)); if (!bond_dev) { pr_err("%s: eek! can't alloc netdev!\n", name); rtnl_unlock();