[PATCH V2 1/4] mwl8k: Do not stop tx queues

[PATCH V2 1/4] mwl8k: Do not stop tx queues

[Nishant Sarmukadam]

From: Pradeep Nemavat <pnemavat@marvell.com>

This is in preparation to support life time expiry of packets in the
hardware to avoid head-of-line blocking where a slow client can
hog a tx queue and affect the traffic to a faster client from the same
queue. Time stamp the packets in driver to allow dropping them in the
hardware if they are queued for more than 500ms.

If queues are stopped, packets will be queued up outside the driver.
Since we will be able to timestamp the packets only after they hit the
driver, the timestamp will be less accurate since we cannot consider
the time the packets spent in queues outside the driver. With this commit,
to achieve accurate timestamping, the tx queues will never be stopped.
Now, we need to be prepared for a situation where packets hit the driver
even after the tx queues are full. Drop all such packets in the driver itself.

Signed-off-by: Pradeep Nemavat <pnemavat@marvell.com>
Signed-off-by: Nishant Sarmukadam <nishants@marvell.com>
---
 drivers/net/wireless/mwl8k.c |   20 +++++++-------------
 1 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/mwl8k.c b/drivers/net/wireless/mwl8k.c
index ebaae35..7968301 100644
--- a/drivers/net/wireless/mwl8k.c
+++ b/drivers/net/wireless/mwl8k.c
@@ -1666,10 +1666,6 @@ mwl8k_txq_reclaim(struct ieee80211_hw *hw, int index, int limit, int force)
 		processed++;
 	}
 
-	if (index < MWL8K_TX_WMM_QUEUES && processed && priv->radio_on &&
-	    !mutex_is_locked(&priv->fw_mutex))
-		ieee80211_wake_queue(hw, index);
-
 	return processed;
 }
 
@@ -1951,13 +1947,14 @@ mwl8k_txq_xmit(struct ieee80211_hw *hw, int index, struct sk_buff *skb)
 
 	txq = priv->txq + index;
 
-	if (index >= MWL8K_TX_WMM_QUEUES && txq->len >= MWL8K_TX_DESCS) {
-		/* This is the case in which the tx packet is destined for an
-		 * AMPDU queue and that AMPDU queue is full.  Because we don't
-		 * start and stop the AMPDU queues, we must drop these packets.
-		 */
-		dev_kfree_skb(skb);
+	if (txq->len >= MWL8K_TX_DESCS) {
+		if (start_ba_session) {
+			spin_lock(&priv->stream_lock);
+			mwl8k_remove_stream(hw, stream);
+			spin_unlock(&priv->stream_lock);
+		}
 		spin_unlock_bh(&priv->tx_lock);
+		dev_kfree_skb(skb);
 		return;
 	}
 
@@ -1985,9 +1982,6 @@ mwl8k_txq_xmit(struct ieee80211_hw *hw, int index, struct sk_buff *skb)
 	if (txq->tail == MWL8K_TX_DESCS)
 		txq->tail = 0;
 
-	if (txq->head == txq->tail && index < MWL8K_TX_WMM_QUEUES)
-		ieee80211_stop_queue(hw, index);
-
 	mwl8k_tx_start(priv);
 
 	spin_unlock_bh(&priv->tx_lock);
-- 
1.6.0.3

[PATCH V2 2/4] mwl8k: Add timestamp information for tx packets

[Nishant Sarmukadam]

From: Pradeep Nemavat <pnemavat@marvell.com>

Timestamp tx packets using a HW micro-second timer.
This timestamp will be compared to the current timestamp
in the hardware and if the difference is greater than 500ms,
the packet will be dropped.

Signed-off-by: Pradeep Nemavat <pnemavat@marvell.com>
Signed-off-by: Nishant Sarmukadam <nishants@marvell.com>
---
 drivers/net/wireless/mwl8k.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/mwl8k.c b/drivers/net/wireless/mwl8k.c
index 7968301..aca0139 100644
--- a/drivers/net/wireless/mwl8k.c
+++ b/drivers/net/wireless/mwl8k.c
@@ -1792,6 +1792,14 @@ static inline void mwl8k_tx_count_packet(struct ieee80211_sta *sta, u8 tid)
 		tx_stats->pkts++;
 }
 
