In the ODP context an empty mask netlink attribute usually means that
the flow should be an exact match.
odp_flow_key_to_mask() instead returns a struct flow_wildcards
with matches only on recirc_id and vlan_tci.
A more appropriate behavior is to handle a missing (zero length) netlink
mask specially (like we do in userspace and Linux datapath) and create
an exact match flow_wildcards from the original flow.
This fixes a bug in revalidate_ukey(): every flow created with
megaflows disabled would be revalidated away, because the mask would
seem too generic. (Another possible fix would be to handle the special
case of a missing mask in revalidate_ukey(), but this seems a more
generic solution).
Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
---
lib/dpif-netdev.c | 65 +++++++++++++++----------------------------
lib/odp-util.c | 26 +++++++++++++++--
lib/odp-util.h | 2 +-
ofproto/ofproto-dpif-upcall.c | 5 ++--
tests/test-odp.c | 4 +--
utilities/ovs-dpctl.c | 2 +-
6 files changed, 54 insertions(+), 50 deletions(-)
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 78f8636..5fe4430 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1100,48 +1100,29 @@ static int
dpif_netdev_mask_from_nlattrs(const struct nlattr *key, uint32_t key_len,
const struct nlattr *mask_key,
uint32_t mask_key_len, const struct flow *flow,
- struct flow *mask)
-{
- if (mask_key_len) {
- enum odp_key_fitness fitness;
-
- fitness = odp_flow_key_to_mask(mask_key, mask_key_len, mask, flow);
- if (fitness) {
- /* This should not happen: it indicates that
- * odp_flow_key_from_mask() and odp_flow_key_to_mask()
- * disagree on the acceptable form of a mask. Log the problem
- * as an error, with enough details to enable debugging. */
- static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-
- if (!VLOG_DROP_ERR(&rl)) {
- struct ds s;
-
- ds_init(&s);
- odp_flow_format(key, key_len, mask_key, mask_key_len, NULL, &s,
- true);
- VLOG_ERR("internal error parsing flow mask %s (%s)",
- ds_cstr(&s), odp_key_fitness_to_string(fitness));
- ds_destroy(&s);
- }
+ struct flow_wildcards *mask)
+{
+ enum odp_key_fitness fitness;
- return EINVAL;
- }
- } else {
- enum mf_field_id id;
- /* No mask key, unwildcard everything except fields whose
- * prerequisities are not met. */
- memset(mask, 0x0, sizeof *mask);
-
- for (id = 0; id < MFF_N_IDS; ++id) {
- /* Skip registers and metadata. */
- if (!(id >= MFF_REG0 && id < MFF_REG0 + FLOW_N_REGS)
- && id != MFF_METADATA) {
- const struct mf_field *mf = mf_from_id(id);
- if (mf_are_prereqs_ok(mf, flow)) {
- mf_mask_field(mf, mask);
- }
- }
+ fitness = odp_flow_key_to_mask(mask_key, mask_key_len, mask, flow);
+ if (fitness) {
+ /* This should not happen: it indicates that
+ * odp_flow_key_from_mask() and odp_flow_key_to_mask()
+ * disagree on the acceptable form of a mask. Log the problem
+ * as an error, with enough details to enable debugging. */
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+
+ if (!VLOG_DROP_ERR(&rl)) {
+ struct ds s;
+
+ ds_init(&s);
+ odp_flow_format(key, key_len, mask_key, mask_key_len, NULL, &s,
+ true);
+ VLOG_ERR("internal error parsing flow mask %s (%s)",
+ ds_cstr(&s), odp_key_fitness_to_string(fitness));
+ ds_destroy(&s);
}
+ return EINVAL;
}
/* Force unwildcard the in_port.
@@ -1150,7 +1131,7 @@ dpif_netdev_mask_from_nlattrs(const struct nlattr *key,
uint32_t key_len,
* above because "everything" only includes the 16-bit OpenFlow port number
* mask->in_port.ofp_port, which only covers half of the 32-bit datapath
* port number mask->in_port.odp_port. */
- mask->in_port.odp_port = u32_to_odp(UINT32_MAX);
+ mask->masks.in_port.odp_port = u32_to_odp(UINT32_MAX);
return 0;
}
@@ -1313,7 +1294,7 @@ dpif_netdev_flow_put(struct dpif *dpif, const struct
dpif_flow_put *put)
}
error = dpif_netdev_mask_from_nlattrs(put->key, put->key_len,
put->mask, put->mask_len,
- &flow, &wc.masks);
+ &flow, &wc);
if (error) {
return error;
}
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 3ef13b4..59466cc 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -29,6 +29,7 @@
#include "dpif.h"
#include "dynamic-string.h"
#include "flow.h"
+#include "meta-flow.h"
#include "netlink.h"
#include "ofpbuf.h"
#include "packets.h"
@@ -3422,9 +3423,30 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t
key_len,
* 'key' fits our expectations for what a flow key should contain. */
enum odp_key_fitness
odp_flow_key_to_mask(const struct nlattr *key, size_t key_len,
- struct flow *mask, const struct flow *flow)
+ struct flow_wildcards *mask, const struct flow *flow)
{
- return odp_flow_key_to_flow__(key, key_len, mask, flow);
+ if (key_len) {
+ return odp_flow_key_to_flow__(key, key_len, &mask->masks, flow);
+ } else {
+ enum mf_field_id id;
+ /* A missing mask means that the flow should be exact matched.
+ * Generate an appropriate exact wildcard for the flow by
+ * unwildcarding everything except fields whose prerequisities
+ * are not met. */
+ memset(mask, 0x0, sizeof *mask);
+
+ for (id = 0; id < MFF_N_IDS; ++id) {
+ /* Skip registers and metadata. */
+ if (!(id >= MFF_REG0 && id < MFF_REG0 + FLOW_N_REGS)
+ && id != MFF_METADATA) {
+ const struct mf_field *mf = mf_from_id(id);
+ if (mf_are_prereqs_ok(mf, flow)) {
+ mf_mask_field(mf, &mask->masks);
+ }
+ }
+ }
+ return ODP_FIT_PERFECT;
+ }
}
/* Returns 'fitness' as a string, for use in debug messages. */
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 0dfbcca..ff6133d 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -173,7 +173,7 @@ enum odp_key_fitness {
enum odp_key_fitness odp_flow_key_to_flow(const struct nlattr *, size_t,
struct flow *);
enum odp_key_fitness odp_flow_key_to_mask(const struct nlattr *key, size_t len,
- struct flow *mask,
+ struct flow_wildcards *mask,
const struct flow *flow);
const char *odp_key_fitness_to_string(enum odp_key_fitness);
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 193e6b7..6123590 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1217,15 +1217,16 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key
*ukey,
{
uint64_t slow_path_buf[128 / 8];
struct xlate_out xout, *xoutp;
+ struct flow_wildcards dp_mask;
struct netflow *netflow;
struct ofproto_dpif *ofproto;
struct dpif_flow_stats push;
struct ofpbuf xout_actions;
- struct flow flow, dp_mask;
uint32_t *dp32, *xout32;
odp_port_t odp_in_port;
struct xlate_in xin;
long long int last_used;
+ struct flow flow;
int error;
size_t i;
bool may_learn, ok;
@@ -1313,7 +1314,7 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key
*ukey,
* mask in the kernel is more specific i.e. less wildcarded, than what
* we've calculated here. This guarantees we don't catch any packets we
* shouldn't with the megaflow. */
- dp32 = (uint32_t *) &dp_mask;
+ dp32 = (uint32_t *) &dp_mask.masks;
xout32 = (uint32_t *) &xout.wc.masks;
for (i = 0; i < FLOW_U32S; i++) {
if ((dp32[i] | xout32[i]) != dp32[i]) {
diff --git a/tests/test-odp.c b/tests/test-odp.c
index 30cdbbf..ab3dfd4 100644
--- a/tests/test-odp.c
+++ b/tests/test-odp.c
@@ -186,8 +186,8 @@ parse_filter(char *filter_parse)
struct minimatch minimatch;
odp_flow_key_to_flow(ofpbuf_data(&odp_key), ofpbuf_size(&odp_key),
&flow);
- odp_flow_key_to_mask(ofpbuf_data(&odp_mask),
ofpbuf_size(&odp_mask), &wc.masks,
- &flow);
+ odp_flow_key_to_mask(ofpbuf_data(&odp_mask),
ofpbuf_size(&odp_mask),
+ &wc, &flow);
match_init(&match, &flow, &wc);
match_init(&match_filter, &flow_filter, &wc);
diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c
index ccc55b5..7a38ff6 100644
--- a/utilities/ovs-dpctl.c
+++ b/utilities/ovs-dpctl.c
@@ -807,7 +807,7 @@ dpctl_dump_flows(int argc, char *argv[])
struct minimatch minimatch;
odp_flow_key_to_flow(key, key_len, &flow);
- odp_flow_key_to_mask(mask, mask_len, &wc.masks, &flow);
+ odp_flow_key_to_mask(mask, mask_len, &wc, &flow);
match_init(&match, &flow, &wc);
match_init(&match_filter, &flow_filter, &wc);
--
2.1.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
