[ovs-dev] [bug10576 4/5] learn: Fix bugs when learn actions use subfields wider than 64 bits.
Ben Pfaff
blp at nicira.com
Thu Apr 12 00:15:37 UTC 2012
Bug #10576.
Reported-by: James Schmidt <jschmidt at nicira.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
include/openflow/nicira-ext.h | 3 ++
lib/learn.c | 45 +++++++++++++++++++++++++++++++----------
tests/learn.at | 42 ++++++++++++++++++++++++++++++++++++++
3 files changed, 79 insertions(+), 11 deletions(-)
diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index 7586a52..f8b863c 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -787,6 +787,9 @@ enum nx_mp_algorithm {
* field[ofs:ofs+n_bits-1]. Actions are executed in the same order as the
* flow_mod_specs.
*
+ * A single NXAST_REG_LOAD action writes no more than 64 bits, so n_bits
+ * greater than 64 yields multiple NXAST_REG_LOAD actions.
+ *
* The flow_mod_spec destination spec for 'dst' of 2 (when 'src' is 0) is
* empty. It has the following meaning:
*
diff --git a/lib/learn.c b/lib/learn.c
index 94fc1b0..d2db792 100644
--- a/lib/learn.c
+++ b/lib/learn.c
@@ -177,7 +177,14 @@ learn_check(const struct nx_action_learn *learn, const struct flow *flow)
if (dst_type == NX_LEARN_DST_MATCH
&& src_type == NX_LEARN_SRC_IMMEDIATE) {
- mf_set_subfield(&dst, value, &rule);
+ 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. */
+ }
}
}
}
@@ -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
More information about the dev
mailing list