[ovs-dev] [PATCH] datapath-windows: Do not modify port field for ICMP during SNAT/DNAT

Sairam Venugopal vsairam at vmware.com
Fri Aug 11 21:36:11 UTC 2017


Acked-by: Sairam Venugopal <vsairam at vmware.com>




On 8/10/17, 8:59 PM, "ovs-dev-bounces at openvswitch.org on behalf of Anand Kumar" <ovs-dev-bounces at openvswitch.org on behalf of kumaranand at vmware.com> wrote:

>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/id/code into account
>during the lookups. Fix this to make it similar with the other conntrack
>module.
>
>Signed-off-by: Anand Kumar <kumaranand at vmware.com>
>---
> datapath-windows/ovsext/Conntrack-nat.c | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
>diff --git a/datapath-windows/ovsext/Conntrack-nat.c b/datapath-windows/ovsext/Conntrack-nat.c
>index ae6b92c..eb6f9db 100644
>--- a/datapath-windows/ovsext/Conntrack-nat.c
>+++ b/datapath-windows/ovsext/Conntrack-nat.c
>@@ -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;
> }
>@@ -44,6 +50,12 @@ OvsNatKeyAreSame(const OVS_CT_KEY *key1, const OVS_CT_KEY *key2)
>     FIELD_COMPARE(src.port);
>     FIELD_COMPARE(dst.port);
>     FIELD_COMPARE(zone);
>+    FIELD_COMPARE(src.icmp_id);
>+    FIELD_COMPARE(dst.icmp_id);
>+    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
> }
>@@ -253,6 +265,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
>@@ -271,7 +284,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) &&
>@@ -310,10 +323,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);
>-- 
>2.9.3.windows.1
>
>_______________________________________________
>dev mailing list
>dev at openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=Z6vowHUOjP5ysP_g372c49Nqc1vEKqHKNBkR5Q5Z7uo&m=DsmJOWCzvx6z5N9EJRhI-qvjbQUILl-ehTn-JLw4RZQ&s=NPGcReKGVlLHh2rvCJh0r8h-l38sSxpm8V-uPaVe9sY&e= 


More information about the dev mailing list