[ovs-dev] [bug10576 4/5] learn: Fix bugs when learn actions use subfields wider than 64 bits.

Ethan Jackson ethan at nicira.com
Sat Apr 14 00:58:28 UTC 2012


> +                if (n_bits <= 64) {
> +                    mf_set_subfield(&dst, value, &rule);
> +                } else {
> +                    /* We're only setting subfields to allow us to check
> +                     * prerequisites.  No prerequisite depends on the value of
> +                     * a field that is wider than 64 bits.  So just skip
> +                     * setting it entirely. */
> +                }

Is there some sort of assertion we could make here in case this
assumption changes in the future?  If there isn't anything convenient,
I think the comment is sufficient.

Everything else looks good, thanks.

Ethan


>             }
>         }
>     }
> @@ -223,10 +230,10 @@ learn_execute(const struct nx_action_learn *learn, const struct flow *flow,
>         int n_bits = header & NX_LEARN_N_BITS_MASK;
>         int src_type = header & NX_LEARN_SRC_MASK;
>         int dst_type = header & NX_LEARN_DST_MASK;
> -        uint64_t value;
> +        union mf_subvalue value;
>
> -        struct nx_action_reg_load *load;
>         struct mf_subfield dst;
> +        int chunk, ofs;
>
>         if (!header) {
>             break;
> @@ -236,27 +243,43 @@ learn_execute(const struct nx_action_learn *learn, const struct flow *flow,
>             struct mf_subfield src;
>
>             get_subfield(n_bits, &p, &src);
> -            value = mf_get_subfield(&src, flow);
> +            mf_read_subfield(&src, flow, &value);
>         } else {
> -            value = get_bits(n_bits, &p);
> +            int p_bytes = 2 * DIV_ROUND_UP(n_bits, 16);
> +
> +            memset(&value, 0, sizeof value);
> +            bitwise_copy(p, p_bytes, 0,
> +                         &value, sizeof value, 0,
> +                         n_bits);
> +            p = (const uint8_t *) p + p_bytes;
>         }
>
>         switch (dst_type) {
>         case NX_LEARN_DST_MATCH:
>             get_subfield(n_bits, &p, &dst);
> -            mf_set_subfield(&dst, value, &fm->cr);
> +            mf_write_subfield(&dst, &value, &fm->cr);
>             break;
>
>         case NX_LEARN_DST_LOAD:
>             get_subfield(n_bits, &p, &dst);
> -            load = ofputil_put_NXAST_REG_LOAD(&actions);
> -            load->ofs_nbits = nxm_encode_ofs_nbits(dst.ofs, dst.n_bits);
> -            load->dst = htonl(dst.field->nxm_header);
> -            load->value = htonll(value);
> +            for (ofs = 0; ofs < n_bits; ofs += chunk) {
> +                struct nx_action_reg_load *load;
> +
> +                chunk = MIN(n_bits - ofs, 64);
> +
> +                load = ofputil_put_NXAST_REG_LOAD(&actions);
> +                load->ofs_nbits = nxm_encode_ofs_nbits(dst.ofs + ofs, chunk);
> +                load->dst = htonl(dst.field->nxm_header);
> +                bitwise_copy(&value, sizeof value, ofs,
> +                             &load->value, sizeof load->value, 0,
> +                             chunk);
> +            }
>             break;
>
>         case NX_LEARN_DST_OUTPUT:
> -            ofputil_put_OFPAT10_OUTPUT(&actions)->port = htons(value);
> +            if (n_bits <= 16 || is_all_zeros(value.u8, sizeof value - 2)) {
> +                ofputil_put_OFPAT10_OUTPUT(&actions)->port = value.be16[7];
> +            }
>             break;
>         }
>     }
> diff --git a/tests/learn.at b/tests/learn.at
> index 861c2f9..8431937 100644
> --- a/tests/learn.at
> +++ b/tests/learn.at
> @@ -112,6 +112,48 @@ NXST_FLOW reply:
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
>
> +AT_SETUP([learning action - TCPv4 port learning])
> +OVS_VSWITCHD_START(
> +  [add-port br0 eth0 -- set Interface eth0 type=dummy -- \
> +   add-port br0 eth1 -- set Interface eth1 type=dummy -- \
> +   add-port br0 eth2 -- set Interface eth2 type=dummy])
> +# Set up flow table for TCPv4 port learning.
> +AT_CHECK([[ovs-ofctl add-flow br0 'table=0 tcp actions=learn(table=1, hard_timeout=60, eth_type=0x800, nw_proto=6, NXM_OF_IP_SRC[]=NXM_OF_IP_DST[], NXM_OF_IP_DST[]=NXM_OF_IP_SRC[], NXM_OF_TCP_SRC[]=NXM_OF_TCP_DST[], NXM_OF_TCP_DST[]=NXM_OF_TCP_SRC[]), flood']])
> +
> +# Trace a TCPv4 packet arriving on port 3.
> +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port(3),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:06),eth_type(0x0800),ipv4(src=192.168.0.2,dst=192.168.0.1,proto=6,tos=0,ttl=64,frag=no),tcp(src=40000,dst=80)' -generate], [0], [stdout])
> +AT_CHECK([tail -1 stdout], [0], [Datapath actions: 2,0,1
> +])
> +
> +# Check for the learning entry.
> +AT_CHECK([ovs-ofctl dump-flows br0 table=1 | ofctl_strip | sort], [0], [dnl
> + table=1, hard_timeout=60,tcp,nw_src=192.168.0.1,nw_dst=192.168.0.2,tp_src=80,tp_dst=40000 actions=drop
> +NXST_FLOW reply:
> +])
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +
> +AT_SETUP([learning action - TCPv6 port learning])
> +OVS_VSWITCHD_START(
> +  [add-port br0 eth0 -- set Interface eth0 type=dummy -- \
> +   add-port br0 eth1 -- set Interface eth1 type=dummy -- \
> +   add-port br0 eth2 -- set Interface eth2 type=dummy])
> +# Set up flow table for TCPv6 port learning.
> +AT_CHECK([[ovs-ofctl add-flow br0 'table=0 tcp6 actions=learn(table=1, hard_timeout=60, eth_type=0x86dd, nw_proto=6, NXM_NX_IPV6_SRC[]=NXM_NX_IPV6_DST[], NXM_NX_IPV6_DST[]=NXM_NX_IPV6_SRC[], NXM_OF_TCP_SRC[]=NXM_OF_TCP_DST[], NXM_OF_TCP_DST[]=NXM_OF_TCP_SRC[]), flood']])
> +
> +# Trace a TCPv6 packet arriving on port 3.
> +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port(3),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:06),eth_type(0x86dd),ipv6(src=fec0::2,dst=fec0::1,label=0,proto=6,tclass=0,hlimit=255,frag=no),tcp(src=40000,dst=80)' -generate], [0], [stdout])
> +AT_CHECK([tail -1 stdout], [0], [Datapath actions: 2,0,1
> +])
> +
> +# Check for the learning entry.
> +AT_CHECK([ovs-ofctl dump-flows br0 table=1 | ofctl_strip | sort], [0], [dnl
> + table=1, hard_timeout=60,tcp6,ipv6_src=fec0::1,ipv6_dst=fec0::2,tp_src=80,tp_dst=40000 actions=drop
> +NXST_FLOW reply:
> +])
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +
>  AT_SETUP([learning action - fin_timeout feature])
>  # This is a totally artificial use of the "learn" action.  The only purpose
>  # is to check that specifying fin_idle_timeout or fin_hard_timeout causes
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list