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

[Eric Dumazet]

On Tue, Aug 24, 2021 at 5:52 AM Zhongya Yan <yan2228598786@xxxxxxxxx> 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@xxxxxxxxx>
> ---

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
>