* [PATCH] rtlwifi: Initialize rate_init member of struct rate_control_ops
@ 2013-02-11 21:18 Catalin Iacob
2013-02-11 21:22 ` Johannes Berg
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Catalin Iacob @ 2013-02-11 21:18 UTC (permalink / raw)
To: Larry Finger; +Cc: John W. Linville, linux-wireless, Catalin Iacob
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 <iacobcatalin@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] rtlwifi: Initialize rate_init member of struct rate_control_ops
2013-02-11 21:18 [PATCH] rtlwifi: Initialize rate_init member of struct rate_control_ops Catalin Iacob
@ 2013-02-11 21:22 ` Johannes Berg
2013-02-11 21:37 ` Larry Finger
2013-02-11 21:58 ` Larry Finger
2 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2013-02-11 21:22 UTC (permalink / raw)
To: Catalin Iacob; +Cc: Larry Finger, John W. Linville, linux-wireless
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rtlwifi: Initialize rate_init member of struct rate_control_ops
2013-02-11 21:18 [PATCH] rtlwifi: Initialize rate_init member of struct rate_control_ops Catalin Iacob
2013-02-11 21:22 ` Johannes Berg
@ 2013-02-11 21:37 ` Larry Finger
2013-02-11 21:42 ` Johannes Berg
2013-02-11 21:58 ` Larry Finger
2 siblings, 1 reply; 6+ messages in thread
From: Larry Finger @ 2013-02-11 21:37 UTC (permalink / raw)
To: Catalin Iacob; +Cc: John W. Linville, linux-wireless, johannes Berg
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 <iacobcatalin@gmail.com>
> ---
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rtlwifi: Initialize rate_init member of struct rate_control_ops
2013-02-11 21:37 ` Larry Finger
@ 2013-02-11 21:42 ` Johannes Berg
2013-02-11 21:48 ` Larry Finger
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Berg @ 2013-02-11 21:42 UTC (permalink / raw)
To: Larry Finger; +Cc: Catalin Iacob, John W. Linville, linux-wireless
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rtlwifi: Initialize rate_init member of struct rate_control_ops
2013-02-11 21:42 ` Johannes Berg
@ 2013-02-11 21:48 ` Larry Finger
0 siblings, 0 replies; 6+ messages in thread
From: Larry Finger @ 2013-02-11 21:48 UTC (permalink / raw)
To: Johannes Berg; +Cc: Catalin Iacob, John W. Linville, linux-wireless
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rtlwifi: Initialize rate_init member of struct rate_control_ops
2013-02-11 21:18 [PATCH] rtlwifi: Initialize rate_init member of struct rate_control_ops Catalin Iacob
2013-02-11 21:22 ` Johannes Berg
2013-02-11 21:37 ` Larry Finger
@ 2013-02-11 21:58 ` Larry Finger
2 siblings, 0 replies; 6+ messages in thread
From: Larry Finger @ 2013-02-11 21:58 UTC (permalink / raw)
To: Catalin Iacob; +Cc: John W. Linville, linux-wireless
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 <iacobcatalin@gmail.com>
ACKed-by: Larry Finger <Larry.Finger@lwfinger.net>
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-02-11 21:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-11 21:18 [PATCH] rtlwifi: Initialize rate_init member of struct rate_control_ops Catalin Iacob
2013-02-11 21:22 ` Johannes Berg
2013-02-11 21:37 ` Larry Finger
2013-02-11 21:42 ` Johannes Berg
2013-02-11 21:48 ` Larry Finger
2013-02-11 21:58 ` Larry Finger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).