2023-07-26 07:39:11

by Dmitry Antipov

Subject: [PATCH 2/5] wifi: mwifiex: drop BUG_ON() from TX error handling

Remove 'BUG_ON()' from 'mwifiex_process_sta_txpd()' and
'mwifiex_process_uap_txpd()'. In case of insufficient
headrom, issue warning and return NULL, which should be
gracefully handled in 'mwifiex_process_tx()'. Also mark
error handling branches with 'unlikely()' and simplify
error messages (there is no need to use formatted output
to print the value which is known to be zero).

Signed-off-by: Dmitry Antipov <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/sta_tx.c | 13 ++++++++-----
drivers/net/wireless/marvell/mwifiex/uap_txrx.c | 13 ++++++++-----
2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/sta_tx.c b/drivers/net/wireless/marvell/mwifiex/sta_tx.c
index 13c0e67ededf..5995a81f1ce9 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_tx.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_tx.c
@@ -39,14 +39,17 @@ void *mwifiex_process_sta_txpd(struct mwifiex_private *priv,
u16 pkt_type, pkt_offset;
int hroom = adapter->intf_hdr_len;

- if (!skb->len) {
- mwifiex_dbg(adapter, ERROR,
- "Tx: bad packet length: %d\n", skb->len);
+ if (unlikely(!skb->len)) {
+ mwifiex_dbg(adapter, ERROR, "Tx: empty skb\n");
tx_info->status_code = -1;
return skb->data;
}
-
- BUG_ON(skb_headroom(skb) < MWIFIEX_MIN_DATA_HEADER_LEN);
+ if (unlikely(skb_headroom(skb) < MWIFIEX_MIN_DATA_HEADER_LEN)) {
+ mwifiex_dbg(adapter, ERROR,
+ "Tx: insufficient skb headroom: %u < %u\n",
+ skb_headroom(skb), MWIFIEX_MIN_DATA_HEADER_LEN);
+ return NULL;
+ }

pkt_type = mwifiex_is_skb_mgmt_frame(skb) ? PKT_TYPE_MGMT : 0;

diff --git a/drivers/net/wireless/marvell/mwifiex/uap_txrx.c b/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
index e495f7eaea03..ff953e8e7413 100644
--- a/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
+++ b/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
@@ -452,14 +452,17 @@ void *mwifiex_process_uap_txpd(struct mwifiex_private *priv,
u16 pkt_type, pkt_offset;
int hroom = adapter->intf_hdr_len;

- if (!skb->len) {
- mwifiex_dbg(adapter, ERROR,
- "Tx: bad packet length: %d\n", skb->len);
+ if (unlikely(!skb->len)) {
+ mwifiex_dbg(adapter, ERROR, "Tx: empty skb\n");
tx_info->status_code = -1;
return skb->data;
}
-
- BUG_ON(skb_headroom(skb) < MWIFIEX_MIN_DATA_HEADER_LEN);
+ if (unlikely(skb_headroom(skb) < MWIFIEX_MIN_DATA_HEADER_LEN)) {
+ mwifiex_dbg(adapter, ERROR,
+ "Tx: insufficient skb headroom: %u < %u\n",
+ skb_headroom(skb), MWIFIEX_MIN_DATA_HEADER_LEN);
+ return NULL;
+ }

pkt_type = mwifiex_is_skb_mgmt_frame(skb) ? PKT_TYPE_MGMT : 0;

--
2.41.0

2023-07-26 19:40:13

by Brian Norris

Subject: Re: [PATCH 2/5] wifi: mwifiex: drop BUG_ON() from TX error handling

On Wed, Jul 26, 2023 at 10:20:53AM +0300, Dmitry Antipov wrote:
> Remove 'BUG_ON()' from 'mwifiex_process_sta_txpd()' and
> 'mwifiex_process_uap_txpd()'. In case of insufficient
> headrom, issue warning and return NULL, which should be
> gracefully handled in 'mwifiex_process_tx()'. Also mark
> error handling branches with 'unlikely()' and simplify
> error messages (there is no need to use formatted output
> to print the value which is known to be zero).
>
> Signed-off-by: Dmitry Antipov <[email protected]>

You already sent a version of this patch, and I already gave you
feedback:

https://lore.kernel.org/all/[email protected]/

You didn't respond to or integrate that feedback.

Brian