Thanks for the report

I think there are two separate issues:
1/ Fallback to ephemeral ports for SNAT being less restrictive than in
kernel
2/ Wasted local variable port incrementing work for ICMPv4/v6

I sent an alternative series here:
https://mail.openvswitch.org/pipermail/ovs-dev/2019-February/356654.html

Darrell


On Mon, Feb 25, 2019 at 7:03 AM solomon <[email protected]> wrote:

>
> If setting the port range in SNAT, we expect that the selected port is in
> the range we set.
> At the same time, this behavior is consistent with the kernel-datapath.
>
> The port has no meaning for the icmp/icmpv6 protocol.
> If no key is available, it will exit early.
>
> Signed-off-by: LiWei <[email protected]>
> ---
>  lib/conntrack.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 4028ba9a0..c3fd7d63d 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -2115,6 +2115,12 @@ nat_range_hash(const struct conn *conn, uint32_t
> basis)
>      return hash_finish(hash, 0);
>  }
>
> +/* This function selects a unique new connection key based on the ip and
> port
> + * settings in the nat tuple. For the case where the port range is not
> + * specified, the ephemeral port from 1024 to 65535 can be selected when
> the
> + * address is exhausted.
> + * However, the ephemeral ports are not enabled by ICMP/ICMPv6 and DNAT.
> + */
>  static bool
>  nat_select_range_tuple(struct conntrack *ct, const struct conn *conn,
>                         struct conn *nat_conn)
> @@ -2126,12 +2132,14 @@ nat_select_range_tuple(struct conntrack *ct, const
> struct conn *conn,
>      uint16_t max_port;
>      uint16_t first_port;
>      uint32_t hash = nat_range_hash(conn, ct->hash_basis);
> +    bool ephemeral_ports_tried = true;
>
>      if ((conn->nat_info->nat_action & NAT_ACTION_SRC) &&
>          (!(conn->nat_info->nat_action & NAT_ACTION_SRC_PORT))) {
>          min_port = ntohs(conn->key.src.port);
>          max_port = ntohs(conn->key.src.port);
>          first_port = min_port;
> +        ephemeral_ports_tried = false;
>      } else if ((conn->nat_info->nat_action & NAT_ACTION_DST) &&
>                 (!(conn->nat_info->nat_action & NAT_ACTION_DST_PORT))) {
>          min_port = ntohs(conn->key.dst.port);
> @@ -2175,9 +2183,6 @@ nat_select_range_tuple(struct conntrack *ct, const
> struct conn *conn,
>
>      uint16_t port = first_port;
>      bool all_ports_tried = false;
> -    /* For DNAT, we don't use ephemeral ports. */
> -    bool ephemeral_ports_tried = conn->nat_info->nat_action &
> NAT_ACTION_DST
> -                                 ? true : false;
>      union ct_addr first_addr = ct_addr;
>
>      while (true) {
> @@ -2190,6 +2195,7 @@ nat_select_range_tuple(struct conntrack *ct, const
> struct conn *conn,
>          if ((conn->key.nw_proto == IPPROTO_ICMP) ||
>              (conn->key.nw_proto == IPPROTO_ICMPV6)) {
>              all_ports_tried = true;
> +            ephemeral_ports_tried = true;
>          } else if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
>              nat_conn->rev_key.dst.port = htons(port);
>          } else {
> --
> 2.11.0
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to