This tracepoint can be used to trace synack retransmits. It maintains
pointer to struct request_sock.
We cannot simply reuse trace_tcp_retransmit_skb() here, because the
sk here is the LISTEN socket. The IP addresses and ports should be
extracted from struct request_sock.
Signed-off-by: Song Liu <redacted>
---
include/trace/events/tcp.h | 56 ++++++++++++++++++++++++++++++++++++++++++++++
net/ipv4/tcp_output.c | 1 +
2 files changed, 57 insertions(+)
On Oct 24, 2017, at 4:57 PM, Song Liu [off-list ref] wrote:
This tracepoint can be used to trace synack retransmits. It maintains
pointer to struct request_sock.
We cannot simply reuse trace_tcp_retransmit_skb() here, because the
sk here is the LISTEN socket. The IP addresses and ports should be
extracted from struct request_sock.
Signed-off-by: Song Liu <redacted>
---
include/trace/events/tcp.h | 56 ++++++++++++++++++++++++++++++++++++++++++++++
net/ipv4/tcp_output.c | 1 +
2 files changed, 57 insertions(+)
In this case, we are putting CONFIG_IPV6 in TRACE_EVENT macro, which generates
warnings like:
./include/trace/events/tcp.h:274:1: error: directive in argument list
./include/trace/events/tcp.h:281:1: error: directive in argument list
Seems these warning cannot be easily avoided. This is also the same pattern we
have been using in include/trace/events/tcp.h.
Any suggestions on how shall we proceed from here?
Thanks,
Song
From: Cong Wang <hidden> Date: 2017-10-27 02:01:47
On Thu, Oct 26, 2017 at 4:50 PM, Song Liu [off-list ref] wrote:
In this case, we are putting CONFIG_IPV6 in TRACE_EVENT macro, which generates
warnings like:
./include/trace/events/tcp.h:274:1: error: directive in argument list
./include/trace/events/tcp.h:281:1: error: directive in argument list
Seems these warning cannot be easily avoided. This is also the same pattern we
have been using in include/trace/events/tcp.h.
Hmm, we use the same so why it only complains about this one?
Any suggestions on how shall we proceed from here?
I think this warning is harmless, so perhaps not worthy time to
shut it up, unless sparse provides a simple way to do so.
On Oct 26, 2017, at 7:01 PM, Cong Wang [off-list ref] wrote:
On Thu, Oct 26, 2017 at 4:50 PM, Song Liu [off-list ref] wrote:
quoted
In this case, we are putting CONFIG_IPV6 in TRACE_EVENT macro, which generates
warnings like:
./include/trace/events/tcp.h:274:1: error: directive in argument list
./include/trace/events/tcp.h:281:1: error: directive in argument list
Seems these warning cannot be easily avoided. This is also the same pattern we
have been using in include/trace/events/tcp.h.
Hmm, we use the same so why it only complains about this one?\
sparse reports same warning for all the lines in tcp.h. Don't know why
kbuild test bot only complains about this patch.
quoted
Any suggestions on how shall we proceed from here?
I think this warning is harmless, so perhaps not worthy time to
shut it up, unless sparse provides a simple way to do so.
On Oct 26, 2017, at 7:01 PM, Cong Wang [off-list ref] wrote:
On Thu, Oct 26, 2017 at 4:50 PM, Song Liu [off-list ref] wrote:
quoted
In this case, we are putting CONFIG_IPV6 in TRACE_EVENT macro, which generates
warnings like:
./include/trace/events/tcp.h:274:1: error: directive in argument list
./include/trace/events/tcp.h:281:1: error: directive in argument list
Seems these warning cannot be easily avoided. This is also the same pattern we
have been using in include/trace/events/tcp.h.
Hmm, we use the same so why it only complains about this one?\
sparse reports same warning for all the lines in tcp.h. Don't know why
kbuild test bot only complains about this patch.
it's safe to ignore this sparse error.
The #ifdef inside TP_fast_assign() in net.h have been there for years
without issues.
$ make C=2 net/core/dev.o
../include/trace/events/net.h:170:1: error: directive in argument list
../include/trace/events/net.h:172:1: error: directive in argument list
../include/trace/events/net.h:174:1: error: directive in argument list
From: Alban Crequy <hidden> Date: 2017-10-27 20:38:32
Hi,
On 25 October 2017 at 01:57, Song Liu [off-list ref] wrote:
quoted hunk
This tracepoint can be used to trace synack retransmits. It maintains
pointer to struct request_sock.
We cannot simply reuse trace_tcp_retransmit_skb() here, because the
sk here is the LISTEN socket. The IP addresses and ports should be
extracted from struct request_sock.
Signed-off-by: Song Liu <redacted>
---
include/trace/events/tcp.h | 56 ++++++++++++++++++++++++++++++++++++++++++++++
net/ipv4/tcp_output.c | 1 +
2 files changed, 57 insertions(+)
Would it make sense to add the inode of the network namespace that
owns the socket? (along with the major/minor of the nsfs)
If the kernel later gains tracepoints for TCP connect, accept, close
including the netns ino, then I might be able to replace some
ebpf-kprobes code by ebpf-tracepoints code :)
Hi,
On 25 October 2017 at 01:57, Song Liu [off-list ref] wrote:
quoted
This tracepoint can be used to trace synack retransmits. It maintains
pointer to struct request_sock.
We cannot simply reuse trace_tcp_retransmit_skb() here, because the
sk here is the LISTEN socket. The IP addresses and ports should be
extracted from struct request_sock.
Signed-off-by: Song Liu <redacted>
---
include/trace/events/tcp.h | 56 ++++++++++++++++++++++++++++++++++++++++++++++
net/ipv4/tcp_output.c | 1 +
2 files changed, 57 insertions(+)
Would it make sense to add the inode of the network namespace that
owns the socket? (along with the major/minor of the nsfs)
We cannot do this.
netns ino is not unique identifier of netns.
we can do such hack only inside programs by
walking skb->dev->nd_net->net with bpf_probe_read() and realizing
that this is unstable interface and not technically correct.
If the kernel later gains tracepoints for TCP connect, accept, close
including the netns ino, then I might be able to replace some
ebpf-kprobes code by ebpf-tracepoints code :)
What is the use case for tracepoints in connect/accept/close ?
Just because some _useful_ bcc script is using kprobe in particular
kernel function it doesn't mean yet that we need a tracepoint in there.
imo the general rule for tracepoints is to only add them when it's 100%
certain that this is the right place for it and kprobe approach
is not enough or not possible.
In the case of recent addition of tcp tracepoints the main thing
they achieve (vs our old kprobe approach) is that they are accurate.
In this particular patch the kprobe on tcp_rtx_synack() is not
the same as trace_tcp_retransmit_synack(), since it incorrectly
counts failed send_synack(). It's solvable via kretprobe
on tcp_rtx_synack() and checking %rax inside the bpf program,
but kretprobes add runtime overhead and much slower than tracepoints.