2008-09-10 22:16:11

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 00/18] mac80211 cleanups and fixes

Hi,

Here's another series to clean up a bit including a few things that
I've been meaning to do for a long time:

* mac80211: move ieee80211_set_freq to utils
* mac80211: make bridge_packets a virtual interface option
* mac80211: clean up some comments
* mac80211: inform driver of basic rateset
* mac80211: use nl80211 interface types

I'd been talking with Sujith about this one:

* mac80211: share STA information with driver

and decided to just do it; as it stands it's not very useful
but I expect it to be used soon, if not we can remove the
EXPORT_SYMBOL again.

johannes



2008-09-11 03:28:17

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 25/18] mac80211: pass AP vif pointer for VLANs

We cannot pass a VLAN vif pointer to the driver since those are
entirely virtual and we never tell the driver.

Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/tx.c | 4 ++++
1 file changed, 4 insertions(+)

--- everything.orig/net/mac80211/tx.c 2008-09-11 05:22:58.000000000 +0200
+++ everything/net/mac80211/tx.c 2008-09-11 05:25:16.000000000 +0200
@@ -1351,6 +1351,10 @@ int ieee80211_master_start_xmit(struct s
return 0;
}

+ if (osdata->vif.type == NL80211_IFTYPE_AP_VLAN)
+ osdata = container_of(osdata->bss,
+ struct ieee80211_sub_if_data,
+ u.ap);
info->control.vif = &osdata->vif;
ret = ieee80211_tx(odev, skb);
dev_put(odev);



2008-09-11 08:31:20

by Sujith

[permalink] [raw]
Subject: [PATCH 00/18] mac80211 cleanups and fixes

Johannes Berg wrote:
> Hi,
>
> Here's another series to clean up a bit including a few things that
> I've been meaning to do for a long time:
>
> * mac80211: move ieee80211_set_freq to utils
> * mac80211: make bridge_packets a virtual interface option
> * mac80211: clean up some comments
> * mac80211: inform driver of basic rateset
> * mac80211: use nl80211 interface types
>
> I'd been talking with Sujith about this one:
>
> * mac80211: share STA information with driver
>
> and decided to just do it; as it stands it's not very useful
> but I expect it to be used soon, if not we can remove the
> EXPORT_SYMBOL again.
>

This would be very useful for ath9k. Making use of the
private area for each STA can enable removal of the node list
that is maintained currently.

Sujith

2008-09-11 17:40:19

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 00/18] mac80211 cleanups and fixes

On Thu, 2008-09-11 at 10:33 -0700, Luis R. Rodriguez wrote:
> On Thu, Sep 11, 2008 at 09:53:15AM -0700, Johannes Berg wrote:
> > On Thu, 2008-09-11 at 09:50 -0700, Luis R. Rodriguez wrote:
> > > > I had actually tried
> > > > (http://johannes.sipsolutions.net/patches/kernel/ath9k-sta-node.patch)
>
> Just a comment so far from the patch.
>
> > + rcu_read_unlock();
> > +
> > + /* the "!an" here is fine even outside RCU lock */
>
> Why is that? I fail to see that.

Well take the larger bit of code:

struct something *an = NULL;

...

rcu_read_lock();
sta = ieee80211_find_sta(hw, hdr->addr2);
if (sta)
an = (void *) sta->drv_priv;

if (an) {
ath_rx_input(sc, an,
hw->conf.ht_conf.ht_supported,
skb, status, &st);
}
rcu_read_unlock();

/* the "!an" here is fine even outside RCU lock */
if (!an || (st != ATH_RX_CONSUMED))
__ieee80211_rx(hw, skb, &rx_status);


So at this point it's only checking whether above it had a pointer, it's
not accessing it. Think of the "an" variable, after rcu_read_unlock(),
as a bool indicating whether or not the code that just happened had
access to the node or not.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-11 00:23:35

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 20/18] mac80211: move last_txrate_idx into RC algorithms

This variable in sta_info is only used in a meaningful way
by the Intel RC algorithms, so move it into those.

Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-3945-rs.c | 20 ++++++++++++--------
drivers/net/wireless/iwlwifi/iwl-agn-rs.c | 15 +++++++++------
net/mac80211/rc80211_pid_algo.c | 2 --
net/mac80211/sta_info.h | 2 --
4 files changed, 21 insertions(+), 18 deletions(-)

--- everything.orig/drivers/net/wireless/iwlwifi/iwl-agn-rs.c 2008-09-11 02:15:29.000000000 +0200
+++ everything/drivers/net/wireless/iwlwifi/iwl-agn-rs.c 2008-09-11 02:19:12.000000000 +0200
@@ -163,6 +163,9 @@ struct iwl_lq_sta {
u32 dbg_fixed_rate;
#endif
struct iwl_priv *drv;
+
+ /* used to be in sta_info */
+ int last_txrate_idx;
};

