linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] mac80211: Avoid filling up mesh preq queue with redundant requests
@ 2011-10-29  5:05 Thomas Pedersen
  2011-10-29  5:05 ` [PATCH 2/4] mac80211: QoS multicast frames have No Ack policy Thomas Pedersen
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Thomas Pedersen @ 2011-10-29  5:05 UTC (permalink / raw)
  To: linux-wireless; +Cc: devel, Javier Cardona, johannes, linville

From: Javier Cardona <javier@cozybit.com>

Don't accept redundant PREQs for a given destination. This fixes a
problem under high load:

kernel: [20386.250913] mesh_queue_preq: 235 callbacks suppressed
kernel: [20386.253335] Mesh HWMP (mesh0): PREQ node queue full
kernel: [20386.253352] Mesh HWMP (mesh0): PREQ node queue full
(...)

The 802.11s protocol has a provision to limit the rate of path requests
(PREQs) are transmitted (dot11MeshHWMPpreqMinInterval) but there was no
limit on the rate at which PREQs were being queued up.  There is a valid
reason for queuing PREQs: this way we can even out PREQ bursts.  But
queueing multiple PREQs for the same destination is useless.

Reported-by: Pedro Larbig <pedro.larbig@carhs.de>
Signed-off-by: Javier Cardona <javier@cozybit.com>
---
v2: fix merge conflict

 net/mac80211/mesh.h      |    3 +++
 net/mac80211/mesh_hwmp.c |   15 +++++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/mesh.h b/net/mac80211/mesh.h
index 8c00e2d..b3745e8 100644
--- a/net/mac80211/mesh.h
+++ b/net/mac80211/mesh.h
@@ -31,6 +31,8 @@
  * @MESH_PATH_FIXED: the mesh path has been manually set and should not be
  * 	modified
  * @MESH_PATH_RESOLVED: the mesh path can has been resolved
+ * @MESH_PATH_REQ_QUEUED: there is an unsent path request for this destination
+ * already queued up, waiting for the discovery process to start.
  *
  * MESH_PATH_RESOLVED is used by the mesh path timer to
  * decide when to stop or cancel the mesh path discovery.
@@ -41,6 +43,7 @@ enum mesh_path_flags {
 	MESH_PATH_SN_VALID =	BIT(2),
 	MESH_PATH_FIXED	=	BIT(3),
 	MESH_PATH_RESOLVED =	BIT(4),
+	MESH_PATH_REQ_QUEUED =	BIT(5),
 };
 
 /**
diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index 9a1f8bb..b7d9dfd 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -867,9 +867,19 @@ static void mesh_queue_preq(struct mesh_path *mpath, u8 flags)
 		return;
 	}
 
+	spin_lock_bh(&mpath->state_lock);
+	if (mpath->flags & MESH_PATH_REQ_QUEUED) {
+		spin_unlock_bh(&mpath->state_lock);
+		spin_unlock_bh(&ifmsh->mesh_preq_queue_lock);
+		return;
+	}
+
 	memcpy(preq_node->dst, mpath->dst, ETH_ALEN);
 	preq_node->flags = flags;
 
+	mpath->flags |= MESH_PATH_REQ_QUEUED;
+	spin_unlock_bh(&mpath->state_lock);
+
 	list_add_tail(&preq_node->list, &ifmsh->preq_queue.list);
 	++ifmsh->preq_queue_len;
 	spin_unlock_bh(&ifmsh->mesh_preq_queue_lock);
@@ -921,6 +931,7 @@ void mesh_path_start_discovery(struct ieee80211_sub_if_data *sdata)
 		goto enddiscovery;
 
 	spin_lock_bh(&mpath->state_lock);
+	mpath->flags &= ~MESH_PATH_REQ_QUEUED;
 	if (preq_node->flags & PREQ_Q_F_START) {
 		if (mpath->flags & MESH_PATH_RESOLVING) {
 			spin_unlock_bh(&mpath->state_lock);
@@ -1028,8 +1039,7 @@ int mesh_nexthop_lookup(struct sk_buff *skb,
 			mesh_queue_preq(mpath, PREQ_Q_F_START);
 		}
 
-		if (skb_queue_len(&mpath->frame_queue) >=
-				MESH_FRAME_QUEUE_LEN)
+		if (skb_queue_len(&mpath->frame_queue) >= MESH_FRAME_QUEUE_LEN)
 			skb_to_free = skb_dequeue(&mpath->frame_queue);
 
 		info->flags |= IEEE80211_TX_INTFL_NEED_TXPROCESSING;
@@ -1062,6 +1072,7 @@ void mesh_path_timer(unsigned long data)
 		++mpath->discovery_retries;
 		mpath->discovery_timeout *= 2;
 		spin_unlock_bh(&mpath->state_lock);
+		mpath->flags &= ~MESH_PATH_REQ_QUEUED;
 		mesh_queue_preq(mpath, 0);
 	} else {
 		mpath->flags = 0;
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/4] mac80211: QoS multicast frames have No Ack policy
  2011-10-29  5:05 [PATCH 1/4] mac80211: Avoid filling up mesh preq queue with redundant requests Thomas Pedersen
@ 2011-10-29  5:05 ` Thomas Pedersen
  2011-10-29  9:40   ` Christian Lamparter
  2011-11-02 10:17   ` Johannes Berg
  2011-10-29  5:05 ` [PATCH 3/4] mac80211: check if frame is really part of this BA Thomas Pedersen
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 12+ messages in thread
From: Thomas Pedersen @ 2011-10-29  5:05 UTC (permalink / raw)
  To: linux-wireless; +Cc: devel, Thomas Pedersen, johannes, linville

Previously QoS multicast frames had the Normal Acknowledgment QoS
control bits set. This would cause broadcast frames to be discarded by
peers with which we have a BA session, since their sequence number would
fall outside the allowed range. Set No Ack QoS control bits on multicast
QoS frames and filter these in de-aggregation code.

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---
 net/mac80211/rx.c  |    8 +++++++-
 net/mac80211/wme.c |    3 ++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index b867bd5..ee9e71b 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -747,7 +747,7 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx)
 	struct sta_info *sta = rx->sta;
 	struct tid_ampdu_rx *tid_agg_rx;
 	u16 sc;
-	int tid;
+	u8 tid, ack_policy;
 
 	if (!ieee80211_is_data_qos(hdr->frame_control))
 		goto dont_reorder;
@@ -760,6 +760,7 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx)
 	if (!sta)
 		goto dont_reorder;
 
+	ack_policy = *ieee80211_get_qos_ctl(hdr) & ~IEEE80211_QOS_CTL_TID_MASK;
 	tid = *ieee80211_get_qos_ctl(hdr) & IEEE80211_QOS_CTL_TID_MASK;
 
 	tid_agg_rx = rcu_dereference(sta->ampdu_mlme.tid_rx[tid]);
@@ -770,6 +771,11 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx)
 	if (unlikely(hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_NULLFUNC)))
 		goto dont_reorder;
 
+	/* not part of a BA session */
+	if (!((ack_policy == IEEE80211_QOS_CTL_ACK_POLICY_BLOCKACK) ||
+	      (ack_policy == IEEE80211_QOS_CTL_ACK_POLICY_NORMAL)))
+		goto dont_reorder;
+
 	/* new, potentially un-ordered, ampdu frame - process it */
 
 	/* reset session timer */
diff --git a/net/mac80211/wme.c b/net/mac80211/wme.c
index fd52e69..a440a4a 100644
--- a/net/mac80211/wme.c
+++ b/net/mac80211/wme.c
@@ -147,7 +147,8 @@ void ieee80211_set_qos_hdr(struct ieee80211_sub_if_data *sdata,
 
 		tid = skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK;
 
-		if (unlikely(sdata->local->wifi_wme_noack_test))
+		if (unlikely(sdata->local->wifi_wme_noack_test) ||
+		    is_multicast_ether_addr(hdr->addr1))
 			ack_policy |= IEEE80211_QOS_CTL_ACK_POLICY_NOACK;
 		/* qos header is 2 bytes */
 		*p++ = ack_policy | tid;
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/4] mac80211: check if frame is really part of this BA
  2011-10-29  5:05 [PATCH 1/4] mac80211: Avoid filling up mesh preq queue with redundant requests Thomas Pedersen
  2011-10-29  5:05 ` [PATCH 2/4] mac80211: QoS multicast frames have No Ack policy Thomas Pedersen
