This partially reverts commit 44ba973699b831414c3f8eef68ee5a7fe1208a05.
rate_control_rate_init assumes the rate_init member of
struct rate_control_ops is not NULL therefore not initializing it leads to
an oops as soon the driver succesfully associates to an AP.
The removal of rate_update from 44ba973699b831414c3f8eef68ee5a7fe1208a05
is ok because rate_update is checked for NULL before being
called.
Signed-off-by: Catalin Iacob <[email protected]>
---
drivers/net/wireless/rtlwifi/rc.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/wireless/rtlwifi/rc.c b/drivers/net/wireless/rtlwifi/rc.c
index 204f46c..6ad8bb7 100644
--- a/drivers/net/wireless/rtlwifi/rc.c
+++ b/drivers/net/wireless/rtlwifi/rc.c
@@ -217,6 +217,12 @@ static void rtl_tx_status(void *ppriv,
}
}
+static void rtl_rate_init(void *ppriv,
+ struct ieee80211_supported_band *sband,
+ struct ieee80211_sta *sta, void *priv_sta)
+{
+}
+
static void *rtl_rate_alloc(struct ieee80211_hw *hw,
struct dentry *debugfsdir)
{
@@ -261,6 +267,7 @@ static struct rate_control_ops rtl_rate_ops = {
.free = rtl_rate_free,
.alloc_sta = rtl_rate_alloc_sta,
.free_sta = rtl_rate_free_sta,
+ .rate_init = rtl_rate_init,
.tx_status = rtl_tx_status,
.get_rate = rtl_get_rate,
};
--
1.8.1.1
On 02/11/2013 03:18 PM, Catalin Iacob wrote:
> This partially reverts commit 44ba973699b831414c3f8eef68ee5a7fe1208a05.
>
> rate_control_rate_init assumes the rate_init member of
> struct rate_control_ops is not NULL therefore not initializing it leads to
> an oops as soon the driver succesfully associates to an AP.
>
> The removal of rate_update from 44ba973699b831414c3f8eef68ee5a7fe1208a05
> is ok because rate_update is checked for NULL before being
> called.
>
> Signed-off-by: Catalin Iacob <[email protected]>
ACKed-by: Larry Finger <[email protected]>
John,
This patch should be applied. Obviously, I missed the oops in my testing.
The real fix will be to not supply any rate-control operations as suggested by
Johannes, but that will take some time.
Larry
On Mon, 2013-02-11 at 22:18 +0100, Catalin Iacob wrote:
> This partially reverts commit 44ba973699b831414c3f8eef68ee5a7fe1208a05.
>
> rate_control_rate_init assumes the rate_init member of
> struct rate_control_ops is not NULL therefore not initializing it leads to
> an oops as soon the driver succesfully associates to an AP.
>
> The removal of rate_update from 44ba973699b831414c3f8eef68ee5a7fe1208a05
> is ok because rate_update is checked for NULL before being
> called.
OTOH, if the driver supports HT then rate_update is critical to its
functioning properly, e.g. for SMPS updates.
johannes
On 02/11/2013 03:18 PM, Catalin Iacob wrote:
> This partially reverts commit 44ba973699b831414c3f8eef68ee5a7fe1208a05.
>
> rate_control_rate_init assumes the rate_init member of
> struct rate_control_ops is not NULL therefore not initializing it leads to
> an oops as soon the driver succesfully associates to an AP.
>
> The removal of rate_update from 44ba973699b831414c3f8eef68ee5a7fe1208a05
> is ok because rate_update is checked for NULL before being
> called.
>
> Signed-off-by: Catalin Iacob <[email protected]>
> ---
> drivers/net/wireless/rtlwifi/rc.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/wireless/rtlwifi/rc.c b/drivers/net/wireless/rtlwifi/rc.c
> index 204f46c..6ad8bb7 100644
> --- a/drivers/net/wireless/rtlwifi/rc.c
> +++ b/drivers/net/wireless/rtlwifi/rc.c
> @@ -217,6 +217,12 @@ static void rtl_tx_status(void *ppriv,
> }
> }
>
> +static void rtl_rate_init(void *ppriv,
> + struct ieee80211_supported_band *sband,
> + struct ieee80211_sta *sta, void *priv_sta)
> +{
> +}
> +
> static void *rtl_rate_alloc(struct ieee80211_hw *hw,
> struct dentry *debugfsdir)
> {
> @@ -261,6 +267,7 @@ static struct rate_control_ops rtl_rate_ops = {
> .free = rtl_rate_free,
> .alloc_sta = rtl_rate_alloc_sta,
> .free_sta = rtl_rate_free_sta,
> + .rate_init = rtl_rate_init,
> .tx_status = rtl_tx_status,
> .get_rate = rtl_get_rate,
> };
Shouldn't rate_control_rate_init() in net/mac80211/rate.h be changed to protect
against the oops? I don't see any value in the client driver having to provide a
dummy routine. The rtlwifi family of drivers use their own rate-control
mechanism, and get no rate-control info from mac80211.
Larry
On 02/11/2013 03:42 PM, Johannes Berg wrote:
> On Mon, 2013-02-11 at 15:37 -0600, Larry Finger wrote:
>
>>> + .rate_init = rtl_rate_init,
>>> .tx_status = rtl_tx_status,
>>> .get_rate = rtl_get_rate,
>>> };
>>
>> Shouldn't rate_control_rate_init() in net/mac80211/rate.h be changed to protect
>> against the oops? I don't see any value in the client driver having to provide a
>> dummy routine. The rtlwifi family of drivers use their own rate-control
>> mechanism, and get no rate-control info from mac80211.
>
> In this case, I see no value in them providing rate control operations
> at all, and they could just set IEEE80211_HW_HAS_RATE_CONTROL, no?
I think that makes sense.
Larry
On Mon, 2013-02-11 at 15:37 -0600, Larry Finger wrote:
> > + .rate_init = rtl_rate_init,
> > .tx_status = rtl_tx_status,
> > .get_rate = rtl_get_rate,
> > };
>
> Shouldn't rate_control_rate_init() in net/mac80211/rate.h be changed to protect
> against the oops? I don't see any value in the client driver having to provide a
> dummy routine. The rtlwifi family of drivers use their own rate-control
> mechanism, and get no rate-control info from mac80211.
In this case, I see no value in them providing rate control operations
at all, and they could just set IEEE80211_HW_HAS_RATE_CONTROL, no?
johannes