[ovs-dev] datapath-windows: Include ICMP type and code fields to find a matching NAT entry

Update OvsNatKeyAreSame() and OvsHashNatKey() to include ICMP type and code
fields, so that ICMP_ECHO_REQUEST packets are not matched with ICMP_ECHO_REPLY
packets and vice versa.

Signed-off-by: Anand Kumar <kumaranand@vmware.com>
---
 datapath-windows/ovsext/Conntrack-nat.c | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

Hi Anand,

Please do not change NAT code to include other fields in ct_endpoint. NAT
doesn't care about those fields. As long as the address and port are
identical, we will reverse NAT it. For example, even if we receive an ICMP
reply, we will match it against a ICMP request entry to do the reverse NAT.
We don't support ipv6 yet and don't want to be bothered by the extra fields
in src.addr and dst.addr either. So those FIELD_COMPARE are the exact
things we want to compare, nothing less.

Best regards,
Yin Lin

On Fri, Jun 23, 2017 at 2:27 PM, Anand Kumar <kumaranand@vmware.com> wrote:

> Update OvsNatKeyAreSame() and OvsHashNatKey() to include ICMP type and code
> fields, so that ICMP_ECHO_REQUEST packets are not matched with
> ICMP_ECHO_REPLY
> packets and vice versa.
>
> Signed-off-by: Anand Kumar <kumaranand@vmware.com>
> ---
>  datapath-windows/ovsext/Conntrack-nat.c | 32
> ++++++++++----------------------
>  1 file changed, 10 insertions(+), 22 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Conntrack-nat.c
> b/datapath-windows/ovsext/Conntrack-nat.c
> index ae6b92c..f3c282c 100644
> --- a/datapath-windows/ovsext/Conntrack-nat.c
> +++ b/datapath-windows/ovsext/Conntrack-nat.c
> @@ -13,17 +13,11 @@ PLIST_ENTRY ovsUnNatTable = NULL;
>  static __inline UINT32
>  OvsHashNatKey(const OVS_CT_KEY *key)
>  {
> -    UINT32 hash = 0;
> -#define HASH_ADD(field) \
> -    hash = OvsJhashBytes(&key->field, sizeof(key->field), hash)
> -
> -    HASH_ADD(src.addr.ipv4_aligned);
> -    HASH_ADD(dst.addr.ipv4_aligned);
> -    HASH_ADD(src.port);
> -    HASH_ADD(dst.port);
> -    HASH_ADD(zone);
> -#undef HASH_ADD
> -    return hash;
> +    UINT32 hash[3];
> +    hash[0] = OvsJhashBytes(&key->src, sizeof(key->src), 0);
> +    hash[1] = OvsJhashBytes(&key->dst, sizeof(key->dst), 0);
> +    hash[2] = OvsJhashBytes(&key->zone, sizeof(key->zone), 0);
> +    return OvsJhashWords(hash, 3, OVS_HASH_BASIS);
>  }
>
>  /*
> @@ -35,17 +29,11 @@ OvsHashNatKey(const OVS_CT_KEY *key)
>  static __inline BOOLEAN
>  OvsNatKeyAreSame(const OVS_CT_KEY *key1, const OVS_CT_KEY *key2)
>  {
> -    // XXX: Compare IPv6 key as well
> -#define FIELD_COMPARE(field) \
> -    if (key1->field != key2->field) return FALSE
> -
> -    FIELD_COMPARE(src.addr.ipv4_aligned);
> -    FIELD_COMPARE(dst.addr.ipv4_aligned);
> -    FIELD_COMPARE(src.port);
> -    FIELD_COMPARE(dst.port);
> -    FIELD_COMPARE(zone);
> -    return TRUE;
> -#undef FIELD_COMPARE
> +    return ((NdisEqualMemory(&key1->src, &key2->src,
> +                             sizeof(struct ct_endpoint))) &&
> +            (NdisEqualMemory(&key1->dst, &key2->dst,
> +                             sizeof(struct ct_endpoint))) &&
> +            (key1->zone == key2->zone));
>  }
>
>  /*
> --
> 2.9.3.windows.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>