[ovs-dev] [VLAN/SNAP 1/3] datapath: Fix handling of 802.1Q and SNAP headers.

[Ben Pfaff]

On Wed, Aug 04, 2010 at 02:06:49PM -0700, Jesse Gross wrote:
> On Wed, Jul 28, 2010 at 2:01 PM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > The kernel and user datapaths have code that assumes that 802.1Q headers
> > are used only inside Ethernet II frames, not inside SNAP-encapsulated
> > frames.  But the kernel and user flow_extract() implementations would
> > interpret 802.1Q headers inside SNAP headers as being valid VLANs.  This
> > would cause packet corruption if any VLAN-related actions were to be taken,
> > so change the two flow_extract() implementations only to accept 802.1Q as
> > an Ethernet II frame type, not as a SNAP-encoded frame type.
> >
> > 802.1Q-2005 says that this is correct anyhow:
> >
> >    Where the ISS instance used to transmit and receive tagged frames is
> >    provided by a media access control method that can support Ethernet
> >    Type encoding directly (e.g., is an IEEE 802.3 or IEEE 802.11 MAC) or
> >    is media access method independent (e.g., 6.6), the TPID is Ethernet
> >    Type encoded, i.e., is two octets in length and comprises solely the
> >    assigned Ethernet Type value.
> >
> >    Where the ISS instance is provided by a media access method that
> >    cannot directly support Ethernet Type encoding (e.g., is an IEEE
> >    802.5 or FDDI MAC), the TPID is encoded according to the rule for
> >    a Subnetwork Access Protocol (Clause 10 of IEEE Std 802) that
> >    encapsulates Ethernet frames over LLC, and comprises the SNAP
> >    header (AA-AA-03) followed by the SNAP PID (00-00-00) followed by
> >    the two octets of the assigned Ethernet Type value.
> >
> > All of the media that OVS handles supports Ethernet Type fields, so to me
> > that means that we don't have to handle 802.1Q-inside-SNAP.
> 
> I'm fairly certain that it is possible for us to receive non-Ethernet II
> 802.3 packets from legacy devices.  This is really the only purpose of SNAP
> (I suppose that you could define your own protocol that doesn't have an
> EtherType and put it inside of SNAP but that seems like even more of an edge
> case), so if we are going to support SNAP in general then that implicitly
> means that we expect to potentially get non-Ethernet II frames.  Linux may
> not really handle SNAP packets but it does at least check for them on
> Ethernet devices (in eth_type_trans()), which means that it thinks in might
> get them.

We have a function to compose SNAP non-Ethernet frames:
compose_benign_packet().  But I would certainly consider that an edge
case.

> > On the other hand, we *do* have to handle SNAP-inside-802.1Q, because this
> > is actually allowed by the standards.  So this commit also adds that
> > support.
> >
> 
> This seems like a really weird case to me as it implies that you have a
> device that supports an EtherType (because you are using it for the VLAN
> TPID) but the packet isn't using it inside the VLAN.  The only use case that
> I can think of is if you have a legacy device connected to an access port of
> a modern switch and the switch is putting the SNAP encoded packets onto a
> VLAN.  Is this the use case that you were imagining?

I think that there's some confusion here.  I compiled a list of packet
framing formats that we are interested in, along with how they are
interpreted currently and how they would be interpreted after my commit
is applied, with an analysis of the changes.  Here it is.  Does this
clear the air?  (I'll add this to the change log if we commit this.)

Common cases
------------

       Ethernet
    +------------+
1.  |dst|src|TYPE|
    +------------+

       Ethernet       LLC         SNAP
    +------------+ +--------+ +-----------+
2.  |dst|src| len| |aa|aa|03| |000000|TYPE|
    +------------+ +--------+ +-----------+

       Ethernet       802.1Q
    +------------+ +---------+
3.  |dst|src|8100| |VLAN|TYPE|
    +------------+ +---------+

       Ethernet       802.1Q      LLC         SNAP
    +------------+ +---------+ +--------+ +-----------+
4.  |dst|src|8100| |VLAN| LEN| |aa|aa|03| |000000|TYPE|
    +------------+ +---------+ +--------+ +-----------+

Unusual cases
-------------

       Ethernet       LLC         SNAP         802.1Q
    +------------+ +--------+ +-----------+ +---------+
5.  |dst|src| len| |aa|aa|03| |000000|8100| |VLAN|TYPE|
    +------------+ +--------+ +-----------+ +---------+

       Ethernet       LLC
    +------------+ +--------+
6.  |dst|src| len| |xx|xx|xx|
    +------------+ +--------+

       Ethernet       LLC         SNAP
    +------------+ +--------+ +-----------+
7.  |dst|src| len| |aa|aa|03| |xxxxxx|xxxx|
    +------------+ +--------+ +-----------+

       Ethernet       802.1Q      LLC    
    +------------+ +---------+ +--------+
8.  |dst|src|8100| |VLAN| LEN| |xx|xx|xx|
    +------------+ +---------+ +--------+

       Ethernet       802.1Q      LLC         SNAP
    +------------+ +---------+ +--------+ +-----------+
9.  |dst|src|8100| |VLAN| LEN| |aa|aa|03| |xxxxxx|xxxx|
    +------------+ +---------+ +--------+ +-----------+


Behavior
--------

   ---------------  ---------------
       Current         Proposed
   dl_type dl_vlan  dl_type dl_vlan  Notes
   ------- -------  ------- -------  -------------------------------------
1.   TYPE    ffff     TYPE    ffff   no change
2.   TYPE    ffff     TYPE    ffff   no change
3.   TYPE    VLAN     TYPE    VLAN   no change
4.    LEN    VLAN     TYPE    VLAN   proposal fixes behavior
5.   TYPE    VLAN     8100    ffff   802.1Q says this is invalid framing
6.   05ff    ffff     05ff    ffff   no change
7.   05ff    ffff     05ff    ffff   no change
8.    LEN    VLAN     05ff    VLAN   proposal fixes behavior
9.    LEN    VLAN     05ff    VLAN   proposal fixes behavior