lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 17 Nov 2019 20:30:27 -0800 From: Florian Fainelli <f.fainelli@...il.com> To: Vladimir Oltean <olteanv@...il.com>, andrew@...n.ch, vivien.didelot@...il.com, davem@...emloft.net Cc: netdev@...r.kernel.org Subject: Re: [RFC PATCH net-next] net: dsa: tag_8021q: Allow DSA tags and VLAN filtering simultaneously On 11/17/2019 1:14 PM, Vladimir Oltean wrote: > There are very good reasons to want this (see documentation reference to > br_if.c), and there are also very good reasons for not enabling it by > default. So a devlink param named best_effort_vlan_filtering, currently > driver-specific and exported only by sja1105, is used to configure this. > > In practice, this is perhaps the way that most users are going to use > the switch in. Best-effort untagged traffic can be bridged with any net > device in the system or terminated locally, and VLAN-tagged streams are > forwarded autonomously in a time-sensitive manner according to their > PCP (they need not transit the CPU). For those cases where the CPU needs > to terminate some VLAN-tagged traffic, using the DSA master is still an > option. > > A complication while implementing this was the fact that > __netif_receive_skb_core calls __vlan_hwaccel_clear_tag right before > passing the skb to the DSA packet_type handler. This means that the > tagger does not see the VLAN tag in the skb, nor in the skb meta data. > The patch that starting zeroing the skb VLAN tag is: > > commit d4b812dea4a236f729526facf97df1a9d18e191c > Author: Eric Dumazet <edumazet@...gle.com> > Date: Thu Jul 18 07:19:26 2013 -0700 > > vlan: mask vlan prio bits > > In commit 48cc32d38a52d0b68f91a171a8d00531edc6a46e > ("vlan: don't deliver frames for unknown vlans to protocols") > Florian made sure we set pkt_type to PACKET_OTHERHOST > if the vlan id is set and we could find a vlan device for this > particular id. > > But we also have a problem if prio bits are set. > > Steinar reported an issue on a router receiving IPv6 frames with a > vlan tag of 4000 (id 0, prio 2), and tunneled into a sit device, > because skb->vlan_tci is set. > > Forwarded frame is completely corrupted : We can see (8100:4000) > being inserted in the middle of IPv6 source address : > > 16:48:00.780413 IP6 2001:16d8:8100:4000:ee1c:0:9d9:bc87 > > 9f94:4d95:2001:67c:29f4::: ICMP6, unknown icmp6 type (0), length 64 > 0x0000: 0000 0029 8000 c7c3 7103 0001 a0ae e651 > 0x0010: 0000 0000 ccce 0b00 0000 0000 1011 1213 > 0x0020: 1415 1617 1819 1a1b 1c1d 1e1f 2021 2223 > 0x0030: 2425 2627 2829 2a2b 2c2d 2e2f 3031 3233 > > It seems we are not really ready to properly cope with this right now. > > We can probably do better in future kernels : > vlan_get_ingress_priority() should be a netdev property instead of > a per vlan_dev one. > > For stable kernels, lets clear vlan_tci to fix the bugs. > > Reported-by: Steinar H. Gunderson <sesse@...gle.com> > Signed-off-by: Eric Dumazet <edumazet@...gle.com> > Signed-off-by: David S. Miller <davem@...emloft.net> > > The patch doesn't say why "we are not really ready to properly cope with > this right now", and hence why the best solution is to remove the VLAN > tag from skb's that don't have a local VLAN sub-interface interested in > them. And I have no idea either. > > But the above patch has a loophole: if the VLAN tag is not > hw-accelerated, it isn't removed from the skb if there is no VLAN > sub-interface interested in it (our case). So we are hooking into the > .ndo_fix_features callback of the DSA master and clearing the rxvlan > offload feature, so the DSA tagger will always see the VLAN as part of > the skb data. This is symmetrical with the ETH_P_DSA_8021Q case and does > not need special treatment in the tagger. But perhaps the workaround is > brittle and might break if not understood well enough. > > The disabling of the rxvlan feature of the DSA master is unconditional. > The reasoning is that at first sight, no DSA master with regular frame > parsing abilities could be able to locate the VLAN tag with any of the > existing taggers anyway, and therefore, adding a property in dsa_switch > to control the rxvlan feature of the master would seem like useless > boilerplate. > > Signed-off-by: Vladimir Oltean <olteanv@...il.com> > --- [snip] > +best_effort_vlan_filtering > + [DEVICE, DRIVER-SPECIFIC] > + Allow plain ETH_P_8021Q headers to be used as DSA tags. > + Benefits: > + - Can terminate untagged traffic over switch net > + devices even when enslaved to a bridge with > + vlan_filtering=1. > + - Can do QoS based on VLAN PCP for autonomously > + forwarded frames. > + Drawbacks: > + - User cannot change pvid via 'bridge' commands. This > + would break source port identification on RX for > + untagged traffic. > + - User cannot use VLANs in range 1024-3071. If the > + switch receives frames with such VIDs, it will > + misinterpret them as DSA tags. > + - Cannot terminate VLAN-tagged traffic on local device. > + There is no way to deduce the source port from these. > + One could still use the DSA master though. Could we use QinQ to possibly solve these problems and would that work for your switch? I do not really mind being restricted to not being able to change the default_pvid or have a reduced VLAN range, but being able to test VLAN tags terminated on DSA slave network devices is a valuable thing to do. -- Florian
Powered by blists - more mailing lists