Skip to content

Commit

Permalink
meta-flow: Remove metadata prerequisite on ether type.
Browse files Browse the repository at this point in the history
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 daf4d3c ("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@dtdream.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/330052.html
Fixes: 7befb20 ("nx-match: Fix oxm decode.")
Fixes: daf4d3c ("odp: Support conntrack orig tuple key.")
Suggested-by: Numan Siddique <nusiddiq@redhat.com>
Suggested-by: Ben Pfaff <blp@ovn.org>
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
Tested-by: Numan Siddique <nusiddiq@redhat.com>
Acked-by:  Numan Siddique <nusiddiq@redhat.com>
Acked-by: Ben Pfaff <blp@ovn.org>
  • Loading branch information
jrajahalme committed Mar 24, 2017
1 parent 64fb5f8 commit e75aad8
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 14 deletions.
2 changes: 0 additions & 2 deletions build-aux/extract-ofp-fields
Expand Up @@ -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",
Expand Down
10 changes: 4 additions & 6 deletions include/openvswitch/meta-flow.h
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 0 additions & 6 deletions lib/meta-flow.c
Expand Up @@ -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);
Expand Down
10 changes: 10 additions & 0 deletions lib/ofp-util.c
Expand Up @@ -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;
}
Expand Down

0 comments on commit e75aad8

Please sign in to comment.