* [PATCH net-next 0/8] inet: better const qualifier awareness
@ 2023-03-15 15:42 Eric Dumazet
2023-03-15 15:42 ` [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk() Eric Dumazet
` (7 more replies)
0 siblings, 8 replies; 32+ messages in thread
From: Eric Dumazet @ 2023-03-15 15:42 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, netdev, eric.dumazet, Eric Dumazet
inet_sk() can be changed to propagate const qualifier.
Following patches adds more const qualifiers.
Eric Dumazet (8):
inet: preserve const qualifier in inet_sk()
ipv4: constify ip_mc_sf_allow() socket argument
udp: constify __udp_is_mcast_sock() socket argument
ipv6: constify inet6_mc_check()
udp6: constify __udp_v6_is_mcast_sock() socket argument
ipv6: raw: constify raw_v6_match() socket argument
ipv4: raw: constify raw_v4_match() socket argument
inet_diag: constify raw_lookup() socket argument
include/linux/igmp.h | 2 +-
include/net/addrconf.h | 2 +-
include/net/inet_sock.h | 9 +++++----
include/net/raw.h | 2 +-
include/net/rawv6.h | 2 +-
include/trace/events/sock.h | 4 ++--
include/trace/events/tcp.h | 2 +-
net/ipv4/igmp.c | 4 ++--
net/ipv4/ip_output.c | 5 +++--
net/ipv4/raw.c | 4 ++--
net/ipv4/raw_diag.c | 2 +-
net/ipv4/udp.c | 4 ++--
net/ipv6/mcast.c | 8 ++++----
net/ipv6/ping.c | 2 +-
net/ipv6/raw.c | 2 +-
net/ipv6/udp.c | 6 +++---
net/mptcp/sockopt.c | 2 +-
security/lsm_audit.c | 4 ++--
18 files changed, 34 insertions(+), 32 deletions(-)
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-15 15:42 [PATCH net-next 0/8] inet: better const qualifier awareness Eric Dumazet
@ 2023-03-15 15:42 ` Eric Dumazet
2023-03-15 18:52 ` Simon Horman
2023-03-15 21:28 ` Jakub Kicinski
2023-03-15 15:42 ` [PATCH net-next 2/8] ipv4: constify ip_mc_sf_allow() socket argument Eric Dumazet
` (6 subsequent siblings)
7 siblings, 2 replies; 32+ messages in thread
From: Eric Dumazet @ 2023-03-15 15:42 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, netdev, eric.dumazet, Eric Dumazet
We can change inet_sk() to propagate const qualifier of its argument.
This should avoid some potential errors caused by accidental
(const -> not_const) promotion.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/inet_sock.h | 9 +++++----
include/trace/events/sock.h | 4 ++--
include/trace/events/tcp.h | 2 +-
net/ipv4/ip_output.c | 5 +++--
net/ipv6/ping.c | 2 +-
net/ipv6/udp.c | 2 +-
net/mptcp/sockopt.c | 2 +-
security/lsm_audit.c | 4 ++--
8 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index 51857117ac0995cee20c1e62752c470d2a473fa8..6eb8235be67f8b8265cd86782aed2b489e8850ee 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -305,10 +305,11 @@ static inline struct sock *skb_to_full_sk(const struct sk_buff *skb)
return sk_to_full_sk(skb->sk);
}
-static inline struct inet_sock *inet_sk(const struct sock *sk)
-{
- return (struct inet_sock *)sk;
-}
+#define inet_sk(sk) \
+ _Generic(sk, \
+ const struct sock * : ((const struct inet_sock *)(sk)), \
+ struct sock * : ((struct inet_sock *)(sk)) \
+ )
static inline void __inet_sk_copy_descendant(struct sock *sk_to,
const struct sock *sk_from,
diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
index 03d19fc562f872d68cfb2349797f0924cfba718b..fd206a6ab5b85a8eb50fc9303abc28d74c50ea07 100644
--- a/include/trace/events/sock.h
+++ b/include/trace/events/sock.h
@@ -158,7 +158,7 @@ TRACE_EVENT(inet_sock_set_state,
),
TP_fast_assign(
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
struct in6_addr *pin6;
__be32 *p32;
@@ -222,7 +222,7 @@ TRACE_EVENT(inet_sk_error_report,
),
TP_fast_assign(
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
struct in6_addr *pin6;
__be32 *p32;
diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
index 901b440238d5fc203b78cff1081b4b7e1e2eb4bd..bf06db8d2046c4a7f59070b724ce6fc7762f9d4b 100644
--- a/include/trace/events/tcp.h
+++ b/include/trace/events/tcp.h
@@ -67,7 +67,7 @@ DECLARE_EVENT_CLASS(tcp_event_sk_skb,
),
TP_fast_assign(
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
__be32 *p32;
__entry->skbaddr = skb;
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index e7bef36ce26f5b5e5503eaf14319b2465b779598..cb04dbad9ea474fcaa4b5ba31c4a37299c4e1a8d 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -129,7 +129,8 @@ int ip_local_out(struct net *net, struct sock *sk, struct sk_buff *skb)
}
EXPORT_SYMBOL_GPL(ip_local_out);
-static inline int ip_select_ttl(struct inet_sock *inet, struct dst_entry *dst)
+static inline int ip_select_ttl(const struct inet_sock *inet,
+ const struct dst_entry *dst)
{
int ttl = inet->uc_ttl;
@@ -146,7 +147,7 @@ int ip_build_and_send_pkt(struct sk_buff *skb, const struct sock *sk,
__be32 saddr, __be32 daddr, struct ip_options_rcu *opt,
u8 tos)
{
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
struct rtable *rt = skb_rtable(skb);
struct net *net = sock_net(sk);
struct iphdr *iph;
diff --git a/net/ipv6/ping.c b/net/ipv6/ping.c
index 808983bc2ec9f92be7d13237fb7d52b6423ccf23..c4835dbdfcff7912465713cdac9914b0f5585972 100644
--- a/net/ipv6/ping.c
+++ b/net/ipv6/ping.c
@@ -237,7 +237,7 @@ static int ping_v6_seq_show(struct seq_file *seq, void *v)
seq_puts(seq, IPV6_SEQ_DGRAM_HEADER);
} else {
int bucket = ((struct ping_iter_state *) seq->private)->bucket;
- struct inet_sock *inet = inet_sk(v);
+ struct inet_sock *inet = inet_sk((struct sock *)v);
__u16 srcp = ntohs(inet->inet_sport);
__u16 destp = ntohs(inet->inet_dport);
ip6_dgram_sock_seq_show(seq, v, srcp, destp, bucket);
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 9fb2f33ee3a76a09bbe15a9aaf1371a804f91ee2..ab4ae886235ac9557219c901c5041adfa8b026ef 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1708,7 +1708,7 @@ int udp6_seq_show(struct seq_file *seq, void *v)
seq_puts(seq, IPV6_SEQ_DGRAM_HEADER);
} else {
int bucket = ((struct udp_iter_state *)seq->private)->bucket;
- struct inet_sock *inet = inet_sk(v);
+ const struct inet_sock *inet = inet_sk((const struct sock *)v);
__u16 srcp = ntohs(inet->inet_sport);
__u16 destp = ntohs(inet->inet_dport);
__ip6_dgram_sock_seq_show(seq, v, srcp, destp,
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 8a9656248b0f09e78ebf3d6d787449f33505b81f..5cef4d3d21ac824ab6d3a5ee8be3bd00cce63925 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -1046,7 +1046,7 @@ static int mptcp_getsockopt_tcpinfo(struct mptcp_sock *msk, char __user *optval,
static void mptcp_get_sub_addrs(const struct sock *sk, struct mptcp_subflow_addrs *a)
{
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
memset(a, 0, sizeof(*a));
diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index a7355b4b9bb86d173f94e43109148bc0ea27d1b3..00d3bdd386e294ecd562bfa8ce502bf179ad32d9 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -317,7 +317,7 @@ static void dump_common_audit_data(struct audit_buffer *ab,
switch (sk->sk_family) {
case AF_INET: {
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
print_ipv4_addr(ab, inet->inet_rcv_saddr,
inet->inet_sport,
@@ -329,7 +329,7 @@ static void dump_common_audit_data(struct audit_buffer *ab,
}
#if IS_ENABLED(CONFIG_IPV6)
case AF_INET6: {
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
print_ipv6_addr(ab, &sk->sk_v6_rcv_saddr,
inet->inet_sport,
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH net-next 2/8] ipv4: constify ip_mc_sf_allow() socket argument
2023-03-15 15:42 [PATCH net-next 0/8] inet: better const qualifier awareness Eric Dumazet
2023-03-15 15:42 ` [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk() Eric Dumazet
@ 2023-03-15 15:42 ` Eric Dumazet
2023-03-15 18:52 ` Simon Horman
2023-03-15 15:42 ` [PATCH net-next 3/8] udp: constify __udp_is_mcast_sock() " Eric Dumazet
` (5 subsequent siblings)
7 siblings, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2023-03-15 15:42 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, netdev, eric.dumazet, Eric Dumazet
This clarifies ip_mc_sf_allow() intent.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/igmp.h | 2 +-
net/ipv4/igmp.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/igmp.h b/include/linux/igmp.h
index b19d3284551f083d0eec3353cd8ec1f486ae4b42..ebf4349a53af7888104e8c5fe0d7af0e5604ae69 100644
--- a/include/linux/igmp.h
+++ b/include/linux/igmp.h
@@ -122,7 +122,7 @@ extern int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
sockptr_t optval, sockptr_t optlen);
extern int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
sockptr_t optval, size_t offset);
-extern int ip_mc_sf_allow(struct sock *sk, __be32 local, __be32 rmt,
+extern int ip_mc_sf_allow(const struct sock *sk, __be32 local, __be32 rmt,
int dif, int sdif);
extern void ip_mc_init_dev(struct in_device *);
extern void ip_mc_destroy_dev(struct in_device *);
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index c920aa9a62a988bf91a5420e59eb5878c271bf9a..48ff5f13e7979dc00da60b466ee2e74ddce0891b 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -2638,10 +2638,10 @@ int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
/*
* check if a multicast source filter allows delivery for a given <src,dst,intf>
*/
-int ip_mc_sf_allow(struct sock *sk, __be32 loc_addr, __be32 rmt_addr,
+int ip_mc_sf_allow(const struct sock *sk, __be32 loc_addr, __be32 rmt_addr,
int dif, int sdif)
{
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
struct ip_mc_socklist *pmc;
struct ip_sf_socklist *psl;
int i;
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH net-next 3/8] udp: constify __udp_is_mcast_sock() socket argument
2023-03-15 15:42 [PATCH net-next 0/8] inet: better const qualifier awareness Eric Dumazet
2023-03-15 15:42 ` [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk() Eric Dumazet
2023-03-15 15:42 ` [PATCH net-next 2/8] ipv4: constify ip_mc_sf_allow() socket argument Eric Dumazet
@ 2023-03-15 15:42 ` Eric Dumazet
2023-03-15 18:53 ` Simon Horman
2023-03-15 15:42 ` [PATCH net-next 4/8] ipv6: constify inet6_mc_check() Eric Dumazet
` (4 subsequent siblings)
7 siblings, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2023-03-15 15:42 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, netdev, eric.dumazet, Eric Dumazet
This clarifies __udp_is_mcast_sock() intent.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/udp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index dc8feb54d835f0824aa6833e36db34f686c456ec..aa32afd871ee50968f7bb8152401be60dece1454 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -578,12 +578,12 @@ struct sock *udp4_lib_lookup(struct net *net, __be32 saddr, __be16 sport,
EXPORT_SYMBOL_GPL(udp4_lib_lookup);
#endif
-static inline bool __udp_is_mcast_sock(struct net *net, struct sock *sk,
+static inline bool __udp_is_mcast_sock(struct net *net, const struct sock *sk,
__be16 loc_port, __be32 loc_addr,
__be16 rmt_port, __be32 rmt_addr,
int dif, int sdif, unsigned short hnum)
{
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
if (!net_eq(sock_net(sk), net) ||
udp_sk(sk)->udp_port_hash != hnum ||
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH net-next 4/8] ipv6: constify inet6_mc_check()
2023-03-15 15:42 [PATCH net-next 0/8] inet: better const qualifier awareness Eric Dumazet
` (2 preceding siblings ...)
2023-03-15 15:42 ` [PATCH net-next 3/8] udp: constify __udp_is_mcast_sock() " Eric Dumazet
@ 2023-03-15 15:42 ` Eric Dumazet
2023-03-15 18:53 ` Simon Horman
2023-03-15 15:42 ` [PATCH net-next 5/8] udp6: constify __udp_v6_is_mcast_sock() socket argument Eric Dumazet
` (3 subsequent siblings)
7 siblings, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2023-03-15 15:42 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, netdev, eric.dumazet, Eric Dumazet
inet6_mc_check() is essentially a read-only function.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/addrconf.h | 2 +-
net/ipv6/mcast.c | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index c04f359655b86feed2b4b42cc69b90c63088238a..82da55101b5a30b2a5512d964429d2c5f73d03fd 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -223,7 +223,7 @@ int ipv6_sock_mc_drop(struct sock *sk, int ifindex,
const struct in6_addr *addr);
void __ipv6_sock_mc_close(struct sock *sk);
void ipv6_sock_mc_close(struct sock *sk);
-bool inet6_mc_check(struct sock *sk, const struct in6_addr *mc_addr,
+bool inet6_mc_check(const struct sock *sk, const struct in6_addr *mc_addr,
const struct in6_addr *src_addr);
int ipv6_dev_mc_inc(struct net_device *dev, const struct in6_addr *addr);
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 1c02160cf7a4c54f0d8687b8368e5f6151ab0bce..714cdc9e2b8edfb925a061a722c38b37b1c6088e 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -627,12 +627,12 @@ int ip6_mc_msfget(struct sock *sk, struct group_filter *gsf,
return 0;
}
-bool inet6_mc_check(struct sock *sk, const struct in6_addr *mc_addr,
+bool inet6_mc_check(const struct sock *sk, const struct in6_addr *mc_addr,
const struct in6_addr *src_addr)
{
- struct ipv6_pinfo *np = inet6_sk(sk);
- struct ipv6_mc_socklist *mc;
- struct ip6_sf_socklist *psl;
+ const struct ipv6_pinfo *np = inet6_sk(sk);
+ const struct ipv6_mc_socklist *mc;
+ const struct ip6_sf_socklist *psl;
bool rv = true;
rcu_read_lock();
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH net-next 5/8] udp6: constify __udp_v6_is_mcast_sock() socket argument
2023-03-15 15:42 [PATCH net-next 0/8] inet: better const qualifier awareness Eric Dumazet
` (3 preceding siblings ...)
2023-03-15 15:42 ` [PATCH net-next 4/8] ipv6: constify inet6_mc_check() Eric Dumazet
@ 2023-03-15 15:42 ` Eric Dumazet
2023-03-15 18:53 ` Simon Horman
2023-03-15 15:42 ` [PATCH net-next 6/8] ipv6: raw: constify raw_v6_match() " Eric Dumazet
` (2 subsequent siblings)
7 siblings, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2023-03-15 15:42 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, netdev, eric.dumazet, Eric Dumazet
This clarifies __udp_v6_is_mcast_sock() intent.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/udp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index ab4ae886235ac9557219c901c5041adfa8b026ef..d350e57c479299e732bd3595c1964acddde2d876 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -805,12 +805,12 @@ static int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
return 0;
}
-static bool __udp_v6_is_mcast_sock(struct net *net, struct sock *sk,
+static bool __udp_v6_is_mcast_sock(struct net *net, const struct sock *sk,
__be16 loc_port, const struct in6_addr *loc_addr,
__be16 rmt_port, const struct in6_addr *rmt_addr,
int dif, int sdif, unsigned short hnum)
{
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
if (!net_eq(sock_net(sk), net))
return false;
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH net-next 6/8] ipv6: raw: constify raw_v6_match() socket argument
2023-03-15 15:42 [PATCH net-next 0/8] inet: better const qualifier awareness Eric Dumazet
` (4 preceding siblings ...)
2023-03-15 15:42 ` [PATCH net-next 5/8] udp6: constify __udp_v6_is_mcast_sock() socket argument Eric Dumazet
@ 2023-03-15 15:42 ` Eric Dumazet
2023-03-15 18:54 ` Simon Horman
2023-03-15 15:42 ` [PATCH net-next 7/8] ipv4: raw: constify raw_v4_match() " Eric Dumazet
2023-03-15 15:42 ` [PATCH net-next 8/8] inet_diag: constify raw_lookup() " Eric Dumazet
7 siblings, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2023-03-15 15:42 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, netdev, eric.dumazet, Eric Dumazet
This clarifies raw_v6_match() intent.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/rawv6.h | 2 +-
net/ipv6/raw.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/net/rawv6.h b/include/net/rawv6.h
index bc70909625f60dcd819f50a258841d20e5ba0c68..82810cbe37984437569186783f39f166e89cb9b8 100644
--- a/include/net/rawv6.h
+++ b/include/net/rawv6.h
@@ -6,7 +6,7 @@
#include <net/raw.h>
extern struct raw_hashinfo raw_v6_hashinfo;
-bool raw_v6_match(struct net *net, struct sock *sk, unsigned short num,
+bool raw_v6_match(struct net *net, const struct sock *sk, unsigned short num,
const struct in6_addr *loc_addr,
const struct in6_addr *rmt_addr, int dif, int sdif);
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index bac9ba747bdecf8df24acb6c980a822d8f237403..6ac2f2690c44c96e9ac4cb7368ee7abdbeaf4334 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -64,7 +64,7 @@
struct raw_hashinfo raw_v6_hashinfo;
EXPORT_SYMBOL_GPL(raw_v6_hashinfo);
-bool raw_v6_match(struct net *net, struct sock *sk, unsigned short num,
+bool raw_v6_match(struct net *net, const struct sock *sk, unsigned short num,
const struct in6_addr *loc_addr,
const struct in6_addr *rmt_addr, int dif, int sdif)
{
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH net-next 7/8] ipv4: raw: constify raw_v4_match() socket argument
2023-03-15 15:42 [PATCH net-next 0/8] inet: better const qualifier awareness Eric Dumazet
` (5 preceding siblings ...)
2023-03-15 15:42 ` [PATCH net-next 6/8] ipv6: raw: constify raw_v6_match() " Eric Dumazet
@ 2023-03-15 15:42 ` Eric Dumazet
2023-03-15 18:54 ` Simon Horman
2023-03-15 15:42 ` [PATCH net-next 8/8] inet_diag: constify raw_lookup() " Eric Dumazet
7 siblings, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2023-03-15 15:42 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, netdev, eric.dumazet, Eric Dumazet
This clarifies raw_v4_match() intent.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/raw.h | 2 +-
net/ipv4/raw.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/net/raw.h b/include/net/raw.h
index 2c004c20ed996d1dbe07f2c8d25edd2ce03cca03..7ad15830cf38460f1fae3a187986d74faef6dd1d 100644
--- a/include/net/raw.h
+++ b/include/net/raw.h
@@ -22,7 +22,7 @@
extern struct proto raw_prot;
extern struct raw_hashinfo raw_v4_hashinfo;
-bool raw_v4_match(struct net *net, struct sock *sk, unsigned short num,
+bool raw_v4_match(struct net *net, const struct sock *sk, unsigned short num,
__be32 raddr, __be32 laddr, int dif, int sdif);
int raw_abort(struct sock *sk, int err);
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 94df935ee0c5a83a4b1393653b79ac6060b4f12a..3cf68695b40ddc0e79c8fbd62f317048cf5c88e3 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -116,10 +116,10 @@ void raw_unhash_sk(struct sock *sk)
}
EXPORT_SYMBOL_GPL(raw_unhash_sk);
-bool raw_v4_match(struct net *net, struct sock *sk, unsigned short num,
+bool raw_v4_match(struct net *net, const struct sock *sk, unsigned short num,
__be32 raddr, __be32 laddr, int dif, int sdif)
{
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
if (net_eq(sock_net(sk), net) && inet->inet_num == num &&
!(inet->inet_daddr && inet->inet_daddr != raddr) &&
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH net-next 8/8] inet_diag: constify raw_lookup() socket argument
2023-03-15 15:42 [PATCH net-next 0/8] inet: better const qualifier awareness Eric Dumazet
` (6 preceding siblings ...)
2023-03-15 15:42 ` [PATCH net-next 7/8] ipv4: raw: constify raw_v4_match() " Eric Dumazet
@ 2023-03-15 15:42 ` Eric Dumazet
2023-03-15 18:54 ` Simon Horman
7 siblings, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2023-03-15 15:42 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: David Ahern, netdev, eric.dumazet, Eric Dumazet
Now both raw_v4_match() and raw_v6_match() accept a const socket,
raw_lookup() can do the same to clarify its role.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/raw_diag.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/raw_diag.c b/net/ipv4/raw_diag.c
index 999321834b94a8f7f2a4996575b7cfaafb6fa2b7..bca49a844f01083cdfdade6f52852d48ddb36d70 100644
--- a/net/ipv4/raw_diag.c
+++ b/net/ipv4/raw_diag.c
@@ -34,7 +34,7 @@ raw_get_hashinfo(const struct inet_diag_req_v2 *r)
* use helper to figure it out.
*/
-static bool raw_lookup(struct net *net, struct sock *sk,
+static bool raw_lookup(struct net *net, const struct sock *sk,
const struct inet_diag_req_v2 *req)
{
struct inet_diag_req_raw *r = (void *)req;
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-15 15:42 ` [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk() Eric Dumazet
@ 2023-03-15 18:52 ` Simon Horman
2023-03-15 21:28 ` Jakub Kicinski
1 sibling, 0 replies; 32+ messages in thread
From: Simon Horman @ 2023-03-15 18:52 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, David Ahern,
netdev, eric.dumazet
On Wed, Mar 15, 2023 at 03:42:38PM +0000, Eric Dumazet wrote:
> We can change inet_sk() to propagate const qualifier of its argument.
>
> This should avoid some potential errors caused by accidental
> (const -> not_const) promotion.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Thanks Eric,
very nice to see this kind of change.
Reviewed-by: Simon Horman <simon.horman@corigine.com>
...
> diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
> index 51857117ac0995cee20c1e62752c470d2a473fa8..6eb8235be67f8b8265cd86782aed2b489e8850ee 100644
> --- a/include/net/inet_sock.h
> +++ b/include/net/inet_sock.h
> @@ -305,10 +305,11 @@ static inline struct sock *skb_to_full_sk(const struct sk_buff *skb)
> return sk_to_full_sk(skb->sk);
> }
>
> -static inline struct inet_sock *inet_sk(const struct sock *sk)
> -{
> - return (struct inet_sock *)sk;
> -}
> +#define inet_sk(sk) \
> + _Generic(sk, \
nit: checkpatch tells me that
There are spaces before tabs (before the '\') on the line above,
and this macro uses spaces for indentation.
> + const struct sock * : ((const struct inet_sock *)(sk)), \
> + struct sock * : ((struct inet_sock *)(sk)) \
> + )
>
> static inline void __inet_sk_copy_descendant(struct sock *sk_to,
> const struct sock *sk_from,
...
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 2/8] ipv4: constify ip_mc_sf_allow() socket argument
2023-03-15 15:42 ` [PATCH net-next 2/8] ipv4: constify ip_mc_sf_allow() socket argument Eric Dumazet
@ 2023-03-15 18:52 ` Simon Horman
0 siblings, 0 replies; 32+ messages in thread
From: Simon Horman @ 2023-03-15 18:52 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, David Ahern,
netdev, eric.dumazet
On Wed, Mar 15, 2023 at 03:42:39PM +0000, Eric Dumazet wrote:
> This clarifies ip_mc_sf_allow() intent.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 3/8] udp: constify __udp_is_mcast_sock() socket argument
2023-03-15 15:42 ` [PATCH net-next 3/8] udp: constify __udp_is_mcast_sock() " Eric Dumazet
@ 2023-03-15 18:53 ` Simon Horman
0 siblings, 0 replies; 32+ messages in thread
From: Simon Horman @ 2023-03-15 18:53 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, David Ahern,
netdev, eric.dumazet
On Wed, Mar 15, 2023 at 03:42:40PM +0000, Eric Dumazet wrote:
> This clarifies __udp_is_mcast_sock() intent.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 4/8] ipv6: constify inet6_mc_check()
2023-03-15 15:42 ` [PATCH net-next 4/8] ipv6: constify inet6_mc_check() Eric Dumazet
@ 2023-03-15 18:53 ` Simon Horman
0 siblings, 0 replies; 32+ messages in thread
From: Simon Horman @ 2023-03-15 18:53 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, David Ahern,
netdev, eric.dumazet
On Wed, Mar 15, 2023 at 03:42:41PM +0000, Eric Dumazet wrote:
> inet6_mc_check() is essentially a read-only function.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 5/8] udp6: constify __udp_v6_is_mcast_sock() socket argument
2023-03-15 15:42 ` [PATCH net-next 5/8] udp6: constify __udp_v6_is_mcast_sock() socket argument Eric Dumazet
@ 2023-03-15 18:53 ` Simon Horman
0 siblings, 0 replies; 32+ messages in thread
From: Simon Horman @ 2023-03-15 18:53 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, David Ahern,
netdev, eric.dumazet
On Wed, Mar 15, 2023 at 03:42:42PM +0000, Eric Dumazet wrote:
> This clarifies __udp_v6_is_mcast_sock() intent.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 6/8] ipv6: raw: constify raw_v6_match() socket argument
2023-03-15 15:42 ` [PATCH net-next 6/8] ipv6: raw: constify raw_v6_match() " Eric Dumazet
@ 2023-03-15 18:54 ` Simon Horman
0 siblings, 0 replies; 32+ messages in thread
From: Simon Horman @ 2023-03-15 18:54 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, David Ahern,
netdev, eric.dumazet
On Wed, Mar 15, 2023 at 03:42:43PM +0000, Eric Dumazet wrote:
> This clarifies raw_v6_match() intent.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 7/8] ipv4: raw: constify raw_v4_match() socket argument
2023-03-15 15:42 ` [PATCH net-next 7/8] ipv4: raw: constify raw_v4_match() " Eric Dumazet
@ 2023-03-15 18:54 ` Simon Horman
0 siblings, 0 replies; 32+ messages in thread
From: Simon Horman @ 2023-03-15 18:54 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, David Ahern,
netdev, eric.dumazet
On Wed, Mar 15, 2023 at 03:42:44PM +0000, Eric Dumazet wrote:
> This clarifies raw_v4_match() intent.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 8/8] inet_diag: constify raw_lookup() socket argument
2023-03-15 15:42 ` [PATCH net-next 8/8] inet_diag: constify raw_lookup() " Eric Dumazet
@ 2023-03-15 18:54 ` Simon Horman
0 siblings, 0 replies; 32+ messages in thread
From: Simon Horman @ 2023-03-15 18:54 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, David Ahern,
netdev, eric.dumazet
On Wed, Mar 15, 2023 at 03:42:45PM +0000, Eric Dumazet wrote:
> Now both raw_v4_match() and raw_v6_match() accept a const socket,
> raw_lookup() can do the same to clarify its role.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-15 15:42 ` [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk() Eric Dumazet
2023-03-15 18:52 ` Simon Horman
@ 2023-03-15 21:28 ` Jakub Kicinski
2023-03-15 22:37 ` Eric Dumazet
1 sibling, 1 reply; 32+ messages in thread
From: Jakub Kicinski @ 2023-03-15 21:28 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Paolo Abeni, David Ahern, netdev, eric.dumazet,
Greg Kroah-Hartman, Linus Torvalds
On Wed, 15 Mar 2023 15:42:38 +0000 Eric Dumazet wrote:
> -static inline struct inet_sock *inet_sk(const struct sock *sk)
> -{
> - return (struct inet_sock *)sk;
> -}
> +#define inet_sk(sk) \
> + _Generic(sk, \
> + const struct sock * : ((const struct inet_sock *)(sk)), \
> + struct sock * : ((struct inet_sock *)(sk)) \
> + )
Could we possibly use container_of_const() or define a new common
macro for this downcast? I'm worried it will spread and it's a bit
verbose.
(I think Greg is constifying things too so let me add him and Linus.)
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-15 21:28 ` Jakub Kicinski
@ 2023-03-15 22:37 ` Eric Dumazet
2023-03-15 23:08 ` Jakub Kicinski
2023-03-15 23:21 ` Linus Torvalds
0 siblings, 2 replies; 32+ messages in thread
From: Eric Dumazet @ 2023-03-15 22:37 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S . Miller, Paolo Abeni, David Ahern, netdev, eric.dumazet,
Greg Kroah-Hartman, Linus Torvalds
On Wed, Mar 15, 2023 at 2:28 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 15 Mar 2023 15:42:38 +0000 Eric Dumazet wrote:
> > -static inline struct inet_sock *inet_sk(const struct sock *sk)
> > -{
> > - return (struct inet_sock *)sk;
> > -}
> > +#define inet_sk(sk) \
> > + _Generic(sk, \
> > + const struct sock * : ((const struct inet_sock *)(sk)), \
> > + struct sock * : ((struct inet_sock *)(sk)) \
> > + )
>
> Could we possibly use container_of_const() or define a new common
> macro for this downcast? I'm worried it will spread and it's a bit
> verbose.
I did see container_of_const() but the default: case was not appealing to me.
Maybe something like this?
diff --git a/include/linux/container_of.h b/include/linux/container_of.h
index 713890c867bea78804defe1a015e3c362f40f85d..9a24d8db1f4c46166c07589bb084eda9b9ede8ba
100644
--- a/include/linux/container_of.h
+++ b/include/linux/container_of.h
@@ -35,4 +35,10 @@
default: ((type *)container_of(ptr, type, member)) \
)
+#define promote_to_type(ptr, oldtype, newtype) \
+ _Generic(ptr, \
+ const oldtype *: ((const newtype *)(ptr)), \
+ oldtype *: ((newtype *)(ptr)) \
+ )
+
#endif /* _LINUX_CONTAINER_OF_H */
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index 6eb8235be67f8b8265cd86782aed2b489e8850ee..96f41b3c1a3e4b6ad1c6d2058ca46686be282c03
100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -305,11 +305,7 @@ static inline struct sock *skb_to_full_sk(const
struct sk_buff *skb)
return sk_to_full_sk(skb->sk);
}
-#define inet_sk(sk) \
- _Generic(sk, \
- const struct sock * : ((const struct inet_sock
*)(sk)), \
- struct sock * : ((struct inet_sock *)(sk)) \
- )
+#define inet_sk(sk) promote_to_type(sk, struct sock, struct inet_sock)
static inline void __inet_sk_copy_descendant(struct sock *sk_to,
const struct sock *sk_from,
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-15 22:37 ` Eric Dumazet
@ 2023-03-15 23:08 ` Jakub Kicinski
2023-03-15 23:16 ` Eric Dumazet
2023-03-15 23:21 ` Linus Torvalds
1 sibling, 1 reply; 32+ messages in thread
From: Jakub Kicinski @ 2023-03-15 23:08 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Paolo Abeni, David Ahern, netdev, eric.dumazet,
Greg Kroah-Hartman, Linus Torvalds
On Wed, 15 Mar 2023 15:37:50 -0700 Eric Dumazet wrote:
> I did see container_of_const() but the default: case was not appealing to me.
>
> Maybe something like this?
>
> diff --git a/include/linux/container_of.h b/include/linux/container_of.h
> index 713890c867bea78804defe1a015e3c362f40f85d..9a24d8db1f4c46166c07589bb084eda9b9ede8ba
> 100644
> --- a/include/linux/container_of.h
> +++ b/include/linux/container_of.h
> @@ -35,4 +35,10 @@
> default: ((type *)container_of(ptr, type, member)) \
> )
>
> +#define promote_to_type(ptr, oldtype, newtype) \
> + _Generic(ptr, \
> + const oldtype *: ((const newtype *)(ptr)), \
> + oldtype *: ((newtype *)(ptr)) \
> + )
Perfect, I'll defer to you on whether you want to patch it on top
or repost the series.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-15 23:08 ` Jakub Kicinski
@ 2023-03-15 23:16 ` Eric Dumazet
0 siblings, 0 replies; 32+ messages in thread
From: Eric Dumazet @ 2023-03-15 23:16 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David S . Miller, Paolo Abeni, David Ahern, netdev, eric.dumazet,
Greg Kroah-Hartman, Linus Torvalds
On Wed, Mar 15, 2023 at 4:08 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 15 Mar 2023 15:37:50 -0700 Eric Dumazet wrote:
> > I did see container_of_const() but the default: case was not appealing to me.
> >
> > Maybe something like this?
> >
> > diff --git a/include/linux/container_of.h b/include/linux/container_of.h
> > index 713890c867bea78804defe1a015e3c362f40f85d..9a24d8db1f4c46166c07589bb084eda9b9ede8ba
> > 100644
> > --- a/include/linux/container_of.h
> > +++ b/include/linux/container_of.h
> > @@ -35,4 +35,10 @@
> > default: ((type *)container_of(ptr, type, member)) \
> > )
> >
> > +#define promote_to_type(ptr, oldtype, newtype) \
> > + _Generic(ptr, \
> > + const oldtype *: ((const newtype *)(ptr)), \
> > + oldtype *: ((newtype *)(ptr)) \
> > + )
>
> Perfect, I'll defer to you on whether you want to patch it on top
> or repost the series.
I'll post a v2 tomorrow, thank you.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-15 22:37 ` Eric Dumazet
2023-03-15 23:08 ` Jakub Kicinski
@ 2023-03-15 23:21 ` Linus Torvalds
2023-03-15 23:37 ` Eric Dumazet
2023-03-15 23:39 ` Linus Torvalds
1 sibling, 2 replies; 32+ messages in thread
From: Linus Torvalds @ 2023-03-15 23:21 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, David Ahern,
netdev, eric.dumazet, Greg Kroah-Hartman
On Wed, Mar 15, 2023 at 3:38 PM Eric Dumazet <edumazet@google.com> wrote:
>
> Maybe something like this?
Please no.
> +#define promote_to_type(ptr, oldtype, newtype) \
> + _Generic(ptr, \
> + const oldtype *: ((const newtype *)(ptr)), \
> + oldtype *: ((newtype *)(ptr)) \
> + )
That's just a very ugly way to just do a cast. It's wrong.
> +#define inet_sk(sk) promote_to_type(sk, struct sock, struct inet_sock)
This is horrid.
Why isn't this just doing
#define inet_sk(ptr) container_of(ptr, struct inet_sock, sk)
which is different from a plain cast in that it actually checks that
"yes, struct inet_sock has a member called 'sk' that has the right
type, so now we can convert from that sk to the containing structure".
That's very different from just randomly casting a pointer to another
pointer, like the current inet_sk() does, and like that disgusting
promote_to_type() macro does.
We really strive for proper type safety in the kernel. And that very
much means *not* doing random casts.
At least that "inet_sk(sk)" version using generics didn't take random
pointer types. But I really don't see why you don't just use
"container_of()", which is actually type-safe, and would allow "struct
inet_sock" to contain the "struct sock" somewhere else than in the
first field.
Hmm? Am I missing something that is happening in linux-next?
Linus
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-15 23:21 ` Linus Torvalds
@ 2023-03-15 23:37 ` Eric Dumazet
2023-03-15 23:47 ` Linus Torvalds
2023-03-15 23:39 ` Linus Torvalds
1 sibling, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2023-03-15 23:37 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, David Ahern,
netdev, eric.dumazet, Greg Kroah-Hartman
On Wed, Mar 15, 2023 at 4:21 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Wed, Mar 15, 2023 at 3:38 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > Maybe something like this?
>
> Please no.
>
> > +#define promote_to_type(ptr, oldtype, newtype) \
> > + _Generic(ptr, \
> > + const oldtype *: ((const newtype *)(ptr)), \
> > + oldtype *: ((newtype *)(ptr)) \
> > + )
>
> That's just a very ugly way to just do a cast. It's wrong.
>
> > +#define inet_sk(sk) promote_to_type(sk, struct sock, struct inet_sock)
>
> This is horrid.
>
> Why isn't this just doing
>
> #define inet_sk(ptr) container_of(ptr, struct inet_sock, sk)
>
> which is different from a plain cast in that it actually checks that
> "yes, struct inet_sock has a member called 'sk' that has the right
> type, so now we can convert from that sk to the containing structure".
>
> That's very different from just randomly casting a pointer to another
> pointer, like the current inet_sk() does, and like that disgusting
> promote_to_type() macro does.
>
> We really strive for proper type safety in the kernel. And that very
> much means *not* doing random casts.
>
> At least that "inet_sk(sk)" version using generics didn't take random
> pointer types. But I really don't see why you don't just use
> "container_of()", which is actually type-safe, and would allow "struct
> inet_sock" to contain the "struct sock" somewhere else than in the
> first field.
>
> Hmm? Am I missing something that is happening in linux-next?
Yep. my goal was to have const awareness.
https://patchwork.kernel.org/project/netdevbpf/patch/20230315154245.3405750-2-edumazet@google.com/
(and whole series in
https://patchwork.kernel.org/project/netdevbpf/list/?series=730398&state=*
)
This:
#define inet_sk(ptr) container_of(ptr, struct inet_sock, sk)
does not really cover what I wanted, does it ?
>
> Linus
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-15 23:21 ` Linus Torvalds
2023-03-15 23:37 ` Eric Dumazet
@ 2023-03-15 23:39 ` Linus Torvalds
2023-03-15 23:45 ` Eric Dumazet
1 sibling, 1 reply; 32+ messages in thread
From: Linus Torvalds @ 2023-03-15 23:39 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, David Ahern,
netdev, eric.dumazet, Greg Kroah-Hartman
On Wed, Mar 15, 2023 at 4:21 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> But I really don't see why you don't just use
> "container_of()", which is actually type-safe, and would allow "struct
> inet_sock" to contain the "struct sock" somewhere else than in the
> first field.
I guess that doesn't work, because of how sk_alloc() and friends are set up.
But I do think the networking code should strive to avoid those
"randomly cast one socket type to another".
For example, when you have a TCP socket, instead of casting from
"struct sock *" to "struct tcp_sock *" in tcp_sk(), and getting it all
wrong wrt const, I think you should do the same thing:
#define tcp_sock(ptr) container_of(ptr, struct tcp_sock,
inet_conn.icsk_inet.sk)
to really *show* that you know that "yes, struct tcp_sock contains a
'sk' embedded three structures down!".
And no, I did not actually *test* that at all. But it should get
'const' right (because container_of() gets const right these days),
_and_ it actually verifies that the type you claim has another type
embedded in it actually does so.
Linus
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-15 23:39 ` Linus Torvalds
@ 2023-03-15 23:45 ` Eric Dumazet
2023-03-15 23:54 ` Linus Torvalds
0 siblings, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2023-03-15 23:45 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, David Ahern,
netdev, eric.dumazet, Greg Kroah-Hartman
On Wed, Mar 15, 2023 at 4:40 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Wed, Mar 15, 2023 at 4:21 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > But I really don't see why you don't just use
> > "container_of()", which is actually type-safe, and would allow "struct
> > inet_sock" to contain the "struct sock" somewhere else than in the
> > first field.
>
> I guess that doesn't work, because of how sk_alloc() and friends are set up.
>
> But I do think the networking code should strive to avoid those
> "randomly cast one socket type to another".
Note that my v1 proposed this thing:
#define inet_sk(sk) \
+ _Generic(sk, \
+ const struct sock * : ((const struct inet_sock *)(sk)), \
+ struct sock * : ((struct inet_sock *)(sk)) \
+ )
Jakub feedback was to instead use a common helper, and CCed you on the
discussion.
I do not see yet how to do this 'without cast'
Let me know if you are ok with the v1 approach.
>
> For example, when you have a TCP socket, instead of casting from
> "struct sock *" to "struct tcp_sock *" in tcp_sk(), and getting it all
> wrong wrt const, I think you should do the same thing:
>
> #define tcp_sock(ptr) container_of(ptr, struct tcp_sock,
> inet_conn.icsk_inet.sk)
>
> to really *show* that you know that "yes, struct tcp_sock contains a
> 'sk' embedded three structures down!".
>
> And no, I did not actually *test* that at all. But it should get
> 'const' right (because container_of() gets const right these days),
> _and_ it actually verifies that the type you claim has another type
> embedded in it actually does so.
>
> Linus
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-15 23:37 ` Eric Dumazet
@ 2023-03-15 23:47 ` Linus Torvalds
0 siblings, 0 replies; 32+ messages in thread
From: Linus Torvalds @ 2023-03-15 23:47 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, David Ahern,
netdev, eric.dumazet, Greg Kroah-Hartman
On Wed, Mar 15, 2023 at 4:37 PM Eric Dumazet <edumazet@google.com> wrote:
>
> Yep. my goal was to have const awareness.
>
> This:
>
> #define inet_sk(ptr) container_of(ptr, struct inet_sock, sk)
>
> does not really cover what I wanted, does it ?
Ugh. You should use "container_of_const()" of course.
I *thought* we already fixed plain "container_of()" to do the right
thing,, and got rid of "container_of_const()", but that was obviously
in my dreams.
Linus
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-15 23:45 ` Eric Dumazet
@ 2023-03-15 23:54 ` Linus Torvalds
2023-03-15 23:56 ` Eric Dumazet
0 siblings, 1 reply; 32+ messages in thread
From: Linus Torvalds @ 2023-03-15 23:54 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, David Ahern,
netdev, eric.dumazet, Greg Kroah-Hartman
On Wed, Mar 15, 2023 at 4:46 PM Eric Dumazet <edumazet@google.com> wrote:
>
> Note that my v1 proposed this thing:
>
> #define inet_sk(sk) \
> + _Generic(sk, \
> + const struct sock * : ((const struct inet_sock *)(sk)), \
> + struct sock * : ((struct inet_sock *)(sk)) \
> + )
Right. That was better.
But:
> Jakub feedback was to instead use a common helper, and CCed you on the
> discussion.
> I do not see yet how to do this 'without cast'
That's _exactly_ what container_of_const() does.
I actually would want to make "container_of()" itself do that
correctly, but we clearly have too many broken cases as-is, so
'container_of_const' it is.
Linus
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-15 23:54 ` Linus Torvalds
@ 2023-03-15 23:56 ` Eric Dumazet
2023-03-16 0:02 ` Linus Torvalds
0 siblings, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2023-03-15 23:56 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, David Ahern,
netdev, eric.dumazet, Greg Kroah-Hartman
On Wed, Mar 15, 2023 at 4:54 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Wed, Mar 15, 2023 at 4:46 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > Note that my v1 proposed this thing:
> >
> > #define inet_sk(sk) \
> > + _Generic(sk, \
> > + const struct sock * : ((const struct inet_sock *)(sk)), \
> > + struct sock * : ((struct inet_sock *)(sk)) \
> > + )
>
> Right. That was better.
>
> But:
>
> > Jakub feedback was to instead use a common helper, and CCed you on the
> > discussion.
> > I do not see yet how to do this 'without cast'
>
> That's _exactly_ what container_of_const() does.
>
> I actually would want to make "container_of()" itself do that
> correctly, but we clearly have too many broken cases as-is, so
> 'container_of_const' it is.
container_of_const() does not detect this bug at compile time, does it ?
struct sk_buff *skb = ...;
struct inet_sk *inet = inet_sk(skb);
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-15 23:56 ` Eric Dumazet
@ 2023-03-16 0:02 ` Linus Torvalds
2023-03-16 0:11 ` Eric Dumazet
0 siblings, 1 reply; 32+ messages in thread
From: Linus Torvalds @ 2023-03-16 0:02 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, David Ahern,
netdev, eric.dumazet, Greg Kroah-Hartman
On Wed, Mar 15, 2023 at 4:56 PM Eric Dumazet <edumazet@google.com> wrote:
>
> container_of_const() does not detect this bug at compile time, does it ?
>
> struct sk_buff *skb = ...;
>
> struct inet_sk *inet = inet_sk(skb);
You didn't actually test it, did you?
I get about 40 lines of error messages about how broken that is, starting with
./include/linux/build_bug.h:78:41: error: static assertion failed:
"pointer type mismatch in container_of()"
exactly because yes, 'container_of()' is very pissy indeed about bogus
conversions.
You can only convert a named member into a containing structure, and
'inet_sk' does not contain a member named 'sk' that is of type 'struct
skb_buff'.
Linus
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-16 0:02 ` Linus Torvalds
@ 2023-03-16 0:11 ` Eric Dumazet
2023-03-16 0:23 ` Eric Dumazet
0 siblings, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2023-03-16 0:11 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, David Ahern,
netdev, eric.dumazet, Greg Kroah-Hartman
On Wed, Mar 15, 2023 at 5:02 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Wed, Mar 15, 2023 at 4:56 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > container_of_const() does not detect this bug at compile time, does it ?
> >
> > struct sk_buff *skb = ...;
> >
> > struct inet_sk *inet = inet_sk(skb);
>
> You didn't actually test it, did you?
I thought I did, sorry. I will spend more time on it.
>
> I get about 40 lines of error messages about how broken that is, starting with
>
> ./include/linux/build_bug.h:78:41: error: static assertion failed:
> "pointer type mismatch in container_of()"
>
> exactly because yes, 'container_of()' is very pissy indeed about bogus
> conversions.
>
> You can only convert a named member into a containing structure, and
> 'inet_sk' does not contain a member named 'sk' that is of type 'struct
> skb_buff'.
>
> Linus
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-16 0:11 ` Eric Dumazet
@ 2023-03-16 0:23 ` Eric Dumazet
2023-03-16 0:34 ` Linus Torvalds
0 siblings, 1 reply; 32+ messages in thread
From: Eric Dumazet @ 2023-03-16 0:23 UTC (permalink / raw)
To: Linus Torvalds
Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, David Ahern,
netdev, eric.dumazet, Greg Kroah-Hartman
On Wed, Mar 15, 2023 at 5:11 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Wed, Mar 15, 2023 at 5:02 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > On Wed, Mar 15, 2023 at 4:56 PM Eric Dumazet <edumazet@google.com> wrote:
> > >
> > > container_of_const() does not detect this bug at compile time, does it ?
> > >
> > > struct sk_buff *skb = ...;
> > >
> > > struct inet_sk *inet = inet_sk(skb);
> >
> > You didn't actually test it, did you?
>
> I thought I did, sorry. I will spend more time on it.
Oh I might have been fooled, because of course we can not use sk for
the macro parameter name.
-ENOTENOUGHSLEEP
Basically something like this should work just fine
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index 6eb8235be67f8b8265cd86782aed2b489e8850ee..caa20a9055310f5ef108f9b1bb43214a3d598b9e
100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -305,11 +305,7 @@ static inline struct sock *skb_to_full_sk(const
struct sk_buff *skb)
return sk_to_full_sk(skb->sk);
}
-#define inet_sk(sk) \
- _Generic(sk, \
- const struct sock * : ((const struct inet_sock
*)(sk)), \
- struct sock * : ((struct inet_sock *)(sk)) \
- )
+#define inet_sk(ptr) container_of_const(ptr, struct inet_sock, sk)
static inline void __inet_sk_copy_descendant(struct sock *sk_to,
const struct sock *sk_from,
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk()
2023-03-16 0:23 ` Eric Dumazet
@ 2023-03-16 0:34 ` Linus Torvalds
0 siblings, 0 replies; 32+ messages in thread
From: Linus Torvalds @ 2023-03-16 0:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jakub Kicinski, David S . Miller, Paolo Abeni, David Ahern,
netdev, eric.dumazet, Greg Kroah-Hartman
[-- Attachment #1: Type: text/plain, Size: 1062 bytes --]
On Wed, Mar 15, 2023 at 5:23 PM Eric Dumazet <edumazet@google.com> wrote:
>
> Oh I might have been fooled, because of course we can not use sk for
> the macro parameter name.
Heh. Yeah, that will rename the 'sk' member name too, and cause some
*very* strange errors.
> Basically something like this should work just fine
Yup.
I'm testing it on the current git tree, and I'm finding a number of
random issues, but it looks manageable. Attached is what I ended up
with.
You have presumably already fixed these issues in your tree.
Btw, it's very much things like 'tcp_sk()' too, and doing that with
#define tcp_sk(ptr) container_of_const(ptr, struct tcp_sock,
inet_conn.icsk_inet.sk)
actually shows some fundamental problems. For example, we have
tcp_synq_overflow() that takes a const sk pointer, but then it does
WRITE_ONCE(tcp_sk(sk)->rx_opt.ts_recent_stamp, now);
which is a big no-no and writes to that sock thing.
I didn't even try any of the other 'xyzzy_sk()' conversions.
Linus
[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 4133 bytes --]
include/linux/tcp.h | 5 +----
include/net/inet_sock.h | 5 +----
include/trace/events/sock.h | 4 ++--
include/trace/events/tcp.h | 2 +-
net/ipv4/ip_output.c | 4 ++--
net/mptcp/sockopt.c | 2 +-
security/lsm_audit.c | 6 +++---
7 files changed, 11 insertions(+), 17 deletions(-)
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index 51857117ac09..caa20a905531 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -305,10 +305,7 @@ static inline struct sock *skb_to_full_sk(const struct sk_buff *skb)
return sk_to_full_sk(skb->sk);
}
-static inline struct inet_sock *inet_sk(const struct sock *sk)
-{
- return (struct inet_sock *)sk;
-}
+#define inet_sk(ptr) container_of_const(ptr, struct inet_sock, sk)
static inline void __inet_sk_copy_descendant(struct sock *sk_to,
const struct sock *sk_from,
diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
index 03d19fc562f8..fd206a6ab5b8 100644
--- a/include/trace/events/sock.h
+++ b/include/trace/events/sock.h
@@ -158,7 +158,7 @@ TRACE_EVENT(inet_sock_set_state,
),
TP_fast_assign(
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
struct in6_addr *pin6;
__be32 *p32;
@@ -222,7 +222,7 @@ TRACE_EVENT(inet_sk_error_report,
),
TP_fast_assign(
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
struct in6_addr *pin6;
__be32 *p32;
diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
index 901b440238d5..bf06db8d2046 100644
--- a/include/trace/events/tcp.h
+++ b/include/trace/events/tcp.h
@@ -67,7 +67,7 @@ DECLARE_EVENT_CLASS(tcp_event_sk_skb,
),
TP_fast_assign(
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
__be32 *p32;
__entry->skbaddr = skb;
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 4e4e308c3230..64971054f0c5 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -129,7 +129,7 @@ int ip_local_out(struct net *net, struct sock *sk, struct sk_buff *skb)
}
EXPORT_SYMBOL_GPL(ip_local_out);
-static inline int ip_select_ttl(struct inet_sock *inet, struct dst_entry *dst)
+static inline int ip_select_ttl(const struct inet_sock *inet, struct dst_entry *dst)
{
int ttl = inet->uc_ttl;
@@ -146,7 +146,7 @@ int ip_build_and_send_pkt(struct sk_buff *skb, const struct sock *sk,
__be32 saddr, __be32 daddr, struct ip_options_rcu *opt,
u8 tos)
{
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
struct rtable *rt = skb_rtable(skb);
struct net *net = sock_net(sk);
struct iphdr *iph;
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 8a9656248b0f..5cef4d3d21ac 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -1046,7 +1046,7 @@ static int mptcp_getsockopt_tcpinfo(struct mptcp_sock *msk, char __user *optval,
static void mptcp_get_sub_addrs(const struct sock *sk, struct mptcp_subflow_addrs *a)
{
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
memset(a, 0, sizeof(*a));
diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index a7355b4b9bb8..368e77ca43c4 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -310,14 +310,14 @@ 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;
switch (sk->sk_family) {
case AF_INET: {
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
print_ipv4_addr(ab, inet->inet_rcv_saddr,
inet->inet_sport,
@@ -329,7 +329,7 @@ static void dump_common_audit_data(struct audit_buffer *ab,
}
#if IS_ENABLED(CONFIG_IPV6)
case AF_INET6: {
- struct inet_sock *inet = inet_sk(sk);
+ const struct inet_sock *inet = inet_sk(sk);
print_ipv6_addr(ab, &sk->sk_v6_rcv_saddr,
inet->inet_sport,
^ permalink raw reply related [flat|nested] 32+ messages in thread
end of thread, other threads:[~2023-03-16 0:36 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-15 15:42 [PATCH net-next 0/8] inet: better const qualifier awareness Eric Dumazet
2023-03-15 15:42 ` [PATCH net-next 1/8] inet: preserve const qualifier in inet_sk() Eric Dumazet
2023-03-15 18:52 ` Simon Horman
2023-03-15 21:28 ` Jakub Kicinski
2023-03-15 22:37 ` Eric Dumazet
2023-03-15 23:08 ` Jakub Kicinski
2023-03-15 23:16 ` Eric Dumazet
2023-03-15 23:21 ` Linus Torvalds
2023-03-15 23:37 ` Eric Dumazet
2023-03-15 23:47 ` Linus Torvalds
2023-03-15 23:39 ` Linus Torvalds
2023-03-15 23:45 ` Eric Dumazet
2023-03-15 23:54 ` Linus Torvalds
2023-03-15 23:56 ` Eric Dumazet
2023-03-16 0:02 ` Linus Torvalds
2023-03-16 0:11 ` Eric Dumazet
2023-03-16 0:23 ` Eric Dumazet
2023-03-16 0:34 ` Linus Torvalds
2023-03-15 15:42 ` [PATCH net-next 2/8] ipv4: constify ip_mc_sf_allow() socket argument Eric Dumazet
2023-03-15 18:52 ` Simon Horman
2023-03-15 15:42 ` [PATCH net-next 3/8] udp: constify __udp_is_mcast_sock() " Eric Dumazet
2023-03-15 18:53 ` Simon Horman
2023-03-15 15:42 ` [PATCH net-next 4/8] ipv6: constify inet6_mc_check() Eric Dumazet
2023-03-15 18:53 ` Simon Horman
2023-03-15 15:42 ` [PATCH net-next 5/8] udp6: constify __udp_v6_is_mcast_sock() socket argument Eric Dumazet
2023-03-15 18:53 ` Simon Horman
2023-03-15 15:42 ` [PATCH net-next 6/8] ipv6: raw: constify raw_v6_match() " Eric Dumazet
2023-03-15 18:54 ` Simon Horman
2023-03-15 15:42 ` [PATCH net-next 7/8] ipv4: raw: constify raw_v4_match() " Eric Dumazet
2023-03-15 18:54 ` Simon Horman
2023-03-15 15:42 ` [PATCH net-next 8/8] inet_diag: constify raw_lookup() " Eric Dumazet
2023-03-15 18:54 ` Simon Horman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.