netdev - Re: [PATCH net-next v2 0/9] net: add skb drop reasons to TCP packet receive

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ac145b7b-8c78-88dd-ded5-c780bf7e94e1@kernel.org>
Date:   Fri, 18 Feb 2022 14:23:39 -0700
From:   David Ahern <dsahern@...nel.org>
To:     menglong8.dong@...il.com, kuba@...nel.org
Cc:     edumazet@...gle.com, davem@...emloft.net, rostedt@...dmis.org,
        mingo@...hat.com, yoshfuji@...ux-ipv6.org, ast@...nel.org,
        daniel@...earbox.net, hawk@...nel.org, john.fastabend@...il.com,
        imagedong@...cent.com, talalahmad@...gle.com,
        keescook@...omium.org, ilias.apalodimas@...aro.org, alobakin@...me,
        memxor@...il.com, atenart@...nel.org, bigeasy@...utronix.de,
        pabeni@...hat.com, linyunsheng@...wei.com, arnd@...db.de,
        yajun.deng@...ux.dev, roopa@...dia.com, willemb@...gle.com,
        vvs@...tuozzo.com, cong.wang@...edance.com,
        luiz.von.dentz@...el.com, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org, bpf@...r.kernel.org, flyingpeng@...cent.com
Subject: Re: [PATCH net-next v2 0/9] net: add skb drop reasons to TCP packet
 receive

On 2/18/22 1:31 AM, menglong8.dong@...il.com wrote:
> From: Menglong Dong <imagedong@...cent.com>
> 
> In the commit c504e5c2f964 ("net: skb: introduce kfree_skb_reason()"),
> we added the support of reporting the reasons of skb drops to kfree_skb
> tracepoint. And in this series patches, reasons for skb drops are added
> to TCP layer (both TCPv4 and TCPv6 are considered).
> Following functions are processed:
> 
> tcp_v4_rcv()
> tcp_v6_rcv()
> tcp_v4_inbound_md5_hash()
> tcp_v6_inbound_md5_hash()
> tcp_add_backlog()
> tcp_v4_do_rcv()
> tcp_v6_do_rcv()
> tcp_rcv_established()
> tcp_data_queue()
> tcp_data_queue_ofo()
> 
> The functions we handled are mostly for packet ingress, as skb drops
> hardly happens in the egress path of TCP layer. However, it's a little
> complex for TCP state processing, as I find that it's hard to report skb
> drop reasons to where it is freed. For example, when skb is dropped in
> tcp_rcv_state_process(), the reason can be caused by the call of
> tcp_v4_conn_request(), and it's hard to return a drop reason from
> tcp_v4_conn_request(). So such cases are skipped  for this moment.
> 
> Following new drop reasons are introduced (what they mean can be see
> in the document for them):
> 
> /* SKB_DROP_REASON_TCP_MD5* corresponding to LINUX_MIB_TCPMD5* */
> SKB_DROP_REASON_TCP_MD5NOTFOUND
> SKB_DROP_REASON_TCP_MD5UNEXPECTED
> SKB_DROP_REASON_TCP_MD5FAILURE
> SKB_DROP_REASON_SOCKET_BACKLOG
> SKB_DROP_REASON_TCP_FLAGS
> SKB_DROP_REASON_TCP_ZEROWINDOW
> SKB_DROP_REASON_TCP_OLD_DATA
> SKB_DROP_REASON_TCP_OVERWINDOW
> /* corresponding to LINUX_MIB_TCPOFOMERGE */
> SKB_DROP_REASON_TCP_OFOMERGE
> 
> Here is a example to get TCP packet drop reasons from ftrace:
> 
> $ echo 1 > /sys/kernel/debug/tracing/events/skb/kfree_skb/enable
> $ cat /sys/kernel/debug/tracing/trace
> $ <idle>-0       [036] ..s1.   647.428165: kfree_skb: skbaddr=000000004d037db6 protocol=2048 location=0000000074cd1243 reason: NO_SOCKET
> $ <idle>-0       [020] ..s2.   639.676674: kfree_skb: skbaddr=00000000bcbfa42d protocol=2048 location=00000000bfe89d35 reason: PROTO_MEM
> 
> From the reason 'PROTO_MEM' we can know that the skb is dropped because
> the memory configured in net.ipv4.tcp_mem is up to the limition.
> 
> Changes since v1:
> - enrich the document for this series patches in the cover letter,
>   as Eric suggested
> - fix compile warning report by Jakub in the 6th patch
> - let NO_SOCKET trump the XFRM failure in the 2th and 3th patches
> 
> Menglong Dong (9):
>   net: tcp: introduce tcp_drop_reason()
>   net: tcp: add skb drop reasons to tcp_v4_rcv()
>   net: tcp: use kfree_skb_reason() for tcp_v6_rcv()
>   net: tcp: add skb drop reasons to tcp_v{4,6}_inbound_md5_hash()
>   net: tcp: add skb drop reasons to tcp_add_backlog()
>   net: tcp: use kfree_skb_reason() for tcp_v{4,6}_do_rcv()
>   net: tcp: use tcp_drop_reason() for tcp_rcv_established()
>   net: tcp: use tcp_drop_reason() for tcp_data_queue()
>   net: tcp: use tcp_drop_reason() for tcp_data_queue_ofo()
> 
>  include/linux/skbuff.h     | 34 ++++++++++++++++++++++++++++++
>  include/net/tcp.h          |  3 ++-
>  include/trace/events/skb.h | 10 +++++++++
>  net/ipv4/tcp_input.c       | 42 +++++++++++++++++++++++++++++---------
>  net/ipv4/tcp_ipv4.c        | 32 +++++++++++++++++++++--------
>  net/ipv6/tcp_ipv6.c        | 39 +++++++++++++++++++++++++++--------
>  6 files changed, 132 insertions(+), 28 deletions(-)
> 

LGTM. for the set:

Reviewed-by: David Ahern <dsahern@...nel.org>