Re: [RFC PATCH net-next] net: use a dedicated tracepoint for kfree_skb_list()

[Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>]

On Fri, Oct 23, 2020 at 10:52:14PM +0200, Davide Caratti wrote:
> kfree_skb_list() calls kfree_skb(), thus triggering as many dropwatch
> events as the number of skbs in the list. This can disturb the analysis
> of packet drops, e.g. with fragmented echo requests generated by ICMP
> sockets, or with regular SCTP packets: when consume_skb() frees them,
> the kernel's drop monitor may wrongly account for several packet drops:
> 
>  consume skb()
>    skb_release_data()
>      kfree_skb_list()
>        kfree_skb() <-- false dropwatch event

Seems the problem lies with skb_release_data() calling
kfree_skb_list() while it should have been a, say, consume_skb_list(),
and not generate further kfree_skb calls.

Maybe a bool parameter on skb_release_data to signal that it should
call consume_skb_list (which doesn't exist) instead?

> 
> don't call kfree_skb() when freeing a skb list, use a dedicated
> tracepoint instead. By printing "cur" and "next", it also becomes
> possible to reconstruct the skb list from its members.

I like the new probe alone. It helps to have more visibility on drops
such as those from __dev_xmit_skb() and how they happen.

But as a solution to the problem stated, seems it can be confusing.
Say one is debugging a tx drop issue. AFAICT one would have to watch
both probe points anyway, as the drop could be on a layer below than
SCTP. So I'm not seeing how it helps much, other than possibly causing
drop_watch to miss drops (by not listening to the new trace point).

  Marcelo