tcp/dccp: replace using only even ports with all ports

Commit Message

xiaolinkui July 22, 2024, 9:41 a.m. UTC

From: Linkui Xiao <[e-mail]>

In commit 207184853dbd ("tcp/dccp: change source port selection at connect()
time"), the purpose is to address the issue of increased costs when all even
ports are in use.

But in my testing environment, this more cost issue has not been resolved.

The testing environment is as follows:
1. build an HTTP server(http://192.168.55.1:9999/);
2. on the client side, use the ab command to test the number of connections,
then kill it and simulate a large number of TIME-WAIT connections:

TARGET_TIME_WAIT=16384
CONCURRENCY=20000
MAX_CONCURRENCY=20000
MIN_CONCURRENCY=5000

while true; do
  CURRENT_TIME_WAIT=$(ss -tanp | grep TIME-WAIT | wc -l)
  echo "Current TIME_WAIT connections: $CURRENT_TIME_WAIT"

  if [ "$CURRENT_TIME_WAIT" -lt "$TARGET_TIME_WAIT" ]; then
    if [ "$CONCURRENCY" -lt "$MAX_CONCURRENCY" ]; then
      CONCURRENCY=$((CONCURRENCY + 5000))
      if [ "$CONCURRENCY" -gt "$MAX_CONCURRENCY" ]; then
        CONCURRENCY=$MAX_CONCURRENCY
      fi
      echo "Increasing concurrency to: $CONCURRENCY"
    fi
  elif [ "$CURRENT_TIME_WAIT" -gt "$TARGET_TIME_WAIT" ]; then
    if [ "$CONCURRENCY" -gt "$MIN_CONCURRENCY" ]; then
      CONCURRENCY=$((CONCURRENCY - 5000))
      if [ "$CONCURRENCY" -lt "$MIN_CONCURRENCY" ]; then
        CONCURRENCY=$MIN_CONCURRENCY
      fi
      echo "Decreasing concurrency to: $CONCURRENCY"
    fi
  fi

  ab -r -n 100000 -c "$CONCURRENCY" http://192.168.55.1:9999/ &

  AB_PID=$!
  sleep 1
  kill $AB_PID
  sleep 1
done

On the client side, use the command "mpstat - P ALL 1" to monitor the load
situation.It can be observed that the load of %sys decreased by about 50%
after patching.

Signed-off-by: Linkui Xiao <[e-mail]>
---
 net/ipv4/inet_hashtables.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Eric Dumazet July 22, 2024, 2:50 p.m. UTC | #1

On Mon, Jul 22, 2024 at 2:41 AM <[e-mail]> wrote:
>
> From: Linkui Xiao <[e-mail]>
>
> In commit 207184853dbd ("tcp/dccp: change source port selection at connect()
> time"), the purpose is to address the issue of increased costs when all even
> ports are in use.
>
> But in my testing environment, this more cost issue has not been resolved.

You missed the whole point of 1580ab63fc9a ("tcp/dccp: better use of
ephemeral ports in connect()")

Have you read 207184853dbd ("tcp/dccp: change source port selection at
connect() ..." changelog and are you using IP_LOCAL_PORT_RANGE ?

Eric Dumazet July 25, 2024, 7:32 a.m. UTC | #2

On Thu, Jul 25, 2024 at 9:07 AM xiaolinkui <[e-mail]> wrote:
>
> Thank you for your reply．
>
> At 2024-07-22 21:50:39, "Eric Dumazet" <[e-mail]> wrote:
> >On Mon, Jul 22, 2024 at 2:41 AM <[e-mail]> wrote:
> >>
> >> From: Linkui Xiao <[e-mail]>
> >>
> >> In commit 207184853dbd ("tcp/dccp: change source port selection at connect()
> >> time"), the purpose is to address the issue of increased costs when all even
> >> ports are in use.
> >>
> >> But in my testing environment, this more cost issue has not been resolved.
> >
> >You missed the whole point of 1580ab63fc9a ("tcp/dccp: better use of
> >ephemeral ports in connect()")
> >
> >Have you read 207184853dbd ("tcp/dccp: change source port selection at
> >connect() ..." changelog and are you using IP_LOCAL_PORT_RANGE ?
>
> There seems to be some difference between IP_LOCAL_PORT_RANGE
> and "sysctl net.ipv4.ip_local_port_range".We can use the following system
> calls at the user layer to use IP_LOCAL_PORT_RANGE:
> setsockopt(sockfd, IPPROTO_IP, IP_LOCAL_PORT_RANGE, &opt, sizeof(opt));
>
> But user behavior is uncontrollable．Is there any other way to use IP_LOCAL_PORT_RANGE？

If user behavior can not be changed, this is on their end.

Sorry, we won't accept a patch going to the terrible situation we had
before, where applications would fail completely in many cases.

Patch

diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 48d0d494185b..4192531ba2d3 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -1007,7 +1007,7 @@  int __inet_hash_connect(struct inet_timewait_death_row *death_row,
 	u32 remaining, offset;
 	int ret, i, low, high;
 	bool local_ports;
-	int step, l3mdev;
+	int l3mdev;
 	u32 index;
 
 	if (port) {
@@ -1020,7 +1020,6 @@  int __inet_hash_connect(struct inet_timewait_death_row *death_row,
 	l3mdev = inet_sk_bound_l3mdev(sk);
 
 	local_ports = inet_sk_get_local_port_range(sk, &low, &high);
-	step = local_ports ? 1 : 2;
 
 	high++; /* [32768, 60999] -> [32768, 61000[ */
 	remaining = high - low;
@@ -1041,7 +1040,7 @@  int __inet_hash_connect(struct inet_timewait_death_row *death_row,
 		offset &= ~1U;
 other_parity_scan:
 	port = low + offset;
-	for (i = 0; i < remaining; i += step, port += step) {
+	for (i = 0; i < remaining; i += 1, port += 1) {
 		if (unlikely(port >= high))
 			port -= remaining;
 		if (inet_is_local_reserved_port(net, port))
@@ -1108,8 +1107,8 @@  int __inet_hash_connect(struct inet_timewait_death_row *death_row,
 	 * on low contention the randomness is maximal and on high contention
 	 * it may be inexistent.
 	 */
-	i = max_t(int, i, get_random_u32_below(8) * step);
-	WRITE_ONCE(table_perturb[index], READ_ONCE(table_perturb[index]) + i + step);
+	i = max_t(int, i, get_random_u32_below(8) * 1);
+	WRITE_ONCE(table_perturb[index], READ_ONCE(table_perturb[index]) + i + 1);
 
 	/* Head lock still held and bh's disabled */
 	inet_bind_hash(sk, tb, tb2, port);