Thread (2 messages) flat view 2 messages, 2 authors, 2021-09-02

Re: [PATCH] net: tcp_drop adds `reason` and SNMP parameters for tracing v3

From: Eric Dumazet <edumazet@google.com>
Date: 2021-09-02 15:45:17
Also in: lkml

On Thu, Sep 2, 2021 at 2:34 AM Zhongya Yan [off-list ref] wrote:
quoted hunk ↗ jump to hunk
I used the suggestion from `Brendan Gregg`. In addition to the
`reason` parameter there is also the `field` parameter pointing
to `SNMP` to distinguish the `tcp_drop` cause. I know what I
submitted is not accurate, so I am submitting the current
patch to get comments and criticism from everyone so that I
can submit better code and solutions.And of course to make me
more familiar and understand the `linux` kernel network code.
Thank you everyone!

Signed-off-by: Zhongya Yan <redacted>
---
 include/trace/events/tcp.h |  39 +++---------
 net/ipv4/tcp_input.c       | 126 ++++++++++++++-----------------------
 2 files changed, 57 insertions(+), 108 deletions(-)
diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
index 699539702ea9..80892660458e 100644
--- a/include/trace/events/tcp.h
+++ b/include/trace/events/tcp.h
@@ -371,28 +371,10 @@ DEFINE_EVENT(tcp_event_skb, tcp_bad_csum,
        TP_ARGS(skb)
 );

-// from author @{Steven Rostedt}
-#define TCP_DROP_REASON                                                        \
-       REASON_STRING(TCP_OFO_QUEUE,            ofo_queue)                      \
-       REASON_STRING(TCP_DATA_QUEUE_OFO,               data_queue_ofo)                 \
-       REASON_STRING(TCP_DATA_QUEUE,           data_queue)                     \
-       REASON_STRING(TCP_PRUNE_OFO_QUEUE,              prune_ofo_queue)                \
-       REASON_STRING(TCP_VALIDATE_INCOMING,    validate_incoming)              \
-       REASON_STRING(TCP_RCV_ESTABLISHED,              rcv_established)                \
-       REASON_STRING(TCP_RCV_SYNSENT_STATE_PROCESS, rcv_synsent_state_process) \
-       REASON_STRINGe(TCP_RCV_STATE_PROCESS,   rcv_state_proces)
??? On which tree / branch this patch is based on ?
quoted hunk ↗ jump to hunk
-
-#undef REASON_STRING
-#undef REASON_STRINGe
-
-#define REASON_STRING(code, name) {code , #name},
-#define REASON_STRINGe(code, name) {code, #name}
-
-
 TRACE_EVENT(tcp_drop,
-               TP_PROTO(struct sock *sk, struct sk_buff *skb, __u32 reason),
+               TP_PROTO(struct sock *sk, struct sk_buff *skb, int field, const char *reason),

-               TP_ARGS(sk, skb, reason),
+               TP_ARGS(sk, skb, field, reason),

                TP_STRUCT__entry(
                        __array(__u8, saddr, sizeof(struct sockaddr_in6))
@@ -409,9 +391,8 @@ TRACE_EVENT(tcp_drop,
                        __field(__u32, srtt)
                        __field(__u32, rcv_wnd)
                        __field(__u64, sock_cookie)
-                       __field(__u32, reason)
-                       __field(__u32, reason_code)
-                       __field(__u32, reason_line)
+                       __field(int, field)
+                       __string(reason, reason)
                        ),

                TP_fast_assign(
@@ -437,21 +418,19 @@ TRACE_EVENT(tcp_drop,
                                __entry->ssthresh = tcp_current_ssthresh(sk);
                                __entry->srtt = tp->srtt_us >> 3;
                                __entry->sock_cookie = sock_gen_cookie(sk);
-                               __entry->reason = reason;
-                               __entry->reason_code = TCP_DROP_CODE(reason);
-                               __entry->reason_line = TCP_DROP_LINE(reason);
+                               __entry->field = field;
+
+                               __assign_str(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 reason_type=%s reason_line=%d",
+                               sock_cookie=%llx field=%d 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,
-                               __print_symbolic(__entry->reason_code, TCP_DROP_REASON),
-                               __entry->reason_line)
+                               field, __get_str(reason))
 );

 #endif /* _TRACE_TCP_H */
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index b2bc49f1f0de..bd33fd466e1e 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -100,7 +100,6 @@ int sysctl_tcp_max_orphans __read_mostly = NR_FILE;
 #define FLAG_UPDATE_TS_RECENT  0x4000 /* tcp_replace_ts_recent() */
 #define FLAG_NO_CHALLENGE_ACK  0x8000 /* do not call tcp_send_challenge_ack()  */
 #define FLAG_ACK_MAYBE_DELAYED 0x10000 /* Likely a delayed ACK */
-#define FLAG_DSACK_TLP         0x20000 /* DSACK for tail loss probe */

 #define FLAG_ACKED             (FLAG_DATA_ACKED|FLAG_SYN_ACKED)
 #define FLAG_NOT_DUP           (FLAG_DATA|FLAG_WIN_UPDATE|FLAG_ACKED)
@@ -455,12 +454,11 @@ static void tcp_sndbuf_expand(struct sock *sk)
  */

 /* Slow part of check#2. */
-static int __tcp_grow_window(const struct sock *sk, const struct sk_buff *skb,
-                            unsigned int skbtruesize)
+static int __tcp_grow_window(const struct sock *sk, const struct sk_buff *skb)
???
 {
        struct tcp_sock *tp = tcp_sk(sk);
        /* Optimize this! */
-       int truesize = tcp_win_from_space(sk, skbtruesize) >> 1;
+       int truesize = tcp_win_from_space(sk, skb->truesize) >> 1;
???
quoted hunk ↗ jump to hunk
        int window = tcp_win_from_space(sk, sock_net(sk)->ipv4.sysctl_tcp_rmem[2]) >> 1;

        while (tp->rcv_ssthresh <= window) {
@@ -473,27 +471,7 @@ static int __tcp_grow_window(const struct sock *sk, const struct sk_buff *skb,
        return 0;
 }

-/* Even if skb appears to have a bad len/truesize ratio, TCP coalescing
- * can play nice with us, as sk_buff and skb->head might be either
- * freed or shared with up to MAX_SKB_FRAGS segments.
- * Only give a boost to drivers using page frag(s) to hold the frame(s),
- * and if no payload was pulled in skb->head before reaching us.
- */
-static u32 truesize_adjust(bool adjust, const struct sk_buff *skb)
-{
-       u32 truesize = skb->truesize;
-
-       if (adjust && !skb_headlen(skb)) {
-               truesize -= SKB_TRUESIZE(skb_end_offset(skb));
-               /* paranoid check, some drivers might be buggy */
-               if (unlikely((int)truesize < (int)skb->len))
-                       truesize = skb->truesize;
-       }
-       return truesize;
-}
It seems clear you are doing wrong things.

Have you silently reverted a prior patch ?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help