@ 2011-10-29  5:05 ` Thomas Pedersen
  2011-11-02 10:19   ` Johannes Berg
  2011-10-29  5:05 ` [PATCH 4/4] mac80211: Populate QoS header on mesh frames sent to gates Thomas Pedersen
  2011-11-02 10:18 ` [PATCH 1/4] mac80211: Avoid filling up mesh preq queue with redundant requests Johannes Berg
  3 siblings, 1 reply; 12+ messages in thread
From: Thomas Pedersen @ 2011-10-29  5:05 UTC (permalink / raw)
  To: linux-wireless; +Cc: devel, Thomas Pedersen, johannes, linville

There was an an implicit assumption that any QoS data frame received
from a STA/TID with an active BA session was sent to this vif as part of
a BA.  This is not true if IFF_PROMISC is enabled and the frame was
destined for a different peer, for example. Don't treat these frames as
part of a BA from the sending STA.

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---
 net/mac80211/rx.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index ee9e71b..ad39216 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -776,6 +776,10 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx)
 	      (ack_policy == IEEE80211_QOS_CTL_ACK_POLICY_NORMAL)))
 		goto dont_reorder;
 
+	/* not actually part of this BA session */
+	if (compare_ether_addr(hdr->addr1, rx->sdata->vif.addr) != 0)
+		goto dont_reorder;
+
 	/* new, potentially un-ordered, ampdu frame - process it */
 
 	/* reset session timer */
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/4] mac80211: Populate QoS header on mesh frames sent to gates.
  2011-10-29  5:05 [PATCH 1/4] mac80211: Avoid filling up mesh preq queue with redundant requests Thomas Pedersen
  2011-10-29  5:05 ` [PATCH 2/4] mac80211: QoS multicast frames have No Ack policy Thomas Pedersen
  2011-10-29  5:05 ` [PATCH 3/4] mac80211: check if frame is really part of this BA Thomas Pedersen
