On Wed, Aug 03, 2016 at 10:25:21AM -0400, Eric Garver wrote: > On Sun, Jul 31, 2016 at 08:22:47AM +0800, Xiao Liang wrote: > > On Thu, Jul 28, 2016 at 2:40 AM, Ben Pfaff <b...@ovn.org> wrote: > > > On Tue, Jul 12, 2016 at 11:38:54PM +0800, Xiao Liang wrote: > .. snip .. > > > I'm concerned about backward compatibility. Consider some application > > > built on Open vSwitch using OpenFlow. Today, it can distinguish > > > single-tagged and double-tagged packets (that use outer Ethertype > > > 0x8100), as follows: > > > > > > - A single-tagged packet has vlan_tci != 0 and some non-VLAN > > > dl_type. > > > > > > - A double-tagged packet has vlan_tci != 0 and a VLAN dl_type. > > > > > > With this patch, this won't work, because neither kind of packet has a > > > VLAN dl_type. Instead, applications need to first match on the outer > > > VLAN, then pop it off, then match on the inner VLAN. This difference > > > could lead to security problems in applications. An application > > > might, for example, want to pop an outer VLAN and forward the packet, > > > but only if there is no inner VLAN. If it is implemented according to > > > the previous rules, then it will not notice the inner VLAN. > > > > Maybe some applications are implemented this way, but they are > > probably wrong. OpenFlow says eth_type is "ethernet type of the > > OpenFlow packet payload, after VLAN tags", so it is the payload > > ethtype for a double-tagged packet. It's the same for single-tagged > > packet: application must explicitly match vlan_tci to decide whether > > it has VLAN tag. > > The problem is that there is currently no way to peek inner VLAN > > without popping the outer one. I heard from Tom that you have plan to > > support TPID matching. Is it possible to add extension match fields > > like TPID1, TPID2 to match both TPIDs? This may also provide a way to > > differentiate 0x8100 and 0x88a8. > > I'm in agreement with Xiao here.
I gave a response directly to Xiao. Backward incompatibility is one thing but adding gratuitous security vulnerabilities to existing applications that have working since day one is not acceptable. > > > There are probably multiple ways to deal with this problem. Let me > > > propose one. It is somewhat awkward, so there might be a more > > > graceful way. Basically the idea is to retain the current view from > > > an OpenFlow perspective: > > > > > > - Packet without tags: vlan_tci == 0, dl_type is non-VLAN > > > - Packet with 1 tag: vlan_tci != 0, dl_type is non-VLAN > > > - Packet with 2+ tags: vlan_tci != 0, dl_type is 2nd VLAN > > > > > > So, when a packet with 2 tags pops off the outermost one, dl_type > > > becomes the inner Ethertype (such as IPv4) and vlan_tci becomes the > > > single remaining VLAN. > > > > > > I think there's another backward compatibility risk. This patch adds > > > support for TPID 0x88a8 without adding any way for OpenFlow > > > applications to distinguish 88a8 from 8100. This means that > > > applications that previously handled 0x8100 VLANs will now handle > > > 0x88a8 VLANs whereas previously they were able to filter these out. I > > > don't have a wonderful idea on how to handle this but I think we need > > > some way. (The OpenFlow spec is totally unhelpful here.) > > The OF 1.1 spec reads "the outermost VLAN tag", which is ambiguous with > 802.1ad. > > However, OF 1.5 clarifies to say the "outermost _802.1q_ tag". See pages > 77 and 205 of the spec. To me this implies that OF does not specify > matching the 802.1ad tag at all (i.e vlan_tci=.. should match frames > with outer TPID 0x8100, but not 0x88a8). It does specify pushing/popping > 802.1ad though. > > I believe if we follow the OF 1.5 definition it removes most (all?) > backwards compatibility issues raised by Ben. But we also can't match on > 0x88a8 VLANs. Maybe that can be solved with out-of-spec explicit TPID > matching like Xiao mentioned above. The OpenFlow specs aren't written from this kind of language-lawyer standpoint. When they say 802.1Q they just mean VLAN. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
