diff mbox series

[ovs-dev] conntrack: Fix icmp_id conflicts in snat

Message ID 20221211071810.883711-1-wushaohua@chinatelecom.cn
State Superseded
Headers show
Series [ovs-dev] conntrack: Fix icmp_id conflicts in snat | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

wushaohua@chinatelecom.cn Dec. 11, 2022, 7:18 a.m. UTC 
From: wushaohua <wushaohua@chinatelecom.cn>

The icmp_id maybe conflicts in snat
Description:
If multiple devices send icmp packets with the same icmp_id,
the sip of the packets changes to the same source ip address after the snat operation,
and the packets are sent to the same destination ip address.
Only one ct entry is created.The data packets sent by other devices cannot be sent to the peer device

ovs-appctl dpctl/dump-conntrack
icmp,orig=(src=10.0.0.4,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=500,type=0,code=0)

After fixing the problem:
ovs-appctl dpctl/dump-conntrack
icmp,orig=(src=10.0.0.3,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=501,type=0,code=0)
icmp,orig=(src=10.0.0.1,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=500,type=0,code=0)
icmp,orig=(src=10.0.0.7,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=505,type=0,code=0)
icmp,orig=(src=10.0.0.4,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=502,type=0,code=0)
icmp,orig=(src=10.0.0.5,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=503,type=0,code=0)
icmp,orig=(src=10.0.0.6,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=504,type=0,code=0)

Signed-off-by: wushaohua <wushaohua@chinatelecom.cn>
---
 lib/conntrack.c | 63 ++++++++++++++++++++++++++++++++++++++++++++-----
 lib/packets.c   | 22 +++++++++++++++++
 lib/packets.h   |  2 ++
 3 files changed, 81 insertions(+), 6 deletions(-)

Comments

wenxu@chinatelecom.cn Dec. 12, 2022, 4:22 a.m. UTC | #1 
wenxu@chinatelecom.cn
 
From: wushaohua
Date: 2022-12-11 15:18
To: dev
CC: wenxu; zoum2
Subject: [PATCH] conntrack: Fix icmp_id conflicts in snat
From: wushaohua <wushaohua@chinatelecom.cn>
 
The icmp_id maybe conflicts in snat
Description:
If multiple devices send icmp packets with the same icmp_id,
the sip of the packets changes to the same source ip address after the snat operation,
and the packets are sent to the same destination ip address.
Only one ct entry is created.The data packets sent by other devices cannot be sent to the peer device
 
ovs-appctl dpctl/dump-conntrack
icmp,orig=(src=10.0.0.4,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=500,type=0,code=0)
 
After fixing the problem:
ovs-appctl dpctl/dump-conntrack
icmp,orig=(src=10.0.0.3,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=501,type=0,code=0)
icmp,orig=(src=10.0.0.1,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=500,type=0,code=0)
icmp,orig=(src=10.0.0.7,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=505,type=0,code=0)
icmp,orig=(src=10.0.0.4,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=502,type=0,code=0)
icmp,orig=(src=10.0.0.5,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=503,type=0,code=0)
icmp,orig=(src=10.0.0.6,dst=10.0.0.2,id=500,type=8,code=0),reply=(src=10.0.0.2,dst=10.0.0.201,id=504,type=0,code=0)
 
Signed-off-by: wushaohua <wushaohua@chinatelecom.cn>
---
lib/conntrack.c | 63 ++++++++++++++++++++++++++++++++++++++++++++-----
lib/packets.c   | 22 +++++++++++++++++
lib/packets.h   |  2 ++
3 files changed, 81 insertions(+), 6 deletions(-)
 
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 550b2be9b..16182d232 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -697,6 +697,36 @@ get_ip_proto(const struct dp_packet *pkt)
     return ip_proto;
}
+static bool _is_icmpv4_info_message(uint8_t nw_proto,uint8_t icmp_type)
+{
+    if (nw_proto != IPPROTO_ICMP) {
+        return false;
+    }
+    switch (icmp_type) {
+        case ICMP4_ECHO_REQUEST:
+        case ICMP4_TIMESTAMP:
+        case ICMP4_INFOREQUEST:
+        case ICMP4_ECHO_REPLY:
+        case ICMP4_TIMESTAMPREPLY:
+        case ICMP4_INFOREPLY:
+            return true;
+        default:
+            return false;
+    }
+}
+
+static bool packet_is_icmpv4_info_message(const struct dp_packet *pkt)
+{
+    uint8_t ip_proto,icmp_type;
+    ip_proto = get_ip_proto(pkt);
+    if (ip_proto != IPPROTO_ICMP) {
+        return false;
+    } else {
+        icmp_type = packet_get_icmp_type(pkt);
+    }
+    return _is_icmpv4_info_message(ip_proto,icmp_type);
+}
+


