On 10/02/2018 02:17 PM, Eric Dumazet wrote: > On Tue, Oct 2, 2018 at 1:07 PM Florian Fainelli <f.faine...@gmail.com> wrote: >> >> Hi Eric, Neil, >> >> Should not __dev_kfree_skb_any() call kfree_skb() instead of >> dev_kfree_skb() which is aliased to consumes_skb() and therefore does >> not flag the skb with SKB_REASON_DROPPED? >> >> If we take the in_irq() || irqs_disabled() branch, we will be calling >> __dev_kfree_skb_irq() which takes care of setting the skb_free_reason >> frmo the caller. >> >> Is there an implied semantic with dev_kfree_skb() that it means it was >> freed by the network device and therefore this equals to a consumption >> (not a drop)? The comment above dev_kfree_skb_any() seems to imply this >> should be a context unaware replacement for kfree_skb(). > > > Really the problem here is that we have more than one thousand calls > to dev_kfree_skb_any() > (compared to ~ 90 calls to dev_consume_skb_any()) > > So it will be a huge task cleaning all this.
So you are kind of saying this is an established behavior, don't change it :) One could argue that if people were happily sprinkling dev_kfree_skb_any() in error or success paths, and all SKB freeing was accounted for as "consumed" instead of "dropped" in non-atomic context, this may not be such a big deal to reverse that and make it "dropped" in all contexts? > > (dev_kfree_skb_any() calls were added way before the drop monitor stuff) > -- Florian