@ 2011-10-29  5:05 ` Thomas Pedersen
  2011-11-02 10:18 ` [PATCH 1/4] mac80211: Avoid filling up mesh preq queue with redundant requests Johannes Berg
  3 siblings, 0 replies; 12+ messages in thread
From: Thomas Pedersen @ 2011-10-29  5:05 UTC (permalink / raw)
  To: linux-wireless; +Cc: devel, Javier Cardona, johannes, linville

From: Javier Cardona <javier@cozybit.com>

Also set correct skb queue mapping once next hop is known, and set
txinfo jiffies when forwarding.

Signed-off-by: Javier Cardona <javier@cozybit.com>
---
 net/mac80211/mesh_hwmp.c    |    5 ++++-
 net/mac80211/mesh_pathtbl.c |    5 +++++
 net/mac80211/rx.c           |    8 +++++---
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index b7d9dfd..d46109b 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -1028,8 +1028,11 @@ int mesh_nexthop_lookup(struct sk_buff *skb,
 					PREQ_Q_F_START | PREQ_Q_F_REFRESH);
 		}
 		next_hop = rcu_dereference(mpath->next_hop);
-		if (next_hop)
+		if (next_hop) {
 			memcpy(hdr->addr1, next_hop->sta.addr, ETH_ALEN);
+			skb_set_queue_mapping(skb,
+				ieee80211_select_queue(sdata, skb));
+		}
 		else
 			err = -ENOENT;
 	} else {
diff --git a/net/mac80211/mesh_pathtbl.c b/net/mac80211/mesh_pathtbl.c
index 332b5ff1..9d2f55f 100644
--- a/net/mac80211/mesh_pathtbl.c
+++ b/net/mac80211/mesh_pathtbl.c
@@ -270,6 +270,11 @@ static void prepare_for_gate(struct sk_buff *skb, char *dst_addr,
 	memcpy(hdr->addr1, next_hop, ETH_ALEN);
 	rcu_read_unlock();
 	memcpy(hdr->addr3, dst_addr, ETH_ALEN);
+
+	/*  once next hop is set we can set qos header */
+	skb_set_queue_mapping(skb,
+			ieee80211_select_queue(gate_mpath->sdata, skb));
+	ieee80211_set_qos_hdr(gate_mpath->sdata, skb);
 }
 
 /**
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index ad39216..dc60a19 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1963,12 +1963,10 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx)
 			memset(info, 0, sizeof(*info));
 			info->flags |= IEEE80211_TX_INTFL_NEED_TXPROCESSING;
 			info->control.vif = &rx->sdata->vif;
+			info->control.jiffies = jiffies;
 			if (is_multicast_ether_addr(fwd_hdr->addr1)) {
 				IEEE80211_IFSTA_MESH_CTR_INC(&sdata->u.mesh,
 								fwded_mcast);
-				skb_set_queue_mapping(fwd_skb,
-					ieee80211_select_queue(sdata, fwd_skb));
-				ieee80211_set_qos_hdr(sdata, fwd_skb);
 			} else {
 				int err;
 				/*
@@ -1989,6 +1987,10 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx)
 			}
 			IEEE80211_IFSTA_MESH_CTR_INC(&sdata->u.mesh,
 						     fwded_frames);
+
+			/* next hop is now known, update the queue mapping */
+			skb_set_queue_mapping(fwd_skb,
+				ieee80211_select_queue(sdata, fwd_skb));
 			ieee80211_add_pending_skb(local, fwd_skb);
 		}
 	}
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] mac80211: QoS multicast frames have No Ack policy
  2011-10-29  5:05 ` [PATCH 2/4] mac80211: QoS multicast frames have No Ack policy Thomas Pedersen
@ 2011-10-29  9:40   ` Christian Lamparter
  2011-10-31  7:03     ` Thomas Pedersen
  2011-11-02 10:17   ` Johannes Berg
  1 sibling, 1 reply; 12+ messages in thread
