> On Jun 23, 2015, at 11:38 AM, Thomas F Herbert <thomasfherb...@gmail.com>
> wrote:
>
> From: "Thomas F. Herbert" <thomasfherb...@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 <thomasfherb...@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);
Maybe I am missing something, but I am surprised that the new struct flow
fields are not populated in miniflow_extract() following the call to
parse_vlan(). Or is the customer TCI not present in the packet?
Jarno
> @@ -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", ðertype);
> + error = str_to_u16(arg, "push_vlan", ðertype);
> 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
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
