[ovs-dev] [PATCH] Add OpenFlow support for 802.1AD Ethertype

Dave Benson dbenson at verdantnetworks.com
Fri Oct 10 14:06:29 UTC 2014


Previous behavior forced the use of the 802.1Q Ethertype for all VLAN headers.
In order to open the door for future support of VLAN stacking or Provider Based
Bridging, support of the 802.1AD Ethertype is a logical first step.

This patch adds support for OpenFlow flows to specify push_vlan actions which
may now specify the 0x88a8 Ethertype.  Userspace and fastpath forwarding are
supported.  The various mechanisms to dump flows correctly show the two
possible Ethertypes.

Bridging (aka normal mode) has not been addressed with this patch.

Testing:
1 Added a test to ofproto-dpif.at.
2 Tested on Xubuntu 14.04 (kernel version 3.13).  Created two OVS bridges with
  a physical ethernet between them.  Attached VM clients and passed ping and
  iperf traffic between the VMs.  Tested configurations using both possible
  VLAN Ethertypes.  Verified correct packet formats with wireshark.
  Verified the various dump-flow commands (ofctl, dpif and dpctl) returned the
  correct information.
3 Prior to the rebase to latest, testing was also done on Xubuntu 13.10 (kernel
  version 3.11).

This work was done independantly, although I read with interest the patches from
Thomas Herbert and Avinash Andrew and the related comments.  I believe this work
will be complementary to further efforts in this area.

Thank you for your consideration,
Dave Benson

Signed-off-by: Dave Benson <dbenson at verdantnetworks.com>
---
 NEWS                         |  8 +++++++-
 datapath/actions.c           |  4 ++--
 datapath/datapath.h          |  6 ++++++
 datapath/flow.c              |  8 +++++---
 datapath/flow.h              |  1 +
 datapath/flow_netlink.c      | 12 ++++++++----
 datapath/vlan.h              | 19 +++++++++++++++++++
 datapath/vport-netdev.c      |  2 +-
 lib/flow.c                   | 36 +++++++++++++++++++++++++++++-------
 lib/flow.h                   | 11 +++++++----
 lib/match.c                  |  2 +-
 lib/netdev-dummy.c           |  2 +-
 lib/nx-match.c               |  2 +-
 lib/odp-execute.c            |  7 ++++++-
 lib/odp-util.c               | 34 ++++++++++++++++++++++------------
 lib/odp-util.h               |  2 +-
 lib/ofp-actions.c            | 20 ++++++++------------
 lib/ofp-actions.h            | 10 +++++++++-
 lib/ofp-util.c               |  2 +-
 lib/packets.c                |  4 ++--
 lib/packets.h                |  8 +++++++-
 ofproto/ofproto-dpif-xlate.c |  6 +++---
 tests/ofproto-dpif.at        | 40 ++++++++++++++++++++++++++++++++++++++++
 utilities/ovs-ofctl.8.in     |  8 ++++++--
 24 files changed, 193 insertions(+), 61 deletions(-)

diff --git a/NEWS b/NEWS
index 74984a7..c37befa 100644
--- a/NEWS
+++ b/NEWS
@@ -36,7 +36,13 @@ Post-v2.3.0
      still needed, so this should not be enabled on production environments.
    - Stats are no longer updated on fake bond interface.
    - Keep active bond slave selection across OVS restart.
-
+   - OpenFlow support for 802.1AD Ethertype has been added.  It is now possible
+     to set the ethertype in the ovs-ofctl push_vlan action.  The ethertype
+     must be either 0x8100 (802.1Q) or 0x88a8 (802.1AD).  Both userspace
+     and fastpath forwarding is supported.  When using the push_vlan action,
+     the OpenFlow option must specify version 1.1 or greater.  This is true
+     not only to add flows but to dump them as well.  The other means for
+     dumping flows (ovs-appctl and ovs-dpctl) do not require any options.
 
 v2.3.0 - 14 Aug 2014
 ---------------------
