2013-07-30 01:48:19

by Chun-Yeow Yeoh

[permalink] [raw]
Subject: [PATCH] mac80211: allow lowest basic rate for unicast management frame in mesh

Allow lowest basic rate to be used for unicast management frame in
mesh. Otherwise, the lowest supported rate is used for unicast
management frame, such as 1Mbps for 2.4GHz and 6Mbps for 5GHz. Rename
the rc_send_low_broadcast to re_send_low_basicrate since now it is
also applied to unicast management frame in mesh.

Signed-off-by: Chun-Yeow Yeoh <[email protected]>
---
net/mac80211/rate.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index ba63ac8..8038d47 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -210,7 +210,7 @@ static bool rc_no_data_or_no_ack_use_min(struct ieee80211_tx_rate_control *txrc)
!ieee80211_is_data(fc);
}

-static void rc_send_low_broadcast(s8 *idx, u32 basic_rates,
+static void rc_send_low_basicrate(s8 *idx, u32 basic_rates,
struct ieee80211_supported_band *sband)
{
u8 i;
@@ -269,6 +269,7 @@ bool rate_control_send_low(struct ieee80211_sta *sta,
{
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(txrc->skb);
struct ieee80211_supported_band *sband = txrc->sband;
+ struct sta_info *sta_mesh = container_of(sta, struct sta_info, sta);
int mcast_rate;

if (!sta || !priv_sta || rc_no_data_or_no_ack_use_min(txrc)) {
@@ -281,10 +282,16 @@ bool rate_control_send_low(struct ieee80211_sta *sta,
return true;
}

- rc_send_low_broadcast(&info->control.rates[0].idx,
+ rc_send_low_basicrate(&info->control.rates[0].idx,
txrc->bss_conf->basic_rates,
sband);
}
+
+ if (sta && ieee80211_vif_is_mesh(&sta_mesh->sdata->vif))
+ rc_send_low_basicrate(&info->control.rates[0].idx,
+ txrc->bss_conf->basic_rates,
+ sband);
+
return true;
}
return false;
--
1.7.9.5



2013-08-02 05:14:46

by Chun-Yeow Yeoh

[permalink] [raw]
Subject: Re: [PATCH] mac80211: allow lowest basic rate for unicast management frame in mesh

> That doesn't seem like a good idea - IMHO you shouldn't use container_of
> on NULL even if it still works in the end.
>
> Also it might be a good idea to use more regular names: struct
> ieee80211_sta *pubsta, struct sta_info *sta.

Ok.

> I don't get this - that just duplicates the code above, no chance to
> refactor it a bit instead of duplicating the call?

I will take a look on this first.

---
Chun-Yeow

2013-08-01 07:44:34

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: allow lowest basic rate for unicast management frame in mesh

On Mon, 2013-07-29 at 18:48 -0700, Chun-Yeow Yeoh wrote:
> Allow lowest basic rate to be used for unicast management frame in
> mesh. Otherwise, the lowest supported rate is used for unicast
> management frame, such as 1Mbps for 2.4GHz and 6Mbps for 5GHz. Rename
> the rc_send_low_broadcast to re_send_low_basicrate since now it is
> also applied to unicast management frame in mesh.
>
> Signed-off-by: Chun-Yeow Yeoh <[email protected]>
> ---
> net/mac80211/rate.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
> index ba63ac8..8038d47 100644
> --- a/net/mac80211/rate.c
> +++ b/net/mac80211/rate.c
> @@ -210,7 +210,7 @@ static bool rc_no_data_or_no_ack_use_min(struct ieee80211_tx_rate_control *txrc)
> !ieee80211_is_data(fc);
> }
>
> -static void rc_send_low_broadcast(s8 *idx, u32 basic_rates,
> +static void rc_send_low_basicrate(s8 *idx, u32 basic_rates,
> struct ieee80211_supported_band *sband)
> {
> u8 i;
> @@ -269,6 +269,7 @@ bool rate_control_send_low(struct ieee80211_sta *sta,
> {
> struct ieee80211_tx_info *info = IEEE80211_SKB_CB(txrc->skb);
> struct ieee80211_supported_band *sband = txrc->sband;
> + struct sta_info *sta_mesh = container_of(sta, struct sta_info, sta);

That doesn't seem like a good idea - IMHO you shouldn't use container_of
on NULL even if it still works in the end.

Also it might be a good idea to use more regular names: struct
ieee80211_sta *pubsta, struct sta_info *sta.


> @@ -281,10 +282,16 @@ bool rate_control_send_low(struct ieee80211_sta *sta,
> return true;
> }
>
> - rc_send_low_broadcast(&info->control.rates[0].idx,
> + rc_send_low_basicrate(&info->control.rates[0].idx,
> txrc->bss_conf->basic_rates,
> sband);
> }
> +
> + if (sta && ieee80211_vif_is_mesh(&sta_mesh->sdata->vif))
> + rc_send_low_basicrate(&info->control.rates[0].idx,
> + txrc->bss_conf->basic_rates,
> + sband);

I don't get this - that just duplicates the code above, no chance to
refactor it a bit instead of duplicating the call?

johannes