On Sun, Jun 23, 2013 at 7:29 PM, Simon Horman <ho...@verge.net.au> wrote:
> It appears that in the case where vs_match_init() is called from
> ovs_flow_metadata_from_nlattrs() it is undesirable to set the flow key as
> some of its values are set earlier on in ovs_flow_metadata_from_nlattrs().
> Furthermore in the case where ovs_flow_metadata_from_nlattrs() is called
> from ovs_packet_cmd_execute() key has been partially initialised by
> ovs_flow_extract().
>
> This manifests in a problem when executing actions as elements of key are
> used when verifying some actions. For example a dec_ttl action verifies the
> proto of the flow. An example of a flow that fails as a result of this
> problem is:
>
> ovs-ofctl add-flow br0 "ip actions=dec_ttl,normal"
>
> This patch resolves this problem by not clearing key in ovs_match_init()
> and instead clearing it before calling ovs_match_init() when it is called
> other than by ovs_flow_metadata_from_nlattrs().
>
> This appears to be a regression added by "datapath: Mega flow
> implementation", a1c564be1e2ffc31f8da09ab654c8ed987907fe5.
>
> Cc: Andy Zhou <az...@nicira.com>
> Signed-off-by: Simon Horman <ho...@verge.net.au>
>
> ---
>
> I am unsure of the exact intention of clearing the key and thus the
> correctness of this patch. However, it does appear to resolve
> the problem that I describe above.
Thanks, I think this can simplified a little bit. I applied the following patch:
commit c121c7ce259016fb5630c382bf99f9ac4d5900fc
Author: Jesse Gross <je...@nicira.com>
Date: Mon Jun 24 12:21:29 2013 -0700
datapath: Do not clear key in ovs_match_init()
When executing packets sent from userspace, the majority of the
flow information is extracted from the packet itself and a small
amount of metadata supplied by userspace is added. However, when
adding this metadata, the extracted flow information is currently
being cleared.
This manifests in a problem when executing actions as elements of key are
used when verifying some actions. For example a dec_ttl action verifies th
proto of the flow. An example of a flow that fails as a result of this
problem is:
ovs-ofctl add-flow br0 "ip actions=dec_ttl,normal"
This is a regression added by "datapath: Mega flow implementation",
a1c564be1e2ffc31f8da09ab654c8ed987907fe5.
CC: Andy Zhou <az...@nicira.com>
Reported-by: Simon Horman <ho...@verge.net.au>
Signed-off-by: Jesse Gross <je...@nicira.com>
diff --git a/datapath/flow.c b/datapath/flow.c
index 499e3e2..39de931 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -1601,7 +1601,8 @@ int ovs_flow_metadata_from_nlattrs(struct sw_flow *flow,
if (err)
return -EINVAL;
- ovs_match_init(&match, &flow->key, NULL);
+ memset(&match, 0, sizeof(match));
+ match.key = &flow->key;
err = metadata_from_nlattrs(&match, &attrs, a, false);
if (err)
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
