[ovs-dev] [PATCH] datapath: Do not clear key in ovs_match_init()

Simon Horman horms at verge.net.au
Mon Jun 24 02:29:49 UTC 2013


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 <azhou at nicira.com>
Signed-off-by: Simon Horman <horms at 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.
---
 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




More information about the dev mailing list