2007-12-20 09:48:32

by Ron Rindjunsky

[permalink] [raw]
Subject: [PATCH 0/8 v2] mac80211/iwlwifi: integrate IEEE802.11n A-MPDU Rx MLME

This series of patches contains implementation for the IEEE802.11n
aggregated MPDUs (A-MPDU) MLME. The series handles Rx A-MPDU (recipient party).

Please notice:
===============
Patch "mac80211: A-MPDU Rx adding BAR handling capability" depends on patch
"mac80211: pass in PS_POLL frames" that was accepted as a stand alone patch.

Notes to this series:
======================
This series of patches splits into:
- patches 1-7 add 802.11n Rx A-MPDU MLME support to mac80211.
- patch 8 demonstrates the use of the above mac80211's MLME through iwl4965 low level driver

The patches in the series should not be treated as stand alone patches,
but as a complete MLME according to IEEE802.11n spec, although separation
to patches by subjects was made for ease of handling.

The patches do _not_ break any existing driver.
Patches were made (and tested to work) with wireless branch 2.6.24-rc3 (#everything)

The MLME framework includes the following:
- A-MPDU MLME (for aggregated Rx per STA/TID)
- addBA request support.
- delBA request support (recipient party).
- Block Ack Request (BAR) support.
- A-MPDU reordering.


---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.



2007-12-20 09:42:08

by Ron Rindjunsky

[permalink] [raw]
Subject: [PATCH 8/8 v2] iwlwifi: A-MPDU Rx flow enabled

This patch enables the A-MPDU Rx flow. it contains several
adjustments to new mac80211 A-MPDU Rx flow.

Signed-off-by: Ron Rindjunsky <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-4965.c | 66 +++++++++++++--------------
drivers/net/wireless/iwlwifi/iwl-4965.h | 7 +--
drivers/net/wireless/iwlwifi/iwl4965-base.c | 3 +-
3 files changed, 36 insertions(+), 40 deletions(-)

Index: wl2_6_24_rc5/drivers/net/wireless/iwlwifi/iwl-4965.c
===================================================================
--- wl2_6_24_rc5.orig/drivers/net/wireless/iwlwifi/iwl-4965.c
+++ wl2_6_24_rc5/drivers/net/wireless/iwlwifi/iwl-4965.c
@@ -3868,9 +3868,7 @@ static void iwl4965_rx_reply_rx(struct i
.antenna = 0,
.rate = iwl4965_hw_get_rate(rx_start->rate_n_flags),
.flag = 0,
-#ifdef CONFIG_IWL4965_HT_AGG
- .ordered = 0
-#endif /* CONFIG_IWL4965_HT_AGG */
+ .ordered = TXRX_CONTINUE
};
u8 network_packet;

@@ -4065,7 +4063,7 @@ static void iwl4965_rx_reply_rx(struct i
break;

case IEEE80211_FTYPE_CTL:
-#ifdef CONFIG_IWL4965_HT_AGG
+#ifdef CONFIG_IWL4965_HT
switch (fc & IEEE80211_FCTL_STYPE) {
case IEEE80211_STYPE_BACK_REQ:
IWL_DEBUG_HT("IEEE80211_STYPE_BACK_REQ arrived\n");
@@ -4076,7 +4074,6 @@ static void iwl4965_rx_reply_rx(struct i
break;
}
#endif
-
break;

case IEEE80211_FTYPE_DATA: {
@@ -4663,8 +4660,6 @@ void iwl4965_set_ht_add_station(struct i
return;
}

-#ifdef CONFIG_IWL4965_HT_AGG
-
static void iwl4965_sta_modify_add_ba_tid(struct iwl4965_priv *priv,
int sta_id, int tid, u16 ssn)
{
@@ -4696,6 +4691,36 @@ static void iwl4965_sta_modify_del_ba_ti
iwl4965_send_add_station(priv, &priv->stations[sta_id].sta, CMD_ASYNC);
}

+int iwl4965_mac_ampdu_action(struct ieee80211_hw *hw,
+ enum ieee80211_ampdu_mlme_action action,
+ const u8 *addr, u16 tid, u16 ssn)
+{
+ struct iwl4965_priv *priv = hw->priv;
+ int sta_id;
+ DECLARE_MAC_BUF(mac);
+
+ IWL_DEBUG_HT("A-MPDU action on da=%s tid=%d ",
+ print_mac(mac, addr), tid);
+ sta_id = iwl4965_hw_find_station(priv, addr);
+ switch (action) {
+ case IEEE80211_AMPDU_RX_START:
+ IWL_DEBUG_HT("start Rx\n");
+ iwl4965_sta_modify_add_ba_tid(priv, sta_id, tid, ssn);
+ break;
+ case IEEE80211_AMPDU_RX_STOP:
+ IWL_DEBUG_HT("stop Rx\n");
+ iwl4965_sta_modify_del_ba_tid(priv, sta_id, tid);
+ break;
+ default:
+ IWL_DEBUG_HT("unknown\n");
+ return -EINVAL;
+ break;
+ }
+ return 0;
+}
+
+#ifdef CONFIG_IWL4965_HT_AGG
+
static const u16 default_tid_to_tx_fifo[] = {
IWL_TX_FIFO_AC1,
IWL_TX_FIFO_AC0,
@@ -4825,33 +4850,6 @@ int iwl4965_mac_ht_tx_agg_stop(struct ie
return 0;
}

-int iwl4965_mac_ht_rx_agg_start(struct ieee80211_hw *hw, u8 *da,
- u16 tid, u16 start_seq_num)
-{
- struct iwl4965_priv *priv = hw->priv;
- int sta_id;
- DECLARE_MAC_BUF(mac);
-
- IWL_WARNING("iwl-AGG iwl4965_mac_ht_rx_agg_start on da=%s"
- " tid=%d\n", print_mac(mac, da), tid);
- sta_id = iwl4965_hw_find_station(priv, da);
- iwl4965_sta_modify_add_ba_tid(priv, sta_id, tid, start_seq_num);
- return 0;
-}
-
-int iwl4965_mac_ht_rx_agg_stop(struct ieee80211_hw *hw, u8 *da,
- u16 tid, int generator)
-{
- struct iwl4965_priv *priv = hw->priv;
- int sta_id;
- DECLARE_MAC_BUF(mac);
-
- IWL_WARNING("iwl-AGG iwl4965_mac_ht_rx_agg_stop on da=%s tid=%d\n",
- print_mac(mac, da), tid);
- sta_id = iwl4965_hw_find_station(priv, da);
- iwl4965_sta_modify_del_ba_tid(priv, sta_id, tid);
- return 0;
-}

#endif /* CONFIG_IWL4965_HT_AGG */
#endif /* CONFIG_IWL4965_HT */
Index: wl2_6_24_rc5/drivers/net/wireless/iwlwifi/iwl-4965.h
===================================================================
--- wl2_6_24_rc5.orig/drivers/net/wireless/iwlwifi/iwl-4965.h
+++ wl2_6_24_rc5/drivers/net/wireless/iwlwifi/iwl-4965.h
@@ -802,13 +802,12 @@ extern void iwl4965_set_rxon_ht(struct i
struct iwl_ht_info *ht_info);
extern void iwl4965_set_ht_add_station(struct iwl4965_priv *priv, u8 index,
struct ieee80211_ht_info *sta_ht_inf);
+extern int iwl4965_mac_ampdu_action(struct ieee80211_hw *hw,
+ enum ieee80211_ampdu_mlme_action action,
+ const u8 *addr, u16 tid, u16 ssn);
#ifdef CONFIG_IWL4965_HT_AGG
extern int iwl4965_mac_ht_tx_agg_start(struct ieee80211_hw *hw, u8 *da,
u16 tid, u16 *start_seq_num);
-extern int iwl4965_mac_ht_rx_agg_start(struct ieee80211_hw *hw, u8 *da,
- u16 tid, u16 start_seq_num);
-extern int iwl4965_mac_ht_rx_agg_stop(struct ieee80211_hw *hw, u8 *da,
- u16 tid, int generator);
extern int iwl4965_mac_ht_tx_agg_stop(struct ieee80211_hw *hw, u8 *da,
u16 tid, int generator);
extern void iwl4965_turn_off_agg(struct iwl4965_priv *priv, u8 tid);
Index: wl2_6_24_rc5/drivers/net/wireless/iwlwifi/iwl4965-base.c
===================================================================
--- wl2_6_24_rc5.orig/drivers/net/wireless/iwlwifi/iwl4965-base.c
+++ wl2_6_24_rc5/drivers/net/wireless/iwlwifi/iwl4965-base.c
@@ -9054,11 +9054,10 @@ static struct ieee80211_ops iwl4965_hw_o
.erp_ie_changed = iwl4965_mac_erp_ie_changed,
#ifdef CONFIG_IWL4965_HT
.conf_ht = iwl4965_mac_conf_ht,
+ .ampdu_action = iwl4965_mac_ampdu_action,
#ifdef CONFIG_IWL4965_HT_AGG
.ht_tx_agg_start = iwl4965_mac_ht_tx_agg_start,
.ht_tx_agg_stop = iwl4965_mac_ht_tx_agg_stop,
- .ht_rx_agg_start = iwl4965_mac_ht_rx_agg_start,
- .ht_rx_agg_stop = iwl4965_mac_ht_rx_agg_stop,
#endif /* CONFIG_IWL4965_HT_AGG */
#endif /* CONFIG_IWL4965_HT */
.hw_scan = iwl4965_mac_hw_scan
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


2007-12-20 09:48:33

by Ron Rindjunsky

[permalink] [raw]
Subject: [PATCH 2/8 v2] mac80211: A-MPDU Rx add MLME structures

This patch adds the needed structures to describe the Rx aggregation MLME per STA
new:
- struct tid_ampdu_rx: TID aggregation information (Rx)
- struct sta_ampdu_mlme: MLME aggregation information for STA
changed:
- struct sta_info: ampdu_mlme added to describe A-MPDU MLME per STA,
and timer_to_tid added to map timer id into TID

Signed-off-by: Ron Rindjunsky <[email protected]>
---
net/mac80211/sta_info.h | 47 +++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index e1a4ac1..96fe3ed 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -31,6 +31,51 @@
#define WLAN_STA_WME BIT(9)
#define WLAN_STA_WDS BIT(27)

+#define STA_TID_NUM 16
+#define ADDBA_RESP_INTERVAL HZ
+
+#define HT_AGG_STATE_INITIATOR_SHIFT (4)
+
+#define HT_AGG_STATE_REQ_STOP_BA_MSK BIT(3)
+
+#define HT_AGG_STATE_IDLE (0x0)
+#define HT_AGG_STATE_OPERATIONAL (0x7)
+
+/**
+ * struct tid_ampdu_rx - TID aggregation information (Rx).
+ *
+ * @state: TID's state in session state machine.
+ * @dialog_token: dialog token for aggregation session
+ * @ssn: Starting Sequence Number expected to be aggregated.
+ * @buf_size: buffer size for incoming A-MPDUs
+ * @timeout: reset timer value.
+ * @head_seq_num: head sequence number in reordering buffer.
+ * @stored_mpdu_num: number of MPDUs in reordering buffer
+ * @reorder_buf: buffer to reorder incoming aggregated MPDUs
+ * @session_timer: check if peer keeps Tx-ing on the TID (by timeout value)
+ */
+struct tid_ampdu_rx {
+ u8 state;
+ u8 dialog_token;
+ u16 ssn;
+ u16 buf_size;
+ u16 timeout;
+ u16 head_seq_num;
+ u16 stored_mpdu_num;
+ struct sk_buff **reorder_buf;
+ struct timer_list session_timer;
+};
+
+/**
+ * struct sta_ampdu_mlme - STA aggregation information.
+ *
+ * @tid_agg_info_rx: aggregation info for Rx per TID
+ * @ampdu_rx: for locking sections in aggregation Rx flow
+ */
+struct sta_ampdu_mlme {
+ struct tid_ampdu_rx tid_rx[STA_TID_NUM];
+ spinlock_t ampdu_rx;
+};

struct sta_info {
struct kref kref;
@@ -101,6 +146,8 @@ struct sta_info {

struct ieee80211_ht_info ht_info; /* 802.11n HT capabilities
of this STA */
+ struct sta_ampdu_mlme ampdu_mlme;
+ u8 timer_to_tid[STA_TID_NUM]; /* convert timer id to tid */

#ifdef CONFIG_MAC80211_DEBUGFS
struct sta_info_debugfsdentries {
--
1.5.3.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


2007-12-20 09:48:33

by Ron Rindjunsky

[permalink] [raw]
Subject: [PATCH 4/8 v2] mac80211: A-MPDU Rx MLME data initialization

This patch initialize A-MPDU MLME data for Rx sessions.

Signed-off-by: Ron Rindjunsky <[email protected]>
---
net/mac80211/sta_info.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index cc28e82..c03a714 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -135,6 +135,7 @@ struct sta_info * sta_info_add(struct ieee80211_local *local,
struct net_device *dev, u8 *addr, gfp_t gfp)
{
struct sta_info *sta;
+ int i;
DECLARE_MAC_BUF(mac);

sta = kzalloc(sizeof(*sta), gfp);
@@ -154,6 +155,19 @@ struct sta_info * sta_info_add(struct ieee80211_local *local,
memcpy(sta->addr, addr, ETH_ALEN);
sta->local = local;
sta->dev = dev;
+ spin_lock_init(&sta->ampdu_mlme.ampdu_rx);
+ for (i = 0; i < STA_TID_NUM; i++) {
+ /* timer_to_tid must be initialized with identity mapping to
+ * enable session_timer's data differentiation. refer to
+ * sta_rx_agg_session_timer_expired for useage */
+ sta->timer_to_tid[i] = i;
+ /* rx timers */
+ sta->ampdu_mlme.tid_rx[i].session_timer.function =
+ sta_rx_agg_session_timer_expired;
+ sta->ampdu_mlme.tid_rx[i].session_timer.data =
+ (unsigned long)&sta->timer_to_tid[i];
+ init_timer(&sta->ampdu_mlme.tid_rx[i].session_timer);
+ }
skb_queue_head_init(&sta->ps_tx_buf);
skb_queue_head_init(&sta->tx_filtered);
__sta_info_get(sta); /* sta used by caller, decremented by
--
1.5.3.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


2007-12-24 08:53:35

by Ron Rindjunsky

[permalink] [raw]
Subject: Re: [PATCH 3/8 v2] mac80211: A-MPDU Rx adding basic functionality

> A few minor comments, looks good otherwise.
>
> > status = WLAN_STATUS_REQUEST_DECLINED;
> >
> > +
>
> spurious newline added, I'd think.

will do

>
> > + /* prepare reordering buffer */
> > + tid_agg_rx->reorder_buf =
> > + kmalloc(buf_size * sizeof(struct sk_buf *), GFP_ATOMIC);
> > + if (!tid_agg_rx->reorder_buf) {
> > + printk(KERN_ERR "can not allocate reordering buffer "
> > + "to tid %d\n", tid);
>
> Should that be ratelimited just in case somebody is trying over and over
> again while we're under memory pressure?
>

done

> > + skb = dev_alloc_skb(sizeof(*mgmt) + local->hw.extra_tx_headroom + 1 +
> > + sizeof(mgmt->u.action.u.delba));
>
> What's "+ 1"?

that's for the category byte. in fact i think that the problem is not
here, but in a wrong allocation size in send_addba_resp that i will
fix.

>
> > + printk(KERN_DEBUG "rx BA session requested to stop on "
> > + "unactive tid %d\n", tid);
>
> typo: "inactive"

thanks

>
> > + sta = sta_info_get(local, temp_sta->addr);
> > +
> > + if (!sta)
> > + return;
>
> Still have that?
>

i have cleaned it now, thatnks.

> Thanks,
> johannes
>
>

2007-12-20 10:43:22

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 0/8 v2] mac80211/iwlwifi: integrate IEEE802.11n A-MPDU Rx MLME


On Thu, 2007-12-20 at 11:39 +0200, Ron Rindjunsky wrote:
> This series of patches contains implementation for the IEEE802.11n
> aggregated MPDUs (A-MPDU) MLME. The series handles Rx A-MPDU (recipient party).

Thanks. I'll re-review once you've commented on my last mail from this
morning.

johannes


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

2007-12-20 09:42:08

by Ron Rindjunsky

[permalink] [raw]
Subject: [PATCH 6/8 v2] mac80211: A-MPDU Rx adding BAR handling capability

This patch adds the ability to handle Block Ack Request

Signed-off-by: Ron Rindjunsky <[email protected]>
---
net/mac80211/ieee80211_i.h | 3 +-
net/mac80211/rx.c | 53 ++++++++++++++++++++++++++++++++++++++++---
net/mac80211/util.c | 15 +++++++++++-
3 files changed, 65 insertions(+), 6 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 1034b71..9215c22 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -799,7 +799,8 @@ int ieee80211_subif_start_xmit(struct sk_buff *skb, struct net_device *dev);
extern void *mac80211_wiphy_privid; /* for wiphy privid */
extern const unsigned char rfc1042_header[6];
extern const unsigned char bridge_tunnel_header[6];
-u8 *ieee80211_get_bssid(struct ieee80211_hdr *hdr, size_t len);
+u8 *ieee80211_get_bssid(struct ieee80211_hdr *hdr, size_t len,
+ enum ieee80211_if_types type);
int ieee80211_is_eapol(const struct sk_buff *skb, int hdrlen);
int ieee80211_frame_duration(struct ieee80211_local *local, size_t len,
int rate, int erp, int short_preamble);
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 0d29e7b..c9811c4 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -64,7 +64,9 @@ static inline int should_drop_frame(struct ieee80211_rx_status *status,
if (((hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_FTYPE)) ==
cpu_to_le16(IEEE80211_FTYPE_CTL)) &&
((hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_STYPE)) !=
- cpu_to_le16(IEEE80211_STYPE_PSPOLL)))
+ cpu_to_le16(IEEE80211_STYPE_PSPOLL)) &&
+ ((hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_STYPE)) !=
+ cpu_to_le16(IEEE80211_STYPE_BACK_REQ)))
return 1;
return 0;
}
@@ -800,7 +802,8 @@ ieee80211_rx_h_sta_process(struct ieee80211_txrx_data *rx)
* BSSID to avoid keeping the current IBSS network alive in cases where
* other STAs are using different BSSID. */
if (rx->sdata->type == IEEE80211_IF_TYPE_IBSS) {
- u8 *bssid = ieee80211_get_bssid(hdr, rx->skb->len);
+ u8 *bssid = ieee80211_get_bssid(hdr, rx->skb->len,
+ IEEE80211_IF_TYPE_IBSS);
if (compare_ether_addr(bssid, rx->sdata->u.sta.bssid) == 0)
sta->last_rx = jiffies;
} else
@@ -1528,6 +1531,48 @@ ieee80211_rx_h_data(struct ieee80211_txrx_data *rx)
}

static ieee80211_txrx_result
+ieee80211_rx_h_ctrl(struct ieee80211_txrx_data *rx)
+{
+ struct ieee80211_local *local = rx->local;
+ struct ieee80211_hw *hw = &local->hw;
+ struct sk_buff *skb = rx->skb;
+ struct ieee80211_bar *bar = (struct ieee80211_bar *) skb->data;
+ struct tid_ampdu_rx *tid_agg_rx;
+ u16 start_seq_num;
+ u16 tid;
+
+ if (likely((rx->fc & IEEE80211_FCTL_FTYPE) != IEEE80211_FTYPE_CTL))
+ return TXRX_CONTINUE;
+
+ if ((rx->fc & IEEE80211_FCTL_STYPE) == IEEE80211_STYPE_BACK_REQ) {
+ if (!rx->sta)
+ return TXRX_CONTINUE;
+ tid = le16_to_cpu(bar->control) >> 12;
+ tid_agg_rx = &(rx->sta->ampdu_mlme.tid_rx[tid]);
+ if (tid_agg_rx->state != HT_AGG_STATE_OPERATIONAL)
+ return TXRX_CONTINUE;
+
+ start_seq_num = le16_to_cpu(bar->start_seq_num) >> 4;
+
+ /* reset session timer */
+ if (tid_agg_rx->timeout) {
+ unsigned long expires =
+ jiffies + (tid_agg_rx->timeout / 1000) * HZ;
+ mod_timer(&tid_agg_rx->session_timer, expires);
+ }
+
+ /* manage reordering buffer according to requested */
+ /* sequence number */
+ ieee80211_sta_manage_reorder_buf(hw, rx, tid_agg_rx,
+ NULL, start_seq_num, 1);
+
+ return TXRX_DROP;
+ }
+
+ return TXRX_CONTINUE;
+}
+
+static ieee80211_txrx_result
ieee80211_rx_h_mgmt(struct ieee80211_txrx_data *rx)
{
struct ieee80211_sub_if_data *sdata;
@@ -1678,6 +1723,7 @@ ieee80211_rx_handler ieee80211_rx_handlers[] =
ieee80211_rx_h_remove_qos_control,
ieee80211_rx_h_amsdu,
ieee80211_rx_h_data,
+ ieee80211_rx_h_ctrl,
ieee80211_rx_h_mgmt,
NULL
};
@@ -1839,8 +1885,6 @@ void __ieee80211_rx(struct ieee80211_hw *hw, struct sk_buff *skb,
return;
}

- bssid = ieee80211_get_bssid(hdr, skb->len);
-
list_for_each_entry_rcu(sdata, &local->interfaces, list) {
if (!netif_running(sdata->dev))
continue;
@@ -1848,6 +1892,7 @@ void __ieee80211_rx(struct ieee80211_hw *hw, struct sk_buff *skb,
if (sdata->type == IEEE80211_IF_TYPE_MNTR)
continue;

+ bssid = ieee80211_get_bssid(hdr, skb->len, sdata->type);
rx.flags |= IEEE80211_TXRXD_RXRA_MATCH;
prepres = prepare_for_handlers(sdata, bssid, &rx, hdr);
/* prepare_for_handlers can change sta */
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index fb7fd89..d567b83 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -131,7 +131,8 @@ void ieee80211_prepare_rates(struct ieee80211_local *local,
}
}

-u8 *ieee80211_get_bssid(struct ieee80211_hdr *hdr, size_t len)
+u8 *ieee80211_get_bssid(struct ieee80211_hdr *hdr, size_t len,
+ enum ieee80211_if_types type)
{
u16 fc;

@@ -163,6 +164,18 @@ u8 *ieee80211_get_bssid(struct ieee80211_hdr *hdr, size_t len)
case IEEE80211_FTYPE_CTL:
if ((fc & IEEE80211_FCTL_STYPE) == IEEE80211_STYPE_PSPOLL)
return hdr->addr1;
+ else if ((fc & IEEE80211_FCTL_STYPE) ==
+ IEEE80211_STYPE_BACK_REQ) {
+ switch (type) {
+ case IEEE80211_IF_TYPE_STA:
+ return hdr->addr2;
+ case IEEE80211_IF_TYPE_AP:
+ case IEEE80211_IF_TYPE_VLAN:
+ return hdr->addr1;
+ default:
+ return NULL;
+ }
+ }
else
return NULL;
}
--
1.5.3.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


2007-12-25 12:51:20

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 5/8 v2] mac80211: A-MPDU Rx handling aggregation reordering


> what about list_for_each_entry sta inside ieee80211_stop to stop the
> session? i tried it and it worked really well.

Sounds about right, you just need to make sure that you have the right
locks and only tear down sessions for the virtual interface that's being
stopped, not all if you have sessions on different virtual interfaces
(say on a multi-BSS AP when one BSS is stopped)

johannes


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

2007-12-20 09:48:32

by Ron Rindjunsky

[permalink] [raw]
Subject: [PATCH 5/8 v2] mac80211: A-MPDU Rx handling aggregation reordering

This patch handles the reordering of the Rx A-MPDU.
This issue occurs when the sequence of the internal MPDUs is not in the
right order. such a case can be encountered for example when some MPDUs from
previous aggregations were recieved, while others failed, so current A-MPDU
will contain a mix of re-transmited MPDUs and new ones.

Signed-off-by: Ron Rindjunsky <[email protected]>
---
include/net/mac80211.h | 19 +++++
net/mac80211/ieee80211_i.h | 5 -
net/mac80211/ieee80211_sta.c | 17 ++++-
net/mac80211/rx.c | 169 +++++++++++++++++++++++++++++++++++++++++-
4 files changed, 203 insertions(+), 7 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 418cd38..246c012 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -361,6 +361,23 @@ enum mac80211_rx_flags {
RX_FLAG_FAILED_PLCP_CRC = 1<<6,
};

+/*
+ * enum ieee80211_txrx_result - frame's status in Tx/Rx flow
+ *
+ * This enum gives an indication about the frame's current
+ * status in the flow. this enum is relevant both to Tx and Rx
+ * flows.
+ * @TXRX_CONTINUE: frame may proceed to next stage of the flow.
+ * @TXRX_DROP: frame should be dropped.
+ * @TXRX_QUEUED: frame is queued at this point, but may be
+ * put back in the flow in the future.
+ */
+typedef enum {
+ TXRX_CONTINUE,
+ TXRX_DROP,
+ TXRX_QUEUED
+} ieee80211_txrx_result;
+
/**
* struct ieee80211_rx_status - receive status
*
@@ -376,6 +393,7 @@ enum mac80211_rx_flags {
* @noise: PHY noise when receiving this frame
* @antenna: antenna used
* @rate: data rate
+ * @ordered: used in A-MPSDU reordering scheme
* @flag: %RX_FLAG_*
*/
struct ieee80211_rx_status {
@@ -388,6 +406,7 @@ struct ieee80211_rx_status {
int noise;
int antenna;
int rate;
+ ieee80211_txrx_result ordered;
int flag;
};

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index cf2256b..1034b71 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -108,11 +108,6 @@ struct ieee80211_sta_bss {
u8 erp_value;
};

-
-typedef enum {
- TXRX_CONTINUE, TXRX_DROP, TXRX_QUEUED
-} ieee80211_txrx_result;
-
/* flags used in struct ieee80211_txrx_data.flags */
/* whether the MSDU was fragmented */
#define IEEE80211_TXRXD_FRAGMENTED BIT(0)
diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c
index f696dbc..00ffa81 100644
--- a/net/mac80211/ieee80211_sta.c
+++ b/net/mac80211/ieee80211_sta.c
@@ -1212,7 +1212,8 @@ void ieee80211_sta_stop_rx_BA_session(struct net_device *dev, u8 *ra, u16 tid,
struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
struct ieee80211_hw *hw = &local->hw;
struct sta_info *sta;
- int ret;
+ struct ieee80211_rx_status status;
+ int ret, i;

sta = sta_info_get(local, ra);
if (!sta)
@@ -1254,6 +1255,20 @@ void ieee80211_sta_stop_rx_BA_session(struct net_device *dev, u8 *ra, u16 tid,
ieee80211_send_delba(dev, ra, tid, 0, reason);

/* free the reordering buffer */
+ for (i = 0; i < sta->ampdu_mlme.tid_rx[tid].buf_size; i++) {
+ if (sta->ampdu_mlme.tid_rx[tid].reorder_buf[i]) {
+ /* release the reordered frames to stack,
+ * but drop them there */
+ memcpy(&status, sta->ampdu_mlme.tid_rx[tid].
+ reorder_buf[i]->cb, sizeof(status));
+ status.ordered = TXRX_DROP;
+ __ieee80211_rx(hw, sta->ampdu_mlme.
+ tid_rx[tid].reorder_buf[i],
+ &status);
+ sta->ampdu_mlme.tid_rx[tid].stored_mpdu_num--;
+ sta->ampdu_mlme.tid_rx[tid].reorder_buf[i] = NULL;
+ }
+ }
kfree(sta->ampdu_mlme.tid_rx[tid].reorder_buf);

sta->ampdu_mlme.tid_rx[tid].state = HT_AGG_STATE_IDLE;
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index e94cf30..0d29e7b 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -226,6 +226,114 @@ ieee80211_rx_monitor(struct ieee80211_local *local, struct sk_buff *origskb,
return origskb;
}

+#define SEQ_MODULO 0x1000
+#define SEQ_MASK 0xfff
+
+static inline int seq_less(u16 sq1, u16 sq2)
+{
+ return (((sq1 - sq2) & SEQ_MASK) > (SEQ_MODULO >> 1));
+}
+
+static inline u16 seq_inc(u16 sq)
+{
+ return ((sq + 1) & SEQ_MASK);
+}
+
+static inline u16 seq_sub(u16 sq1, u16 sq2)
+{
+ return ((sq1 - sq2) & SEQ_MASK);
+}
+
+ieee80211_txrx_result
+ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw,
+ struct ieee80211_txrx_data *rx,
+ struct tid_ampdu_rx *tid_agg_rx,
+ struct sk_buff *skb, u16 mpdu_seq_num,
+ int bar_req)
+{
+ struct ieee80211_rx_status status;
+ u16 head_seq_num, buf_size;
+ int index;
+
+ buf_size = tid_agg_rx->buf_size;
+ head_seq_num = tid_agg_rx->head_seq_num;
+
+ /* frame with out of date sequence number */
+ if (seq_less(mpdu_seq_num, head_seq_num))
+ return TXRX_DROP;
+
+ /* if frame sequence number exceeds our buffering window size or
+ * block Ack Request arrived - release stored frames */
+ if ((!seq_less(mpdu_seq_num, head_seq_num + buf_size)) || (bar_req)) {
+ /* new head to the ordering buffer */
+ if (bar_req)
+ head_seq_num = mpdu_seq_num;
+ else
+ head_seq_num =
+ seq_inc(seq_sub(mpdu_seq_num, buf_size));
+ /* release stored frames up to new head to stack */
+ while (seq_less(tid_agg_rx->head_seq_num, head_seq_num)) {
+ index = seq_sub(tid_agg_rx->head_seq_num,
+ tid_agg_rx->ssn)
+ % tid_agg_rx->buf_size;
+
+ if (tid_agg_rx->reorder_buf[index]) {
+ /* release the reordered frames to stack */
+ memcpy(&status,
+ tid_agg_rx->reorder_buf[index]->cb,
+ sizeof(status));
+
+ status.ordered = TXRX_QUEUED;
+ __ieee80211_rx(hw,
+ tid_agg_rx->reorder_buf[index],
+ &status);
+ tid_agg_rx->stored_mpdu_num--;
+ tid_agg_rx->reorder_buf[index] = NULL;
+ }
+ tid_agg_rx->head_seq_num =
+ seq_inc(tid_agg_rx->head_seq_num);
+ }
+ if (bar_req)
+ return TXRX_DROP;
+ }
+
+ /* now the new frame is always in the range of the reordering */
+ /* buffer window */
+ index = seq_sub(mpdu_seq_num, tid_agg_rx->ssn)
+ % tid_agg_rx->buf_size;
+ /* check if we already stored this frame */
+ if (tid_agg_rx->reorder_buf[index])
+ return TXRX_DROP;
+
+ /* if arrived mpdu is in the right order and nothing else stored */
+ /* release it immediately */
+ if (mpdu_seq_num == tid_agg_rx->head_seq_num &&
+ tid_agg_rx->stored_mpdu_num == 0) {
+ tid_agg_rx->head_seq_num =
+ seq_inc(tid_agg_rx->head_seq_num);
+ return TXRX_CONTINUE;
+ }
+
+ /* put the frame in the reordering buffer */
+ tid_agg_rx->reorder_buf[index] = skb;
+ tid_agg_rx->stored_mpdu_num++;
+ /* release the buffer until next missing frame */
+ index = seq_sub(tid_agg_rx->head_seq_num, tid_agg_rx->ssn)
+ % tid_agg_rx->buf_size;
+ while (tid_agg_rx->reorder_buf[index]) {
+ /* release the reordered frame back to stack */
+ memcpy(&status, tid_agg_rx->reorder_buf[index]->cb,
+ sizeof(status));
+ status.ordered = TXRX_QUEUED;
+ __ieee80211_rx(hw, tid_agg_rx->reorder_buf[index], &status);
+ tid_agg_rx->stored_mpdu_num--;
+ tid_agg_rx->reorder_buf[index] = NULL;
+ tid_agg_rx->head_seq_num = seq_inc(tid_agg_rx->head_seq_num);
+ index = seq_sub(tid_agg_rx->head_seq_num,
+ tid_agg_rx->ssn) % tid_agg_rx->buf_size;
+ }
+ return TXRX_QUEUED;
+}

/* pre-rx handlers
*
@@ -285,7 +393,7 @@ ieee80211_rx_h_load_stats(struct ieee80211_txrx_data *rx)

/* Estimate total channel use caused by this frame */

- if (unlikely(mode->num_rates < 0))
+ if (unlikely(mode->num_rates < 0) || (rx->u.rx.status->ordered))
return TXRX_CONTINUE;

rate = &mode->rates[0];
@@ -320,10 +428,69 @@ ieee80211_rx_h_load_stats(struct ieee80211_txrx_data *rx)
return TXRX_CONTINUE;
}

+static ieee80211_txrx_result
+ieee80211_rx_h_reorder_ampdu(struct ieee80211_txrx_data *rx)
+{
+ struct ieee80211_local *local = rx->local;
+ struct ieee80211_hw *hw = &local->hw;
+ struct sk_buff *skb = rx->skb;
+ struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+ struct tid_ampdu_rx *tid_agg_rx;
+ u16 sc;
+ u16 mpdu_seq_num;
+
+ if (!rx->sta)
+ return TXRX_CONTINUE;
+
+ tid_agg_rx = &(rx->sta->ampdu_mlme.tid_rx[rx->u.rx.queue]);
+
+ /* filter the QoS data rx stream according to
+ * STA/TID and check if this STA/TID is on aggregation */
+ if ((tid_agg_rx->state != HT_AGG_STATE_OPERATIONAL) ||
+ (!WLAN_FC_IS_QOS_DATA(rx->fc)))
+ return TXRX_CONTINUE;
+
+ /* null data frames are excluded */
+ if (rx->fc & IEEE80211_STYPE_QOS_NULLFUNC)
+ return TXRX_CONTINUE;
+
+ /* check if this frame has already been queued and ordered */
+ if (rx->u.rx.status->ordered == TXRX_QUEUED)
+ return TXRX_CONTINUE;
+
+ /* check if this frame has already been queued and ordered but
+ * should be deleted */
+ if (rx->u.rx.status->ordered == TXRX_DROP)
+ return TXRX_DROP;
+
+ /* new un-ordered ampdu frame - process it */
+
+ /* reset session timer */
+ if (tid_agg_rx->timeout) {
+ unsigned long expires =
+ jiffies + (tid_agg_rx->timeout / 1000) * HZ;
+ mod_timer(&tid_agg_rx->session_timer, expires);
+ }
+
+ /* if this mpdu is fragmented - terminate rx aggregation session */
+ sc = le16_to_cpu(hdr->seq_ctrl);
+ if (sc & IEEE80211_SCTL_FRAG) {
+ ieee80211_sta_stop_rx_BA_session(rx->dev, rx->sta->addr,
+ rx->u.rx.queue, 0, WLAN_REASON_QSTA_REQUIRE_SETUP);
+ return TXRX_CONTINUE;
+ }
+
+ /* according to mpdu sequence number deal with reordering buffer */
+ mpdu_seq_num = (sc & IEEE80211_SCTL_SEQ) >> 4;
+ return ieee80211_sta_manage_reorder_buf(hw, rx, tid_agg_rx, skb,
+ mpdu_seq_num, 0);
+}
+
ieee80211_rx_handler ieee80211_rx_pre_handlers[] =
{
ieee80211_rx_h_parse_qos,
ieee80211_rx_h_load_stats,
+ ieee80211_rx_h_reorder_ampdu,
NULL
};

--
1.5.3.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


2007-12-20 17:45:22

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 5/8 v2] mac80211: A-MPDU Rx handling aggregation reordering


> @@ -1212,7 +1212,8 @@ void ieee80211_sta_stop_rx_BA_session(struct net_device *dev, u8 *ra, u16 tid,
> struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> struct ieee80211_hw *hw = &local->hw;
> struct sta_info *sta;
> - int ret;
> + struct ieee80211_rx_status status;
> + int ret, i;
>
> sta = sta_info_get(local, ra);
> if (!sta)
> @@ -1254,6 +1255,20 @@ void ieee80211_sta_stop_rx_BA_session(struct net_device *dev, u8 *ra, u16 tid,
> ieee80211_send_delba(dev, ra, tid, 0, reason);
>
> /* free the reordering buffer */
> + for (i = 0; i < sta->ampdu_mlme.tid_rx[tid].buf_size; i++) {

Something I just thought of but probably simply missed: where's the
session torn down when we ifdown an interface while an A-MPDU session is
active? I didn't go through all the patches again, did I miss that
somewhere?

johannes


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

2007-12-25 10:08:35

by Ron Rindjunsky

[permalink] [raw]
Subject: Re: [PATCH 5/8 v2] mac80211: A-MPDU Rx handling aggregation reordering

> > @@ -1212,7 +1212,8 @@ void ieee80211_sta_stop_rx_BA_session(struct net_device *dev, u8 *ra, u16 tid,
> > struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> > struct ieee80211_hw *hw = &local->hw;
> > struct sta_info *sta;
> > - int ret;
> > + struct ieee80211_rx_status status;
> > + int ret, i;
> >
> > sta = sta_info_get(local, ra);
> > if (!sta)
> > @@ -1254,6 +1255,20 @@ void ieee80211_sta_stop_rx_BA_session(struct net_device *dev, u8 *ra, u16 tid,
> > ieee80211_send_delba(dev, ra, tid, 0, reason);
> >
> > /* free the reordering buffer */
> > + for (i = 0; i < sta->ampdu_mlme.tid_rx[tid].buf_size; i++) {
>
> Something I just thought of but probably simply missed: where's the
> session torn down when we ifdown an interface while an A-MPDU session is
> active? I didn't go through all the patches again, did I miss that
> somewhere?
>

No, and it should be done.
were would you recommend to put this tear down? i need a flow were i
can be sta specific, and is called only once per ifdown.
sta_info_release related flow, were i thought to put it, is risking me
with endless loops of kref_put calls.

> johannes
>
>

2007-12-20 09:48:34

by Ron Rindjunsky

[permalink] [raw]
Subject: [PATCH 1/8 v2] mac80211: A-MPDU Rx add low level driver API

This patch adds the API to perform A-MPDU actions between mac80211 and low
level driver.

Signed-off-by: Ron Rindjunsky <[email protected]>
---
include/net/mac80211.h | 21 +++++++++++++++++++++
1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 0d67b33..418cd38 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -915,6 +915,18 @@ enum ieee80211_erp_change_flags {
IEEE80211_ERP_CHANGE_PREAMBLE = 1<<1,
};

+/**
+ * enum ieee80211_ampdu_mlme_action - A-MPDU actions
+ *
+ * These flags are used with the ampdu_action() callback in
+ * &struct ieee80211_ops to indicate which action is needed.
+ * @IEEE80211_AMPDU_RX_START: start Rx aggregation
+ * @IEEE80211_AMPDU_RX_STOP: stop Rx aggregation
+ */
+enum ieee80211_ampdu_mlme_action {
+ IEEE80211_AMPDU_RX_START,
+ IEEE80211_AMPDU_RX_STOP,
+};

/**
* struct ieee80211_ops - callbacks from mac80211 to the driver
@@ -1043,6 +1055,12 @@ enum ieee80211_erp_change_flags {
* used to determine whether to reply to Probe Requests.
*
* @conf_ht: Configures low level driver with 802.11n HT data. Must be atomic.
+ *
+ * @ampdu_action: Perform a certain A-MPDU action
+ * The RA/TID combination determines the destination and TID we want
+ * the ampdu action to be performed for. The action is defined through
+ * ieee80211_ampdu_mlme_action. Starting sequence number (@ssn)
+ * is the first frame we expect to perform the action on.
*/
struct ieee80211_ops {
int (*tx)(struct ieee80211_hw *hw, struct sk_buff *skb,
@@ -1089,6 +1107,9 @@ struct ieee80211_ops {
struct ieee80211_tx_control *control);
int (*tx_last_beacon)(struct ieee80211_hw *hw);
int (*conf_ht)(struct ieee80211_hw *hw, struct ieee80211_conf *conf);
+ int (*ampdu_action)(struct ieee80211_hw *hw,
+ enum ieee80211_ampdu_mlme_action action,
+ const u8 *ra, u16 tid, u16 ssn);
};

/**
--
1.5.3.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


2007-12-20 09:42:07

by Ron Rindjunsky

[permalink] [raw]
Subject: [PATCH 7/8 v2] mac80211: A-MPDU Rx handling DELBA requests

This patch opens the flow to DELBA management frames, and handles end
of A-MPDU session produced by this event.

Signed-off-by: Ron Rindjunsky <[email protected]>
---
net/mac80211/ieee80211_sta.c | 40 +++++++++++++++++++++++++++++++++++++++-
1 files changed, 39 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c
index 00ffa81..9301a53 100644
--- a/net/mac80211/ieee80211_sta.c
+++ b/net/mac80211/ieee80211_sta.c
@@ -63,6 +63,8 @@
#define IEEE80211_ADDBA_PARAM_POLICY_MASK 0x0002
#define IEEE80211_ADDBA_PARAM_TID_MASK 0x003C
#define IEEE80211_ADDBA_PARAM_BUF_SIZE_MASK 0xFFA0
+#define IEEE80211_DELBA_PARAM_TID_MASK 0xF000
+#define IEEE80211_DELBA_PARAM_INITIATOR_MASK 0x0800

/* next values represent the buffer size for A-MPDU frame.
* According to IEEE802.11n spec size varies from 8K to 64K (in powers of 2) */
@@ -1275,6 +1277,36 @@ void ieee80211_sta_stop_rx_BA_session(struct net_device *dev, u8 *ra, u16 tid,
sta_info_put(sta);
}

+static void ieee80211_sta_process_delba(struct net_device *dev,
+ struct ieee80211_mgmt *mgmt, size_t len)
+{
+ struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+ struct sta_info *sta;
+ u16 tid, params;
+ u16 initiator;
+ DECLARE_MAC_BUF(mac);
+
+ sta = sta_info_get(local, mgmt->sa);
+ if (!sta)
+ return;
+
+ params = le16_to_cpu(mgmt->u.action.u.delba.params);
+ tid = (params & IEEE80211_DELBA_PARAM_TID_MASK) >> 12;
+ initiator = (params & IEEE80211_DELBA_PARAM_INITIATOR_MASK) >> 11;
+
+#ifdef CONFIG_MAC80211_HT_DEBUG
+ if (net_ratelimit())
+ printk(KERN_DEBUG "delba from %s on tid %d reason code %d\n",
+ print_mac(mac, mgmt->sa), tid,
+ mgmt->u.action.u.delba.reason_code);
+#endif /* CONFIG_MAC80211_HT_DEBUG */
+
+ if (initiator == WLAN_BACK_INITIATOR)
+ ieee80211_sta_stop_rx_BA_session(dev, sta->addr, tid,
+ WLAN_BACK_INITIATOR, 0);
+ sta_info_put(sta);
+}
+
/*
* After receiving Block Ack Request (BAR) we activated a
* timer after each frame arrives from the originator.
@@ -2238,9 +2270,15 @@ void ieee80211_rx_mgmt_action(struct net_device *dev,
break;
ieee80211_sta_process_addba_request(dev, mgmt, len);
break;
+ case WLAN_ACTION_DELBA:
+ if (len < (IEEE80211_MIN_ACTION_SIZE +
+ sizeof(mgmt->u.action.u.delba)))
+ break;
+ ieee80211_sta_process_delba(dev, mgmt, len);
+ break;
default:
if (net_ratelimit())
- printk(KERN_DEBUG "%s: received unsupported BACK\n",
+ printk(KERN_DEBUG "%s: Rx unknown A-MPDU action\n",
dev->name);
break;
}
--
1.5.3.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


2007-12-25 14:41:51

by Ron Rindjunsky

[permalink] [raw]
Subject: Re: [PATCH 5/8 v2] mac80211: A-MPDU Rx handling aggregation reordering

> > what about list_for_each_entry sta inside ieee80211_stop to stop the
> > session? i tried it and it worked really well.
>
> Sounds about right, you just need to make sure that you have the right
> locks and only tear down sessions for the virtual interface that's being
> stopped, not all if you have sessions on different virtual interfaces
> (say on a multi-BSS AP when one BSS is stopped)

ok. i not sure i got it all the way right, but it is in the patch.
As i don't want to hold the whole series any more, i am sending the
series over, and if this specific issue is not well i'll issue only a
patch for it.
thanks
ron

>
> johannes
>
>

2007-12-20 17:42:54

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 3/8 v2] mac80211: A-MPDU Rx adding basic functionality

A few minor comments, looks good otherwise.

> status = WLAN_STATUS_REQUEST_DECLINED;
>
> +

spurious newline added, I'd think.

> + /* prepare reordering buffer */
> + tid_agg_rx->reorder_buf =
> + kmalloc(buf_size * sizeof(struct sk_buf *), GFP_ATOMIC);
> + if (!tid_agg_rx->reorder_buf) {
> + printk(KERN_ERR "can not allocate reordering buffer "
> + "to tid %d\n", tid);

Should that be ratelimited just in case somebody is trying over and over
again while we're under memory pressure?

> + skb = dev_alloc_skb(sizeof(*mgmt) + local->hw.extra_tx_headroom + 1 +
> + sizeof(mgmt->u.action.u.delba));

What's "+ 1"?

> + printk(KERN_DEBUG "rx BA session requested to stop on "
> + "unactive tid %d\n", tid);

typo: "inactive"

> + sta = sta_info_get(local, temp_sta->addr);
> +
> + if (!sta)
> + return;

Still have that?

Thanks,
johannes


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

2007-12-20 09:48:34

by Ron Rindjunsky

[permalink] [raw]
Subject: [PATCH 3/8 v2] mac80211: A-MPDU Rx adding basic functionality

This patch adds the basic needed abilities and functions for A-MPDU Rx session
changed functions:
- ieee80211_sta_process_addba_request - Rx A-MPDU initialization enabled
added functions:
- ieee80211_send_delba - used for sending out Del BA in A-MPDU sessions
- ieee80211_sta_stop_rx_BA_session - stopping Rx A-MPDU session
- sta_rx_agg_session_timer_expired - stops A-MPDU Rx use if load is too low

Signed-off-by: Ron Rindjunsky <[email protected]>
---
include/linux/ieee80211.h | 7 ++
net/mac80211/ieee80211_i.h | 3 +
net/mac80211/ieee80211_sta.c | 228 ++++++++++++++++++++++++++++++++++++++++--
net/mac80211/sta_info.c | 3 +
4 files changed, 231 insertions(+), 10 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 3e64159..4d5a4c9 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -472,6 +472,13 @@ enum ieee80211_back_actioncode {
WLAN_ACTION_DELBA = 2,
};

+/* BACK (block-ack) parties */
+enum ieee80211_back_parties {
+ WLAN_BACK_RECIPIENT = 0,
+ WLAN_BACK_INITIATOR = 1,
+ WLAN_BACK_TIMER = 2,
+};
+
/* A-MSDU 802.11n */
#define IEEE80211_QOS_CONTROL_A_MSDU_PRESENT 0x0080

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index b54ed5f..cf2256b 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -770,6 +770,9 @@ int ieee80211_ht_cap_ie_to_ht_info(struct ieee80211_ht_cap *ht_cap_ie,
int ieee80211_ht_addt_info_ie_to_ht_bss_info(
struct ieee80211_ht_addt_info *ht_add_info_ie,
struct ieee80211_ht_bss_info *bss_info);
+void ieee80211_sta_stop_rx_BA_session(struct net_device *dev, u8 *da,
+ u16 tid, u16 initiator, u16 reason);
+void sta_rx_agg_session_timer_expired(unsigned long data);
/* ieee80211_iface.c */
int ieee80211_if_add(struct net_device *dev, const char *name,
struct net_device **new_dev, int type);
diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c
index 2e0f861..f696dbc 100644
--- a/net/mac80211/ieee80211_sta.c
+++ b/net/mac80211/ieee80211_sta.c
@@ -64,6 +64,11 @@
#define IEEE80211_ADDBA_PARAM_TID_MASK 0x003C
#define IEEE80211_ADDBA_PARAM_BUF_SIZE_MASK 0xFFA0

+/* next values represent the buffer size for A-MPDU frame.
+ * According to IEEE802.11n spec size varies from 8K to 64K (in powers of 2) */
+#define IEEE80211_MIN_AMPDU_BUF 0x8
+#define IEEE80211_MAX_AMPDU_BUF 0x40
+
static void ieee80211_send_probe_req(struct net_device *dev, u8 *dst,
u8 *ssid, size_t ssid_len);
static struct ieee80211_sta_bss *
@@ -1051,9 +1056,14 @@ static void ieee80211_sta_process_addba_request(struct net_device *dev,
size_t len)
{
struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+ struct ieee80211_hw *hw = &local->hw;
+ struct ieee80211_conf *conf = &hw->conf;
struct sta_info *sta;
- u16 capab, tid, timeout, ba_policy, buf_size, status;
+ struct tid_ampdu_rx *tid_agg_rx;
+ u16 capab, tid, timeout, ba_policy, buf_size, start_seq_num, status;
u8 dialog_token;
+ int ret = -EOPNOTSUPP;
+ DECLARE_MAC_BUF(mac);

sta = sta_info_get(local, mgmt->sa);
if (!sta)
@@ -1062,28 +1072,226 @@ static void ieee80211_sta_process_addba_request(struct net_device *dev,
/* extract session parameters from addba request frame */
dialog_token = mgmt->u.action.u.addba_req.dialog_token;
timeout = le16_to_cpu(mgmt->u.action.u.addba_req.timeout);
+ start_seq_num =
+ le16_to_cpu(mgmt->u.action.u.addba_req.start_seq_num) >> 4;

capab = le16_to_cpu(mgmt->u.action.u.addba_req.capab);
ba_policy = (capab & IEEE80211_ADDBA_PARAM_POLICY_MASK) >> 1;
tid = (capab & IEEE80211_ADDBA_PARAM_TID_MASK) >> 2;
buf_size = (capab & IEEE80211_ADDBA_PARAM_BUF_SIZE_MASK) >> 6;

- /* TODO - currently aggregation is declined (A-MPDU add BA request
- * acceptance is not obligatory by 802.11n draft), but here is
- * the entry point for dealing with it */
-#ifdef MAC80211_HT_DEBUG
- if (net_ratelimit())
- printk(KERN_DEBUG "Add Block Ack request arrived,"
- " currently denying it\n");
-#endif /* MAC80211_HT_DEBUG */
-
status = WLAN_STATUS_REQUEST_DECLINED;

+
+ /* sanity check for incoming parameters:
+ * check if configuration can support the BA policy
+ * and if buffer size does not exceeds max value */
+ if (((ba_policy != 1)
+ && (!(conf->ht_conf.cap & IEEE80211_HT_CAP_DELAY_BA)))
+ || (buf_size > IEEE80211_MAX_AMPDU_BUF)) {
+ status = WLAN_STATUS_INVALID_QOS_PARAM;
+#ifdef CONFIG_MAC80211_HT_DEBUG
+ if (net_ratelimit())
+ printk(KERN_DEBUG "Block Ack Req with bad params from "
+ "%s on tid %u. policy %d, buffer size %d\n",
+ print_mac(mac, mgmt->sa), tid, ba_policy,
+ buf_size);
+#endif /* CONFIG_MAC80211_HT_DEBUG */
+ goto end_no_lock;
+ }
+ /* determine default buffer size */
+ if (buf_size == 0) {
+ struct ieee80211_hw_mode *mode = conf->mode;
+ buf_size = IEEE80211_MIN_AMPDU_BUF;
+ buf_size = buf_size << mode->ht_info.ampdu_factor;
+ }
+
+ tid_agg_rx = &sta->ampdu_mlme.tid_rx[tid];
+
+ /* examine state machine */
+ spin_lock_bh(&sta->ampdu_mlme.ampdu_rx);
+
+ if (tid_agg_rx->state != HT_AGG_STATE_IDLE) {
+#ifdef CONFIG_MAC80211_HT_DEBUG
+ if (net_ratelimit())
+ printk(KERN_DEBUG "unexpected Block Ack Req from "
+ "%s on tid %u\n",
+ print_mac(mac, mgmt->sa), tid);
+#endif /* CONFIG_MAC80211_HT_DEBUG */
+ goto end;
+ }
+
+ /* prepare reordering buffer */
+ tid_agg_rx->reorder_buf =
+ kmalloc(buf_size * sizeof(struct sk_buf *), GFP_ATOMIC);
+ if (!tid_agg_rx->reorder_buf) {
+ printk(KERN_ERR "can not allocate reordering buffer "
+ "to tid %d\n", tid);
+ goto end;
+ }
+ memset(tid_agg_rx->reorder_buf, 0,
+ buf_size * sizeof(struct sk_buf *));
+
+ if (local->ops->ampdu_action)
+ ret = local->ops->ampdu_action(hw, IEEE80211_AMPDU_RX_START,
+ sta->addr, tid, start_seq_num);
+#ifdef CONFIG_MAC80211_HT_DEBUG
+ printk(KERN_DEBUG "Rx A-MPDU on tid %d result %d", tid, ret);
+#endif /* CONFIG_MAC80211_HT_DEBUG */
+
+ if (ret) {
+ kfree(tid_agg_rx->reorder_buf);
+ goto end;
+ }
+
+ /* change state and send addba resp */
+ tid_agg_rx->state = HT_AGG_STATE_OPERATIONAL;
+ tid_agg_rx->dialog_token = dialog_token;
+ tid_agg_rx->ssn = start_seq_num;
+ tid_agg_rx->head_seq_num = start_seq_num;
+ tid_agg_rx->buf_size = buf_size;
+ tid_agg_rx->timeout = timeout;
+ tid_agg_rx->stored_mpdu_num = 0;
+ status = WLAN_STATUS_SUCCESS;
+end:
+ spin_unlock_bh(&sta->ampdu_mlme.ampdu_rx);
+
+end_no_lock:
ieee80211_send_addba_resp(sta->dev, sta->addr, tid, dialog_token,
status, 1, buf_size, timeout);
sta_info_put(sta);
}

+void ieee80211_send_delba(struct net_device *dev, const u8 *da, u16 tid,
+ u16 initiator, u16 reason_code)
+{
+ struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ struct ieee80211_if_sta *ifsta = &sdata->u.sta;
+ struct sk_buff *skb;
+ struct ieee80211_mgmt *mgmt;
+ u16 params;
+
+ skb = dev_alloc_skb(sizeof(*mgmt) + local->hw.extra_tx_headroom + 1 +
+ sizeof(mgmt->u.action.u.delba));
+
+ if (!skb) {
+ printk(KERN_ERR "%s: failed to allocate buffer "
+ "for delba frame\n", dev->name);
+ return;
+ }
+
+ skb_reserve(skb, local->hw.extra_tx_headroom);
+ mgmt = (struct ieee80211_mgmt *) skb_put(skb, 24);
+ memset(mgmt, 0, 24);
+ memcpy(mgmt->da, da, ETH_ALEN);
+ memcpy(mgmt->sa, dev->dev_addr, ETH_ALEN);
+ if (sdata->type == IEEE80211_IF_TYPE_AP)
+ memcpy(mgmt->bssid, dev->dev_addr, ETH_ALEN);
+ else
+ memcpy(mgmt->bssid, ifsta->bssid, ETH_ALEN);
+ mgmt->frame_control = IEEE80211_FC(IEEE80211_FTYPE_MGMT,
+ IEEE80211_STYPE_ACTION);
+
+ skb_put(skb, 1 + sizeof(mgmt->u.action.u.delba));
+
+ mgmt->u.action.category = WLAN_CATEGORY_BACK;
+ mgmt->u.action.u.delba.action_code = WLAN_ACTION_DELBA;
+ params = (u16)(initiator << 11); /* bit 11 initiator */
+ params |= (u16)(tid << 12); /* bit 15:12 TID number */
+
+ mgmt->u.action.u.delba.params = cpu_to_le16(params);
+ mgmt->u.action.u.delba.reason_code = cpu_to_le16(reason_code);
+
+ ieee80211_sta_tx(dev, skb, 0);
+}
+
+void ieee80211_sta_stop_rx_BA_session(struct net_device *dev, u8 *ra, u16 tid,
+ u16 initiator, u16 reason)
+{
+ struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+ struct ieee80211_hw *hw = &local->hw;
+ struct sta_info *sta;
+ int ret;
+
+ sta = sta_info_get(local, ra);
+ if (!sta)
+ return;
+
+ /* check if TID is in operational state */
+ spin_lock_bh(&sta->ampdu_mlme.ampdu_rx);
+ if (sta->ampdu_mlme.tid_rx[tid].state
+ != HT_AGG_STATE_OPERATIONAL) {
+ spin_unlock_bh(&sta->ampdu_mlme.ampdu_rx);
+ if (net_ratelimit())
+ printk(KERN_DEBUG "rx BA session requested to stop on "
+ "unactive tid %d\n", tid);
+ sta_info_put(sta);
+ return;
+ }
+ sta->ampdu_mlme.tid_rx[tid].state =
+ HT_AGG_STATE_REQ_STOP_BA_MSK |
+ (initiator << HT_AGG_STATE_INITIATOR_SHIFT);
+ spin_unlock_bh(&sta->ampdu_mlme.ampdu_rx);
+
+ /* stop HW Rx aggregation. ampdu_action existence
+ * already verified in session init so we add the BUG_ON */
+ BUG_ON(!local->ops->ampdu_action);
+
+ ret = local->ops->ampdu_action(hw, IEEE80211_AMPDU_RX_STOP,
+ ra, tid, EINVAL);
+ if (ret)
+ printk(KERN_DEBUG "HW problem - can not stop rx "
+ "aggergation for tid %d\n", tid);
+
+ /* shutdown timer has not expired */
+ if (initiator != WLAN_BACK_TIMER)
+ del_timer_sync(&sta->ampdu_mlme.tid_rx[tid].
+ session_timer);
+
+ /* check if this is a self generated aggregation halt */
+ if (initiator == WLAN_BACK_RECIPIENT || initiator == WLAN_BACK_TIMER)
+ ieee80211_send_delba(dev, ra, tid, 0, reason);
+
+ /* free the reordering buffer */
+ kfree(sta->ampdu_mlme.tid_rx[tid].reorder_buf);
+
+ sta->ampdu_mlme.tid_rx[tid].state = HT_AGG_STATE_IDLE;
+ sta_info_put(sta);
+}
+
+/*
+ * After receiving Block Ack Request (BAR) we activated a
+ * timer after each frame arrives from the originator.
+ * if this timer expires ieee80211_sta_stop_rx_BA_session will be executed.
+ */
+void sta_rx_agg_session_timer_expired(unsigned long data)
+{
+ /* not an elegant detour, but there is no choice as the timer passes
+ * only one argument, and ieee80211_local is needed here, so init
+ * flow in sta_info_add gives the TID as data, while the timer_to_id
+ * array gives the sta through container_of */
+ u8 *ptid = (u8 *)data;
+ u8 *timer_to_id = ptid - *ptid;
+ struct sta_info *temp_sta = container_of(timer_to_id, struct sta_info,
+ timer_to_tid[0]);
+ struct ieee80211_local *local = temp_sta->local;
+ struct sta_info *sta;
+ u16 tid = (u16)*ptid;
+
+ sta = sta_info_get(local, temp_sta->addr);
+
+ if (!sta)
+ return;
+
+ printk(KERN_DEBUG "rx session timer expired on tid %d\n", tid);
+ ieee80211_sta_stop_rx_BA_session(sta->dev, sta->addr, tid,
+ WLAN_BACK_TIMER,
+ WLAN_REASON_QSTA_TIMEOUT);
+ sta_info_put(sta);
+}
+
+
static void ieee80211_rx_mgmt_auth(struct net_device *dev,
struct ieee80211_if_sta *ifsta,
struct ieee80211_mgmt *mgmt,
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index e849155..cc28e82 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -103,6 +103,7 @@ static void sta_info_release(struct kref *kref)
struct sta_info *sta = container_of(kref, struct sta_info, kref);
struct ieee80211_local *local = sta->local;
struct sk_buff *skb;
+ int i;

/* free sta structure; it has already been removed from
* hash table etc. external structures. Make sure that all
@@ -115,6 +116,8 @@ static void sta_info_release(struct kref *kref)
while ((skb = skb_dequeue(&sta->tx_filtered)) != NULL) {
dev_kfree_skb_any(skb);
}
+ for (i = 0; i < STA_TID_NUM; i++)
+ del_timer_sync(&sta->ampdu_mlme.tid_rx[i].session_timer);
rate_control_free_sta(sta->rate_ctrl, sta->rate_ctrl_priv);
rate_control_put(sta->rate_ctrl);
kfree(sta);
--
1.5.3.3

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


2007-12-25 11:35:41

by Ron Rindjunsky

[permalink] [raw]
Subject: Re: [PATCH 5/8 v2] mac80211: A-MPDU Rx handling aggregation reordering

> > No, and it should be done.
> > were would you recommend to put this tear down? i need a flow were i
> > can be sta specific, and is called only once per ifdown.
> > sta_info_release related flow, were i thought to put it, is risking me
> > with endless loops of kref_put calls.
>
> That's actually an interesting question. We should, like with keys,
> remove all STA info from the driver when the corresponding interface is
> down. I'd think it should happen at the same time. I guess you'll have
> to add some sort of loop to tear down stations to the down code, I don't
> really see any easy way right now.
>

what about list_for_each_entry sta inside ieee80211_stop to stop the
session? i tried it and it worked really well.

> johannes
>
>

2007-12-25 10:39:40

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 5/8 v2] mac80211: A-MPDU Rx handling aggregation reordering


> No, and it should be done.
> were would you recommend to put this tear down? i need a flow were i
> can be sta specific, and is called only once per ifdown.
> sta_info_release related flow, were i thought to put it, is risking me
> with endless loops of kref_put calls.

That's actually an interesting question. We should, like with keys,
remove all STA info from the driver when the corresponding interface is
down. I'd think it should happen at the same time. I guess you'll have
to add some sort of loop to tear down stations to the down code, I don't
really see any easy way right now.

johannes


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