上面的我觉得可以换成 
+
+static bool packet_is_icmpv4_info_message(const struct dp_packet *pkt)
+{
+    uint8_t ip_proto, icmp_type;
+
+    ip_proto = get_ip_proto(pkt);
+    if (ip_proto == IPPROTO_ICMP) {
+         icmp_type = packet_get_icmp_type(pkt);
+
+         switch (icmp_type) {  //换成if感觉更好
+              case ICMP4_ECHO_REQUEST:
+              case ICMP4_TIMESTAMP:
+              case ICMP4_INFOREQUEST:
+              case ICMP4_ECHO_REPLY:
+              case ICMP4_TIMESTAMPREPLY:
+              case ICMP4_INFOREPLY:
+                  return true;
+             default:
+                 return false;
+        } 
+    } 
+   
+    return false;
+}
+
static bool
is_ftp_ctl(const enum ct_alg_ctl_type ct_alg_ctl)
{
@@ -773,6 +803,9 @@ pat_packet(struct dp_packet *pkt, const struct conn *conn)
         } else if (conn->key.nw_proto == IPPROTO_UDP) {
             struct udp_header *uh = dp_packet_l4(pkt);
             packet_set_udp_port(pkt, conn->rev_key.dst.port, uh->udp_dst);
+        } else if (packet_is_icmpv4_info_message(pkt) &&
+            conn->key.nw_proto == IPPROTO_ICMP) {
+            packet_set_icmp_id(pkt, conn->rev_key.dst.icmp_id);
         }
     } else if (conn->nat_action & NAT_ACTION_DST) {
         if (conn->key.nw_proto == IPPROTO_TCP) {
@@ -781,6 +814,9 @@ pat_packet(struct dp_packet *pkt, const struct conn *conn)
         } else if (conn->key.nw_proto == IPPROTO_UDP) {
             packet_set_udp_port(pkt, conn->rev_key.dst.port,
                                 conn->rev_key.src.port);
+        } else if (packet_is_icmpv4_info_message(pkt) &&
+            conn->key.nw_proto == IPPROTO_ICMP) {
+            packet_set_icmp_id(pkt, conn->rev_key.src.icmp_id);
         }
     }
}
@@ -831,13 +867,19 @@ un_pat_packet(struct dp_packet *pkt, const struct conn *conn)
         } else if (conn->key.nw_proto == IPPROTO_UDP) {
             struct udp_header *uh = dp_packet_l4(pkt);
             packet_set_udp_port(pkt, uh->udp_src, conn->key.src.port);
-        }
+        } else if (packet_is_icmpv4_info_message(pkt) &&
+            conn->key.nw_proto == IPPROTO_ICMP) {
+            packet_set_icmp_id(pkt, conn->key.src.icmp_id);
+       }
     } else if (conn->nat_action & NAT_ACTION_DST) {
         if (conn->key.nw_proto == IPPROTO_TCP) {
             packet_set_tcp_port(pkt, conn->key.dst.port, conn->key.src.port);
         } else if (conn->key.nw_proto == IPPROTO_UDP) {
             packet_set_udp_port(pkt, conn->key.dst.port, conn->key.src.port);
-        }
+        } else if (packet_is_icmpv4_info_message(pkt) &&
+            conn->key.nw_proto == IPPROTO_ICMP) {
+            packet_set_icmp_id(pkt, conn->key.dst.icmp_id);
+       }
     }
}
@@ -2366,8 +2408,8 @@ store_addr_to_key(union ct_addr *addr, struct conn_key *key,
static bool
nat_get_unique_l4(struct conntrack *ct, struct conn *nat_conn,
-                  ovs_be16 *port, uint16_t curr, uint16_t min,
-                  uint16_t max)
+                  ovs_be16 *port, ovs_be16 *peer_port,
+                  uint16_t curr, uint16_t min, uint16_t max)
{
     static const unsigned int max_attempts = 128;
     uint16_t range = max - min + 1;
@@ -2388,6 +2430,9 @@ another_round:
         }
         *port = htons(curr);
