diff mbox series

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

Message ID 20221215034951.1754772-1-wushaohua@chinatelecom.cn
State Changes Requested
Headers show
Series [ovs-dev,v2] 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. 15, 2022, 3:49 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 | 51 +++++++++++++++++++++++++++++++++++++++++++------
 lib/packets.c   | 22 +++++++++++++++++++++
 lib/packets.h   |  2 ++
 3 files changed, 69 insertions(+), 6 deletions(-)

Comments

Mike Pattrick June 5, 2023, 7:18 p.m. UTC | #1 
On Wed, Dec 14, 2022 at 10:50 PM <wushaohua@chinatelecom.cn> wrote:
>
> 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>

Hello wushaohua,

Unfortunately, this patch no longer applies cleanly to the master
branch due to other changes that have been applied. But It's quite
simple to  rebase with a 3-way merge.

This patch does resolve the issue you identified. I think it could be
improved by adding a unit tests, I've included an example below. Other
than that, it looks good to me.


Cheers,
M

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 4c378e1d0..abcd1d8fe 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -5497,6 +5497,41 @@
tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP

+AT_SETUP([conntrack - SNAT with icmp_id])
+CHECK_CONNTRACK()
+CHECK_CONNTRACK_NAT()
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+NS_CHECK_EXEC([at_ns0], [ip link set dev p0 address 80:88:88:88:88:88])
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
+
+dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic
from ns1->ns0.
+AT_DATA([flows.txt], [dnl
+in_port=1,ip,action=ct(commit,zone=1,nat(src=10.1.1.240)),2
+in_port=2,ct_state=-trk,ip,action=ct(table=0,zone=1,nat)
+in_port=2,ct_state=+trk,ct_zone=1,ip,action=1
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+ovs-appctl dpctl/flush-conntrack
+
+ovs-ofctl packet-out br0
"in_port=ovs-p0,packet=e2b2c2a8b8c8e2b2c2a9b9c908004500001c00010000400164dc0a0101020a0101010800f60a01f40001
actions=resubmit(,0)"
+ovs-ofctl packet-out br0
"in_port=ovs-p0,packet=e2b2c2a8b8c8e2b2c2a9b9c908004500001c00010000400164db0a0101030a0101010800f60a01f40001
actions=resubmit(,0)"
+ovs-ofctl packet-out br0
"in_port=ovs-p0,packet=e2b2c2a8b8c8e2b2c2a9b9c908004500001c00010000400164da0a0101040a0101010800f60a01f40001
actions=resubmit(,0)"
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | sort], [0], [dnl
+icmp,orig=(src=10.1.1.2,dst=10.1.1.1,id=500,type=8,code=0),reply=(src=10.1.1.1,dst=10.1.1.240,id=500,type=0,code=0),zone=1
+icmp,orig=(src=10.1.1.3,dst=10.1.1.1,id=500,type=8,code=0),reply=(src=10.1.1.1,dst=10.1.1.240,id=501,type=0,code=0),zone=1
+icmp,orig=(src=10.1.1.4,dst=10.1.1.1,id=500,type=8,code=0),reply=(src=10.1.1.1,dst=10.1.1.240,id=502,type=0,code=0),zone=1
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+


> ---
>  lib/conntrack.c | 51 +++++++++++++++++++++++++++++++++++++++++++------
>  lib/packets.c   | 22 +++++++++++++++++++++
>  lib/packets.h   |  2 ++
>  3 files changed, 69 insertions(+), 6 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 550b2be9b..a5d3e6a3f 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -697,6 +697,24 @@ get_ip_proto(const struct dp_packet *pkt)
>      return ip_proto;
>  }
>
> +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);
> +        if (icmp_type == ICMP4_ECHO_REQUEST ||
> +            icmp_type == ICMP4_ECHO_REPLY ||
> +            icmp_type == ICMP4_TIMESTAMP ||
> +            icmp_type == ICMP4_TIMESTAMPREPLY ||
> +            icmp_type == ICMP4_INFOREQUEST ||
> +            icmp_type == ICMP4_INFOREPLY)
> +            return true;
> +    }
> +    return false;
> +}
>  static bool
>  is_ftp_ctl(const enum ct_alg_ctl_type ct_alg_ctl)
>  {
> @@ -773,6 +791,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 +802,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 +855,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 +2396,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 +2418,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 +2434,9 @@ another_round:
>      }
>
>      *port = htons(orig);
> +     if (peer_port) {
> +         *peer_port = htons(orig);
> +     }
>
>      return false;
>  }
> @@ -2434,7 +2470,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 +2506,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,
> --
> 2.30.2
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
wushaohua@chinatelecom.cn July 2, 2023, 8:03 a.m. UTC | #2 
Hi Mike，
Thank you for your review. I have followed your advice and added unit tests as suggested.

Best wishes
shaohua



 
From: Mike Pattrick
Date: 2023-06-06 03:18
To: wushaohua
CC: dev; ctyun-rd1-INF-Network
Subject: Re: [ovs-dev] [PATCH v2] conntrack: Fix icmp_id conflicts in snat
On Wed, Dec 14, 2022 at 10:50 PM <wushaohua@chinatelecom.cn> wrote:
>
> 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>
 
