netdev - Re: [RFC PATCH net-next] net: use a dedicated tracepoint for kfree_skb

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] [day] [month] [year] [list]

Message-ID: <20201027223518.GA11029@localhost.localdomain>
Date:   Tue, 27 Oct 2020 19:35:18 -0300
From:   Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
To:     Davide Caratti <dcaratti@...hat.com>
Cc:     netdev@...r.kernel.org, Xin Long <lucien.xin@...il.com>,
        linux-sctp@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>
Subject: Re: [RFC PATCH net-next] net: use a dedicated tracepoint for
 kfree_skb_list()

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