[ovs-dev] [PATCH 2/2] ofproto-dpif: Always un-wildcard 'dl_type'.
Justin Pettit
jpettit at nicira.com
Wed Jun 26 05:44:13 UTC 2013
Thanks for the reviews. I pushed to all affected branches.
--Justin
On Jun 25, 2013, at 10:23 PM, Ben Pfaff <blp at nicira.com> wrote:
> Looks good.
>
> On Jun 25, 2013 6:29 PM, "Justin Pettit" <jpettit at nicira.com> wrote:
> We always look at the fragment status and often look at other L3
> headers when processing the packet, so just un-wildcard the
> Ethertype.
>
> Signed-off-by: Justin Pettit <jpettit at nicira.com>
> ---
> lib/bfd.c | 1 -
> lib/cfm.c | 1 -
> lib/flow.c | 1 -
> ofproto/netflow.c | 1 -
> ofproto/ofproto-dpif-xlate.c | 13 +------------
> ofproto/ofproto-dpif.c | 1 +
> tests/ofproto-dpif.at | 22 +++++++++++-----------
> 7 files changed, 13 insertions(+), 27 deletions(-)
>
> diff --git a/lib/bfd.c b/lib/bfd.c
> index ddf8c4c..42d68ad 100644
> --- a/lib/bfd.c
> +++ b/lib/bfd.c
> @@ -416,7 +416,6 @@ bfd_put_packet(struct bfd *bfd, struct ofpbuf *p,
> bool
> bfd_should_process_flow(const struct flow *flow, struct flow_wildcards *wc)
> {
> - memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
> memset(&wc->masks.tp_dst, 0xff, sizeof wc->masks.tp_dst);
> return (flow->dl_type == htons(ETH_TYPE_IP)
> diff --git a/lib/cfm.c b/lib/cfm.c
> index 67c2002..1c98ed8 100644
> --- a/lib/cfm.c
> +++ b/lib/cfm.c
> @@ -584,7 +584,6 @@ cfm_should_process_flow(const struct cfm *cfm, const struct flow *flow,
> struct flow_wildcards *wc)
> {
> memset(&wc->masks.dl_dst, 0xff, sizeof wc->masks.dl_dst);
> - memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> if (cfm->check_tnl_key) {
> memset(&wc->masks.tunnel.tun_id, 0xff, sizeof wc->masks.tunnel.tun_id);
> }
> diff --git a/lib/flow.c b/lib/flow.c
> index 32ac99a..a42fea1 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -791,7 +791,6 @@ flow_mask_hash_fields(struct flow_wildcards *wc, enum nx_hash_fields fields)
> case NX_HASH_FIELDS_SYMMETRIC_L4:
> memset(&wc->masks.dl_src, 0xff, sizeof wc->masks.dl_src);
> memset(&wc->masks.dl_dst, 0xff, sizeof wc->masks.dl_dst);
> - memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
> memset(&wc->masks.nw_src, 0xff, sizeof wc->masks.nw_src);
> memset(&wc->masks.nw_dst, 0xff, sizeof wc->masks.nw_dst);
> diff --git a/ofproto/netflow.c b/ofproto/netflow.c
> index 2e32f21..c7eb2b5 100644
> --- a/ofproto/netflow.c
> +++ b/ofproto/netflow.c
> @@ -54,7 +54,6 @@ struct netflow {
> void
> netflow_mask_wc(struct flow_wildcards *wc)
> {
> - memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
> memset(&wc->masks.nw_src, 0xff, sizeof wc->masks.nw_src);
> memset(&wc->masks.nw_dst, 0xff, sizeof wc->masks.nw_dst);
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 36f54ee..d77e4df 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -548,7 +548,6 @@ xlate_normal(struct xlate_ctx *ctx)
> ctx->xout->has_normal = true;
>
> /* Check the dl_type, since we may check for gratuituous ARP. */
> - memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> memset(&wc->masks.dl_src, 0xff, sizeof wc->masks.dl_src);
> memset(&wc->masks.dl_dst, 0xff, sizeof wc->masks.dl_dst);
> wc->masks.vlan_tci |= htons(VLAN_VID_MASK | VLAN_CFI);
> @@ -813,7 +812,6 @@ process_special(struct xlate_ctx *ctx, const struct flow *flow,
> return SLOW_BFD;
> } else if (ofport->bundle && ofport->bundle->lacp
> && flow->dl_type == htons(ETH_TYPE_LACP)) {
> - memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> if (packet) {
> lacp_process_packet(ofport->bundle->lacp, ofport, packet);
> }
> @@ -1147,7 +1145,6 @@ compose_mpls_push_action(struct xlate_ctx *ctx, ovs_be16 eth_type)
>
> ovs_assert(eth_type_mpls(eth_type));
>
> - memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> memset(&wc->masks.mpls_lse, 0xff, sizeof wc->masks.mpls_lse);
> memset(&wc->masks.mpls_depth, 0xff, sizeof wc->masks.mpls_depth);
>
> @@ -1182,7 +1179,6 @@ compose_mpls_pop_action(struct xlate_ctx *ctx, ovs_be16 eth_type)
> ovs_assert(eth_type_mpls(ctx->xin->flow.dl_type));
> ovs_assert(!eth_type_mpls(eth_type));
>
> - memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> memset(&wc->masks.mpls_lse, 0xff, sizeof wc->masks.mpls_lse);
> memset(&wc->masks.mpls_depth, 0xff, sizeof wc->masks.mpls_depth);
>
> @@ -1239,7 +1235,6 @@ compose_dec_mpls_ttl_action(struct xlate_ctx *ctx)
> uint8_t ttl = mpls_lse_to_ttl(flow->mpls_lse);
> struct flow_wildcards *wc = &ctx->xout->wc;
>
> - memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> memset(&wc->masks.mpls_lse, 0xff, sizeof wc->masks.mpls_lse);
>
> if (!eth_type_mpls(flow->dl_type)) {
> @@ -1594,14 +1589,12 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
> break;
>
> case OFPACT_SET_IPV4_SRC:
> - memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> if (flow->dl_type == htons(ETH_TYPE_IP)) {
> flow->nw_src = ofpact_get_SET_IPV4_SRC(a)->ipv4;
> }
> break;
>
> case OFPACT_SET_IPV4_DST:
> - memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> if (flow->dl_type == htons(ETH_TYPE_IP)) {
> flow->nw_dst = ofpact_get_SET_IPV4_DST(a)->ipv4;
> }
> @@ -1609,7 +1602,6 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
>
> case OFPACT_SET_IPV4_DSCP:
> /* OpenFlow 1.0 only supports IPv4. */
> - memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> if (flow->dl_type == htons(ETH_TYPE_IP)) {
> flow->nw_tos &= ~IP_DSCP_MASK;
> flow->nw_tos |= ofpact_get_SET_IPV4_DSCP(a)->dscp;
> @@ -1617,7 +1609,6 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
> break;
>
> case OFPACT_SET_L4_SRC_PORT:
> - memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
> if (is_ip_any(flow)) {
> flow->tp_src = htons(ofpact_get_SET_L4_SRC_PORT(a)->port);
> @@ -1625,7 +1616,6 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
> break;
>
> case OFPACT_SET_L4_DST_PORT:
> - memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
> if (is_ip_any(flow)) {
> flow->tp_dst = htons(ofpact_get_SET_L4_DST_PORT(a)->port);
> @@ -1687,7 +1677,6 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
> break;
>
> case OFPACT_DEC_TTL:
> - memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> if (compose_dec_ttl(ctx, ofpact_get_DEC_TTL(a))) {
> goto out;
> }
> @@ -1719,7 +1708,6 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
> break;
>
> case OFPACT_FIN_TIMEOUT:
> - memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
> ctx->xout->has_fin_timeout = true;
> xlate_fin_timeout(ctx, ofpact_get_FIN_TIMEOUT(a));
> @@ -1908,6 +1896,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
> flow_wildcards_init_catchall(wc);
> memset(&wc->masks.in_port, 0xff, sizeof wc->masks.in_port);
> memset(&wc->masks.skb_priority, 0xff, sizeof wc->masks.skb_priority);
> + memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> wc->masks.nw_frag |= FLOW_NW_FRAG_MASK;
>
> if (tnl_port_should_receive(&ctx.xin->flow)) {
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 2fff66e..c6a7abc 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -5174,6 +5174,7 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto,
> }
>
> if (wc) {
> + memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
> wc->masks.nw_frag |= FLOW_NW_FRAG_MASK;
> }
>
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 21ca5e7..88492cf 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -2227,7 +2227,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
> AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.4,dst=10.0.0.3,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
> AT_CHECK([ovs-appctl dpif/dump-megaflows br0 | STRIP_XOUT], [0], [dnl
> -skb_priority=0,in_port=1,nw_frag=no, n_subfacets:2, used:0.0s, Datapath actions: <del>
> +skb_priority=0,ip,in_port=1,nw_frag=no, n_subfacets:2, used:0.0s, Datapath actions: <del>
> ])
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> @@ -2242,8 +2242,8 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
> AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.4,dst=10.0.0.3,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
> AT_CHECK([ovs-appctl dpif/dump-megaflows br0 | STRIP_XOUT], [0], [dnl
> -skb_priority=0,in_port=1,dl_src=50:54:00:00:00:09,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> -skb_priority=0,in_port=1,dl_src=50:54:00:00:00:0b,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> +skb_priority=0,ip,in_port=1,dl_src=50:54:00:00:00:09,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> +skb_priority=0,ip,in_port=1,dl_src=50:54:00:00:00:0b,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> ])
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> @@ -2474,7 +2474,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
> AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.4,dst=10.0.0.3,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
> AT_CHECK([ovs-appctl dpif/dump-megaflows br0 | STRIP_XOUT], [0], [dnl
> -skb_priority=0,in_port=1,nw_frag=no, n_subfacets:2, used:0.0s, Datapath actions: <del>
> +skb_priority=0,ip,in_port=1,nw_frag=no, n_subfacets:2, used:0.0s, Datapath actions: <del>
> ])
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> @@ -2494,8 +2494,8 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
> AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x8100),vlan(vid=11,pcp=7),encap(eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0))'])
> AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.4,dst=10.0.0.3,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
> AT_CHECK([ovs-appctl dpif/dump-megaflows br0 | STRIP_XOUT], [0], [dnl
> -skb_priority=0,in_port=1,dl_vlan=11,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> -skb_priority=0,in_port=1,vlan_tci=0x0000/0x1fff,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> +skb_priority=0,ip,in_port=1,dl_vlan=11,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> +skb_priority=0,ip,in_port=1,vlan_tci=0x0000/0x1fff,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> ])
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> @@ -2545,8 +2545,8 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:
> AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.4,dst=10.0.0.3,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
> dnl The original flow is missing due to a revalidation.
> AT_CHECK([ovs-appctl dpif/dump-megaflows br0 | STRIP_XOUT], [0], [dnl
> -skb_priority=0,in_port=1,vlan_tci=0x0000/0x0fff,dl_src=50:54:00:00:00:09,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> -skb_priority=0,in_port=1,vlan_tci=0x0000/0x0fff,dl_src=50:54:00:00:00:0b,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> +skb_priority=0,ip,in_port=1,vlan_tci=0x0000/0x0fff,dl_src=50:54:00:00:00:09,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> +skb_priority=0,ip,in_port=1,vlan_tci=0x0000/0x0fff,dl_src=50:54:00:00:00:0b,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> ])
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> @@ -2573,9 +2573,9 @@ AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:
> AT_CHECK([ovs-appctl netdev-dummy/receive p3 'in_port(3),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0xfd,ttl=128,frag=no),icmp(type=8,code=0)'])
> AT_CHECK([ovs-appctl netdev-dummy/receive p3 'in_port(3),eth(src=50:54:00:00:00:0b,dst=50:54:00:00:00:0c),eth_type(0x0800),ipv4(src=10.0.0.4,dst=10.0.0.3,proto=1,tos=0x1,ttl=64,frag=no),icmp(type=8,code=0)'])
> AT_CHECK([ovs-appctl dpif/dump-megaflows br0 | STRIP_XOUT], [0], [dnl
> -skb_priority=0,in_port=1,nw_ecn=1,nw_frag=no, n_subfacets:2, used:0.0s, Datapath actions: <del>
> -skb_priority=0,in_port=3,nw_tos=0,nw_ecn=1,nw_ttl=64,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> -skb_priority=0,in_port=3,nw_tos=252,nw_ecn=1,nw_ttl=128,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> +skb_priority=0,ip,in_port=1,nw_ecn=1,nw_frag=no, n_subfacets:2, used:0.0s, Datapath actions: <del>
> +skb_priority=0,ip,in_port=3,nw_tos=0,nw_ecn=1,nw_ttl=64,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> +skb_priority=0,ip,in_port=3,nw_tos=252,nw_ecn=1,nw_ttl=128,nw_frag=no, n_subfacets:1, used:0.0s, Datapath actions: <del>
> ])
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> --
> 1.7.9.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list