On 6 March 2017 at 16:22, Jarno Rajahalme <[email protected]> wrote: > The decoder of packet_in messages should not fail on encountering > unknown metadata fields. This allows the switch to add new features > without breaking controllers. The controllers should, however, copy > the metadata fields from the packet_int to packet_out so that the > switch gets back the full metadata. OVN is already doing this. > > Signed-off-by: Jarno Rajahalme <[email protected]> > --- > lib/nx-match.c | 25 ++++++++++++++++--------- > lib/nx-match.h | 4 ++-- > lib/ofp-util.c | 5 +++-- > 3 files changed, 21 insertions(+), 13 deletions(-) > > diff --git a/lib/nx-match.c b/lib/nx-match.c > index 95516a1..bcc1347 100644 > --- a/lib/nx-match.c > +++ b/lib/nx-match.c > @@ -504,6 +504,9 @@ nx_pull_match_entry(struct ofpbuf *b, bool allow_cookie, > return 0; > } > > +/* Prerequisites will only be checked when 'strict' is 'true'. This allows > + * decoding conntrack original direction 5-tuple IP addresses without the > + * ethertype being present, when decoding metadata only. */ > 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, > @@ -539,7 +542,7 @@ nx_pull_raw(const uint8_t *p, unsigned int match_len, > bool strict, > *cookie = value.be64; > *cookie_mask = mask.be64; > } > - } else if (!mf_are_match_prereqs_ok(field, match)) { > + } else if (strict && !mf_are_match_prereqs_ok(field, match)) { > error = OFPERR_OFPBMC_BAD_PREREQ; > } else if (!mf_is_all_wild(field, &match->wc)) { > error = OFPERR_OFPBMC_DUP_FIELD; > @@ -607,7 +610,8 @@ nx_pull_match(struct ofpbuf *b, unsigned int match_len, > struct match *match, > } > > /* Behaves the same as nx_pull_match(), but skips over unknown NXM headers, > - * instead of failing with an error. */ > + * instead of failing with an error, and does not check for field > + * prerequisities. */ > enum ofperr > nx_pull_match_loose(struct ofpbuf *b, unsigned int match_len, > struct match *match, > @@ -664,8 +668,9 @@ oxm_pull_match(struct ofpbuf *b, const struct tun_table > *tun_table, > return oxm_pull_match__(b, true, tun_table, match); > } > > -/* Behaves the same as oxm_pull_match() with one exception. Skips over > unknown > - * OXM headers instead of failing with an error when they are encountered. */ > +/* Behaves the same as oxm_pull_match() with two exceptions. Skips over > + * unknown OXM headers instead of failing with an error when they are > + * encountered, and does not check for field prerequisities. */ > enum ofperr > oxm_pull_match_loose(struct ofpbuf *b, const struct tun_table *tun_table, > struct match *match) > @@ -676,14 +681,16 @@ oxm_pull_match_loose(struct ofpbuf *b, const struct > tun_table *tun_table, > /* Parses the OXM match description in the 'oxm_len' bytes in 'oxm'. Stores > * the result in 'match'. > * > - * Fails with an error when encountering unknown OXM headers. > + * Returns 0 if successful, otherwise an OpenFlow error code. > * > - * Returns 0 if successful, otherwise an OpenFlow error code. */ > + * Encountering unknown OXM headers or missing field prerequisites are not > + * considered as error conditions. > + */ > enum ofperr > -oxm_decode_match(const void *oxm, size_t oxm_len, > - const struct tun_table *tun_table, struct match *match) > +oxm_decode_match_loose(const void *oxm, size_t oxm_len, > + const struct tun_table *tun_table, struct match > *match) > { > - return nx_pull_raw(oxm, oxm_len, true, match, NULL, NULL, tun_table); > + return nx_pull_raw(oxm, oxm_len, false, match, NULL, NULL, tun_table); > } > > /* 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 631ab48..b599731 100644 > --- a/lib/nx-match.h > +++ b/lib/nx-match.h > @@ -61,8 +61,8 @@ enum ofperr oxm_pull_match(struct ofpbuf *, const struct > tun_table *, > 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, const struct tun_table *, > - struct match *); > +enum ofperr oxm_decode_match_loose(const void *, size_t, > + const struct tun_table *, struct match *); > enum ofperr oxm_pull_field_array(const void *, size_t fields_len, > struct field_array *); > > diff --git a/lib/ofp-util.c b/lib/ofp-util.c > index 7d40cbb..ed66dd1 100644 > --- a/lib/ofp-util.c > +++ b/lib/ofp-util.c > @@ -3397,8 +3397,9 @@ decode_nx_packet_in2(const struct ofp_header *oh, bool > loose, > } > > case NXPINT_METADATA: > - error = oxm_decode_match(payload.msg, ofpbuf_msgsize(&payload), > - tun_table, &pin->flow_metadata); > + error = oxm_decode_match_loose(payload.msg, > + ofpbuf_msgsize(&payload), > + tun_table, &pin->flow_metadata);
Should we use 'loose' to determine whether to strictly decode or ignore unknown OXMs? _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