From: Christian Lamparter @ 2011-10-29  9:40 UTC (permalink / raw)
  To: Thomas Pedersen; +Cc: linux-wireless, devel, johannes, linville

On Saturday 29 October 2011 07:05:30 Thomas Pedersen wrote:
> Previously QoS multicast frames had the Normal Acknowledgment QoS
> control bits set. This would cause broadcast frames to be discarded by
> peers with which we have a BA session, since their sequence number would
> fall outside the allowed range. Set No Ack QoS control bits on multicast
> QoS frames and filter these in de-aggregation code.
> 
> Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
> ---
>  net/mac80211/rx.c  |    8 +++++++-
>  net/mac80211/wme.c |    3 ++-
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index b867bd5..ee9e71b 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -747,7 +747,7 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx)
>  	struct sta_info *sta = rx->sta;
>  	struct tid_ampdu_rx *tid_agg_rx;
>  	u16 sc;
> -	int tid;
> +	u8 tid, ack_policy;
>  
>  	if (!ieee80211_is_data_qos(hdr->frame_control))
>  		goto dont_reorder;
> @@ -760,6 +760,7 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx)
>  	if (!sta)
>  		goto dont_reorder;
>  
> +	ack_policy = *ieee80211_get_qos_ctl(hdr) & ~IEEE80211_QOS_CTL_TID_MASK;
uh, while it might be a bit far fetched, but ack_policy might now clash with
frames that have set:

IEEE80211_QOS_CTL_EOSP (0x10)
IEEE80211_QOS_CTL_A_MSDU_PRESENT (0x80)
IEEE80211_QOS_CTL_MESH_CONTROL_PRESENT (0x100)

so I think we would be better of with something like:

(in include/linux/ieee80211.h)
#define IEEE80211_QOS_CTL_ACK_POLICY_MASK 0x0060

ack_policy = *ieee80211_get_qos_ctl(hdr) & ~IEEE80211_QOS_CTL_ACK_POLICY_MASK;

> @@ -770,6 +771,11 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx)
>  	if (unlikely(hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_NULLFUNC)))
>  		goto dont_reorder;
>  
> +	/* not part of a BA session */
> +	if (!((ack_policy == IEEE80211_QOS_CTL_ACK_POLICY_BLOCKACK) ||
> +	      (ack_policy == IEEE80211_QOS_CTL_ACK_POLICY_NORMAL)))
> +		goto dont_reorder;
> +

Regards,
	Chr

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] mac80211: QoS multicast frames have No Ack policy
  2011-10-29  9:40   ` Christian Lamparter
@ 2011-10-31  7:03     ` Thomas Pedersen
  2011-10-31  9:16       ` Christian Lamparter
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Pedersen @ 2011-10-31  7:03 UTC (permalink / raw)
  To: Christian Lamparter; +Cc: linux-wireless, devel, johannes, linville

On Sat, Oct 29, 2011 at 2:40 AM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> On Saturday 29 October 2011 07:05:30 Thomas Pedersen wrote:
>> Previously QoS multicast frames had the Normal Acknowledgment QoS
>> control bits set. This would cause broadcast frames to be discarded by
>> peers with which we have a BA session, since their sequence number would
>> fall outside the allowed range. Set No Ack QoS control bits on multicast
>> QoS frames and filter these in de-aggregation code.
>>
>> Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
>> ---
>>  net/mac80211/rx.c  |    8 +++++++-
>>  net/mac80211/wme.c |    3 ++-
>>  2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
>> index b867bd5..ee9e71b 100644
>> --- a/net/mac80211/rx.c
>> +++ b/net/mac80211/rx.c
>> @@ -747,7 +747,7 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx)
>>       struct sta_info *sta = rx->sta;
>>       struct tid_ampdu_rx *tid_agg_rx;
>>       u16 sc;
>> -     int tid;
>> +     u8 tid, ack_policy;
>>
>>       if (!ieee80211_is_data_qos(hdr->frame_control))
>>               goto dont_reorder;
>> @@ -760,6 +760,7 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx)
>>       if (!sta)
>>               goto dont_reorder;
>>
>> +     ack_policy = *ieee80211_get_qos_ctl(hdr) & ~IEEE80211_QOS_CTL_TID_MASK;
> uh, while it might be a bit far fetched, but ack_policy might now clash with
> frames that have set:
>
> IEEE80211_QOS_CTL_EOSP (0x10)
> IEEE80211_QOS_CTL_A_MSDU_PRESENT (0x80)
> IEEE80211_QOS_CTL_MESH_CONTROL_PRESENT (0x100)
>
> so I think we would be better of with something like:
>
> (in include/linux/ieee80211.h)
> #define IEEE80211_QOS_CTL_ACK_POLICY_MASK 0x0060
>
> ack_policy = *ieee80211_get_qos_ctl(hdr) & ~IEEE80211_QOS_CTL_ACK_POLICY_MASK;

Good idea. Will fix and resubmit.
>
>> @@ -770,6 +771,11 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx)
>>       if (unlikely(hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_NULLFUNC)))
>>               goto dont_reorder;
>>
>> +     /* not part of a BA session */
>> +     if (!((ack_policy == IEEE80211_QOS_CTL_ACK_POLICY_BLOCKACK) ||
>> +           (ack_policy == IEEE80211_QOS_CTL_ACK_POLICY_NORMAL)))
>> +             goto dont_reorder;
>> +


Thanks,
Thomas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] mac80211: QoS multicast frames have No Ack policy
  2011-10-31  7:03     ` Thomas Pedersen
