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(-)
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
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 :)
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
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!
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;
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.
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): - Unstable: 1 failed test(s): packetdrill_regressions 🔴: - Task: https://cirrus-ci.com/task/6157288147255296 - Summary: https://api.cirrus-ci.com/v1/artifact/task/6157288147255296/summary/summary.txt - KVM Validation: debug (only selftest_mptcp_join): - Success! ✅: - Task: https://cirrus-ci.com/task/5594338193833984 - Summary: https://api.cirrus-ci.com/v1/artifact/task/5594338193833984/summary/summary.txt Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/4b23a08aba91 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)
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/7914279679 Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/4b23a08aba91 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)
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.
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!
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/5986065517903872 - Summary: https://api.cirrus-ci.com/v1/artifact/task/5986065517903872/summary/summary.txt - KVM Validation: debug (only selftest_mptcp_join): - Success! ✅: - Task: https://cirrus-ci.com/task/5423115564482560 - Summary: https://api.cirrus-ci.com/v1/artifact/task/5423115564482560/summary/summary.txt Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/a36a5e8538ed 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)
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/7906812299 Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/a36a5e8538ed 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)
© 2016 - 2024 Red Hat, Inc.