netdev - Re: [PATCH v5 05/11] net: mac802154: Create a transmit bus error helper

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

Message-ID: <20220407120251.5b15db9d@xps13>
Date:   Thu, 7 Apr 2022 12:02:51 +0200
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Alexander Aring <alex.aring@...il.com>
Cc:     Stefan Schmidt <stefan@...enfreihafen.org>,
        linux-wpan - ML <linux-wpan@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        "open list:NETWORKING [GENERAL]" <netdev@...r.kernel.org>,
        David Girault <david.girault@...vo.com>,
        Romuald Despres <romuald.despres@...vo.com>,
        Frederic Blain <frederic.blain@...vo.com>,
        Nicolas Schodet <nico@...fr.eu.org>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH v5 05/11] net: mac802154: Create a transmit bus error
 helper


miquel.raynal@...tlin.com wrote on Thu, 7 Apr 2022 09:56:05 +0200:

> Hi Alexander,
> 
> alex.aring@...il.com wrote on Wed, 6 Apr 2022 17:43:30 -0400:
> 
> > Hi,
> > 
> > On Wed, Apr 6, 2022 at 11:34 AM Miquel Raynal <miquel.raynal@...tlin.com> wrote:  
> > >
> > > A few drivers do the full transmit operation asynchronously, which means
> > > that a bus error that happens when forwarding the packet to the
> > > transmitter will not be reported immediately. The solution in this case
> > > is to call this new helper to free the necessary resources, restart the
> > > the queue and return a generic TRAC error code: IEEE802154_SYSTEM_ERROR.
> > >
> > > Suggested-by: Alexander Aring <alex.aring@...il.com>
> > > Signed-off-by: Miquel Raynal <miquel.raynal@...tlin.com>
> > > ---
> > >  include/net/mac802154.h |  9 +++++++++
> > >  net/mac802154/util.c    | 10 ++++++++++
> > >  2 files changed, 19 insertions(+)
> > >
> > > diff --git a/include/net/mac802154.h b/include/net/mac802154.h
> > > index abbe88dc9df5..5240d94aad8e 100644
> > > --- a/include/net/mac802154.h
> > > +++ b/include/net/mac802154.h
> > > @@ -498,6 +498,15 @@ void ieee802154_stop_queue(struct ieee802154_hw *hw);
> > >  void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct sk_buff *skb,
> > >                               bool ifs_handling);
> > >
> > > +/**
> > > + * ieee802154_xmit_bus_error - frame could not be delivered to the trasmitter
> > > + *                             because of a bus error
> > > + *
> > > + * @hw: pointer as obtained from ieee802154_alloc_hw().
> > > + * @skb: buffer for transmission
> > > + */
> > > +void ieee802154_xmit_bus_error(struct ieee802154_hw *hw, struct sk_buff *skb);
> > > +
> > >  /**
> > >   * ieee802154_xmit_error - frame transmission failed
> > >   *
> > > diff --git a/net/mac802154/util.c b/net/mac802154/util.c
> > > index ec523335336c..79ba803c40c9 100644
> > > --- a/net/mac802154/util.c
> > > +++ b/net/mac802154/util.c
> > > @@ -102,6 +102,16 @@ void ieee802154_xmit_error(struct ieee802154_hw *hw, struct sk_buff *skb,
> > >  }
> > >  EXPORT_SYMBOL(ieee802154_xmit_error);
> > >
> > > +void ieee802154_xmit_bus_error(struct ieee802154_hw *hw, struct sk_buff *skb)
> > > +{
> > > +       struct ieee802154_local *local = hw_to_local(hw);
> > > +
> > > +       local->tx_result = IEEE802154_SYSTEM_ERROR;
> > > +       ieee802154_wake_queue(hw);
> > > +       dev_kfree_skb_any(skb);
> > > +}
> > > +EXPORT_SYMBOL(ieee802154_xmit_bus_error);
> > > +    
> > 
> > why not calling ieee802154_xmit_error(..., IEEE802154_SYSTEM_ERROR) ?
> > Just don't give the user a chance to pick a error code if something
> > bad happened.  
> 
> Oh ok, I assumed, based on your last comment, that you wanted a
> dedicated helper for that, but if just calling xmit_error() with the
> a fixed value is enough I'll drop this commit.

I am re-reading your comment and actually you want this helper, but you
advise to call ieee802154_xmit_error() instead of re-writing its
content, which I agree with. So I will adapt the series in this
direction, hopefully that will match your expectations.

Thanks,
Miquèl