@ 2011-10-31  9:16       ` Christian Lamparter
  0 siblings, 0 replies; 12+ messages in thread
From: Christian Lamparter @ 2011-10-31  9:16 UTC (permalink / raw)
  To: Thomas Pedersen; +Cc: linux-wireless, devel, johannes, linville

On Monday, October 31, 2011 08:03:12 AM Thomas Pedersen wrote:
> On Sat, Oct 29, 2011 at 2:40 AM, Christian Lamparter <chunkeey@googlemail.com> wrote:
> > ack_policy = *ieee80211_get_qos_ctl(hdr) & ~IEEE80211_QOS_CTL_ACK_POLICY_MASK;
> 
> Good idea. Will fix and resubmit.

hmpf, make that: (stupid c&p typo :) )
ack_policy = *ieee80211_get_qos_ctl(hdr) & IEEE80211_QOS_CTL_ACK_POLICY_MASK;

Regards,
	Chr

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] mac80211: QoS multicast frames have No Ack policy
  2011-10-29  5:05 ` [PATCH 2/4] mac80211: QoS multicast frames have No Ack policy Thomas Pedersen
  2011-10-29  9:40   ` Christian Lamparter
@ 2011-11-02 10:17   ` Johannes Berg
  2011-11-02 17:35     ` Thomas Pedersen
  1 sibling, 1 reply; 12+ messages in thread
From: Johannes Berg @ 2011-11-02 10:17 UTC (permalink / raw)
  To: Thomas Pedersen; +Cc: linux-wireless, devel, linville

On Fri, 2011-10-28 at 22:05 -0700, Thomas Pedersen wrote:
> Previously QoS multicast frames had the Normal Acknowledgment QoS
> control bits set. This would cause broadcast frames to be discarded by
> peers with which we have a BA session, since their sequence number would
> fall outside the allowed range. Set No Ack QoS control bits on multicast
> QoS frames and filter these in de-aggregation code.

I'm not sure why you would attempt to deaggregate broadcast frames but I
guess mesh is a bit special.

> @@ -770,6 +771,11 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx)
>  	if (unlikely(hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_NULLFUNC)))
>  		goto dont_reorder;
>  
> +	/* not part of a BA session */
> +	if (!((ack_policy == IEEE80211_QOS_CTL_ACK_POLICY_BLOCKACK) ||
> +	      (ack_policy == IEEE80211_QOS_CTL_ACK_POLICY_NORMAL)))
> +		goto dont_reorder;

Maybe ack_policy != BA && ack_policy != NORMAl would be easier to read?

> --- a/net/mac80211/wme.c
> +++ b/net/mac80211/wme.c
> @@ -147,7 +147,8 @@ void ieee80211_set_qos_hdr(struct ieee80211_sub_if_data *sdata,
>  
>  		tid = skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK;
>  
> -		if (unlikely(sdata->local->wifi_wme_noack_test))
> +		if (unlikely(sdata->local->wifi_wme_noack_test) ||
> +		    is_multicast_ether_addr(hdr->addr1))
>  			ack_policy |= IEEE80211_QOS_CTL_ACK_POLICY_NOACK;

Interestingly, this seems to have been a bug for a long time --
7.1.3.5.3 indicates this is supposed to be done.

johannes


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] mac80211: Avoid filling up mesh preq queue with redundant requests
  2011-10-29  5:05 [PATCH 1/4] mac80211: Avoid filling up mesh preq queue with redundant requests Thomas Pedersen
                   ` (2 preceding siblings ...)
  2011-10-29  5:05 ` [PATCH 4/4] mac80211: Populate QoS header on mesh frames sent to gates Thomas Pedersen
@ 2011-11-02 10:18 ` Johannes Berg
  3 siblings, 0 replies; 12+ messages in thread
From: Johannes Berg @ 2011-11-02 10:18 UTC (permalink / raw)
  To: Thomas Pedersen; +Cc: linux-wireless, devel, Javier Cardona, linville

