| Message ID | 152112003454.30586.15301041903593569660.stgit@localhost.localdomain |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next,1/5] net: Revert "ipv4: get rid of ip_ra_lock" | expand |
Hi Kirill,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on v4.16-rc4]
[also build test WARNING on next-20180316]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Kirill-Tkhai/Rework-ip_ra_chain-protection/20180317-032841
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> net/ipv4/ip_input.c:162:19: sparse: incompatible types in comparison expression (different address spaces)
--
>> net/ipv4/ip_sockglue.c:347:18: sparse: incorrect type in assignment (different address spaces) @@ expected struct ip_ra_chain [noderef] <asn:4>**rap @@ got n [noderef] <asn:4>**rap @@
net/ipv4/ip_sockglue.c:347:18: expected struct ip_ra_chain [noderef] <asn:4>**rap
net/ipv4/ip_sockglue.c:347:18: got struct ip_ra_chain **<noident>
vim +162 net/ipv4/ip_input.c
150
151 /*
152 * Process Router Attention IP option (RFC 2113)
153 */
154 bool ip_call_ra_chain(struct sk_buff *skb)
155 {
156 struct ip_ra_chain *ra;
157 u8 protocol = ip_hdr(skb)->protocol;
158 struct sock *last = NULL;
159 struct net_device *dev = skb->dev;
160 struct net *net = dev_net(dev);
161
> 162 for (ra = rcu_dereference(net->ipv4.ra_chain); ra; ra = rcu_dereference(ra->next)) {
163 struct sock *sk = ra->sk;
164
165 /* If socket is bound to an interface, only report
166 * the packet if it came from that interface.
167 */
168 if (sk && inet_sk(sk)->inet_num == protocol &&
169 (!sk->sk_bound_dev_if ||
170 sk->sk_bound_dev_if == dev->ifindex)) {
171 if (ip_is_fragment(ip_hdr(skb))) {
172 if (ip_defrag(net, skb, IP_DEFRAG_CALL_RA_CHAIN))
173 return true;
174 }
175 if (last) {
176 struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC);
177 if (skb2)
178 raw_rcv(last, skb2);
179 }
180 last = sk;
181 }
182 }
183
184 if (last) {
185 raw_rcv(last, skb);
186 return true;
187 }
188 return false;
189 }
190
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi, On 17.03.2018 02:22, kbuild test robot wrote: > Hi Kirill, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on v4.16-rc4] > [also build test WARNING on next-20180316] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] > > url: https://github.com/0day-ci/linux/commits/Kirill-Tkhai/Rework-ip_ra_chain-protection/20180317-032841 > reproduce: > # apt-get install sparse > make ARCH=x86_64 allmodconfig > make C=1 CF=-D__CHECK_ENDIAN__ > > > sparse warnings: (new ones prefixed by >>) > >>> net/ipv4/ip_input.c:162:19: sparse: incompatible types in comparison expression (different address spaces) thanks for reporting this! Forgot to add __rcu modifier to member type. I'll send v2 version. Kirill > -- >>> net/ipv4/ip_sockglue.c:347:18: sparse: incorrect type in assignment (different address spaces) @@ expected struct ip_ra_chain [noderef] <asn:4>**rap @@ got n [noderef] <asn:4>**rap @@ > net/ipv4/ip_sockglue.c:347:18: expected struct ip_ra_chain [noderef] <asn:4>**rap > net/ipv4/ip_sockglue.c:347:18: got struct ip_ra_chain **<noident> > > vim +162 net/ipv4/ip_input.c > > 150 > 151 /* > 152 * Process Router Attention IP option (RFC 2113) > 153 */ > 154 bool ip_call_ra_chain(struct sk_buff *skb) > 155 { > 156 struct ip_ra_chain *ra; > 157 u8 protocol = ip_hdr(skb)->protocol; > 158 struct sock *last = NULL; > 159 struct net_device *dev = skb->dev; > 160 struct net *net = dev_net(dev); > 161 > > 162 for (ra = rcu_dereference(net->ipv4.ra_chain); ra; ra = rcu_dereference(ra->next)) { > 163 struct sock *sk = ra->sk; > 164 > 165 /* If socket is bound to an interface, only report > 166 * the packet if it came from that interface. > 167 */ > 168 if (sk && inet_sk(sk)->inet_num == protocol && > 169 (!sk->sk_bound_dev_if || > 170 sk->sk_bound_dev_if == dev->ifindex)) { > 171 if (ip_is_fragment(ip_hdr(skb))) { > 172 if (ip_defrag(net, skb, IP_DEFRAG_CALL_RA_CHAIN)) > 173 return true; > 174 } > 175 if (last) { > 176 struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC); > 177 if (skb2) > 178 raw_rcv(last, skb2); > 179 } > 180 last = sk; > 181 } > 182 } > 183 > 184 if (last) { > 185 raw_rcv(last, skb); > 186 return true; > 187 } > 188 return false; > 189 } > 190 > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation >
diff --git a/include/net/ip.h b/include/net/ip.h index fe63ba95d12b..d53b5a9eae34 100644 --- a/include/net/ip.h +++ b/include/net/ip.h @@ -91,6 +91,17 @@ static inline int inet_sdif(struct sk_buff *skb) return 0; } +/* Special input handler for packets caught by router alert option. + They are selected only by protocol field, and then processed likely + local ones; but only if someone wants them! Otherwise, router + not running rsvpd will kill RSVP. + + It is user level problem, what it will make with them. + I have no idea, how it will masquearde or NAT them (it is joke, joke :-)), + but receiver should be enough clever f.e. to forward mtrace requests, + sent to multicast group to reach destination designated router. + */ + struct ip_ra_chain { struct ip_ra_chain __rcu *next; struct sock *sk; @@ -101,8 +112,6 @@ struct ip_ra_chain { struct rcu_head rcu; }; -extern struct ip_ra_chain __rcu *ip_ra_chain; - /* IP flags. */ #define IP_CE 0x8000 /* Flag: "Congestion" */ #define IP_DF 0x4000 /* Flag: "Don't Fragment" */ diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h index 3a970e429ab6..23c208fcf1a0 100644 --- a/include/net/netns/ipv4.h +++ b/include/net/netns/ipv4.h @@ -49,6 +49,7 @@ struct netns_ipv4 { #endif struct ipv4_devconf *devconf_all; struct ipv4_devconf *devconf_dflt; + struct ip_ra_chain *ra_chain; #ifdef CONFIG_IP_MULTIPLE_TABLES struct fib_rules_ops *rules_ops; bool fib_has_custom_rules; diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c index 57fc13c6ab2b..7582713dd18f 100644 --- a/net/ipv4/ip_input.c +++ b/net/ipv4/ip_input.c @@ -159,7 +159,7 @@ bool ip_call_ra_chain(struct sk_buff *skb) struct net_device *dev = skb->dev; struct net *net = dev_net(dev); - for (ra = rcu_dereference(ip_ra_chain); ra; ra = rcu_dereference(ra->next)) { + for (ra = rcu_dereference(net->ipv4.ra_chain); ra; ra = rcu_dereference(ra->next)) { struct sock *sk = ra->sk; /* If socket is bound to an interface, only report @@ -167,8 +167,7 @@ bool ip_call_ra_chain(struct sk_buff *skb) */ if (sk && inet_sk(sk)->inet_num == protocol && (!sk->sk_bound_dev_if || - sk->sk_bound_dev_if == dev->ifindex) && - net_eq(sock_net(sk), net)) { + sk->sk_bound_dev_if == dev->ifindex)) { if (ip_is_fragment(ip_hdr(skb))) { if (ip_defrag(net, skb, IP_DEFRAG_CALL_RA_CHAIN)) return true; diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c index bf5f44b27b7e..f36d35fe924b 100644 --- a/net/ipv4/ip_sockglue.c +++ b/net/ipv4/ip_sockglue.c @@ -322,18 +322,6 @@ int ip_cmsg_send(struct sock *sk, struct msghdr *msg, struct ipcm_cookie *ipc, return 0; } - -/* Special input handler for packets caught by router alert option. - They are selected only by protocol field, and then processed likely - local ones; but only if someone wants them! Otherwise, router - not running rsvpd will kill RSVP. - - It is user level problem, what it will make with them. - I have no idea, how it will masquearde or NAT them (it is joke, joke :-)), - but receiver should be enough clever f.e. to forward mtrace requests, - sent to multicast group to reach destination designated router. - */ -struct ip_ra_chain __rcu *ip_ra_chain; static DEFINE_SPINLOCK(ip_ra_lock); @@ -350,6 +338,7 @@ int ip_ra_control(struct sock *sk, unsigned char on, { struct ip_ra_chain *ra, *new_ra; struct ip_ra_chain __rcu **rap; + struct net *net = sock_net(sk); if (sk->sk_type != SOCK_RAW || inet_sk(sk)->inet_num == IPPROTO_RAW) return -EINVAL; @@ -357,7 +346,7 @@ int ip_ra_control(struct sock *sk, unsigned char on, new_ra = on ? kmalloc(sizeof(*new_ra), GFP_KERNEL) : NULL; spin_lock_bh(&ip_ra_lock); - for (rap = &ip_ra_chain; + for (rap = &net->ipv4.ra_chain; (ra = rcu_dereference_protected(*rap, lockdep_is_held(&ip_ra_lock))) != NULL; rap = &ra->next) {
This is optimization, which makes ip_call_ra_chain() iterate less sockets to find the sockets it's looking for. Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com> --- include/net/ip.h | 13 +++++++++++-- include/net/netns/ipv4.h | 1 + net/ipv4/ip_input.c | 5 ++--- net/ipv4/ip_sockglue.c | 15 ++------------- 4 files changed, 16 insertions(+), 18 deletions(-)