From d40533fc820c30a461e7eefe4bcfee3f799d0f11 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 14 Dec 2018 18:16:55 -0800 Subject: 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 --- lib/odp-util.c | 342 ++++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 266 insertions(+), 76 deletions(-) (limited to 'lib/odp-util.c') 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() -- cgit v1.2.1