| Message ID | 20170315230141.32414-3-joe@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
Looks good to me: Acked-by: Jarno Rajahalme <jarno@ovn.org> > On Mar 15, 2017, at 4:01 PM, Joe Stringer <joe@ovn.org> wrote: > > From: Yi-Hung Wei <yihung.wei@gmail.com> > > vl_mff_map is introduced in commit 04f48a68c428 ("ofp-actions: Fix variable > length meta-flow OXMs") to account variable length mf_field, and it is used > to decode variable length mf_field in ofp_action. In this patch, vl_mff_map > is further used to decode the variable length match field as well. > > Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> > Signed-off-by: Joe Stringer <joe@ovn.org> > --- > include/openvswitch/ofp-util.h | 6 ++-- > lib/learning-switch.c | 2 +- > lib/nx-match.c | 46 ++++++++++++++++-------- > lib/nx-match.h | 7 ++-- > lib/ofp-print.c | 4 +-- > lib/ofp-util.c | 80 +++++++++++++++++++++++++++++------------- > ofproto/ofproto.c | 11 +++--- > ovn/controller/pinctrl.c | 2 +- > tests/ofproto.at | 15 +++++--- > utilities/ovs-ofctl.c | 13 +++---- > 10 files changed, 123 insertions(+), 63 deletions(-) > > diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h > index 0c3a10aa4264..e73a942a3e15 100644 > --- a/include/openvswitch/ofp-util.h > +++ b/include/openvswitch/ofp-util.h > @@ -222,7 +222,7 @@ void ofputil_match_to_ofp10_match(const struct match *, struct ofp10_match *); > > /* Work with ofp11_match. */ > enum ofperr ofputil_pull_ofp11_match(struct ofpbuf *, const struct tun_table *, > - struct match *, > + const struct vl_mff_map *, struct match *, > uint16_t *padded_match_len); > enum ofperr ofputil_pull_ofp11_mask(struct ofpbuf *, struct match *, > struct mf_bitmap *bm); > @@ -352,7 +352,7 @@ struct ofputil_flow_stats_request { > > enum ofperr ofputil_decode_flow_stats_request( > struct ofputil_flow_stats_request *, const struct ofp_header *, > - const struct tun_table *); > + const struct tun_table *, const struct vl_mff_map *); > struct ofpbuf *ofputil_encode_flow_stats_request( > const struct ofputil_flow_stats_request *, enum ofputil_protocol); > > @@ -457,6 +457,7 @@ void ofputil_packet_in_destroy(struct ofputil_packet_in *); > > enum ofperr ofputil_decode_packet_in(const struct ofp_header *, bool loose, > const struct tun_table *, > + const struct vl_mff_map *, > struct ofputil_packet_in *, > size_t *total_len, uint32_t *buffer_id, > struct ofpbuf *continuation); > @@ -509,6 +510,7 @@ struct ofpbuf *ofputil_encode_packet_in_private( > enum ofperr ofputil_decode_packet_in_private( > const struct ofp_header *, bool loose, > const struct tun_table *, > + const struct vl_mff_map *, > struct ofputil_packet_in_private *, > size_t *total_len, uint32_t *buffer_id); > > diff --git a/lib/learning-switch.c b/lib/learning-switch.c > index bc757f46dd7a..77155d04fcc0 100644 > --- a/lib/learning-switch.c > +++ b/lib/learning-switch.c > @@ -523,7 +523,7 @@ process_packet_in(struct lswitch *sw, const struct ofp_header *oh) > struct dp_packet pkt; > struct flow flow; > > - error = ofputil_decode_packet_in(oh, true, NULL, &pi, NULL, > + error = ofputil_decode_packet_in(oh, true, NULL, NULL, &pi, NULL, > &buffer_id, NULL); > if (error) { > VLOG_WARN_RL(&rl, "failed to decode packet-in: %s", > diff --git a/lib/nx-match.c b/lib/nx-match.c > index c258869eec80..124cb71eb7c8 100644 > --- a/lib/nx-match.c > +++ b/lib/nx-match.c > @@ -480,13 +480,14 @@ nx_pull_header(struct ofpbuf *b, const struct vl_mff_map *vl_mff_map, > > static enum ofperr > nx_pull_match_entry(struct ofpbuf *b, bool allow_cookie, > + const struct vl_mff_map *vl_mff_map, > const struct mf_field **field, > union mf_value *value, union mf_value *mask) > { > enum ofperr error; > uint64_t header; > > - error = nx_pull_entry__(b, allow_cookie, NULL, &header, field, value, > + error = nx_pull_entry__(b, allow_cookie, vl_mff_map, &header, field, value, > mask); > if (error) { > return error; > @@ -507,7 +508,8 @@ nx_pull_match_entry(struct ofpbuf *b, bool allow_cookie, > static enum ofperr > nx_pull_raw(const uint8_t *p, unsigned int match_len, bool strict, > struct match *match, ovs_be64 *cookie, ovs_be64 *cookie_mask, > - const struct tun_table *tun_table) > + const struct tun_table *tun_table, > + const struct vl_mff_map *vl_mff_map) > { > ovs_assert((cookie != NULL) == (cookie_mask != NULL)); > > @@ -525,7 +527,8 @@ nx_pull_raw(const uint8_t *p, unsigned int match_len, bool strict, > union mf_value mask; > enum ofperr error; > > - error = nx_pull_match_entry(&b, cookie != NULL, &field, &value, &mask); > + error = nx_pull_match_entry(&b, cookie != NULL, vl_mff_map, &field, > + &value, &mask); > if (error) { > if (error == OFPERR_OFPBMC_BAD_FIELD && !strict) { > continue; > @@ -571,7 +574,8 @@ static enum ofperr > nx_pull_match__(struct ofpbuf *b, unsigned int match_len, bool strict, > struct match *match, > ovs_be64 *cookie, ovs_be64 *cookie_mask, > - const struct tun_table *tun_table) > + const struct tun_table *tun_table, > + const struct vl_mff_map *vl_mff_map) > { > uint8_t *p = NULL; > > @@ -586,7 +590,7 @@ nx_pull_match__(struct ofpbuf *b, unsigned int match_len, bool strict, > } > > return nx_pull_raw(p, match_len, strict, match, cookie, cookie_mask, > - tun_table); > + tun_table, vl_mff_map); > } > > /* Parses the nx_match formatted match description in 'b' with length > @@ -594,16 +598,21 @@ nx_pull_match__(struct ofpbuf *b, unsigned int match_len, bool strict, > * are valid pointers, then stores the cookie and mask in them if 'b' contains > * a "NXM_NX_COOKIE*" match. Otherwise, stores 0 in both. > * > + * 'vl_mff_map" is an optional parameter that is used to validate the length > + * of variable length mf_fields in 'match'. If it is not provided, the > + * default mf_fields with maximum length will be used. > + * > * Fails with an error upon encountering an unknown NXM header. > * > * Returns 0 if successful, otherwise an OpenFlow error code. */ > enum ofperr > nx_pull_match(struct ofpbuf *b, unsigned int match_len, struct match *match, > ovs_be64 *cookie, ovs_be64 *cookie_mask, > - const struct tun_table *tun_table) > + const struct tun_table *tun_table, > + const struct vl_mff_map *vl_mff_map) > { > return nx_pull_match__(b, match_len, true, match, cookie, cookie_mask, > - tun_table); > + tun_table, vl_mff_map); > } > > /* Behaves the same as nx_pull_match(), but skips over unknown NXM headers, > @@ -615,12 +624,13 @@ nx_pull_match_loose(struct ofpbuf *b, unsigned int match_len, > const struct tun_table *tun_table) > { > return nx_pull_match__(b, match_len, false, match, cookie, cookie_mask, > - tun_table); > + tun_table, NULL); > } > > static enum ofperr > oxm_pull_match__(struct ofpbuf *b, bool strict, > - const struct tun_table *tun_table, struct match *match) > + const struct tun_table *tun_table, > + const struct vl_mff_map *vl_mff_map, struct match *match) > { > struct ofp11_match_header *omh = b->data; > uint8_t *p; > @@ -648,20 +658,24 @@ oxm_pull_match__(struct ofpbuf *b, bool strict, > } > > return nx_pull_raw(p + sizeof *omh, match_len - sizeof *omh, > - strict, match, NULL, NULL, tun_table); > + strict, match, NULL, NULL, tun_table, vl_mff_map); > } > > /* Parses the oxm formatted match description preceded by a struct > * ofp11_match_header in 'b'. Stores the result in 'match'. > * > + * 'vl_mff_map' is an optional parameter that is used to validate the length > + * of variable length mf_fields in 'match'. If it is not provided, the > + * default mf_fields with maximum length will be used. > + * > * Fails with an error when encountering unknown OXM headers. > * > * Returns 0 if successful, otherwise an OpenFlow error code. */ > enum ofperr > oxm_pull_match(struct ofpbuf *b, const struct tun_table *tun_table, > - struct match *match) > + const struct vl_mff_map *vl_mff_map, struct match *match) > { > - return oxm_pull_match__(b, true, tun_table, match); > + return oxm_pull_match__(b, true, tun_table, vl_mff_map, match); > } > > /* Behaves the same as oxm_pull_match() with one exception. Skips over unknown > @@ -670,7 +684,7 @@ enum ofperr > oxm_pull_match_loose(struct ofpbuf *b, const struct tun_table *tun_table, > struct match *match) > { > - return oxm_pull_match__(b, false, tun_table, match); > + return oxm_pull_match__(b, false, tun_table, NULL, match); > } > > /* Parses the OXM match description in the 'oxm_len' bytes in 'oxm'. Stores > @@ -683,9 +697,11 @@ oxm_pull_match_loose(struct ofpbuf *b, const struct tun_table *tun_table, > */ > enum ofperr > oxm_decode_match(const void *oxm, size_t oxm_len, bool loose, > - const struct tun_table *tun_table, struct match *match) > + const struct tun_table *tun_table, > + const struct vl_mff_map *vl_mff_map, struct match *match) > { > - return nx_pull_raw(oxm, oxm_len, !loose, match, NULL, NULL, tun_table); > + return nx_pull_raw(oxm, oxm_len, !loose, match, NULL, NULL, tun_table, > + vl_mff_map); > } > > /* Verify an array of OXM TLVs treating value of each TLV as a mask, > diff --git a/lib/nx-match.h b/lib/nx-match.h > index e103dd5fa74d..03e2b9db1a64 100644 > --- a/lib/nx-match.h > +++ b/lib/nx-match.h > @@ -52,17 +52,18 @@ char *mf_parse_subfield(struct mf_subfield *, const char *s) > enum ofperr nx_pull_match(struct ofpbuf *, unsigned int match_len, > struct match *, > ovs_be64 *cookie, ovs_be64 *cookie_mask, > - const struct tun_table *); > + const struct tun_table *, const struct vl_mff_map *); > enum ofperr nx_pull_match_loose(struct ofpbuf *, unsigned int match_len, > struct match *, ovs_be64 *cookie, > ovs_be64 *cookie_mask, > const struct tun_table *); > enum ofperr oxm_pull_match(struct ofpbuf *, const struct tun_table *, > - struct match *); > + const struct vl_mff_map *, struct match *); > enum ofperr oxm_pull_match_loose(struct ofpbuf *, const struct tun_table *, > struct match *); > enum ofperr oxm_decode_match(const void *, size_t, bool, > - const struct tun_table *, struct match *); > + const struct tun_table *, > + const struct vl_mff_map *, struct match *); > enum ofperr oxm_pull_field_array(const void *, size_t fields_len, > struct field_array *); > > diff --git a/lib/ofp-print.c b/lib/ofp-print.c > index f7f7df26f5e1..80dbf6e16e86 100644 > --- a/lib/ofp-print.c > +++ b/lib/ofp-print.c > @@ -118,7 +118,7 @@ ofp_print_packet_in(struct ds *string, const struct ofp_header *oh, > size_t total_len; > enum ofperr error; > > - error = ofputil_decode_packet_in_private(oh, true, NULL, > + error = ofputil_decode_packet_in_private(oh, true, NULL, NULL, > &pin, &total_len, &buffer_id); > if (error) { > ofp_print_error(string, error); > @@ -1603,7 +1603,7 @@ ofp_print_flow_stats_request(struct ds *string, const struct ofp_header *oh) > struct ofputil_flow_stats_request fsr; > enum ofperr error; > > - error = ofputil_decode_flow_stats_request(&fsr, oh, NULL); > + error = ofputil_decode_flow_stats_request(&fsr, oh, NULL, NULL); > if (error) { > ofp_print_error(string, error); > return; > diff --git a/lib/ofp-util.c b/lib/ofp-util.c > index d3153370f2e6..c48081fe3e7f 100644 > --- a/lib/ofp-util.c > +++ b/lib/ofp-util.c > @@ -278,6 +278,7 @@ ofputil_match_to_ofp10_match(const struct match *match, > > enum ofperr > ofputil_pull_ofp11_match(struct ofpbuf *buf, const struct tun_table *tun_table, > + const struct vl_mff_map *vl_mff_map, > struct match *match, uint16_t *padded_match_len) > { > struct ofp11_match_header *omh = buf->data; > @@ -307,7 +308,7 @@ ofputil_pull_ofp11_match(struct ofpbuf *buf, const struct tun_table *tun_table, > if (padded_match_len) { > *padded_match_len = ROUND_UP(match_len, 8); > } > - return oxm_pull_match(buf, tun_table, match); > + return oxm_pull_match(buf, tun_table, vl_mff_map, match); > > default: > return OFPERR_OFPBMC_BAD_TYPE; > @@ -1585,7 +1586,8 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm, > > ofm = ofpbuf_pull(&b, sizeof *ofm); > > - error = ofputil_pull_ofp11_match(&b, tun_table, &fm->match, NULL); > + error = ofputil_pull_ofp11_match(&b, tun_table, vl_mff_map, &fm->match, > + NULL); > if (error) { > return error; > } > @@ -1681,7 +1683,7 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm, > nfm = ofpbuf_pull(&b, sizeof *nfm); > error = nx_pull_match(&b, ntohs(nfm->match_len), > &fm->match, &fm->cookie, &fm->cookie_mask, > - tun_table); > + tun_table, vl_mff_map); > if (error) { > return error; > } > @@ -2271,7 +2273,8 @@ ofputil_decode_ofpst10_flow_request(struct ofputil_flow_stats_request *fsr, > static enum ofperr > ofputil_decode_ofpst11_flow_request(struct ofputil_flow_stats_request *fsr, > struct ofpbuf *b, bool aggregate, > - const struct tun_table *tun_table) > + const struct tun_table *tun_table, > + const struct vl_mff_map *vl_mff_map) > { > const struct ofp11_flow_stats_request *ofsr; > enum ofperr error; > @@ -2286,7 +2289,8 @@ ofputil_decode_ofpst11_flow_request(struct ofputil_flow_stats_request *fsr, > fsr->out_group = ntohl(ofsr->out_group); > fsr->cookie = ofsr->cookie; > fsr->cookie_mask = ofsr->cookie_mask; > - error = ofputil_pull_ofp11_match(b, tun_table, &fsr->match, NULL); > + error = ofputil_pull_ofp11_match(b, tun_table, vl_mff_map, &fsr->match, > + NULL); > if (error) { > return error; > } > @@ -2297,14 +2301,16 @@ ofputil_decode_ofpst11_flow_request(struct ofputil_flow_stats_request *fsr, > static enum ofperr > ofputil_decode_nxst_flow_request(struct ofputil_flow_stats_request *fsr, > struct ofpbuf *b, bool aggregate, > - const struct tun_table *tun_table) > + const struct tun_table *tun_table, > + const struct vl_mff_map *vl_mff_map) > { > const struct nx_flow_stats_request *nfsr; > enum ofperr error; > > nfsr = ofpbuf_pull(b, sizeof *nfsr); > error = nx_pull_match(b, ntohs(nfsr->match_len), &fsr->match, > - &fsr->cookie, &fsr->cookie_mask, tun_table); > + &fsr->cookie, &fsr->cookie_mask, tun_table, > + vl_mff_map); > if (error) { > return error; > } > @@ -2714,11 +2720,16 @@ ofputil_pull_queue_get_config_reply(struct ofpbuf *msg, > > /* Converts an OFPST_FLOW, OFPST_AGGREGATE, NXST_FLOW, or NXST_AGGREGATE > * request 'oh', into an abstract flow_stats_request in 'fsr'. Returns 0 if > - * successful, otherwise an OpenFlow error code. */ > + * successful, otherwise an OpenFlow error code. > + * > + * 'vl_mff_map' is an optional parameter that is used to validate the length > + * of variable length mf_fields in 'match'. If it is not provided, the > + * default mf_fields with maximum length will be used. */ > enum ofperr > ofputil_decode_flow_stats_request(struct ofputil_flow_stats_request *fsr, > const struct ofp_header *oh, > - const struct tun_table *tun_table) > + const struct tun_table *tun_table, > + const struct vl_mff_map *vl_mff_map) > { > struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); > enum ofpraw raw = ofpraw_pull_assert(&b); > @@ -2730,16 +2741,20 @@ ofputil_decode_flow_stats_request(struct ofputil_flow_stats_request *fsr, > return ofputil_decode_ofpst10_flow_request(fsr, b.data, true); > > case OFPRAW_OFPST11_FLOW_REQUEST: > - return ofputil_decode_ofpst11_flow_request(fsr, &b, false, tun_table); > + return ofputil_decode_ofpst11_flow_request(fsr, &b, false, tun_table, > + vl_mff_map); > > case OFPRAW_OFPST11_AGGREGATE_REQUEST: > - return ofputil_decode_ofpst11_flow_request(fsr, &b, true, tun_table); > + return ofputil_decode_ofpst11_flow_request(fsr, &b, true, tun_table, > + vl_mff_map); > > case OFPRAW_NXST_FLOW_REQUEST: > - return ofputil_decode_nxst_flow_request(fsr, &b, false, tun_table); > + return ofputil_decode_nxst_flow_request(fsr, &b, false, tun_table, > + vl_mff_map); > > case OFPRAW_NXST_AGGREGATE_REQUEST: > - return ofputil_decode_nxst_flow_request(fsr, &b, true, tun_table); > + return ofputil_decode_nxst_flow_request(fsr, &b, true, tun_table, > + vl_mff_map); > > default: > /* Hey, the caller lied. */ > @@ -2883,7 +2898,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs, > return EINVAL; > } > > - if (ofputil_pull_ofp11_match(msg, NULL, &fs->match, > + if (ofputil_pull_ofp11_match(msg, NULL, NULL, &fs->match, > &padded_match_len)) { > VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad match"); > return EINVAL; > @@ -2966,7 +2981,8 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs, > "claims invalid length %"PRIuSIZE, match_len, length); > return EINVAL; > } > - if (nx_pull_match(msg, match_len, &fs->match, NULL, NULL, NULL)) { > + if (nx_pull_match(msg, match_len, &fs->match, NULL, NULL, NULL, > + NULL)) { > return EINVAL; > } > instructions_len = length - sizeof *nfs - ROUND_UP(match_len, 8); > @@ -3185,7 +3201,7 @@ ofputil_decode_flow_removed(struct ofputil_flow_removed *fr, > > ofr = ofpbuf_pull(&b, sizeof *ofr); > > - error = ofputil_pull_ofp11_match(&b, NULL, &fr->match, NULL); > + error = ofputil_pull_ofp11_match(&b, NULL, NULL, &fr->match, NULL); > if (error) { > return error; > } > @@ -3222,7 +3238,7 @@ ofputil_decode_flow_removed(struct ofputil_flow_removed *fr, > > nfr = ofpbuf_pull(&b, sizeof *nfr); > error = nx_pull_match(&b, ntohs(nfr->match_len), &fr->match, NULL, > - NULL, NULL); > + NULL, NULL, NULL); > if (error) { > return error; > } > @@ -3344,6 +3360,7 @@ ofputil_encode_flow_removed(const struct ofputil_flow_removed *fr, > static enum ofperr > decode_nx_packet_in2(const struct ofp_header *oh, bool loose, > const struct tun_table *tun_table, > + const struct vl_mff_map *vl_mff_map, > struct ofputil_packet_in *pin, > size_t *total_len, uint32_t *buffer_id, > struct ofpbuf *continuation) > @@ -3398,7 +3415,8 @@ decode_nx_packet_in2(const struct ofp_header *oh, bool loose, > > case NXPINT_METADATA: > error = oxm_decode_match(payload.msg, ofpbuf_msgsize(&payload), > - loose, tun_table, &pin->flow_metadata); > + loose, tun_table, vl_mff_map, > + &pin->flow_metadata); > break; > > case NXPINT_USERDATA: > @@ -3452,10 +3470,15 @@ decode_nx_packet_in2(const struct ofp_header *oh, bool loose, > * it separately from the original OpenFlow message. This is also true for > * 'pin->userdata' (which could also end up NULL if there is no userdata). > * > + * 'vl_mff_map' is an optional parameter that is used to validate the length > + * of variable length mf_fields in 'match'. If it is not provided, the > + * default mf_fields with maximum length will be used. > + * > * Returns 0 if successful, otherwise an OpenFlow error code. */ > enum ofperr > ofputil_decode_packet_in(const struct ofp_header *oh, bool loose, > const struct tun_table *tun_table, > + const struct vl_mff_map *vl_mff_map, > struct ofputil_packet_in *pin, > size_t *total_lenp, uint32_t *buffer_idp, > struct ofpbuf *continuation) > @@ -3555,9 +3578,9 @@ ofputil_decode_packet_in(const struct ofp_header *oh, bool loose, > pin->packet = b.data; > pin->packet_len = b.size; > } else if (raw == OFPRAW_NXT_PACKET_IN2 || raw == OFPRAW_NXT_RESUME) { > - enum ofperr error = decode_nx_packet_in2(oh, loose, tun_table, pin, > - &total_len, &buffer_id, > - continuation); > + enum ofperr error = decode_nx_packet_in2(oh, loose, tun_table, > + vl_mff_map, pin, &total_len, > + &buffer_id, continuation); > if (error) { > return error; > } > @@ -4026,11 +4049,16 @@ parse_actions_property(struct ofpbuf *property, enum ofp_version version, > * opaque to any process other than ovs-vswitchd, so this function should not > * be used outside ovs-vswitchd. > * > + * 'vl_mff_map' is an optional parameter that is used to validate the length > + * of variable length mf_fields in 'match'. If it is not provided, the > + * default mf_fields with maximum length will be used. > + * > * When successful, 'pin' contains some dynamically allocated data. Call > * ofputil_packet_in_private_destroy() to free this data. */ > enum ofperr > ofputil_decode_packet_in_private(const struct ofp_header *oh, bool loose, > const struct tun_table *tun_table, > + const struct vl_mff_map *vl_mff_map, > struct ofputil_packet_in_private *pin, > size_t *total_len, uint32_t *buffer_id) > { > @@ -4038,8 +4066,9 @@ ofputil_decode_packet_in_private(const struct ofp_header *oh, bool loose, > > struct ofpbuf continuation; > enum ofperr error; > - error = ofputil_decode_packet_in(oh, loose, tun_table, &pin->public, > - total_len, buffer_id, &continuation); > + error = ofputil_decode_packet_in(oh, loose, tun_table, vl_mff_map, > + &pin->public, total_len, buffer_id, > + &continuation); > if (error) { > return error; > } > @@ -6609,7 +6638,7 @@ ofputil_decode_flow_monitor_request(struct ofputil_flow_monitor_request *rq, > rq->table_id = nfmr->table_id; > > return nx_pull_match(msg, ntohs(nfmr->match_len), &rq->match, NULL, > - NULL, NULL); > + NULL, NULL, NULL); > } > > void > @@ -6717,7 +6746,8 @@ ofputil_decode_flow_update(struct ofputil_flow_update *update, > update->cookie = nfuf->cookie; > update->priority = ntohs(nfuf->priority); > > - error = nx_pull_match(msg, match_len, &update->match, NULL, NULL, NULL); > + error = nx_pull_match(msg, match_len, &update->match, NULL, NULL, NULL, > + NULL); > if (error) { > return error; > } > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c > index 9234b56250ab..04712004dc86 100644 > --- a/ofproto/ofproto.c > +++ b/ofproto/ofproto.c > @@ -3568,8 +3568,9 @@ handle_nxt_resume(struct ofconn *ofconn, const struct ofp_header *oh) > enum ofperr error; > > error = ofputil_decode_packet_in_private(oh, false, > - ofproto_get_tun_tab(ofproto), &pin, > - NULL, NULL); > + ofproto_get_tun_tab(ofproto), > + &ofproto->vl_mff_map, &pin, NULL, > + NULL); > if (error) { > return error; > } > @@ -4273,7 +4274,8 @@ handle_flow_stats_request(struct ofconn *ofconn, > enum ofperr error; > > error = ofputil_decode_flow_stats_request(&fsr, request, > - ofproto_get_tun_tab(ofproto)); > + ofproto_get_tun_tab(ofproto), > + &ofproto->vl_mff_map); > if (error) { > return error; > } > @@ -4438,7 +4440,8 @@ handle_aggregate_stats_request(struct ofconn *ofconn, > enum ofperr error; > > error = ofputil_decode_flow_stats_request(&request, oh, > - ofproto_get_tun_tab(ofproto)); > + ofproto_get_tun_tab(ofproto), > + &ofproto->vl_mff_map); > if (error) { > return error; > } > diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c > index 0cdbf87cfb11..0380c8481ecf 100644 > --- a/ovn/controller/pinctrl.c > +++ b/ovn/controller/pinctrl.c > @@ -665,7 +665,7 @@ process_packet_in(const struct ofp_header *msg) > > struct ofputil_packet_in pin; > struct ofpbuf continuation; > - enum ofperr error = ofputil_decode_packet_in(msg, true, NULL, &pin, > + enum ofperr error = ofputil_decode_packet_in(msg, true, NULL, NULL, &pin, > NULL, NULL, &continuation); > > if (error) { > diff --git a/tests/ofproto.at b/tests/ofproto.at > index c899ec80c6cf..2136a2f6cb90 100644 > --- a/tests/ofproto.at > +++ b/tests/ofproto.at > @@ -5800,10 +5800,17 @@ AT_CHECK([strip_xids < stderr | sed '/FLOW_MOD/,$d'], [0], [dnl > OFPT_ERROR: OFPBAC_BAD_SET_LEN > ]) > > -AT_CHECK([ovs-ofctl dump-flows br0], [0], [stdout]) > -AT_CHECK([sed -e 's/duration=[[0-9.]]*s/duration=?s/' -e 's/idle_age=[[0-9]]*/idle_age=?/' stdout], [0], [dnl > -NXST_FLOW reply (xid=0x4): > - cookie=0x0, duration=?s, table=0, n_packets=0, n_bytes=0, idle_age=?, in_port=1 actions=move:NXM_NX_TUN_METADATA0[[0..31]]->NXM_NX_REG0[[]] > +dnl Check match field with tun_metadata > +AT_CHECK([ovs-ofctl add-flow br0 "tun_metadata0=0x11223344 actions=output:2"], [0], [], [stderr]) > +AT_CHECK([ovs-ofctl add-flow br0 "tun_metadata1=0x11223344 actions=output:2"], [1], [], [stderr]) > +AT_CHECK([strip_xids < stderr | sed '/FLOW_MOD/,$d'], [0], [dnl > +OFPT_ERROR: NXFMFC_INVALID_TLV_FIELD > +]) > + > +AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip], [0], [dnl) > +NXST_FLOW reply: > + in_port=1 actions=move:NXM_NX_TUN_METADATA0[[0..31]]->NXM_NX_REG0[[]] > + tun_metadata0=0x11223344 actions=output:2 > ]) > > OVS_VSWITCHD_STOP(["/NXFMFC_INVALID_TLV_FIELD/d > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c > index 77332e0f8c16..426e2fbc6a1f 100644 > --- a/utilities/ovs-ofctl.c > +++ b/utilities/ovs-ofctl.c > @@ -1878,8 +1878,8 @@ monitor_vconn(struct vconn *vconn, bool reply_to_echo_requests, > struct ofputil_packet_in pin; > struct ofpbuf continuation; > > - error = ofputil_decode_packet_in(b->data, true, NULL, &pin, > - NULL, NULL, > + error = ofputil_decode_packet_in(b->data, true, NULL, NULL, > + &pin, NULL, NULL, > &continuation); > if (error) { > fprintf(stderr, "decoding packet-in failed: %s", > @@ -3741,10 +3741,10 @@ ofctl_parse_nxm__(bool oxm, enum ofp_version version) > /* Convert nx_match to match. */ > if (strict) { > if (oxm) { > - error = oxm_pull_match(&nx_match, NULL, &match); > + error = oxm_pull_match(&nx_match, NULL, NULL, &match); > } else { > error = nx_pull_match(&nx_match, match_len, &match, > - &cookie, &cookie_mask, NULL); > + &cookie, &cookie_mask, NULL, NULL); > } > } else { > if (oxm) { > @@ -4161,7 +4161,8 @@ ofctl_check_vlan(struct ovs_cmdl_context *ctx) > ofpbuf_init(&nxm, 0); > nxm_match_len = nx_put_match(&nxm, &match, htonll(0), htonll(0)); > nxm_s = nx_match_to_string(nxm.data, nxm_match_len); > - error = nx_pull_match(&nxm, nxm_match_len, &nxm_match, NULL, NULL, NULL); > + error = nx_pull_match(&nxm, nxm_match_len, &nxm_match, NULL, NULL, NULL, > + NULL); > printf("NXM: %s -> ", nxm_s); > if (error) { > printf("%s\n", ofperr_to_string(error)); > @@ -4177,7 +4178,7 @@ ofctl_check_vlan(struct ovs_cmdl_context *ctx) > ofpbuf_init(&nxm, 0); > nxm_match_len = oxm_put_match(&nxm, &match, OFP12_VERSION); > nxm_s = oxm_match_to_string(&nxm, nxm_match_len); > - error = oxm_pull_match(&nxm, NULL, &nxm_match); > + error = oxm_pull_match(&nxm, NULL, NULL, &nxm_match); > printf("OXM: %s -> ", nxm_s); > if (error) { > printf("%s\n", ofperr_to_string(error)); > -- > 2.11.1 >
diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h index 0c3a10aa4264..e73a942a3e15 100644 --- a/include/openvswitch/ofp-util.h +++ b/include/openvswitch/ofp-util.h @@ -222,7 +222,7 @@ void ofputil_match_to_ofp10_match(const struct match *, struct ofp10_match *); /* Work with ofp11_match. */ enum ofperr ofputil_pull_ofp11_match(struct ofpbuf *, const struct tun_table *, - struct match *, + const struct vl_mff_map *, struct match *, uint16_t *padded_match_len); enum ofperr ofputil_pull_ofp11_mask(struct ofpbuf *, struct match *, struct mf_bitmap *bm); @@ -352,7 +352,7 @@ struct ofputil_flow_stats_request { enum ofperr ofputil_decode_flow_stats_request( struct ofputil_flow_stats_request *, const struct ofp_header *, - const struct tun_table *); + const struct tun_table *, const struct vl_mff_map *); struct ofpbuf *ofputil_encode_flow_stats_request( const struct ofputil_flow_stats_request *, enum ofputil_protocol); @@ -457,6 +457,7 @@ void ofputil_packet_in_destroy(struct ofputil_packet_in *); enum ofperr ofputil_decode_packet_in(const struct ofp_header *, bool loose, const struct tun_table *, + const struct vl_mff_map *, struct ofputil_packet_in *, size_t *total_len, uint32_t *buffer_id, struct ofpbuf *continuation); @@ -509,6 +510,7 @@ struct ofpbuf *ofputil_encode_packet_in_private( enum ofperr ofputil_decode_packet_in_private( const struct ofp_header *, bool loose, const struct tun_table *, + const struct vl_mff_map *, struct ofputil_packet_in_private *, size_t *total_len, uint32_t *buffer_id); diff --git a/lib/learning-switch.c b/lib/learning-switch.c index bc757f46dd7a..77155d04fcc0 100644 --- a/lib/learning-switch.c +++ b/lib/learning-switch.c @@ -523,7 +523,7 @@ process_packet_in(struct lswitch *sw, const struct ofp_header *oh) struct dp_packet pkt; struct flow flow; - error = ofputil_decode_packet_in(oh, true, NULL, &pi, NULL, + error = ofputil_decode_packet_in(oh, true, NULL, NULL, &pi, NULL, &buffer_id, NULL); if (error) { VLOG_WARN_RL(&rl, "failed to decode packet-in: %s", diff --git a/lib/nx-match.c b/lib/nx-match.c index c258869eec80..124cb71eb7c8 100644 --- a/lib/nx-match.c +++ b/lib/nx-match.c @@ -480,13 +480,14 @@ nx_pull_header(struct ofpbuf *b, const struct vl_mff_map *vl_mff_map, static enum ofperr nx_pull_match_entry(struct ofpbuf *b, bool allow_cookie, + const struct vl_mff_map *vl_mff_map, const struct mf_field **field, union mf_value *value, union mf_value *mask) { enum ofperr error; uint64_t header; - error = nx_pull_entry__(b, allow_cookie, NULL, &header, field, value, + error = nx_pull_entry__(b, allow_cookie, vl_mff_map, &header, field, value, mask); if (error) { return error; @@ -507,7 +508,8 @@ nx_pull_match_entry(struct ofpbuf *b, bool allow_cookie, static enum ofperr nx_pull_raw(const uint8_t *p, unsigned int match_len, bool strict, struct match *match, ovs_be64 *cookie, ovs_be64 *cookie_mask, - const struct tun_table *tun_table) + const struct tun_table *tun_table, + const struct vl_mff_map *vl_mff_map) { ovs_assert((cookie != NULL) == (cookie_mask != NULL)); @@ -525,7 +527,8 @@ nx_pull_raw(const uint8_t *p, unsigned int match_len, bool strict, union mf_value mask; enum ofperr error; - error = nx_pull_match_entry(&b, cookie != NULL, &field, &value, &mask); + error = nx_pull_match_entry(&b, cookie != NULL, vl_mff_map, &field, + &value, &mask); if (error) { if (error == OFPERR_OFPBMC_BAD_FIELD && !strict) { continue; @@ -571,7 +574,8 @@ static enum ofperr nx_pull_match__(struct ofpbuf *b, unsigned int match_len, bool strict, struct match *match, ovs_be64 *cookie, ovs_be64 *cookie_mask, - const struct tun_table *tun_table) + const struct tun_table *tun_table, + const struct vl_mff_map *vl_mff_map) { uint8_t *p = NULL; @@ -586,7 +590,7 @@ nx_pull_match__(struct ofpbuf *b, unsigned int match_len, bool strict, } return nx_pull_raw(p, match_len, strict, match, cookie, cookie_mask, - tun_table); + tun_table, vl_mff_map); } /* Parses the nx_match formatted match description in 'b' with length @@ -594,16 +598,21 @@ nx_pull_match__(struct ofpbuf *b, unsigned int match_len, bool strict, * are valid pointers, then stores the cookie and mask in them if 'b' contains * a "NXM_NX_COOKIE*" match. Otherwise, stores 0 in both. * + * 'vl_mff_map" is an optional parameter that is used to validate the length + * of variable length mf_fields in 'match'. If it is not provided, the + * default mf_fields with maximum length will be used. + * * Fails with an error upon encountering an unknown NXM header. * * Returns 0 if successful, otherwise an OpenFlow error code. */ enum ofperr nx_pull_match(struct ofpbuf *b, unsigned int match_len, struct match *match, ovs_be64 *cookie, ovs_be64 *cookie_mask, - const struct tun_table *tun_table) + const struct tun_table *tun_table, + const struct vl_mff_map *vl_mff_map) { return nx_pull_match__(b, match_len, true, match, cookie, cookie_mask, - tun_table); + tun_table, vl_mff_map); } /* Behaves the same as nx_pull_match(), but skips over unknown NXM headers, @@ -615,12 +624,13 @@ nx_pull_match_loose(struct ofpbuf *b, unsigned int match_len, const struct tun_table *tun_table) { return nx_pull_match__(b, match_len, false, match, cookie, cookie_mask, - tun_table); + tun_table, NULL); } static enum ofperr oxm_pull_match__(struct ofpbuf *b, bool strict, - const struct tun_table *tun_table, struct match *match) + const struct tun_table *tun_table, + const struct vl_mff_map *vl_mff_map, struct match *match) { struct ofp11_match_header *omh = b->data; uint8_t *p; @@ -648,20 +658,24 @@ oxm_pull_match__(struct ofpbuf *b, bool strict, } return nx_pull_raw(p + sizeof *omh, match_len - sizeof *omh, - strict, match, NULL, NULL, tun_table); + strict, match, NULL, NULL, tun_table, vl_mff_map); } /* Parses the oxm formatted match description preceded by a struct * ofp11_match_header in 'b'. Stores the result in 'match'. * + * 'vl_mff_map' is an optional parameter that is used to validate the length + * of variable length mf_fields in 'match'. If it is not provided, the + * default mf_fields with maximum length will be used. + * * Fails with an error when encountering unknown OXM headers. * * Returns 0 if successful, otherwise an OpenFlow error code. */ enum ofperr oxm_pull_match(struct ofpbuf *b, const struct tun_table *tun_table, - struct match *match) + const struct vl_mff_map *vl_mff_map, struct match *match) { - return oxm_pull_match__(b, true, tun_table, match); + return oxm_pull_match__(b, true, tun_table, vl_mff_map, match); } /* Behaves the same as oxm_pull_match() with one exception. Skips over unknown @@ -670,7 +684,7 @@ enum ofperr oxm_pull_match_loose(struct ofpbuf *b, const struct tun_table *tun_table, struct match *match) { - return oxm_pull_match__(b, false, tun_table, match); + return oxm_pull_match__(b, false, tun_table, NULL, match); } /* Parses the OXM match description in the 'oxm_len' bytes in 'oxm'. Stores @@ -683,9 +697,11 @@ oxm_pull_match_loose(struct ofpbuf *b, const struct tun_table *tun_table, */ enum ofperr oxm_decode_match(const void *oxm, size_t oxm_len, bool loose, - const struct tun_table *tun_table, struct match *match) + const struct tun_table *tun_table, + const struct vl_mff_map *vl_mff_map, struct match *match) { - return nx_pull_raw(oxm, oxm_len, !loose, match, NULL, NULL, tun_table); + return nx_pull_raw(oxm, oxm_len, !loose, match, NULL, NULL, tun_table, + vl_mff_map); } /* Verify an array of OXM TLVs treating value of each TLV as a mask, diff --git a/lib/nx-match.h b/lib/nx-match.h index e103dd5fa74d..03e2b9db1a64 100644 --- a/lib/nx-match.h +++ b/lib/nx-match.h @@ -52,17 +52,18 @@ char *mf_parse_subfield(struct mf_subfield *, const char *s) enum ofperr nx_pull_match(struct ofpbuf *, unsigned int match_len, struct match *, ovs_be64 *cookie, ovs_be64 *cookie_mask, - const struct tun_table *); + const struct tun_table *, const struct vl_mff_map *); enum ofperr nx_pull_match_loose(struct ofpbuf *, unsigned int match_len, struct match *, ovs_be64 *cookie, ovs_be64 *cookie_mask, const struct tun_table *); enum ofperr oxm_pull_match(struct ofpbuf *, const struct tun_table *, - struct match *); + const struct vl_mff_map *, struct match *); enum ofperr oxm_pull_match_loose(struct ofpbuf *, const struct tun_table *, struct match *); enum ofperr oxm_decode_match(const void *, size_t, bool, - const struct tun_table *, struct match *); + const struct tun_table *, + const struct vl_mff_map *, struct match *); enum ofperr oxm_pull_field_array(const void *, size_t fields_len, struct field_array *); diff --git a/lib/ofp-print.c b/lib/ofp-print.c index f7f7df26f5e1..80dbf6e16e86 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -118,7 +118,7 @@ ofp_print_packet_in(struct ds *string, const struct ofp_header *oh, size_t total_len; enum ofperr error; - error = ofputil_decode_packet_in_private(oh, true, NULL, + error = ofputil_decode_packet_in_private(oh, true, NULL, NULL, &pin, &total_len, &buffer_id); if (error) { ofp_print_error(string, error); @@ -1603,7 +1603,7 @@ ofp_print_flow_stats_request(struct ds *string, const struct ofp_header *oh) struct ofputil_flow_stats_request fsr; enum ofperr error; - error = ofputil_decode_flow_stats_request(&fsr, oh, NULL); + error = ofputil_decode_flow_stats_request(&fsr, oh, NULL, NULL); if (error) { ofp_print_error(string, error); return; diff --git a/lib/ofp-util.c b/lib/ofp-util.c index d3153370f2e6..c48081fe3e7f 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -278,6 +278,7 @@ ofputil_match_to_ofp10_match(const struct match *match, enum ofperr ofputil_pull_ofp11_match(struct ofpbuf *buf, const struct tun_table *tun_table, + const struct vl_mff_map *vl_mff_map, struct match *match, uint16_t *padded_match_len) { struct ofp11_match_header *omh = buf->data; @@ -307,7 +308,7 @@ ofputil_pull_ofp11_match(struct ofpbuf *buf, const struct tun_table *tun_table, if (padded_match_len) { *padded_match_len = ROUND_UP(match_len, 8); } - return oxm_pull_match(buf, tun_table, match); + return oxm_pull_match(buf, tun_table, vl_mff_map, match); default: return OFPERR_OFPBMC_BAD_TYPE; @@ -1585,7 +1586,8 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm, ofm = ofpbuf_pull(&b, sizeof *ofm); - error = ofputil_pull_ofp11_match(&b, tun_table, &fm->match, NULL); + error = ofputil_pull_ofp11_match(&b, tun_table, vl_mff_map, &fm->match, + NULL); if (error) { return error; } @@ -1681,7 +1683,7 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm, nfm = ofpbuf_pull(&b, sizeof *nfm); error = nx_pull_match(&b, ntohs(nfm->match_len), &fm->match, &fm->cookie, &fm->cookie_mask, - tun_table); + tun_table, vl_mff_map); if (error) { return error; } @@ -2271,7 +2273,8 @@ ofputil_decode_ofpst10_flow_request(struct ofputil_flow_stats_request *fsr, static enum ofperr ofputil_decode_ofpst11_flow_request(struct ofputil_flow_stats_request *fsr, struct ofpbuf *b, bool aggregate, - const struct tun_table *tun_table) + const struct tun_table *tun_table, + const struct vl_mff_map *vl_mff_map) { const struct ofp11_flow_stats_request *ofsr; enum ofperr error; @@ -2286,7 +2289,8 @@ ofputil_decode_ofpst11_flow_request(struct ofputil_flow_stats_request *fsr, fsr->out_group = ntohl(ofsr->out_group); fsr->cookie = ofsr->cookie; fsr->cookie_mask = ofsr->cookie_mask; - error = ofputil_pull_ofp11_match(b, tun_table, &fsr->match, NULL); + error = ofputil_pull_ofp11_match(b, tun_table, vl_mff_map, &fsr->match, + NULL); if (error) { return error; } @@ -2297,14 +2301,16 @@ ofputil_decode_ofpst11_flow_request(struct ofputil_flow_stats_request *fsr, static enum ofperr ofputil_decode_nxst_flow_request(struct ofputil_flow_stats_request *fsr, struct ofpbuf *b, bool aggregate, - const struct tun_table *tun_table) + const struct tun_table *tun_table, + const struct vl_mff_map *vl_mff_map) { const struct nx_flow_stats_request *nfsr; enum ofperr error; nfsr = ofpbuf_pull(b, sizeof *nfsr); error = nx_pull_match(b, ntohs(nfsr->match_len), &fsr->match, - &fsr->cookie, &fsr->cookie_mask, tun_table); + &fsr->cookie, &fsr->cookie_mask, tun_table, + vl_mff_map); if (error) { return error; } @@ -2714,11 +2720,16 @@ ofputil_pull_queue_get_config_reply(struct ofpbuf *msg, /* Converts an OFPST_FLOW, OFPST_AGGREGATE, NXST_FLOW, or NXST_AGGREGATE * request 'oh', into an abstract flow_stats_request in 'fsr'. Returns 0 if - * successful, otherwise an OpenFlow error code. */ + * successful, otherwise an OpenFlow error code. + * + * 'vl_mff_map' is an optional parameter that is used to validate the length + * of variable length mf_fields in 'match'. If it is not provided, the + * default mf_fields with maximum length will be used. */ enum ofperr ofputil_decode_flow_stats_request(struct ofputil_flow_stats_request *fsr, const struct ofp_header *oh, - const struct tun_table *tun_table) + const struct tun_table *tun_table, + const struct vl_mff_map *vl_mff_map) { struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); enum ofpraw raw = ofpraw_pull_assert(&b); @@ -2730,16 +2741,20 @@ ofputil_decode_flow_stats_request(struct ofputil_flow_stats_request *fsr, return ofputil_decode_ofpst10_flow_request(fsr, b.data, true); case OFPRAW_OFPST11_FLOW_REQUEST: - return ofputil_decode_ofpst11_flow_request(fsr, &b, false, tun_table); + return ofputil_decode_ofpst11_flow_request(fsr, &b, false, tun_table, + vl_mff_map); case OFPRAW_OFPST11_AGGREGATE_REQUEST: - return ofputil_decode_ofpst11_flow_request(fsr, &b, true, tun_table); + return ofputil_decode_ofpst11_flow_request(fsr, &b, true, tun_table, + vl_mff_map); case OFPRAW_NXST_FLOW_REQUEST: - return ofputil_decode_nxst_flow_request(fsr, &b, false, tun_table); + return ofputil_decode_nxst_flow_request(fsr, &b, false, tun_table, + vl_mff_map); case OFPRAW_NXST_AGGREGATE_REQUEST: - return ofputil_decode_nxst_flow_request(fsr, &b, true, tun_table); + return ofputil_decode_nxst_flow_request(fsr, &b, true, tun_table, + vl_mff_map); default: /* Hey, the caller lied. */ @@ -2883,7 +2898,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs, return EINVAL; } - if (ofputil_pull_ofp11_match(msg, NULL, &fs->match, + if (ofputil_pull_ofp11_match(msg, NULL, NULL, &fs->match, &padded_match_len)) { VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad match"); return EINVAL; @@ -2966,7 +2981,8 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs, "claims invalid length %"PRIuSIZE, match_len, length); return EINVAL; } - if (nx_pull_match(msg, match_len, &fs->match, NULL, NULL, NULL)) { + if (nx_pull_match(msg, match_len, &fs->match, NULL, NULL, NULL, + NULL)) { return EINVAL; } instructions_len = length - sizeof *nfs - ROUND_UP(match_len, 8); @@ -3185,7 +3201,7 @@ ofputil_decode_flow_removed(struct ofputil_flow_removed *fr, ofr = ofpbuf_pull(&b, sizeof *ofr); - error = ofputil_pull_ofp11_match(&b, NULL, &fr->match, NULL); + error = ofputil_pull_ofp11_match(&b, NULL, NULL, &fr->match, NULL); if (error) { return error; } @@ -3222,7 +3238,7 @@ ofputil_decode_flow_removed(struct ofputil_flow_removed *fr, nfr = ofpbuf_pull(&b, sizeof *nfr); error = nx_pull_match(&b, ntohs(nfr->match_len), &fr->match, NULL, - NULL, NULL); + NULL, NULL, NULL); if (error) { return error; } @@ -3344,6 +3360,7 @@ ofputil_encode_flow_removed(const struct ofputil_flow_removed *fr, static enum ofperr decode_nx_packet_in2(const struct ofp_header *oh, bool loose, const struct tun_table *tun_table, + const struct vl_mff_map *vl_mff_map, struct ofputil_packet_in *pin, size_t *total_len, uint32_t *buffer_id, struct ofpbuf *continuation) @@ -3398,7 +3415,8 @@ decode_nx_packet_in2(const struct ofp_header *oh, bool loose, case NXPINT_METADATA: error = oxm_decode_match(payload.msg, ofpbuf_msgsize(&payload), - loose, tun_table, &pin->flow_metadata); + loose, tun_table, vl_mff_map, + &pin->flow_metadata); break; case NXPINT_USERDATA: @@ -3452,10 +3470,15 @@ decode_nx_packet_in2(const struct ofp_header *oh, bool loose, * it separately from the original OpenFlow message. This is also true for * 'pin->userdata' (which could also end up NULL if there is no userdata). * + * 'vl_mff_map' is an optional parameter that is used to validate the length + * of variable length mf_fields in 'match'. If it is not provided, the + * default mf_fields with maximum length will be used. + * * Returns 0 if successful, otherwise an OpenFlow error code. */ enum ofperr ofputil_decode_packet_in(const struct ofp_header *oh, bool loose, const struct tun_table *tun_table, + const struct vl_mff_map *vl_mff_map, struct ofputil_packet_in *pin, size_t *total_lenp, uint32_t *buffer_idp, struct ofpbuf *continuation) @@ -3555,9 +3578,9 @@ ofputil_decode_packet_in(const struct ofp_header *oh, bool loose, pin->packet = b.data; pin->packet_len = b.size; } else if (raw == OFPRAW_NXT_PACKET_IN2 || raw == OFPRAW_NXT_RESUME) { - enum ofperr error = decode_nx_packet_in2(oh, loose, tun_table, pin, - &total_len, &buffer_id, - continuation); + enum ofperr error = decode_nx_packet_in2(oh, loose, tun_table, + vl_mff_map, pin, &total_len, + &buffer_id, continuation); if (error) { return error; } @@ -4026,11 +4049,16 @@ parse_actions_property(struct ofpbuf *property, enum ofp_version version, * opaque to any process other than ovs-vswitchd, so this function should not * be used outside ovs-vswitchd. * + * 'vl_mff_map' is an optional parameter that is used to validate the length + * of variable length mf_fields in 'match'. If it is not provided, the + * default mf_fields with maximum length will be used. + * * When successful, 'pin' contains some dynamically allocated data. Call * ofputil_packet_in_private_destroy() to free this data. */ enum ofperr ofputil_decode_packet_in_private(const struct ofp_header *oh, bool loose, const struct tun_table *tun_table, + const struct vl_mff_map *vl_mff_map, struct ofputil_packet_in_private *pin, size_t *total_len, uint32_t *buffer_id) { @@ -4038,8 +4066,9 @@ ofputil_decode_packet_in_private(const struct ofp_header *oh, bool loose, struct ofpbuf continuation; enum ofperr error; - error = ofputil_decode_packet_in(oh, loose, tun_table, &pin->public, - total_len, buffer_id, &continuation); + error = ofputil_decode_packet_in(oh, loose, tun_table, vl_mff_map, + &pin->public, total_len, buffer_id, + &continuation); if (error) { return error; } @@ -6609,7 +6638,7 @@ ofputil_decode_flow_monitor_request(struct ofputil_flow_monitor_request *rq, rq->table_id = nfmr->table_id; return nx_pull_match(msg, ntohs(nfmr->match_len), &rq->match, NULL, - NULL, NULL); + NULL, NULL, NULL); } void @@ -6717,7 +6746,8 @@ ofputil_decode_flow_update(struct ofputil_flow_update *update, update->cookie = nfuf->cookie; update->priority = ntohs(nfuf->priority); - error = nx_pull_match(msg, match_len, &update->match, NULL, NULL, NULL); + error = nx_pull_match(msg, match_len, &update->match, NULL, NULL, NULL, + NULL); if (error) { return error; } diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 9234b56250ab..04712004dc86 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -3568,8 +3568,9 @@ handle_nxt_resume(struct ofconn *ofconn, const struct ofp_header *oh) enum ofperr error; error = ofputil_decode_packet_in_private(oh, false, - ofproto_get_tun_tab(ofproto), &pin, - NULL, NULL); + ofproto_get_tun_tab(ofproto), + &ofproto->vl_mff_map, &pin, NULL, + NULL); if (error) { return error; } @@ -4273,7 +4274,8 @@ handle_flow_stats_request(struct ofconn *ofconn, enum ofperr error; error = ofputil_decode_flow_stats_request(&fsr, request, - ofproto_get_tun_tab(ofproto)); + ofproto_get_tun_tab(ofproto), + &ofproto->vl_mff_map); if (error) { return error; } @@ -4438,7 +4440,8 @@ handle_aggregate_stats_request(struct ofconn *ofconn, enum ofperr error; error = ofputil_decode_flow_stats_request(&request, oh, - ofproto_get_tun_tab(ofproto)); + ofproto_get_tun_tab(ofproto), + &ofproto->vl_mff_map); if (error) { return error; } diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c index 0cdbf87cfb11..0380c8481ecf 100644 --- a/ovn/controller/pinctrl.c +++ b/ovn/controller/pinctrl.c @@ -665,7 +665,7 @@ process_packet_in(const struct ofp_header *msg) struct ofputil_packet_in pin; struct ofpbuf continuation; - enum ofperr error = ofputil_decode_packet_in(msg, true, NULL, &pin, + enum ofperr error = ofputil_decode_packet_in(msg, true, NULL, NULL, &pin, NULL, NULL, &continuation); if (error) { diff --git a/tests/ofproto.at b/tests/ofproto.at index c899ec80c6cf..2136a2f6cb90 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -5800,10 +5800,17 @@ AT_CHECK([strip_xids < stderr | sed '/FLOW_MOD/,$d'], [0], [dnl OFPT_ERROR: OFPBAC_BAD_SET_LEN ]) -AT_CHECK([ovs-ofctl dump-flows br0], [0], [stdout]) -AT_CHECK([sed -e 's/duration=[[0-9.]]*s/duration=?s/' -e 's/idle_age=[[0-9]]*/idle_age=?/' stdout], [0], [dnl -NXST_FLOW reply (xid=0x4): - cookie=0x0, duration=?s, table=0, n_packets=0, n_bytes=0, idle_age=?, in_port=1 actions=move:NXM_NX_TUN_METADATA0[[0..31]]->NXM_NX_REG0[[]] +dnl Check match field with tun_metadata +AT_CHECK([ovs-ofctl add-flow br0 "tun_metadata0=0x11223344 actions=output:2"], [0], [], [stderr]) +AT_CHECK([ovs-ofctl add-flow br0 "tun_metadata1=0x11223344 actions=output:2"], [1], [], [stderr]) +AT_CHECK([strip_xids < stderr | sed '/FLOW_MOD/,$d'], [0], [dnl +OFPT_ERROR: NXFMFC_INVALID_TLV_FIELD +]) + +AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip], [0], [dnl) +NXST_FLOW reply: + in_port=1 actions=move:NXM_NX_TUN_METADATA0[[0..31]]->NXM_NX_REG0[[]] + tun_metadata0=0x11223344 actions=output:2 ]) OVS_VSWITCHD_STOP(["/NXFMFC_INVALID_TLV_FIELD/d diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index 77332e0f8c16..426e2fbc6a1f 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -1878,8 +1878,8 @@ monitor_vconn(struct vconn *vconn, bool reply_to_echo_requests, struct ofputil_packet_in pin; struct ofpbuf continuation; - error = ofputil_decode_packet_in(b->data, true, NULL, &pin, - NULL, NULL, + error = ofputil_decode_packet_in(b->data, true, NULL, NULL, + &pin, NULL, NULL, &continuation); if (error) { fprintf(stderr, "decoding packet-in failed: %s", @@ -3741,10 +3741,10 @@ ofctl_parse_nxm__(bool oxm, enum ofp_version version) /* Convert nx_match to match. */ if (strict) { if (oxm) { - error = oxm_pull_match(&nx_match, NULL, &match); + error = oxm_pull_match(&nx_match, NULL, NULL, &match); } else { error = nx_pull_match(&nx_match, match_len, &match, - &cookie, &cookie_mask, NULL); + &cookie, &cookie_mask, NULL, NULL); } } else { if (oxm) { @@ -4161,7 +4161,8 @@ ofctl_check_vlan(struct ovs_cmdl_context *ctx) ofpbuf_init(&nxm, 0); nxm_match_len = nx_put_match(&nxm, &match, htonll(0), htonll(0)); nxm_s = nx_match_to_string(nxm.data, nxm_match_len); - error = nx_pull_match(&nxm, nxm_match_len, &nxm_match, NULL, NULL, NULL); + error = nx_pull_match(&nxm, nxm_match_len, &nxm_match, NULL, NULL, NULL, + NULL); printf("NXM: %s -> ", nxm_s); if (error) { printf("%s\n", ofperr_to_string(error)); @@ -4177,7 +4178,7 @@ ofctl_check_vlan(struct ovs_cmdl_context *ctx) ofpbuf_init(&nxm, 0); nxm_match_len = oxm_put_match(&nxm, &match, OFP12_VERSION); nxm_s = oxm_match_to_string(&nxm, nxm_match_len); - error = oxm_pull_match(&nxm, NULL, &nxm_match); + error = oxm_pull_match(&nxm, NULL, NULL, &nxm_match); printf("OXM: %s -> ", nxm_s); if (error) { printf("%s\n", ofperr_to_string(error));