On Mon, Aug 29, 2016 at 11:58:14AM -0700, Jesse Gross wrote:
> When flows are read by ovs-ofctl (either from a switch or a file),
> tunnel metadata space is dynamically allocated since there isn't a
> preset table. This works well for single flows but doesn't handle
> groups of flows that must be compared to each other. In this case,
> each flow will have its own independent allocation making comparisons
> meaningless.
>
> Even worse is that when these matches are later serialized (either
> for display or in NXM format), the metadata allocation has been
> stripped off of the matches. The serialization code then attempts to
> use the global table, which is also not available, leading to a
> dereference of a NULL pointer.
>
> Solving this problem requires building an overall metadata table.
> Since we don't know the maximum size of a field (particularly for
> flows read from a file), it's necessary to do this in two passes.
> The first pass records the maximum size for each field as well as
> stores the received matches. The second pass creates a metadata
> table based on the sizes, adjusts the match layout based on the new
> allocation, and then replays the stored matches for comparison.
> Later serialization will used the generated table to output the
> flows.
>
> Signed-off-by: Jesse Gross <je...@kernel.org>
Good catch.
Acked-by: Ben Pfaff <b...@ovn.org>
Suggested improvements:
--8<--------------------------cut here-------------------------->8--
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index dbeeebf..803684c 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -3066,16 +3066,14 @@ generate_tun_metadata(struct fte_state *state)
static void
remap_match(struct match *match)
{
- struct tun_metadata flow, flow_mask;
int i;
if (!match->tun_md.valid) {
return;
}
- memcpy(&flow, &match->flow.tunnel.metadata, sizeof flow);
- memcpy(&flow_mask, &match->wc.masks.tunnel.metadata, sizeof flow_mask);
-
+ struct tun_metadata flow = match->flow.tunnel.metadata;
+ struct tun_metadata flow_mask = match->wc.masks.tunnel.metadata;
memset(&match->flow.tunnel.metadata, 0, sizeof
match->flow.tunnel.metadata);
memset(&match->wc.masks.tunnel.metadata, 0,
sizeof match->wc.masks.tunnel.metadata);
@@ -3105,7 +3103,7 @@ remap_match(struct match *match)
* allocated space. In the case of flows coming from a file, we don't
* even know the size of each field when we need to do the allocation.
* When the flows come in, each flow has an individual allocation based
- * on it's own fields. However, this allocation is not the same across
+ * on its own fields. However, this allocation is not the same across
* different flows and therefore fields are not directly comparable.
*
* In the first pass, we record the maximum size of each tunnel metadata
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