On Fri, 2011-10-28 at 22:05 -0700, Thomas Pedersen wrote:

> @@ -1062,6 +1072,7 @@ void mesh_path_timer(unsigned long data)
>  		++mpath->discovery_retries;
>  		mpath->discovery_timeout *= 2;
>  		spin_unlock_bh(&mpath->state_lock);
> +		mpath->flags &= ~MESH_PATH_REQ_QUEUED;

locking spot-check -- is that really correct?

johannes


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] mac80211: check if frame is really part of this BA
  2011-10-29  5:05 ` [PATCH 3/4] mac80211: check if frame is really part of this BA Thomas Pedersen
@ 2011-11-02 10:19   ` Johannes Berg
  0 siblings, 0 replies; 12+ messages in thread
From: Johannes Berg @ 2011-11-02 10:19 UTC (permalink / raw)
  To: Thomas Pedersen; +Cc: linux-wireless, devel, linville

On Fri, 2011-10-28 at 22:05 -0700, Thomas Pedersen wrote:
> There was an an implicit assumption that any QoS data frame received
> from a STA/TID with an active BA session was sent to this vif as part of
> a BA.  This is not true if IFF_PROMISC is enabled and the frame was
> destined for a different peer, for example. Don't treat these frames as
> part of a BA from the sending STA.
> 
> Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
> ---
>  net/mac80211/rx.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index ee9e71b..ad39216 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -776,6 +776,10 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx)
>  	      (ack_policy == IEEE80211_QOS_CTL_ACK_POLICY_NORMAL)))
>  		goto dont_reorder;
>  
> +	/* not actually part of this BA session */
> +	if (compare_ether_addr(hdr->addr1, rx->sdata->vif.addr) != 0)
> +		goto dont_reorder;

Huh that seems strange. Shouldn't that rather check
IEEE80211_RX_RA_MATCH?

johannes


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] mac80211: QoS multicast frames have No Ack policy
  2011-11-02 10:17   ` Johannes Berg
@ 2011-11-02 17:35     ` Thomas Pedersen
  2011-11-02 17:54       ` Thomas Pedersen
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Pedersen @ 2011-11-02 17:35 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, devel, linville

On Wed, Nov 2, 2011 at 3:17 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2011-10-28 at 22:05 -0700, Thomas Pedersen wrote:
>> Previously QoS multicast frames had the Normal Acknowledgment QoS
>> control bits set. This would cause broadcast frames to be discarded by
>> peers with which we have a BA session, since their sequence number would
>> fall outside the allowed range. Set No Ack QoS control bits on multicast
>> QoS frames and filter these in de-aggregation code.
>
> I'm not sure why you would attempt to deaggregate broadcast frames but I
> guess mesh is a bit special.
>
>> @@ -770,6 +771,11 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx)
>>       if (unlikely(hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_NULLFUNC)))
>>               goto dont_reorder;
>>
>> +     /* not part of a BA session */
>> +     if (!((ack_policy == IEEE80211_QOS_CTL_ACK_POLICY_BLOCKACK) ||
>> +           (ack_policy == IEEE80211_QOS_CTL_ACK_POLICY_NORMAL)))
>> +             goto dont_reorder;
>
> Maybe ack_policy != BA && ack_policy != NORMAl would be easier to read?

OK.
>
>> --- a/net/mac80211/wme.c
>> +++ b/net/mac80211/wme.c
>> @@ -147,7 +147,8 @@ void ieee80211_set_qos_hdr(struct ieee80211_sub_if_data *sdata,
>>
>>               tid = skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK;
>>
>> -             if (unlikely(sdata->local->wifi_wme_noack_test))
>> +             if (unlikely(sdata->local->wifi_wme_noack_test) ||
>> +                 is_multicast_ether_addr(hdr->addr1))
>>                       ack_policy |= IEEE80211_QOS_CTL_ACK_POLICY_NOACK;
>
> Interestingly, this seems to have been a bug for a long time --
> 7.1.3.5.3 indicates this is supposed to be done.

I'll fix up the other patches as well and resubmit.

Thanks!
Thomas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] mac80211: QoS multicast frames have No Ack policy
  2011-11-02 17:35     ` Thomas Pedersen
