[ovs-dev] [PATCH v4 1/4] Add support for 802.1ad (QinQ tunneling)
Ben Pfaff
blp at ovn.org
Wed Aug 3 22:18:19 UTC 2016
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 <blp at 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.
More information about the dev
mailing list