The attachment looks missing to me. On Wed, Jan 16, 2019 at 10:29:39PM +0000, Iyengar, Prashanth wrote: > Hi Aaron, > > Apologies for the formatting issues earlier. > Attached the patch to the message here as suggested. > Let me know if this works. Otherwise, will look to submit a pull request. > > Sorry for the inconvenience. > > Thanks, > Prashanth > > -----Original Message----- > From: Aaron Conole <[email protected]> > Sent: Wednesday, January 16, 2019 06:05 > To: Iyengar, Prashanth <[email protected]> > Cc: [email protected]; Gupta, Rahul <[email protected]>; > Katagiri, Junichi <[email protected]>; van der Peet, Tony > <[email protected]> > Subject: Re: [ovs-dev] Fix OpenFlow v1.3.4 Conformance test failures: > 430.500, 430.510 > > "Iyengar, Prashanth" <[email protected]> writes: > > > 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]> > > --- > > There seems to be some formatting issues not just with the code, but also > with this particular patch. If it continues to be an issue, consider either > attaching the patch to the message, or submitting a pull request on github > and notifying the list. > > > 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); > > The spacing here isn't correct. > > > 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, > > Looks like a number of lines got collapsed? > > > 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) > > Likewise, spacing > > > { > > 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, > > Something weird here. > > > 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) { > > > Please reformat as: > > +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 > > Something weird here with the patch... > > > --- 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 > > These trailers aren't appropriate on a public mailing list. > > > 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 > 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