+/* HW micro second timer register
+ * located at offset 0xA600. This
+ * will be used to timestamp tx
+ * packets.
+ */
+
+#define	MWL8K_HW_TIMER_REGISTER	(priv->regs + 0xA600)
+
 static void
 mwl8k_txq_xmit(struct ieee80211_hw *hw, int index, struct sk_buff *skb)
 {
@@ -1972,6 +1980,10 @@ mwl8k_txq_xmit(struct ieee80211_hw *hw, int index, struct sk_buff *skb)
 		tx->peer_id = MWL8K_STA(tx_info->control.sta)->peer_id;
 	else
 		tx->peer_id = 0;
+
+	if (priv->ap_fw)
+		tx->timestamp = cpu_to_le32(ioread32(MWL8K_HW_TIMER_REGISTER));
+
 	wmb();
 	tx->status = cpu_to_le32(MWL8K_TXD_STATUS_FW_OWNED | txstatus);
 
-- 
1.6.0.3

[PATCH V2 3/4] mwl8k: Reserve buffers for tx management frames

[Nishant Sarmukadam]

Since queues are not stopped anymore, management frames would be
dropped if the corresponding tx queue is full.
This can cause issues say when we want to setup an ampdu stream and
action frames i.e addba requests keep getting dropped frequently.
Fix this by reserving some buffers to allow management frames to
go through in queue full conditions.

Signed-off-by: Nishant Sarmukadam <nishants@marvell.com>
Signed-off-by: Pradeep Nemavat <pnemavat@marvell.com>
---
 drivers/net/wireless/mwl8k.c |   30 ++++++++++++++++++++++--------
 1 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/mwl8k.c b/drivers/net/wireless/mwl8k.c
index aca0139..82cd725 100644
--- a/drivers/net/wireless/mwl8k.c
+++ b/drivers/net/wireless/mwl8k.c
@@ -1818,6 +1818,7 @@ mwl8k_txq_xmit(struct ieee80211_hw *hw, int index, struct sk_buff *skb)
 	u8 tid = 0;
 	struct mwl8k_ampdu_stream *stream = NULL;
 	bool start_ba_session = false;
+	bool mgmtframe = false;
 	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)skb->data;
 
 	wh = (struct ieee80211_hdr *)skb->data;
@@ -1826,6 +1827,9 @@ mwl8k_txq_xmit(struct ieee80211_hw *hw, int index, struct sk_buff *skb)
 	else
 		qos = 0;
 
+	if (ieee80211_is_mgmt(wh->frame_control))
+		mgmtframe = true;
+
 	if (priv->ap_fw)
 		mwl8k_encapsulate_tx_frame(skb);
 	else
@@ -1955,15 +1959,25 @@ mwl8k_txq_xmit(struct ieee80211_hw *hw, int index, struct sk_buff *skb)
 
 	txq = priv->txq + index;
 
