IGMP sent to Foreign VLAN

IGMP sent to Foreign VLAN

[Jarod Neuner]

I noticed a problem where the kernel is not responding to IGMP Queries
from several Level-3 network switches.  This behavior is caused by these
switches using VLAN tags on IGMP messages.  At first I thought the
switches were out of spec, but every other network stack I've checked
delivers packets with unconfigured VLAN assignments directly to the
incoming interface.  

This patch corrects my problem, but I'm still interested in discussion
about whether or not this should be a parameter in /proc/sys/net or even
if this policy change should be applied elsewhere in the the network
stack.

---

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 4bf014e..b65a8fd 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -158,9 +158,13 @@ int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
        rcu_read_lock();
        skb->dev = __find_vlan_dev(dev, vlan_id);
        if (!skb->dev) {
-               pr_debug("%s: ERROR: No net_device for VID: %u on dev: %s\n",
+               pr_debug("%s: WARNING: Forwarding VID: %u to dev: %s\n",
                         __func__, vlan_id, dev->name);
-               goto err_unlock;
+               skb->dev = dev;
+               skb_pull_rcsum(skb, VLAN_HLEN);
+               vlan_set_encap_proto(skb, vhdr);
+               rcu_read_lock();
+               return 0;
        }

        skb->dev->last_rx = jiffies;

Re: IGMP sent to Foreign VLAN

[David Miller]

From: Jarod Neuner <j.neuner@networkharbor.com>
Date: Mon, 6 Oct 2008 14:51:47 -0500

> I noticed a problem where the kernel is not responding to IGMP Queries
> from several Level-3 network switches.  This behavior is caused by these
> switches using VLAN tags on IGMP messages.  At first I thought the
> switches were out of spec, but every other network stack I've checked
> delivers packets with unconfigured VLAN assignments directly to the
> incoming interface.  
> 
> This patch corrects my problem, but I'm still interested in discussion
> about whether or not this should be a parameter in /proc/sys/net or even
> if this policy change should be applied elsewhere in the the network
> stack.

Interesting.

Patrick, any opinion?

> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index 4bf014e..b65a8fd 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -158,9 +158,13 @@ int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
>         rcu_read_lock();
>         skb->dev = __find_vlan_dev(dev, vlan_id);
>         if (!skb->dev) {
> -               pr_debug("%s: ERROR: No net_device for VID: %u on dev: %s\n",
> +               pr_debug("%s: WARNING: Forwarding VID: %u to dev: %s\n",
>                          __func__, vlan_id, dev->name);
> -               goto err_unlock;
> +               skb->dev = dev;
> +               skb_pull_rcsum(skb, VLAN_HLEN);
> +               vlan_set_encap_proto(skb, vhdr);
> +               rcu_read_lock();
> +               return 0;
>         }
> 
>         skb->dev->last_rx = jiffies;
> 
> 

Re: IGMP sent to Foreign VLAN

[Patrick McHardy]

David Miller wrote:
> From: Jarod Neuner <j.neuner@networkharbor.com>
> Date: Mon, 6 Oct 2008 14:51:47 -0500
>
>   
>> I noticed a problem where the kernel is not responding to IGMP Queries
>> from several Level-3 network switches.  This behavior is caused by these
>> switches using VLAN tags on IGMP messages.  At first I thought the
>> switches were out of spec, but every other network stack I've checked
>> delivers packets with unconfigured VLAN assignments directly to the
>> incoming interface.  
>>
>> This patch corrects my problem, but I'm still interested in discussion
>> about whether or not this should be a parameter in /proc/sys/net or even
>> if this policy change should be applied elsewhere in the the network
>> stack.
>>     
>
> Interesting.
>   

Indeed.
> Patrick, any opinion?
>   

I don't like a /proc flag very much, but I believe it should be
configurable. To make this behaviour consistent there also needs
to be some indication to drivers to disable hardware filters, so
we really should have an explicit action from the user first.

We've been talking about an IFF_ALLVLAN flag on netdev a while
ago, which would disable VLAN hardware filters, similar to
IFF_ALLMULTI. An additional flag on the ethernet device could
indicate that it should receive unknown VLANs directly. That
would introduce some possible inconsistencies however since the
flag could be set without the VLAN code even loaded, in which
case it would not have any effect.

Another possibility would be to use a catch-all VLAN device with
VID 0xfff (reserved for implementation use). This would allow
to configure priority mappings, header reordering etc. and have
separate statistics. The drivers could just use the magic VID
as an indication to disable filtering, but I would still suggest
to add the IFF_ALLVLAN flag because its useful on its own.

>> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
>> index 4bf014e..b65a8fd 100644
>> --- a/net/8021q/vlan_dev.c
>> +++ b/net/8021q/vlan_dev.c
>> @@ -158,9 +158,13 @@ int vlan_skb_recv(struct sk_buff *skb, struct net_device *dev,
>>         rcu_read_lock();
>>         skb->dev = __find_vlan_dev(dev, vlan_id);
>>         if (!skb->dev) {
>> -               pr_debug("%s: ERROR: No net_device for VID: %u on dev: %s\n",
>> +               pr_debug("%s: WARNING: Forwarding VID: %u to dev: %s\n",
>>                          __func__, vlan_id, dev->name);
>> -               goto err_unlock;
>> +               skb->dev = dev;
>> +               skb_pull_rcsum(skb, VLAN_HLEN);
>> +               vlan_set_encap_proto(skb, vhdr);
>> +               rcu_read_lock();
>> +               return 0;
>>     

This can't be right though, the packet needs to be passed
up the stack. The same change will also be needed in
__vlan_hwaccel_rx().

Re: IGMP sent to Foreign VLAN

[Jarod Neuner]

> Patrick McHardy wrote:
> We've been talking about an IFF_ALLVLAN flag on netdev a while
> ago, which would disable VLAN hardware filters, similar to
> IFF_ALLMULTI. An additional flag on the ethernet device could
> indicate that it should receive unknown VLANs directly. That
> would introduce some possible inconsistencies however since the
> flag could be set without the VLAN code even loaded, in which
> case it would not have any effect.

My original thought was to do something like this in net/core/dev.c
using a method similar to handle_bridge or handle_macvlen.  So, if the
packet doesn't get handled by the ptype_base list and IFF_ALLVLAN is
set, then strip the header and let the packet through.  The sticky point
would be whether or not this policy should be enabled by default, as it
seems to be in other network stacks.

> 
> Another possibility would be to use a catch-all VLAN device with
> VID 0xfff (reserved for implementation use). This would allow
> to configure priority mappings, header reordering etc. and have
> separate statistics. The drivers could just use the magic VID
> as an indication to disable filtering, but I would still suggest
> to add the IFF_ALLVLAN flag because its useful on its own.

Most switches treat VLAN 1 as the "Default" or "Administration" VLAN.
It might make sense to map VLAN 1 to the incoming interface, and then
use that as a catch all.  Then again, that might be a terrible idea as
well. =)

> This can't be right though, the packet needs to be passed
> up the stack. The same change will also be needed in
> __vlan_hwaccel_rx().

Duly noted - thanks.

Re: IGMP sent to Foreign VLAN

[Patrick McHardy]

Jarod Neuner wrote:
>> Patrick McHardy wrote:
>> We've been talking about an IFF_ALLVLAN flag on netdev a while
>> ago, which would disable VLAN hardware filters, similar to
>> IFF_ALLMULTI. An additional flag on the ethernet device could
>> indicate that it should receive unknown VLANs directly. That
>> would introduce some possible inconsistencies however since the
>> flag could be set without the VLAN code even loaded, in which
>> case it would not have any effect.
> 
> My original thought was to do something like this in net/core/dev.c
> using a method similar to handle_bridge or handle_macvlen.  So, if the
> packet doesn't get handled by the ptype_base list and IFF_ALLVLAN is
> set, then strip the header and let the packet through.  The sticky point
> would be whether or not this policy should be enabled by default, as it
> seems to be in other network stacks.

I don't think we should change the default, it would probably
catch some people by surprise. It might not be handled properly
by packet filtering rules etc.

>> Another possibility would be to use a catch-all VLAN device with
>> VID 0xfff (reserved for implementation use). This would allow
>> to configure priority mappings, header reordering etc. and have
>> separate statistics. The drivers could just use the magic VID
>> as an indication to disable filtering, but I would still suggest
>> to add the IFF_ALLVLAN flag because its useful on its own.
> 
> Most switches treat VLAN 1 as the "Default" or "Administration" VLAN.
> It might make sense to map VLAN 1 to the incoming interface, and then
> use that as a catch all.  Then again, that might be a terrible idea as
> well. =)

I prefer 0xfff because its not used for anything else so far.
Especially the administrative VLAN (even if only by convention)
doesn't seem by a good idea because its pretty likely you
would treat it differently from unknown VLANs wrt. filtering.

So .. would you be interested in implementing this properly?
I think its a good change and I could help you if needed or
take care of some parts like the drivers myself.

Re: IGMP sent to Foreign VLAN

[Jarod Neuner]

On Thu, 2008-10-09 at 07:31 -0500, Patrick McHardy wrote:
> Jarod Neuner wrote:
> > My original thought was to do something like this in net/core/dev.c
> > using a method similar to handle_bridge or handle_macvlen.  So, if the
> > packet doesn't get handled by the ptype_base list and IFF_ALLVLAN is
> > set, then strip the header and let the packet through.  The sticky point
> > would be whether or not this policy should be enabled by default, as it
> > seems to be in other network stacks.
> 
> I don't think we should change the default, it would probably
> catch some people by surprise. It might not be handled properly
> by packet filtering rules etc.

On the other hand, I was surprised that VLAN packets were being dropped
altogether.  Net admins tend to assign a link to a particular VLAN with
little regard to the VLAN configuration of the hosts on that link.  I'm
thinking of two general situations:

1) If the kernel is resident on an application device (PC, Multimedia
Device, SOHO Router, etc.), and a packet for a particular VLAN reaches
the network interface with a correct MAC and a correct IP, then they
were probably delivered correctly, whether that host is configured with
that VLAN ID or even if the VLAN module is loaded.

2) If the kernel is configured to route incoming VLAN packets, and a
packet arrives with an unconfigured VLAN ID, then it seems perfectly
reasonable to route it as if it had no VLAN tag.

I'm sure someone has a setup that expects that foreign VLANs will be
dropped - but I suspect far more are generally indifferent to the
policy.  There might even be a handful that will be pleasantly surprised
when IGMP snooping suddenly starts to work.

> > Most switches treat VLAN 1 as the "Default" or "Administration" VLAN.
> > It might make sense to map VLAN 1 to the incoming interface, and then
> > use that as a catch all.  Then again, that might be a terrible idea as
> > well. =)
> 
> I prefer 0xfff because its not used for anything else so far.
> Especially the administrative VLAN (even if only by convention)
> doesn't seem by a good idea because its pretty likely you
> would treat it differently from unknown VLANs wrt. filtering.

