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

[PATCH v1 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.

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 include/net/sock.h  | 1 -
 net/core/sock.c     | 6 +++---
 net/mptcp/sockopt.c | 4 +---
 net/smc/af_smc.c    | 5 ++---
 4 files changed, 6 insertions(+), 10 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..0a58dc861908 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1194,10 +1194,9 @@ int sk_setsockopt(struct sock *sk, int level, int optname,
 
 	switch (optname) {
 	case SO_DEBUG:
+		/* deprecated, but kept for compatibility. */
 		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);
@@ -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..f6d90eef3d7c 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -81,7 +81,7 @@ static void mptcp_sol_socket_sync_intval(struct mptcp_sock *msk, int optname, in
 
 		switch (optname) {
 		case SO_DEBUG:
-			sock_valbool_flag(ssk, SOCK_DBG, !!val);
+			/* deprecated. */
 			break;
 		case SO_KEEPALIVE:
 			if (ssk->sk_prot->keepalive)
@@ -1458,8 +1458,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 v1 net-next] net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.

[Gerd Bayer]

On Tue, 2024-02-13 at 14:31 -0800, Kuniyuki Iwashima wrote:

Hi Kuniyuki,

I'm adding the newly appointed SMC maintainers to chime in.

> 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.
> 
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
>  include/net/sock.h  | 1 -
>  net/core/sock.c     | 6 +++---
>  net/mptcp/sockopt.c | 4 +---
>  net/smc/af_smc.c    | 5 ++---
>  4 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 88bf810394a5..0a58dc861908 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1194,10 +1194,9 @@ int sk_setsockopt(struct sock *sk, int level,
> int optname,
>  
>         switch (optname) {
>         case SO_DEBUG:
> +               /* deprecated, but kept for compatibility. */
Not 100% sure about the language - but isn't the DEBUG feature
*removed* rather than just *deprecated*?

>                 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);
> @@ -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. */
Same here.

> +               v.val = 0;
>                 break;
>  
>         case SO_DONTROUTE:
> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index da37e4541a5d..f6d90eef3d7c 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -81,7 +81,7 @@ static void mptcp_sol_socket_sync_intval(struct
> mptcp_sock *msk, int optname, in
>  
>                 switch (optname) {
>                 case SO_DEBUG:
> -                       sock_valbool_flag(ssk, SOCK_DBG, !!val);
> +                       /* deprecated. */
and here.

>                         break;
>                 case SO_KEEPALIVE:
>                         if (ssk->sk_prot->keepalive)
> @@ -1458,8 +1458,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)
>  {
The SMC changes look good to me, so feel free to add my
Reviewed-by: Gerd Bayer <gbayer@linux.ibm.com>

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

[Matthieu Baerts]

Hi Kuniyuki,

On 13/02/2024 23:31, 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.

Thank you for looking at this!

My review here below is only about the modifications related to MPTCP.

(...)

> diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> index da37e4541a5d..f6d90eef3d7c 100644
> --- a/net/mptcp/sockopt.c
> +++ b/net/mptcp/sockopt.c
> @@ -81,7 +81,7 @@ static void mptcp_sol_socket_sync_intval(struct mptcp_sock *msk, int optname, in
>  
>  		switch (optname) {
>  		case SO_DEBUG:
> -			sock_valbool_flag(ssk, SOCK_DBG, !!val);
> +			/* deprecated. */

If it is now a NOOP, maybe better to:

 - remove SO_DEBUG from mptcp_sol_socket_sync_intval() and
   mptcp_setsockopt_sol_socket_int()

 - move it just above the "return 0" in mptcp_setsockopt_sol_socket()
with the "deprecated" (or "removed") comment

By doing that, we avoid a lock, plus going through the list of subflows
for nothing.

WDYT?

>  			break;
>  		case SO_KEEPALIVE:
>  			if (ssk->sk_prot->keepalive)
> @@ -1458,8 +1458,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);

The rest looks good to me.

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

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

[Kuniyuki Iwashima]

From: Gerd Bayer <gbayer@linux.ibm.com>
Date: Wed, 14 Feb 2024 09:14:33 +0100
> On Tue, 2024-02-13 at 14:31 -0800, Kuniyuki Iwashima wrote:
> 
> Hi Kuniyuki,
> 
> I'm adding the newly appointed SMC maintainers to chime in.
> 
> > 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.
> > 
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> > ---
> >  include/net/sock.h  | 1 -
> >  net/core/sock.c     | 6 +++---
> >  net/mptcp/sockopt.c | 4 +---
> >  net/smc/af_smc.c    | 5 ++---
> >  4 files changed, 6 insertions(+), 10 deletions(-)
> > 
> > diff --git a/net/core/sock.c b/net/core/sock.c
> > index 88bf810394a5..0a58dc861908 100644
> > --- a/net/core/sock.c
> > +++ b/net/core/sock.c
> > @@ -1194,10 +1194,9 @@ int sk_setsockopt(struct sock *sk, int level,
> > int optname,
> >  
> >         switch (optname) {
> >         case SO_DEBUG:
> > +               /* deprecated, but kept for compatibility. */
> Not 100% sure about the language - but isn't the DEBUG feature
> *removed* rather than just *deprecated*?

SO_DEBUG is still here, and the user won't get -ENOPROTOOPT.
That's why I wrote deprecated, no strong preference though.

> 
> >                 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);
> > @@ -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. */
> Same here.
> 
> > +               v.val = 0;
> >                 break;
> >  
> >         case SO_DONTROUTE:
> > diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> > index da37e4541a5d..f6d90eef3d7c 100644
> > --- a/net/mptcp/sockopt.c
> > +++ b/net/mptcp/sockopt.c
> > @@ -81,7 +81,7 @@ static void mptcp_sol_socket_sync_intval(struct
> > mptcp_sock *msk, int optname, in
> >  
> >                 switch (optname) {
> >                 case SO_DEBUG:
> > -                       sock_valbool_flag(ssk, SOCK_DBG, !!val);
> > +                       /* deprecated. */
> and here.
> 
> >                         break;
> >                 case SO_KEEPALIVE:
> >                         if (ssk->sk_prot->keepalive)
> > @@ -1458,8 +1458,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)
> >  {
> The SMC changes look good to me, so feel free to add my
> Reviewed-by: Gerd Bayer <gbayer@linux.ibm.com>

Thanks!

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

[Kuniyuki Iwashima]

From: Matthieu Baerts <matttbe@kernel.org>
Date: Wed, 14 Feb 2024 10:32:58 +0100
> Hi Kuniyuki,
> 
> On 13/02/2024 23:31, 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.
> 
> Thank you for looking at this!
> 
> My review here below is only about the modifications related to MPTCP.
> 
> (...)
> 
> > diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> > index da37e4541a5d..f6d90eef3d7c 100644
> > --- a/net/mptcp/sockopt.c
> > +++ b/net/mptcp/sockopt.c
> > @@ -81,7 +81,7 @@ static void mptcp_sol_socket_sync_intval(struct mptcp_sock *msk, int optname, in
> >  
> >  		switch (optname) {
> >  		case SO_DEBUG:
> > -			sock_valbool_flag(ssk, SOCK_DBG, !!val);
> > +			/* deprecated. */
> 
> If it is now a NOOP, maybe better to:
> 
>  - remove SO_DEBUG from mptcp_sol_socket_sync_intval() and
>    mptcp_setsockopt_sol_socket_int()
> 
>  - move it just above the "return 0" in mptcp_setsockopt_sol_socket()
> with the "deprecated" (or "removed") comment
> 
> By doing that, we avoid a lock, plus going through the list of subflows
> for nothing.
> 
> WDYT?

Sounds good!

And I can apply the similar change to the general setsockopt()
not to take lock_sock().


> 
> >  			break;
> >  		case SO_KEEPALIVE:
> >  			if (ssk->sk_prot->keepalive)
> > @@ -1458,8 +1458,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);
> 
> The rest looks good to me.

Thanks!

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

[Neal Cardwell]

On Tue, Feb 13, 2024 at 3:32 PM Kuniyuki Iwashima <kuniyu@amazon.com> 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.
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---

I would like to kindly implore you to please not remove the
functionality of the SO_DEBUG socket option. This socket option is a
key mechanism that the Google TCP team uses for automated testing of
Linux TCP, including BBR congestion control.

Widely used tools like netperf allow users to enable the SO_DEBUG
socket option via the command line (-g in netperf). Then debugging
code in the kernel can use the SOCK_DBG bit to decide whether to take
special actions, such as logging debug information, which can be used
to generate graphs or assertions about correct internal behavior. For
example, the transperf network testing tool that our team open-sourced
- https://github.com/google/transperf - uses the netperf -g/SO_DEBUG
mechanism to trigger debug logging that we use for testing,
troubleshooting, analysis, and development.

The SO_DEBUG mechanism is nice in that it works well no matter what
policy an application or benchmarking tool uses for choosing other
attributes (like port numbers) that could conceivably be used to point
out connections that should receive debug treatment. For example, most
benchmarking or production workloads will effectively end up with
random port numbers, which makes port numbers hard to use  for
triggering debug treatment.

This mechanism is very simple and battle-tested, it works well, and
IMHO it would be a tragedy to remove it. It would cause our team
meaningful headaches to replace it. Please keep the SO_DEBUG socket
option functionality as-is. :-)

Thanks for your consideration on this!

Best regards,
neal

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

[Kuniyuki Iwashima]

From: Neal Cardwell <ncardwell@google.com>
Date: Thu, 15 Feb 2024 12:57:35 -0700
> On Tue, Feb 13, 2024 at 3:32 PM Kuniyuki Iwashima <kuniyu@amazon.com> 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.
> >
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> > ---
> 
> I would like to kindly implore you to please not remove the
> functionality of the SO_DEBUG socket option. This socket option is a
> key mechanism that the Google TCP team uses for automated testing of
> Linux TCP, including BBR congestion control.
> 
> Widely used tools like netperf allow users to enable the SO_DEBUG
> socket option via the command line (-g in netperf). Then debugging
> code in the kernel can use the SOCK_DBG bit to decide whether to take
> special actions, such as logging debug information, which can be used
> to generate graphs or assertions about correct internal behavior. For
> example, the transperf network testing tool that our team open-sourced
> - https://github.com/google/transperf - uses the netperf -g/SO_DEBUG
> mechanism to trigger debug logging that we use for testing,
> troubleshooting, analysis, and development.
> 
> The SO_DEBUG mechanism is nice in that it works well no matter what
> policy an application or benchmarking tool uses for choosing other
> attributes (like port numbers) that could conceivably be used to point
> out connections that should receive debug treatment. For example, most
> benchmarking or production workloads will effectively end up with
> random port numbers, which makes port numbers hard to use  for
> triggering debug treatment.
> 
> This mechanism is very simple and battle-tested, it works well, and
> IMHO it would be a tragedy to remove it. It would cause our team
> meaningful headaches to replace it. Please keep the SO_DEBUG socket
> option functionality as-is. :-)
> 
> Thanks for your consideration on this!

Oh that's an interesting use case!
I didn't think of out-of-tree uses.
Sure, I'll drop the patch.

Thanks!

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

[Neal Cardwell]

On Thu, Feb 15, 2024 at 1:16 PM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> From: Neal Cardwell <ncardwell@google.com>
> Date: Thu, 15 Feb 2024 12:57:35 -0700
> > On Tue, Feb 13, 2024 at 3:32 PM Kuniyuki Iwashima <kuniyu@amazon.com> 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.
> > >
> > > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> > > ---
> >
> > I would like to kindly implore you to please not remove the
> > functionality of the SO_DEBUG socket option. This socket option is a
> > key mechanism that the Google TCP team uses for automated testing of
> > Linux TCP, including BBR congestion control.
> >
> > Widely used tools like netperf allow users to enable the SO_DEBUG
> > socket option via the command line (-g in netperf). Then debugging
> > code in the kernel can use the SOCK_DBG bit to decide whether to take
> > special actions, such as logging debug information, which can be used
> > to generate graphs or assertions about correct internal behavior. For
> > example, the transperf network testing tool that our team open-sourced
> > - https://github.com/google/transperf - uses the netperf -g/SO_DEBUG
> > mechanism to trigger debug logging that we use for testing,
> > troubleshooting, analysis, and development.
> >
> > The SO_DEBUG mechanism is nice in that it works well no matter what
> > policy an application or benchmarking tool uses for choosing other
> > attributes (like port numbers) that could conceivably be used to point
> > out connections that should receive debug treatment. For example, most
> > benchmarking or production workloads will effectively end up with
> > random port numbers, which makes port numbers hard to use  for
> > triggering debug treatment.
> >
> > This mechanism is very simple and battle-tested, it works well, and
> > IMHO it would be a tragedy to remove it. It would cause our team
> > meaningful headaches to replace it. Please keep the SO_DEBUG socket
> > option functionality as-is. :-)
> >
> > Thanks for your consideration on this!
>
> Oh that's an interesting use case!
> I didn't think of out-of-tree uses.
> Sure, I'll drop the patch.
>
> Thanks!

Great! Thank you!

neal