summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--lib/dpctl.c31
-rw-r--r--lib/dpif-netdev.c4
-rw-r--r--lib/netdev-dummy.c15
-rw-r--r--lib/odp-execute.c6
-rw-r--r--lib/odp-util.c342
-rw-r--r--lib/odp-util.h15
-rw-r--r--ofproto/ofproto-dpif-sflow.c2
-rw-r--r--ofproto/ofproto-dpif-trace.c78
-rw-r--r--ofproto/ofproto-dpif-upcall.c15
-rw-r--r--ofproto/ofproto-dpif-xlate.c25
-rw-r--r--ofproto/ofproto-dpif-xlate.h3
-rw-r--r--ofproto/ofproto-dpif.c6
-rw-r--r--tests/odp.at2
-rw-r--r--tests/ofproto-dpif.at2
-rw-r--r--tests/test-odp.c26
-rw-r--r--tests/tunnel.at4
16 files changed, 401 insertions, 175 deletions
diff --git a/lib/dpctl.c b/lib/dpctl.c
index f5a09b70f..947a49099 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008-2018 Nicira, Inc.
+ * Copyright (c) 2008-2019 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -1023,8 +1023,8 @@ dpctl_dump_flows(int argc, const char *argv[], struct dpctl_params *dpctl_p)
struct match match, match_filter;
struct minimatch minimatch;
- odp_flow_key_to_flow(f.key, f.key_len, &flow);
- odp_flow_key_to_mask(f.mask, f.mask_len, &wc, &flow);
+ odp_flow_key_to_flow(f.key, f.key_len, &flow, NULL);
+ odp_flow_key_to_mask(f.mask, f.mask_len, &wc, &flow, NULL);
match_init(&match, &flow, &wc);
match_init(&match_filter, &flow_filter, &wc);
@@ -1112,10 +1112,12 @@ dpctl_put_flow(int argc, const char *argv[], enum dpif_flow_put_flags flags,
ofpbuf_init(&key, 0);
ofpbuf_init(&mask, 0);
- error = odp_flow_from_string(key_s, &port_names, &key, &mask);
+ char *error_s;
+ error = odp_flow_from_string(key_s, &port_names, &key, &mask, &error_s);
simap_destroy(&port_names);
if (error) {
- dpctl_error(dpctl_p, error, "parsing flow key");
+ dpctl_error(dpctl_p, error, "parsing flow key (%s)", error_s);
+ free(error_s);
goto out_freekeymask;
}
@@ -1264,9 +1266,11 @@ dpctl_del_flow(int argc, const char *argv[], struct dpctl_params *dpctl_p)
ofpbuf_init(&key, 0);
ofpbuf_init(&mask, 0);
- error = odp_flow_from_string(key_s, &port_names, &key, &mask);
+ char *error_s;
+ error = odp_flow_from_string(key_s, &port_names, &key, &mask, &error_s);
if (error) {
- dpctl_error(dpctl_p, error, "parsing flow key");
+ dpctl_error(dpctl_p, error, "%s", error_s);
+ free(error_s);
goto out;
}
@@ -2281,9 +2285,12 @@ dpctl_normalize_actions(int argc, const char *argv[],
/* Parse flow key. */
ofpbuf_init(&keybuf, 0);
- error = odp_flow_from_string(argv[1], &port_names, &keybuf, NULL);
+ char *error_s;
+ error = odp_flow_from_string(argv[1], &port_names, &keybuf, NULL,
+ &error_s);
if (error) {
- dpctl_error(dpctl_p, error, "odp_flow_key_from_string");
+ dpctl_error(dpctl_p, error, "odp_flow_key_from_string (%s)", error_s);
+ free(error_s);
goto out_freekeybuf;
}
@@ -2292,9 +2299,11 @@ dpctl_normalize_actions(int argc, const char *argv[],
&s, dpctl_p->verbosity);
dpctl_print(dpctl_p, "input flow: %s\n", ds_cstr(&s));
- error = odp_flow_key_to_flow(keybuf.data, keybuf.size, &flow);
+ error = odp_flow_key_to_flow(keybuf.data, keybuf.size, &flow, &error_s);
if (error) {
- dpctl_error(dpctl_p, error, "odp_flow_key_to_flow");
+ dpctl_error(dpctl_p, error, "odp_flow_key_to_flow failed (%s)",
+ error_s ? error_s : "reason unknown");
+ free(error_s);
goto out_freekeybuf;
}
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 77ac1d2c1..ec84378a0 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3072,7 +3072,7 @@ dpif_netdev_mask_from_nlattrs(const struct nlattr *key, uint32_t key_len,
{
enum odp_key_fitness fitness;
- fitness = odp_flow_key_to_mask(mask_key, mask_key_len, wc, flow);
+ fitness = odp_flow_key_to_mask(mask_key, mask_key_len, wc, flow, NULL);
if (fitness) {
if (!probe) {
/* This should not happen: it indicates that
@@ -3103,7 +3103,7 @@ static int
dpif_netdev_flow_from_nlattrs(const struct nlattr *key, uint32_t key_len,
struct flow *flow, bool probe)
{
- if (odp_flow_key_to_flow(key, key_len, flow)) {
+ if (odp_flow_key_to_flow(key, key_len, flow, NULL)) {
if (!probe) {
/* This should not happen: it indicates that
* odp_flow_key_from_flow() and odp_flow_key_to_flow() disagree on
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 72b4f7adc..78aa34957 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -1462,8 +1462,10 @@ eth_from_packet(const char *s)
}
static struct dp_packet *
-eth_from_flow(const char *s, size_t packet_size)
+eth_from_flow(const char *s, size_t packet_size, char **errorp)
{
+ *errorp = NULL;
+
enum odp_key_fitness fitness;
struct dp_packet *packet;
struct ofpbuf odp_key;
@@ -1477,14 +1479,14 @@ eth_from_flow(const char *s, size_t packet_size)
* settle for parsing a datapath key for now.
*/
ofpbuf_init(&odp_key, 0);
- error = odp_flow_from_string(s, NULL, &odp_key, NULL);
+ error = odp_flow_from_string(s, NULL, &odp_key, NULL, errorp);
if (error) {
ofpbuf_uninit(&odp_key);
return NULL;
}
/* Convert odp_key to flow. */
- fitness = odp_flow_key_to_flow(odp_key.data, odp_key.size, &flow);
+ fitness = odp_flow_key_to_flow(odp_key.data, odp_key.size, &flow, errorp);
if (fitness == ODP_FIT_ERROR) {
ofpbuf_uninit(&odp_key);
return NULL;
@@ -1592,10 +1594,11 @@ netdev_dummy_receive(struct unixctl_conn *conn,
i += 2;
}
/* Try parse 'argv[i]' as odp flow. */
- packet = eth_from_flow(flow_str, packet_size);
-
+ char *error_s;
+ packet = eth_from_flow(flow_str, packet_size, &error_s);
if (!packet) {
- unixctl_command_reply_error(conn, "bad packet or flow syntax");
+ unixctl_command_reply_error(conn, error_s);
+ free(error_s);
goto exit;
}
}
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 5d0713375..3ad91547c 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -198,7 +198,7 @@ odp_set_sctp(struct dp_packet *packet, const struct ovs_key_sctp *key,
static void
odp_set_tunnel_action(const struct nlattr *a, struct flow_tnl *tun_key)
{
- ovs_assert(odp_tun_key_from_attr(a, tun_key) != ODP_FIT_ERROR);
+ ovs_assert(odp_tun_key_from_attr(a, tun_key, NULL) != ODP_FIT_ERROR);
}
static void
@@ -297,9 +297,9 @@ odp_set_nsh(struct dp_packet *packet, const struct nlattr *a, bool has_mask)
ovs_be32 path_hdr;
if (has_mask) {
- odp_nsh_key_from_attr(a, &key, &mask);
+ odp_nsh_key_from_attr(a, &key, &mask, NULL);
} else {
- odp_nsh_key_from_attr(a, &key, NULL);
+ odp_nsh_key_from_attr(a, &key, NULL, NULL);
}
if (!has_mask) {
diff --git a/lib/odp-util.c b/lib/odp-util.c
index e288ae8e5..8bcbd2c30 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2019 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -2647,10 +2647,48 @@ odp_nsh_hdr_from_attr(const struct nlattr *attr,
return ODP_FIT_PERFECT;
}
+/* Reports the error 'msg', which is formatted as with printf().
+ *
+ * If 'errorp' is nonnull, then some the wants the error report to come
+ * directly back to it, so the function stores the error message into '*errorp'
+ * (after first freeing it in case there's something there already).
+ *
+ * Otherwise, logs the message at WARN level, rate-limited. */
+static void OVS_PRINTF_FORMAT(3, 4)
+odp_parse_error(struct vlog_rate_limit *rl, char **errorp,
+ const char *msg, ...)
+{
+ if (OVS_UNLIKELY(errorp)) {
+ free(*errorp);
+
+ va_list args;
+ va_start(args, msg);
+ *errorp = xvasprintf(msg, args);
+ va_end(args);
+ } else if (!VLOG_DROP_WARN(rl)) {
+ va_list args;
+ va_start(args, msg);
+ char *error = xvasprintf(msg, args);
+ va_end(args);
+
+ VLOG_WARN("%s", error);
+
+ free(error);
+ }
+}
+
+/* Parses OVS_KEY_ATTR_NSH attribute 'attr' into 'nsh' and 'nsh_mask' and
+ * returns fitness. If 'errorp' is nonnull and the function returns
+ * ODP_FIT_ERROR, stores a malloc()'d error message in '*errorp'. */
enum odp_key_fitness
odp_nsh_key_from_attr(const struct nlattr *attr, struct ovs_key_nsh *nsh,
- struct ovs_key_nsh *nsh_mask)
+ struct ovs_key_nsh *nsh_mask, char **errorp)
{
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+ if (errorp) {
+ *errorp = NULL;
+ }
+
unsigned int left;
const struct nlattr *a;
bool unknown = false;
@@ -2661,17 +2699,18 @@ odp_nsh_key_from_attr(const struct nlattr *attr, struct ovs_key_nsh *nsh,
size_t len = nl_attr_get_size(a);
int expected_len = odp_key_attr_len(ovs_nsh_key_attr_lens,
OVS_NSH_KEY_ATTR_MAX, type);
-
- /* the attribute can have mask, len is 2 * expected_len for that case.
- */
- if ((len != expected_len) && (len != 2 * expected_len) &&
- (expected_len >= 0)) {
- return ODP_FIT_ERROR;
- }
-
- if ((nsh_mask && (expected_len >= 0) && (len != 2 * expected_len)) ||
- (!nsh_mask && (expected_len >= 0) && (len == 2 * expected_len))) {
- return ODP_FIT_ERROR;
+ if (expected_len) {
+ if (nsh_mask) {
+ expected_len *= 2;
+ }
+ if (len != expected_len) {
+ odp_parse_error(&rl, errorp, "NSH %s attribute %"PRIu16" "
+ "should have length %d but actually has "
+ "%"PRIuSIZE,
+ nsh_mask ? "mask" : "key",
+ type, expected_len, len);
+ return ODP_FIT_ERROR;
+ }
}
switch (type) {
@@ -2719,16 +2758,24 @@ odp_nsh_key_from_attr(const struct nlattr *attr, struct ovs_key_nsh *nsh,
}
if (has_md1 && nsh->mdtype != NSH_M_TYPE1 && !nsh_mask) {
+ odp_parse_error(&rl, errorp, "OVS_NSH_KEY_ATTR_MD1 present but "
+ "declared mdtype %"PRIu8" is not %d (NSH_M_TYPE1)",
+ nsh->mdtype, NSH_M_TYPE1);
return ODP_FIT_ERROR;
}
return ODP_FIT_PERFECT;
}
+/* Parses OVS_KEY_ATTR_TUNNEL attribute 'attr' into 'tun' and returns fitness.
+ * If the attribute is a key, 'is_mask' should be false; if it is a mask,
+ * 'is_mask' should be true. If 'errorp' is nonnull and the function returns
+ * ODP_FIT_ERROR, stores a malloc()'d error message in '*errorp'. */
static enum odp_key_fitness
odp_tun_key_from_attr__(const struct nlattr *attr, bool is_mask,
- struct flow_tnl *tun)
+ struct flow_tnl *tun, char **errorp)
{
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
unsigned int left;
const struct nlattr *a;
bool ttl = false;
@@ -2741,6 +2788,9 @@ odp_tun_key_from_attr__(const struct nlattr *attr, bool is_mask,
OVS_TUNNEL_ATTR_MAX, type);
if (len != expected_len && expected_len >= 0) {
+ odp_parse_error(&rl, errorp, "tunnel key attribute %"PRIu16" "
+ "should have length %d but actually has %"PRIuSIZE,
+ type, expected_len, len);
return ODP_FIT_ERROR;
}
@@ -2790,6 +2840,7 @@ odp_tun_key_from_attr__(const struct nlattr *attr, bool is_mask,
struct nlattr *ext[ARRAY_SIZE(vxlan_opts_policy)];
if (!nl_parse_nested(a, vxlan_opts_policy, ext, ARRAY_SIZE(ext))) {
+ odp_parse_error(&rl, errorp, "error parsing VXLAN options");
return ODP_FIT_ERROR;
}
@@ -2829,6 +2880,7 @@ odp_tun_key_from_attr__(const struct nlattr *attr, bool is_mask,
}
if (!ttl) {
+ odp_parse_error(&rl, errorp, "tunnel options missing TTL");
return ODP_FIT_ERROR;
}
if (unknown) {
@@ -2837,11 +2889,19 @@ odp_tun_key_from_attr__(const struct nlattr *attr, bool is_mask,
return ODP_FIT_PERFECT;
}
+/* Parses OVS_KEY_ATTR_TUNNEL key attribute 'attr' into 'tun' and returns
+ * fitness. The attribute should be a key (not a mask). If 'errorp' is
+ * nonnull, stores NULL into '*errorp' on success, otherwise a malloc()'d error
+ * message. */
enum odp_key_fitness
-odp_tun_key_from_attr(const struct nlattr *attr, struct flow_tnl *tun)
+odp_tun_key_from_attr(const struct nlattr *attr, struct flow_tnl *tun,
+ char **errorp)
{
+ if (errorp) {
+ *errorp = NULL;
+ }
memset(tun, 0, sizeof *tun);
- return odp_tun_key_from_attr__(attr, false, tun);
+ return odp_tun_key_from_attr__(attr, false, tun, errorp);
}
static void
@@ -5646,12 +5706,12 @@ parse_odp_key_mask_attr(struct parse_odp_context *context, const char *s,
return -EINVAL;
}
-/* Parses the string representation of a datapath flow key, in the
- * format output by odp_flow_key_format(). Returns 0 if successful,
- * otherwise a positive errno value. On success, the flow key is
- * appended to 'key' as a series of Netlink attributes. On failure, no
- * data is appended to 'key'. Either way, 'key''s data might be
- * reallocated.
+/* Parses the string representation of a datapath flow key, in the format
+ * output by odp_flow_key_format(). Returns 0 if successful, otherwise a
+ * positive errno value. On success, stores NULL into '*errorp' and the flow
+ * key is appended to 'key' as a series of Netlink attributes. On failure,
+ * stores a malloc()'d error message in '*errorp' without changing the data in
+ * 'key'. Either way, 'key''s data might be reallocated.
*
* If 'port_names' is nonnull, it points to an simap that maps from a port name
* to a port number. (Port names may be used instead of port numbers in
@@ -5662,8 +5722,11 @@ parse_odp_key_mask_attr(struct parse_odp_context *context, const char *s,
* have duplicated keys. odp_flow_key_to_flow() will detect those errors. */
int
odp_flow_from_string(const char *s, const struct simap *port_names,
- struct ofpbuf *key, struct ofpbuf *mask)
+ struct ofpbuf *key, struct ofpbuf *mask,
+ char **errorp)
{
+ *errorp = NULL;
+
const size_t old_size = key->size;
struct parse_odp_context context = (struct parse_odp_context) {
.port_names = port_names,
@@ -5680,6 +5743,7 @@ odp_flow_from_string(const char *s, const struct simap *port_names,
ovs_u128 ufid;
retval = odp_ufid_from_string(s, &ufid);
if (retval < 0) {
+ *errorp = xasprintf("syntax error at %s", s);
key->size = old_size;
return -retval;
} else if (retval > 0) {
@@ -5689,6 +5753,7 @@ odp_flow_from_string(const char *s, const struct simap *port_names,
retval = parse_odp_key_mask_attr(&context, s, key, mask);
if (retval < 0) {
+ *errorp = xasprintf("syntax error at %s", s);
key->size = old_size;
return -retval;
}
@@ -6133,7 +6198,7 @@ odp_key_to_dp_packet(const struct nlattr *key, size_t key_len,
case OVS_KEY_ATTR_TUNNEL: {
enum odp_key_fitness res;
- res = odp_tun_key_from_attr(nla, &md->tunnel);
+ res = odp_tun_key_from_attr(nla, &md->tunnel, NULL);
if (res == ODP_FIT_ERROR) {
memset(&md->tunnel, 0, sizeof md->tunnel);
}
@@ -6240,10 +6305,22 @@ odp_to_ovs_frag(uint8_t odp_frag, bool is_mask)
: FLOW_NW_FRAG_ANY | FLOW_NW_FRAG_LATER;
}
+/* Parses the attributes in the 'key_len' bytes of 'key' into 'attrs', which
+ * must have OVS_KEY_ATTR_MAX + 1 elements. Stores each attribute in 'key'
+ * into the corresponding element of 'attrs'.
+ *
+ * Stores a bitmask of the attributes' indexes found in 'key' into
+ * '*present_attrsp'.
+ *
+ * If an attribute beyond OVS_KEY_ATTR_MAX is found, stores its attribute type
+ * (or one of them, if more than one) into '*out_of_range_attrp', otherwise 0.
+ *
+ * If 'errorp' is nonnull and the function returns false, stores a malloc()'d
+ * error message in '*errorp'. */
static bool
parse_flow_nlattrs(const struct nlattr *key, size_t key_len,
const struct nlattr *attrs[], uint64_t *present_attrsp,
- int *out_of_range_attrp)
+ int *out_of_range_attrp, char **errorp)
{
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(10, 10);
const struct nlattr *nla;
@@ -6262,10 +6339,11 @@ parse_flow_nlattrs(const struct nlattr *key, size_t key_len,
if (len != expected_len && expected_len >= 0) {
char namebuf[OVS_KEY_ATTR_BUFSIZE];
- VLOG_ERR_RL(&rl, "attribute %s has length %"PRIuSIZE" but should have "
- "length %d", ovs_key_attr_to_string(type, namebuf,
- sizeof namebuf),
- len, expected_len);
+ odp_parse_error(&rl, errorp, "attribute %s has length %"PRIuSIZE" "
+ "but should have length %d",
+ ovs_key_attr_to_string(type, namebuf,
+ sizeof namebuf),
+ len, expected_len);
return false;
}
@@ -6275,9 +6353,10 @@ parse_flow_nlattrs(const struct nlattr *key, size_t key_len,
if (present_attrs & (UINT64_C(1) << type)) {
char namebuf[OVS_KEY_ATTR_BUFSIZE];
- VLOG_ERR_RL(&rl, "duplicate %s attribute in flow key",
- ovs_key_attr_to_string(type,
- namebuf, sizeof namebuf));
+ odp_parse_error(&rl, errorp,
+ "duplicate %s attribute in flow key",
+ ovs_key_attr_to_string(type, namebuf,
+ sizeof namebuf));
return false;
}
@@ -6286,7 +6365,7 @@ parse_flow_nlattrs(const struct nlattr *key, size_t key_len,
}
}
if (left) {
- VLOG_ERR_RL(&rl, "trailing garbage in flow key");
+ odp_parse_error(&rl, errorp, "trailing garbage in flow key");
return false;
}
@@ -6321,10 +6400,22 @@ check_expectations(uint64_t present_attrs, int out_of_range_attr,
return ODP_FIT_PERFECT;
}
+/* Initializes 'flow->dl_type' based on the attributes in 'attrs', in which the
+ * attributes in the bit-mask 'present_attrs' are present. Returns true if
+ * successful, false on failure.
+ *
+ * Sets 1-bits in '*expected_attrs' for the attributes in 'attrs' that were
+ * consulted. 'flow' is assumed to be a flow key unless 'src_flow' is nonnull,
+ * in which case 'flow' is a flow mask and 'src_flow' is its corresponding
+ * previously parsed flow key.
+ *
+ * If 'errorp' is nonnull and the function returns false, stores a malloc()'d
+ * error message in '*errorp'. */
static bool
parse_ethertype(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
uint64_t present_attrs, uint64_t *expected_attrs,
- struct flow *flow, const struct flow *src_flow)
+ struct flow *flow, const struct flow *src_flow,
+ char **errorp)
{
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
bool is_mask = flow != src_flow;
@@ -6332,12 +6423,16 @@ parse_ethertype(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ETHERTYPE)) {
flow->dl_type = nl_attr_get_be16(attrs[OVS_KEY_ATTR_ETHERTYPE]);
if (!is_mask && ntohs(flow->dl_type) < ETH_TYPE_MIN) {
- VLOG_ERR_RL(&rl, "invalid Ethertype %"PRIu16" in flow key",
- ntohs(flow->dl_type));
+ odp_parse_error(&rl, errorp,
+ "invalid Ethertype %"PRIu16" in flow key",
+ ntohs(flow->dl_type));
return false;
}
if (is_mask && ntohs(src_flow->dl_type) < ETH_TYPE_MIN &&
flow->dl_type != htons(0xffff)) {
+ odp_parse_error(&rl, errorp, "can't bitwise match non-Ethernet II "
+ "\"Ethertype\" %#"PRIx16" (with mask %#"PRIx16")",
+ ntohs(src_flow->dl_type), ntohs(flow->dl_type));
return false;
}
*expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ETHERTYPE;
@@ -6358,19 +6453,37 @@ parse_ethertype(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
flow->dl_type = htons(0xffff);
} else if (ntohs(src_flow->dl_type) < ETH_TYPE_MIN) {
/* See comments in odp_flow_key_from_flow__(). */
- VLOG_ERR_RL(&rl, "mask expected for non-Ethernet II frame");
+ odp_parse_error(&rl, errorp,
+ "mask expected for non-Ethernet II frame");
return false;
}
}
return true;
}
+/* Initializes MPLS, L3, and L4 fields in 'flow' based on the attributes in
+ * 'attrs', in which the attributes in the bit-mask 'present_attrs' are
+ * present. The caller also indicates an out-of-range attribute
+ * 'out_of_range_attr' if one was present when parsing (if so, the fitness
+ * cannot be perfect).
+ *
+ * Sets 1-bits in '*expected_attrs' for the attributes in 'attrs' that were
+ * consulted. 'flow' is assumed to be a flow key unless 'src_flow' is nonnull,
+ * in which case 'flow' is a flow mask and 'src_flow' is its corresponding
+ * previously parsed flow key.
+ *
+ * Returns fitness based on any discrepancies between present and expected
+ * attributes, except that a 'need_check' of false overrides this.
+ *
+ * If 'errorp' is nonnull and the function returns false, stores a malloc()'d
+ * error message in '*errorp'. 'key' and 'key_len' are just used for error
+ * reporting in this case. */
static enum odp_key_fitness
parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
uint64_t present_attrs, int out_of_range_attr,
uint64_t *expected_attrs, struct flow *flow,
const struct nlattr *key, size_t key_len,
- const struct flow *src_flow, bool need_check)
+ const struct flow *src_flow, bool need_check, char **errorp)
{
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
bool is_mask = src_flow != flow;
@@ -6389,9 +6502,15 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
int i;
if (!size || size % sizeof(ovs_be32)) {
+ odp_parse_error(&rl, errorp,
+ "MPLS LSEs have invalid length %"PRIuSIZE,
+ size);
return ODP_FIT_ERROR;
}
if (flow->mpls_lse[0] && flow->dl_type != htons(0xffff)) {
+ odp_parse_error(&rl, errorp,
+ "unexpected MPLS Ethertype mask %x"PRIx16,
+ ntohs(flow->dl_type));
return ODP_FIT_ERROR;
}
@@ -6406,6 +6525,8 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
/* BOS may be set only in the innermost label. */
for (i = 0; i < n - 1; i++) {
if (flow->mpls_lse[i] & htonl(MPLS_BOS_MASK)) {
+ odp_parse_error(&rl, errorp,
+ "MPLS BOS set in non-innermost label");
return ODP_FIT_ERROR;
}
}
@@ -6429,8 +6550,11 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
ipv4_key = nl_attr_get(attrs[OVS_KEY_ATTR_IPV4]);
put_ipv4_key(ipv4_key, flow, is_mask);
if (flow->nw_frag > FLOW_NW_FRAG_MASK) {
+ odp_parse_error(&rl, errorp, "OVS_KEY_ATTR_IPV4 has invalid "
+ "nw_frag %#"PRIx8, flow->nw_frag);
return ODP_FIT_ERROR;
}
+
if (is_mask) {
check_start = ipv4_key;
check_len = sizeof *ipv4_key;
@@ -6447,6 +6571,8 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
ipv6_key = nl_attr_get(attrs[OVS_KEY_ATTR_IPV6]);
put_ipv6_key(ipv6_key, flow, is_mask);
if (flow->nw_frag > FLOW_NW_FRAG_MASK) {
+ odp_parse_error(&rl, errorp, "OVS_KEY_ATTR_IPV6 has invalid "
+ "nw_frag %#"PRIx8, flow->nw_frag);
return ODP_FIT_ERROR;
}
if (is_mask) {
@@ -6465,8 +6591,9 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
arp_key = nl_attr_get(attrs[OVS_KEY_ATTR_ARP]);
if (!is_mask && (arp_key->arp_op & htons(0xff00))) {
- VLOG_ERR_RL(&rl, "unsupported ARP opcode %"PRIu16" in flow "
- "key", ntohs(arp_key->arp_op));
+ odp_parse_error(&rl, errorp,
+ "unsupported ARP opcode %"PRIu16" in flow "
+ "key", ntohs(arp_key->arp_op));
return ODP_FIT_ERROR;
}
put_arp_key(arp_key, flow);
@@ -6481,7 +6608,10 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
*expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_NSH;
}
if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_NSH)) {
- odp_nsh_key_from_attr(attrs[OVS_KEY_ATTR_NSH], &flow->nsh, NULL);
+ if (odp_nsh_key_from_attr(attrs[OVS_KEY_ATTR_NSH], &flow->nsh,
+ NULL, errorp) == ODP_FIT_ERROR) {
+ return ODP_FIT_ERROR;
+ }
if (is_mask) {
check_start = nl_attr_get(attrs[OVS_KEY_ATTR_NSH]);
check_len = nl_attr_get_size(attrs[OVS_KEY_ATTR_NSH]);
@@ -6494,6 +6624,10 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
if (check_len > 0) { /* Happens only when 'is_mask'. */
if (!is_all_zeros(check_start, check_len) &&
flow->dl_type != htons(0xffff)) {
+ odp_parse_error(&rl, errorp, "unexpected L3 matching with "
+ "masked Ethertype %#"PRIx16"/%#"PRIx16,
+ ntohs(src_flow->dl_type),
+ ntohs(flow->dl_type));
return ODP_FIT_ERROR;
} else {
*expected_attrs |= UINT64_C(1) << expected_bit;
@@ -6594,6 +6728,12 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
if (!is_all_zeros(nd_key, sizeof *nd_key) &&
(flow->tp_src != htons(0xff) ||
flow->tp_dst != htons(0xff))) {
+ odp_parse_error(&rl, errorp,
+ "ICMP (src,dst) masks should be "
+ "(0xff,0xff) but are actually "
+ "(%#"PRIx16",%#"PRIx16")",
+ ntohs(flow->tp_src),
+ ntohs(flow->tp_dst));
return ODP_FIT_ERROR;
} else {
*expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ND;
@@ -6639,6 +6779,8 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
}
if (is_mask && expected_bit != OVS_KEY_ATTR_UNSPEC) {
if ((flow->tp_src || flow->tp_dst) && flow->nw_proto != 0xff) {
+ odp_parse_error(&rl, errorp, "flow matches on L4 ports but does "
+ "not define an L4 protocol");
return ODP_FIT_ERROR;
} else {
*expected_attrs |= UINT64_C(1) << expected_bit;
@@ -6656,7 +6798,7 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
uint64_t present_attrs, int out_of_range_attr,
uint64_t expected_attrs, struct flow *flow,
const struct nlattr *key, size_t key_len,
- const struct flow *src_flow)
+ const struct flow *src_flow, char **errorp)
{
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
bool is_mask = src_flow != flow;
@@ -6708,9 +6850,9 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
}
return fitness;
} else if (!(flow->vlans[encaps].tci & htons(VLAN_CFI))) {
- VLOG_ERR_RL(&rl, "OVS_KEY_ATTR_VLAN 0x%04"PRIx16" is nonzero "
- "but CFI bit is not set",
- ntohs(flow->vlans[encaps].tci));
+ odp_parse_error(
+ &rl, errorp, "OVS_KEY_ATTR_VLAN 0x%04"PRIx16" is nonzero "
+ "but CFI bit is not set", ntohs(flow->vlans[encaps].tci));
return ODP_FIT_ERROR;
}
} else {
@@ -6721,20 +6863,21 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
/* Now parse the encapsulated attributes. */
if (!parse_flow_nlattrs(nl_attr_get(encap), nl_attr_get_size(encap),
- attrs, &present_attrs, &out_of_range_attr)) {
+ attrs, &present_attrs, &out_of_range_attr,
+ errorp)) {
return ODP_FIT_ERROR;
}
expected_attrs = 0;
if (!parse_ethertype(attrs, present_attrs, &expected_attrs,
- flow, src_flow)) {
+ flow, src_flow, errorp)) {
return ODP_FIT_ERROR;
}
encap_fitness = parse_l2_5_onward(attrs, present_attrs,
out_of_range_attr,
&expected_attrs,
flow, key, key_len,
- src_flow, false);
+ src_flow, false, errorp);
if (encap_fitness != ODP_FIT_PERFECT) {
return encap_fitness;
}
@@ -6747,8 +6890,14 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
static enum odp_key_fitness
odp_flow_key_to_flow__(const struct nlattr *key, size_t key_len,
- struct flow *flow, const struct flow *src_flow)
+ struct flow *flow, const struct flow *src_flow,
+ char **errorp)
{
+ enum odp_key_fitness fitness = ODP_FIT_ERROR;
+ if (errorp) {
+ *errorp = NULL;
+ }
+
const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1];
uint64_t expected_attrs;
uint64_t present_attrs;
@@ -6759,8 +6908,8 @@ odp_flow_key_to_flow__(const struct nlattr *key, size_t key_len,
/* Parse attributes. */
if (!parse_flow_nlattrs(key, key_len, attrs, &present_attrs,
- &out_of_range_attr)) {
- return ODP_FIT_ERROR;
+ &out_of_range_attr, errorp)) {
+ goto exit;
}
expected_attrs = 0;
@@ -6829,9 +6978,9 @@ odp_flow_key_to_flow__(const struct nlattr *key, size_t key_len,
enum odp_key_fitness res;
res = odp_tun_key_from_attr__(attrs[OVS_KEY_ATTR_TUNNEL], is_mask,
- &flow->tunnel);
+ &flow->tunnel, errorp);
if (res == ODP_FIT_ERROR) {
- return ODP_FIT_ERROR;
+ goto exit;
} else if (res == ODP_FIT_PERFECT) {
expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_TUNNEL;
}
@@ -6878,28 +7027,56 @@ odp_flow_key_to_flow__(const struct nlattr *key, size_t key_len,
/* Get Ethertype or 802.1Q TPID or FLOW_DL_TYPE_NONE. */
if (!parse_ethertype(attrs, present_attrs, &expected_attrs, flow,
- src_flow)) {
- return ODP_FIT_ERROR;
+ src_flow, errorp)) {
+ goto exit;
}
if (is_mask
? (src_flow->vlans[0].tci & htons(VLAN_CFI)) != 0
: eth_type_vlan(src_flow->dl_type)) {
- return parse_8021q_onward(attrs, present_attrs, out_of_range_attr,
- expected_attrs, flow, key, key_len, src_flow);
+ fitness = parse_8021q_onward(attrs, present_attrs, out_of_range_attr,
+ expected_attrs, flow, key, key_len,
+ src_flow, errorp);
+ } else {
+ if (is_mask) {
+ /* A missing VLAN mask means exact match on vlan_tci 0 (== no
+ * VLAN). */
+ flow->vlans[0].tpid = htons(0xffff);
+ flow->vlans[0].tci = htons(0xffff);
+ if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_VLAN)) {
+ flow->vlans[0].tci = nl_attr_get_be16(
+ attrs[OVS_KEY_ATTR_VLAN]);
+ expected_attrs |= (UINT64_C(1) << OVS_KEY_ATTR_VLAN);
+ }
+ }
+ fitness = parse_l2_5_onward(attrs, present_attrs, out_of_range_attr,
+ &expected_attrs, flow, key, key_len,
+ src_flow, true, errorp);
}
- if (is_mask) {
- /* A missing VLAN mask means exact match on vlan_tci 0 (== no VLAN). */
- flow->vlans[0].tpid = htons(0xffff);
- flow->vlans[0].tci = htons(0xffff);
- if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_VLAN)) {
- flow->vlans[0].tci = nl_attr_get_be16(attrs[OVS_KEY_ATTR_VLAN]);
- expected_attrs |= (UINT64_C(1) << OVS_KEY_ATTR_VLAN);
+
+exit:;
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+ if (fitness == ODP_FIT_ERROR && (errorp || !VLOG_DROP_WARN(&rl))) {
+ struct ds s = DS_EMPTY_INITIALIZER;
+ if (is_mask) {
+ ds_put_cstr(&s, "the flow mask in error is: ");
+ odp_flow_key_format(key, key_len, &s);
+ ds_put_cstr(&s, ", for the following flow key: ");
+ flow_format(&s, src_flow, NULL);
+ } else {
+ ds_put_cstr(&s, "the flow key in error is: ");
+ odp_flow_key_format(key, key_len, &s);
}
+ if (errorp) {
+ char *old_error = *errorp;
+ *errorp = xasprintf("%s; %s", old_error, ds_cstr(&s));
+ free(old_error);
+ } else {
+ VLOG_WARN("%s", ds_cstr(&s));
+ }
+ ds_destroy(&s);
}
- return parse_l2_5_onward(attrs, present_attrs, out_of_range_attr,
- &expected_attrs, flow, key, key_len,
- src_flow, true);
+ return fitness;
}
/* Converts the 'key_len' bytes of OVS_KEY_ATTR_* attributes in 'key' to a flow
@@ -6916,28 +7093,40 @@ odp_flow_key_to_flow__(const struct nlattr *key, size_t key_len,
* by looking at the attributes for lower-level protocols, e.g. if the network
* protocol in OVS_KEY_ATTR_IPV4 or OVS_KEY_ATTR_IPV6 is IPPROTO_TCP then we
* know that a OVS_KEY_ATTR_TCP attribute must appear and that otherwise it
- * must be absent. */
+ * must be absent.
+ *
+ * If 'errorp' is nonnull, this function uses it for detailed error reports: if
+ * the return value is ODP_FIT_ERROR, it stores a malloc()'d error string in
+ * '*errorp', otherwise NULL. */
enum odp_key_fitness
odp_flow_key_to_flow(const struct nlattr *key, size_t key_len,
- struct flow *flow)
+ struct flow *flow, char **errorp)
{
- return odp_flow_key_to_flow__(key, key_len, flow, flow);
+ return odp_flow_key_to_flow__(key, key_len, flow, flow, errorp);
}
/* Converts the 'mask_key_len' bytes of OVS_KEY_ATTR_* attributes in 'mask_key'
* to a mask structure in 'mask'. 'flow' must be a previously translated flow
* corresponding to 'mask' and similarly flow_key/flow_key_len must be the
* attributes from that flow. Returns an ODP_FIT_* value that indicates how
- * well 'key' fits our expectations for what a flow key should contain. */
+ * well 'key' fits our expectations for what a flow key should contain.
+ *
+ * If 'errorp' is nonnull, this function uses it for detailed error reports: if
+ * the return value is ODP_FIT_ERROR, it stores a malloc()'d error string in
+ * '*errorp', otherwise NULL. */
enum odp_key_fitness
odp_flow_key_to_mask(const struct nlattr *mask_key, size_t mask_key_len,
- struct flow_wildcards *mask, const struct flow *src_flow)
+ struct flow_wildcards *mask, const struct flow *src_flow,
+ char **errorp)
{
if (mask_key_len) {
return odp_flow_key_to_flow__(mask_key, mask_key_len,
- &mask->masks, src_flow);
-
+ &mask->masks, src_flow, errorp);
} else {
+ if (errorp) {
+ *errorp = NULL;
+ }
+
/* A missing mask means that the flow should be exact matched.
* Generate an appropriate exact wildcard for the flow. */
flow_wildcards_init_for_packet(mask, src_flow);
@@ -6956,7 +7145,7 @@ parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len,
{
enum odp_key_fitness fitness;
- fitness = odp_flow_key_to_flow(key, key_len, &match->flow);
+ fitness = odp_flow_key_to_flow(key, key_len, &match->flow, NULL);
if (fitness) {
/* This should not happen: it indicates that
* odp_flow_key_from_flow() and odp_flow_key_to_flow() disagree on
@@ -6976,7 +7165,8 @@ parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len,
return EINVAL;
}
- fitness = odp_flow_key_to_mask(mask, mask_len, &match->wc, &match->flow);
+ fitness = odp_flow_key_to_mask(mask, mask_len, &match->wc, &match->flow,
+ NULL);
if (fitness) {
/* This should not happen: it indicates that
* odp_flow_key_from_mask() and odp_flow_key_to_mask()
diff --git a/lib/odp-util.h b/lib/odp-util.h
index aa8e6efe3..a03e82532 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -157,10 +157,11 @@ struct odputil_keybuf {
};
enum odp_key_fitness odp_tun_key_from_attr(const struct nlattr *,
- struct flow_tnl *);
+ struct flow_tnl *, char **errorp);
enum odp_key_fitness odp_nsh_key_from_attr(const struct nlattr *,
struct ovs_key_nsh *,
- struct ovs_key_nsh *);
+ struct ovs_key_nsh *,
+ char **errorp);
enum odp_key_fitness odp_nsh_hdr_from_attr(const struct nlattr *,
struct nsh_hdr *, size_t);
@@ -172,9 +173,8 @@ void odp_flow_format(const struct nlattr *key, size_t key_len,
const struct hmap *portno_names, struct ds *,
bool verbose);
void odp_flow_key_format(const struct nlattr *, size_t, struct ds *);
-int odp_flow_from_string(const char *s,
- const struct simap *port_names,
- struct ofpbuf *, struct ofpbuf *);
+int odp_flow_from_string(const char *s, const struct simap *port_names,
+ struct ofpbuf *, struct ofpbuf *, char **errorp);
/* ODP_SUPPORT_FIELD(TYPE, FIELD_NAME, FIELD_DESCRIPTION)
*
@@ -261,11 +261,12 @@ enum odp_key_fitness {
ODP_FIT_ERROR, /* The key was invalid. */
};
enum odp_key_fitness odp_flow_key_to_flow(const struct nlattr *, size_t,
- struct flow *);
+ struct flow *, char **errorp);
enum odp_key_fitness odp_flow_key_to_mask(const struct nlattr *mask_key,
size_t mask_key_len,
struct flow_wildcards *mask,
- const struct flow *flow);
+ const struct flow *flow,
+ char **errorp);
int parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len,
const struct nlattr *mask, size_t mask_len,
struct match *match);
diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index bc4ffee62..22cbdae7a 100644
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -988,7 +988,7 @@ sflow_read_set_action(const struct nlattr *attr,
/* Do not handle multi-encap for now. */
sflow_actions->tunnel_err = true;
} else {
- if (odp_tun_key_from_attr(attr, &sflow_actions->tunnel)
+ if (odp_tun_key_from_attr(attr, &sflow_actions->tunnel, NULL)
== ODP_FIT_ERROR) {
/* Tunnel parsing error. */
sflow_actions->tunnel_err = true;
diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
index 4a981e1e4..8ae8a221a 100644
--- a/ofproto/ofproto-dpif-trace.c
+++ b/ofproto/ofproto-dpif-trace.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2019 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -195,8 +195,7 @@ parse_flow_and_packet(int argc, const char *argv[],
bool *consistent)
{
const struct dpif_backer *backer = NULL;
- const char *error = NULL;
- char *m_err = NULL;
+ char *error = NULL;
struct simap port_names = SIMAP_INITIALIZER(&port_names);
struct dp_packet *packet = NULL;
uint8_t *l7 = NULL;
@@ -219,7 +218,7 @@ parse_flow_and_packet(int argc, const char *argv[],
generate_packet = true;
} else if (!strcmp(arg, "--l7")) {
if (i + 1 >= argc) {
- m_err = xasprintf("Missing argument for option %s", arg);
+ error = xasprintf("Missing argument for option %s", arg);
goto exit;
}
@@ -228,7 +227,7 @@ parse_flow_and_packet(int argc, const char *argv[],
dp_packet_init(&payload, 0);
if (dp_packet_put_hex(&payload, argv[++i], NULL)[0] != '\0') {
dp_packet_uninit(&payload);
- error = "Trailing garbage in packet data";
+ error = xstrdup("Trailing garbage in packet data");
goto exit;
}
free(l7);
@@ -236,14 +235,14 @@ parse_flow_and_packet(int argc, const char *argv[],
l7 = dp_packet_steal_data(&payload);
} else if (!strcmp(arg, "--l7-len")) {
if (i + 1 >= argc) {
- m_err = xasprintf("Missing argument for option %s", arg);
+ error = xasprintf("Missing argument for option %s", arg);
goto exit;
}
free(l7);
l7 = NULL;
l7_len = atoi(argv[++i]);
if (l7_len > 64000) {
- m_err = xasprintf("%s: too much L7 data", argv[i]);
+ error = xasprintf("%s: too much L7 data", argv[i]);
goto exit;
}
} else if (consistent
@@ -252,7 +251,7 @@ parse_flow_and_packet(int argc, const char *argv[],
*consistent = true;
} else if (!strcmp(arg, "--ct-next")) {
if (i + 1 >= argc) {
- m_err = xasprintf("Missing argument for option %s", arg);
+ error = xasprintf("Missing argument for option %s", arg);
goto exit;
}
@@ -260,15 +259,15 @@ parse_flow_and_packet(int argc, const char *argv[],
struct ds ds = DS_EMPTY_INITIALIZER;
if (!parse_ct_state(argv[++i], 0, &ct_state, &ds)
|| !validate_ct_state(ct_state, &ds)) {
- m_err = ds_steal_cstr(&ds);
+ error = ds_steal_cstr(&ds);
goto exit;
}
oftrace_push_ct_state(next_ct_states, ct_state);
} else if (arg[0] == '-') {
- m_err = xasprintf("%s: unknown option", arg);
+ error = xasprintf("%s: unknown option", arg);
goto exit;
} else if (n_args >= ARRAY_SIZE(args)) {
- m_err = xstrdup("too many arguments");
+ error = xstrdup("too many arguments");
goto exit;
} else {
args[n_args++] = arg;
@@ -293,14 +292,14 @@ parse_flow_and_packet(int argc, const char *argv[],
* If there is a packet, we strip it off.
*/
if (!generate_packet && n_args > 1) {
- error = eth_from_hex(args[n_args - 1], &packet);
- if (!error) {
+ const char *const_error = eth_from_hex(args[n_args - 1], &packet);
+ if (!const_error) {
n_args--;
} else if (n_args > 2) {
/* The 3-argument form must end in a hex string. */
+ error = xstrdup(const_error);
goto exit;
}
- error = NULL;
}
/* We stripped off the packet if there was one, so 'args' now has one of
@@ -331,7 +330,7 @@ parse_flow_and_packet(int argc, const char *argv[],
backer = node->data;
}
} else {
- error = "Syntax error";
+ error = xstrdup("Syntax error");
goto exit;
}
if (backer && backer->dpif) {
@@ -348,21 +347,20 @@ parse_flow_and_packet(int argc, const char *argv[],
* returns 0, the flow is a odp_flow. If function
* parse_ofp_exact_flow() returns NULL, the flow is a br_flow. */
if (!odp_flow_from_string(args[n_args - 1], &port_names,
- &odp_key, &odp_mask)) {
+ &odp_key, &odp_mask, &error)) {
if (!backer) {
- error = "Cannot find the datapath";
+ error = xstrdup("Cannot find the datapath");
goto exit;
}
- if (odp_flow_key_to_flow(odp_key.data, odp_key.size, flow) == ODP_FIT_ERROR) {
- error = "Failed to parse datapath flow key";
+ if (odp_flow_key_to_flow(odp_key.data, odp_key.size, flow, &error)
+ == ODP_FIT_ERROR) {
goto exit;
}
*ofprotop = xlate_lookup_ofproto(backer, flow,
- &flow->in_port.ofp_port);
+ &flow->in_port.ofp_port, &error);
if (*ofprotop == NULL) {
- error = "Invalid datapath flow";
goto exit;
}
@@ -375,27 +373,26 @@ parse_flow_and_packet(int argc, const char *argv[],
* in OpenFlow format, which is what we use here. */
if (flow->tunnel.flags & FLOW_TNL_F_UDPIF) {
struct flow_tnl tnl;
- int err;
-
memcpy(&tnl, &flow->tunnel, sizeof tnl);
- err = tun_metadata_from_geneve_udpif(flow->tunnel.metadata.tab,
- &tnl, &tnl, &flow->tunnel);
+ int err = tun_metadata_from_geneve_udpif(
+ flow->tunnel.metadata.tab, &tnl, &tnl, &flow->tunnel);
if (err) {
- error = "Failed to parse Geneve options";
+ error = xstrdup("Failed to parse Geneve options");
goto exit;
}
}
+ } else if (n_args != 2) {
+ char *s = error;
+ error = xasprintf("%s (or the bridge name was omitted)", s);
+ free(s);
+ goto exit;
} else {
- char *err;
-
- if (n_args != 2) {
- error = "Must specify bridge name";
- goto exit;
- }
+ free(error);
+ error = NULL;
*ofprotop = ofproto_dpif_lookup_by_name(args[0]);
if (!*ofprotop) {
- m_err = xasprintf("%s: unknown bridge", args[0]);
+ error = xasprintf("%s: unknown bridge", args[0]);
goto exit;
}
@@ -405,12 +402,12 @@ parse_flow_and_packet(int argc, const char *argv[],
ofputil_port_map_put(&map, ofport->ofp_port,
netdev_get_name(ofport->netdev));
}
- err = parse_ofp_exact_flow(flow, NULL,
- ofproto_get_tun_tab(&(*ofprotop)->up),
- args[n_args - 1], &map);
+ char *err = parse_ofp_exact_flow(flow, NULL,
+ ofproto_get_tun_tab(&(*ofprotop)->up),
+ args[n_args - 1], &map);
ofputil_port_map_destroy(&map);
if (err) {
- m_err = xasprintf("Bad openflow flow syntax: %s", err);
+ error = xasprintf("Bad openflow flow syntax: %s", err);
free(err);
goto exit;
}
@@ -428,10 +425,7 @@ parse_flow_and_packet(int argc, const char *argv[],
}
exit:
- if (error && !m_err) {
- m_err = xstrdup(error);
- }
- if (m_err) {
+ if (error) {
dp_packet_delete(packet);
packet = NULL;
}
@@ -440,7 +434,7 @@ exit:
ofpbuf_uninit(&odp_mask);
simap_destroy(&port_names);
free(l7);
- return m_err;
+ return error;
}
static void
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index dc3082477..a19aa9576 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -796,7 +796,7 @@ recv_upcalls(struct handler *handler)
}
upcall->fitness = odp_flow_key_to_flow(dupcall->key, dupcall->key_len,
- flow);
+ flow, NULL);
if (upcall->fitness == ODP_FIT_ERROR) {
goto free_dupcall;
}
@@ -1437,7 +1437,8 @@ process_upcall(struct udpif *udpif, struct upcall *upcall,
memset(&ipfix_actions, 0, sizeof ipfix_actions);
if (upcall->out_tun_key) {
- odp_tun_key_from_attr(upcall->out_tun_key, &output_tunnel_key);
+ odp_tun_key_from_attr(upcall->out_tun_key, &output_tunnel_key,
+ NULL);
}
actions_len = dpif_read_actions(udpif, upcall, flow,
@@ -2093,7 +2094,7 @@ xlate_key(struct udpif *udpif, const struct nlattr *key, unsigned int len,
struct xlate_in xin;
int error;
- fitness = odp_flow_key_to_flow(key, len, &ctx->flow);
+ fitness = odp_flow_key_to_flow(key, len, &ctx->flow, NULL);
if (fitness == ODP_FIT_ERROR) {
return EINVAL;
}
@@ -2186,7 +2187,8 @@ revalidate_ukey__(struct udpif *udpif, const struct udpif_key *ukey,
struct ofproto_dpif *ofproto;
ofp_port_t ofp_in_port;
- ofproto = xlate_lookup_ofproto(udpif->backer, &ctx.flow, &ofp_in_port);
+ ofproto = xlate_lookup_ofproto(udpif->backer, &ctx.flow, &ofp_in_port,
+ NULL);
ofpbuf_clear(odp_actions);
@@ -2199,7 +2201,8 @@ revalidate_ukey__(struct udpif *udpif, const struct udpif_key *ukey,
ofproto->up.slowpath_meter_id, &ofproto->uuid);
}
- if (odp_flow_key_to_mask(ukey->mask, ukey->mask_len, &dp_mask, &ctx.flow)
+ if (odp_flow_key_to_mask(ukey->mask, ukey->mask_len, &dp_mask, &ctx.flow,
+ NULL)
== ODP_FIT_ERROR) {
goto exit;
}
@@ -2523,7 +2526,7 @@ ukey_to_flow_netdev(struct udpif *udpif, struct udpif_key *ukey)
netdev_close(ukey->in_netdev);
ukey->in_netdev = NULL;
}
- res = odp_tun_key_from_attr(k, &tnl);
+ res = odp_tun_key_from_attr(k, &tnl, NULL);
if (res != ODP_FIT_ERROR) {
ukey->in_netdev = flow_get_tunnel_netdev(&tnl);
break;
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 81b72be37..c4014d71b 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1482,8 +1482,10 @@ xlate_ofport_remove(struct ofport_dpif *ofport)
}
static struct ofproto_dpif *
-xlate_lookup_ofproto_(const struct dpif_backer *backer, const struct flow *flow,
- ofp_port_t *ofp_in_port, const struct xport **xportp)
+xlate_lookup_ofproto_(const struct dpif_backer *backer,
+ const struct flow *flow,
+ ofp_port_t *ofp_in_port, const struct xport **xportp,
+ char **errorp)
{
struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
const struct xport *xport;
@@ -1495,6 +1497,10 @@ xlate_lookup_ofproto_(const struct dpif_backer *backer, const struct flow *flow,
recirc_id_node = recirc_id_node_find(flow->recirc_id);
if (OVS_UNLIKELY(!recirc_id_node)) {
+ if (errorp) {
+ *errorp = xasprintf("no recirculation data for recirc_id "
+ "%"PRIu32, flow->recirc_id);
+ }
return NULL;
}
@@ -1514,10 +1520,19 @@ xlate_lookup_ofproto_(const struct dpif_backer *backer, const struct flow *flow,
? tnl_port_receive(flow)
: odp_port_to_ofport(backer, flow->in_port.odp_port));
if (OVS_UNLIKELY(!xport)) {
+ if (errorp) {
+ *errorp = (tnl_port_should_receive(flow)
+ ? xstrdup("no OpenFlow tunnel port for this packet")
+ : xasprintf("no OpenFlow tunnel port for datapath "
+ "port %"PRIu32, flow->in_port.odp_port));
+ }
return NULL;
}
out:
+ if (errorp) {
+ *errorp = NULL;
+ }
*xportp = xport;
if (ofp_in_port) {
*ofp_in_port = xport->ofp_port;
@@ -1529,11 +1544,11 @@ out:
* returns the corresponding struct ofproto_dpif and OpenFlow port number. */
struct ofproto_dpif *
xlate_lookup_ofproto(const struct dpif_backer *backer, const struct flow *flow,
- ofp_port_t *ofp_in_port)
+ ofp_port_t *ofp_in_port, char **errorp)
{
const struct xport *xport;
- return xlate_lookup_ofproto_(backer, flow, ofp_in_port, &xport);
+ return xlate_lookup_ofproto_(backer, flow, ofp_in_port, &xport, errorp);
}
/* Given a datapath and flow metadata ('backer', and 'flow' respectively),
@@ -1554,7 +1569,7 @@ xlate_lookup(const struct dpif_backer *backer, const struct flow *flow,
struct ofproto_dpif *ofproto;
const struct xport *xport;
- ofproto = xlate_lookup_ofproto_(backer, flow, ofp_in_port, &xport);
+ ofproto = xlate_lookup_ofproto_(backer, flow, ofp_in_port, &xport, NULL);
if (!ofproto) {
return ENODEV;
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 0a5a52887..5a51d7b30 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -199,7 +199,8 @@ void xlate_ofport_remove(struct ofport_dpif *);
struct ofproto_dpif * xlate_lookup_ofproto(const struct dpif_backer *,
const struct flow *,
- ofp_port_t *ofp_in_port);
+ ofp_port_t *ofp_in_port,
+ char **errorp);
int xlate_lookup(const struct dpif_backer *, const struct flow *,
struct ofproto_dpif **, struct dpif_ipfix **,
struct dpif_sflow **, struct netflow **,
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 50c959bbf..21dd54bab 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5695,8 +5695,10 @@ ofproto_unixctl_dpif_dump_flows(struct unixctl_conn *conn,
while (dpif_flow_dump_next(flow_dump_thread, &f, 1)) {
struct flow flow;
- if (odp_flow_key_to_flow(f.key, f.key_len, &flow) == ODP_FIT_ERROR
- || xlate_lookup_ofproto(ofproto->backer, &flow, NULL) != ofproto) {
+ if ((odp_flow_key_to_flow(f.key, f.key_len, &flow, NULL)
+ == ODP_FIT_ERROR)
+ || (xlate_lookup_ofproto(ofproto->backer, &flow, NULL, NULL)
+ != ofproto)) {
continue;
}
diff --git a/tests/odp.at b/tests/odp.at
index 558c0c6a0..f92f989ca 100644
--- a/tests/odp.at
+++ b/tests/odp.at
@@ -397,6 +397,6 @@ AT_DATA([odp-in.txt], [dnl
encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap()))))))))))))))))))))))))))))))))
])
AT_CHECK_UNQUOTED([ovstest test-odp parse-keys < odp-in.txt], [0], [dnl
-odp_flow_from_string: error
+odp_flow_from_string: error (syntax error at encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap())))))))))))))))))))))))))))))))))
])
AT_CLEANUP
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index ded2ef013..03e1fab05 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5707,7 +5707,7 @@ m4_foreach(
[AT_CHECK([ovs-appctl ofproto/trace "$br_flow" option],
[2], [], [stderr])
AT_CHECK([tail -2 stderr], [0], [dnl
-Must specify bridge name
+syntax error at in_port=1,dl_src=50:54:00:00:00:01,dl_dst=50:54:00:00:00:02 (or the bridge name was omitted)
ovs-appctl: ovs-vswitchd: server returned an error
])])
diff --git a/tests/test-odp.c b/tests/test-odp.c
index f61846422..09fec706a 100644
--- a/tests/test-odp.c
+++ b/tests/test-odp.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2011, 2012, 2013, 2014 Nicira, Inc.
+ * Copyright (c) 2011, 2012, 2013, 2014, 2019 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -46,10 +46,12 @@ parse_keys(bool wc_keys)
/* Convert string to OVS DP key. */
ofpbuf_init(&odp_key, 0);
ofpbuf_init(&odp_mask, 0);
+ char *error_s;
error = odp_flow_from_string(ds_cstr(&in), NULL,
- &odp_key, &odp_mask);
+ &odp_key, &odp_mask, &error_s);
if (error) {
- printf("odp_flow_from_string: error\n");
+ printf("odp_flow_from_string: error (%s)\n", error_s);
+ free(error_s);
goto next;
}
@@ -67,7 +69,8 @@ parse_keys(bool wc_keys)
};
/* Convert odp_key to flow. */
- fitness = odp_flow_key_to_flow(odp_key.data, odp_key.size, &flow);
+ fitness = odp_flow_key_to_flow(odp_key.data, odp_key.size, &flow,
+ &error_s);
switch (fitness) {
case ODP_FIT_PERFECT:
break;
@@ -81,7 +84,8 @@ parse_keys(bool wc_keys)
break;
case ODP_FIT_ERROR:
- printf("odp_flow_key_to_flow: error\n");
+ printf("odp_flow_key_to_flow: error (%s)\n", error_s);
+ free(error_s);
goto next;
}
/* Convert cls_rule back to odp_key. */
@@ -182,8 +186,11 @@ parse_filter(char *filter_parse)
/* Convert string to OVS DP key. */
ofpbuf_init(&odp_key, 0);
ofpbuf_init(&odp_mask, 0);
- if (odp_flow_from_string(ds_cstr(&in), NULL, &odp_key, &odp_mask)) {
- printf("odp_flow_from_string: error\n");
+ char *error_s;
+ if (odp_flow_from_string(ds_cstr(&in), NULL, &odp_key, &odp_mask,
+ &error_s)) {
+ printf("odp_flow_from_string: error (%s)\n", error_s);
+ free(error_s);
goto next;
}
@@ -193,8 +200,9 @@ parse_filter(char *filter_parse)
struct match match, match_filter;
struct minimatch minimatch;
- odp_flow_key_to_flow(odp_key.data, odp_key.size, &flow);
- odp_flow_key_to_mask(odp_mask.data, odp_mask.size, &wc, &flow);
+ odp_flow_key_to_flow(odp_key.data, odp_key.size, &flow, NULL);
+ odp_flow_key_to_mask(odp_mask.data, odp_mask.size, &wc, &flow,
+ NULL);
match_init(&match, &flow, &wc);
match_init(&match_filter, &flow_filter, &wc);
diff --git a/tests/tunnel.at b/tests/tunnel.at
index 55fb1d391..035c54f67 100644
--- a/tests/tunnel.at
+++ b/tests/tunnel.at
@@ -55,7 +55,7 @@ AT_CHECK([tail -1 stdout], [0],
dnl nonexistent tunnel
AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'tunnel(src=5.5.5.5,dst=6.6.6.6,ttl=64,flags()),in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no),tcp(src=8,dst=9)'], [2], [ignore], [dnl
-Invalid datapath flow
+no OpenFlow tunnel port for this packet
ovs-appctl: ovs-vswitchd: server returned an error
])
@@ -314,7 +314,7 @@ set(tunnel(tun_id=0x3,dst=1.1.1.1,ttl=64,flags(df|key))),1
])
AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'tunnel(tun_id=0xf,src=1.1.1.1,dst=2.2.2.2,ttl=64,flags(key)),in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=0,ttl=64,frag=no),tcp(src=8,dst=9)'], [2], [ignore], [dnl
-Invalid datapath flow
+no OpenFlow tunnel port for this packet
ovs-appctl: ovs-vswitchd: server returned an error
])
OVS_VSWITCHD_STOP(["/receive tunnel port not found/d"])
View Lorry Controller Status