diff mbox series

[mptcp-next,1/2] mptcp: sockopt: support IP_LOCAL_PORT_RANGE and IP_BIND_ADDRESS_NO_PORT

Message ID 20231122155919.11393-2-max@internet.ru (mailing list archive)
State Superseded, archived
Headers show
Series more sockopts for ephemeral ports | expand

Checks

Context Check Description
matttbe/build success Build and static analysis OK
matttbe/checkpatch fail total: 4 errors, 0 warnings, 0 checks, 75 lines checked
matttbe/KVM_Validation__normal__except_selftest_mptcp_join_ success Success! ✅
matttbe/KVM_Validation__normal__only_selftest_mptcp_join_ success Success! ✅
matttbe/KVM_Validation__debug__only_selftest_mptcp_join_ success Success! ✅
matttbe/KVM_Validation__debug__except_selftest_mptcp_join_ warning Unstable: 1 failed test(s): packetdrill_sockopts

Commit Message

Maxim Galaganov Nov. 22, 2023, 3:59 p.m. UTC
Support for IP_BIND_ADDRESS_NO_PORT sockopt was introduced in
90c337da1524 ("inet: add IP_BIND_ADDRESS_NO_PORT to overcome bind(0)
limitations"). Recently ca571e2eb7eb ("inet: move
inet->bind_address_no_port to inet->inet_flags") allowed its value to be
accessed without locking the socket.

Support for (newer) IP_LOCAL_PORT_RANGE sockopt was introduced in
91d0b78c5177 ("inet: Add IP_LOCAL_PORT_RANGE socket option"). In the
same series a selftest was added in ae5439658cce ("selftests/net: Cover
the IP_LOCAL_PORT_RANGE socket option"). This selftest also covers the
IP_BIND_ADDRESS_NO_PORT sockopt.

This patch enables getsockopt()/setsockopt() on MPTCP sockets for these
socket options, syncing set values to subflows in sync_socket_options().
Ephemeral port range is synced to subflows, enabling NAT usecase
described in 91d0b78c5177.

Signed-off-by: Maxim Galaganov <max@internet.ru>
---
 net/mptcp/sockopt.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

Comments

Mat Martineau Nov. 29, 2023, 1:15 a.m. UTC | #1
On Wed, 22 Nov 2023, Maxim Galaganov wrote:

> Support for IP_BIND_ADDRESS_NO_PORT sockopt was introduced in

Hi Maxim -

Thanks for the patches!

> 90c337da1524 ("inet: add IP_BIND_ADDRESS_NO_PORT to overcome bind(0)
> limitations"). Recently ca571e2eb7eb ("inet: move
> inet->bind_address_no_port to inet->inet_flags") allowed its value to be
> accessed without locking the socket.

checkpatch.pl is failing in the CI since it expects the word "commit" 
before each abbreviated hash. Please add that in each of the cases where 
there's a <sha> ("title") pattern.

>
> Support for (newer) IP_LOCAL_PORT_RANGE sockopt was introduced in
> 91d0b78c5177 ("inet: Add IP_LOCAL_PORT_RANGE socket option"). In the
> same series a selftest was added in ae5439658cce ("selftests/net: Cover
> the IP_LOCAL_PORT_RANGE socket option"). This selftest also covers the
> IP_BIND_ADDRESS_NO_PORT sockopt.
>
> This patch enables getsockopt()/setsockopt() on MPTCP sockets for these
> socket options, syncing set values to subflows in sync_socket_options().
> Ephemeral port range is synced to subflows, enabling NAT usecase
> described in 91d0b78c5177.
>
> Signed-off-by: Maxim Galaganov <max@internet.ru>
> ---
> net/mptcp/sockopt.c | 29 ++++++++++++++++++++++++++++-
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index cabe856b2a45..53f27354412f 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -440,6 +440,8 @@ static bool mptcp_supported_sockopt(int level, int optname)
> 		/* should work fine */
> 		case IP_FREEBIND:
> 		case IP_TRANSPARENT:
> +		case IP_BIND_ADDRESS_NO_PORT:
> +		case IP_LOCAL_PORT_RANGE:
>
> 		/* the following are control cmsg related */
> 		case IP_PKTINFO:
> @@ -455,7 +457,6 @@ static bool mptcp_supported_sockopt(int level, int optname)
> 		/* common stuff that need some love */
> 		case IP_TOS:
> 		case IP_TTL:
> -		case IP_BIND_ADDRESS_NO_PORT:
> 		case IP_MTU_DISCOVER:
> 		case IP_RECVERR:
>
> @@ -688,6 +689,7 @@ static int mptcp_setsockopt_sol_ip_set_transparent(struct mptcp_sock *msk, int o

This function is no longer specific to IP_TRANSPARENT (and never really 
was), can you add a 3rd patch renaming it to 
mptcp_setsockopt_sol_ip_set()?

> {
> 	struct sock *sk = (struct sock *)msk;
> 	struct sock *ssk;
> +	bool slow;
> 	int err;
>
> 	err = ip_setsockopt(sk, SOL_IP, optname, optval, optlen);

The code otherwise looks good to me. The port range could already be 
configured by sysctl, so there don't appear to be any concerns with the 
path managers handling a non-default range. And the options are validated 
here in the call to ip_setsockopt().

Thanks,

Mat

> @@ -710,6 +712,16 @@ static int mptcp_setsockopt_sol_ip_set_transparent(struct mptcp_sock *msk, int o
> 		inet_assign_bit(TRANSPARENT, ssk,
> 				inet_test_bit(TRANSPARENT, sk));
> 		break;
> +	case IP_BIND_ADDRESS_NO_PORT:
> +		inet_assign_bit(BIND_ADDRESS_NO_PORT, ssk,
> +				inet_test_bit(BIND_ADDRESS_NO_PORT, sk));
> +		break;
> +	case IP_LOCAL_PORT_RANGE:
> +		slow = lock_sock_fast(ssk);
> +		inet_sk(ssk)->local_port_range.lo = inet_sk(sk)->local_port_range.lo;
> +		inet_sk(ssk)->local_port_range.hi = inet_sk(sk)->local_port_range.hi;
> +		unlock_sock_fast(ssk, slow);
> +		break;
> 	default:
> 		release_sock(sk);
> 		WARN_ON_ONCE(1);
> @@ -755,6 +767,8 @@ static int mptcp_setsockopt_v4(struct mptcp_sock *msk, int optname,
> 	switch (optname) {
> 	case IP_FREEBIND:
> 	case IP_TRANSPARENT:
> +	case IP_BIND_ADDRESS_NO_PORT:
> +	case IP_LOCAL_PORT_RANGE:
> 		return mptcp_setsockopt_sol_ip_set_transparent(msk, optname, optval, optlen);
> 	case IP_TOS:
> 		return mptcp_setsockopt_v4_set_tos(msk, optname, optval, optlen);
> @@ -1346,10 +1360,20 @@ static int mptcp_getsockopt_v4(struct mptcp_sock *msk, int optname,
> 			       char __user *optval, int __user *optlen)
> {
> 	struct sock *sk = (void *)msk;
> +	bool slow;
> +	int val;
>
> 	switch (optname) {
> 	case IP_TOS:
> 		return mptcp_put_int_option(msk, optval, optlen, READ_ONCE(inet_sk(sk)->tos));
> +	case IP_BIND_ADDRESS_NO_PORT:
> +		return mptcp_put_int_option(msk, optval, optlen,
> +				inet_test_bit(BIND_ADDRESS_NO_PORT, sk));
> +	case IP_LOCAL_PORT_RANGE:
> +		slow = lock_sock_fast(sk);
> +		val = inet_sk(sk)->local_port_range.hi << 16 | inet_sk(sk)->local_port_range.lo;
> +		unlock_sock_fast(sk, slow);
> +		return mptcp_put_int_option(msk, optval, optlen, val);
> 	}
>
> 	return -EOPNOTSUPP;
> @@ -1450,6 +1474,9 @@ static void sync_socket_options(struct mptcp_sock *msk, struct sock *ssk)
>
> 	inet_assign_bit(TRANSPARENT, ssk, inet_test_bit(TRANSPARENT, sk));
> 	inet_assign_bit(FREEBIND, ssk, inet_test_bit(FREEBIND, sk));
> +	inet_assign_bit(BIND_ADDRESS_NO_PORT, ssk, inet_test_bit(BIND_ADDRESS_NO_PORT, sk));
> +	inet_sk(ssk)->local_port_range.lo = inet_sk(sk)->local_port_range.lo;
> +	inet_sk(ssk)->local_port_range.hi = inet_sk(sk)->local_port_range.hi;
> }
>
> void mptcp_sockopt_sync_locked(struct mptcp_sock *msk, struct sock *ssk)
> -- 
> 2.42.0
>
>
>
diff mbox series

Patch

diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index cabe856b2a45..53f27354412f 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -440,6 +440,8 @@  static bool mptcp_supported_sockopt(int level, int optname)
 		/* should work fine */
 		case IP_FREEBIND:
 		case IP_TRANSPARENT:
+		case IP_BIND_ADDRESS_NO_PORT:
+		case IP_LOCAL_PORT_RANGE:
 
 		/* the following are control cmsg related */
 		case IP_PKTINFO:
@@ -455,7 +457,6 @@  static bool mptcp_supported_sockopt(int level, int optname)
 		/* common stuff that need some love */
 		case IP_TOS:
 		case IP_TTL:
-		case IP_BIND_ADDRESS_NO_PORT:
 		case IP_MTU_DISCOVER:
 		case IP_RECVERR:
 
@@ -688,6 +689,7 @@  static int mptcp_setsockopt_sol_ip_set_transparent(struct mptcp_sock *msk, int o
 {
 	struct sock *sk = (struct sock *)msk;
 	struct sock *ssk;
+	bool slow;
 	int err;
 
 	err = ip_setsockopt(sk, SOL_IP, optname, optval, optlen);
@@ -710,6 +712,16 @@  static int mptcp_setsockopt_sol_ip_set_transparent(struct mptcp_sock *msk, int o
 		inet_assign_bit(TRANSPARENT, ssk,
 				inet_test_bit(TRANSPARENT, sk));
 		break;
+	case IP_BIND_ADDRESS_NO_PORT:
+		inet_assign_bit(BIND_ADDRESS_NO_PORT, ssk,
+				inet_test_bit(BIND_ADDRESS_NO_PORT, sk));
+		break;
+	case IP_LOCAL_PORT_RANGE:
+		slow = lock_sock_fast(ssk);
+		inet_sk(ssk)->local_port_range.lo = inet_sk(sk)->local_port_range.lo;
+		inet_sk(ssk)->local_port_range.hi = inet_sk(sk)->local_port_range.hi;
+		unlock_sock_fast(ssk, slow);
+		break;
 	default:
 		release_sock(sk);
 		WARN_ON_ONCE(1);
@@ -755,6 +767,8 @@  static int mptcp_setsockopt_v4(struct mptcp_sock *msk, int optname,
 	switch (optname) {
 	case IP_FREEBIND:
 	case IP_TRANSPARENT:
+	case IP_BIND_ADDRESS_NO_PORT:
+	case IP_LOCAL_PORT_RANGE:
 		return mptcp_setsockopt_sol_ip_set_transparent(msk, optname, optval, optlen);
 	case IP_TOS:
 		return mptcp_setsockopt_v4_set_tos(msk, optname, optval, optlen);
@@ -1346,10 +1360,20 @@  static int mptcp_getsockopt_v4(struct mptcp_sock *msk, int optname,
 			       char __user *optval, int __user *optlen)
 {
 	struct sock *sk = (void *)msk;
+	bool slow;
+	int val;
 
 	switch (optname) {
 	case IP_TOS:
 		return mptcp_put_int_option(msk, optval, optlen, READ_ONCE(inet_sk(sk)->tos));
+	case IP_BIND_ADDRESS_NO_PORT:
+		return mptcp_put_int_option(msk, optval, optlen,
+				inet_test_bit(BIND_ADDRESS_NO_PORT, sk));
+	case IP_LOCAL_PORT_RANGE:
+		slow = lock_sock_fast(sk);
+		val = inet_sk(sk)->local_port_range.hi << 16 | inet_sk(sk)->local_port_range.lo;
+		unlock_sock_fast(sk, slow);
+		return mptcp_put_int_option(msk, optval, optlen, val);
 	}
 
 	return -EOPNOTSUPP;
@@ -1450,6 +1474,9 @@  static void sync_socket_options(struct mptcp_sock *msk, struct sock *ssk)
 
 	inet_assign_bit(TRANSPARENT, ssk, inet_test_bit(TRANSPARENT, sk));
 	inet_assign_bit(FREEBIND, ssk, inet_test_bit(FREEBIND, sk));
+	inet_assign_bit(BIND_ADDRESS_NO_PORT, ssk, inet_test_bit(BIND_ADDRESS_NO_PORT, sk));
+	inet_sk(ssk)->local_port_range.lo = inet_sk(sk)->local_port_range.lo;
+	inet_sk(ssk)->local_port_range.hi = inet_sk(sk)->local_port_range.hi;
 }
 
 void mptcp_sockopt_sync_locked(struct mptcp_sock *msk, struct sock *ssk)