-	if (txq->len >= MWL8K_TX_DESCS) {
-		if (start_ba_session) {
-			spin_lock(&priv->stream_lock);
-			mwl8k_remove_stream(hw, stream);
-			spin_unlock(&priv->stream_lock);
+	/* Mgmt frames that go out frequently are probe responses.
+	 * Other mgmt frames got out relatively infrequently.
+	 * Hence reserve 2 buffers so that other mgmt frames do not get
+	 * dropped due to an already queued probe response in one of
+	 * the reserved buffers.
+	 */
+
+	if (txq->len >= (MWL8K_TX_DESCS - 2)) {
+		if ((mgmtframe == false) ||
+			(txq->len == MWL8K_TX_DESCS)) {
+			if (start_ba_session) {
+				spin_lock(&priv->stream_lock);
+				mwl8k_remove_stream(hw, stream);
+				spin_unlock(&priv->stream_lock);
+			}
+			spin_unlock_bh(&priv->tx_lock);
+			dev_kfree_skb(skb);
+			return;
 		}
-		spin_unlock_bh(&priv->tx_lock);
-		dev_kfree_skb(skb);
-		return;
 	}
 
 	BUG_ON(txq->skb[txq->tail] != NULL);
-- 
1.6.0.3

[PATCH V2 4/4] mwl8k: Enable life time expiry for tx packets in the hardware

[Nishant Sarmukadam]

Tell the firmware to enable the life time expiry of tx packets
in the hardware. The hardware will now refer to the timestamp
in every tx packet and decide whether the packet needs to be
dropped or transmitted.

Signed-off-by: Nishant Sarmukadam <nishants@marvell.com>
Signed-off-by: Pradeep Nemavat <pnemavat@marvell.com>
---
 drivers/net/wireless/mwl8k.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/mwl8k.c b/drivers/net/wireless/mwl8k.c
index 82cd725..7db4bb9 100644
--- a/drivers/net/wireless/mwl8k.c
+++ b/drivers/net/wireless/mwl8k.c
@@ -2502,7 +2502,8 @@ static int mwl8k_cmd_set_hw_spec(struct ieee80211_hw *hw)
 
 	cmd->flags = cpu_to_le32(MWL8K_SET_HW_SPEC_FLAG_HOST_DECR_MGMT |
 				 MWL8K_SET_HW_SPEC_FLAG_HOSTFORM_PROBERESP |
-				 MWL8K_SET_HW_SPEC_FLAG_HOSTFORM_BEACON);
+				 MWL8K_SET_HW_SPEC_FLAG_HOSTFORM_BEACON |
+				 MWL8K_SET_HW_SPEC_FLAG_ENABLE_LIFE_TIME_EXPIRY);
 	cmd->num_tx_desc_per_queue = cpu_to_le32(MWL8K_TX_DESCS);
 	cmd->total_rxd = cpu_to_le32(MWL8K_RX_DESCS);
 
-- 
1.6.0.3

Re: [PATCH V2 1/4] mwl8k: Do not stop tx queues

[Lennert Buytenhek]

On Tue, Apr 19, 2011 at 10:16:19PM +0530, Nishant Sarmukadam wrote:

> From: Pradeep Nemavat <pnemavat@marvell.com>
> 
> This is in preparation to support life time expiry of packets in the
> hardware to avoid head-of-line blocking where a slow client can
> hog a tx queue and affect the traffic to a faster client from the same
> queue. Time stamp the packets in driver to allow dropping them in the
> hardware if they are queued for more than 500ms.
> 
> If queues are stopped, packets will be queued up outside the driver.
> Since we will be able to timestamp the packets only after they hit the
> driver, the timestamp will be less accurate since we cannot consider
> the time the packets spent in queues outside the driver. With this commit,
> to achieve accurate timestamping, the tx queues will never be stopped.
> Now, we need to be prepared for a situation where packets hit the driver
> even after the tx queues are full. Drop all such packets in the driver itself.

It might be worth adding that after this patch, the mac80211 queues are
only stopped when firmware commands are executing (and when the interface
is down'ed).

Also, it's somewhat doubtful that this should really be called HOL
blocking.

Apart from those, OK with the explanation.

Re: [PATCH V2 2/4] mwl8k: Add timestamp information for tx packets

[Lennert Buytenhek]

On Tue, Apr 19, 2011 at 10:16:20PM +0530, Nishant Sarmukadam wrote:

> From: Pradeep Nemavat <pnemavat@marvell.com>
> 
> Timestamp tx packets using a HW micro-second timer.
> This timestamp will be compared to the current timestamp
> in the hardware and if the difference is greater than 500ms,
> the packet will be dropped.
> 
> Signed-off-by: Pradeep Nemavat <pnemavat@marvell.com>
> Signed-off-by: Nishant Sarmukadam <nishants@marvell.com>
> ---
>  drivers/net/wireless/mwl8k.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/wireless/mwl8k.c b/drivers/net/wireless/mwl8k.c
> index 7968301..aca0139 100644
> --- a/drivers/net/wireless/mwl8k.c
> +++ b/drivers/net/wireless/mwl8k.c
> @@ -1792,6 +1792,14 @@ static inline void mwl8k_tx_count_packet(struct ieee80211_sta *sta, u8 tid)
>  		tx_stats->pkts++;
>  }
>  
> +/* HW micro second timer register
> + * located at offset 0xA600. This
> + * will be used to timestamp tx
> + * packets.
> + */
> +
> +#define	MWL8K_HW_TIMER_REGISTER	(priv->regs + 0xA600)

1. Ugly implicit 'priv' argument here. :-(

   Please just do something like:

 #define  MWL8K_A2H_INT_RX_READY                  (1 << 1)
 #define  MWL8K_A2H_INT_TX_DONE                   (1 << 0)
+
+/* Misc registers */
+#define MWL8K_HW_TIMER_REGISTER		0x0000a600

 #define MWL8K_A2H_EVENTS        (MWL8K_A2H_INT_DUMMY | \
                                  MWL8K_A2H_INT_CHNL_SWITCHED | \

   and then just use it as ioread32(priv->regs + MWL8K_HW_TIMER_REGISTER).


2. Does this register exist on 8687/superfly3, or is it SJ only?

Re: [PATCH V2 3/4] mwl8k: Reserve buffers for tx management frames

[Lennert Buytenhek]

On Tue, Apr 19, 2011 at 10:16:21PM +0530, Nishant Sarmukadam wrote:

> -	if (txq->len >= MWL8K_TX_DESCS) {
> -		if (start_ba_session) {
> -			spin_lock(&priv->stream_lock);
> -			mwl8k_remove_stream(hw, stream);
> -			spin_unlock(&priv->stream_lock);
> +	/* Mgmt frames that go out frequently are probe responses.
> +	 * Other mgmt frames got out relatively infrequently.
> +	 * Hence reserve 2 buffers so that other mgmt frames do not get
> +	 * dropped due to an already queued probe response in one of
> +	 * the reserved buffers.
> +	 */

Comment formatting..


> +	if (txq->len >= (MWL8K_TX_DESCS - 2)) {
> +		if ((mgmtframe == false) ||
> +			(txq->len == MWL8K_TX_DESCS)) {
> +			if (start_ba_session) {
> +				spin_lock(&priv->stream_lock);
> +				mwl8k_remove_stream(hw, stream);
> +				spin_unlock(&priv->stream_lock);
> +			}

..and lots of superfluous parentheses here:

	txq->len >= MWL8K_TX_DESCS - 2
	mgmtframe == false
	txq->len == MWL8K_TX_DESCS

Re: [PATCH V2 2/4] mwl8k: Add timestamp information for tx packets

[Nishant Sarmukadam]

On Thu, 2011-04-21 at 00:42 -0700, Lennert Buytenhek wrote:
> On Tue, Apr 19, 2011 at 10:16:20PM +0530, Nishant Sarmukadam wrote:
> 
> > From: Pradeep Nemavat <pnemavat@marvell.com>
> > 
> > Timestamp tx packets using a HW micro-second timer.
> > This timestamp will be compared to the current timestamp
> > in the hardware and if the difference is greater than 500ms,
> > the packet will be dropped.
> > 
> > Signed-off-by: Pradeep Nemavat <pnemavat@marvell.com>
> > Signed-off-by: Nishant Sarmukadam <nishants@marvell.com>
> > ---
> >  drivers/net/wireless/mwl8k.c |   12 ++++++++++++
> >  1 files changed, 12 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/mwl8k.c b/drivers/net/wireless/mwl8k.c
> > index 7968301..aca0139 100644
> > --- a/drivers/net/wireless/mwl8k.c
> > +++ b/drivers/net/wireless/mwl8k.c
> > @@ -1792,6 +1792,14 @@ static inline void mwl8k_tx_count_packet(struct ieee80211_sta *sta, u8 tid)
> >  		tx_stats->pkts++;
> >  }
> >  
> > +/* HW micro second timer register
> > + * located at offset 0xA600. This
> > + * will be used to timestamp tx
> > + * packets.
> > + */
> > +
> > +#define	MWL8K_HW_TIMER_REGISTER	(priv->regs + 0xA600)
> 
> 1. Ugly implicit 'priv' argument here. :-(
> 
>    Please just do something like:
> 
>  #define  MWL8K_A2H_INT_RX_READY                  (1 << 1)
>  #define  MWL8K_A2H_INT_TX_DONE                   (1 << 0)
> +
> +/* Misc registers */
> +#define MWL8K_HW_TIMER_REGISTER		0x0000a600
> 
>  #define MWL8K_A2H_EVENTS        (MWL8K_A2H_INT_DUMMY | \
>                                   MWL8K_A2H_INT_CHNL_SWITCHED | \
> 
>    and then just use it as ioread32(priv->regs + MWL8K_HW_TIMER_REGISTER).
> 
> 
Ok, will send out patch set V3 incorporating this change. 

> 2. Does this register exist on 8687/superfly3, or is it SJ only?

Yes, this does exist on 8687 and superfly3 as well. 

Re: [PATCH V2 3/4] mwl8k: Reserve buffers for tx management frames

[Nishant Sarmukadam]

On Thu, 2011-04-21 at 00:44 -0700, Lennert Buytenhek wrote:
> On Tue, Apr 19, 2011 at 10:16:21PM +0530, Nishant Sarmukadam wrote:
> 
> > -	if (txq->len >= MWL8K_TX_DESCS) {
> > -		if (start_ba_session) {
> > -			spin_lock(&priv->stream_lock);
> > -			mwl8k_remove_stream(hw, stream);
> > -			spin_unlock(&priv->stream_lock);
> > +	/* Mgmt frames that go out frequently are probe responses.
> > +	 * Other mgmt frames got out relatively infrequently.
> > +	 * Hence reserve 2 buffers so that other mgmt frames do not get
> > +	 * dropped due to an already queued probe response in one of
> > +	 * the reserved buffers.
> > +	 */
> 
> Comment formatting..
> 
> 
> > +	if (txq->len >= (MWL8K_TX_DESCS - 2)) {
> > +		if ((mgmtframe == false) ||
> > +			(txq->len == MWL8K_TX_DESCS)) {
> > +			if (start_ba_session) {
> > +				spin_lock(&priv->stream_lock);
> > +				mwl8k_remove_stream(hw, stream);
> > +				spin_unlock(&priv->stream_lock);
> > +			}
> 
> ..and lots of superfluous parentheses here:
> 
> 	txq->len >= MWL8K_TX_DESCS - 2
> 	mgmtframe == false
> 	txq->len == MWL8K_TX_DESCS

I will address both these comments in patch set V3.