[ovs-dev] [PATCH] meta-flow: Remove metadata prerequisite on ether type.

Numan Siddique nusiddiq at redhat.com
Thu Mar 23 04:06:12 UTC 2017


On Thu, Mar 23, 2017 at 5:18 AM, Jarno Rajahalme <jarno at ovn.org> wrote:

> Conntrack original direction tuple fields depend on the conntrack
> state and the type of the packet that was tracked.  These dependencies
> were encoded as OpenFlow prerequisites in commit daf4d3c18da4 ("odp:
> Support conntrack orig tuple key.").  However, having a prerequisite
> from a metadata field to a packet header turned out to be problematic,
> since sometimes we are decoding metadata fields alone, so that the
> packet type field is not available.
>
> The reason for the packet type dependency is that the IP addresses in
> the original direction tuple can be either IPv4 or IPv6 addresses, and
> it would be invalid to match on IPv4 original direction tuple
> addresses for an IPv6 packet and vica verca.  Upon closer look,
> however, allowing this kind of mismatched match only causes the flow
> to never match anything, rather than causing more severe problems.
>
> This patch removes the formal prerequisite on the packet type, but
> replaces that with an explicit check for the mismatch on flow install.
> This way we can still return an error to the controller if it tries to
> install a mismatched flow.
>
> Reported-by: Dong Jun <dongj at dtdream.com>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/
> 330052.html
> Fixes: 7befb20d0f70 ("nx-match: Fix oxm decode.")
> Fixes: daf4d3c18da4 ("odp: Support conntrack orig tuple key.")
> Suggested-by: Numan Siddique <nusiddiq at redhat.com>
> Suggested-by: Ben Pfaff <blp at ovn.org>
> Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
>

Tested-by: Numan Siddique <nusiddiq at redhat.com>
Acked-by:  Numan Siddique <nusiddiq at redhat.com>


---
>  build-aux/extract-ofp-fields    |  2 --
>  include/openvswitch/meta-flow.h | 10 ++++------
>  lib/meta-flow.c                 |  6 ------
>  lib/ofp-util.c                  | 10 ++++++++++
>  4 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
> index a26d558..af7c69b 100755
> --- a/build-aux/extract-ofp-fields
> +++ b/build-aux/extract-ofp-fields
> @@ -45,8 +45,6 @@ PREREQS = {"none": "MFP_NONE",
>             "IPv6": "MFP_IPV6",
>             "IPv4/IPv6": "MFP_IP_ANY",
>             "CT": "MFP_CT_VALID",
> -           "CTv4": "MFP_CTV4_VALID",
> -           "CTv6": "MFP_CTV6_VALID",
>             "MPLS": "MFP_MPLS",
>             "TCP": "MFP_TCP",
>             "UDP": "MFP_UDP",
> diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-
> flow.h
> index 29ccadb..11852d2 100644
> --- a/include/openvswitch/meta-flow.h
> +++ b/include/openvswitch/meta-flow.h
> @@ -772,7 +772,7 @@ enum OVS_PACKED_ENUM mf_field_id {
>       * Type: be32.
>       * Maskable: bitwise.
>       * Formatting: IPv4.
> -     * Prerequisites: CTv4.
> +     * Prerequisites: CT.
>       * Access: read-only.
>       * NXM: NXM_NX_CT_NW_SRC(120) since v2.8.
>       * OXM: none.
> @@ -791,7 +791,7 @@ enum OVS_PACKED_ENUM mf_field_id {
>       * Type: be32.
>       * Maskable: bitwise.
>       * Formatting: IPv4.
> -     * Prerequisites: CTv4.
> +     * Prerequisites: CT.
>       * Access: read-only.
>       * NXM: NXM_NX_CT_NW_DST(121) since v2.8.
>       * OXM: none.
> @@ -810,7 +810,7 @@ enum OVS_PACKED_ENUM mf_field_id {
>       * Type: be128.
>       * Maskable: bitwise.
>       * Formatting: IPv6.
> -     * Prerequisites: CTv6.
> +     * Prerequisites: CT.
>       * Access: read-only.
>       * NXM: NXM_NX_CT_IPV6_SRC(122) since v2.8.
>       * OXM: none.
> @@ -829,7 +829,7 @@ enum OVS_PACKED_ENUM mf_field_id {
>       * Type: be128.
>       * Maskable: bitwise.
>       * Formatting: IPv6.
> -     * Prerequisites: CTv6.
> +     * Prerequisites: CT.
>       * Access: read-only.
>       * NXM: NXM_NX_CT_IPV6_DST(123) since v2.8.
>       * OXM: none.
> @@ -1824,8 +1824,6 @@ enum OVS_PACKED_ENUM mf_prereqs {
>      MFP_ICMPV4,
>      MFP_ICMPV6,
>      MFP_CT_VALID,               /* Implies IPv4 or IPv6. */
> -    MFP_CTV4_VALID,             /* MFP_CT_VALID and IPv4. */
> -    MFP_CTV6_VALID,             /* MFP_CT_VALID and IPv6. */
>
>      /* L2+L3+L4 requirements. */
>      MFP_ND,
> diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> index 93fbc5b..6b97794 100644
> --- a/lib/meta-flow.c
> +++ b/lib/meta-flow.c
> @@ -419,12 +419,6 @@ mf_are_prereqs_ok__(const struct mf_field *mf, const
> struct flow *flow,
>          return is_ip_any(flow);
>      case MFP_CT_VALID:
>          return is_ct_valid(flow, mask, wc);
> -    case MFP_CTV4_VALID:
> -        return flow->dl_type == htons(ETH_TYPE_IP)
> -            && is_ct_valid(flow, mask, wc);
> -    case MFP_CTV6_VALID:
> -        return flow->dl_type == htons(ETH_TYPE_IPV6)
> -            && is_ct_valid(flow, mask, wc);
>      case MFP_TCP:
>          /* Matching !FRAG_LATER is not enforced (mask is not checked). */
>          return is_tcp(flow, wc) && !(flow->nw_frag & FLOW_NW_FRAG_LATER);
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index b2f96ea..54c83fa 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -1724,6 +1724,16 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
>          }
>      }
>
> +    /* Check for mismatched conntrack original direction tuple address
> fields
> +     * w.r.t. the IP version of the match. */
> +    if (((fm->match.wc.masks.ct_nw_src || fm->match.wc.masks.ct_nw_dst)
> +         && fm->match.flow.dl_type != htons(ETH_TYPE_IP))
> +        || ((ipv6_addr_is_set(&fm->match.wc.masks.ct_ipv6_src)
> +             || ipv6_addr_is_set(&fm->match.wc.masks.ct_ipv6_dst))
> +            && fm->match.flow.dl_type != htons(ETH_TYPE_IPV6))) {
> +        return OFPERR_OFPBAC_MATCH_INCONSISTENT;
> +    }
> +
>      if (fm->command > OFPFC_DELETE_STRICT) {
>          return OFPERR_OFPFMFC_BAD_COMMAND;
>      }
> --
> 2.1.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>


More information about the dev mailing list