Agreed.

> So .. would you be interested in implementing this properly?
> I think its a good change and I could help you if needed or
> take care of some parts like the drivers myself.

I've got quite a bit on my plate at the moment, but I will give it a
shot.  I'll try to come up with some of the IFF_ALLVLAN functionality
over the next few days and get back to you.

Re: IGMP sent to Foreign VLAN

[Patrick McHardy]

Jarod Neuner wrote:
> On Thu, 2008-10-09 at 07:31 -0500, Patrick McHardy wrote:
>> I don't think we should change the default, it would probably
>> catch some people by surprise. It might not be handled properly
>> by packet filtering rules etc.
> 
> On the other hand, I was surprised that VLAN packets were being dropped
> altogether.  Net admins tend to assign a link to a particular VLAN with
> little regard to the VLAN configuration of the hosts on that link.  I'm
> thinking of two general situations:
> 
> 1) If the kernel is resident on an application device (PC, Multimedia
> Device, SOHO Router, etc.), and a packet for a particular VLAN reaches
> the network interface with a correct MAC and a correct IP, then they
> were probably delivered correctly, whether that host is configured with
> that VLAN ID or even if the VLAN module is loaded.
> 
> 2) If the kernel is configured to route incoming VLAN packets, and a
> packet arrives with an unconfigured VLAN ID, then it seems perfectly
> reasonable to route it as if it had no VLAN tag.
> 
> I'm sure someone has a setup that expects that foreign VLANs will be
> dropped - but I suspect far more are generally indifferent to the
> policy.  There might even be a handful that will be pleasantly surprised
> when IGMP snooping suddenly starts to work.

Possible, but besides the fact that this has been our default
since even before VLAN was merged, a reason why this absolutely
has to be manually enabled is that it requires to disable hardware
filters for consistent, driver-independant behaviour.

>> So .. would you be interested in implementing this properly?
>> I think its a good change and I could help you if needed or
>> take care of some parts like the drivers myself.
> 
> I've got quite a bit on my plate at the moment, but I will give it a
> shot.  I'll try to come up with some of the IFF_ALLVLAN functionality
> over the next few days and get back to you.

Great, thanks.

Re: IGMP sent to Foreign VLAN

[Benny Amorsen]

Jarod Neuner <j.neuner@networkharbor.com> writes:

> My original thought was to do something like this in net/core/dev.c
> using a method similar to handle_bridge or handle_macvlen.  So, if the
> packet doesn't get handled by the ptype_base list and IFF_ALLVLAN is
> set, then strip the header and let the packet through.  The sticky point
> would be whether or not this policy should be enabled by default, as it
> seems to be in other network stacks.

So if I tcpdump on the base device, IFF_ALLVLAN gets set and packets
from unknown VLAN's suddenly lose their VLAN headers? That's a
security problem.


/Benny