All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.