Skip to contentSkip to conversation

ath11k: handle RX fragments

bug report Dan Carpenter 2 participants 2 messages Apr 7, 2020 – Apr 13, 2020 1 message not in the archive Qualcomm Atheros ATH11K wireless driver

lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

Hello Manikanta Pubbisetty,

The patch 243874c64c81: "ath11k: handle RX fragments" from Mar 16, 2020, leads to the following static checker warning:

	drivers/net/wireless/ath/ath11k/dp_rx.c:3365 ath11k_dp_rx_frag_h_mpdu()
	warn: missing error code here? 'ath11k_dp_rx_h_defrag()' failed. 'ret' = '0'

drivers/net/wireless/ath/ath11k/dp_rx.c
3343 HAL_WBM_REL_BM_ACT_PUT_IN_IDLE);
3344 }
3345
3346 if (!rx_tid->last_frag_no ||
3347 rx_tid->rx_frag_bitmap != GENMASK(rx_tid->last_frag_no, 0)) {
3348 mod_timer(&rx_tid->frag_timer, jiffies +
3349 ATH11K_DP_RX_FRAGMENT_TIMEOUT_MS);

  3350                  goto out_unlock;
                        ^^^^^^^^^^^^^^^

All these gotos should probably set error codes instead of returning success.

3351 }
3352

  3353          spin_unlock_bh(&ab->base_lock);
  3354          del_timer_sync(&rx_tid->frag_timer);
  3355          spin_lock_bh(&ab->base_lock);
  3356  
  3357          peer = ath11k_peer_find_by_id(ab, peer_id);
  3358          if (!peer)
  3359                  goto err_frags_cleanup;
                        ^^^^^^^^^^^^^^^^^^^^^^

Etc.

  3360  
  3361          if (!ath11k_dp_rx_h_defrag_validate_incr_pn(ar, rx_tid))
  3362                  goto err_frags_cleanup;
  3363  
  3364          if (ath11k_dp_rx_h_defrag(ar, peer, rx_tid, &defrag_skb))
  3365                  goto err_frags_cleanup;
  3366  
  3367          if (!defrag_skb)
  3368                  goto err_frags_cleanup;
  3369  
  3370          if (ath11k_dp_rx_h_defrag_reo_reinject(ar, rx_tid, defrag_skb))
  3371                  goto err_frags_cleanup;
  3372  
  3373          ath11k_dp_rx_frags_cleanup(rx_tid, false);
  3374          goto out_unlock;
  3375  
  3376  err_frags_cleanup:
  3377          dev_kfree_skb_any(defrag_skb);
  3378          ath11k_dp_rx_frags_cleanup(rx_tid, true);
  3379  out_unlock:
  3380          spin_unlock_bh(&ab->base_lock);
  3381          return ret;
  3382  }

regards,
dan carpenter

_______________________________________________
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k

A message that is not in the archive (sent off-list or not yet received): BY5PR02MB6820A065D360B48F7251CADAF7DD0@BY5PR02MB6820.namprd02.prod.outlook.com

#2 lore raw

Hi Dan,

Quoted 27 lines

-----Original Message-----
From: [email protected] <[email protected]> On Behalf Of Dan Carpenter
Sent: Tuesday, April 7, 2020 7:11 PM
To: [email protected]
Cc: [email protected]; [email protected]
Subject: [EXT] [bug report] ath11k: handle RX fragments

Hello Manikanta Pubbisetty,

The patch 243874c64c81: "ath11k: handle RX fragments" from Mar 16, 2020, leads to the following static checker warning:

drivers/net/wireless/ath/ath11k/dp_rx.c:3365 ath11k_dp_rx_frag_h_mpdu()
warn: missing error code here? 'ath11k_dp_rx_h_defrag()' failed. 'ret' = '0'

drivers/net/wireless/ath/ath11k/dp_rx.c
3343
HAL_WBM_REL_BM_ACT_PUT_IN_IDLE);
3344 }
3345
3346 if (!rx_tid->last_frag_no ||
3347 rx_tid->rx_frag_bitmap !=
GENMASK(rx_tid->last_frag_no, 0)) {
3348 mod_timer(&rx_tid->frag_timer, jiffies +
3349
ATH11K_DP_RX_FRAGMENT_TIMEOUT_MS);

Quoted 4 lines
  3350                  goto out_unlock;
                        ^^^^^^^^^^^^^^^

All these gotos should probably set error codes instead of returning success.

Thanks for pointing out. But, this function was written in this way on purpose.
The error retval to caller and error goto's in this function perform different cleanups and moving them to caller will only make the caller function complex which is better to avoid.

Thanks,
Sriram.R

_______________________________________________
ath11k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath11k