>From fc0a9e1329573692c04438370f85e87125a268b0 Mon Sep 17 00:00:00 2001 From: Prashanth Iyengar <[email protected]> Date: Mon, 14 Jan 2019 12:53:11 -0800 Subject: [PATCH] Fix OpenFlow v1.3.4 Conf test failures: 430.500, 430.510
This commit adds additional verification to nx_pull_header__() in lib/nx-match.c to distinguish between bad match and bad action header conditions and return the appropriate error type/code. Signed-off-by: Prashanth Iyengar <[email protected]> Reviewed-by: Tony van der Peet <[email protected]> Reviewed-by: Rahul Gupta <[email protected]> --- lib/meta-flow.c | 2 +- lib/nx-match.c | 62 +++++++++++++++++++++++++++++++++++++------- lib/nx-match.h | 2 +- tests/ofp-actions.at | 14 ++++++++++ 4 files changed, 68 insertions(+), 12 deletions(-) diff --git a/lib/meta-flow.c b/lib/meta-flow.c index b6d9e92b6..8f9662943 100644 --- a/lib/meta-flow.c +++ b/lib/meta-flow.c @@ -3503,7 +3503,7 @@ mf_vl_mff_nx_pull_entry(struct ofpbuf *b, const struct vl_mff_map *vl_mff_map, const struct mf_field **field, union mf_value *value, union mf_value *mask, uint64_t *tlv_bitmap) { - enum ofperr error = nx_pull_entry(b, vl_mff_map, field, value, mask); + enum ofperr error = nx_pull_entry(b, vl_mff_map, field, value, + mask, true); if (error) { return error; } diff --git a/lib/nx-match.c b/lib/nx-match.c index fd3eac0c0..33b95a2d4 100644 --- a/lib/nx-match.c +++ b/lib/nx-match.c @@ -97,6 +97,7 @@ enum ofp12_oxm_class { /* Functions for extracting raw field values from OXM/NXM headers. */ static uint32_t nxm_vendor(uint64_t header) { return header; } +enum ofperr nxm_validate_action_field_header(uint64_t header); static int nxm_class(uint64_t header) { return header >> 48; } static int nxm_field(uint64_t header) { return (header >> 41) & 0x7f; } static bool nxm_hasmask(uint64_t header) { return (header >> 40) & 1; } @@ -313,7 +314,7 @@ is_cookie_pseudoheader(uint64_t header) static enum ofperr nx_pull_header__(struct ofpbuf *b, bool allow_cookie, const struct vl_mff_map *vl_mff_map, uint64_t *header, - const struct mf_field **field) + const struct mf_field **field, bool is_action) { if (b->size < 4) { goto bad_len; @@ -340,7 +341,16 @@ nx_pull_header__(struct ofpbuf *b, bool allow_cookie, if (!*field && !(allow_cookie && is_cookie_pseudoheader(*header))) { VLOG_DBG_RL(&rl, "OXM header "NXM_HEADER_FMT" is unknown", NXM_HEADER_ARGS(*header)); - return OFPERR_OFPBMC_BAD_FIELD; + if (is_action) { + enum ofperr h_error = nxm_validate_action_field_header(*header); + if (h_error) { + *field = NULL; + return h_error; + } + return OFPERR_OFPBAC_BAD_SET_TYPE; + } else { + return OFPERR_OFPBMC_BAD_FIELD; + } } else if (mf_vl_mff_invalid(*field, vl_mff_map)) { return OFPERR_NXFMFC_INVALID_TLV_FIELD; } @@ -381,7 +391,7 @@ static enum ofperr nx_pull_entry__(struct ofpbuf *b, bool allow_cookie, const struct vl_mff_map *vl_mff_map, uint64_t *header, const struct mf_field **field_, - union mf_value *value, union mf_value *mask) + union mf_value *value, union mf_value *mask, bool + is_action) { const struct mf_field *field; enum ofperr header_error; @@ -390,7 +400,7 @@ nx_pull_entry__(struct ofpbuf *b, bool allow_cookie, int width; header_error = nx_pull_header__(b, allow_cookie, vl_mff_map, header, - &field); + &field, is_action); if (header_error && header_error != OFPERR_OFPBMC_BAD_FIELD) { return header_error; } @@ -442,15 +452,20 @@ nx_pull_entry__(struct ofpbuf *b, bool allow_cookie, * * If a NULL 'mask' is supplied, masked OXM or NXM entries are treated as * errors (with OFPERR_OFPBMC_BAD_MASK). + * + * The "bool is_action" is supplied to differentiate between match and + action + * headers. This is done in order to return appropriate error type and + code for + * bad match or bad action conditions. If set to True, indicates that + the + * OXM or NXM entries belong to an action header. */ enum ofperr nx_pull_entry(struct ofpbuf *b, const struct vl_mff_map *vl_mff_map, const struct mf_field **field, union mf_value *value, - union mf_value *mask) + union mf_value *mask, bool is_action) { uint64_t header; - return nx_pull_entry__(b, false, vl_mff_map, &header, field, value, mask); + return nx_pull_entry__(b, false, vl_mff_map, &header, field, value, + mask, is_action); } /* Attempts to pull an NXM or OXM header from the beginning of 'b'. If @@ -470,7 +485,7 @@ nx_pull_header(struct ofpbuf *b, const struct vl_mff_map *vl_mff_map, enum ofperr error; uint64_t header; - error = nx_pull_header__(b, false, vl_mff_map, &header, field); + error = nx_pull_header__(b, false, vl_mff_map, &header, field, + false); if (masked) { *masked = !error && nxm_hasmask(header); } else if (!error && nxm_hasmask(header)) { @@ -489,7 +504,7 @@ nx_pull_match_entry(struct ofpbuf *b, bool allow_cookie, uint64_t header; error = nx_pull_entry__(b, allow_cookie, vl_mff_map, &header, field, value, - mask); + mask, false); if (error) { return error; } @@ -739,7 +754,7 @@ oxm_pull_field_array(const void *fields_data, size_t fields_len, uint64_t header; error = nx_pull_entry__(&b, false, NULL, &header, &field, &value, - NULL); + NULL, false); if (error) { VLOG_DBG_RL(&rl, "error pulling field array field"); } else if (!field) { @@ -1488,7 +1503,7 @@ nx_match_to_string(const uint8_t *p, unsigned int match_len) uint64_t header; int value_len; - error = nx_pull_entry__(&b, true, NULL, &header, NULL, &value, &mask); + error = nx_pull_entry__(&b, true, NULL, &header, NULL, &value, + &mask, false); if (error) { break; } @@ -2244,6 +2259,33 @@ nxm_field_by_header(uint64_t header) return NULL; } +enum ofperr +nxm_validate_action_field_header(uint64_t header) { + const struct nxm_field_index *nfi; + uint64_t header_no_len; + + nxm_init(); + if (nxm_hasmask(header)) { + header = nxm_make_exact_header(header); + } + + header_no_len = nxm_no_len(header); + + HMAP_FOR_EACH_IN_BUCKET (nfi, header_node, hash_uint64(header_no_len), + &nxm_header_map) { + if (header_no_len == nxm_no_len(nfi->nf.header)) { + if (nxm_length(header) > 0) { + if (nxm_length(header) != nxm_length(nfi->nf.header)) { + return OFPERR_OFPBAC_BAD_SET_LEN; + } + } + } + } + + return 0; +} + static const struct nxm_field * nxm_field_by_name(const char *name, size_t len) { diff --git a/lib/nx-match.h b/lib/nx-match.h index e304072d3..9be40a981 100644 --- a/lib/nx-match.h +++ b/lib/nx-match.h @@ -82,7 +82,7 @@ int oxm_put_field_array(struct ofpbuf *, const struct field_array *, * ID followed by a value and possibly a mask). */ enum ofperr nx_pull_entry(struct ofpbuf *, const struct vl_mff_map *, const struct mf_field **, union mf_value *value, - union mf_value *mask); + union mf_value *mask, bool is_action); enum ofperr nx_pull_header(struct ofpbuf *, const struct vl_mff_map *, const struct mf_field **, bool *masked); void nxm_put_entry_raw(struct ofpbuf *, enum mf_field_id field, diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at index e320a92a8..38c2b5cbd 100644 --- a/tests/ofp-actions.at +++ b/tests/ofp-actions.at @@ -746,6 +746,20 @@ dnl Check the ONF extension form of "copy_field". # actions=move:NXM_OF_IN_PORT[]->NXM_OF_VLAN_TCI[] ffff 0020 4f4e4600 0c80 0000 0010 0000 0000 0000 00000002 00000802 00000000 +dnl Check OpenFlow v1.3.4 Conformance Test: 430.500. +# bad OpenFlow13 actions: OFPBAC_BAD_SET_TYPE & ofp_actions|WARN|bad +action at offset 0 (OFPBAC_BAD_SET_TYPE): +& 00000000 00 19 00 08 80 00 fe 00-00 00 00 10 00 00 00 01 & 00000010 +00 00 00 00 00 00 00 00- +0019 0008 8000fe00 000000100000 000100000000 00000000 + +dnl Check OpenFlow v1.3.4 Conformance Test: 430.510. +# bad OpenFlow13 actions: OFPBAC_BAD_SET_LEN & ofp_actions|WARN|bad +action at offset 0 (OFPBAC_BAD_SET_LEN): +& 00000000 00 19 00 10 80 00 08 07-00 01 02 03 04 05 00 00 & 00000010 +00 00 00 10 00 00 00 01- +0019 0010 80000807 000102030405 000000000010 00000001 + ]) sed '/^[[#&]]/d' < test-data > input.txt sed -n 's/^# //p; /^$/p' < test-data > expout -- 2.17.1 This e-mail message is for the sole use of the intended recipient(s) and may contain confidential and privileged information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply E-mail and destroy all copies of the original message. If you are the intended recipient, please be advised that the content of this message is subject to access, review and disclosure by the sender's E-mail System Administrator. Privacy Policy We are committed to respecting your privacy and keeping you informed about our practices. Read our Privacy Policy: Allied Telesis Privacy Policy<http://www.alliedtelesis.com/legal/privacy> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
