Re: rtlwifi: potential bugs

[Pkshih <pkshih@realtek.com>]

On Wed, 2021-05-05 at 11:23 +0000, Inigo Huguet wrote:
> On Fri, Apr 23, 2021 at 2:56 PM Inigo Huguet <ihuguet@redhat.com> wrote:
> >
> > Hello,
> >
> > Executing some static analysis on the kernel, we've got this results
> > affecting rtlwifi drivers:
> >
> > Error: IDENTICAL_BRANCHES (CWE-398): [#def212]
> > kernel-5.11.0-0.rc7.151/linux-5.11.0-0.rc7.151.el9.x86_64/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2813:
> > identical_branches: The same code is executed regardless of whether
> > "bt_rssi_state == BTC_RSSI_STATE_HIGH || bt_rssi_state ==
> > BTC_RSSI_STATE_STAY_HIGH" is true, because the 'then' and 'else'
> > branches are identical. Should one of the branches be modified, or the
> > entire 'if' statement replaced?
> > # 2811|   }
> > # 2812|
> > # 2813|-> if ((bt_rssi_state == BTC_RSSI_STATE_HIGH) ||
> > # 2814|      (bt_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) {
> > # 2815|   btc8821a2ant_ps_tdma(btcoexist, NORMAL_EXEC, true, 23);
> >
> > Error: IDENTICAL_BRANCHES (CWE-398): [#def213]
> > kernel-5.11.0-0.rc7.151/linux-5.11.0-0.rc7.151.el9.x86_64/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2947:
> > identical_branches: The same code is executed regardless of whether
> > "bt_rssi_state == BTC_RSSI_STATE_HIGH || bt_rssi_state ==
> > BTC_RSSI_STATE_STAY_HIGH" is true, because the 'then' and 'else'
> > branches are identical. Should one of the branches be modified, or the
> > entire 'if' statement replaced?
> > # 2945|   }
> > # 2946|
> > # 2947|-> if ((bt_rssi_state == BTC_RSSI_STATE_HIGH) ||
> > # 2948|      (bt_rssi_state == BTC_RSSI_STATE_STAY_HIGH))
> > # 2949|   btc8821a2ant_ps_tdma(btcoexist, NORMAL_EXEC, true, 26);
> >
> > Error: IDENTICAL_BRANCHES (CWE-398): [#def214]
> > kernel-5.11.0-0.rc7.151/linux-5.11.0-0.rc7.151.el9.x86_64/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3135:
> > identical_branches: The same code is executed regardless of whether
> > "wifi_bw == BTC_WIFI_BW_LEGACY" is true, because the 'then' and 'else'
> > branches are identical. Should one of the branches be modified, or the
> > entire 'if' statement replaced?
> > # 3133|   btcoexist->btc_get(btcoexist, BTC_GET_U4_WIFI_BW, &wifi_bw);
> > # 3134|
> > # 3135|-> if (wifi_bw == BTC_WIFI_BW_LEGACY) {
> > # 3136|   /* for HID at 11b/g mode */
> > # 3137|   btc8821a2ant_coex_table(btcoexist, NORMAL_EXEC, 0x55ff55ff,
> >
> > Error: IDENTICAL_BRANCHES (CWE-398): [#def215]
> > kernel-5.11.0-0.rc7.151/linux-5.11.0-0.rc7.151.el9.x86_64/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3324:
> > identical_branches: The same code is executed regardless of whether
> > "bt_rssi_state == BTC_RSSI_STATE_HIGH || bt_rssi_state ==
> > BTC_RSSI_STATE_STAY_HIGH" is true, because the 'then' and 'else'
> > branches are identical. Should one of the branches be modified, or the
> > entire 'if' statement replaced?
> > # 3322|   }
> > # 3323|
> > # 3324|-> if ((bt_rssi_state == BTC_RSSI_STATE_HIGH) ||
> > # 3325|      (bt_rssi_state == BTC_RSSI_STATE_STAY_HIGH)) {
> > # 3326|   btc8821a2ant_ps_tdma(btcoexist, NORMAL_EXEC, true, 23);
> >
> >
> > In my opinion, they seem to be real bugs. However, it's very difficult
> > to imagine what actions must be taken on each branch of the if-else
> > because they strongly depend on magic numbers, which are different
> > configurations for the hw, I guess.
> >
> > Can the maintainers confirm if these are real bugs and see how to fix them?
> >
> > Regards
> > --
> > Íñigo Huguet
> 
> Hello,
> 
> A few weeks ago I sent the message above notifying a potential bug in
> rtlwifi module. I just wanted to be sure that it has been received.
> Can the maintainers acknowledge whether they have seen it?
> 

Hi,

Not real bugs. The coexistence programmers preserve the same code of
branches intentionally to fine tune performance easier, because bandwidth and
RSSI strength are highly related to coexistence performance.
The basic rule of performance tuning is to assign most time slot to BT
for realtime application, and WiFi uses remaining time slot but don't lower
than low bound.

--
Ping-Ke