From: Simon Horman Date: Fri, 15 Mar 2013 14:27:11 +0000 (+0100) Subject: Remove encal_dl_type from struct flow X-Git-Tag: sliver-openvswitch-1.10.90-1~10^2~56 X-Git-Url: http://git.onelab.eu/?p=sliver-openvswitch.git;a=commitdiff_plain;h=cff78c8844bcce7c6b20fe30d56b5509232039d2 Remove encal_dl_type from struct flow There were plans to use this in conjunction with inner/outer flows, however that plan has been changed in favour of using recirculation. This leaves us with the current usage. encal_dl_type is currently only used to allow decoding of packets used in the test suite. However, this is a bit of a fudge and the packets may be provided as hexadecimal instead. Also remove comments from parse_l2_5_onward() relating to MPLS which are not in keeping with the commenting throughout the rest of the function. Signed-off-by: Simon Horman Signed-off-by: Jesse Gross --- diff --git a/lib/flow.c b/lib/flow.c index e335ecfcf..678b8f0d5 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -492,7 +492,7 @@ flow_zero_wildcards(struct flow *flow, const struct flow_wildcards *wildcards) void flow_get_metadata(const struct flow *flow, struct flow_metadata *fmd) { - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 19); + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 20); fmd->tun_id = flow->tunnel.tun_id; fmd->metadata = flow->metadata; @@ -856,9 +856,6 @@ flow_set_mpls_bos(struct flow *flow, uint8_t bos) void flow_compose(struct ofpbuf *b, const struct flow *flow) { - ovs_be16 inner_dl_type; - - inner_dl_type = flow_innermost_dl_type(flow); eth_compose(b, flow->dl_dst, flow->dl_src, ntohs(flow->dl_type), 0); if (flow->dl_type == htons(FLOW_DL_TYPE_NONE)) { struct eth_header *eth = b->l2; @@ -870,7 +867,7 @@ flow_compose(struct ofpbuf *b, const struct flow *flow) eth_push_vlan(b, flow->vlan_tci); } - if (inner_dl_type == htons(ETH_TYPE_IP)) { + if (flow->dl_type == htons(ETH_TYPE_IP)) { struct ip_header *ip; b->l3 = ip = ofpbuf_put_zeros(b, sizeof *ip); @@ -916,10 +913,10 @@ flow_compose(struct ofpbuf *b, const struct flow *flow) ip->ip_tot_len = htons((uint8_t *) b->data + b->size - (uint8_t *) b->l3); ip->ip_csum = csum(ip, sizeof *ip); - } else if (inner_dl_type == htons(ETH_TYPE_IPV6)) { + } else if (flow->dl_type == htons(ETH_TYPE_IPV6)) { /* XXX */ - } else if (inner_dl_type == htons(ETH_TYPE_ARP) || - inner_dl_type == htons(ETH_TYPE_RARP)) { + } else if (flow->dl_type == htons(ETH_TYPE_ARP) || + flow->dl_type == htons(ETH_TYPE_RARP)) { struct arp_eth_header *arp; b->l3 = arp = ofpbuf_put_zeros(b, sizeof *arp); diff --git a/lib/flow.h b/lib/flow.h index 577689b65..6e169d68e 100644 --- a/lib/flow.h +++ b/lib/flow.h @@ -36,7 +36,7 @@ struct ofpbuf; /* 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 19 +#define FLOW_WC_SEQ 20 #define FLOW_N_REGS 8 BUILD_ASSERT_DECL(FLOW_N_REGS <= NXM_NX_MAX_REGS); @@ -95,7 +95,6 @@ struct flow { uint16_t mpls_depth; /* Depth of MPLS stack. */ ovs_be16 vlan_tci; /* If 802.1Q, TCI | VLAN_CFI; otherwise 0. */ ovs_be16 dl_type; /* Ethernet frame type. */ - ovs_be16 encap_dl_type; /* MPLS encapsulated Ethernet frame type */ ovs_be16 tp_src; /* TCP/UDP source port. */ ovs_be16 tp_dst; /* TCP/UDP destination port. */ uint8_t dl_src[6]; /* Ethernet source address. */ @@ -106,7 +105,7 @@ struct flow { uint8_t arp_tha[6]; /* ARP/ND target hardware address. */ uint8_t nw_ttl; /* IP TTL/Hop Limit. */ uint8_t nw_frag; /* FLOW_FRAG_* flags. */ - uint8_t zeros[4]; + uint8_t zeros[6]; }; BUILD_ASSERT_DECL(sizeof(struct flow) % 4 == 0); @@ -114,7 +113,7 @@ BUILD_ASSERT_DECL(sizeof(struct flow) % 4 == 0); /* Remember to update FLOW_WC_SEQ when changing 'struct flow'. */ BUILD_ASSERT_DECL(sizeof(struct flow) == sizeof(struct flow_tnl) + 160 && - FLOW_WC_SEQ == 19); + FLOW_WC_SEQ == 20); /* Represents the metadata fields of struct flow. */ struct flow_metadata { @@ -127,17 +126,6 @@ struct flow_metadata { void flow_extract(struct ofpbuf *, uint32_t priority, uint32_t mark, const struct flow_tnl *, uint16_t in_port, struct flow *); -/* Returns the innermost dl_type. - * If there's an outer and an inner type then the inner type is returned. - * Otherwise, if there is only one type then it is returned. */ -static inline ovs_be16 -flow_innermost_dl_type(const struct flow *flow) -{ - return (flow->encap_dl_type == htons(0) - ? flow->dl_type - : flow->encap_dl_type); -} - void flow_zero_wildcards(struct flow *, const struct flow_wildcards *); void flow_get_metadata(const struct flow *, struct flow_metadata *); diff --git a/lib/match.c b/lib/match.c index f4b0a6c14..2aa4e895d 100644 --- a/lib/match.c +++ b/lib/match.c @@ -849,7 +849,7 @@ match_format(const struct match *match, struct ds *s, unsigned int priority) int i; - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 19); + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 20); if (priority != OFP_DEFAULT_PRIORITY) { ds_put_format(s, "priority=%u,", priority); diff --git a/lib/nx-match.c b/lib/nx-match.c index 6efc94df3..bfead6808 100644 --- a/lib/nx-match.c +++ b/lib/nx-match.c @@ -573,7 +573,7 @@ nx_put_raw(struct ofpbuf *b, bool oxm, const struct match *match, int match_len; int i; - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 19); + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 20); /* Metadata. */ if (match->wc.masks.in_port) { diff --git a/lib/odp-util.c b/lib/odp-util.c index 80acad361..54e5b1287 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -1805,32 +1805,16 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], const struct nlattr *key, size_t key_len) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); - ovs_be16 dl_type; - /* Parse MPLS label stack entry */ if (eth_type_mpls(flow->dl_type)) { - /* Calculate fitness of outer attributes. */ expected_attrs |= (UINT64_C(1) << OVS_KEY_ATTR_MPLS); - /* Get the MPLS LSE value. */ if (!(present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_MPLS))) { return ODP_FIT_TOO_LITTLE; } flow->mpls_lse = nl_attr_get_be32(attrs[OVS_KEY_ATTR_MPLS]); flow->mpls_depth++; - - if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_IPV4)) { - flow->encap_dl_type = htons(ETH_TYPE_IP); - } else if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_IPV6)) { - flow->encap_dl_type = htons(ETH_TYPE_IPV6); - } else if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ARP)) { - flow->encap_dl_type = htons(ETH_TYPE_ARP); - } - } - - dl_type = flow_innermost_dl_type(flow); - - if (dl_type == htons(ETH_TYPE_IP)) { + } else if (flow->dl_type == htons(ETH_TYPE_IP)) { expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_IPV4; if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_IPV4)) { const struct ovs_key_ipv4 *ipv4_key; @@ -1845,7 +1829,7 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], return ODP_FIT_ERROR; } } - } else if (dl_type == htons(ETH_TYPE_IPV6)) { + } else if (flow->dl_type == htons(ETH_TYPE_IPV6)) { expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_IPV6; if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_IPV6)) { const struct ovs_key_ipv6 *ipv6_key; @@ -1861,8 +1845,8 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], return ODP_FIT_ERROR; } } - } else if (dl_type == htons(ETH_TYPE_ARP) || - dl_type == htons(ETH_TYPE_RARP)) { + } else if (flow->dl_type == htons(ETH_TYPE_ARP) || + flow->dl_type == htons(ETH_TYPE_RARP)) { expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ARP; if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ARP)) { const struct ovs_key_arp *arp_key; @@ -1882,8 +1866,8 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], } if (flow->nw_proto == IPPROTO_TCP - && (dl_type == htons(ETH_TYPE_IP) || - dl_type == htons(ETH_TYPE_IPV6)) + && (flow->dl_type == htons(ETH_TYPE_IP) || + flow->dl_type == htons(ETH_TYPE_IPV6)) && !(flow->nw_frag & FLOW_NW_FRAG_LATER)) { expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_TCP; if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_TCP)) { @@ -1894,8 +1878,8 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], flow->tp_dst = tcp_key->tcp_dst; } } else if (flow->nw_proto == IPPROTO_UDP - && (dl_type == htons(ETH_TYPE_IP) || - dl_type == htons(ETH_TYPE_IPV6)) + && (flow->dl_type == htons(ETH_TYPE_IP) || + flow->dl_type == htons(ETH_TYPE_IPV6)) && !(flow->nw_frag & FLOW_NW_FRAG_LATER)) { expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_UDP; if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_UDP)) { @@ -1906,7 +1890,7 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], flow->tp_dst = udp_key->udp_dst; } } else if (flow->nw_proto == IPPROTO_ICMP - && dl_type == htons(ETH_TYPE_IP) + && flow->dl_type == htons(ETH_TYPE_IP) && !(flow->nw_frag & FLOW_NW_FRAG_LATER)) { expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ICMP; if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ICMP)) { @@ -1917,7 +1901,7 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], flow->tp_dst = htons(icmp_key->icmp_code); } } else if (flow->nw_proto == IPPROTO_ICMPV6 - && dl_type == htons(ETH_TYPE_IPV6) + && flow->dl_type == htons(ETH_TYPE_IPV6) && !(flow->nw_frag & FLOW_NW_FRAG_LATER)) { expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ICMPV6; if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ICMPV6)) { @@ -2334,16 +2318,14 @@ static void commit_set_nw_action(const struct flow *flow, struct flow *base, struct ofpbuf *odp_actions) { - ovs_be16 dl_type = flow_innermost_dl_type(flow); - /* Check if flow really have an IP header. */ if (!flow->nw_proto) { return; } - if (dl_type == htons(ETH_TYPE_IP)) { + if (flow->dl_type == htons(ETH_TYPE_IP)) { commit_set_ipv4_action(flow, base, odp_actions); - } else if (dl_type == htons(ETH_TYPE_IPV6)) { + } else if (flow->dl_type == htons(ETH_TYPE_IPV6)) { commit_set_ipv6_action(flow, base, odp_actions); } } diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 06e149ab3..6b78f84c2 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -84,7 +84,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 == 19); + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 20); /* Initialize most of wc. */ flow_wildcards_init_catchall(wc); @@ -1058,7 +1058,7 @@ ofputil_usable_protocols(const struct match *match) { const struct flow_wildcards *wc = &match->wc; - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 19); + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 20); /* tunnel params other than tun_id can't be sent in a flow_mod */ if (!tun_parms_fully_wildcarded(wc)) { diff --git a/lib/packets.c b/lib/packets.c index c33ae08c5..77aa7d38c 100644 --- a/lib/packets.c +++ b/lib/packets.c @@ -887,8 +887,7 @@ packet_set_udp_port(struct ofpbuf *packet, ovs_be16 src, ovs_be16 dst) uint8_t packet_get_tcp_flags(const struct ofpbuf *packet, const struct flow *flow) { - ovs_be16 dl_type = flow_innermost_dl_type(flow); - if (dl_type_is_ip_any(dl_type) && + if (dl_type_is_ip_any(flow->dl_type) && flow->nw_proto == IPPROTO_TCP && packet->l7) { const struct tcp_header *tcp = packet->l4; return TCP_FLAGS(tcp->tcp_ctl); diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 421e9d4b2..0b72013da 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -5736,7 +5736,7 @@ compose_output_action__(struct action_xlate_ctx *ctx, uint16_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 == 19); + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 20); if (!ofport) { xlate_report(ctx, "Nonexistent output port"); @@ -6065,7 +6065,6 @@ execute_mpls_push_action(struct action_xlate_ctx *ctx, ovs_be16 eth_type) tc = (ctx->flow.nw_tos & IP_DSCP_MASK) >> 2; ttl = ctx->flow.nw_ttl ? ctx->flow.nw_ttl : 0x40; ctx->flow.mpls_lse = set_mpls_lse_values(ttl, tc, 1, label); - ctx->flow.encap_dl_type = ctx->flow.dl_type; ctx->flow.mpls_depth = 1; } ctx->flow.dl_type = eth_type; @@ -6082,7 +6081,6 @@ execute_mpls_pop_action(struct action_xlate_ctx *ctx, ovs_be16 eth_type) ctx->flow.mpls_lse = htonl(0); if (!ctx->flow.mpls_depth) { ctx->flow.dl_type = eth_type; - ctx->flow.encap_dl_type = htons(0); } } } diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 0a2a900a3..354fdc91f 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -509,22 +509,29 @@ mplsm(label:1000,tc:7,ttl:64,bos:1),metadata=0,in_port=0,vlan_tci=0x0000,dl_src= dnl Modified MPLS pop action. +dnl The input is a frame with two MPLS headers which tcpdump -vve shows as: +dnl 60:66:66:66:66:66 > 50:54:00:00:00:07, ethertype MPLS multicast (0x8847), length 66: MPLS (label 20, exp 0, ttl 32) +dnl (tos 0x0, ttl 64, id 0, offset 0, flags [none], proto TCP (6), length 44) + AT_CHECK([ovs-ofctl monitor br0 65534 -P nxm --detach --pidfile 2> ofctl_monitor.log]) for i in 1 2 3; do - ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=60:66:66:66:66:66,dst=50:54:00:00:00:07),eth_type(0x8847),mpls(label=10,tc=3,ttl=100,bos=1),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no)' + ovs-appctl netdev-dummy/receive p1 '50 54 00 00 00 07 60 66 66 66 66 66 88 47 00 01 41 20 45 00 00 2c 00 00 00 00 40 06 3b 78 c0 a8 00 01 c0 a8 00 02 00 50 00 00 00 00 00 2a 00 00 00 2a 50 00 27 10 77 44 00 00 48 4f 47 45' done +#for i in 2 3; do +# ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=60:66:66:66:66:66,dst=50:54:00:00:00:07),eth_type(0x8847),mpls(label=10,tc=3,ttl=100,bos=1)' +#done OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit]) AT_CHECK([cat ofctl_monitor.log], [0], [dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=56 in_port=1 (via action) data_len=56 (unbuffered) -tcp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:66:66,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64 tcp_csum:0 +NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=58 in_port=1 (via action) data_len=58 (unbuffered) +tcp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:66:66,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=80,tp_dst=0 tcp_csum:7744 dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=56 in_port=1 (via action) data_len=56 (unbuffered) -tcp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:66:66,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64 tcp_csum:0 +NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=58 in_port=1 (via action) data_len=58 (unbuffered) +tcp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:66:66,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=80,tp_dst=0 tcp_csum:7744 dnl -NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=56 in_port=1 (via action) data_len=56 (unbuffered) -tcp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:66:66,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64 tcp_csum:0 +NXT_PACKET_IN (xid=0x0): cookie=0xd total_len=58 in_port=1 (via action) data_len=58 (unbuffered) +tcp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=60:66:66:66:66:66,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=80,tp_dst=0 tcp_csum:7744 ]) dnl Checksum TCP. @@ -620,7 +627,7 @@ AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl cookie=0xa, n_packets=3, n_bytes=180, dl_src=40:44:44:44:44:48 actions=push_mpls:0x8847,load:0xa->OXM_OF_MPLS_LABEL[[]],load:0x3->OXM_OF_MPLS_TC[[]],set_mpls_ttl(10),dec_mpls_ttl,CONTROLLER:65535 cookie=0xb, n_packets=3, n_bytes=180, dl_src=50:55:55:55:55:55,dl_type=0x8847 actions=load:0x3e8->OXM_OF_MPLS_LABEL[[]],CONTROLLER:65535 cookie=0xc, n_packets=3, n_bytes=180, dl_src=70:77:77:77:77:77 actions=push_mpls:0x8848,load:0x3e8->OXM_OF_MPLS_LABEL[[]],load:0x7->OXM_OF_MPLS_TC[[]],CONTROLLER:65535 - cookie=0xd, n_packets=3, n_bytes=180, dl_src=60:66:66:66:66:66 actions=pop_mpls:0x0800,CONTROLLER:65535 + cookie=0xd, n_packets=3, n_bytes=186, dl_src=60:66:66:66:66:66 actions=pop_mpls:0x0800,CONTROLLER:65535 n_packets=3, n_bytes=180, dl_src=10:11:11:11:11:11 actions=CONTROLLER:65535 NXST_FLOW reply: ])