[bug report] ath11k: handle RX fragments

[bug report] ath11k: handle RX fragments

[Dan Carpenter]

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

Re: [bug report] ath11k: handle RX fragments

[Sriram R]

Hi Dan,

> -----Original Message-----
> From: linux-wireless-owner@vger.kernel.org
> <linux-wireless-owner@vger.kernel.org> On Behalf Of Dan Carpenter
> Sent: Tuesday, April 7, 2020 7:11 PM
> To: mpubbise@codeaurora.org
> Cc: ath11k@lists.infradead.org; linux-wireless@vger.kernel.org
> 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);
>   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