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 <[email protected]>
Signed-off-by: Simon Horman <[email protected]>
---
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.
---
datapath/datapath.c | 4 +++-
datapath/flow.c | 2 --
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index a514e74..0b6664a 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1260,6 +1260,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
if (!a[OVS_FLOW_ATTR_KEY])
goto error;
+ memset(&key, 0, sizeof(key));
ovs_match_init(&match, &key, &mask);
error = ovs_match_from_nlattrs(&match,
a[OVS_FLOW_ATTR_KEY], a[OVS_FLOW_ATTR_MASK]);
@@ -1407,7 +1408,7 @@ static int ovs_flow_cmd_get(struct sk_buff *skb, struct
genl_info *info)
if (!a[OVS_FLOW_ATTR_KEY])
return -EINVAL;
-
+ memset(&key, 0, sizeof(key));
ovs_match_init(&match, &key, NULL);
err = ovs_match_from_nlattrs(&match, a[OVS_FLOW_ATTR_KEY], NULL);
if (err)
@@ -1465,6 +1466,7 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct
genl_info *info)
goto unlock;
}
+ memset(&key, 0, sizeof(key));
ovs_match_init(&match, &key, NULL);
err = ovs_match_from_nlattrs(&match, a[OVS_FLOW_ATTR_KEY], NULL);
if (err)
diff --git a/datapath/flow.c b/datapath/flow.c
index 499e3e2..d4333d7 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -108,8 +108,6 @@ void ovs_match_init(struct sw_flow_match *match,
match->key = key;
match->mask = mask;
- memset(key, 0, sizeof(*key));
-
if (mask) {
memset(&mask->key, 0, sizeof(mask->key));
mask->range.start = mask->range.end = 0;
--
1.8.2.1
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
