| Message ID | 20220406153441.1667375-6-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ieee802154: Better Tx error handling | expand |
Hi, On Wed, Apr 6, 2022 at 11:34 AM Miquel Raynal <miquel.raynal@bootlin.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@gmail.com> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.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. - Alex
Hi Alexander, alex.aring@gmail.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@bootlin.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@gmail.com> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.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. Thanks, Miquèl
miquel.raynal@bootlin.com wrote on Thu, 7 Apr 2022 09:56:05 +0200: > Hi Alexander, > > alex.aring@gmail.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@bootlin.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@gmail.com> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.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
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); + void ieee802154_stop_device(struct ieee802154_local *local) { flush_workqueue(local->workqueue);
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@gmail.com> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- include/net/mac802154.h | 9 +++++++++ net/mac802154/util.c | 10 ++++++++++ 2 files changed, 19 insertions(+)