+        if (peer_port) {
+            *peer_port = htons(curr);
+        }
         if (!conn_lookup(ct, &nat_conn->rev_key,
                          time_msec(), NULL, NULL)) {
             return true;
@@ -2401,6 +2446,9 @@ another_round:
     }
     *port = htons(orig);
+     if (peer_port) {
+         *peer_port = htons(orig);
+     }
     return false;
}
@@ -2434,7 +2482,8 @@ nat_get_unique_tuple(struct conntrack *ct, const struct conn *conn,
     uint32_t hash = nat_range_hash(conn, ct->hash_basis, nat_info);
     union ct_addr min_addr = {0}, max_addr = {0}, addr = {0};
     bool pat_proto = conn->key.nw_proto == IPPROTO_TCP ||
-                     conn->key.nw_proto == IPPROTO_UDP;
+                     conn->key.nw_proto == IPPROTO_UDP ||
+                     conn->key.nw_proto == IPPROTO_ICMP;
     uint16_t min_dport, max_dport, curr_dport;
     uint16_t min_sport, max_sport, curr_sport;
@@ -2469,11 +2518,13 @@ nat_get_unique_tuple(struct conntrack *ct, const struct conn *conn,
     bool found = false;
     if (nat_info->nat_action & NAT_ACTION_DST_PORT) {
         found = nat_get_unique_l4(ct, nat_conn, &nat_conn->rev_key.src.port,
-                                  curr_dport, min_dport, max_dport);
+                                  NULL,curr_dport, min_dport, max_dport);
     }
     if (!found) {
         found = nat_get_unique_l4(ct, nat_conn, &nat_conn->rev_key.dst.port,
+                                  nat_conn->rev_key.nw_proto == IPPROTO_ICMP ?
+                                  &nat_conn->rev_key.src.port : NULL,
                                   curr_sport, min_sport, max_sport);
     }
diff --git a/lib/packets.c b/lib/packets.c
index 1dcd4a6fc..11d21d8c3 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1441,6 +1441,28 @@ packet_set_icmp(struct dp_packet *packet, uint8_t type, uint8_t code)
     pkt_metadata_init_conn(&packet->md);
}
+/* Sets the ICMP id of the ICMP header contained in 'packet'.
+ * 'packet' must be a valid ICMP packet with its l4 offset properly
+ * populated. */
+void
+packet_set_icmp_id(struct dp_packet *packet, ovs_be16 icmp_id)
+{
+    struct icmp_header *ih = dp_packet_l4(packet);
+    ovs_be16 orig_ic = ih->icmp_fields.echo.id;
+
+    if (icmp_id != orig_ic) {
+        ih->icmp_fields.echo.id = icmp_id;
+        ih->icmp_csum = recalc_csum16(ih->icmp_csum, orig_ic, icmp_id);
+    }
+    pkt_metadata_init_conn(&packet->md);
+}
+
+uint8_t
+packet_get_icmp_type(const struct dp_packet *packet)
+{
+    struct icmp_header *ih = dp_packet_l4(packet);
+    return ih->icmp_type;
+}
/* Sets the IGMP type to IGMP_HOST_MEMBERSHIP_QUERY and populates the
  * v3 query header fields in 'packet'. 'packet' must be a valid IGMPv3
  * query packet with its l4 offset properly populated.
diff --git a/lib/packets.h b/lib/packets.h
index 5bdf6e4bb..f30ace119 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1602,6 +1602,8 @@ void packet_set_tcp_port(struct dp_packet *, ovs_be16 src, ovs_be16 dst);
void packet_set_udp_port(struct dp_packet *, ovs_be16 src, ovs_be16 dst);
void packet_set_sctp_port(struct dp_packet *, ovs_be16 src, ovs_be16 dst);
void packet_set_icmp(struct dp_packet *, uint8_t type, uint8_t code);
+void packet_set_icmp_id(struct dp_packet *, ovs_be16 icmp_id);
+uint8_t packet_get_icmp_type(const struct dp_packet *packet);
void packet_set_nd(struct dp_packet *, const struct in6_addr *target,
                    const struct eth_addr sll, const struct eth_addr tll);
void packet_set_nd_ext(struct dp_packet *packet,
diff mbox series

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 550b2be9b..16182d232 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -697,6 +697,36 @@  get_ip_proto(const struct dp_packet *pkt)
     return ip_proto;
 }
 
+static bool _is_icmpv4_info_message(uint8_t nw_proto,uint8_t icmp_type)
+{
+    if (nw_proto != IPPROTO_ICMP) {
+        return false;
+    }
+    switch (icmp_type) {
+        case ICMP4_ECHO_REQUEST:
+        case ICMP4_TIMESTAMP:
+        case ICMP4_INFOREQUEST:
+        case ICMP4_ECHO_REPLY:
+        case ICMP4_TIMESTAMPREPLY:
+        case ICMP4_INFOREPLY:
+            return true;
+        default:
+            return false;
+    }
+}
+
+static bool packet_is_icmpv4_info_message(const struct dp_packet *pkt)
+{
+    uint8_t ip_proto,icmp_type;
+    ip_proto = get_ip_proto(pkt);
+    if (ip_proto != IPPROTO_ICMP) {
+        return false;
+    } else {
+        icmp_type = packet_get_icmp_type(pkt);
+    }
+    return _is_icmpv4_info_message(ip_proto,icmp_type);
+}
+
 static bool
 is_ftp_ctl(const enum ct_alg_ctl_type ct_alg_ctl)
 {
@@ -773,6 +803,9 @@  pat_packet(struct dp_packet *pkt, const struct conn *conn)
         } else if (conn->key.nw_proto == IPPROTO_UDP) {
             struct udp_header *uh = dp_packet_l4(pkt);
             packet_set_udp_port(pkt, conn->rev_key.dst.port, uh->udp_dst);
+        } else if (packet_is_icmpv4_info_message(pkt) &&
+            conn->key.nw_proto == IPPROTO_ICMP) {
+            packet_set_icmp_id(pkt, conn->rev_key.dst.icmp_id);
         }
     } else if (conn->nat_action & NAT_ACTION_DST) {
         if (conn->key.nw_proto == IPPROTO_TCP) {
@@ -781,6 +814,9 @@  pat_packet(struct dp_packet *pkt, const struct conn *conn)
         } else if (conn->key.nw_proto == IPPROTO_UDP) {
             packet_set_udp_port(pkt, conn->rev_key.dst.port,
                                 conn->rev_key.src.port);
+        } else if (packet_is_icmpv4_info_message(pkt) &&
+            conn->key.nw_proto == IPPROTO_ICMP) {
+            packet_set_icmp_id(pkt, conn->rev_key.src.icmp_id);
         }
     }
 }
@@ -831,13 +867,19 @@  un_pat_packet(struct dp_packet *pkt, const struct conn *conn)
         } else if (conn->key.nw_proto == IPPROTO_UDP) {
             struct udp_header *uh = dp_packet_l4(pkt);
             packet_set_udp_port(pkt, uh->udp_src, conn->key.src.port);
-        }
+        } else if (packet_is_icmpv4_info_message(pkt) &&
+            conn->key.nw_proto == IPPROTO_ICMP) {
+            packet_set_icmp_id(pkt, conn->key.src.icmp_id);
+       }
     } else if (conn->nat_action & NAT_ACTION_DST) {
         if (conn->key.nw_proto == IPPROTO_TCP) {
             packet_set_tcp_port(pkt, conn->key.dst.port, conn->key.src.port);
         } else if (conn->key.nw_proto == IPPROTO_UDP) {
             packet_set_udp_port(pkt, conn->key.dst.port, conn->key.src.port);
-        }
+        } else if (packet_is_icmpv4_info_message(pkt) &&
+            conn->key.nw_proto == IPPROTO_ICMP) {
+            packet_set_icmp_id(pkt, conn->key.dst.icmp_id);
+       }
     }
 }
 
@@ -2366,8 +2408,8 @@  store_addr_to_key(union ct_addr *addr, struct conn_key *key,
 
 static bool
 nat_get_unique_l4(struct conntrack *ct, struct conn *nat_conn,
-                  ovs_be16 *port, uint16_t curr, uint16_t min,
-                  uint16_t max)
+                  ovs_be16 *port, ovs_be16 *peer_port,
+                  uint16_t curr, uint16_t min, uint16_t max)
 {
     static const unsigned int max_attempts = 128;
     uint16_t range = max - min + 1;
@@ -2388,6 +2430,9 @@  another_round:
         }
 
         *port = htons(curr);
+        if (peer_port) {
+            *peer_port = htons(curr);
+        }
         if (!conn_lookup(ct, &nat_conn->rev_key,
                          time_msec(), NULL, NULL)) {
             return true;
@@ -2401,6 +2446,9 @@  another_round:
     }
 
     *port = htons(orig);
+     if (peer_port) {
+         *peer_port = htons(orig);
+     }
 
     return false;
 }
@@ -2434,7 +2482,8 @@  nat_get_unique_tuple(struct conntrack *ct, const struct conn *conn,
     uint32_t hash = nat_range_hash(conn, ct->hash_basis, nat_info);
     union ct_addr min_addr = {0}, max_addr = {0}, addr = {0};
     bool pat_proto = conn->key.nw_proto == IPPROTO_TCP ||
-                     conn->key.nw_proto == IPPROTO_UDP;
+                     conn->key.nw_proto == IPPROTO_UDP ||
+                     conn->key.nw_proto == IPPROTO_ICMP;
     uint16_t min_dport, max_dport, curr_dport;
     uint16_t min_sport, max_sport, curr_sport;
 
@@ -2469,11 +2518,13 @@  nat_get_unique_tuple(struct conntrack *ct, const struct conn *conn,
     bool found = false;
     if (nat_info->nat_action & NAT_ACTION_DST_PORT) {
         found = nat_get_unique_l4(ct, nat_conn, &nat_conn->rev_key.src.port,
-                                  curr_dport, min_dport, max_dport);
+                                  NULL,curr_dport, min_dport, max_dport);
     }
 
     if (!found) {
         found = nat_get_unique_l4(ct, nat_conn, &nat_conn->rev_key.dst.port,
+                                  nat_conn->rev_key.nw_proto == IPPROTO_ICMP ?
+                                  &nat_conn->rev_key.src.port : NULL,
                                   curr_sport, min_sport, max_sport);
     }
 
diff --git a/lib/packets.c b/lib/packets.c
index 1dcd4a6fc..11d21d8c3 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1441,6 +1441,28 @@  packet_set_icmp(struct dp_packet *packet, uint8_t type, uint8_t code)
     pkt_metadata_init_conn(&packet->md);
 }
 
+/* Sets the ICMP id of the ICMP header contained in 'packet'.
+ * 'packet' must be a valid ICMP packet with its l4 offset properly
+ * populated. */
+void
+packet_set_icmp_id(struct dp_packet *packet, ovs_be16 icmp_id)
+{
+    struct icmp_header *ih = dp_packet_l4(packet);
+    ovs_be16 orig_ic = ih->icmp_fields.echo.id;
+
+    if (icmp_id != orig_ic) {
+        ih->icmp_fields.echo.id = icmp_id;
+        ih->icmp_csum = recalc_csum16(ih->icmp_csum, orig_ic, icmp_id);
+    }
+    pkt_metadata_init_conn(&packet->md);
+}
+
+uint8_t
+packet_get_icmp_type(const struct dp_packet *packet)
+{
+    struct icmp_header *ih = dp_packet_l4(packet);
+    return ih->icmp_type;
+}
 /* Sets the IGMP type to IGMP_HOST_MEMBERSHIP_QUERY and populates the
  * v3 query header fields in 'packet'. 'packet' must be a valid IGMPv3
  * query packet with its l4 offset properly populated.
diff --git a/lib/packets.h b/lib/packets.h
index 5bdf6e4bb..f30ace119 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1602,6 +1602,8 @@  void packet_set_tcp_port(struct dp_packet *, ovs_be16 src, ovs_be16 dst);
 void packet_set_udp_port(struct dp_packet *, ovs_be16 src, ovs_be16 dst);
 void packet_set_sctp_port(struct dp_packet *, ovs_be16 src, ovs_be16 dst);
 void packet_set_icmp(struct dp_packet *, uint8_t type, uint8_t code);
+void packet_set_icmp_id(struct dp_packet *, ovs_be16 icmp_id);
+uint8_t packet_get_icmp_type(const struct dp_packet *packet);
 void packet_set_nd(struct dp_packet *, const struct in6_addr *target,
                    const struct eth_addr sll, const struct eth_addr tll);
 void packet_set_nd_ext(struct dp_packet *packet,