netdev - Re: [Patch net-next] tcp: add a tracepoint for tcp_retransmit

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Tue, 10 Oct 2017 14:37:11 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Yuchung Cheng <ycheng@...gle.com>,
        Neal Cardwell <ncardwell@...gle.com>,
        Martin KaFai Lau <kafai@...com>,
        Brendan Gregg <brendan.d.gregg@...il.com>,
        Hannes Frederic Sowa <hannes@...essinduktion.org>,
        Song Liu <songliubraving@...com>
Subject: Re: [Patch net-next] tcp: add a tracepoint for tcp_retransmit_skb()

On Tue, Oct 10, 2017 at 10:38 AM, Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> I'm happy to see new tracepoints being added to tcp stack, but I'm concerned
> with practical usability of them.
> Like the above tracepoint definition makes it not very useful from bpf point of view,
> since 'sk' pointer is not recored by as part of the tracepoint.
> In bpf/tracing world we prefer tracepoints to have raw pointers recorded
> in TP_STRUCT__entry() and _not_ printed in TP_printk()
> (since pointers are useless for userspace).
> Like trace_kfree_skb() tracepoint records raw 'skb' pointer and we can
> walk whatever sk_buff fields we need inside the program.
> Such approach allows tracepoint to be usable in many more scenarios, since
> bpf program can examine kernel datastructures.


Sure, I am happy to add them for BPF. The current version is merely
for our own use case, other use cases like this are always welcome!


> Over the last few years we've been running tcp statistics framework (similar to web10g)
> using 8 kprobes in tcp stack with bpf programs extracting the data and now we're
> ready to share this experience with the community. Right now we're working on a set
> of tracepoints for tcp stack to make the interface more accurate, faster and more stable.
> We're planning to send an RFC patch with these new tracepoints in the comming weeks.

Great! Looking forward to it!

>
> More concrete, if you can make this trace_tcp_retransmit_skb() to record
> sk, skb pointers and err code at the end of __tcp_retransmit_skb() it will solve
> our need as well.


Note, currently I only call trace_tcp_retransmit_skb() for successful
retransmissions, since you mentioned err code, I guess you want it
for failures too? I am not sure if tracing unsuccessful TCP retransmissions
is meaningful here, I guess it's needed for BPF to track TCP states?

It doesn't harm to add it, at least we can filter out err!=0 since we
only care about successful ones.


>
> So far our list of kprobes is:
> int kprobe__tcp_validate_incoming
> int kprobe__tcp_send_active_reset
> int kprobe__tcp_v4_send_reset
> int kprobe__tcp_v6_send_reset
> int kprobe__tcp_v4_destroy_sock
> int kprobe__tcp_set_state
> int kprobe__tcp_retransmit_skb
> int kprobe__tcp_rtx_synack
>
> with tracepoints we can consolidate two of them into one and drop
> another one for sure. Notice that tcp_retransmit_skb is on our list too
> and currently we're doing extra work inside the program to make it more
> accurate which will be unnecessary if this tracepoint is at the end
> of __tcp_retransmit_skb().

Yeah, with these tracepoints we would be able to trace more TCP
state changes.

Thanks!