static void rs_rate_scale_perform(struct iwl_priv *priv,
@@ -1746,7 +1749,7 @@ static void rs_rate_scale_perform(struct
is_green = lq_sta->is_green;

/* current tx rate */
- index = sta->last_txrate_idx;
+ index = lq_sta->last_txrate_idx;

IWL_DEBUG_RATE("Rate scale index %d for type %d\n", index,
tbl->lq_type);
@@ -2059,7 +2062,7 @@ lq_update:
out:
tbl->current_rate = rate_n_flags_from_tbl(tbl, index, is_green);
i = index;
- sta->last_txrate_idx = i;
+ lq_sta->last_txrate_idx = i;

/* sta->txrate_idx is an index to A mode rates which start
* at IWL_FIRST_OFDM_RATE
@@ -2090,7 +2093,7 @@ static void rs_initialize_lq(struct iwl_
goto out;

lq_sta = (struct iwl_lq_sta *)sta->rate_ctrl_priv;
- i = sta->last_txrate_idx;
+ i = lq_sta->last_txrate_idx;

if ((lq_sta->lq.sta_id == 0xff) &&
(priv->iw_mode == NL80211_IFTYPE_ADHOC))
@@ -2161,7 +2164,7 @@ static void rs_get_rate(void *priv_rate,
}

lq_sta = (struct iwl_lq_sta *)sta->rate_ctrl_priv;
- i = sta->last_txrate_idx;
+ i = lq_sta->last_txrate_idx;

if ((priv->iw_mode == NL80211_IFTYPE_ADHOC) &&
!lq_sta->ibss_sta_added) {
@@ -2270,10 +2273,10 @@ static void rs_rate_init(void *priv_rate
if (sta->supp_rates[sband->band] & BIT(i))
sta->txrate_idx = i;

- sta->last_txrate_idx = sta->txrate_idx;
+ lq_sta->last_txrate_idx = sta->txrate_idx;
/* For MODE_IEEE80211A, skip over cck rates in global rate table */
if (local->hw.conf.channel->band == IEEE80211_BAND_5GHZ)
- sta->last_txrate_idx += IWL_FIRST_OFDM_RATE;
+ lq_sta->last_txrate_idx += IWL_FIRST_OFDM_RATE;

lq_sta->is_dup = 0;
lq_sta->is_green = rs_use_green(priv, conf);
--- everything.orig/net/mac80211/rc80211_pid_algo.c 2008-09-11 02:19:12.000000000 +0200
+++ everything/net/mac80211/rc80211_pid_algo.c 2008-09-11 02:19:12.000000000 +0200
@@ -329,8 +329,6 @@ static void rate_control_pid_get_rate(vo
if (rateidx >= sband->n_bitrates)
rateidx = sband->n_bitrates - 1;

- sta->last_txrate_idx = rateidx;
-
rcu_read_unlock();

sel->rate_idx = rateidx;
--- everything.orig/net/mac80211/sta_info.h 2008-09-11 02:19:12.000000000 +0200
+++ everything/net/mac80211/sta_info.h 2008-09-11 02:19:12.000000000 +0200
@@ -200,7 +200,6 @@ struct sta_ampdu_mlme {
* @tx_bytes: TBD
* @tx_fragments: number of transmitted MPDUs
* @txrate_idx: TBD
- * @last_txrate_idx: TBD
* @tid_seq: TBD
* @wme_tx_queue: TBD
* @ampdu_mlme: TBD
@@ -278,7 +277,6 @@ struct sta_info {
unsigned long tx_bytes;
unsigned long tx_fragments;
int txrate_idx;
- int last_txrate_idx;
u16 tid_seq[IEEE80211_QOS_CTL_TID_MASK + 1];
#ifdef CONFIG_MAC80211_DEBUG_COUNTERS
unsigned int wme_tx_queue[NUM_RX_DATA_QUEUES];
--- everything.orig/drivers/net/wireless/iwlwifi/iwl-3945-rs.c 2008-09-11 02:15:08.000000000 +0200
+++ everything/drivers/net/wireless/iwlwifi/iwl-3945-rs.c 2008-09-11 02:19:12.000000000 +0200
@@ -65,6 +65,9 @@ struct iwl3945_rs_sta {
u8 ibss_sta_added;
struct timer_list rate_scale_flush;
struct iwl3945_rate_scale_data win[IWL_RATE_COUNT];
+
+ /* used to be in sta_info */
+ int last_txrate_idx;
};

static s32 iwl3945_expected_tpt_g[IWL_RATE_COUNT] = {
@@ -319,6 +322,7 @@ static void iwl3945_collect_tx_data(stru
static void rs_rate_init(void *priv_rate, void *priv_sta,
struct ieee80211_local *local, struct sta_info *sta)
{
+ struct iwl3945_rs_sta *rs_sta = (void *)sta->rate_ctrl_priv;
int i;

IWL_DEBUG_RATE("enter\n");
@@ -335,11 +339,11 @@ static void rs_rate_init(void *priv_rate
}
}

- sta->last_txrate_idx = sta->txrate_idx;
+ rs_sta->last_txrate_idx = sta->txrate_idx;

/* For 5 GHz band it start at IWL_FIRST_OFDM_RATE */
if (local->hw.conf.channel->band == IEEE80211_BAND_5GHZ)
- sta->last_txrate_idx += IWL_FIRST_OFDM_RATE;
+ rs_sta->last_txrate_idx += IWL_FIRST_OFDM_RATE;

IWL_DEBUG_RATE("leave\n");
}
@@ -674,14 +678,14 @@ static void rs_get_rate(void *priv_rate,
return;
}

+ rs_sta = (void *)sta->rate_ctrl_priv;
+
rate_mask = sta->supp_rates[sband->band];
- index = min(sta->last_txrate_idx & 0xffff, IWL_RATE_COUNT - 1);
+ index = min(rs_sta->last_txrate_idx & 0xffff, IWL_RATE_COUNT - 1);

if (sband->band == IEEE80211_BAND_5GHZ)
rate_mask = rate_mask << IWL_FIRST_OFDM_RATE;

- rs_sta = (void *)sta->rate_ctrl_priv;
-
if ((priv->iw_mode == NL80211_IFTYPE_ADHOC) &&
!rs_sta->ibss_sta_added) {
u8 sta_id = iwl3945_hw_find_station(priv, hdr->addr1);
@@ -803,11 +807,11 @@ static void rs_get_rate(void *priv_rate,

out:

- sta->last_txrate_idx = index;
+ rs_sta->last_txrate_idx = index;
if (sband->band == IEEE80211_BAND_5GHZ)
- sta->txrate_idx = sta->last_txrate_idx - IWL_FIRST_OFDM_RATE;
+ sta->txrate_idx = rs_sta->last_txrate_idx - IWL_FIRST_OFDM_RATE;
else
- sta->txrate_idx = sta->last_txrate_idx;
+ sta->txrate_idx = rs_sta->last_txrate_idx;

rcu_read_unlock();




2008-09-11 17:11:08

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 00/18] mac80211 cleanups and fixes

On Thu, Sep 11, 2008 at 09:53:15AM -0700, Johannes Berg wrote:
> On Thu, 2008-09-11 at 09:50 -0700, Luis R. Rodriguez wrote:
>
> > > I had actually tried
> > > (http://johannes.sipsolutions.net/patches/kernel/ath9k-sta-node.patch)
> > > but you're using refcounting for the nodes while mac80211 RCU-protects
> > > them, so I didn't get very far.
> >
> > Good thing I read RCU docs yesterday.
>
> Heh.
>
> > We should be able to remove ref counts and simply use RCU and
> > sychronize_rcu() before deletion.
>
> Well you don't get to control deletion then, that's another thing.

Does it matter? Our driver won't care as long as the entry still exists.
Just as with stale routing tables, does it matter if we keep thinking we
have a node for a few moments later?

> > > It needs a bit more effort to make sure
> > > you don't have node pointers stick around in some tx descriptor after
> > > mac80211 decides to remove a node (which may very well happen while
> > > frames are queued)
> >
> > We can call_rcu() here no, and let the driver deal with what it needs to
> > prior to deletion?
>
> The driver already gets notified via sta_notify() that the sta is about
> to be deleted, it must make sure that no matter what sort of references
> it kept, the sta will not be accessed after that sta_notify call.

Ah good point. Then what is the issue?

Luis

2008-09-12 07:46:13

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 00/18] mac80211 cleanups and fixes

On Fri, 2008-09-12 at 08:44 +0530, Sujith wrote:
> Johannes Berg wrote:
> > I had actually tried
> > (http://johannes.sipsolutions.net/patches/kernel/ath9k-sta-node.patch)
> > but you're using refcounting for the nodes while mac80211 RCU-protects
> > them, so I didn't get very far. It needs a bit more effort to make sure
> > you don't have node pointers stick around in some tx descriptor after
> > mac80211 decides to remove a node (which may very well happen while
> > frames are queued)
>
> Well, the node stuff is sprinkled throughout ath9k right now, I'll start
> cleaning up all that and revamp the aggregation logic in the process too.
> Do you mind if I build on this patch ?

Not at all, feel free to use it for whatever you want.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-11 01:05:14

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 22/18] mac80211: move txrate_idx into RC algorithms

The sta_info->txrate_idx member isn't used by all RC algorithms
in the way it was intended to be used, move it into those that
require it (only PID) and keep track in the core code of which
rate was last used for reporting to userspace and the mesh MLME.

Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/ath9k/rc.c | 1
drivers/net/wireless/iwlwifi/iwl-3945-rs.c | 10 ++------
drivers/net/wireless/iwlwifi/iwl-agn-rs.c | 13 +----------
net/mac80211/mesh_hwmp.c | 2 -
net/mac80211/rc80211_pid.h | 2 +
net/mac80211/rc80211_pid_algo.c | 33 ++++++++++++++++-------------
net/mac80211/sta_info.h | 2 -
net/mac80211/tx.c | 2 +
net/mac80211/wext.c | 4 +--
9 files changed, 32 insertions(+), 37 deletions(-)

--- everything.orig/net/mac80211/mesh_hwmp.c 2008-09-11 02:39:33.000000000 +0200
+++ everything/net/mac80211/mesh_hwmp.c 2008-09-11 02:39:39.000000000 +0200
@@ -223,7 +223,7 @@ static u32 airtime_link_metric_get(struc
/* bitrate is in units of 100 Kbps, while we need rate in units of
* 1Mbps. This will be corrected on tx_time computation.
*/
- rate = sband->bitrates[sta->txrate_idx].bitrate;
+ rate = sband->bitrates[sta->last_txrate_idx].bitrate;
tx_time = (device_constant + 10 * test_frame_len / rate);
estimated_retx = ((1 << (2 * ARITH_SHIFT)) / (s_unit - err));
result = (tx_time * estimated_retx) >> (2 * ARITH_SHIFT) ;
--- everything.orig/net/mac80211/sta_info.h 2008-09-11 02:39:18.000000000 +0200
+++ everything/net/mac80211/sta_info.h 2008-09-11 02:40:07.000000000 +0200
@@ -274,7 +274,7 @@ struct sta_info {
unsigned long tx_packets;
unsigned long tx_bytes;
unsigned long tx_fragments;
- int txrate_idx;
+ unsigned int last_txrate_idx;
u16 tid_seq[IEEE80211_QOS_CTL_TID_MASK + 1];
#ifdef CONFIG_MAC80211_DEBUG_COUNTERS
unsigned int wme_tx_queue[NUM_RX_DATA_QUEUES];
--- everything.orig/net/mac80211/wext.c 2008-09-11 02:39:43.000000000 +0200
+++ everything/net/mac80211/wext.c 2008-09-11 02:39:50.000000000 +0200
@@ -636,8 +636,8 @@ static int ieee80211_ioctl_giwrate(struc

sta = sta_info_get(local, sdata->u.sta.bssid);

- if (sta && sta->txrate_idx < sband->n_bitrates)
- rate->value = sband->bitrates[sta->txrate_idx].bitrate;
+ if (sta && sta->last_txrate_idx < sband->n_bitrates)
+ rate->value = sband->bitrates[sta->last_txrate_idx].bitrate;
else
rate->value = 0;

--- everything.orig/net/mac80211/rc80211_pid.h 2008-09-11 02:40:21.000000000 +0200
+++ everything/net/mac80211/rc80211_pid.h 2008-09-11 02:40:38.000000000 +0200
@@ -180,6 +180,8 @@ struct rc_pid_sta_info {
u32 tx_num_failed;
u32 tx_num_xmit;

+ int txrate_idx;
+
/* Average failed frames percentage error (i.e. actual vs. target
* percentage), scaled by RC_PID_SMOOTHING. This value is computed
* using using an exponential weighted average technique:
--- everything.orig/net/mac80211/rc80211_pid_algo.c 2008-09-11 02:40:17.000000000 +0200
+++ everything/net/mac80211/rc80211_pid_algo.c 2008-09-11 02:45:02.000000000 +0200
@@ -75,7 +75,8 @@ static void rate_control_pid_adjust_rate
struct ieee80211_sub_if_data *sdata;
struct ieee80211_supported_band *sband;
int cur_sorted, new_sorted, probe, tmp, n_bitrates, band;
- int cur = sta->txrate_idx;
+ struct rc_pid_sta_info *spinfo = (void *)sta->rate_ctrl_priv;
+ int cur = spinfo->txrate_idx;

sdata = sta->sdata;
sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
@@ -111,7 +112,7 @@ static void rate_control_pid_adjust_rate
/* Fit the rate found to the nearest supported rate. */
do {
if (rate_supported(sta, band, rinfo[tmp].index)) {
- sta->txrate_idx = rinfo[tmp].index;
+ spinfo->txrate_idx = rinfo[tmp].index;
break;
}
if (adj < 0)
@@ -121,9 +122,9 @@ static void rate_control_pid_adjust_rate
} while (tmp < n_bitrates && tmp >= 0);

#ifdef CONFIG_MAC80211_DEBUGFS
- rate_control_pid_event_rate_change(
- &((struct rc_pid_sta_info *)sta->rate_ctrl_priv)->events,
- sta->txrate_idx, sband->bitrates[sta->txrate_idx].bitrate);
+ rate_control_pid_event_rate_change(&spinfo->events,
+ spinfo->txrate_idx,
+ sband->bitrates[spinfo->txrate_idx].bitrate);
#endif
}

@@ -190,16 +191,16 @@ static void rate_control_pid_sample(stru
spinfo->tx_num_failed = 0;

/* If we just switched rate, update the rate behaviour info. */
- if (pinfo->oldrate != sta->txrate_idx) {
+ if (pinfo->oldrate != spinfo->txrate_idx) {

i = rinfo[pinfo->oldrate].rev_index;
- j = rinfo[sta->txrate_idx].rev_index;
+ j = rinfo[spinfo->txrate_idx].rev_index;

tmp = (pf - spinfo->last_pf);
tmp = RC_PID_DO_ARITH_RIGHT_SHIFT(tmp, RC_PID_ARITH_SHIFT);

rinfo[j].diff = rinfo[i].diff + tmp;
- pinfo->oldrate = sta->txrate_idx;
+ pinfo->oldrate = spinfo->txrate_idx;
}
rate_control_pid_normalize(pinfo, sband->n_bitrates);

@@ -252,19 +253,20 @@ static void rate_control_pid_tx_status(v
if (!sta)
goto unlock;

+ spinfo = sta->rate_ctrl_priv;
+
/* Don't update the state if we're not controlling the rate. */
sdata = sta->sdata;
if (sdata->force_unicast_rateidx > -1) {
- sta->txrate_idx = sdata->max_ratectrl_rateidx;
+ spinfo->txrate_idx = sdata->max_ratectrl_rateidx;
goto unlock;
}

/* Ignore all frames that were sent with a different rate than the rate
* we currently advise mac80211 to use. */
- if (info->tx_rate_idx != sta->txrate_idx)
+ if (info->tx_rate_idx != spinfo->txrate_idx)
goto unlock;

- spinfo = sta->rate_ctrl_priv;
spinfo->tx_num_xmit++;

#ifdef CONFIG_MAC80211_DEBUGFS
@@ -301,6 +303,7 @@ static void rate_control_pid_get_rate(vo
struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
struct ieee80211_sub_if_data *sdata;
+ struct rc_pid_sta_info *spinfo;
struct sta_info *sta;
int rateidx;
u16 fc;
@@ -321,10 +324,11 @@ static void rate_control_pid_get_rate(vo

/* If a forced rate is in effect, select it. */
sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ spinfo = (struct rc_pid_sta_info *)sta->rate_ctrl_priv;
if (sdata->force_unicast_rateidx > -1)
- sta->txrate_idx = sdata->force_unicast_rateidx;
+ spinfo->txrate_idx = sdata->force_unicast_rateidx;

- rateidx = sta->txrate_idx;
+ rateidx = spinfo->txrate_idx;

if (rateidx >= sband->n_bitrates)
rateidx = sband->n_bitrates - 1;
@@ -349,9 +353,10 @@ static void rate_control_pid_rate_init(v
* Until that method is implemented, we will use the lowest supported
* rate as a workaround. */
struct ieee80211_supported_band *sband;
+ struct rc_pid_sta_info *spinfo = (void *)sta->rate_ctrl_priv;

sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
- sta->txrate_idx = rate_lowest_index(local, sband, sta);
+ spinfo->txrate_idx = rate_lowest_index(local, sband, sta);
sta->fail_avg = 0;
}

--- everything.orig/drivers/net/wireless/ath9k/rc.c 2008-09-11 02:45:29.000000000 +0200
+++ everything/drivers/net/wireless/ath9k/rc.c 2008-09-11 02:45:46.000000000 +0200
@@ -2039,7 +2039,6 @@ static void ath_rate_init(void *priv, vo
DPRINTF(sc, ATH_DBG_RATE, "%s\n", __func__);

sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
- sta->txrate_idx = rate_lowest_index(local, sband, sta);

ath_setup_rates(local, sta);
if (conf->flags & IEEE80211_CONF_SUPPORT_HT_MODE) {
--- everything.orig/drivers/net/wireless/iwlwifi/iwl-3945-rs.c 2008-09-11 02:45:55.000000000 +0200
+++ everything/drivers/net/wireless/iwlwifi/iwl-3945-rs.c 2008-09-11 02:46:49.000000000 +0200
@@ -334,13 +334,11 @@ static void rs_rate_init(void *priv_rate

for (i = IWL_RATE_COUNT - 1; i >= 0; i--) {
if (sta->sta.supp_rates[local->hw.conf.channel->band] & (1 << i)) {
- sta->txrate_idx = i;
+ rs_sta->last_txrate_idx = i;
break;
}
}

- rs_sta->last_txrate_idx = sta->txrate_idx;
-
/* For 5 GHz band it start at IWL_FIRST_OFDM_RATE */
if (local->hw.conf.channel->band == IEEE80211_BAND_5GHZ)
rs_sta->last_txrate_idx += IWL_FIRST_OFDM_RATE;
@@ -809,15 +807,13 @@ static void rs_get_rate(void *priv_rate,

rs_sta->last_txrate_idx = index;
if (sband->band == IEEE80211_BAND_5GHZ)
- sta->txrate_idx = rs_sta->last_txrate_idx - IWL_FIRST_OFDM_RATE;
+ sel->rate_idx = rs_sta->last_txrate_idx - IWL_FIRST_OFDM_RATE;
else
- sta->txrate_idx = rs_sta->last_txrate_idx;
+ sel->rate_idx = rs_sta->last_txrate_idx;

rcu_read_unlock();

IWL_DEBUG_RATE("leave: %d\n", index);
-
- sel->rate_idx = sta->txrate_idx;
}

static struct rate_control_ops rs_ops = {
--- everything.orig/drivers/net/wireless/iwlwifi/iwl-agn-rs.c 2008-09-11 02:45:55.000000000 +0200
+++ everything/drivers/net/wireless/iwlwifi/iwl-agn-rs.c 2008-09-11 03:03:18.000000000 +0200
@@ -2064,14 +2064,6 @@ out:
i = index;
lq_sta->last_txrate_idx = i;

- /* sta->txrate_idx is an index to A mode rates which start
- * at IWL_FIRST_OFDM_RATE
- */
- if (lq_sta->band == IEEE80211_BAND_5GHZ)
- sta->txrate_idx = i - IWL_FIRST_OFDM_RATE;
- else
- sta->txrate_idx = i;
-
return;
}

@@ -2234,7 +2226,6 @@ static void rs_rate_init(void *priv_rate

lq_sta->flush_timer = 0;
lq_sta->supp_rates = sta->sta.supp_rates[sband->band];
- sta->txrate_idx = 3;
for (j = 0; j < LQ_SIZE; j++)
for (i = 0; i < IWL_RATE_COUNT; i++)
rs_rate_scale_clear_window(&lq_sta->lq_info[j].win[i]);
@@ -2269,11 +2260,11 @@ static void rs_rate_init(void *priv_rate
}

/* Find highest tx rate supported by hardware and destination station */
+ lq_sta->last_txrate_idx = 3;
for (i = 0; i < sband->n_bitrates; i++)
if (sta->sta.supp_rates[sband->band] & BIT(i))
- sta->txrate_idx = i;
+ lq_sta->last_txrate_idx = i;

- lq_sta->last_txrate_idx = sta->txrate_idx;
/* For MODE_IEEE80211A, skip over cck rates in global rate table */
if (local->hw.conf.channel->band == IEEE80211_BAND_5GHZ)
lq_sta->last_txrate_idx += IWL_FIRST_OFDM_RATE;
--- everything.orig/net/mac80211/tx.c 2008-09-11 02:56:05.000000000 +0200
+++ everything/net/mac80211/tx.c 2008-09-11 02:56:11.000000000 +0200
@@ -485,6 +485,8 @@ ieee80211_tx_h_rate_ctrl(struct ieee8021

if (likely(tx->rate_idx < 0)) {
rate_control_get_rate(tx->dev, sband, tx->skb, &rsel);
+ if (tx->sta)
+ tx->sta->last_txrate_idx = rsel.rate_idx;
tx->rate_idx = rsel.rate_idx;
if (unlikely(rsel.probe_idx >= 0)) {
info->flags |= IEEE80211_TX_CTL_RATE_CTRL_PROBE;



2008-09-11 00:45:49

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 21/18] mac80211: share sta->supp_rates

As more preparation for a saner rate control algorithm API,
share the supported rates bitmap in the public API.

Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/ath9k/rc.c | 2 +-
drivers/net/wireless/iwlwifi/iwl-3945-rs.c | 4 ++--
drivers/net/wireless/iwlwifi/iwl-agn-rs.c | 6 +++---
include/net/mac80211.h | 2 ++
net/mac80211/cfg.c | 2 +-
net/mac80211/mesh_plink.c | 4 ++--
net/mac80211/mlme.c | 8 ++++----
net/mac80211/rate.h | 2 +-
net/mac80211/sta_info.h | 2 --
9 files changed, 16 insertions(+), 16 deletions(-)

--- everything.orig/include/net/mac80211.h 2008-09-11 02:28:09.000000000 +0200
+++ everything/include/net/mac80211.h 2008-09-11 02:28:23.000000000 +0200
@@ -666,10 +666,12 @@ enum set_key_cmd {
*
* @addr: MAC address
* @aid: AID we assigned to the station if we're an AP
+ * @supp_rates: Bitmap of supported rates (per band)
* @drv_priv: data area for driver use, will always be aligned to
* sizeof(void *), size is determined in hw information.
*/
struct ieee80211_sta {
+ u64 supp_rates[IEEE80211_NUM_BANDS];
u8 addr[ETH_ALEN];
u16 aid;

--- everything.orig/net/mac80211/cfg.c 2008-09-11 02:30:18.000000000 +0200
+++ everything/net/mac80211/cfg.c 2008-09-11 02:30:24.000000000 +0200
@@ -662,7 +662,7 @@ static void sta_apply_parameters(struct
rates |= BIT(j);
}
}
- sta->supp_rates[local->oper_channel->band] = rates;
+ sta->sta.supp_rates[local->oper_channel->band] = rates;
}

if (params->ht_capa) {
--- everything.orig/net/mac80211/mlme.c 2008-09-11 02:29:22.000000000 +0200
+++ everything/net/mac80211/mlme.c 2008-09-11 02:29:58.000000000 +0200
@@ -1332,7 +1332,7 @@ static void ieee80211_rx_mgmt_assoc_resp
}
}

- sta->supp_rates[local->hw.conf.channel->band] = rates;
+ sta->sta.supp_rates[local->hw.conf.channel->band] = rates;
sdata->bss_conf.basic_rates = basic_rates;

/* cf. IEEE 802.11 9.2.12 */
@@ -1528,9 +1528,9 @@ static void ieee80211_rx_bss_info(struct
if (sta) {
u64 prev_rates;

- prev_rates = sta->supp_rates[band];
+ prev_rates = sta->sta.supp_rates[band];
/* make sure mandatory rates are always added */
- sta->supp_rates[band] = supp_rates |
+ sta->sta.supp_rates[band] = supp_rates |
ieee80211_mandatory_rates(local, band);

#ifdef CONFIG_MAC80211_IBSS_DEBUG
@@ -2369,7 +2369,7 @@ struct sta_info *ieee80211_ibss_add_sta(
set_sta_flags(sta, WLAN_STA_AUTHORIZED);

/* make sure mandatory rates are always added */
- sta->supp_rates[band] = supp_rates |
+ sta->sta.supp_rates[band] = supp_rates |
ieee80211_mandatory_rates(local, band);

rate_control_rate_init(sta, local);
--- everything.orig/net/mac80211/rate.h 2008-09-11 02:28:54.000000000 +0200
+++ everything/net/mac80211/rate.h 2008-09-11 02:28:59.000000000 +0200
@@ -134,7 +134,7 @@ static inline int rate_supported(struct
enum ieee80211_band band,
int index)
{
- return (sta == NULL || sta->supp_rates[band] & BIT(index));
+ return (sta == NULL || sta->sta.supp_rates[band] & BIT(index));
}

static inline s8
--- everything.orig/net/mac80211/sta_info.h 2008-09-11 02:27:55.000000000 +0200
+++ everything/net/mac80211/sta_info.h 2008-09-11 02:28:05.000000000 +0200
@@ -168,7 +168,6 @@ struct sta_ampdu_mlme {
* in the header file.
* @flaglock: spinlock for flags accesses
* @ht_info: HT capabilities of this STA
- * @supp_rates: Bitmap of supported rates (per band)
* @addr: MAC address of this STA
* @aid: STA's unique AID (1..2007, 0 = not assigned yet),
* only used in AP (and IBSS?) mode
@@ -228,7 +227,6 @@ struct sta_info {
spinlock_t lock;
spinlock_t flaglock;
struct ieee80211_ht_info ht_info;
- u64 supp_rates[IEEE80211_NUM_BANDS];

u16 listen_interval;

--- everything.orig/net/mac80211/mesh_plink.c 2008-09-11 02:31:19.000000000 +0200
+++ everything/net/mac80211/mesh_plink.c 2008-09-11 02:31:30.000000000 +0200
@@ -106,7 +106,7 @@ static struct sta_info *mesh_plink_alloc
return NULL;

sta->flags = WLAN_STA_AUTHORIZED;
- sta->supp_rates[local->hw.conf.channel->band] = rates;
+ sta->sta.supp_rates[local->hw.conf.channel->band] = rates;

return sta;
}
@@ -243,7 +243,7 @@ void mesh_neighbour_update(u8 *hw_addr,
}

sta->last_rx = jiffies;
- sta->supp_rates[local->hw.conf.channel->band] = rates;
+ sta->sta.supp_rates[local->hw.conf.channel->band] = rates;
if (peer_accepting_plinks && sta->plink_state == PLINK_LISTEN &&
sdata->u.mesh.accepting_plinks &&
sdata->u.mesh.mshcfg.auto_open_plinks)
--- everything.orig/drivers/net/wireless/ath9k/rc.c 2008-09-11 02:33:17.000000000 +0200
+++ everything/drivers/net/wireless/ath9k/rc.c 2008-09-11 02:33:22.000000000 +0200
@@ -1825,7 +1825,7 @@ static void ath_setup_rates(struct ieee8

sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
for (i = 0; i < sband->n_bitrates; i++) {
- if (sta->supp_rates[local->hw.conf.channel->band] & BIT(i)) {
+ if (sta->sta.supp_rates[local->hw.conf.channel->band] & BIT(i)) {
rc_priv->neg_rates.rs_rates[j]
= (sband->bitrates[i].bitrate * 2) / 10;
j++;
--- everything.orig/drivers/net/wireless/iwlwifi/iwl-3945-rs.c 2008-09-11 02:32:31.000000000 +0200
+++ everything/drivers/net/wireless/iwlwifi/iwl-3945-rs.c 2008-09-11 02:32:44.000000000 +0200
@@ -333,7 +333,7 @@ static void rs_rate_init(void *priv_rate
* after assoc.. */

for (i = IWL_RATE_COUNT - 1; i >= 0; i--) {
- if (sta->supp_rates[local->hw.conf.channel->band] & (1 << i)) {
+ if (sta->sta.supp_rates[local->hw.conf.channel->band] & (1 << i)) {
sta->txrate_idx = i;
break;
}
@@ -680,7 +680,7 @@ static void rs_get_rate(void *priv_rate,

rs_sta = (void *)sta->rate_ctrl_priv;

- rate_mask = sta->supp_rates[sband->band];
+ rate_mask = sta->sta.supp_rates[sband->band];
index = min(rs_sta->last_txrate_idx & 0xffff, IWL_RATE_COUNT - 1);

if (sband->band == IEEE80211_BAND_5GHZ)
--- everything.orig/drivers/net/wireless/iwlwifi/iwl-agn-rs.c 2008-09-11 02:32:31.000000000 +0200
+++ everything/drivers/net/wireless/iwlwifi/iwl-agn-rs.c 2008-09-11 02:33:05.000000000 +0200
@@ -1731,7 +1731,7 @@ static void rs_rate_scale_perform(struct
return;

lq_sta = (struct iwl_lq_sta *)sta->rate_ctrl_priv;
- lq_sta->supp_rates = sta->supp_rates[lq_sta->band];
+ lq_sta->supp_rates = sta->sta.supp_rates[lq_sta->band];

tid = rs_tl_add_packet(lq_sta, hdr);

@@ -2233,7 +2233,7 @@ static void rs_rate_init(void *priv_rate
sband = local->hw.wiphy->bands[local->hw.conf.channel->band];

lq_sta->flush_timer = 0;
- lq_sta->supp_rates = sta->supp_rates[sband->band];
+ lq_sta->supp_rates = sta->sta.supp_rates[sband->band];
sta->txrate_idx = 3;
for (j = 0; j < LQ_SIZE; j++)
for (i = 0; i < IWL_RATE_COUNT; i++)
@@ -2270,7 +2270,7 @@ static void rs_rate_init(void *priv_rate

/* Find highest tx rate supported by hardware and destination station */
for (i = 0; i < sband->n_bitrates; i++)
- if (sta->supp_rates[sband->band] & BIT(i))
+ if (sta->sta.supp_rates[sband->band] & BIT(i))
sta->txrate_idx = i;

lq_sta->last_txrate_idx = sta->txrate_idx;



2008-09-11 17:42:59

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 00/18] mac80211 cleanups and fixes

On Thu, 2008-09-11 at 19:39 +0200, Johannes Berg wrote:

> Well take the larger bit of code:
>
> struct something *an = NULL;
>
> ...
>
> rcu_read_lock();
> sta = ieee80211_find_sta(hw, hdr->addr2);
> if (sta)
> an = (void *) sta->drv_priv;
>
> if (an) {
> ath_rx_input(sc, an,
> hw->conf.ht_conf.ht_supported,
> skb, status, &st);
> }
> rcu_read_unlock();
>
> /* the "!an" here is fine even outside RCU lock */
> if (!an || (st != ATH_RX_CONSUMED))
> __ieee80211_rx(hw, skb, &rx_status);
>
>
> So at this point it's only checking whether above it had a pointer, it's
> not accessing it. Think of the "an" variable, after rcu_read_unlock(),
> as a bool indicating whether or not the code that just happened had
> access to the node or not.

That said, here it's probably smarter to just initialise "st" to
something other than _RX_CONSUMED and remove that !an condition
entirely.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-11 17:47:42

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 00/18] mac80211 cleanups and fixes

On Thu, 2008-09-11 at 19:42 +0200, Johannes Berg wrote:
> On Thu, 2008-09-11 at 19:39 +0200, Johannes Berg wrote:
>
> > Well take the larger bit of code:
> >
> > struct something *an = NULL;
> >
> > ...
> >
> > rcu_read_lock();
> > sta = ieee80211_find_sta(hw, hdr->addr2);
> > if (sta)
> > an = (void *) sta->drv_priv;
> >
> > if (an) {
> > ath_rx_input(sc, an,
> > hw->conf.ht_conf.ht_supported,
> > skb, status, &st);
> > }
> > rcu_read_unlock();
> >
> > /* the "!an" here is fine even outside RCU lock */
> > if (!an || (st != ATH_RX_CONSUMED))
> > __ieee80211_rx(hw, skb, &rx_status);
> >
> >
> > So at this point it's only checking whether above it had a pointer, it's
> > not accessing it. Think of the "an" variable, after rcu_read_unlock(),
> > as a bool indicating whether or not the code that just happened had
> > access to the node or not.
>
> That said, here it's probably smarter to just initialise "st" to
> something other than _RX_CONSUMED and remove that !an condition
> entirely.

And why is that not the return value of ath_rx_input? Anyway, I'm
getting off-topic here :)

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-12 03:16:57

by Sujith

[permalink] [raw]
Subject: Re: [PATCH 00/18] mac80211 cleanups and fixes

Johannes Berg wrote:
> I had actually tried
> (http://johannes.sipsolutions.net/patches/kernel/ath9k-sta-node.patch)
> but you're using refcounting for the nodes while mac80211 RCU-protects
> them, so I didn't get very far. It needs a bit more effort to make sure
> you don't have node pointers stick around in some tx descriptor after
> mac80211 decides to remove a node (which may very well happen while
> frames are queued)

Well, the node stuff is sprinkled throughout ath9k right now, I'll start
cleaning up all that and revamp the aggregation logic in the process too.
Do you mind if I build on this patch ?

Sujith

2008-09-11 17:13:58

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 00/18] mac80211 cleanups and fixes

On Thu, 2008-09-11 at 10:10 -0700, Luis R. Rodriguez wrote:

> > The driver already gets notified via sta_notify() that the sta is about
> > to be deleted, it must make sure that no matter what sort of references
> > it kept, the sta will not be accessed after that sta_notify call.
>
> Ah good point. Then what is the issue?

Well, mostly that ath9k right now assumes it controls the lifetime, and
it won't do that afterwards, so since I didn't want to read all of the
code I failed to change it that way because I didn't know where node
pointers are kept. It's not a huge issue really, but the lifetime
management change is somewhat involved, I think.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-11 16:53:58

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 00/18] mac80211 cleanups and fixes

On Thu, 2008-09-11 at 09:50 -0700, Luis R. Rodriguez wrote:

> > I had actually tried
> > (http://johannes.sipsolutions.net/patches/kernel/ath9k-sta-node.patch)
> > but you're using refcounting for the nodes while mac80211 RCU-protects
> > them, so I didn't get very far.
>
> Good thing I read RCU docs yesterday.

Heh.

> We should be able to remove ref counts and simply use RCU and
> sychronize_rcu() before deletion.

Well you don't get to control deletion then, that's another thing.

> > It needs a bit more effort to make sure
> > you don't have node pointers stick around in some tx descriptor after
> > mac80211 decides to remove a node (which may very well happen while
> > frames are queued)
>
> We can call_rcu() here no, and let the driver deal with what it needs to
> prior to deletion?

The driver already gets notified via sta_notify() that the sta is about
to be deleted, it must make sure that no matter what sort of references
it kept, the sta will not be accessed after that sta_notify call.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-11 17:54:51

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 00/18] mac80211 cleanups and fixes

On Thu, Sep 11, 2008 at 10:39:37AM -0700, Johannes Berg wrote:
> On Thu, 2008-09-11 at 10:33 -0700, Luis R. Rodriguez wrote:
> > On Thu, Sep 11, 2008 at 09:53:15AM -0700, Johannes Berg wrote:
> > > On Thu, 2008-09-11 at 09:50 -0700, Luis R. Rodriguez wrote:
> > > > > I had actually tried
> > > > > (http://johannes.sipsolutions.net/patches/kernel/ath9k-sta-node.patch)
> >
> > Just a comment so far from the patch.
> >
> > > + rcu_read_unlock();
> > > +
> > > + /* the "!an" here is fine even outside RCU lock */
> >
> > Why is that? I fail to see that.
>
> Well take the larger bit of code:
>
> struct something *an = NULL;
>
> ...
>
> rcu_read_lock();
> sta = ieee80211_find_sta(hw, hdr->addr2);
> if (sta)
> an = (void *) sta->drv_priv;
>
> if (an) {
> ath_rx_input(sc, an,
> hw->conf.ht_conf.ht_supported,
> skb, status, &st);
> }
> rcu_read_unlock();
>
> /* the "!an" here is fine even outside RCU lock */
> if (!an || (st != ATH_RX_CONSUMED))
> __ieee80211_rx(hw, skb, &rx_status);
>
>
> So at this point it's only checking whether above it had a pointer, it's
> not accessing it. Think of the "an" variable, after rcu_read_unlock(),
> as a bool indicating whether or not the code that just happened had
> access to the node or not.

I see now, thanks :)

Luis

2008-09-11 17:33:44

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 00/18] mac80211 cleanups and fixes

On Thu, Sep 11, 2008 at 09:53:15AM -0700, Johannes Berg wrote:
> On Thu, 2008-09-11 at 09:50 -0700, Luis R. Rodriguez wrote:
> > > I had actually tried
> > > (http://johannes.sipsolutions.net/patches/kernel/ath9k-sta-node.patch)

Just a comment so far from the patch.

> + rcu_read_unlock();
> +
> + /* the "!an" here is fine even outside RCU lock */

Why is that? I fail to see that.

Luis

2008-09-11 16:50:23

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 00/18] mac80211 cleanups and fixes

On Thu, Sep 11, 2008 at 06:22:31AM -0700, Johannes Berg wrote:
> On Thu, 2008-09-11 at 13:59 +0530, Sujith wrote:
>
> > > I'd been talking with Sujith about this one:
> > >
> > > * mac80211: share STA information with driver
> > >
> > > and decided to just do it; as it stands it's not very useful
> > > but I expect it to be used soon, if not we can remove the
> > > EXPORT_SYMBOL again.
> > >
> >
> > This would be very useful for ath9k. Making use of the
> > private area for each STA can enable removal of the node list
> > that is maintained currently.
>
> I had actually tried
> (http://johannes.sipsolutions.net/patches/kernel/ath9k-sta-node.patch)
> but you're using refcounting for the nodes while mac80211 RCU-protects
> them, so I didn't get very far.

Good thing I read RCU docs yesterday.

We should be able to remove ref counts and simply use RCU and
sychronize_rcu() before deletion.

> It needs a bit more effort to make sure
> you don't have node pointers stick around in some tx descriptor after
> mac80211 decides to remove a node (which may very well happen while
> frames are queued)

We can call_rcu() here no, and let the driver deal with what it needs to
prior to deletion?

Luis

2008-09-11 01:17:41

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 24/18] iwlwifi: don't access mac80211's AMPDU state machine

There really is no need, at worst ieee80211_start_tx_ba_session
will log a message when debugging is enabled, and poking such
internals of mac80211 definitely doesn't belong into an RC
algorithm.

Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-agn-rs.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)

--- everything.orig/drivers/net/wireless/iwlwifi/iwl-agn-rs.c 2008-09-11 03:12:48.000000000 +0200
+++ everything/drivers/net/wireless/iwlwifi/iwl-agn-rs.c 2008-09-11 03:12:58.000000000 +0200
@@ -359,15 +359,9 @@ static void rs_tl_turn_on_agg_for_tid(st
struct iwl_lq_sta *lq_data, u8 tid,
struct sta_info *sta)
{
- unsigned long state;
DECLARE_MAC_BUF(mac);

- spin_lock_bh(&sta->lock);
- state = sta->ampdu_mlme.tid_state_tx[tid];
- spin_unlock_bh(&sta->lock);
-
- if (state == HT_AGG_STATE_IDLE &&
- rs_tl_get_load(lq_data, tid) > IWL_AGG_LOAD_THRESHOLD) {
+ if (rs_tl_get_load(lq_data, tid) > IWL_AGG_LOAD_THRESHOLD) {
IWL_DEBUG_HT("Starting Tx agg: STA: %s tid: %d\n",
print_mac(mac, sta->sta.addr), tid);
ieee80211_start_tx_ba_session(priv->hw, sta->sta.addr, tid);



2008-09-11 00:04:05

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 19/18] mac80211: small rate control changes

This patch fixes mac80211 to not rely on the rate control
algorithm to update sta->tx_retry_failed and sta->tx_retry_count
(even if we don't currently use them), removes a number of
completely unused values we don't even show in debugfs and
changes the code in ieee80211_tx_status() to not look up the
sta_info repeatedly.

The only behaviour change here would be not calling the rate
control function rate_control_tx_status() when no sta_info is
found, but all rate control algorithms ignore such calls anyway.

Signed-off-by: Johannes Berg <[email protected]>
---
Now we can also change it to pass in the actual rate control information
to the algorithm, saving all the lookups there and the cross-dir header
inclusion! :)

net/mac80211/main.c | 50 ++++++++++++++++++++--------------------
net/mac80211/rc80211_pid_algo.c | 11 --------
net/mac80211/sta_info.h | 7 -----
3 files changed, 25 insertions(+), 43 deletions(-)

--- everything.orig/net/mac80211/sta_info.h 2008-09-11 01:51:51.000000000 +0200
+++ everything/net/mac80211/sta_info.h 2008-09-11 01:51:51.000000000 +0200
@@ -195,9 +195,6 @@ struct sta_ampdu_mlme {
* @tx_filtered_count: TBD
* @tx_retry_failed: TBD
* @tx_retry_count: TBD
- * @tx_num_consecutive_failures: TBD
- * @tx_num_mpdu_ok: TBD
- * @tx_num_mpdu_fail: TBD
* @fail_avg: moving percentage of failed MSDUs
* @tx_packets: number of RX/TX MSDUs
* @tx_bytes: TBD
@@ -273,10 +270,6 @@ struct sta_info {
/* Updated from TX status path only, no locking requirements */
unsigned long tx_filtered_count;
unsigned long tx_retry_failed, tx_retry_count;
- /* TODO: update in generic code not rate control? */
- u32 tx_num_consecutive_failures;
- u32 tx_num_mpdu_ok;
- u32 tx_num_mpdu_fail;
/* moving percentage of failed MSDUs */
unsigned int fail_avg;

--- everything.orig/net/mac80211/rc80211_pid_algo.c 2008-09-11 01:51:50.000000000 +0200
+++ everything/net/mac80211/rc80211_pid_algo.c 2008-09-11 01:51:51.000000000 +0200
@@ -282,17 +282,6 @@ static void rate_control_pid_tx_status(v
spinfo->tx_num_xmit++;
}

- if (info->status.excessive_retries) {
- sta->tx_retry_failed++;
- sta->tx_num_consecutive_failures++;
- sta->tx_num_mpdu_fail++;
- } else {
- sta->tx_num_consecutive_failures = 0;
- sta->tx_num_mpdu_ok++;
- }
- sta->tx_retry_count += info->status.retry_count;
- sta->tx_num_mpdu_fail += info->status.retry_count;
-
/* Update PID controller state. */
period = (HZ * pinfo->sampling_period + 500) / 1000;
if (!period)
--- everything.orig/net/mac80211/main.c 2008-09-11 01:51:51.000000000 +0200
+++ everything/net/mac80211/main.c 2008-09-11 01:51:51.000000000 +0200
@@ -546,29 +546,27 @@ void ieee80211_tx_status(struct ieee8021

rcu_read_lock();

- if (info->status.excessive_retries) {
- sta = sta_info_get(local, hdr->addr1);
- if (sta) {
- if (test_sta_flags(sta, WLAN_STA_PS)) {
- /*
- * The STA is in power save mode, so assume
- * that this TX packet failed because of that.
- */
- ieee80211_handle_filtered_frame(local, sta, skb);
- rcu_read_unlock();
- return;
- }
+ sta = sta_info_get(local, hdr->addr1);
+
+ if (sta) {
+ if (info->status.excessive_retries &&
+ test_sta_flags(sta, WLAN_STA_PS)) {
+ /*
+ * The STA is in power save mode, so assume
+ * that this TX packet failed because of that.
+ */
+ ieee80211_handle_filtered_frame(local, sta, skb);
+ rcu_read_unlock();
+ return;
}
- }

- fc = hdr->frame_control;
+ fc = hdr->frame_control;
+
+ if ((info->flags & IEEE80211_TX_STAT_AMPDU_NO_BACK) &&
+ (ieee80211_is_data_qos(fc))) {
+ u16 tid, ssn;
+ u8 *qc;

- if ((info->flags & IEEE80211_TX_STAT_AMPDU_NO_BACK) &&
- (ieee80211_is_data_qos(fc))) {
- u16 tid, ssn;
- u8 *qc;
- sta = sta_info_get(local, hdr->addr1);
- if (sta) {
qc = ieee80211_get_qos_ctl(hdr);
tid = qc[0] & 0xf;
ssn = ((le16_to_cpu(hdr->seq_ctrl) + 0x10)
@@ -576,17 +574,19 @@ void ieee80211_tx_status(struct ieee8021
ieee80211_send_bar(sta->sdata, hdr->addr1,
tid, ssn);
}
- }

- if (info->flags & IEEE80211_TX_STAT_TX_FILTERED) {
- sta = sta_info_get(local, hdr->addr1);
- if (sta) {
+ if (info->flags & IEEE80211_TX_STAT_TX_FILTERED) {
ieee80211_handle_filtered_frame(local, sta, skb);
rcu_read_unlock();
return;
+ } else {
+ if (info->status.excessive_retries)
+ sta->tx_retry_failed++;
+ sta->tx_retry_count += info->status.retry_count;
}
- } else
+
rate_control_tx_status(local->mdev, skb);
+ }

rcu_read_unlock();




2008-09-11 01:14:47

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 23/18] mac80211: share sta_info->ht_info

Rate control algorithms may need access to a station's
HT capabilities, so share the ht_info struct in the
public station API.

Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-agn-rs.c | 6 +++---
include/net/mac80211.h | 2 ++
net/mac80211/cfg.c | 2 +-
net/mac80211/mlme.c | 4 ++--
net/mac80211/sta_info.h | 2 --
5 files changed, 8 insertions(+), 8 deletions(-)

--- everything.orig/include/net/mac80211.h 2008-09-11 03:06:16.000000000 +0200
+++ everything/include/net/mac80211.h 2008-09-11 03:06:29.000000000 +0200
@@ -667,6 +667,7 @@ enum set_key_cmd {
* @addr: MAC address
* @aid: AID we assigned to the station if we're an AP
* @supp_rates: Bitmap of supported rates (per band)
+ * @ht_info: HT capabilities of this STA
* @drv_priv: data area for driver use, will always be aligned to
* sizeof(void *), size is determined in hw information.
*/
@@ -674,6 +675,7 @@ struct ieee80211_sta {
u64 supp_rates[IEEE80211_NUM_BANDS];
u8 addr[ETH_ALEN];
u16 aid;
+ struct ieee80211_ht_info ht_info;

/* must be last */
u8 drv_priv[0] __attribute__((__aligned__(sizeof(void *))));
--- everything.orig/net/mac80211/sta_info.h 2008-09-11 03:06:03.000000000 +0200
+++ everything/net/mac80211/sta_info.h 2008-09-11 03:06:12.000000000 +0200
@@ -167,7 +167,6 @@ struct sta_ampdu_mlme {
* @lock: used for locking all fields that require locking, see comments
* in the header file.
* @flaglock: spinlock for flags accesses
- * @ht_info: HT capabilities of this STA
* @addr: MAC address of this STA
* @aid: STA's unique AID (1..2007, 0 = not assigned yet),
* only used in AP (and IBSS?) mode
@@ -226,7 +225,6 @@ struct sta_info {
void *rate_ctrl_priv;
spinlock_t lock;
spinlock_t flaglock;
- struct ieee80211_ht_info ht_info;

u16 listen_interval;

--- everything.orig/net/mac80211/cfg.c 2008-09-11 03:06:52.000000000 +0200
+++ everything/net/mac80211/cfg.c 2008-09-11 03:06:59.000000000 +0200
@@ -667,7 +667,7 @@ static void sta_apply_parameters(struct

if (params->ht_capa) {
ieee80211_ht_cap_ie_to_ht_info(params->ht_capa,
- &sta->ht_info);
+ &sta->sta.ht_info);
}

if (ieee80211_vif_is_mesh(&sdata->vif) && params->plink_action) {
--- everything.orig/net/mac80211/mlme.c 2008-09-11 03:07:12.000000000 +0200
+++ everything/net/mac80211/mlme.c 2008-09-11 03:07:29.000000000 +0200
@@ -1347,11 +1347,11 @@ static void ieee80211_rx_mgmt_assoc_resp
struct ieee80211_ht_bss_info bss_info;
ieee80211_ht_cap_ie_to_ht_info(
(struct ieee80211_ht_cap *)
- elems.ht_cap_elem, &sta->ht_info);
+ elems.ht_cap_elem, &sta->sta.ht_info);
ieee80211_ht_addt_info_ie_to_ht_bss_info(
(struct ieee80211_ht_addt_info *)
elems.ht_info_elem, &bss_info);
- ieee80211_handle_ht(local, 1, &sta->ht_info, &bss_info);
+ ieee80211_handle_ht(local, 1, &sta->sta.ht_info, &bss_info);
}

rate_control_rate_init(sta, local);
--- everything.orig/drivers/net/wireless/iwlwifi/iwl-agn-rs.c 2008-09-11 03:07:56.000000000 +0200
+++ everything/drivers/net/wireless/iwlwifi/iwl-agn-rs.c 2008-09-11 03:08:12.000000000 +0200
@@ -1154,10 +1154,10 @@ static int rs_switch_to_mimo2(struct iwl
s8 is_green = lq_sta->is_green;

if (!(conf->flags & IEEE80211_CONF_SUPPORT_HT_MODE) ||
- !sta->ht_info.ht_supported)
+ !sta->sta.ht_info.ht_supported)
return -1;

- if (((sta->ht_info.cap & IEEE80211_HT_CAP_SM_PS) >> 2)
+ if (((sta->sta.ht_info.cap & IEEE80211_HT_CAP_SM_PS) >> 2)
== WLAN_HT_CAP_SM_PS_STATIC)
return -1;

@@ -1222,7 +1222,7 @@ static int rs_switch_to_siso(struct iwl_
s32 rate;

if (!(conf->flags & IEEE80211_CONF_SUPPORT_HT_MODE) ||
- !sta->ht_info.ht_supported)
+ !sta->sta.ht_info.ht_supported)
return -1;

IWL_DEBUG_RATE("LQ: try to switch to SISO\n");



2008-09-11 17:32:02

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 00/18] mac80211 cleanups and fixes

On Thu, Sep 11, 2008 at 10:13:19AM -0700, Johannes Berg wrote:
> On Thu, 2008-09-11 at 10:10 -0700, Luis R. Rodriguez wrote:
>
> > > The driver already gets notified via sta_notify() that the sta is about
> > > to be deleted, it must make sure that no matter what sort of references
> > > it kept, the sta will not be accessed after that sta_notify call.
> >
> > Ah good point. Then what is the issue?
>
> Well, mostly that ath9k right now assumes it controls the lifetime, and
> it won't do that afterwards, so since I didn't want to read all of the
> code I failed to change it that way because I didn't know where node
> pointers are kept. It's not a huge issue really, but the lifetime
> management change is somewhat involved, I think.

Sure, it just needs a review.

Luis


2008-09-11 13:23:13

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 00/18] mac80211 cleanups and fixes

On Thu, 2008-09-11 at 13:59 +0530, Sujith wrote:

> > I'd been talking with Sujith about this one:
> >
> > * mac80211: share STA information with driver
> >
> > and decided to just do it; as it stands it's not very useful
> > but I expect it to be used soon, if not we can remove the
> > EXPORT_SYMBOL again.
> >
>
> This would be very useful for ath9k. Making use of the
> private area for each STA can enable removal of the node list
> that is maintained currently.

I had actually tried
(http://johannes.sipsolutions.net/patches/kernel/ath9k-sta-node.patch)
but you're using refcounting for the nodes while mac80211 RCU-protects
them, so I didn't get very far. It needs a bit more effort to make sure
you don't have node pointers stick around in some tx descriptor after
mac80211 decides to remove a node (which may very well happen while
frames are queued)

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part