diff mbox

[ovs-dev] ofproto-dpif: Detect support for ct_tuple6.

Message ID 20170601230301.22137-1-joe@ovn.org
State Changes Requested
Headers show

Commit Message

Joe Stringer June 1, 2017, 11:03 p.m. UTC
Support for extracting original direction 5 tuple fields from the
connection tracking module may differ on some platforms between the IPv4
original tuple fields vs. IPv6. Detect IPv6 original tuple support
separately and reflect this support up to the OpenFlow layer.

Signed-off-by: Joe Stringer <joe@ovn.org>
---
CC: Sairam Venugopal <vsairam@vmware.com>
---
 lib/odp-util.h         |  3 ++-
 ofproto/ofproto-dpif.c | 28 +++++++++++++++++++---------
 2 files changed, 21 insertions(+), 10 deletions(-)

Comments

Gregory Rose June 1, 2017, 11:43 p.m. UTC | #1
On 06/01/2017 04:03 PM, Joe Stringer wrote:
> Support for extracting original direction 5 tuple fields from the
> connection tracking module may differ on some platforms between the IPv4
> original tuple fields vs. IPv6. Detect IPv6 original tuple support
> separately and reflect this support up to the OpenFlow layer.
> 
> Signed-off-by: Joe Stringer <joe@ovn.org>
> ---
> CC: Sairam Venugopal <vsairam@vmware.com>
> ---
>   lib/odp-util.h         |  3 ++-
>   ofproto/ofproto-dpif.c | 28 +++++++++++++++++++---------
>   2 files changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/odp-util.h b/lib/odp-util.h
> index fafbb10ee7d2..aa7fb825b710 100644
> --- a/lib/odp-util.h
> +++ b/lib/odp-util.h
> @@ -193,7 +193,8 @@ int odp_flow_from_string(const char *s,
>       ODP_SUPPORT_FIELD(bool, ct_state_nat, "CT state NAT")                    \
>                                                                                \
>       /* Conntrack original direction tuple matching * supported. */           \
> -    ODP_SUPPORT_FIELD(bool, ct_orig_tuple, "CT orig tuple")
> +    ODP_SUPPORT_FIELD(bool, ct_orig_tuple, "CT orig tuple")                  \
> +    ODP_SUPPORT_FIELD(bool, ct_orig_tuple6, "CT orig tuple for IPv6")
>   
>   /* Indicates support for various fields. This defines how flows will be
>    * serialised. */
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index ba9e1ea4001e..0901fd2ea9bb 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1358,6 +1358,7 @@ CHECK_FEATURE__(ct_label, ct_label, ct_label.u64.lo, 1, ETH_TYPE_IP)
>   CHECK_FEATURE__(ct_state_nat, ct_state, ct_state, \
>                   CS_TRACKED|CS_SRC_NAT, ETH_TYPE_IP)
>   CHECK_FEATURE__(ct_orig_tuple, ct_orig_tuple, ct_nw_proto, 1, ETH_TYPE_IP)
> +CHECK_FEATURE__(ct_orig_tuple6, ct_orig_tuple, ct_nw_proto, 1, ETH_TYPE_IPV6)

I don't know enough about this code to say but should this be

  +CHECK_FEATURE__(ct_orig_tuple6, ct_orig_tuple6, ct_nw_proto, 1, 
ETH_TYPE_IPV6)

instead?

Thanks,

- Greg

>   
>   #undef CHECK_FEATURE
>   #undef CHECK_FEATURE__
> @@ -1388,6 +1389,7 @@ check_support(struct dpif_backer *backer)
>   
>       backer->support.odp.ct_state_nat = check_ct_state_nat(backer);
>       backer->support.odp.ct_orig_tuple = check_ct_orig_tuple(backer);
> +    backer->support.odp.ct_orig_tuple6 = check_ct_orig_tuple6(backer);
>   }
>   
>   static int
> @@ -4230,7 +4232,7 @@ check_mask(struct ofproto_dpif *ofproto, const struct miniflow *flow)
>        * the features we know of. */
>       if (support->ct_state && support->ct_zone && support->ct_mark
>           && support->ct_label && support->ct_state_nat
> -        && support->ct_orig_tuple) {
> +        && support->ct_orig_tuple && support->ct_orig_tuple6) {
>           return ct_state & CS_UNSUPPORTED_MASK ? OFPERR_OFPBMC_BAD_MASK : 0;
>       }
>   
> @@ -4247,14 +4249,22 @@ check_mask(struct ofproto_dpif *ofproto, const struct miniflow *flow)
>           return OFPERR_OFPBMC_BAD_MASK;
>       }
>   
> -    if (!support->ct_orig_tuple &&
> -        (MINIFLOW_GET_U8(flow, ct_nw_proto) ||
> -         MINIFLOW_GET_U16(flow, ct_tp_src) ||
> -         MINIFLOW_GET_U16(flow, ct_tp_dst) ||
> -         MINIFLOW_GET_U32(flow, ct_nw_src) ||
> -         MINIFLOW_GET_U32(flow, ct_nw_dst) ||
> -         !ovs_u128_is_zero(MINIFLOW_GET_U128(flow, ct_ipv6_src)) ||
> -         !ovs_u128_is_zero(MINIFLOW_GET_U128(flow, ct_ipv6_dst)))) {
> +    if (!support->ct_orig_tuple && !support->ct_orig_tuple6
> +        && (MINIFLOW_GET_U8(flow, ct_nw_proto)
> +            || MINIFLOW_GET_U16(flow, ct_tp_src)
> +            || MINIFLOW_GET_U16(flow, ct_tp_dst))) {
> +        return OFPERR_OFPBMC_BAD_MASK;
> +    }
> +
> +    if (!support->ct_orig_tuple
> +        && (MINIFLOW_GET_U32(flow, ct_nw_src)
> +            || MINIFLOW_GET_U32(flow, ct_nw_dst))) {
> +        return OFPERR_OFPBMC_BAD_MASK;
> +    }
> +
> +    if (!support->ct_orig_tuple6
> +        && (!ovs_u128_is_zero(MINIFLOW_GET_U128(flow, ct_ipv6_src))
> +            || !ovs_u128_is_zero(MINIFLOW_GET_U128(flow, ct_ipv6_dst)))) {
>           return OFPERR_OFPBMC_BAD_MASK;
>       }
>   
>
Joe Stringer June 1, 2017, 11:54 p.m. UTC | #2
On 1 June 2017 at 16:43, Greg Rose <gvrose8192@gmail.com> wrote:
> On 06/01/2017 04:03 PM, Joe Stringer wrote:
>>
>> Support for extracting original direction 5 tuple fields from the
>> connection tracking module may differ on some platforms between the IPv4
>> original tuple fields vs. IPv6. Detect IPv6 original tuple support
>> separately and reflect this support up to the OpenFlow layer.
>>
>> Signed-off-by: Joe Stringer <joe@ovn.org>
>> ---
>> CC: Sairam Venugopal <vsairam@vmware.com>
>> ---
>>   lib/odp-util.h         |  3 ++-
>>   ofproto/ofproto-dpif.c | 28 +++++++++++++++++++---------
>>   2 files changed, 21 insertions(+), 10 deletions(-)
>>
>> diff --git a/lib/odp-util.h b/lib/odp-util.h
>> index fafbb10ee7d2..aa7fb825b710 100644
>> --- a/lib/odp-util.h
>> +++ b/lib/odp-util.h
>> @@ -193,7 +193,8 @@ int odp_flow_from_string(const char *s,
>>       ODP_SUPPORT_FIELD(bool, ct_state_nat, "CT state NAT")
>> \
>>
>> \
>>       /* Conntrack original direction tuple matching * supported. */
>> \
>> -    ODP_SUPPORT_FIELD(bool, ct_orig_tuple, "CT orig tuple")
>> +    ODP_SUPPORT_FIELD(bool, ct_orig_tuple, "CT orig tuple")
>> \
>> +    ODP_SUPPORT_FIELD(bool, ct_orig_tuple6, "CT orig tuple for IPv6")
>>     /* Indicates support for various fields. This defines how flows will
>> be
>>    * serialised. */
>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> index ba9e1ea4001e..0901fd2ea9bb 100644
>> --- a/ofproto/ofproto-dpif.c
>> +++ b/ofproto/ofproto-dpif.c
>> @@ -1358,6 +1358,7 @@ CHECK_FEATURE__(ct_label, ct_label, ct_label.u64.lo,
>> 1, ETH_TYPE_IP)
>>   CHECK_FEATURE__(ct_state_nat, ct_state, ct_state, \
>>                   CS_TRACKED|CS_SRC_NAT, ETH_TYPE_IP)
>>   CHECK_FEATURE__(ct_orig_tuple, ct_orig_tuple, ct_nw_proto, 1,
>> ETH_TYPE_IP)
>> +CHECK_FEATURE__(ct_orig_tuple6, ct_orig_tuple, ct_nw_proto, 1,
>> ETH_TYPE_IPV6)
>
>
> I don't know enough about this code to say but should this be
>
>  +CHECK_FEATURE__(ct_orig_tuple6, ct_orig_tuple6, ct_nw_proto, 1,
> ETH_TYPE_IPV6)
>
> instead?

Yes, good catch thanks. Let me resubmit that.
diff mbox

Patch

diff --git a/lib/odp-util.h b/lib/odp-util.h
index fafbb10ee7d2..aa7fb825b710 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -193,7 +193,8 @@  int odp_flow_from_string(const char *s,
     ODP_SUPPORT_FIELD(bool, ct_state_nat, "CT state NAT")                    \
                                                                              \
     /* Conntrack original direction tuple matching * supported. */           \
-    ODP_SUPPORT_FIELD(bool, ct_orig_tuple, "CT orig tuple")
+    ODP_SUPPORT_FIELD(bool, ct_orig_tuple, "CT orig tuple")                  \
+    ODP_SUPPORT_FIELD(bool, ct_orig_tuple6, "CT orig tuple for IPv6")
 
 /* Indicates support for various fields. This defines how flows will be
  * serialised. */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ba9e1ea4001e..0901fd2ea9bb 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1358,6 +1358,7 @@  CHECK_FEATURE__(ct_label, ct_label, ct_label.u64.lo, 1, ETH_TYPE_IP)
 CHECK_FEATURE__(ct_state_nat, ct_state, ct_state, \
                 CS_TRACKED|CS_SRC_NAT, ETH_TYPE_IP)
 CHECK_FEATURE__(ct_orig_tuple, ct_orig_tuple, ct_nw_proto, 1, ETH_TYPE_IP)
+CHECK_FEATURE__(ct_orig_tuple6, ct_orig_tuple, ct_nw_proto, 1, ETH_TYPE_IPV6)
 
 #undef CHECK_FEATURE
 #undef CHECK_FEATURE__
@@ -1388,6 +1389,7 @@  check_support(struct dpif_backer *backer)
 
     backer->support.odp.ct_state_nat = check_ct_state_nat(backer);
     backer->support.odp.ct_orig_tuple = check_ct_orig_tuple(backer);
+    backer->support.odp.ct_orig_tuple6 = check_ct_orig_tuple6(backer);
 }
 
 static int
@@ -4230,7 +4232,7 @@  check_mask(struct ofproto_dpif *ofproto, const struct miniflow *flow)
      * the features we know of. */
     if (support->ct_state && support->ct_zone && support->ct_mark
         && support->ct_label && support->ct_state_nat
-        && support->ct_orig_tuple) {
+        && support->ct_orig_tuple && support->ct_orig_tuple6) {
         return ct_state & CS_UNSUPPORTED_MASK ? OFPERR_OFPBMC_BAD_MASK : 0;
     }
 
@@ -4247,14 +4249,22 @@  check_mask(struct ofproto_dpif *ofproto, const struct miniflow *flow)
         return OFPERR_OFPBMC_BAD_MASK;
     }
 
-    if (!support->ct_orig_tuple &&
-        (MINIFLOW_GET_U8(flow, ct_nw_proto) ||
-         MINIFLOW_GET_U16(flow, ct_tp_src) ||
-         MINIFLOW_GET_U16(flow, ct_tp_dst) ||
-         MINIFLOW_GET_U32(flow, ct_nw_src) ||
-         MINIFLOW_GET_U32(flow, ct_nw_dst) ||
-         !ovs_u128_is_zero(MINIFLOW_GET_U128(flow, ct_ipv6_src)) ||
-         !ovs_u128_is_zero(MINIFLOW_GET_U128(flow, ct_ipv6_dst)))) {
+    if (!support->ct_orig_tuple && !support->ct_orig_tuple6
+        && (MINIFLOW_GET_U8(flow, ct_nw_proto)
+            || MINIFLOW_GET_U16(flow, ct_tp_src)
+            || MINIFLOW_GET_U16(flow, ct_tp_dst))) {
+        return OFPERR_OFPBMC_BAD_MASK;
+    }
+
+    if (!support->ct_orig_tuple
+        && (MINIFLOW_GET_U32(flow, ct_nw_src)
+            || MINIFLOW_GET_U32(flow, ct_nw_dst))) {
+        return OFPERR_OFPBMC_BAD_MASK;
+    }
+
+    if (!support->ct_orig_tuple6
+        && (!ovs_u128_is_zero(MINIFLOW_GET_U128(flow, ct_ipv6_src))
+            || !ovs_u128_is_zero(MINIFLOW_GET_U128(flow, ct_ipv6_dst)))) {
         return OFPERR_OFPBMC_BAD_MASK;
     }