linux-kernel - Re: [PATCH] net: tcp_drop adds `reason` parameter for tracing

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, 24 Aug 2021 08:22:11 -0700
From:   Eric Dumazet <edumazet@...gle.com>
To:     Zhongya Yan <yan2228598786@...il.com>
Cc:     Jakub Kicinski <kuba@...nel.org>, netdev <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>,
        David Miller <davem@...emloft.net>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        David Ahern <dsahern@...nel.org>, hengqi.chen@...il.com,
        Yonghong Song <yhs@...com>
Subject: Re: [PATCH] net: tcp_drop adds `reason` parameter for tracing

On Tue, Aug 24, 2021 at 5:52 AM 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>
> ---

That is a good start, but really if people want to use this
tracepoint, they will hit a wall soon.


>         return true;
>
>  discard:

There are many " goto discards;" in this function, so using a common
value ("TCP_VALIDATE_INCOMING" ) is not helpful.


> -       tcp_drop(sk, skb);
> +       tcp_drop(sk, skb, TCP_VALIDATE_INCOMING);
>         return false;
>  }
>
> @@ -5905,7 +5915,7 @@ void tcp_rcv_established(struct sock *sk, struct sk_buff *skb)
>         TCP_INC_STATS(sock_net(sk), TCP_MIB_INERRS);
>
>  discard:
> -       tcp_drop(sk, skb);

Same here.

> +       tcp_drop(sk, skb, TCP_RCV_ESTABLISHED);
>  }
>  EXPORT_SYMBOL(tcp_rcv_established);
>
> @@ -6196,7 +6206,7 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
>                                                   TCP_DELACK_MAX, TCP_RTO_MAX);
>
>  discard:
> -                       tcp_drop(sk, skb);
> +                       tcp_drop(sk, skb, TCP_RCV_SYNSENT_STATE_PROCESS);
>                         return 0;
>                 } else {
>                         tcp_send_ack(sk);
> @@ -6568,7 +6578,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
>
>         if (!queued) {
>  discard:

same here.

> -               tcp_drop(sk, skb);
> +               tcp_drop(sk, skb, TCP_RCV_STATE_PROCESS);
>         }
>         return 0;
>  }
> --
> 2.25.1
>