@ 2011-11-02 17:54       ` Thomas Pedersen
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Pedersen @ 2011-11-02 17:54 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, devel, linville

On Wed, Nov 2, 2011 at 10:35 AM, Thomas Pedersen <thomas@cozybit.com> wrote:
> On Wed, Nov 2, 2011 at 3:17 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
>> On Fri, 2011-10-28 at 22:05 -0700, Thomas Pedersen wrote:
>>> Previously QoS multicast frames had the Normal Acknowledgment QoS
>>> control bits set. This would cause broadcast frames to be discarded by
>>> peers with which we have a BA session, since their sequence number would
>>> fall outside the allowed range. Set No Ack QoS control bits on multicast
>>> QoS frames and filter these in de-aggregation code.
>>
>> I'm not sure why you would attempt to deaggregate broadcast frames but I
>> guess mesh is a bit special.
>>

I don't think it's a mesh specific thing, broadcast frames are just
not being sent out as QoS data in the infrastructure case and
therefore not caught by the de-aggregation code. According to 7.2.2,
broadcast and multicast data frames should go out as QoS if all STAs
in the BSS are QoS STAs. I don't know if this really matters in
practice though.

>>> @@ -770,6 +771,11 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx)
>>>       if (unlikely(hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_NULLFUNC)))
>>>               goto dont_reorder;
>>>
>>> +     /* not part of a BA session */
>>> +     if (!((ack_policy == IEEE80211_QOS_CTL_ACK_POLICY_BLOCKACK) ||
>>> +           (ack_policy == IEEE80211_QOS_CTL_ACK_POLICY_NORMAL)))
>>> +             goto dont_reorder;
>>
>> Maybe ack_policy != BA && ack_policy != NORMAl would be easier to read?
>
> OK.
>>
>>> --- a/net/mac80211/wme.c
>>> +++ b/net/mac80211/wme.c
>>> @@ -147,7 +147,8 @@ void ieee80211_set_qos_hdr(struct ieee80211_sub_if_data *sdata,
>>>
>>>               tid = skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK;
>>>
>>> -             if (unlikely(sdata->local->wifi_wme_noack_test))
>>> +             if (unlikely(sdata->local->wifi_wme_noack_test) ||
>>> +                 is_multicast_ether_addr(hdr->addr1))
>>>                       ack_policy |= IEEE80211_QOS_CTL_ACK_POLICY_NOACK;
>>
>> Interestingly, this seems to have been a bug for a long time --
>> 7.1.3.5.3 indicates this is supposed to be done.
>
> I'll fix up the other patches as well and resubmit.
>
> Thanks!
> Thomas
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2011-11-02 17:54 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-29  5:05 [PATCH 1/4] mac80211: Avoid filling up mesh preq queue with redundant requests Thomas Pedersen
2011-10-29  5:05 ` [PATCH 2/4] mac80211: QoS multicast frames have No Ack policy Thomas Pedersen
2011-10-29  9:40   ` Christian Lamparter
2011-10-31  7:03     ` Thomas Pedersen
2011-10-31  9:16       ` Christian Lamparter
2011-11-02 10:17   ` Johannes Berg
2011-11-02 17:35     ` Thomas Pedersen
2011-11-02 17:54       ` Thomas Pedersen
2011-10-29  5:05 ` [PATCH 3/4] mac80211: check if frame is really part of this BA Thomas Pedersen
2011-11-02 10:19   ` Johannes Berg
2011-10-29  5:05 ` [PATCH 4/4] mac80211: Populate QoS header on mesh frames sent to gates Thomas Pedersen
2011-11-02 10:18 ` [PATCH 1/4] mac80211: Avoid filling up mesh preq queue with redundant requests Johannes Berg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).