lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 16 Mar 2022 20:18:53 -0700 From: Jakub Kicinski <kuba@...nel.org> To: menglong8.dong@...il.com, dsahern@...nel.org Cc: pabeni@...hat.com, rostedt@...dmis.org, mingo@...hat.com, xeb@...l.ru, davem@...emloft.net, yoshfuji@...ux-ipv6.org, imagedong@...cent.com, edumazet@...gle.com, kafai@...com, talalahmad@...gle.com, keescook@...omium.org, alobakin@...me, flyingpeng@...cent.com, mengensun@...cent.com, dongli.zhang@...cle.com, linux-kernel@...r.kernel.org, netdev@...r.kernel.org, benbjiang@...cent.com Subject: Re: [PATCH net-next v3 3/3] net: icmp: add reasons of the skb drops to icmp protocol On Wed, 16 Mar 2022 14:31:48 +0800 menglong8.dong@...il.com wrote: > From: Menglong Dong <imagedong@...cent.com> > > Replace kfree_skb() used in icmp_rcv() and icmpv6_rcv() with > kfree_skb_reason(). > > In order to get the reasons of the skb drops after icmp message handle, > we change the return type of 'handler()' in 'struct icmp_control' from > 'bool' to 'enum skb_drop_reason'. This may change its original > intention, as 'false' means failure, but 'SKB_NOT_DROPPED_YET' means > success now. Therefore, all 'handler' and the call of them need to be > handled. Following 'handler' functions are involved: > > icmp_unreach() > icmp_redirect() > icmp_echo() > icmp_timestamp() > icmp_discard() > > And following new drop reasons are added: > > SKB_DROP_REASON_ICMP_CSUM > SKB_DROP_REASON_ICMP_TYPE > SKB_DROP_REASON_ICMP_BROADCAST > > Reviewed-by: Hao Peng <flyingpeng@...cent.com> > Reviewed-by: Jiang Biao <benbjiang@...cent.com> > Signed-off-by: Menglong Dong <imagedong@...cent.com> I guess this set raises the follow up question to Dave if adding drop reasons to places with MIB exception stats means improving the granularity or one MIB stat == one reason? > -bool ping_rcv(struct sk_buff *skb) > +enum skb_drop_reason ping_rcv(struct sk_buff *skb) > { > + enum skb_drop_reason reason = SKB_DROP_REASON_NO_SOCKET; > struct sock *sk; > struct net *net = dev_net(skb->dev); > struct icmphdr *icmph = icmp_hdr(skb); > - bool rc = false; > > /* We assume the packet has already been checked by icmp_rcv */ > > @@ -980,15 +980,17 @@ bool ping_rcv(struct sk_buff *skb) > struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC); > > pr_debug("rcv on socket %p\n", sk); > - if (skb2 && !ping_queue_rcv_skb(sk, skb2)) > - rc = true; > + if (skb2) > + reason = __ping_queue_rcv_skb(sk, skb2); > + else > + reason = SKB_DROP_REASON_NOMEM; > sock_put(sk); > } > > - if (!rc) > + if (reason) > pr_debug("no socket, dropping\n"); This is going to be printed on memory allocation failures now as well.
Powered by blists - more mailing lists