From patchwork Wed Jun 16 08:49:27 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Pedro Garcia X-Patchwork-Id: 55864 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 9B5DDB7D85 for ; Wed, 16 Jun 2010 18:49:44 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755048Ab0FPIte (ORCPT ); Wed, 16 Jun 2010 04:49:34 -0400 Received: from 13.Red-213-97-209.staticIP.rima-tde.net ([213.97.209.13]:58819 "EHLO smtp1.dondevamos.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754820Ab0FPItc (ORCPT ); Wed, 16 Jun 2010 04:49:32 -0400 Received: from localhost ([127.0.0.1] helo=webmail.dondevamos.com) by smtp1.dondevamos.com with esmtp (Exim 4.69) (envelope-from ) id 1OOoJL-0001Ja-FE; Wed, 16 Jun 2010 10:49:29 +0200 MIME-Version: 1.0 Date: Wed, 16 Jun 2010 10:49:27 +0200 From: Pedro Garcia To: Cc: Patrick McHardy , Ben Hutchings , Eric Dumazet Subject: Re: [PATCH] =?UTF-8?Q?vlan=5Fdev=3A=20VLAN=20=30=20should=20be=20treated?= =?UTF-8?Q?=20as=20=22no=20vlan=20tag=22=20=28=38=30=32=2E=31p=20packet=29?= In-Reply-To: <1276542772.2444.13.camel@edumazet-laptop> References: <1276466190.14011.223.camel@localhost> <5c6d1ac43fd8ad25661ebfba57c02174@dondevamos.com> <1276534945.2074.11.camel@achroite.uk.solarflarecom.com> <4C1662C3.3070708@trash.net> <1276542772.2444.13.camel@edumazet-laptop> Message-ID: <311b59aee7d648c6124a84b5ca06ac60@dondevamos.com> X-Sender: pedro.netdev@dondevamos.com User-Agent: RoundCube Webmail/0.3.1 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Mon, 14 Jun 2010 21:12:52 +0200, Eric Dumazet wrote: > Le lundi 14 juin 2010 à 19:11 +0200, Patrick McHardy a écrit : >> Ben Hutchings wrote: >> > On Mon, 2010-06-14 at 18:49 +0200, Pedro Garcia wrote: >> > >> >> On Sun, 13 Jun 2010 22:56:30 +0100, Ben Hutchings >> >> wrote: >> >> >> >>> I have no particular opinion on this change, but you need to read and >> >>> follow Documentation/SubmittingPatches. >> >>> >> >>> Ben. >> >>> >> >> Sorry, first kernel patch, and I did not know about it. I resubmit >> >> with >> >> the correct style / format: >> >> >> > [...] >> > >> > Sorry, no you haven't. >> > >> > - Networking changes go through David Miller's net-next-2.6 tree so you >> > need to use that as the baseline, not 2.6.26 >> > - Patches should be applicable with -p1, not -p0 (so if you use diff, >> > you should run it from one directory level up) >> > - The patch was word-wrapped >> >> Additionally: >> >> - please use the proper comment style, meaning each line begins >> with a '*' >> >> - the pr_debug statements may be useful for debugging, but are >> a bit excessive for the final version >> >> - + /* 2010-06-13: Pedro Garcia >> >> We have changelogs for this, simply explaining what the code >> does is enough. >> >> - Please CC the maintainer (which is me) >> -- > > Pedro, we have two kind of vlan setups : > > accelerated and non accelerated ones. > > Your patch address non accelated ones only, since you only touch > vlan_skb_recv() > > Accelerated vlan can follow these paths : > > 1) NAPI devices > > vlan_gro_receive() -> vlan_gro_common() > > 2) non NAPI devices > > __vlan_hwaccel_rx() > > So you might also patch __vlan_hwaccel_rx() and vlan_gro_common() > > Please merge following bits to your patch submission : > > http://kerneltrap.org/mailarchive/linux-netdev/2010/5/23/6277868 > > > Good luck for your first patch ! Here it is again. I added the modifications in http://kerneltrap.org/mailarchive/linux-netdev/2010/5/23/6277868 for HW accelerated incoming packets (it did not apply cleanly on the last version of the kernel, so I applied manually). Now, if the VLAN 0 is not explicitly created by the user, VLAN 0 packets will be treated as no VLAN (802.1p packets), instead of dropping them. The patch is now for two files: vlan_core (accel) and vlan_dev (non accel) I can not test on HW accelerated devices, so if someone can check it I will appreciate (even though in the thread above it looked like yes). For non accel I tessted in 2.6.26. Now the patch is for net-next-2.6, and it compiles OK, but I a have to setup a test environment to check it is still OK (should, but better to test). Signed-off-by: Pedro Garcia --- -- 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/8021q/vlan_core.c b/net/8021q/vlan_core.c index 50f58f5..daaca31 100644 --- a/net/8021q/vlan_core.c +++ b/net/8021q/vlan_core.c @@ -8,6 +8,9 @@ int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp, u16 vlan_tci, int polling) { + struct net_device *vlan_dev; + u16 vlan_id; + if (netpoll_rx(skb)) return NET_RX_DROP; @@ -16,10 +19,14 @@ int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp, skb->skb_iif = skb->dev->ifindex; __vlan_hwaccel_put_tag(skb, vlan_tci); - skb->dev = vlan_group_get_device(grp, vlan_tci & VLAN_VID_MASK); + vlan_id = vlan_tci & VLAN_VID_MASK; + vlan_dev = vlan_group_get_device(grp, vlan_id); - if (!skb->dev) - goto drop; + if (vlan_dev) + skb->dev = vlan_dev; + else + if (vlan_id) + goto drop; return (polling ? netif_receive_skb(skb) : netif_rx(skb)); @@ -82,16 +89,22 @@ vlan_gro_common(struct napi_struct *napi, struct vlan_group *grp, unsigned int vlan_tci, struct sk_buff *skb) { struct sk_buff *p; + struct net_device *vlan_dev; + u16 vlan_id; if (skb_bond_should_drop(skb, ACCESS_ONCE(skb->dev->master))) skb->deliver_no_wcard = 1; skb->skb_iif = skb->dev->ifindex; __vlan_hwaccel_put_tag(skb, vlan_tci); - skb->dev = vlan_group_get_device(grp, vlan_tci & VLAN_VID_MASK); - - if (!skb->dev) - goto drop; + vlan_id = vlan_tci & VLAN_VID_MASK; + vlan_dev = vlan_group_get_device(grp, vlan_id); + + if (vlan_dev) + skb->dev = vlan_dev; + else + if (vlan_id) + goto drop; for (p = napi->gro_list; p; p = p->next) { NAPI_GRO_CB(p)->same_flow = diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c index 5298426..65512c3 100644 --- a/net/8021q/vlan_dev.c +++ b/net/8021q/vlan_dev.c @@ -142,6 +142,7 @@ int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev, { struct vlan_hdr *vhdr; struct vlan_rx_stats *rx_stats; + struct net_device *vlan_dev; u16 vlan_id; u16 vlan_tci; @@ -157,8 +158,18 @@ int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev, vlan_id = vlan_tci & VLAN_VID_MASK; rcu_read_lock(); - skb->dev = __find_vlan_dev(dev, vlan_id); - if (!skb->dev) { + vlan_dev = __find_vlan_dev(dev, vlan_id); + + /* If the VLAN device is defined, we use it. + * If not, and the VID is 0, it is a 802.1p packet (not + * really a VLAN), so we will just netif_rx it later to the + * original interface, but with the skb->proto set to the + * wrapped proto: we do nothing here. + */ + + if (vlan_dev) { + skb->dev = vlan_dev; + } else if (vlan_id) { pr_debug("%s: ERROR: No net_device for VID: %u on dev: %s\n", __func__, vlan_id, dev->name); goto err_unlock;