| Message ID | 1639453166-27494-2-git-send-email-wenxu@ucloud.cn |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v8,1/3] conntrack: select correct sport range for well-known origin sport | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
wenxu@ucloud.cn writes: > From: wenxu <wenxu@ucloud.cn> > > Splits the nested loop used to search the unique ports for the > reverse tuple. > It affects only the dnat action, giving more precedence to the dnat > range, similarly to the kernel dp, instead of searching through the > default ephemeral source range for each destination port. > > Signed-off-by: wenxu <wenxu@ucloud.cn> > --- I see you changed the patch a bit (below the initial draft shared off-list). My personal preference is to keep the function calls, if possible. Let's see what's other's opinion about it. -- >8 -- Subject: [PATCH] conntrack: Prefer dst port range during unique tuple search. This commit splits the nested loop used to search the unique ports for the reverse tuple. It affects only the dnat action, giving more precedence to the dnat range, similarly to the kernel dp, instead of searching through the default ephemeral source range for each destination port. Signed-off-by: Paolo Valerio <pvalerio@redhat.com> --- lib/conntrack.c | 49 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index 33a1a9295..fec9f288a 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -2389,6 +2389,22 @@ next_addr_in_range_guarded(union ct_addr *curr, union ct_addr *min, return exhausted; } +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) +{ + FOR_EACH_PORT_IN_RANGE(curr, min, max) { + *port = htons(curr); + if (!conn_lookup(ct, &nat_conn->rev_key, + time_msec(), NULL, NULL)) { + return true; + } + } + + return false; +} + /* This function tries to get a unique tuple. * Every iteration checks that the reverse tuple doesn't * collide with any existing one. @@ -2403,9 +2419,11 @@ next_addr_in_range_guarded(union ct_addr *curr, union ct_addr *min, * * In case of DNAT: * - For each dst IP address in the range (if any). - * - For each dport in range (if any). - * - Try to find a source port in the ephemeral range - * (after testing the port used by the sender). + * - For each dport in range (if any) tries to find + * an unique tuple. + * - Eventually, if the previous attempt fails, + * tries to find a source port in the ephemeral + * range (after testing the port used by the sender). * * If none can be found, return exhaustion to the caller. */ static bool @@ -2449,15 +2467,22 @@ another_round: goto next_addr; } - FOR_EACH_PORT_IN_RANGE(curr_dport, min_dport, max_dport) { - nat_conn->rev_key.src.port = htons(curr_dport); - FOR_EACH_PORT_IN_RANGE(curr_sport, min_sport, max_sport) { - nat_conn->rev_key.dst.port = htons(curr_sport); - if (!conn_lookup(ct, &nat_conn->rev_key, - time_msec(), NULL, NULL)) { - return true; - } - } + nat_conn->rev_key.src.port = htons(curr_dport); + nat_conn->rev_key.dst.port = htons(curr_sport); + + 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); + } + + if (!found) { + found = nat_get_unique_l4(ct, nat_conn, &nat_conn->rev_key.dst.port, + curr_sport, min_sport, max_sport); + } + + if (found) { + return true; } /* Check if next IP is in range and respin. Otherwise, notify
diff --git a/lib/conntrack.c b/lib/conntrack.c index 44f99f3..2a5d72a 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -2411,9 +2411,11 @@ next_addr_in_range_guarded(union ct_addr *curr, union ct_addr *min, * * In case of DNAT: * - For each dst IP address in the range (if any). - * - For each dport in range (if any). - * - Try to find a source port in the ephemeral range - * (after testing the port used by the sender). + * - For each dport in range (if any) tries to find + * an unique tuple. + * - Eventually, if the previous attempt fails, + * tries to find a source port in the ephemeral + * range (after testing the port used by the sender). * * If none can be found, return exhaustion to the caller. */ static bool @@ -2457,10 +2459,9 @@ another_round: goto next_addr; } - FOR_EACH_PORT_IN_RANGE(curr_dport, min_dport, max_dport) { - nat_conn->rev_key.src.port = htons(curr_dport); - FOR_EACH_PORT_IN_RANGE(curr_sport, min_sport, max_sport) { - nat_conn->rev_key.dst.port = htons(curr_sport); + if (nat_info->nat_action & NAT_ACTION_DST_PORT) { + FOR_EACH_PORT_IN_RANGE(curr_dport, min_dport, max_dport) { + nat_conn->rev_key.src.port = htons(curr_dport); if (!conn_lookup(ct, &nat_conn->rev_key, time_msec(), NULL, NULL)) { return true; @@ -2468,6 +2469,14 @@ another_round: } } + FOR_EACH_PORT_IN_RANGE(curr_sport, min_sport, max_sport) { + nat_conn->rev_key.dst.port = htons(curr_sport); + if (!conn_lookup(ct, &nat_conn->rev_key, + time_msec(), NULL, NULL)) { + return true; + } + } + /* Check if next IP is in range and respin. Otherwise, notify * exhaustion to the caller. */ next_addr: