From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Fainelli Subject: Re: __dev_kfree_skb_any() and use of dev_kfree_skb() Date: Tue, 2 Oct 2018 14:54:05 -0700 Message-ID: References: <32568fab-02d4-0484-3237-5d8dc09e273d@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: netdev , Neil Horman , David Miller To: Eric Dumazet Return-path: Received: from mail-ed1-f41.google.com ([209.85.208.41]:36618 "EHLO mail-ed1-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725767AbeJCEjp (ORCPT ); Wed, 3 Oct 2018 00:39:45 -0400 Received: by mail-ed1-f41.google.com with SMTP id f4-v6so3508955edq.3 for ; Tue, 02 Oct 2018 14:54:15 -0700 (PDT) In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 10/02/2018 02:17 PM, Eric Dumazet wrote: > On Tue, Oct 2, 2018 at 1:07 PM Florian Fainelli 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