[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <311b59aee7d648c6124a84b5ca06ac60@dondevamos.com>
Date: Wed, 16 Jun 2010 10:49:27 +0200
From: Pedro Garcia <pedro.netdev@...devamos.com>
To: <netdev@...r.kernel.org>
Cc: Patrick McHardy <kaber@...sh.net>,
Ben Hutchings <bhutchings@...arflare.com>,
Eric Dumazet <eric.dumazet@...il.com>
Subject: Re: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag" (802.1p packet)
On Mon, 14 Jun 2010 21:12:52 +0200, Eric Dumazet <eric.dumazet@...il.com>
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
>> >> <bhutchings@...arflare.com> 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 <pedro.netdev@...devamos.com>
--
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;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists