[ovs-git] Open vSwitch: dpif-netdev: Properly track whether there is a vlan header. (master)

dev at openvswitch.org dev at openvswitch.org
Tue Aug 10 20:41:45 UTC 2010

This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Open vSwitch".

The branch, master has been updated
       via  142181fcd734a2afff9fe13e54fe51c7a2c824d2 (commit)
       via  1805876e50b89816581dac16d7283bd130b84c72 (commit)
       via  50f06e1642e246bb9969035c9e3db7c44e7b95e0 (commit)
      from  eca2df312b03f39a28d361682b692a3dc547a79a (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 142181fcd734a2afff9fe13e54fe51c7a2c824d2
Diffs: http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commitdiff;h=142181fcd734a2afff9fe13e54fe51c7a2c824d2
Author: Ben Pfaff <blp at nicira.com>
dpif-netdev: Properly track whether there is a vlan header.
It looks to me like the current dpif-netdev implementation doesn't handle
the case where a packet comes in without a VLAN and then is subjected to
multiple ODPAT_SET_VLAN_* operations.  dp_netdev_modify_vlan_tci() just
checks the flow key each time to see whether there's a VLAN, but it doesn't
update the flow key to note that there is now a VLAN.

One fix would be to update the flow key, but it's "const" these days.
Instead, add a check for whether the Ethernet type is ETH_TYPE_VLAN,
which should be equivalent.

commit 1805876e50b89816581dac16d7283bd130b84c72
Diffs: http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commitdiff;h=1805876e50b89816581dac16d7283bd130b84c72
Author: Ben Pfaff <blp at nicira.com>
dpif-netdev: Tolerate undersized packets.
Actions that modify packets need to tolerate packets that are too small.
Most of the actions already implicitly do this check, since they check for
appropriate values in the flow key that would only be there if the
corresponding data was present.  But actions to modify the Ethernet header
didn't have a guarantee that the packet was at least 14 bytes long, and
actions to modify the VLAN didn't have such a guarantee either, so this
adds appropriate checks.

Problem found by code inspection.

commit 50f06e1642e246bb9969035c9e3db7c44e7b95e0
Diffs: http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commitdiff;h=50f06e1642e246bb9969035c9e3db7c44e7b95e0
Author: Ben Pfaff <blp at nicira.com>
datapath: Fix handling of 802.1Q and SNAP headers.
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.

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

I verified that, with this change, both SNAP and Ethernet packets are
properly recognized both with and without 802.1Q encapsulation.

I was a bit surprised to find out that Linux does not accept
SNAP-encapsulated IP frames on Ethernet.

Here's a summary of how frames are handled before and after this commit:

Common cases

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|
    +------------+ +---------+ +--------+ +-----------+


   ---------------  ---------------  -------------------------------------
       Before           After
     this commit      this commit
   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

Signed-off-by: Ben Pfaff <blp at nicira.com>


Summary of changes:
 datapath/flow.c    |  100 ++++++++++++++-----------
 lib/dpif-netdev.c  |   22 ++++--
 lib/flow.c         |  216 +++++++++++++++++++++++++++-------------------------
 tests/flowgen.pl   |   14 ++-
 tests/test-flows.c |    1 +
 5 files changed, 192 insertions(+), 161 deletions(-)

Open vSwitch

More information about the git mailing list