odp-util: Improve log messages and error reporting for Netlink parsing.

As a side effect, this also reduces a lot of log messages' severities from
ERR to WARN.  They just didn't seem like messages that in general reported
anything that would prevent functioning.

Signed-off-by: Ben Pfaff <blp@ovn.org>

1 files changed, 266 insertions, 76 deletions

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()