Darrell Ball wrote: > 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 >
Thanks for your quick ack. I have seen the accepted patch-set in the upstream. -- Thanks Solomon > Darrell > > > On Mon, Feb 25, 2019 at 7:03 AM solomon <liwei.solo...@gmail.com> 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 <liwei.solo...@gmail.com> >> --- >> 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 d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev