[ovs-dev,branch-2.7,1/4] nx-match: Fix oxm decode.

Message ID 20170315230141.32414-2-joe@ovn.org
State Accepted
Headers show

Commit Message

Joe Stringer March 15, 2017, 11:01 p.m.
From: Yi-Hung Wei <yihung.wei@gmail.com>

decode_nx_packet_in2() may be used by the switch to parse NXT_RESUME
messages, where we need exact match on the oxm header. It's also used by
OVN to parse NXT_PACKET_IN2 messages. For the switch, strict
prerequisites should be applied but for the controller, this should not
be the case. Pass the 'loose' parameter down to oxm_decode() to apply
these restrictions correctly based on which code is performing decode.

Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
Signed-off-by: Joe Stringer <joe@ovn.org>
---
 lib/nx-match.c | 8 +++++---
 lib/nx-match.h | 4 ++--
 lib/ofp-util.c | 2 +-
 3 files changed, 8 insertions(+), 6 deletions(-)

Comments

Jarno Rajahalme March 15, 2017, 11:22 p.m. | #1
IMO we should also backport the patch (“ofp-util: Ignore unknown fields in ofputil_decode_packet_in2().”) this patch fixed.

  jarno

> On Mar 15, 2017, at 4:01 PM, Joe Stringer <joe@ovn.org> wrote:
> 
> From: Yi-Hung Wei <yihung.wei@gmail.com>
> 
> decode_nx_packet_in2() may be used by the switch to parse NXT_RESUME
> messages, where we need exact match on the oxm header. It's also used by
> OVN to parse NXT_PACKET_IN2 messages. For the switch, strict
> prerequisites should be applied but for the controller, this should not
> be the case. Pass the 'loose' parameter down to oxm_decode() to apply
> these restrictions correctly based on which code is performing decode.
> 
> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
> Signed-off-by: Joe Stringer <joe@ovn.org>
> ---
> lib/nx-match.c | 8 +++++---
> lib/nx-match.h | 4 ++--
> lib/ofp-util.c | 2 +-
> 3 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/nx-match.c b/lib/nx-match.c
> index 91401e2201c6..c258869eec80 100644
> --- a/lib/nx-match.c
> +++ b/lib/nx-match.c
> @@ -678,12 +678,14 @@ oxm_pull_match_loose(struct ofpbuf *b, const struct tun_table *tun_table,
>  *
>  * Fails with an error when encountering unknown OXM headers.
>  *
> - * Returns 0 if successful, otherwise an OpenFlow error code. */
> + * If 'loose' is true, 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,
> +oxm_decode_match(const void *oxm, size_t oxm_len, bool loose,
>                  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, !loose, 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 5dca24a01a49..e103dd5fa74d 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(const void *, size_t, bool,
> +                             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 0c9343ec400b..d3153370f2e6 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -3398,7 +3398,7 @@ 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);
> +                                     loose, tun_table, &pin->flow_metadata);
>             break;
> 
>         case NXPINT_USERDATA:
> -- 
> 2.11.1
>
Jarno Rajahalme April 17, 2017, 9:01 p.m. | #2
This patch should be prepended by a prior patch, as some required changes are now missing:

7befb20d0f70 (“ofp-util: Ignore unknown fields in nx_decode_packet_in2().”)

  Jarno

> On Mar 15, 2017, at 4:01 PM, Joe Stringer <joe@ovn.org> wrote:
> 
> From: Yi-Hung Wei <yihung.wei@gmail.com>
> 
> decode_nx_packet_in2() may be used by the switch to parse NXT_RESUME
> messages, where we need exact match on the oxm header. It's also used by
> OVN to parse NXT_PACKET_IN2 messages. For the switch, strict
> prerequisites should be applied but for the controller, this should not
> be the case. Pass the 'loose' parameter down to oxm_decode() to apply
> these restrictions correctly based on which code is performing decode.
> 
> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
> Signed-off-by: Joe Stringer <joe@ovn.org>
> ---
> lib/nx-match.c | 8 +++++---
> lib/nx-match.h | 4 ++--
> lib/ofp-util.c | 2 +-
> 3 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/nx-match.c b/lib/nx-match.c
> index 91401e2201c6..c258869eec80 100644
> --- a/lib/nx-match.c
> +++ b/lib/nx-match.c
> @@ -678,12 +678,14 @@ oxm_pull_match_loose(struct ofpbuf *b, const struct tun_table *tun_table,
>  *
>  * Fails with an error when encountering unknown OXM headers.
>  *
> - * Returns 0 if successful, otherwise an OpenFlow error code. */
> + * If 'loose' is true, 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,
> +oxm_decode_match(const void *oxm, size_t oxm_len, bool loose,
>                  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, !loose, 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 5dca24a01a49..e103dd5fa74d 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(const void *, size_t, bool,
> +                             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 0c9343ec400b..d3153370f2e6 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -3398,7 +3398,7 @@ 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);
> +                                     loose, tun_table, &pin->flow_metadata);
>             break;
> 
>         case NXPINT_USERDATA:
> -- 
> 2.11.1
>
Joe Stringer April 20, 2017, 7:12 p.m. | #3
On 17 April 2017 at 14:01, Jarno Rajahalme <jarno@ovn.org> wrote:
> This patch should be prepended by a prior patch, as some required changes are now missing:
>
> 7befb20d0f70 (“ofp-util: Ignore unknown fields in nx_decode_packet_in2().”)

Thanks, I cherrypicked this one as well.

Patch

diff --git a/lib/nx-match.c b/lib/nx-match.c
index 91401e2201c6..c258869eec80 100644
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -678,12 +678,14 @@  oxm_pull_match_loose(struct ofpbuf *b, const struct tun_table *tun_table,
  *
  * Fails with an error when encountering unknown OXM headers.
  *
- * Returns 0 if successful, otherwise an OpenFlow error code. */
+ * If 'loose' is true, 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,
+oxm_decode_match(const void *oxm, size_t oxm_len, bool loose,
                  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, !loose, 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 5dca24a01a49..e103dd5fa74d 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(const void *, size_t, bool,
+                             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 0c9343ec400b..d3153370f2e6 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3398,7 +3398,7 @@  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);
+                                     loose, tun_table, &pin->flow_metadata);
             break;
 
         case NXPINT_USERDATA: