[PATCH net-next 00/10] net: better const qualifier awareness

[PATCH net-next 00/10] net: better const qualifier awareness

[Eric Dumazet]

This is a follow-up of d27d367d3b78 ("inet: better const qualifier awareness")

Adopting container_of_const() to perform (struct sock *)->(protocol sock *)
operation is allowing us to propagate const qualifier and thus detect
misuses at compile time.

Most conversions are trivial, because most protocols did not adopt yet
const sk pointers where it could make sense.

Only mptcp and tcp patches (end of this series) are requiring small
adjustments.

Thanks !

Eric Dumazet (10):
  udp: preserve const qualifier in udp_sk()
  af_packet: preserve const qualifier in pkt_sk()
  raw: preserve const qualifier in raw_sk()
  ipv6: raw: preserve const qualifier in raw6_sk()
  dccp: preserve const qualifier in dccp_sk()
  af_unix: preserve const qualifier in unix_sk()
  smc: preserve const qualifier in smc_sk()
  x25: preserve const qualifier in [a]x25_sk()
  mptcp: preserve const qualifier in mptcp_sk()
  tcp: preserve const qualifier in tcp_sk()

 include/linux/dccp.h     |  6 ++----
 include/linux/ipv6.h     |  5 +----
 include/linux/tcp.h      | 10 ++++++----
 include/linux/udp.h      |  5 +----
 include/net/af_unix.h    |  5 +----
 include/net/ax25.h       |  5 +----
 include/net/raw.h        |  5 +----
 include/net/tcp.h        |  2 +-
 include/net/x25.h        |  5 +----
 net/ipv4/tcp.c           |  2 +-
 net/ipv4/tcp_input.c     |  4 ++--
 net/ipv4/tcp_minisocks.c |  5 +++--
 net/ipv4/tcp_output.c    |  9 +++++++--
 net/ipv4/tcp_recovery.c  |  2 +-
 net/mptcp/protocol.c     |  2 +-
 net/mptcp/protocol.h     |  9 +++------
 net/packet/internal.h    |  5 +----
 net/smc/smc.h            |  5 +----
 security/lsm_audit.c     |  2 +-
 19 files changed, 36 insertions(+), 57 deletions(-)

-- 
2.40.0.rc2.332.ga46443480c-goog

[PATCH net-next 01/10] udp: preserve const qualifier in udp_sk()

[Eric Dumazet]

We can change udp_sk() to propagate const qualifier of its argument,
thanks to container_of_const()

This should avoid some potential errors caused by accidental
(const -> not_const) promotion.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Willem de Bruijn <willemb@google.com>
---
 include/linux/udp.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/linux/udp.h b/include/linux/udp.h
index a2892e151644ec347ad52d426400678e4a53b359..43c1fb2d2c21afc01abdf20e4b9c03f04932c19b 100644
--- a/include/linux/udp.h
+++ b/include/linux/udp.h
@@ -97,10 +97,7 @@ struct udp_sock {
 
 #define UDP_MAX_SEGMENTS	(1 << 6UL)
 
-static inline struct udp_sock *udp_sk(const struct sock *sk)
-{
-	return (struct udp_sock *)sk;
-}
+#define udp_sk(ptr) container_of_const(ptr, struct udp_sock, inet.sk)
 
 static inline void udp_set_no_check6_tx(struct sock *sk, bool val)
 {
-- 
2.40.0.rc2.332.ga46443480c-goog

Re: [PATCH net-next 01/10] udp: preserve const qualifier in udp_sk()

[Simon Horman]

On Fri, Mar 17, 2023 at 03:55:30PM +0000, Eric Dumazet wrote:
> We can change udp_sk() to propagate const qualifier of its argument,
> thanks to container_of_const()
> 
> This should avoid some potential errors caused by accidental
> (const -> not_const) promotion.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Willem de Bruijn <willemb@google.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

[PATCH net-next 02/10] af_packet: preserve const qualifier in pkt_sk()

[Eric Dumazet]

We can change pkt_sk() to propagate const qualifier of its argument,
thanks to container_of_const()

This should avoid some potential errors caused by accidental
(const -> not_const) promotion.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Willem de Bruijn <willemb@google.com>
---
 net/packet/internal.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/packet/internal.h b/net/packet/internal.h
index 680703dbce5e04fc26d0fdeab1c1c911b71a8729..e793e99646f1c60f61a8dc5e765f8f544de83972 100644
--- a/net/packet/internal.h
+++ b/net/packet/internal.h
@@ -133,10 +133,7 @@ struct packet_sock {
 	atomic_t		tp_drops ____cacheline_aligned_in_smp;
 };
 
-static inline struct packet_sock *pkt_sk(struct sock *sk)
-{
-	return (struct packet_sock *)sk;
-}
+#define pkt_sk(ptr) container_of_const(ptr, struct packet_sock, sk)
 
 enum packet_sock_flags {
 	PACKET_SOCK_ORIGDEV,
-- 
2.40.0.rc2.332.ga46443480c-goog

Re: [PATCH net-next 02/10] af_packet: preserve const qualifier in pkt_sk()

[Simon Horman]

On Fri, Mar 17, 2023 at 03:55:31PM +0000, Eric Dumazet wrote:
> We can change pkt_sk() to propagate const qualifier of its argument,
> thanks to container_of_const()
> 
> This should avoid some potential errors caused by accidental
> (const -> not_const) promotion.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Willem de Bruijn <willemb@google.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

[PATCH net-next 03/10] raw: preserve const qualifier in raw_sk()

[Eric Dumazet]

We can change raw_sk() to propagate const qualifier of its argument,
thanks to container_of_const()

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/raw.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/net/raw.h b/include/net/raw.h
index 7ad15830cf38460f1fae3a187986d74faef6dd1d..c215af02f7589ee5e77a7c3f89cb216fab638c4c 100644
--- a/include/net/raw.h
+++ b/include/net/raw.h
@@ -83,10 +83,7 @@ struct raw_sock {
 	u32		   ipmr_table;
 };
 
-static inline struct raw_sock *raw_sk(const struct sock *sk)
-{
-	return (struct raw_sock *)sk;
-}
+#define raw_sk(ptr) container_of_const(ptr, struct raw_sock, inet.sk)
 
 static inline bool raw_sk_bound_dev_eq(struct net *net, int bound_dev_if,
 				       int dif, int sdif)
-- 
2.40.0.rc2.332.ga46443480c-goog

Re: [PATCH net-next 03/10] raw: preserve const qualifier in raw_sk()

[Simon Horman]

On Fri, Mar 17, 2023 at 03:55:32PM +0000, Eric Dumazet wrote:
> We can change raw_sk() to propagate const qualifier of its argument,
> thanks to container_of_const()
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

[PATCH net-next 04/10] ipv6: raw: preserve const qualifier in raw6_sk()

[Eric Dumazet]

We can change raw6_sk() to propagate its argument const qualifier,
thanks to container_of_const().

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/ipv6.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 37dfdcfcdd542fe0efc9a1df967be3da931635d4..839247a4f48ea76b5d6daa9a54a7b87627635066 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -336,10 +336,7 @@ static inline struct ipv6_pinfo *inet6_sk(const struct sock *__sk)
 	return sk_fullsock(__sk) ? inet_sk(__sk)->pinet6 : NULL;
 }
 
-static inline struct raw6_sock *raw6_sk(const struct sock *sk)
-{
-	return (struct raw6_sock *)sk;
-}
+#define raw6_sk(ptr) container_of_const(ptr, struct raw6_sock, inet.sk)
 
 #define ipv6_only_sock(sk)	(sk->sk_ipv6only)
 #define ipv6_sk_rxinfo(sk)	((sk)->sk_family == PF_INET6 && \
-- 
2.40.0.rc2.332.ga46443480c-goog

Re: [PATCH net-next 04/10] ipv6: raw: preserve const qualifier in raw6_sk()

[Simon Horman]

On Fri, Mar 17, 2023 at 03:55:33PM +0000, Eric Dumazet wrote:
> We can change raw6_sk() to propagate its argument const qualifier,
> thanks to container_of_const().
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

[PATCH net-next 05/10] dccp: preserve const qualifier in dccp_sk()

[Eric Dumazet]

We can change dccp_sk() to propagate its argument const qualifier,
thanks to container_of_const().

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/dccp.h | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/linux/dccp.h b/include/linux/dccp.h
index 07e547c02fd8b23e05f1f45a3915b2987faddbe9..325af611909f99793491b437231eba20b589c032 100644
--- a/include/linux/dccp.h
+++ b/include/linux/dccp.h
@@ -305,10 +305,8 @@ struct dccp_sock {
 	struct timer_list		dccps_xmit_timer;
 };
 
-static inline struct dccp_sock *dccp_sk(const struct sock *sk)
-{
-	return (struct dccp_sock *)sk;
-}
+#define dccp_sk(ptr)	container_of_const(ptr, struct dccp_sock, \
+					   dccps_inet_connection.icsk_inet.sk)
 
 static inline const char *dccp_role(const struct sock *sk)
 {
-- 
2.40.0.rc2.332.ga46443480c-goog

Re: [PATCH net-next 05/10] dccp: preserve const qualifier in dccp_sk()

[Simon Horman]

On Fri, Mar 17, 2023 at 03:55:34PM +0000, Eric Dumazet wrote:
> We can change dccp_sk() to propagate its argument const qualifier,
> thanks to container_of_const().
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

[PATCH net-next 06/10] af_unix: preserve const qualifier in unix_sk()

[Eric Dumazet]

We can change unix_sk() to propagate its argument const qualifier,
thanks to container_of_const().

We need to change dump_common_audit_data() 'struct unix_sock *u'
local var to get a const attribute.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/af_unix.h | 5 +----
 security/lsm_audit.c  | 2 +-
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index 45ebde587138e59f8331d358420d3fca79d9ee66..824c258143a3ab360b870fda38ba684b70068eee 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -74,10 +74,7 @@ struct unix_sock {
 #endif
 };
 
-static inline struct unix_sock *unix_sk(const struct sock *sk)
-{
-	return (struct unix_sock *)sk;
-}
+#define unix_sk(ptr) container_of_const(ptr, struct unix_sock, sk)
 
 #define peer_wait peer_wq.wait
 
diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index 00d3bdd386e294ecd562bfa8ce502bf179ad32d9..368e77ca43c4a5d5f71a7e9a0a1e58a006796aa6 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -310,7 +310,7 @@ static void dump_common_audit_data(struct audit_buffer *ab,
 	case LSM_AUDIT_DATA_NET:
 		if (a->u.net->sk) {
 			const struct sock *sk = a->u.net->sk;
-			struct unix_sock *u;
+			const struct unix_sock *u;
 			struct unix_address *addr;
 			int len = 0;
 			char *p = NULL;
-- 
2.40.0.rc2.332.ga46443480c-goog

Re: [PATCH net-next 06/10] af_unix: preserve const qualifier in unix_sk()

[Simon Horman]

On Fri, Mar 17, 2023 at 03:55:35PM +0000, Eric Dumazet wrote:
> We can change unix_sk() to propagate its argument const qualifier,
> thanks to container_of_const().
> 
> We need to change dump_common_audit_data() 'struct unix_sock *u'
> local var to get a const attribute.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

[PATCH net-next 07/10] smc: preserve const qualifier in smc_sk()

[Eric Dumazet]

We can change smc_sk() to propagate its argument const qualifier,
thanks to container_of_const().

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Karsten Graul <kgraul@linux.ibm.com>
Cc: Wenjia Zhang <wenjia@linux.ibm.com>
Cc: Jan Karcher <jaka@linux.ibm.com>
---
 net/smc/smc.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/smc/smc.h b/net/smc/smc.h
index 5ed765ea0c731a7f0095cd6a99a0e42d227eaca9..2eeea4cdc7187eed2a3b12888d8f647382f6f2ac 100644
--- a/net/smc/smc.h
+++ b/net/smc/smc.h
@@ -283,10 +283,7 @@ struct smc_sock {				/* smc sock container */
 						 * */
 };
 
-static inline struct smc_sock *smc_sk(const struct sock *sk)
-{
-	return (struct smc_sock *)sk;
-}
+#define smc_sk(ptr) container_of_const(ptr, struct smc_sock, sk)
 
 static inline void smc_init_saved_callbacks(struct smc_sock *smc)
 {
-- 
2.40.0.rc2.332.ga46443480c-goog

Re: [PATCH net-next 07/10] smc: preserve const qualifier in smc_sk()

[Simon Horman]

On Fri, Mar 17, 2023 at 03:55:36PM +0000, Eric Dumazet wrote:
> We can change smc_sk() to propagate its argument const qualifier,
> thanks to container_of_const().
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Karsten Graul <kgraul@linux.ibm.com>
> Cc: Wenjia Zhang <wenjia@linux.ibm.com>
> Cc: Jan Karcher <jaka@linux.ibm.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

Re: [PATCH net-next 07/10] smc: preserve const qualifier in smc_sk()

[Wenjia Zhang]

On 17.03.23 16:55, Eric Dumazet wrote:
> We can change smc_sk() to propagate its argument const qualifier,
> thanks to container_of_const().
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Karsten Graul <kgraul@linux.ibm.com>
> Cc: Wenjia Zhang <wenjia@linux.ibm.com>
> Cc: Jan Karcher <jaka@linux.ibm.com>

Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com>

> ---
>   net/smc/smc.h | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/net/smc/smc.h b/net/smc/smc.h
> index 5ed765ea0c731a7f0095cd6a99a0e42d227eaca9..2eeea4cdc7187eed2a3b12888d8f647382f6f2ac 100644
> --- a/net/smc/smc.h
> +++ b/net/smc/smc.h
> @@ -283,10 +283,7 @@ struct smc_sock {				/* smc sock container */
>   						 * */
>   };
>   
> -static inline struct smc_sock *smc_sk(const struct sock *sk)
> -{
> -	return (struct smc_sock *)sk;
> -}
> +#define smc_sk(ptr) container_of_const(ptr, struct smc_sock, sk)
>   
>   static inline void smc_init_saved_callbacks(struct smc_sock *smc)
>   {

[PATCH net-next 08/10] x25: preserve const qualifier in [a]x25_sk()

[Eric Dumazet]

We can change [a]x25_sk() to propagate their argument const qualifier,
thanks to container_of_const().

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/ax25.h | 5 +----
 include/net/x25.h  | 5 +----
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/include/net/ax25.h b/include/net/ax25.h
index f8cf3629a41934f96f33e5d70ad90cc8ae796d38..0d939e5aee4eca38d2b1bd86f87fe3cd990af67b 100644
--- a/include/net/ax25.h
+++ b/include/net/ax25.h
@@ -260,10 +260,7 @@ struct ax25_sock {
 	struct ax25_cb		*cb;
 };
 
-static inline struct ax25_sock *ax25_sk(const struct sock *sk)
-{
-	return (struct ax25_sock *) sk;
-}
+#define ax25_sk(ptr) container_of_const(ptr, struct ax25_sock, sk)
 
 static inline struct ax25_cb *sk_to_ax25(const struct sock *sk)
 {
diff --git a/include/net/x25.h b/include/net/x25.h
index d7d6c2b4ffa7153b0caef7dd249ba93f5f39e414..597eb53c471e3386108447d46b054852abfcce6c 100644
--- a/include/net/x25.h
+++ b/include/net/x25.h
@@ -177,10 +177,7 @@ struct x25_forward {
 	atomic_t		refcnt;
 };
 
-static inline struct x25_sock *x25_sk(const struct sock *sk)
-{
-	return (struct x25_sock *)sk;
-}
+#define x25_sk(ptr) container_of_const(ptr, struct x25_sock, sk)
 
 /* af_x25.c */
 extern int  sysctl_x25_restart_request_timeout;
-- 
2.40.0.rc2.332.ga46443480c-goog

Re: [PATCH net-next 08/10] x25: preserve const qualifier in [a]x25_sk()

[Simon Horman]

On Fri, Mar 17, 2023 at 03:55:37PM +0000, Eric Dumazet wrote:
> We can change [a]x25_sk() to propagate their argument const qualifier,
> thanks to container_of_const().
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

[PATCH net-next 09/10] mptcp: preserve const qualifier in mptcp_sk()

[Eric Dumazet]

We can change mptcp_sk() to propagate its argument const qualifier,
thanks to container_of_const().

We need to change few things to avoid build errors:

mptcp_set_datafin_timeout() and mptcp_rtx_head() have to accept
non-const sk pointers.

@msk local variable in mptcp_pending_tail() must be const.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 net/mptcp/protocol.c | 2 +-
 net/mptcp/protocol.h | 9 +++------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 3005a5adf715e8d147c119b0b4c13fcc58fe99f6..8c6b6d2643311b1e30f681e2ae843350342ca6e6 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -459,7 +459,7 @@ static bool mptcp_pending_data_fin(struct sock *sk, u64 *seq)
 	return false;
 }
 
-static void mptcp_set_datafin_timeout(const struct sock *sk)
+static void mptcp_set_datafin_timeout(struct sock *sk)
 {
 	struct inet_connection_sock *icsk = inet_csk(sk);
 	u32 retransmits;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 61fd8eabfca2028680e04558b4baca9f48bbaaaa..4ed8ffffb1ca473179217e640a23bc268742628d 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -333,10 +333,7 @@ static inline void msk_owned_by_me(const struct mptcp_sock *msk)
 	sock_owned_by_me((const struct sock *)msk);
 }
 
-static inline struct mptcp_sock *mptcp_sk(const struct sock *sk)
-{
-	return (struct mptcp_sock *)sk;
-}
+#define mptcp_sk(ptr) container_of_const(ptr, struct mptcp_sock, sk.icsk_inet.sk)
 
 /* the msk socket don't use the backlog, also account for the bulk
  * free memory
@@ -370,7 +367,7 @@ static inline struct mptcp_data_frag *mptcp_send_next(struct sock *sk)
 
 static inline struct mptcp_data_frag *mptcp_pending_tail(const struct sock *sk)
 {
-	struct mptcp_sock *msk = mptcp_sk(sk);
+	const struct mptcp_sock *msk = mptcp_sk(sk);
 
 	if (!msk->first_pending)
 		return NULL;
@@ -381,7 +378,7 @@ static inline struct mptcp_data_frag *mptcp_pending_tail(const struct sock *sk)
 	return list_last_entry(&msk->rtx_queue, struct mptcp_data_frag, list);
 }
 
-static inline struct mptcp_data_frag *mptcp_rtx_head(const struct sock *sk)
+static inline struct mptcp_data_frag *mptcp_rtx_head(struct sock *sk)
 {
 	struct mptcp_sock *msk = mptcp_sk(sk);
 
-- 
2.40.0.rc2.332.ga46443480c-goog

Re: [PATCH net-next 09/10] mptcp: preserve const qualifier in mptcp_sk()

[Simon Horman]

On Fri, Mar 17, 2023 at 03:55:38PM +0000, Eric Dumazet wrote:
> We can change mptcp_sk() to propagate its argument const qualifier,
> thanks to container_of_const().
> 
> We need to change few things to avoid build errors:
> 
> mptcp_set_datafin_timeout() and mptcp_rtx_head() have to accept
> non-const sk pointers.
> 
> @msk local variable in mptcp_pending_tail() must be const.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Matthieu Baerts <matthieu.baerts@tessares.net>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

Re: [PATCH net-next 09/10] mptcp: preserve const qualifier in mptcp_sk()

[Matthieu Baerts]

Hi Eric,

On 17/03/2023 16:55, Eric Dumazet wrote:
> We can change mptcp_sk() to propagate its argument const qualifier,
> thanks to container_of_const().
> 
> We need to change few things to avoid build errors:
> 
> mptcp_set_datafin_timeout() and mptcp_rtx_head() have to accept
> non-const sk pointers.
> 
> @msk local variable in mptcp_pending_tail() must be const.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Matthieu Baerts <matthieu.baerts@tessares.net>

Good idea!

Thank you for this patch and for having Cced me.

It looks good to me. I just have one question below if you don't mind.

(...)

> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 61fd8eabfca2028680e04558b4baca9f48bbaaaa..4ed8ffffb1ca473179217e640a23bc268742628d 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h

(...)

> @@ -381,7 +378,7 @@ static inline struct mptcp_data_frag *mptcp_pending_tail(const struct sock *sk)
>  	return list_last_entry(&msk->rtx_queue, struct mptcp_data_frag, list);
>  }
>  
> -static inline struct mptcp_data_frag *mptcp_rtx_head(const struct sock *sk)
> +static inline struct mptcp_data_frag *mptcp_rtx_head(struct sock *sk)

It was not clear to me why you had to remove the "const" qualifier here
and not just have to add one when assigning the msk just below. But then
I looked at what was behind the list_first_entry_or_null() macro used in
this function and understood what was the issue.


My naive approach would be to modify this macro but I guess we don't
want to go down that road, right?

-------------------- 8< --------------------
diff --git a/include/linux/list.h b/include/linux/list.h
index f10344dbad4d..cd770766f451 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -550,7 +550,7 @@ static inline void list_splice_tail_init(struct
list_head *list,
  * Note that if the list is empty, it returns NULL.
  */
 #define list_first_entry_or_null(ptr, type, member) ({ \
-       struct list_head *head__ = (ptr); \
+       const struct list_head *head__ = (ptr); \
        struct list_head *pos__ = READ_ONCE(head__->next); \
        pos__ != head__ ? list_entry(pos__, type, member) : NULL; \
 })
-------------------- 8< --------------------


It looks safe to me to do that but I would not trust myself on a Friday
evening :)
(I'm sure I'm missing something, I'm sorry if it is completely wrong)

Anyway if we cannot modify list_first_entry_or_null() one way or
another, I'm fine with your modification!

Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>

Cheers,
Matt

>  {
>  	struct mptcp_sock *msk = mptcp_sk(sk);
>  

-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Re: [PATCH net-next 09/10] mptcp: preserve const qualifier in mptcp_sk()

[Eric Dumazet]

On Fri, Mar 17, 2023 at 10:32 AM Matthieu Baerts
<matthieu.baerts@tessares.net> wrote:
>
> Hi Eric,
>
> On 17/03/2023 16:55, Eric Dumazet wrote:
> > We can change mptcp_sk() to propagate its argument const qualifier,
> > thanks to container_of_const().
> >
> > We need to change few things to avoid build errors:
> >
> > mptcp_set_datafin_timeout() and mptcp_rtx_head() have to accept
> > non-const sk pointers.
> >
> > @msk local variable in mptcp_pending_tail() must be const.
> >
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Cc: Matthieu Baerts <matthieu.baerts@tessares.net>
>
> Good idea!
>
> Thank you for this patch and for having Cced me.
>
> It looks good to me. I just have one question below if you don't mind.
>
> (...)
>
> > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> > index 61fd8eabfca2028680e04558b4baca9f48bbaaaa..4ed8ffffb1ca473179217e640a23bc268742628d 100644
> > --- a/net/mptcp/protocol.h
> > +++ b/net/mptcp/protocol.h
>
> (...)
>
> > @@ -381,7 +378,7 @@ static inline struct mptcp_data_frag *mptcp_pending_tail(const struct sock *sk)
> >       return list_last_entry(&msk->rtx_queue, struct mptcp_data_frag, list);
> >  }
> >
> > -static inline struct mptcp_data_frag *mptcp_rtx_head(const struct sock *sk)
> > +static inline struct mptcp_data_frag *mptcp_rtx_head(struct sock *sk)
>
> It was not clear to me why you had to remove the "const" qualifier here
> and not just have to add one when assigning the msk just below. But then
> I looked at what was behind the list_first_entry_or_null() macro used in
> this function and understood what was the issue.
>
>
> My naive approach would be to modify this macro but I guess we don't
> want to go down that road, right?
>
> -------------------- 8< --------------------
> diff --git a/include/linux/list.h b/include/linux/list.h
> index f10344dbad4d..cd770766f451 100644
> --- a/include/linux/list.h
> +++ b/include/linux/list.h
> @@ -550,7 +550,7 @@ static inline void list_splice_tail_init(struct
> list_head *list,
>   * Note that if the list is empty, it returns NULL.
>   */
>  #define list_first_entry_or_null(ptr, type, member) ({ \
> -       struct list_head *head__ = (ptr); \
> +       const struct list_head *head__ = (ptr); \
>         struct list_head *pos__ = READ_ONCE(head__->next); \
>         pos__ != head__ ? list_entry(pos__, type, member) : NULL; \
>  })
> -------------------- 8< --------------------

This could work, but it is a bit awkward.

mptcp_rtx_head() is used  in a context where we are changing the
socket, not during a readonly lookup ?

>
>
> It looks safe to me to do that but I would not trust myself on a Friday
> evening :)
> (I'm sure I'm missing something, I'm sorry if it is completely wrong)
>
> Anyway if we cannot modify list_first_entry_or_null() one way or
> another, I'm fine with your modification!
>
> Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>

Thanks !

Re: [PATCH net-next 09/10] mptcp: preserve const qualifier in mptcp_sk()

[Matthieu Baerts]

Hi Eric,

Thank you for your quick reply!

On 17/03/2023 18:47, Eric Dumazet wrote:
> On Fri, Mar 17, 2023 at 10:32 AM Matthieu Baerts
> <matthieu.baerts@tessares.net> wrote:
>> On 17/03/2023 16:55, Eric Dumazet wrote:
>>
>> (...)
>>
>>> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
>>> index 61fd8eabfca2028680e04558b4baca9f48bbaaaa..4ed8ffffb1ca473179217e640a23bc268742628d 100644
>>> --- a/net/mptcp/protocol.h
>>> +++ b/net/mptcp/protocol.h
>>
>> (...)
>>
>>> @@ -381,7 +378,7 @@ static inline struct mptcp_data_frag *mptcp_pending_tail(const struct sock *sk)
>>>       return list_last_entry(&msk->rtx_queue, struct mptcp_data_frag, list);
>>>  }
>>>
>>> -static inline struct mptcp_data_frag *mptcp_rtx_head(const struct sock *sk)
>>> +static inline struct mptcp_data_frag *mptcp_rtx_head(struct sock *sk)
>>
>> It was not clear to me why you had to remove the "const" qualifier here
>> and not just have to add one when assigning the msk just below. But then
>> I looked at what was behind the list_first_entry_or_null() macro used in
>> this function and understood what was the issue.
>>
>>
>> My naive approach would be to modify this macro but I guess we don't
>> want to go down that road, right?
>>
>> -------------------- 8< --------------------
>> diff --git a/include/linux/list.h b/include/linux/list.h
>> index f10344dbad4d..cd770766f451 100644
>> --- a/include/linux/list.h
>> +++ b/include/linux/list.h
>> @@ -550,7 +550,7 @@ static inline void list_splice_tail_init(struct
>> list_head *list,
>>   * Note that if the list is empty, it returns NULL.
>>   */
>>  #define list_first_entry_or_null(ptr, type, member) ({ \
>> -       struct list_head *head__ = (ptr); \
>> +       const struct list_head *head__ = (ptr); \
>>         struct list_head *pos__ = READ_ONCE(head__->next); \
>>         pos__ != head__ ? list_entry(pos__, type, member) : NULL; \
>>  })
>> -------------------- 8< --------------------
> 
> This could work, but it is a bit awkward.
> 
> mptcp_rtx_head() is used  in a context where we are changing the
> socket, not during a readonly lookup ?

Indeed, you are right. It is currently only used in a context where we
are changing the socket.

I can see cases where a new packets scheduler might just need to check
if the rtx queue is empty but then we should probably add a new helper
using list_empty() instead.

So yes, no need to change anything!

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

[PATCH net-next 10/10] tcp: preserve const qualifier in tcp_sk()

[Eric Dumazet]

We can change tcp_sk() to propagate its argument const qualifier,
thanks to container_of_const().

We have two places where a const sock pointer has to be upgraded
to a write one. We have been using const qualifier for lockless
listeners to clearly identify points where writes could happen.

Add tcp_sk_rw() helper to better document these.

tcp_inbound_md5_hash(), __tcp_grow_window(), tcp_reset_check()
and tcp_rack_reo_wnd() get an additional const qualififer
for their @tp local variables.

smc_check_reset_syn_req() also needs a similar change.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/tcp.h      | 10 ++++++----
 include/net/tcp.h        |  2 +-
 net/ipv4/tcp.c           |  2 +-
 net/ipv4/tcp_input.c     |  4 ++--
 net/ipv4/tcp_minisocks.c |  5 +++--
 net/ipv4/tcp_output.c    |  9 +++++++--
 net/ipv4/tcp_recovery.c  |  2 +-
 7 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index ca7f05a130d2d14d530207adcc1cc50b7b830c80..b4c08ac86983568a9511258708724da15d0b999e 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -472,10 +472,12 @@ enum tsq_flags {
 	TCPF_MTU_REDUCED_DEFERRED	= (1UL << TCP_MTU_REDUCED_DEFERRED),
 };
 
-static inline struct tcp_sock *tcp_sk(const struct sock *sk)
-{
-	return (struct tcp_sock *)sk;
-}
+#define tcp_sk(ptr) container_of_const(ptr, struct tcp_sock, inet_conn.icsk_inet.sk)
+
+/* Variant of tcp_sk() upgrading a const sock to a read/write tcp socket.
+ * Used in context of (lockless) tcp listeners.
+ */
+#define tcp_sk_rw(ptr) container_of(ptr, struct tcp_sock, inet_conn.icsk_inet.sk)
 
 struct tcp_timewait_sock {
 	struct inet_timewait_sock tw_sk;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index db9f828e9d1ee4546951a408935b6c5f90851a93..a0a91a988272710470cd20f22e02e49476513239 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -529,7 +529,7 @@ static inline void tcp_synq_overflow(const struct sock *sk)
 
 	last_overflow = READ_ONCE(tcp_sk(sk)->rx_opt.ts_recent_stamp);
 	if (!time_between32(now, last_overflow, last_overflow + HZ))
-		WRITE_ONCE(tcp_sk(sk)->rx_opt.ts_recent_stamp, now);
+		WRITE_ONCE(tcp_sk_rw(sk)->rx_opt.ts_recent_stamp, now);
 }
 
 /* syncookies: no recent synqueue overflow on this listening socket? */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 01569de651b65aa5641fb0c06e0fb81dc40cd85a..fd68d49490f2849a41397be5bf78dbf07cadd7ff 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -4570,7 +4570,7 @@ tcp_inbound_md5_hash(const struct sock *sk, const struct sk_buff *skb,
 	const __u8 *hash_location = NULL;
 	struct tcp_md5sig_key *hash_expected;
 	const struct tcphdr *th = tcp_hdr(skb);
-	struct tcp_sock *tp = tcp_sk(sk);
+	const struct tcp_sock *tp = tcp_sk(sk);
 	int genhash, l3index;
 	u8 newhash[16];
 
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 754ddbe0577f139d209032b4f15787392fe9a1d5..2b75cd9e2e92ea1b6e8e16485a8792790a905ade 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -458,7 +458,7 @@ static void tcp_sndbuf_expand(struct sock *sk)
 static int __tcp_grow_window(const struct sock *sk, const struct sk_buff *skb,
 			     unsigned int skbtruesize)
 {
-	struct tcp_sock *tp = tcp_sk(sk);
+	const struct tcp_sock *tp = tcp_sk(sk);
 	/* Optimize this! */
 	int truesize = tcp_win_from_space(sk, skbtruesize) >> 1;
 	int window = tcp_win_from_space(sk, READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_rmem[2])) >> 1;
@@ -5693,7 +5693,7 @@ static void tcp_urg(struct sock *sk, struct sk_buff *skb, const struct tcphdr *t
  */
 static bool tcp_reset_check(const struct sock *sk, const struct sk_buff *skb)
 {
-	struct tcp_sock *tp = tcp_sk(sk);
+	const struct tcp_sock *tp = tcp_sk(sk);
 
 	return unlikely(TCP_SKB_CB(skb)->seq == (tp->rcv_nxt - 1) &&
 			(1 << sk->sk_state) & (TCPF_CLOSE_WAIT | TCPF_LAST_ACK |
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 9a7ef7732c24c94d4a01d5911ebe51f21371a457..dac0d62120e623ad3f206daa1785eddb39452fd6 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -463,7 +463,7 @@ void tcp_ca_openreq_child(struct sock *sk, const struct dst_entry *dst)
 }
 EXPORT_SYMBOL_GPL(tcp_ca_openreq_child);
 
-static void smc_check_reset_syn_req(struct tcp_sock *oldtp,
+static void smc_check_reset_syn_req(const struct tcp_sock *oldtp,
 				    struct request_sock *req,
 				    struct tcp_sock *newtp)
 {
@@ -492,7 +492,8 @@ struct sock *tcp_create_openreq_child(const struct sock *sk,
 	const struct inet_request_sock *ireq = inet_rsk(req);
 	struct tcp_request_sock *treq = tcp_rsk(req);
 	struct inet_connection_sock *newicsk;
-	struct tcp_sock *oldtp, *newtp;
+	const struct tcp_sock *oldtp;
+	struct tcp_sock *newtp;
 	u32 seq;
 
 	if (!newsk)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f7e00d90a7304cdbaee493d994c6ab1063392d34..b9e07f1951419c3a2858eea0c26f604b96f0be70 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -4127,8 +4127,13 @@ int tcp_rtx_synack(const struct sock *sk, struct request_sock *req)
 	if (!res) {
 		TCP_INC_STATS(sock_net(sk), TCP_MIB_RETRANSSEGS);
 		NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPSYNRETRANS);
-		if (unlikely(tcp_passive_fastopen(sk)))
-			tcp_sk(sk)->total_retrans++;
+		if (unlikely(tcp_passive_fastopen(sk))) {
+			/* sk has const attribute because listeners are lockless.
+			 * However in this case, we are dealing with a passive fastopen
+			 * socket thus we can change total_retrans value.
+			 */
+			tcp_sk_rw(sk)->total_retrans++;
+		}
 		trace_tcp_retransmit_synack(sk, req);
 	}
 	return res;
diff --git a/net/ipv4/tcp_recovery.c b/net/ipv4/tcp_recovery.c
index 50abaa941387d3e7328b6859ea5118937fc12be4..acf4869c5d3b568227aca71d95a765493e452a85 100644
--- a/net/ipv4/tcp_recovery.c
+++ b/net/ipv4/tcp_recovery.c
@@ -4,7 +4,7 @@
 
 static u32 tcp_rack_reo_wnd(const struct sock *sk)
 {
-	struct tcp_sock *tp = tcp_sk(sk);
+	const struct tcp_sock *tp = tcp_sk(sk);
 
 	if (!tp->reord_seen) {
 		/* If reordering has not been observed, be aggressive during
-- 
2.40.0.rc2.332.ga46443480c-goog

Re: [PATCH net-next 10/10] tcp: preserve const qualifier in tcp_sk()

[Simon Horman]

On Fri, Mar 17, 2023 at 03:55:39PM +0000, Eric Dumazet wrote:
> We can change tcp_sk() to propagate its argument const qualifier,
> thanks to container_of_const().
> 
> We have two places where a const sock pointer has to be upgraded
> to a write one. We have been using const qualifier for lockless
> listeners to clearly identify points where writes could happen.
> 
> Add tcp_sk_rw() helper to better document these.
> 
> tcp_inbound_md5_hash(), __tcp_grow_window(), tcp_reset_check()
> and tcp_rack_reo_wnd() get an additional const qualififer
> for their @tp local variables.
> 
> smc_check_reset_syn_req() also needs a similar change.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Upgrading gives me the heebie-jeebies, but ok.

Reviewed-by: Simon Horman <simon.horman@corigine.com>

Re: [PATCH net-next 00/10] net: better const qualifier awareness

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri, 17 Mar 2023 15:55:29 +0000 you wrote:
> This is a follow-up of d27d367d3b78 ("inet: better const qualifier awareness")
> 
> Adopting container_of_const() to perform (struct sock *)->(protocol sock *)
> operation is allowing us to propagate const qualifier and thus detect
> misuses at compile time.
> 
> Most conversions are trivial, because most protocols did not adopt yet
> const sk pointers where it could make sense.
> 
> [...]

Here is the summary with links:
  - [net-next,01/10] udp: preserve const qualifier in udp_sk()
    https://git.kernel.org/netdev/net-next/c/94c540fbfc80
  - [net-next,02/10] af_packet: preserve const qualifier in pkt_sk()
    https://git.kernel.org/netdev/net-next/c/68ac9a8b6e65
  - [net-next,03/10] raw: preserve const qualifier in raw_sk()
    https://git.kernel.org/netdev/net-next/c/0a2db4630b72
  - [net-next,04/10] ipv6: raw: preserve const qualifier in raw6_sk()
    https://git.kernel.org/netdev/net-next/c/47fcae28b9ec
  - [net-next,05/10] dccp: preserve const qualifier in dccp_sk()
    https://git.kernel.org/netdev/net-next/c/ae6084b73992
  - [net-next,06/10] af_unix: preserve const qualifier in unix_sk()
    https://git.kernel.org/netdev/net-next/c/b064ba9c3cfa
  - [net-next,07/10] smc: preserve const qualifier in smc_sk()
    https://git.kernel.org/netdev/net-next/c/407db475d505
  - [net-next,08/10] x25: preserve const qualifier in [a]x25_sk()
    https://git.kernel.org/netdev/net-next/c/c7154ca8e075
  - [net-next,09/10] mptcp: preserve const qualifier in mptcp_sk()
    https://git.kernel.org/netdev/net-next/c/403a40f2304d
  - [net-next,10/10] tcp: preserve const qualifier in tcp_sk()
    https://git.kernel.org/netdev/net-next/c/e9d9da91548b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html