diff --git a/datapath/actions.c b/datapath/actions.c
index b527cb6..967a90b 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -264,7 +264,7 @@ static int pop_vlan(struct sk_buff *skb, struct sw_flow_key *key)
 	if (likely(vlan_tx_tag_present(skb))) {
 		vlan_set_tci(skb, 0);
 	} else {
-		if (unlikely(skb->protocol != htons(ETH_P_8021Q) ||
+		if (unlikely(!vlan_valid_type(skb->protocol) ||
 			     skb->len < VLAN_ETH_HLEN))
 			return 0;
 
@@ -273,7 +273,7 @@ static int pop_vlan(struct sk_buff *skb, struct sw_flow_key *key)
 			return err;
 	}
 	/* move next vlan tag to hw accel tag */
-	if (likely(skb->protocol != htons(ETH_P_8021Q) ||
+	if (likely(!vlan_valid_type(skb->protocol) ||
 		   skb->len < VLAN_ETH_HLEN)) {
 		key->eth.tci = 0;
 		return 0;
diff --git a/datapath/datapath.h b/datapath/datapath.h
index 7dfd5af..eae6aef 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -182,6 +182,12 @@ static inline struct vport *ovs_vport_ovsl(const struct datapath *dp, int port_n
 	return ovs_lookup_vport(dp, port_no);
 }
 
+static inline bool vlan_valid_type(__be16 eth_type)
+{
+  return eth_type == htons(ETH_P_8021Q) ||
+    eth_type == htons(ETH_P_8021AD);
+}
+
 extern struct notifier_block ovs_dp_device_notifier;
 extern struct genl_family dp_vport_genl_family;
 extern struct genl_multicast_group ovs_dp_vport_multicast_group;
diff --git a/datapath/flow.c b/datapath/flow.c
index 62ce331..ca09d44 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -295,7 +295,7 @@ static bool icmp6hdr_ok(struct sk_buff *skb)
 static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
 {
 	struct qtag_prefix {
-		__be16 eth_type; /* ETH_P_8021Q */
+		__be16 eth_type;
 		__be16 tci;
 	};
 	struct qtag_prefix *qp;
@@ -308,6 +308,7 @@ static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
 		return -ENOMEM;
 
 	qp = (struct qtag_prefix *) skb->data;
+	key->eth.vlan_type = qp->eth_type;
 	key->eth.tci = qp->tci | htons(VLAN_TAG_PRESENT);
 	__skb_pull(skb, sizeof(struct qtag_prefix));
 
@@ -470,9 +471,10 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
  	 * update skb->csum here. */
 
 	key->eth.tci = 0;
-	if (vlan_tx_tag_present(skb))
+	if (vlan_tx_tag_present(skb)) {
 		key->eth.tci = htons(vlan_get_tci(skb));
-	else if (eth->h_proto == htons(ETH_P_8021Q))
+		key->eth.vlan_type = vlan_get_proto(skb);
+	} else if (vlan_valid_type(eth->h_proto))
 		if (unlikely(parse_vlan(skb, key)))
 			return -ENOMEM;
 
diff --git a/datapath/flow.h b/datapath/flow.h
index c78b864..299d5f5 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -137,6 +137,7 @@ struct sw_flow_key {
 		u8     src[ETH_ALEN];	/* Ethernet source address. */
 		u8     dst[ETH_ALEN];	/* Ethernet destination address. */
 		__be16 tci;		/* 0 if no VLAN, VLAN_TAG_PRESENT set otherwise. */
+		__be16 vlan_type;	/* VLAN ethertype. */
 		__be16 type;		/* Ethernet frame type. */
 	} eth;
 	union {
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index 762ea03..6de4561 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -978,7 +978,7 @@ int ovs_nla_get_match(struct sw_flow_match *match,
 
 	if ((key_attrs & (1ULL << OVS_KEY_ATTR_ETHERNET)) &&
 	    (key_attrs & (1ULL << OVS_KEY_ATTR_ETHERTYPE)) &&
-	    (nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]) == htons(ETH_P_8021Q))) {
+	    (vlan_valid_type(nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE])))) {
 		__be16 tci;
 
 		if (!((key_attrs & (1ULL << OVS_KEY_ATTR_VLAN)) &&
@@ -987,6 +987,9 @@ int ovs_nla_get_match(struct sw_flow_match *match,
 			return -EINVAL;
 		}
 
+		/* save the vlan ethertype */
+		SW_FLOW_KEY_PUT(match, eth.vlan_type,
+                                nla_get_be16(a[OVS_KEY_ATTR_ETHERTYPE]), false);
 		key_attrs &= ~(1ULL << OVS_KEY_ATTR_ETHERTYPE);
 		tci = nla_get_be16(a[OVS_KEY_ATTR_VLAN]);
 		encap = a[OVS_KEY_ATTR_ENCAP];
@@ -1195,9 +1198,10 @@ int ovs_nla_put_flow(const struct sw_flow_key *swkey,
 	ether_addr_copy(eth_key->eth_src, output->eth.src);
 	ether_addr_copy(eth_key->eth_dst, output->eth.dst);
 
-	if (swkey->eth.tci || swkey->eth.type == htons(ETH_P_8021Q)) {
+	if (swkey->eth.tci || vlan_valid_type(swkey->eth.vlan_type)) {
 		__be16 eth_type;
-		eth_type = !is_mask ? htons(ETH_P_8021Q) : htons(0xffff);
+		eth_type = swkey->eth.vlan_type;
+		eth_type = !is_mask ? eth_type : htons(0xffff);
 		if (nla_put_be16(skb, OVS_KEY_ATTR_ETHERTYPE, eth_type) ||
 		    nla_put_be16(skb, OVS_KEY_ATTR_VLAN, output->eth.tci))
 			goto nla_put_failure;
@@ -1837,7 +1841,7 @@ static int __ovs_nla_copy_actions(const struct nlattr *attr,
 
 		case OVS_ACTION_ATTR_PUSH_VLAN:
 			vlan = nla_data(a);
-			if (vlan->vlan_tpid != htons(ETH_P_8021Q))
+			if (!vlan_valid_type(vlan->vlan_tpid))
 				return -EINVAL;
 			if (!(vlan->vlan_tci & htons(VLAN_TAG_PRESENT)))
 				return -EINVAL;
diff --git a/datapath/vlan.h b/datapath/vlan.h
index 13ae6a7..7e5d4b9 100644
--- a/datapath/vlan.h
+++ b/datapath/vlan.h
@@ -62,4 +62,23 @@ static inline void vlan_set_tci(struct sk_buff *skb, u16 vlan_tci)
 #endif
 	skb->vlan_tci = vlan_tci;
 }
+
+/*
+ * In kernel 3.10, vlan_proto was added to the skbuff.
+ * We can use it to get the vlan ethertype.
+ */
+static inline u16 vlan_get_proto(struct sk_buff *skb)
+{
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,10,0)
+	if (skb->vlan_proto)
+		return skb->vlan_proto;
+#endif
+	if ((skb->protocol) && ((skb->protocol == htons(ETH_P_8021Q)) ||
+                                (skb->protocol == htons(ETH_P_8021AD)))) {
+	    return skb->protocol;
+	} else {
+	    return htons(ETH_P_8021Q);
+	}
+}
+
 #endif /* vlan.h */
diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
index c763491..3877b61 100644
--- a/datapath/vport-netdev.c
+++ b/datapath/vport-netdev.c
@@ -221,7 +221,7 @@ static unsigned int packet_length(const struct sk_buff *skb)
 {
 	unsigned int length = skb->len - ETH_HLEN;
 
-	if (skb->protocol == htons(ETH_P_8021Q))
+	if (vlan_valid_type(skb->protocol))
 		length -= VLAN_HLEN;
 
 	return length;
diff --git a/lib/flow.c b/lib/flow.c
index 7fb53de..f022f11 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -217,21 +217,24 @@ parse_mpls(void **datap, size_t *sizep)
 }
 
 static inline ovs_be16
-parse_vlan(void **datap, size_t *sizep)
+parse_vlan(void **datap, size_t *sizep, ovs_be16 *tpidp)
 {
     const struct eth_header *eth = *datap;
 
     struct qtag_prefix {
-        ovs_be16 eth_type;      /* ETH_TYPE_VLAN */
+        ovs_be16 eth_type;
         ovs_be16 tci;
     };
 
     data_pull(datap, sizep, ETH_ADDR_LEN * 2);
 
-    if (eth->eth_type == htons(ETH_TYPE_VLAN)) {
+    *tpidp = 0;
+
+    if (eth_type_vlan(eth->eth_type)) {
         if (OVS_LIKELY(*sizep
                        >= sizeof(struct qtag_prefix) + sizeof(ovs_be16))) {
             const struct qtag_prefix *qp = data_pull(datap, sizep, sizeof *qp);
+            *tpidp = qp->eth_type;
             return qp->tci | htons(VLAN_CFI);
         }
     }
@@ -394,16 +397,20 @@ miniflow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,
         goto out;
     } else {
         ovs_be16 vlan_tci;
+        ovs_be16 vlan_tpid = 0;
 
         /* Link layer. */
         BUILD_ASSERT(offsetof(struct flow, dl_dst) + 6
                      == offsetof(struct flow, dl_src));
         miniflow_push_words(mf, dl_dst, data, ETH_ADDR_LEN * 2 / 4);
         /* dl_type, vlan_tci. */
-        vlan_tci = parse_vlan(&data, &size);
+        vlan_tci = parse_vlan(&data, &size, &vlan_tpid);
         dl_type = parse_ethertype(&data, &size);
         miniflow_push_be16(mf, dl_type, dl_type);
         miniflow_push_be16(mf, vlan_tci, vlan_tci);
+        miniflow_push_be16(mf, vlan_tpid, vlan_tpid);
+        /* need to do push of be16s in pairs */
+        miniflow_push_be16(mf, vlan_pad, 0);
     }
 
     /* Parse mpls. */
@@ -668,7 +675,7 @@ flow_unwildcard_tp_ports(const struct flow *flow, struct flow_wildcards *wc)
 void
 flow_get_metadata(const struct flow *flow, struct flow_metadata *fmd)
 {
-    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 27);
+    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 28);
 
     fmd->dp_hash = flow->dp_hash;
     fmd->recirc_id = flow->recirc_id;
@@ -1275,6 +1282,21 @@ flow_hash_in_wildcards(const struct flow *flow,
     return hash_finish(hash, 4 * FLOW_U32S);
 }
 
+/* Return the value for the vlan_tpid, or 0 if this flow has no vlan tag.
+ * Return a safe value of ETH_TYPE_VLAN_8021Q if the vlan tpid is invalid.
+ */
+ovs_be16
+flow_get_vlan_tpid(const struct flow *flow)
+{
+    if (!flow->vlan_tci & htons(VLAN_CFI)) {
+        return 0;
+    } else if (eth_type_vlan(flow->vlan_tpid)) {
+        return flow->vlan_tpid;
+    } else {
+        return htons(ETH_TYPE_VLAN_8021Q);
+    }
+}
+
 /* Sets the VLAN VID that 'flow' matches to 'vid', which is interpreted as an
  * OpenFlow 1.0 "dl_vlan" value:
  *
@@ -1458,7 +1480,7 @@ flow_push_mpls(struct flow *flow, int n, ovs_be16 mpls_eth_type,
         flow->mpls_lse[0] = set_mpls_lse_values(ttl, tc, 1, htonl(label));
 
         /* Clear all L3 and L4 fields. */
-        BUILD_ASSERT(FLOW_WC_SEQ == 27);
+        BUILD_ASSERT(FLOW_WC_SEQ == 28);
         memset((char *) flow + FLOW_SEGMENT_2_ENDS_AT, 0,
                sizeof(struct flow) - FLOW_SEGMENT_2_ENDS_AT);
     }
@@ -1645,7 +1667,7 @@ flow_compose(struct ofpbuf *b, const struct flow *flow)
     }
 
     if (flow->vlan_tci & htons(VLAN_CFI)) {
-        eth_push_vlan(b, htons(ETH_TYPE_VLAN), flow->vlan_tci);
+        eth_push_vlan(b, flow_get_vlan_tpid(flow), flow->vlan_tci);
     }
 
     if (flow->dl_type == htons(ETH_TYPE_IP)) {
diff --git a/lib/flow.h b/lib/flow.h
index 2b053da..d43b614 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -38,7 +38,7 @@ struct pkt_metadata;
 /* This sequence number should be incremented whenever anything involving flows
  * or the wildcarding of flows changes.  This will cause build assertion
  * failures in places which likely need to be updated. */
-#define FLOW_WC_SEQ 27
+#define FLOW_WC_SEQ 28
 
 /* Number of Open vSwitch extension 32-bit registers. */
 #define FLOW_N_REGS 8
@@ -107,7 +107,9 @@ struct flow {
     uint8_t dl_dst[6];          /* Ethernet destination address. */
     uint8_t dl_src[6];          /* Ethernet source address. */
     ovs_be16 dl_type;           /* Ethernet frame type. */
-    ovs_be16 vlan_tci;          /* If 802.1Q, TCI | VLAN_CFI; otherwise 0. */
+    ovs_be16 vlan_tci;          /* VLAN TCI | VLAN_CFI; otherwise 0. */
+    ovs_be16 vlan_tpid;         /* VLAN Ethertype */
+    ovs_be16 vlan_pad;          /* vlan pad */
     ovs_be32 mpls_lse[FLOW_MAX_MPLS_LABELS]; /* MPLS label stack entry. */
 
     /* L3 */
@@ -153,8 +155,8 @@ BUILD_ASSERT_DECL(sizeof(struct flow) % 4 == 0);
 
 /* Remember to update FLOW_WC_SEQ when changing 'struct flow'. */
 BUILD_ASSERT_DECL(offsetof(struct flow, dp_hash) + sizeof(uint32_t)
-                  == sizeof(struct flow_tnl) + 176
-                  && FLOW_WC_SEQ == 27);
+                  == sizeof(struct flow_tnl) + 180
+                  && FLOW_WC_SEQ == 28);
 
 /* Incremental points at which flow classification may be performed in
  * segments.
@@ -210,6 +212,7 @@ static inline int flow_compare_3way(const struct flow *, const struct flow *);
 static inline bool flow_equal(const struct flow *, const struct flow *);
 static inline size_t flow_hash(const struct flow *, uint32_t basis);
 
+ovs_be16 flow_get_vlan_tpid(const struct flow *);
 void flow_set_dl_vlan(struct flow *, ovs_be16 vid);
 void flow_set_vlan_vid(struct flow *, ovs_be16 vid);
 void flow_set_vlan_pcp(struct flow *, uint8_t pcp);
diff --git a/lib/match.c b/lib/match.c
index 0eb11f0..5d4d58f 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -862,7 +862,7 @@ match_format(const struct match *match, struct ds *s, unsigned int priority)
 
     int i;
 
-    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 27);
+    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 28);
 
     if (priority != OFP_DEFAULT_PRIORITY) {
         ds_put_format(s, "priority=%u,", priority);
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index a2b1f2c..75f4179 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -869,7 +869,7 @@ netdev_dummy_send(struct netdev *netdev, int qid OVS_UNUSED,
             max_size = dev->mtu + ETH_HEADER_LEN;
             ovs_mutex_unlock(&dev->mutex);
 
-            if (eth->eth_type == htons(ETH_TYPE_VLAN)) {
+            if (eth_type_vlan(eth->eth_type)) {
                 max_size += VLAN_HEADER_LEN;
             }
             if (size > max_size) {
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 38233db..a08293c 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -809,7 +809,7 @@ nx_put_raw(struct ofpbuf *b, enum ofp_version oxm, const struct match *match,
     int match_len;
     int i;
 
-    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 27);
+    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 28);
 
     /* Metadata. */
     if (match->wc.masks.dp_hash) {
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index e2bc6de..c82f27b 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -467,7 +467,12 @@ odp_execute_actions(void *dp, struct dpif_packet **packets, int cnt,
             for (i = 0; i < cnt; i++) {
                 struct ofpbuf *buf = &packets[i]->ofpbuf;
 
-                eth_push_vlan(buf, htons(ETH_TYPE_VLAN), vlan->vlan_tci);
+                /* If vlan_tpid is not already VLAN, then use typical VLAN eth type */
+                if (eth_type_vlan(vlan->vlan_tpid)) {
+                    eth_push_vlan(buf, vlan->vlan_tpid, vlan->vlan_tci);
+                } else {
+                    eth_push_vlan(buf, htons(ETH_TYPE_VLAN), vlan->vlan_tci);
+                }
             }
             break;
         }
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 8f5ed08..0ecd5de 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -2600,11 +2600,16 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const struct flow *flow,
                                        sizeof *eth_key);
     get_ethernet_key(data, eth_key);
 
-    if (flow->vlan_tci != htons(0) || flow->dl_type == htons(ETH_TYPE_VLAN)) {
+    if (flow->vlan_tci != htons(0) || eth_type_vlan(flow->dl_type)) {
         if (export_mask) {
             nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, OVS_BE16_MAX);
         } else {
-            nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, htons(ETH_TYPE_VLAN));
+            /* If flow->dl_type is not already VLAN, then use typical VLAN eth type */
+            if (!eth_type_vlan(flow->dl_type)) {
+                nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, htons(ETH_TYPE_VLAN));
+            } else {
+                nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, flow->dl_type);
+            }
         }
         nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN, data->vlan_tci);
         encap = nl_msg_start_nested(buf, OVS_KEY_ATTR_ENCAP);
@@ -3298,8 +3303,12 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
     fitness = check_expectations(present_attrs, out_of_range_attr,
                                  expected_attrs, key, key_len);
 
-    /* Set vlan_tci.
-     * Remove the TPID from dl_type since it's not the real Ethertype.  */
+    /*
+     * Set vlan_tci.
+     * Remove the TPID from dl_type since it's not the real Ethertype.
+     * But also save it in the flow for vlan processing
+     */
+    flow->vlan_tpid = flow->dl_type;
     flow->dl_type = htons(0);
     flow->vlan_tci = (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_VLAN)
                       ? nl_attr_get_be16(attrs[OVS_KEY_ATTR_VLAN])
@@ -3425,7 +3434,7 @@ odp_flow_key_to_flow__(const struct nlattr *key, size_t key_len,
 
     if (is_mask
         ? (src_flow->vlan_tci & htons(VLAN_CFI)) != 0
-        : src_flow->dl_type == htons(ETH_TYPE_VLAN)) {
+        : eth_type_vlan(src_flow->dl_type)) {
         return parse_8021q_onward(attrs, present_attrs, out_of_range_attr,
                                   expected_attrs, flow, key, key_len, src_flow);
     }
@@ -3668,23 +3677,24 @@ pop_vlan(struct flow *base,
 }
 
 static void
-commit_vlan_action(ovs_be16 vlan_tci, struct flow *base,
+commit_vlan_action(const struct flow *flow, struct flow *base,
                    struct ofpbuf *odp_actions, struct flow_wildcards *wc)
 {
-    if (base->vlan_tci == vlan_tci) {
+    if (base->vlan_tci == flow->vlan_tci) {
         return;
     }
 
     pop_vlan(base, odp_actions, wc);
-    if (vlan_tci & htons(VLAN_CFI)) {
+    if (flow->vlan_tci & htons(VLAN_CFI)) {
         struct ovs_action_push_vlan vlan;
 
-        vlan.vlan_tpid = htons(ETH_TYPE_VLAN);
-        vlan.vlan_tci = vlan_tci;
+        vlan.vlan_tpid = flow_get_vlan_tpid(flow);
+        vlan.vlan_tci = flow->vlan_tci;
         nl_msg_put_unspec(odp_actions, OVS_ACTION_ATTR_PUSH_VLAN,
                           &vlan, sizeof vlan);
     }
-    base->vlan_tci = vlan_tci;
+    base->vlan_tpid = flow_get_vlan_tpid(flow);
+    base->vlan_tci = flow->vlan_tci;
 }
 
 /* Wildcarding already done at action translation time. */
@@ -4034,7 +4044,7 @@ commit_odp_actions(const struct flow *flow, struct flow *base,
     slow = commit_set_nw_action(flow, base, odp_actions, wc, use_masked);
     commit_set_port_action(flow, base, odp_actions, wc, use_masked);
     commit_mpls_action(flow, base, odp_actions);
-    commit_vlan_action(flow->vlan_tci, base, odp_actions, wc);
+    commit_vlan_action(flow, base, odp_actions, wc);
     commit_set_priority_action(flow, base, odp_actions, wc, use_masked);
     commit_set_pkt_mark_action(flow, base, odp_actions, wc, use_masked);
 
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 11b54dd..ef4e82c 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -133,7 +133,7 @@ void odp_portno_names_destroy(struct hmap *portno_names);
  * add another field and forget to adjust this value.
  */
 #define ODPUTIL_FLOW_KEY_BYTES 512
-BUILD_ASSERT_DECL(FLOW_WC_SEQ == 27);
+BUILD_ASSERT_DECL(FLOW_WC_SEQ == 28);
 
 /* A buffer with sufficient size and alignment to hold an nlattr-formatted flow
  * key.  An array of "struct nlattr" might not, in theory, be sufficiently
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index f6818ca..05cfee0 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -1186,24 +1186,22 @@ format_STRIP_VLAN(const struct ofpact_null *a, struct ds *s)
 static enum ofperr
 decode_OFPAT_RAW11_PUSH_VLAN(ovs_be16 eth_type, struct ofpbuf *out)
 {
-    if (eth_type != htons(ETH_TYPE_VLAN_8021Q)) {
-        /* XXX 802.1AD(QinQ) isn't supported at the moment */
+    if (!eth_type_vlan(eth_type)) {
         return OFPERR_OFPBAC_BAD_ARGUMENT;
     }
-    ofpact_put_PUSH_VLAN(out);
+    ofpact_put_PUSH_VLAN(out)->ethertype = eth_type;
     return 0;
 }
 
 static void
-encode_PUSH_VLAN(const struct ofpact_null *null OVS_UNUSED,
+encode_PUSH_VLAN(const struct ofpact_push *push,
                  enum ofp_version ofp_version, struct ofpbuf *out)
 {
     if (ofp_version == OFP10_VERSION) {
         /* PUSH is a side effect of a SET_VLAN_VID/PCP, which should
          * follow this action. */
     } else {
-        /* XXX ETH_TYPE_VLAN_8021AD case */
-        put_OFPAT11_PUSH_VLAN(out, htons(ETH_TYPE_VLAN_8021Q));
+        put_OFPAT11_PUSH_VLAN(out, push->ethertype);
     }
 }
 
@@ -1220,20 +1218,18 @@ parse_PUSH_VLAN(char *arg, struct ofpbuf *ofpacts,
         return error;
     }
 
-    if (ethertype != ETH_TYPE_VLAN_8021Q) {
-        /* XXX ETH_TYPE_VLAN_8021AD case isn't supported */
+    if (!eth_type_vlan(htons(ethertype))) {
         return xasprintf("%s: not a valid VLAN ethertype", arg);
     }
 
-    ofpact_put_PUSH_VLAN(ofpacts);
+    ofpact_put_PUSH_VLAN(ofpacts)->ethertype = htons(ethertype);
     return NULL;
 }
 
 static void
-format_PUSH_VLAN(const struct ofpact_null *a OVS_UNUSED, struct ds *s)
+format_PUSH_VLAN(const struct ofpact_push *a, struct ds *s)
 {
-    /* XXX 802.1AD case*/
-    ds_put_format(s, "push_vlan:%#"PRIx16, ETH_TYPE_VLAN_8021Q);
+    ds_put_format(s, "push_vlan:%#"PRIx16, ntohs(a->ethertype));
 }
 
 /* Action structure for OFPAT10_SET_DL_SRC/DST and OFPAT11_SET_DL_SRC/DST. */
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 5436f24..83257a0 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -66,7 +66,7 @@
     OFPACT(SET_VLAN_VID,    ofpact_vlan_vid,    ofpact, "set_vlan_vid") \
     OFPACT(SET_VLAN_PCP,    ofpact_vlan_pcp,    ofpact, "set_vlan_pcp") \
     OFPACT(STRIP_VLAN,      ofpact_null,        ofpact, "strip_vlan")   \
-    OFPACT(PUSH_VLAN,       ofpact_null,        ofpact, "push_vlan")    \
+    OFPACT(PUSH_VLAN,       ofpact_push,        ofpact, "push_vlan")    \
     OFPACT(SET_ETH_SRC,     ofpact_mac,         ofpact, "mod_dl_src")   \
     OFPACT(SET_ETH_DST,     ofpact_mac,         ofpact, "mod_dl_dst")   \
     OFPACT(SET_IPV4_SRC,    ofpact_ipv4,        ofpact, "mod_nw_src")   \
@@ -317,6 +317,14 @@ struct ofpact_vlan_pcp {
     bool flow_has_vlan;         /* VLAN present at action validation time? */
 };
 
+/* OFPACT_PUSH_VLAN/MPLS/PBB (But MPLS has it's own version right now...)
+ *
+ * Used for OFPAT11_PUSH_VLAN */
+struct ofpact_push {
+    struct ofpact ofpact;
+    ovs_be16 ethertype;
+};
+
 /* OFPACT_SET_ETH_SRC, OFPACT_SET_ETH_DST.
  *
  * Used for OFPAT10_SET_DL_SRC, OFPAT10_SET_DL_DST. */
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index d765d03..8366cfe 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -185,7 +185,7 @@ ofputil_netmask_to_wcbits(ovs_be32 netmask)
 void
 ofputil_wildcard_from_ofpfw10(uint32_t ofpfw, struct flow_wildcards *wc)
 {
-    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 27);
+    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 28);
 
     /* Initialize most of wc. */
     flow_wildcards_init_catchall(wc);
diff --git a/lib/packets.c b/lib/packets.c
index 65d8109..1e10915 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -200,7 +200,7 @@ eth_pop_vlan(struct ofpbuf *packet)
     struct vlan_eth_header *veh = ofpbuf_l2(packet);
 
     if (veh && ofpbuf_size(packet) >= sizeof *veh
-        && veh->veth_type == htons(ETH_TYPE_VLAN)) {
+        && eth_type_vlan(veh->veth_type)) {
 
         memmove((char *)veh + VLAN_HEADER_LEN, veh, 2 * ETH_ADDR_LEN);
         ofpbuf_resize_l2(packet, -VLAN_HEADER_LEN);
@@ -217,7 +217,7 @@ set_ethertype(struct ofpbuf *packet, ovs_be16 eth_type)
         return;
     }
 
-    if (eh->eth_type == htons(ETH_TYPE_VLAN)) {
+    if (eth_type_vlan(eh->eth_type)) {
         ovs_be16 *p;
         char *l2_5 = ofpbuf_l2_5(packet);
 
diff --git a/lib/packets.h b/lib/packets.h
index 26c6ff1..d0d3250 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -251,6 +251,12 @@ static inline bool eth_type_mpls(ovs_be16 eth_type)
         eth_type == htons(ETH_TYPE_MPLS_MCAST);
 }
 
+static inline bool eth_type_vlan(ovs_be16 eth_type)
+{
+    return eth_type == htons(ETH_TYPE_VLAN_8021Q) ||
+        eth_type == htons(ETH_TYPE_VLAN_8021AD);
+}
+
 /* Minimum value for an Ethernet type.  Values below this are IEEE 802.2 frame
  * lengths. */
 #define ETH_TYPE_MIN           0x600
@@ -350,7 +356,7 @@ OVS_PACKED(
 struct vlan_eth_header {
     uint8_t veth_dst[ETH_ADDR_LEN];
     uint8_t veth_src[ETH_ADDR_LEN];
-    ovs_be16 veth_type;         /* Always htons(ETH_TYPE_VLAN). */
+    ovs_be16 veth_type;
     ovs_be16 veth_tci;          /* Lowest 12 bits are VLAN ID. */
     ovs_be16 veth_next_type;
 });
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 851b946..3353f51 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2096,7 +2096,7 @@ xlate_normal(struct xlate_ctx *ctx)
     }
 
     /* Drop malformed frames. */
-    if (flow->dl_type == htons(ETH_TYPE_VLAN) &&
+    if (eth_type_vlan(flow->dl_type) &&
         !(flow->vlan_tci & htons(VLAN_CFI))) {
         if (ctx->xin->packet != NULL) {
             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
@@ -2484,7 +2484,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
 
     /* If 'struct flow' gets additional metadata, we'll need to zero it out
      * before traversing a patch port. */
-    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 27);
+    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 28);
 
     if (!xport) {
         xlate_report(ctx, "Nonexistent output port");
@@ -3705,7 +3705,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_PUSH_VLAN:
-            /* XXX 802.1AD(QinQ) */
+            flow->vlan_tpid = ofpact_get_PUSH_VLAN(a)->ethertype;
             memset(&wc->masks.vlan_tci, 0xff, sizeof wc->masks.vlan_tci);
             flow->vlan_tci = htons(VLAN_CFI);
             break;
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index dd966cd..1f3a185 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -2753,6 +2753,46 @@ done
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - VLAN with AD Ethertype])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], [1], [2], [3], [4])
+
+AT_DATA([flows1.txt], [dnl
+table=0 in_port=1 dl_type=0x8100 actions=pop_vlan,output:2
+table=0 in_port=1 dl_type=0x88a8 actions=pop_vlan,output:3
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows1.txt])
+AT_DATA([flows2.txt], [dnl
+table=0 in_port=2 dl_type=0x0800 actions=push_vlan:0x8100,mod_vlan_vid:9,output:1
+table=0 in_port=3 dl_type=0x0800 actions=push_vlan:0x88a8,set_field:0x1006->vlan_tci,output:1
+table=0 in_port=4 dl_type=0x0800 actions=push_vlan:0x88a8,mod_vlan_vid:11,output:1
+])
+AT_CHECK([ovs-ofctl -O OpenFlow11 add-flows br0 flows2.txt])
+
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x8100,dl_vlan=9'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: pop_vlan,2
+])
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x88a8,dl_vlan=9'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: pop_vlan,3
+])
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=2,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x0800'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: push_vlan(vid=9,pcp=0),1
+])
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=3,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x0800'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: push_vlan(tpid=0x88a8,vid=6,pcp=0),1
+])
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=4,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x0800'], [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: push_vlan(tpid=0x88a8,vid=11,pcp=0),1
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([ofproto-dpif - MPLS handling])
 OVS_VSWITCHD_START([dnl
    add-port br0 p1 -- set Interface p1 type=dummy
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 3f55a72..b257fa4 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -1243,9 +1243,13 @@ Strips the VLAN tag from a packet if it is present.
 .
 .IP \fBpush_vlan\fR:\fIethertype\fR
 Push a new VLAN tag onto the packet.  Ethertype is used as the the Ethertype
-for the tag. Only ethertype 0x8100 should be used. (0x88a8 which the spec
-allows isn't supported at the moment.)
+for the tag. An ethertype of 0x8100 or 0x88a8 can be used.
 A priority of zero and the tag of zero are used for the new tag.
+.IP
+When using the push_vlan action, the OpenFlow option must specify version 1.1
+or greater.  This is true not only to add flows but to dump them as well.
+The other means for dumping flows (ovs-appctl and ovs-dpctl) do not require
+any options.
 .
 .IP \fBpush_mpls\fR:\fIethertype\fR
 Changes the packet's Ethertype to \fIethertype\fR, which must be either
-- 
1.8.3.2




More information about the dev mailing list