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

Kuniyuki Iwashima posted 1 patch 2 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/20240213223135.85957-1-kuniyu@amazon.com
There is a newer version of this series
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(-)
[PATCH v1 net-next] net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.
Posted by Kuniyuki Iwashima 2 months, 2 weeks ago
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.
Posted by Neal Cardwell 2 months, 1 week ago
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.
Posted by Kuniyuki Iwashima 2 months, 1 week ago
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.
Posted by Neal Cardwell 2 months, 1 week ago
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
Re: [PATCH v1 net-next] net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.
Posted by Matthieu Baerts 2 months, 2 weeks ago
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.
Posted by Kuniyuki Iwashima 2 months, 2 weeks ago
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: net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.: Tests Results
Posted by MPTCP CI 2 months, 2 weeks ago
Hi Kuniyuki,

Thank you for your modifications, that's great!

Our CI (Cirrus) did some validations with a debug kernel and here is its report:

- KVM Validation: debug (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/4629533685448704
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4629533685448704/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5755433592291328
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5755433592291328/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/fbdc306e8139


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)
Re: net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.: Tests Results
Posted by MPTCP CI 2 months, 2 weeks ago
Hi Kuniyuki,

Thank you for your modifications, that's great!

Our CI (GitHub Action) did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7898465398

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/fbdc306e8139


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)
Re: [PATCH v1 net-next] net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.
Posted by Gerd Bayer 2 months, 2 weeks ago
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.
Posted by Kuniyuki Iwashima 2 months, 2 weeks ago
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: net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.: Tests Results
Posted by MPTCP CI 2 months, 2 weeks ago
Hi Kuniyuki,

Thank you for your modifications, that's great!

Our CI (Cirrus) did some validations with a debug kernel and here is its report:

- KVM Validation: debug (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5969355108777984
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5969355108777984/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5471411028885504
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5471411028885504/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/b82e0568acae


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)
Re: net: Deprecate SO_DEBUG and reclaim SOCK_DBG bit.: Tests Results
Posted by MPTCP CI 2 months, 2 weeks ago
Hi Kuniyuki,

Thank you for your modifications, that's great!

Our CI (GitHub Action) did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7893771323

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/b82e0568acae


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)