Hello wushaohua,
 
Unfortunately, this patch no longer applies cleanly to the master
branch due to other changes that have been applied. But It's quite
simple to  rebase with a 3-way merge.
 
This patch does resolve the issue you identified. I think it could be
improved by adding a unit tests, I've included an example below. Other
than that, it looks good to me.
 
 
Cheers,
M
 
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 4c378e1d0..abcd1d8fe 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -5497,6 +5497,41 @@
tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
 
+AT_SETUP([conntrack - SNAT with icmp_id])
+CHECK_CONNTRACK()
+CHECK_CONNTRACK_NAT()
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+NS_CHECK_EXEC([at_ns0], [ip link set dev p0 address 80:88:88:88:88:88])
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
+
+dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic
from ns1->ns0.
+AT_DATA([flows.txt], [dnl
+in_port=1,ip,action=ct(commit,zone=1,nat(src=10.1.1.240)),2
+in_port=2,ct_state=-trk,ip,action=ct(table=0,zone=1,nat)
+in_port=2,ct_state=+trk,ct_zone=1,ip,action=1
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+ovs-appctl dpctl/flush-conntrack
+
+ovs-ofctl packet-out br0
"in_port=ovs-p0,packet=e2b2c2a8b8c8e2b2c2a9b9c908004500001c00010000400164dc0a0101020a0101010800f60a01f40001
actions=resubmit(,0)"
+ovs-ofctl packet-out br0
"in_port=ovs-p0,packet=e2b2c2a8b8c8e2b2c2a9b9c908004500001c00010000400164db0a0101030a0101010800f60a01f40001
actions=resubmit(,0)"
+ovs-ofctl packet-out br0
"in_port=ovs-p0,packet=e2b2c2a8b8c8e2b2c2a9b9c908004500001c00010000400164da0a0101040a0101010800f60a01f40001
actions=resubmit(,0)"
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | sort], [0], [dnl
+icmp,orig=(src=10.1.1.2,dst=10.1.1.1,id=500,type=8,code=0),reply=(src=10.1.1.1,dst=10.1.1.240,id=500,type=0,code=0),zone=1
+icmp,orig=(src=10.1.1.3,dst=10.1.1.1,id=500,type=8,code=0),reply=(src=10.1.1.1,dst=10.1.1.240,id=501,type=0,code=0),zone=1
+icmp,orig=(src=10.1.1.4,dst=10.1.1.1,id=500,type=8,code=0),reply=(src=10.1.1.1,dst=10.1.1.240,id=502,type=0,code=0),zone=1
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 
 
> ---
>  lib/conntrack.c | 51 +++++++++++++++++++++++++++++++++++++++++++------
>  lib/packets.c   | 22 +++++++++++++++++++++
>  lib/packets.h   |  2 ++
>  3 files changed, 69 insertions(+), 6 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 550b2be9b..a5d3e6a3f 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -697,6 +697,24 @@ get_ip_proto(const struct dp_packet *pkt)
>      return ip_proto;
>  }
>
> +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);
> +        if (icmp_type == ICMP4_ECHO_REQUEST ||
> +            icmp_type == ICMP4_ECHO_REPLY ||
> +            icmp_type == ICMP4_TIMESTAMP ||
> +            icmp_type == ICMP4_TIMESTAMPREPLY ||
> +            icmp_type == ICMP4_INFOREQUEST ||
> +            icmp_type == ICMP4_INFOREPLY)
> +            return true;
> +    }
> +    return false;
> +}
>  static bool
>  is_ftp_ctl(const enum ct_alg_ctl_type ct_alg_ctl)
>  {
> @@ -773,6 +791,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 +802,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 +855,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 +2396,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 +2418,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 +2434,9 @@ another_round:
>      }
>
>      *port = htons(orig);
> +     if (peer_port) {
> +         *peer_port = htons(orig);
> +     }
>
>      return false;
>  }
> @@ -2434,7 +2470,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 +2506,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,
> --
> 2.30.2
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 550b2be9b..a5d3e6a3f 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -697,6 +697,24 @@  get_ip_proto(const struct dp_packet *pkt)
     return ip_proto;
 }
 
+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);
+        if (icmp_type == ICMP4_ECHO_REQUEST ||
+            icmp_type == ICMP4_ECHO_REPLY ||
+            icmp_type == ICMP4_TIMESTAMP ||
+            icmp_type == ICMP4_TIMESTAMPREPLY ||
+            icmp_type == ICMP4_INFOREQUEST ||
+            icmp_type == ICMP4_INFOREPLY)
+            return true;
+    }
+    return false;
+}
 static bool
 is_ftp_ctl(const enum ct_alg_ctl_type ct_alg_ctl)
 {
@@ -773,6 +791,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 +802,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 +855,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 +2396,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 +2418,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 +2434,9 @@  another_round:
     }
 
     *port = htons(orig);
+     if (peer_port) {
+         *peer_port = htons(orig);
+     }
 
     return false;
 }
@@ -2434,7 +2470,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 +2506,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,