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.
