[ovs-dev] [PATCH V11 1/4] Flow and action changes for 802.1AD

Thomas F Herbert thomasfherbert at gmail.com
Tue Jun 23 18:38:37 UTC 2015


From: "Thomas F. Herbert" <thomasfherbert at gmail.com>

The flow structure is updated to hold the customer tci.
Flow key extraction is changed to add support for ctci and both TPIDs.
Parsing to support pushing and popping with both single and double
tagged vlans. In response to reviewers comments on V6, all changes
affected by the flow structure are gathered in this patch.

Signed-off-by: Thomas F Herbert <thomasfherbert at gmail.com>
---
 lib/flow.c                   | 14 +++++++-------
 lib/flow.h                   | 16 +++++++++++-----
 lib/match.c                  |  2 +-
 lib/nx-match.c               |  2 +-
 lib/ofp-actions.c            | 32 +++++++++++++++++++-------------
 lib/ofp-actions.h            | 14 +++++++++++---
 lib/ofp-util.c               |  2 +-
 ofproto/ofproto-dpif-rid.h   |  2 +-
 ofproto/ofproto-dpif-xlate.c | 13 ++++++++++++-
 9 files changed, 64 insertions(+), 33 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index 3e99d5e..3bfcb26 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -123,7 +123,7 @@ struct mf_ctx {
  * away.  Some GCC versions gave warnings on ALWAYS_INLINE, so these are
  * defined as macros. */
 
-#if (FLOW_WC_SEQ != 31)
+#if (FLOW_WC_SEQ != 32)
 #define MINIFLOW_ASSERT(X) ovs_assert(X)
 BUILD_MESSAGE("FLOW_WC_SEQ changed: miniflow_extract() will have runtime "
                "assertions enabled. Consider updating FLOW_WC_SEQ after "
@@ -291,7 +291,7 @@ parse_vlan(const void **datap, size_t *sizep)
 
     data_pull(datap, sizep, ETH_ADDR_LEN * 2);
 
-    if (eth->eth_type == htons(ETH_TYPE_VLAN)) {
+    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);
@@ -766,7 +766,7 @@ flow_get_metadata(const struct flow *flow, struct match *flow_metadata)
 {
     int i;
 
-    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 31);
+    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 32);
 
     match_init_catchall(flow_metadata);
     if (flow->tunnel.tun_id != htonll(0)) {
@@ -942,7 +942,7 @@ void flow_wildcards_init_for_packet(struct flow_wildcards *wc,
     memset(&wc->masks, 0x0, sizeof wc->masks);
 
     /* Update this function whenever struct flow changes. */
-    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 31);
+    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 32);
 
     if (flow->tunnel.ip_dst) {
         if (flow->tunnel.flags & FLOW_TNL_F_KEY) {
@@ -1041,7 +1041,7 @@ uint64_t
 flow_wc_map(const struct flow *flow)
 {
     /* Update this function whenever struct flow changes. */
-    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 31);
+    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 32);
 
     uint64_t map = (flow->tunnel.ip_dst) ? MINIFLOW_MAP(tunnel) : 0;
 
@@ -1093,7 +1093,7 @@ void
 flow_wildcards_clear_non_packet_fields(struct flow_wildcards *wc)
 {
     /* Update this function whenever struct flow changes. */
-    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 31);
+    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 32);
 
     memset(&wc->masks.metadata, 0, sizeof wc->masks.metadata);
     memset(&wc->masks.regs, 0, sizeof wc->masks.regs);
@@ -1648,7 +1648,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 and dp_hash. */
-        BUILD_ASSERT(FLOW_WC_SEQ == 31);
+        BUILD_ASSERT(FLOW_WC_SEQ == 32);
         memset((char *) flow + FLOW_SEGMENT_2_ENDS_AT, 0,
                sizeof(struct flow) - FLOW_SEGMENT_2_ENDS_AT);
         flow->dp_hash = 0;
diff --git a/lib/flow.h b/lib/flow.h
index 70554e4..9d34775 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -39,7 +39,7 @@ struct match;
 /* 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 31
+#define FLOW_WC_SEQ 32
 
 /* Number of Open vSwitch extension 32-bit registers. */
 #define FLOW_N_REGS 8
@@ -114,7 +114,13 @@ struct flow {
     uint8_t dl_dst[ETH_ADDR_LEN]; /* Ethernet destination address. */
     uint8_t dl_src[ETH_ADDR_LEN]; /* 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;          /* If 802.1Q, TCI | VLAN_CFI; If 802.1ad,
+                                 * outer tag, otherwise 0. */
+    ovs_be16 vlan_ctci;         /* If 802.1ad, Customer TCI | VLAN_CFI,
+                                 * inner tag, otherwise 0. */
+    ovs_be16 vlan_tpid;         /* Vlan protocol type,
+                                 * either 802.1ad or 802.1q. */
+    uint32_t pad2;              /* Pad to 64 bits. */
     ovs_be32 mpls_lse[ROUND_UP(FLOW_MAX_MPLS_LABELS, 2)]; /* MPLS label stack
                                                              (with padding). */
     /* L3 (64-bit aligned) */
@@ -131,7 +137,7 @@ struct flow {
     uint8_t arp_sha[ETH_ADDR_LEN]; /* ARP/ND source hardware address. */
     uint8_t arp_tha[ETH_ADDR_LEN]; /* ARP/ND target hardware address. */
     ovs_be16 tcp_flags;         /* TCP flags. With L3 to avoid matching L4. */
-    ovs_be16 pad2;              /* Pad to 64 bits. */
+    ovs_be16 pad3;              /* Pad to 64 bits. */
 
     /* L4 (64-bit aligned) */
     ovs_be16 tp_src;            /* TCP/UDP/SCTP source port. */
@@ -156,8 +162,8 @@ BUILD_ASSERT_DECL(sizeof(struct flow) % sizeof(uint64_t) == 0);
 
 /* Remember to update FLOW_WC_SEQ when changing 'struct flow'. */
 BUILD_ASSERT_DECL(offsetof(struct flow, igmp_group_ip4) + sizeof(uint32_t)
-                  == sizeof(struct flow_tnl) + 192
-                  && FLOW_WC_SEQ == 31);
+                  == sizeof(struct flow_tnl) + 200
+                  && FLOW_WC_SEQ == 32);
 
 /* Incremental points at which flow classification may be performed in
  * segments.
diff --git a/lib/match.c b/lib/match.c
index 7d0b409..cab70e8 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -912,7 +912,7 @@ match_format(const struct match *match, struct ds *s, int priority)
 
     int i;
 
-    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 31);
+    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 32);
 
     if (priority != OFP_DEFAULT_PRIORITY) {
         ds_put_format(s, "priority=%d,", priority);
diff --git a/lib/nx-match.c b/lib/nx-match.c
index 21f291c..aed283c 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -865,7 +865,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 == 31);
+    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 32);
 
     /* Metadata. */
     if (match->wc.masks.dp_hash) {
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 10e2a92..9ed8b49 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -1269,16 +1269,20 @@ 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 */
+    struct ofpact_push_vlan *oam;
+
+    if (!eth_type_vlan(eth_type)) {
+        /* XXX Only 802.1q or 802.1AD(QinQ) are supported.  */
         return OFPERR_OFPBAC_BAD_ARGUMENT;
     }
-    ofpact_put_PUSH_VLAN(out);
+    oam = ofpact_put_PUSH_VLAN(out);
+    oam->ethertype = eth_type;
+
     return 0;
 }
 
 static void
-encode_PUSH_VLAN(const struct ofpact_null *null OVS_UNUSED,
+encode_PUSH_VLAN(const struct ofpact_push_vlan *push_vlan,
                  enum ofp_version ofp_version, struct ofpbuf *out)
 {
     if (ofp_version == OFP10_VERSION) {
@@ -1286,7 +1290,7 @@ encode_PUSH_VLAN(const struct ofpact_null *null OVS_UNUSED,
          * 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_vlan->ethertype);
     }
 }
 
@@ -1298,25 +1302,25 @@ parse_PUSH_VLAN(char *arg, struct ofpbuf *ofpacts,
     char *error;
 
     *usable_protocols &= OFPUTIL_P_OF11_UP;
-    error = str_to_u16(arg, "ethertype", &ethertype);
+    error = str_to_u16(arg, "push_vlan", &ethertype);
     if (error) {
         return error;
     }
 
-    if (ethertype != ETH_TYPE_VLAN_8021Q) {
-        /* XXX ETH_TYPE_VLAN_8021AD case isn't supported */
+    if (!eth_type_vlan(htons(ethertype))) {
+        /* Check for valid VLAN ethertypes */
         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_vlan *a OVS_UNUSED, 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. */
@@ -5491,8 +5495,9 @@ ofpact_check__(enum ofputil_protocol *usable_protocols, struct ofpact *a,
         return 0;
 
     case OFPACT_PUSH_VLAN:
-        if (flow->vlan_tci & htons(VLAN_CFI)) {
-            /* Multiple VLAN headers not supported. */
+        if (flow->vlan_tci & htons(VLAN_CFI) &&
+            flow->vlan_ctci & htons(VLAN_CFI)) {
+            /* More then 2 levels of VLAN headers not supported. */
             return OFPERR_OFPBAC_BAD_TAG;
         }
         /* Temporary mark that we have a vlan tag. */
@@ -6131,6 +6136,7 @@ ofpacts_get_meter(const struct ofpact ofpacts[], size_t ofpacts_len)
 
 /* Formatting ofpacts. */
 
+
 static void
 ofpact_format(const struct ofpact *a, struct ds *s)
 {
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 785c814..0314da3 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_vlan,   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")   \
@@ -395,7 +395,15 @@ struct ofpact_set_field {
     union mf_value mask;
 };
 
-/* OFPACT_PUSH_VLAN/MPLS/PBB
+/* OFPACT_PUSH_VLAN
+ *
+ * Used for OFPAT11_PUSH_VLAN. */
+struct ofpact_push_vlan {
+    struct ofpact ofpact;
+    ovs_be16 ethertype;
+};
+
+/* OFPACT_PUSH_MPLS
  *
  * Used for NXAST_PUSH_MPLS, OFPAT11_PUSH_MPLS. */
 struct ofpact_push_mpls {
@@ -405,7 +413,7 @@ struct ofpact_push_mpls {
 
 /* OFPACT_POP_MPLS
  *
- * Used for NXAST_POP_MPLS, OFPAT11_POP_MPLS.. */
+ * Used for NXAST_POP_MPLS, OFPAT11_POP_MPLS. */
 struct ofpact_pop_mpls {
     struct ofpact ofpact;
     ovs_be16 ethertype;
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 89359c1..dc063d7 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -195,7 +195,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 == 31);
+    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 32);
 
     /* Initialize most of wc. */
     flow_wildcards_init_catchall(wc);
diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h
index 81a61a2..dc533ce 100644
--- a/ofproto/ofproto-dpif-rid.h
+++ b/ofproto/ofproto-dpif-rid.h
@@ -91,7 +91,7 @@ struct rule;
 /* Metadata for restoring pipeline context after recirculation.  Helpers
  * are inlined below to keep them together with the definition for easier
  * updates. */
-BUILD_ASSERT_DECL(FLOW_WC_SEQ == 31);
+BUILD_ASSERT_DECL(FLOW_WC_SEQ == 32);
 
 struct recirc_metadata {
     /* Metadata in struct flow. */
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 8c68273..7395f14 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2761,6 +2761,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
     struct flow *flow = &ctx->xin->flow;
     struct flow_tnl flow_tnl;
     ovs_be16 flow_vlan_tci;
+    ovs_be16 flow_vlan_ctci;
     uint32_t flow_pkt_mark;
     uint8_t flow_nw_tos;
     odp_port_t out_port, odp_port;
@@ -2769,7 +2770,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 == 31);
+    BUILD_ASSERT_DECL(FLOW_WC_SEQ == 32);
     memset(&flow_tnl, 0, sizeof flow_tnl);
 
     if (!xport) {
@@ -2912,6 +2913,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
     }
 
     flow_vlan_tci = flow->vlan_tci;
+    flow_vlan_ctci = flow->vlan_ctci;
     flow_pkt_mark = flow->pkt_mark;
     flow_nw_tos = flow->nw_tos;
 
@@ -3033,6 +3035,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
  out:
     /* Restore flow */
     flow->vlan_tci = flow_vlan_tci;
+    flow->vlan_ctci = flow_vlan_ctci;
     flow->pkt_mark = flow_pkt_mark;
     flow->nw_tos = flow_nw_tos;
 }
@@ -4174,6 +4177,14 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             /* XXX 802.1AD(QinQ) */
             memset(&wc->masks.vlan_tci, 0xff, sizeof wc->masks.vlan_tci);
             flow->vlan_tci = htons(VLAN_CFI);
+            if (ofpact_get_PUSH_VLAN(a)->ethertype == htons(ETH_TYPE_VLAN_8021AD)) {
+                memset(&wc->masks.vlan_ctci, 0xff, sizeof wc->masks.vlan_ctci);
+                flow->vlan_ctci |= htons(VLAN_CFI);
+                memset(&wc->masks.vlan_tpid, 0xff, sizeof wc->masks.vlan_tpid);
+                flow->vlan_tpid = htons(ETH_TYPE_VLAN_8021AD);
+            } else if (ofpact_get_PUSH_VLAN(a)->ethertype == htons(ETH_TYPE_VLAN_8021Q)) {
+                flow->vlan_tpid = htons(ETH_TYPE_VLAN_8021Q);
+            }
             break;
 
         case OFPACT_SET_ETH_SRC:
-- 
2.1.0




More information about the dev mailing list