-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
Behalf Of Pedro Garcia
Sent: Wednesday, June 16, 2010 11:49 AM
To: netdev@vger.kernel.org
Cc: Patrick McHardy; Ben Hutchings; Eric Dumazet
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 [off-list ref]
wrote:

quoted

Le lundi 14 juin 2010 à 19:11 +0200, Patrick McHardy a écrit :

quoted

Ben Hutchings wrote:

quoted

On Mon, 2010-06-14 at 18:49 +0200, Pedro Garcia wrote:

quoted

On Sun, 13 Jun 2010 22:56:30 +0100, Ben Hutchings
[off-list ref] wrote:

quoted

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 <redacted>
--

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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html