| 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
| ||
|
Message-ID: <20210824112957.3a780186@oasis.local.home>
Date: Tue, 24 Aug 2021 11:29:57 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Zhongya Yan <yan2228598786@...il.com>
Cc: kuba@...nel.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, edumazet@...gle.com,
mingo@...hat.com, davem@...emloft.net, yoshfuji@...ux-ipv6.org,
dsahern@...nel.org, hengqi.chen@...il.com, yhs@...com
Subject: Re: [PATCH] net: tcp_drop adds `reason` parameter for tracing
On Tue, 24 Aug 2021 05:51:40 -0700
Zhongya Yan <yan2228598786@...il.com> wrote:
> When using `tcp_drop(struct sock *sk, struct sk_buff *skb)` we can
> not tell why we need to delete `skb`. To solve this problem I updated the
> method `tcp_drop(struct sock *sk, struct sk_buff *skb, enum tcp_drop_reason reason)`
> to include the source of the deletion when it is done, so you can
> get an idea of the reason for the deletion based on the source.
>
> The current purpose is mainly derived from the suggestions
> of `Yonghong Song` and `brendangregg`:
>
> https://github.com/iovisor/bcc/issues/3533.
>
> "It is worthwhile to mention the context/why we want to this
> tracepoint with bcc issue https://github.com/iovisor/bcc/issues/3533.
> Mainly two reasons: (1). tcp_drop is a tiny function which
> may easily get inlined, a tracepoint is more stable, and (2).
> tcp_drop does not provide enough information on why it is dropped.
> " by Yonghong Song
>
> Signed-off-by: Zhongya Yan <yan2228598786@...il.com>
> ---
> include/net/tcp.h | 11 ++++++++
> include/trace/events/tcp.h | 56 ++++++++++++++++++++++++++++++++++++++
> net/ipv4/tcp_input.c | 34 +++++++++++++++--------
> 3 files changed, 89 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 784d5c3ef1c5..dd8cd8d6f2f1 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -254,6 +254,17 @@ extern atomic_long_t tcp_memory_allocated;
> extern struct percpu_counter tcp_sockets_allocated;
> extern unsigned long tcp_memory_pressure;
>
> +enum tcp_drop_reason {
> + TCP_OFO_QUEUE = 1,
> + TCP_DATA_QUEUE_OFO = 2,
> + TCP_DATA_QUEUE = 3,
> + TCP_PRUNE_OFO_QUEUE = 4,
> + TCP_VALIDATE_INCOMING = 5,
> + TCP_RCV_ESTABLISHED = 6,
> + TCP_RCV_SYNSENT_STATE_PROCESS = 7,
> + TCP_RCV_STATE_PROCESS = 8
As enums increase by one, you could save yourself the burden of
updating the numbers and just have:
enum tcp_drop_reason {
TCP_OFO_QUEUE = 1,
TCP_DATA_QUEUE_OFO,
TCP_DATA_QUEUE,
TCP_PRUNE_OFO_QUEUE,
TCP_VALIDATE_INCOMING,
TCP_RCV_ESTABLISHED,
TCP_RCV_SYNSENT_STATE_PROCESS,
TCP_RCV_STATE_PROCESS
};
Which would do the same.
> +};
> +
> /* optimized version of sk_under_memory_pressure() for TCP sockets */
> static inline bool tcp_under_memory_pressure(const struct sock *sk)
> {
> diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
> index 521059d8dc0a..a0d3d31eb591 100644
> --- a/include/trace/events/tcp.h
> +++ b/include/trace/events/tcp.h
> @@ -371,6 +371,62 @@ DEFINE_EVENT(tcp_event_skb, tcp_bad_csum,
> TP_ARGS(skb)
> );
>
If you would like to see the english translation of what these
"reasons" are and not have to remember which number is which, you can
do the following:
#define TCP_DROP_REASON \
EM(TCP_OFO_QUEUE, ofo_queue) \
EM(TCP_DATA_QUEUE_OFO, data_queue_ofo) \
EM(TCP_DATA_QUEUE, data_queue) \
EM(TCP_PRUNE_OFO_QUEUE, prune_ofo_queue) \
EM(TCP_VALIDATE_INCOMING, validate_incoming) \
EM(TCP_RCV_ESTABLISHED, rcv_established) \
EM(TCP_RCV_SYNSENT_STATE_PROCESS, rcv_synsent_state_process) \
EMe(TCP_RCV_STATE_PROCESS, rcv_state_proces)
#undef EM
#undef EMe
#define EM(a,b) { a, #b },
#define EMe(a, b) { a, #b }
> +TRACE_EVENT(tcp_drop,
> + TP_PROTO(struct sock *sk, struct sk_buff *skb, enum tcp_drop_reason reason),
> +
> + TP_ARGS(sk, skb, reason),
> +
> + TP_STRUCT__entry(
> + __array(__u8, saddr, sizeof(struct sockaddr_in6))
> + __array(__u8, daddr, sizeof(struct sockaddr_in6))
> + __field(__u16, sport)
> + __field(__u16, dport)
> + __field(__u32, mark)
> + __field(__u16, data_len)
> + __field(__u32, snd_nxt)
> + __field(__u32, snd_una)
> + __field(__u32, snd_cwnd)
> + __field(__u32, ssthresh)
> + __field(__u32, snd_wnd)
> + __field(__u32, srtt)
> + __field(__u32, rcv_wnd)
> + __field(__u64, sock_cookie)
> + __field(__u32, reason)
> + ),
> +
> + TP_fast_assign(
> + const struct tcphdr *th = (const struct tcphdr *)skb->data;
> + const struct inet_sock *inet = inet_sk(sk);
> + const struct tcp_sock *tp = tcp_sk(sk);
> +
> + memset(__entry->saddr, 0, sizeof(struct sockaddr_in6));
> + memset(__entry->daddr, 0, sizeof(struct sockaddr_in6));
> +
> + TP_STORE_ADDR_PORTS(__entry, inet, sk);
> +
> + __entry->sport = ntohs(inet->inet_sport);
> + __entry->dport = ntohs(inet->inet_dport);
> + __entry->mark = skb->mark;
> +
> + __entry->data_len = skb->len - __tcp_hdrlen(th);
> + __entry->snd_nxt = tp->snd_nxt;
> + __entry->snd_una = tp->snd_una;
> + __entry->snd_cwnd = tp->snd_cwnd;
> + __entry->snd_wnd = tp->snd_wnd;
> + __entry->rcv_wnd = tp->rcv_wnd;
> + __entry->ssthresh = tcp_current_ssthresh(sk);
> + __entry->srtt = tp->srtt_us >> 3;
> + __entry->sock_cookie = sock_gen_cookie(sk);
> + __entry->reason = reason;
> + ),
> +
> + TP_printk("src=%pISpc dest=%pISpc mark=%#x data_len=%d snd_nxt=%#x snd_una=%#x snd_cwnd=%u ssthresh=%u snd_wnd=%u srtt=%u rcv_wnd=%u sock_cookie=%llx reason=%d",
Then above you can have: "reason=%s"
> + __entry->saddr, __entry->daddr, __entry->mark,
> + __entry->data_len, __entry->snd_nxt, __entry->snd_una,
> + __entry->snd_cwnd, __entry->ssthresh, __entry->snd_wnd,
> + __entry->srtt, __entry->rcv_wnd, __entry->sock_cookie, __entry->reason)
And here:
__print_symbolic(__entry->reason, TCP_DROP_REASON)
-- Steve
> +);
> +
> #endif /* _TRACE_TCP_H */
>
> /* This part must be outside protection */
>
Powered by blists - more mailing lists