[PATCH v2 net-next] net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.

[PATCH v2 net-next] net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.

[Kuniyuki Iwashima]

Recently, commit 8e5443d2b866 ("net: remove SOCK_DEBUG leftovers")
removed the last users of SOCK_DEBUG(), and commit b1dffcf0da22 ("net:
remove SOCK_DEBUG macro") removed the macro.

Now is the time to deprecate the oldest socket option.

Note that setsockopt(SO_DEBUG) is moved not to acquire lock_sock().

Reviewed-by: Gerd Bayer <gbayer@linux.ibm.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
v2:
  * Move setsockopt(SO_DEBUG) code not to acquire lock_sock().

v1: https://lore.kernel.org/netdev/20240213223135.85957-1-kuniyu@amazon.com/
---
 include/net/sock.h  |  1 -
 net/core/sock.c     | 14 +++++++-------
 net/mptcp/sockopt.c |  8 +-------
 net/smc/af_smc.c    |  5 ++---
 4 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index a9d99a9c583f..e20d55a36f9c 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -909,7 +909,6 @@ enum sock_flags {
 	SOCK_TIMESTAMP,
 	SOCK_ZAPPED,
 	SOCK_USE_WRITE_QUEUE, /* whether to call sk->sk_write_space in sock_wfree */
-	SOCK_DBG, /* %SO_DEBUG setting */
 	SOCK_RCVTSTAMP, /* %SO_TIMESTAMP setting */
 	SOCK_RCVTSTAMPNS, /* %SO_TIMESTAMPNS setting */
 	SOCK_LOCALROUTE, /* route locally only, %SO_DONTROUTE setting */
diff --git a/net/core/sock.c b/net/core/sock.c
index 88bf810394a5..c4c406f4742e 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1115,6 +1115,11 @@ int sk_setsockopt(struct sock *sk, int level, int optname,
 
 	/* handle options which do not require locking the socket. */
 	switch (optname) {
+	case SO_DEBUG:
+		/* deprecated, but kept for compatibility */
+		if (val && !sockopt_capable(CAP_NET_ADMIN))
+			ret = -EACCES;
+		return 0;
 	case SO_PRIORITY:
 		if ((val >= 0 && val <= 6) ||
 		    sockopt_ns_capable(sock_net(sk)->user_ns, CAP_NET_RAW) ||
@@ -1193,12 +1198,6 @@ int sk_setsockopt(struct sock *sk, int level, int optname,
 	sockopt_lock_sock(sk);
 
 	switch (optname) {
-	case SO_DEBUG:
-		if (val && !sockopt_capable(CAP_NET_ADMIN))
-			ret = -EACCES;
-		else
-			sock_valbool_flag(sk, SOCK_DBG, valbool);
-		break;
 	case SO_REUSEADDR:
 		sk->sk_reuse = (valbool ? SK_CAN_REUSE : SK_NO_REUSE);
 		break;
@@ -1619,7 +1618,8 @@ int sk_getsockopt(struct sock *sk, int level, int optname,
 
 	switch (optname) {
 	case SO_DEBUG:
-		v.val = sock_flag(sk, SOCK_DBG);
+		/* deprecated. */
+		v.val = 0;
 		break;
 
 	case SO_DONTROUTE:
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index da37e4541a5d..31d09009332a 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -80,9 +80,6 @@ static void mptcp_sol_socket_sync_intval(struct mptcp_sock *msk, int optname, in
 		bool slow = lock_sock_fast(ssk);
 
 		switch (optname) {
-		case SO_DEBUG:
-			sock_valbool_flag(ssk, SOCK_DBG, !!val);
-			break;
 		case SO_KEEPALIVE:
 			if (ssk->sk_prot->keepalive)
 				ssk->sk_prot->keepalive(ssk, !!val);
@@ -183,7 +180,6 @@ static int mptcp_setsockopt_sol_socket_int(struct mptcp_sock *msk, int optname,
 	case SO_KEEPALIVE:
 		mptcp_sol_socket_sync_intval(msk, optname, val);
 		return 0;
-	case SO_DEBUG:
 	case SO_MARK:
 	case SO_PRIORITY:
 	case SO_SNDBUF:
@@ -329,7 +325,6 @@ static int mptcp_setsockopt_sol_socket(struct mptcp_sock *msk, int optname,
 	case SO_RCVBUFFORCE:
 	case SO_MARK:
 	case SO_INCOMING_CPU:
-	case SO_DEBUG:
 	case SO_TIMESTAMP_OLD:
 	case SO_TIMESTAMP_NEW:
 	case SO_TIMESTAMPNS_OLD:
@@ -363,6 +358,7 @@ static int mptcp_setsockopt_sol_socket(struct mptcp_sock *msk, int optname,
 	case SO_WIFI_STATUS:
 	case SO_NOFCS:
 	case SO_SELECT_ERR_QUEUE:
+	case SO_DEBUG: /* deprecated */
 		return 0;
 	}
 
@@ -1458,8 +1454,6 @@ static void sync_socket_options(struct mptcp_sock *msk, struct sock *ssk)
 		sk_dst_reset(ssk);
 	}
 
-	sock_valbool_flag(ssk, SOCK_DBG, sock_flag(sk, SOCK_DBG));
-
 	if (inet_csk(sk)->icsk_ca_ops != inet_csk(ssk)->icsk_ca_ops)
 		tcp_set_congestion_control(ssk, msk->ca_name, false, true);
 	__tcp_sock_set_cork(ssk, !!msk->cork);
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 66763c74ab76..062e16a2766a 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -445,7 +445,6 @@ static int smc_bind(struct socket *sock, struct sockaddr *uaddr,
 			     (1UL << SOCK_LINGER) | \
 			     (1UL << SOCK_BROADCAST) | \
 			     (1UL << SOCK_TIMESTAMP) | \
-			     (1UL << SOCK_DBG) | \
 			     (1UL << SOCK_RCVTSTAMP) | \
 			     (1UL << SOCK_RCVTSTAMPNS) | \
 			     (1UL << SOCK_LOCALROUTE) | \
@@ -511,8 +510,8 @@ static void smc_copy_sock_settings_to_clc(struct smc_sock *smc)
 
 #define SK_FLAGS_CLC_TO_SMC ((1UL << SOCK_URGINLINE) | \
 			     (1UL << SOCK_KEEPOPEN) | \
-			     (1UL << SOCK_LINGER) | \
-			     (1UL << SOCK_DBG))
+			     (1UL << SOCK_LINGER))
+
 /* copy only settings and flags relevant for smc from clc to smc socket */
 static void smc_copy_sock_settings_to_smc(struct smc_sock *smc)
 {
-- 
2.30.2

Re: [PATCH v2 net-next] net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.

[Matthieu Baerts]

Hi Kuniyuki,

On 14/02/2024 20:54, Kuniyuki Iwashima wrote:
> Recently, commit 8e5443d2b866 ("net: remove SOCK_DEBUG leftovers")
> removed the last users of SOCK_DEBUG(), and commit b1dffcf0da22 ("net:
> remove SOCK_DEBUG macro") removed the macro.
> 
> Now is the time to deprecate the oldest socket option.
> 
> Note that setsockopt(SO_DEBUG) is moved not to acquire lock_sock().
> 
> Reviewed-by: Gerd Bayer <gbayer@linux.ibm.com>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
> v2:
>   * Move setsockopt(SO_DEBUG) code not to acquire lock_sock().

Thank you for the v2!

Good idea to have modified that in net/core/sock.c too!

Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>


I don't think we need to do anything else, but just to be sure: do we
need to tell the userspace this socket option has been deprecated?
SO_DEBUG is a bit particular, so I guess it is fine not to do anything
else, but except by looking at the kernel version, I don't think the
userspace can know it has no more effect.

I didn't find many examples of other deprecated socket options, apart
from SO_BSDCOMPAT. For years, there was a warning, removed a few years
ago: f4ecc748533d ("net: Stop warning about SO_BSDCOMPAT usage"). I
guess we don't want that for SO_DEBUG, right?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH v2 net-next] net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.

[Alexandra Winter]

On 14.02.24 20:54, Kuniyuki Iwashima wrote:
> +	case SO_DEBUG:
> +		/* deprecated, but kept for compatibility */
> +		if (val && !sockopt_capable(CAP_NET_ADMIN))
> +			ret = -EACCES;
> +		return 0;

Setting ret has no effect here. Maybe you mean something like:
> +		if (val && !sockopt_capable(CAP_NET_ADMIN))
> +			return -EACCES;
> +		return 0;

or 

return (val && !sockopt_capable(CAP_NET_ADMIN)) ? -EACCESS : 0;

Re: [PATCH v2 net-next] net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.

[Jakub Kicinski]

On Wed, 14 Feb 2024 11:54:07 -0800 Kuniyuki Iwashima wrote:
> Recently, commit 8e5443d2b866 ("net: remove SOCK_DEBUG leftovers")
> removed the last users of SOCK_DEBUG(), and commit b1dffcf0da22 ("net:
> remove SOCK_DEBUG macro") removed the macro.

Unrelated to this patch but speaking of deprecating things - do you
want to go ahead with deleting DCCP? I don't recall our exact plan,
I thought it was supposed to happen early in the year :)

Re: [PATCH v2 net-next] net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.

[Paolo Abeni]

On Thu, 2024-02-15 at 07:07 -0800, Jakub Kicinski wrote:
> On Wed, 14 Feb 2024 11:54:07 -0800 Kuniyuki Iwashima wrote:
> > Recently, commit 8e5443d2b866 ("net: remove SOCK_DEBUG leftovers")
> > removed the last users of SOCK_DEBUG(), and commit b1dffcf0da22 ("net:
> > remove SOCK_DEBUG macro") removed the macro.
> 
> Unrelated to this patch but speaking of deprecating things - do you
> want to go ahead with deleting DCCP? I don't recall our exact plan,
> I thought it was supposed to happen early in the year :)

My personal "current year" counter tend to be outdated till at least
May, but I *think* it's supposed to happen the next year:

https://elixir.bootlin.com/linux/v6.8-rc4/source/net/dccp/proto.c#L193

:)

/P

Re: [PATCH v2 net-next] net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.

[Jakub Kicinski]

On Thu, 15 Feb 2024 17:05:58 +0100 Paolo Abeni wrote:
> > Unrelated to this patch but speaking of deprecating things - do you
> > want to go ahead with deleting DCCP? I don't recall our exact plan,
> > I thought it was supposed to happen early in the year :)  
> 
> My personal "current year" counter tend to be outdated till at least
> May, but I *think* it's supposed to happen the next year:
> 
> https://elixir.bootlin.com/linux/v6.8-rc4/source/net/dccp/proto.c#L193

Apparently mine runs ahead :D
Thank you!

Re: [PATCH v2 net-next] net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.

[Kuniyuki Iwashima]

From: Matthieu Baerts <matttbe@kernel.org>
Date: Thu, 15 Feb 2024 10:58:58 +0100
> Hi Kuniyuki,
> 
> On 14/02/2024 20:54, Kuniyuki Iwashima wrote:
> > Recently, commit 8e5443d2b866 ("net: remove SOCK_DEBUG leftovers")
> > removed the last users of SOCK_DEBUG(), and commit b1dffcf0da22 ("net:
> > remove SOCK_DEBUG macro") removed the macro.
> > 
> > Now is the time to deprecate the oldest socket option.
> > 
> > Note that setsockopt(SO_DEBUG) is moved not to acquire lock_sock().
> > 
> > Reviewed-by: Gerd Bayer <gbayer@linux.ibm.com>
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> > ---
> > v2:
> >   * Move setsockopt(SO_DEBUG) code not to acquire lock_sock().
> 
> Thank you for the v2!
> 
> Good idea to have modified that in net/core/sock.c too!
> 
> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> 
> 
> I don't think we need to do anything else, but just to be sure: do we
> need to tell the userspace this socket option has been deprecated?
> SO_DEBUG is a bit particular, so I guess it is fine not to do anything
> else, but except by looking at the kernel version, I don't think the
> userspace can know it has no more effect.
> 
> I didn't find many examples of other deprecated socket options, apart
> from SO_BSDCOMPAT. For years, there was a warning, removed a few years
> ago: f4ecc748533d ("net: Stop warning about SO_BSDCOMPAT usage"). I
> guess we don't want that for SO_DEBUG, right?

I was also wondering if I should add pr_info_once(), but it seems not
worth adding.

With a rough grep,

  $ git log -S SOCK_DEBUG net

except for the legacy protocols (appletalk, dccp, x25) that 8e5443d2b866
touched recently, TCP was the last real user of SOCK_DEBUG() and it's
2019-02-25 (5 years ago!) that 9946b3410b61 removed the use.

So, I think we don't need such warning.

Thanks!

Re: [PATCH v2 net-next] net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.

[Kuniyuki Iwashima]

From: Alexandra Winter <wintera@linux.ibm.com>
Date: Thu, 15 Feb 2024 15:14:21 +0100
> On 14.02.24 20:54, Kuniyuki Iwashima wrote:
> > +	case SO_DEBUG:
> > +		/* deprecated, but kept for compatibility */
> > +		if (val && !sockopt_capable(CAP_NET_ADMIN))
> > +			ret = -EACCES;
> > +		return 0;
> 
> Setting ret has no effect here. Maybe you mean something like:
> > +		if (val && !sockopt_capable(CAP_NET_ADMIN))
> > +			return -EACCES;
> > +		return 0;
> 
> or 
> 
> return (val && !sockopt_capable(CAP_NET_ADMIN)) ? -EACCESS : 0;

oops, thanks for catching!
will fix in v3.