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
