Skip to content

Commit

Permalink
datapath-windows: Do not modify port field for ICMP during SNAT/DNAT
Browse files Browse the repository at this point in the history
During SNAT/DNAT, we should not be updating the port field of ct_endpoint
struct, as ICMP packets do not have port information. Since port and
icmp_id are overlapped in ct_endpoint struct, icmp_id gets changed.
As a result, NAT look up fails to find a matching entry.

This patch addresses this issue by not modifying icmp_id field during
SNAT/DNAT only for ICMP traffic

The current NAT module doesn't take the ICMP type/code into account
during the lookups. Fix this to make it similar with the other conntrack
module.

Acked-by: Shashank Ram <rams@vmware.com>
Acked-by: Alin Gabriel Serdean <aserdean@ovn.org>
Signed-off-by: Anand Kumar <kumaranand@vmware.com>
Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
  • Loading branch information
Anandkumar26 authored and aserdean committed Aug 17, 2017
1 parent a193048 commit 9fbb4a8
Showing 1 changed file with 19 additions and 3 deletions.
22 changes: 19 additions & 3 deletions datapath-windows/ovsext/Conntrack-nat.c
Expand Up @@ -22,6 +22,12 @@ OvsHashNatKey(const OVS_CT_KEY *key)
HASH_ADD(src.port);
HASH_ADD(dst.port);
HASH_ADD(zone);
/* icmp_id and port overlap in the union */
HASH_ADD(src.icmp_type);
HASH_ADD(dst.icmp_type);
HASH_ADD(src.icmp_code);
HASH_ADD(dst.icmp_code);

#undef HASH_ADD
return hash;
}
Expand All @@ -44,6 +50,11 @@ OvsNatKeyAreSame(const OVS_CT_KEY *key1, const OVS_CT_KEY *key2)
FIELD_COMPARE(src.port);
FIELD_COMPARE(dst.port);
FIELD_COMPARE(zone);
/* icmp_id and port overlap in the union */
FIELD_COMPARE(src.icmp_type);
FIELD_COMPARE(dst.icmp_type);
FIELD_COMPARE(src.icmp_code);
FIELD_COMPARE(dst.icmp_code);
return TRUE;
#undef FIELD_COMPARE
}
Expand Down Expand Up @@ -253,6 +264,7 @@ OvsNatAddEntry(OVS_NAT_ENTRY* entry)
* Update an Conntrack entry with NAT information. Translated address and
* port will be generated and write back to the conntrack entry as a
* result.
* Note: For ICMP, only address is translated.
*----------------------------------------------------------------------------
*/
BOOLEAN
Expand All @@ -271,7 +283,7 @@ OvsNatTranslateCtEntry(OVS_CT_ENTRY *entry)
BOOLEAN allPortsTried;
BOOLEAN originalPortsTried;
struct ct_addr firstAddr;

uint32_t hash = OvsNatHashRange(entry, 0);

if ((entry->natInfo.natAction & NAT_ACTION_SRC) &&
Expand Down Expand Up @@ -310,10 +322,14 @@ OvsNatTranslateCtEntry(OVS_CT_ENTRY *entry)
for (;;) {
if (entry->natInfo.natAction & NAT_ACTION_SRC) {
entry->rev_key.dst.addr = ctAddr;
entry->rev_key.dst.port = htons(port);
if (entry->rev_key.nw_proto != IPPROTO_ICMP) {
entry->rev_key.dst.port = htons(port);
}
} else {
entry->rev_key.src.addr = ctAddr;
entry->rev_key.src.port = htons(port);
if (entry->rev_key.nw_proto != IPPROTO_ICMP) {
entry->rev_key.src.port = htons(port);
}
}

OVS_NAT_ENTRY *natEntry = OvsNatLookup(&entry->rev_key, TRUE);
Expand Down

0 comments on commit 9fbb4a8

Please sign in to comment.