All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ath5k: Updated padding stuff for the RX and TX side. TX side has been 100%
@ 2008-12-15 14:34 Benoit Papillault
  2008-12-15 17:06 ` Jouni Malinen
  2008-12-15 17:07 ` Bob Copeland
  0 siblings, 2 replies; 10+ messages in thread
From: Benoit Papillault @ 2008-12-15 14:34 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, ath5k-devel, Benoit PAPILLAULT

From: Benoit PAPILLAULT <benoit.papillault@free.fr>

This patch is close to the original code except that
ieee80211_get_hdrlen_from_skb() has been replaced by
ath5k_hw_get_hdrlen_from_skb() which is specific to Atheros hardware. The same
probably applies to ath9k as well.

Sign-off-by: Benoit Papillault <benoit.papillault@free.fr>
---
 drivers/net/wireless/ath5k/base.c |   33 ++++++++++++++++-----------------
 drivers/net/wireless/ath5k/base.h |   34 ++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 9d2c597..ac17960 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1192,7 +1192,7 @@ ath5k_txbuf_setup(struct ath5k_softc *sc, struct ath5k_buf *bf)
 		pktlen += info->control.hw_key->icv_len;
 	}
 	ret = ah->ah_setup_tx_desc(ah, ds, pktlen,
-		ieee80211_get_hdrlen_from_skb(skb), AR5K_PKT_TYPE_NORMAL,
+		ath5k_hw_get_hdrlen_from_skb(skb), AR5K_PKT_TYPE_NORMAL,
 		(sc->power_level * 2),
 		ieee80211_get_tx_rate(sc->hw, info)->hw_value,
 		info->control.rates[0].count, keyidx, 0, flags, 0, 0);
@@ -1667,7 +1667,7 @@ ath5k_tasklet_rx(unsigned long data)
 	struct ath5k_desc *ds;
 	int ret;
 	int hdrlen;
-	int padsize;
+	int pad;
 
 	spin_lock(&sc->rxbuflock);
 	if (list_empty(&sc->rxbuf)) {
@@ -1760,11 +1760,11 @@ accept:
 		 * bytes and we can optimize this a bit. In addition, we must
 		 * not try to remove padding from short control frames that do
 		 * not have payload. */
-		hdrlen = ieee80211_get_hdrlen_from_skb(skb);
-		padsize = hdrlen & 3;
-		if (padsize && hdrlen >= 24) {
-			memmove(skb->data + padsize, skb->data, hdrlen);
-			skb_pull(skb, padsize);
+		hdrlen = ath5k_hw_get_hdrlen_from_skb(skb);
+		if (hdrlen & 3) {
+			pad = hdrlen & 3;
+			memmove(skb->data + pad, skb->data, hdrlen);
+			skb_pull(skb, pad);
 		}
 
 		/*
@@ -1965,7 +1965,7 @@ ath5k_beacon_setup(struct ath5k_softc *sc, struct ath5k_buf *bf)
 
 	ds->ds_data = bf->skbaddr;
 	ret = ah->ah_setup_tx_desc(ah, ds, skb->len,
-			ieee80211_get_hdrlen_from_skb(skb),
+			ath5k_hw_get_hdrlen_from_skb(skb),
 			AR5K_PKT_TYPE_BEACON, (sc->power_level * 2),
 			ieee80211_get_tx_rate(sc->hw, info)->hw_value,
 			1, AR5K_TXKEYIX_INVALID,
@@ -2625,7 +2625,7 @@ ath5k_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
 	struct ath5k_buf *bf;
 	unsigned long flags;
 	int hdrlen;
-	int padsize;
+	int pad;
 
 	ath5k_debug_dump_skb(sc, skb, "TX  ", 1);
 
@@ -2636,17 +2636,16 @@ ath5k_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
 	 * the hardware expects the header padded to 4 byte boundaries
 	 * if this is not the case we add the padding after the header
 	 */
-	hdrlen = ieee80211_get_hdrlen_from_skb(skb);
-	padsize = hdrlen & 3;
-	if (padsize && hdrlen >= 24) {
-
-		if (skb_headroom(skb) < padsize) {
+	hdrlen = ath5k_hw_get_hdrlen_from_skb(skb);
+	if (hdrlen & 3) {
+		pad = hdrlen & 3;
+		if (skb_headroom(skb) < pad) {
 			ATH5K_ERR(sc, "tx hdrlen not %%4: %d not enough"
-				  " headroom to pad %d\n", hdrlen, padsize);
+				" headroom to pad %d\n", hdrlen, pad);
 			return -1;
 		}
-		skb_push(skb, padsize);
-		memmove(skb->data, skb->data+padsize, hdrlen);
+		skb_push(skb, pad);
+		memmove(skb->data, skb->data+pad, hdrlen);
 	}
 
 	spin_lock_irqsave(&sc->txbuflock, flags);
diff --git a/drivers/net/wireless/ath5k/base.h b/drivers/net/wireless/ath5k/base.h
index facc60d..85ff036 100644
--- a/drivers/net/wireless/ath5k/base.h
+++ b/drivers/net/wireless/ath5k/base.h
@@ -187,4 +187,38 @@ struct ath5k_softc {
 #define ath5k_hw_hasveol(_ah) \
 	(ath5k_hw_get_capability(_ah, AR5K_CAP_VEOL, 0, NULL) == 0)
 
+/* This formula returns the header length and is specific to Atheros hardware
+ * and differs from 802.11 standards. It has been tested using an AR5212
+ * hardware. */
+
+static inline unsigned int ath5k_hw_get_hdrlen_from_skb(
+	const struct sk_buff *skb)
+{
+	const struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+	unsigned int hdrlen;
+
+	/* Since we need to read Frame Control field, we first check that the
+	 * frames contains at least 2 bytes */
+	if (unlikely(skb->len < 2))
+		return 0;
+
+	hdrlen = 24;
+
+	/* ToDS=1 FromDS=1 frames have an additionnal Address4 field */
+	if (ieee80211_has_a4(hdr->frame_control)) {
+		hdrlen += 6; /* IEEE80211_ADDR_LEN */
+	}
+	
+	/* QoS data frames have an additionnal QoS Control field */
+	if (ieee80211_is_data_qos(hdr->frame_control)) {
+		hdrlen += IEEE80211_QOS_CTL_LEN;
+	}
+
+	/* check that skb already contains at least hdrlen bytes */
+	if (unlikely(hdrlen > skb->len))
+		return 0;
+
+	return hdrlen;
+}
+
 #endif
-- 
1.5.6.5


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

* Re: [PATCH] ath5k: Updated padding stuff for the RX and TX side. TX side has been 100%
  2008-12-15 14:34 [PATCH] ath5k: Updated padding stuff for the RX and TX side. TX side has been 100% Benoit Papillault
@ 2008-12-15 17:06 ` Jouni Malinen
  2008-12-15 21:06   ` Benoit PAPILLAULT
  2008-12-15 17:07 ` Bob Copeland
  1 sibling, 1 reply; 10+ messages in thread
From: Jouni Malinen @ 2008-12-15 17:06 UTC (permalink / raw)
  To: Benoit Papillault; +Cc: linville, linux-wireless, ath5k-devel

On Mon, Dec 15, 2008 at 03:34:58PM +0100, Benoit Papillault wrote:
> This patch is close to the original code except that
> ieee80211_get_hdrlen_from_skb() has been replaced by
> ath5k_hw_get_hdrlen_from_skb() which is specific to Atheros hardware. The same
> probably applies to ath9k as well.

Most of this is changes on how unspecified frames are handled (e.g.,
frames that are not used in IEEE 802.11). What is the use case that
justifies this type of extra complexity to be added into the driver?
Please note that there are no guarantees that all hardware revisions
behave the same as far as undocumented functionality is concerned. The
only clear case when padding is required is data frames with non-empty
frame body.

-- 
Jouni Malinen                                            PGP id EFC895FA

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

* Re: [PATCH] ath5k: Updated padding stuff for the RX and TX side. TX side has been 100%
  2008-12-15 14:34 [PATCH] ath5k: Updated padding stuff for the RX and TX side. TX side has been 100% Benoit Papillault
  2008-12-15 17:06 ` Jouni Malinen
@ 2008-12-15 17:07 ` Bob Copeland
  1 sibling, 0 replies; 10+ messages in thread
From: Bob Copeland @ 2008-12-15 17:07 UTC (permalink / raw)
  To: Benoit Papillault; +Cc: linville, linux-wireless, ath5k-devel

On Mon, Dec 15, 2008 at 9:34 AM, Benoit Papillault
<benoit.papillault@free.fr> wrote:
> From: Benoit PAPILLAULT <benoit.papillault@free.fr>
>
> This patch is close to the original code except that
> ieee80211_get_hdrlen_from_skb() has been replaced by
> ath5k_hw_get_hdrlen_from_skb() which is specific to Atheros hardware. The same
> probably applies to ath9k as well.

I for one would rather still use ieee80211_get_hdrlen_from_skb(), and the
ath5k_pad_size() helper to calculate the pad size (=0 for small control
frames).  Was there any problem with the '> 24' check?

After all we're not computing the _header_ length, just the existence
of the pad (contents of which is meaningless so can't be counted as part
of the header).

-- 
Bob Copeland %% www.bobcopeland.com

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

* Re: [PATCH] ath5k: Updated padding stuff for the RX and TX side. TX side has been 100%
  2008-12-15 17:06 ` Jouni Malinen
@ 2008-12-15 21:06   ` Benoit PAPILLAULT
  2008-12-16 15:22     ` Bob Copeland
  0 siblings, 1 reply; 10+ messages in thread
From: Benoit PAPILLAULT @ 2008-12-15 21:06 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: linville, linux-wireless, ath5k-devel

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jouni Malinen a =E9crit :
> On Mon, Dec 15, 2008 at 03:34:58PM +0100, Benoit Papillault wrote:
>> This patch is close to the original code except that
>> ieee80211_get_hdrlen_from_skb() has been replaced by
>> ath5k_hw_get_hdrlen_from_skb() which is specific to Atheros hardware=
=2E The same
>> probably applies to ath9k as well.
>=20
> Most of this is changes on how unspecified frames are handled (e.g.,
> frames that are not used in IEEE 802.11). What is the use case that
> justifies this type of extra complexity to be added into the driver?
> Please note that there are no guarantees that all hardware revisions
> behave the same as far as undocumented functionality is concerned. Th=
e
> only clear case when padding is required is data frames with non-empt=
y
> frame body.
>=20

Agreed. In fact, I wanted to know how the hardware behaves in the
general case to better understand it in the particular case of 802.11
frames. To me, the bug we have seen when receiving a ACK frame in
monitor mode was not because "ACK is a small control frame", it was
because the header length we computed was smaller than what the hardwar=
e
considered.

Another point is that we have to deal with anything on the RX side, be
it later classified as invalid or not.

As far as the hardware is concerned, I'm not working for Atheros, but I
bet that hardware guys did the same thing for all their chipsets. I
could test it on AR5416 and AR9160 if needed.

To reply to Bob as well, we need to compute the number of padded bytes =
+
the position at which the padding occurs, so proper "header" length is
required.

Just my 2 cents.

Benoit

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFJRsbZOR6EySwP7oIRArWVAJ0fjqY01Xi3qTH7DIJBE9D+c099pQCgp/u5
pHHZiKOzsU83CEecamnuZpY=3D
=3Dw10J
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH] ath5k: Updated padding stuff for the RX and TX side. TX side has been 100%
  2008-12-15 21:06   ` Benoit PAPILLAULT
@ 2008-12-16 15:22     ` Bob Copeland
  2008-12-16 15:42       ` Bob Copeland
  2008-12-17 15:55       ` John W. Linville
  0 siblings, 2 replies; 10+ messages in thread
From: Bob Copeland @ 2008-12-16 15:22 UTC (permalink / raw)
  To: Benoit PAPILLAULT; +Cc: Jouni Malinen, linville, linux-wireless, ath5k-devel

On Mon, Dec 15, 2008 at 4:06 PM, Benoit PAPILLAULT
<benoit.papillault@free.fr> wrote:

> frames. To me, the bug we have seen when receiving a ACK frame in
> monitor mode was not because "ACK is a small control frame", it was
> because the header length we computed was smaller than what the hardware
> considered.

It's because data frames have a payload and ACKs do not.

> To reply to Bob as well, we need to compute the number of padded bytes +
> the position at which the padding occurs, so proper "header" length is
> required.

Well, anyway John already picked up your earlier (better, IMHO) patch.
Now we just need to fix the tx descriptors :)

-- 
Bob Copeland %% www.bobcopeland.com

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

* Re: [PATCH] ath5k: Updated padding stuff for the RX and TX side. TX side has been 100%
  2008-12-16 15:22     ` Bob Copeland
@ 2008-12-16 15:42       ` Bob Copeland
  2008-12-17 15:55       ` John W. Linville
  1 sibling, 0 replies; 10+ messages in thread
From: Bob Copeland @ 2008-12-16 15:42 UTC (permalink / raw)
  To: Benoit PAPILLAULT; +Cc: Jouni Malinen, linville, linux-wireless, ath5k-devel

[-- Attachment #1: Type: text/plain, Size: 2709 bytes --]

On Tue, Dec 16, 2008 at 10:22 AM, Bob Copeland <bcopeland@gmail.com> wrote:
> On Mon, Dec 15, 2008 at 4:06 PM, Benoit PAPILLAULT
> <benoit.papillault@free.fr> wrote:

> Well, anyway John already picked up your earlier (better, IMHO) patch.
> Now we just need to fix the tx descriptors :)

i.e. something like the following (attached as well because gmail will
destroy it).  I only compile-tested it.

diff --git a/drivers/net/wireless/ath5k/ath5k.h
b/drivers/net/wireless/ath5k/ath5k.h
index 13df119..183ffc8 100644
--- a/drivers/net/wireless/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath5k/ath5k.h
@@ -1350,4 +1350,9 @@ static inline u32 ath5k_hw_bitswap(u32 val,
unsigned int bits)
 	return retval;
 }

+static inline int ath5k_pad_size(int hdrlen)
+{
+	return (hdrlen < 24) ? 0 : hdrlen & 3;
+}
+
 #endif
diff --git a/drivers/net/wireless/ath5k/base.c
b/drivers/net/wireless/ath5k/base.c
index f222b4a..0c3e186 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1754,8 +1754,8 @@ accept:
 		 * not try to remove padding from short control frames that do
 		 * not have payload. */
 		hdrlen = ieee80211_get_hdrlen_from_skb(skb);
-		padsize = hdrlen & 3;
-		if (padsize && hdrlen >= 24) {
+		padsize = ath5k_pad_size(hdrlen);
+		if (padsize) {
 			memmove(skb->data + padsize, skb->data, hdrlen);
 			skb_pull(skb, padsize);
 		}
@@ -2620,8 +2620,8 @@ ath5k_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
 	 * if this is not the case we add the padding after the header
 	 */
 	hdrlen = ieee80211_get_hdrlen_from_skb(skb);
-	padsize = hdrlen & 3;
-	if (padsize && hdrlen >= 24) {
+	padsize = ath5k_pad_size(hdrlen);
+	if (padsize) {

 		if (skb_headroom(skb) < padsize) {
 			ATH5K_ERR(sc, "tx hdrlen not %%4: %d not enough"
diff --git a/drivers/net/wireless/ath5k/desc.c
b/drivers/net/wireless/ath5k/desc.c
index 5e362a7..b40a928 100644
--- a/drivers/net/wireless/ath5k/desc.c
+++ b/drivers/net/wireless/ath5k/desc.c
@@ -71,7 +71,7 @@ ath5k_hw_setup_2word_tx_desc(struct ath5k_hw *ah,
struct ath5k_desc *desc,
 	/* Verify and set frame length */

 	/* remove padding we might have added before */
-	frame_len = pkt_len - (hdr_len & 3) + FCS_LEN;
+	frame_len = pkt_len - ath5k_pad_size(hdr_len) + FCS_LEN;

 	if (frame_len & ~AR5K_2W_TX_DESC_CTL0_FRAME_LEN)
 		return -EINVAL;
@@ -202,7 +202,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
 	/* Verify and set frame length */

 	/* remove padding we might have added before */
-	frame_len = pkt_len - (hdr_len & 3) + FCS_LEN;
+	frame_len = pkt_len - ath5k_pad_size(hdr_len) + FCS_LEN;

 	if (frame_len & ~AR5K_4W_TX_DESC_CTL0_FRAME_LEN)
 		return -EINVAL;


-- 
Bob Copeland %% www.bobcopeland.com

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fix_pad.patch --]
[-- Type: text/x-diff; name=fix_pad.patch, Size: 2272 bytes --]

diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
index 13df119..183ffc8 100644
--- a/drivers/net/wireless/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath5k/ath5k.h
@@ -1350,4 +1350,9 @@ static inline u32 ath5k_hw_bitswap(u32 val, unsigned int bits)
 	return retval;
 }
 
+static inline int ath5k_pad_size(int hdrlen)
+{
+	return (hdrlen < 24) ? 0 : hdrlen & 3;
+}
+
 #endif
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index f222b4a..0c3e186 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1754,8 +1754,8 @@ accept:
 		 * not try to remove padding from short control frames that do
 		 * not have payload. */
 		hdrlen = ieee80211_get_hdrlen_from_skb(skb);
-		padsize = hdrlen & 3;
-		if (padsize && hdrlen >= 24) {
+		padsize = ath5k_pad_size(hdrlen);
+		if (padsize) {
 			memmove(skb->data + padsize, skb->data, hdrlen);
 			skb_pull(skb, padsize);
 		}
@@ -2620,8 +2620,8 @@ ath5k_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
 	 * if this is not the case we add the padding after the header
 	 */
 	hdrlen = ieee80211_get_hdrlen_from_skb(skb);
-	padsize = hdrlen & 3;
-	if (padsize && hdrlen >= 24) {
+	padsize = ath5k_pad_size(hdrlen);
+	if (padsize) {
 
 		if (skb_headroom(skb) < padsize) {
 			ATH5K_ERR(sc, "tx hdrlen not %%4: %d not enough"
diff --git a/drivers/net/wireless/ath5k/desc.c b/drivers/net/wireless/ath5k/desc.c
index 5e362a7..b40a928 100644
--- a/drivers/net/wireless/ath5k/desc.c
+++ b/drivers/net/wireless/ath5k/desc.c
@@ -71,7 +71,7 @@ ath5k_hw_setup_2word_tx_desc(struct ath5k_hw *ah, struct ath5k_desc *desc,
 	/* Verify and set frame length */
 
 	/* remove padding we might have added before */
-	frame_len = pkt_len - (hdr_len & 3) + FCS_LEN;
+	frame_len = pkt_len - ath5k_pad_size(hdr_len) + FCS_LEN;
 
 	if (frame_len & ~AR5K_2W_TX_DESC_CTL0_FRAME_LEN)
 		return -EINVAL;
@@ -202,7 +202,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
 	/* Verify and set frame length */
 
 	/* remove padding we might have added before */
-	frame_len = pkt_len - (hdr_len & 3) + FCS_LEN;
+	frame_len = pkt_len - ath5k_pad_size(hdr_len) + FCS_LEN;
 
 	if (frame_len & ~AR5K_4W_TX_DESC_CTL0_FRAME_LEN)
 		return -EINVAL;

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

* Re: [PATCH] ath5k: Updated padding stuff for the RX and TX side. TX side has been 100%
  2008-12-16 15:22     ` Bob Copeland
  2008-12-16 15:42       ` Bob Copeland
@ 2008-12-17 15:55       ` John W. Linville
  2008-12-17 17:40         ` Bob Copeland
  1 sibling, 1 reply; 10+ messages in thread
From: John W. Linville @ 2008-12-17 15:55 UTC (permalink / raw)
  To: Bob Copeland
  Cc: Benoit PAPILLAULT, Jouni Malinen, linux-wireless, ath5k-devel

On Tue, Dec 16, 2008 at 10:22:39AM -0500, Bob Copeland wrote:
> On Mon, Dec 15, 2008 at 4:06 PM, Benoit PAPILLAULT
> <benoit.papillault@free.fr> wrote:
> 
> > frames. To me, the bug we have seen when receiving a ACK frame in
> > monitor mode was not because "ACK is a small control frame", it was
> > because the header length we computed was smaller than what the hardware
> > considered.
> 
> It's because data frames have a payload and ACKs do not.
> 
> > To reply to Bob as well, we need to compute the number of padded bytes +
> > the position at which the padding occurs, so proper "header" length is
> > required.
> 
> Well, anyway John already picked up your earlier (better, IMHO) patch.
> Now we just need to fix the tx descriptors :)

Based on that comment, I'm dropping this version of the patch.
Feel free to submit additional patches to implement whatever might
be missing now.

John
-- 
John W. Linville		Linux should be at the core
linville@tuxdriver.com			of your literate lifestyle.

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

* Re: [PATCH] ath5k: Updated padding stuff for the RX and TX side. TX side has been 100%
  2008-12-17 15:55       ` John W. Linville
@ 2008-12-17 17:40         ` Bob Copeland
  2008-12-17 18:35           ` John W. Linville
  0 siblings, 1 reply; 10+ messages in thread
From: Bob Copeland @ 2008-12-17 17:40 UTC (permalink / raw)
  To: John W. Linville
  Cc: Benoit PAPILLAULT, Jouni Malinen, linux-wireless, ath5k-devel

On Wed, Dec 17, 2008 at 10:55 AM, John W. Linville
<linville@tuxdriver.com> wrote:
> On Tue, Dec 16, 2008 at 10:22:39AM -0500, Bob Copeland wrote:
>> Well, anyway John already picked up your earlier (better, IMHO) patch.
>> Now we just need to fix the tx descriptors :)
>
> Based on that comment, I'm dropping this version of the patch.
> Feel free to submit additional patches to implement whatever might
> be missing now.

Okay. I think the one you took before, plus my diff should be sufficient.
Do you want me to submit a proper patch that just fixes the TX, or should
we regroup and combine both up into a single patch on our side?

Either way I'll give it a little testing tonight to make sure we didn't
miss anything.

-- 
Bob Copeland %% www.bobcopeland.com

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

* Re: [PATCH] ath5k: Updated padding stuff for the RX and TX side. TX side has been 100%
  2008-12-17 17:40         ` Bob Copeland
@ 2008-12-17 18:35           ` John W. Linville
  2008-12-19 15:53             ` Bob Copeland
  0 siblings, 1 reply; 10+ messages in thread
From: John W. Linville @ 2008-12-17 18:35 UTC (permalink / raw)
  To: Bob Copeland
  Cc: Benoit PAPILLAULT, Jouni Malinen, linux-wireless, ath5k-devel

On Wed, Dec 17, 2008 at 12:40:56PM -0500, Bob Copeland wrote:
> On Wed, Dec 17, 2008 at 10:55 AM, John W. Linville
> <linville@tuxdriver.com> wrote:
> > On Tue, Dec 16, 2008 at 10:22:39AM -0500, Bob Copeland wrote:
> >> Well, anyway John already picked up your earlier (better, IMHO) patch.
> >> Now we just need to fix the tx descriptors :)
> >
> > Based on that comment, I'm dropping this version of the patch.
> > Feel free to submit additional patches to implement whatever might
> > be missing now.
> 
> Okay. I think the one you took before, plus my diff should be sufficient.
> Do you want me to submit a proper patch that just fixes the TX, or should
> we regroup and combine both up into a single patch on our side?
> 
> Either way I'll give it a little testing tonight to make sure we didn't
> miss anything.

Just your new patch will be sufficient I think...?

Thanks,

John
-- 
John W. Linville		Linux should be at the core
linville@tuxdriver.com			of your literate lifestyle.

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

* Re: [PATCH] ath5k: Updated padding stuff for the RX and TX side. TX side has been 100%
  2008-12-17 18:35           ` John W. Linville
@ 2008-12-19 15:53             ` Bob Copeland
  0 siblings, 0 replies; 10+ messages in thread
From: Bob Copeland @ 2008-12-19 15:53 UTC (permalink / raw)
  To: John W. Linville
  Cc: Benoit PAPILLAULT, Jouni Malinen, linux-wireless, ath5k-devel

On Wed, 17 Dec 2008 13:35:27 -0500, John W. Linville wrote
> > Okay. I think the one you took before, plus my diff should be sufficient.
> > Do you want me to submit a proper patch that just fixes the TX, or should
> > we regroup and combine both up into a single patch on our side?
> 
> Just your new patch will be sufficient I think...?

Ok here you go.  In fact we won't trigger this case very often since
those packets are usually sent by hardware, but anyway we should be
consistent everywhere.  This is on top of 28bd3684fb5.

>From 853eff110f7aefaf150b189ac8a87b77fa9447c5 Mon Sep 17 00:00:00 2001
From: Bob Copeland <me@bobcopeland.com>
Date: Thu, 18 Dec 2008 23:23:05 -0500
Subject: [PATCH] ath5k: correct packet length in tx descriptors

Packet length calculation (which includes frame check sequence)
should take into account whether we add a pad field or not.
Extract the calculation into a helper and use it in both places.

Changes to desc.c
Changes-licensed-under: ISC

Changes to ath5k.h, base.c
Changes-licensed-under: 3-Clause-BSD

Signed-off-by: Bob Copeland <me@bobcopeland.com>
---
 drivers/net/wireless/ath5k/ath5k.h |    5 +++++
 drivers/net/wireless/ath5k/base.c  |    8 ++++----
 drivers/net/wireless/ath5k/desc.c  |    4 ++--
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath5k/ath5k.h
b/drivers/net/wireless/ath5k/ath5k.h
index 13df119..183ffc8 100644
--- a/drivers/net/wireless/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath5k/ath5k.h
@@ -1350,4 +1350,9 @@ static inline u32 ath5k_hw_bitswap(u32 val, unsigned int
bits)
 	return retval;
 }
 
+static inline int ath5k_pad_size(int hdrlen)
+{
+	return (hdrlen < 24) ? 0 : hdrlen & 3;
+}
+
 #endif
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 9d2c597..40c0155 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1761,8 +1761,8 @@ accept:
 		 * not try to remove padding from short control frames that do
 		 * not have payload. */
 		hdrlen = ieee80211_get_hdrlen_from_skb(skb);
-		padsize = hdrlen & 3;
-		if (padsize && hdrlen >= 24) {
+		padsize = ath5k_pad_size(hdrlen);
+		if (padsize) {
 			memmove(skb->data + padsize, skb->data, hdrlen);
 			skb_pull(skb, padsize);
 		}
@@ -2637,8 +2637,8 @@ ath5k_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
 	 * if this is not the case we add the padding after the header
 	 */
 	hdrlen = ieee80211_get_hdrlen_from_skb(skb);
-	padsize = hdrlen & 3;
-	if (padsize && hdrlen >= 24) {
+	padsize = ath5k_pad_size(hdrlen);
+	if (padsize) {
 
 		if (skb_headroom(skb) < padsize) {
 			ATH5K_ERR(sc, "tx hdrlen not %%4: %d not enough"
diff --git a/drivers/net/wireless/ath5k/desc.c b/drivers/net/wireless/ath5k/desc.c
index 5e362a7..b40a928 100644
--- a/drivers/net/wireless/ath5k/desc.c
+++ b/drivers/net/wireless/ath5k/desc.c
@@ -71,7 +71,7 @@ ath5k_hw_setup_2word_tx_desc(struct ath5k_hw *ah, struct
ath5k_desc *desc,
 	/* Verify and set frame length */
 
 	/* remove padding we might have added before */
-	frame_len = pkt_len - (hdr_len & 3) + FCS_LEN;
+	frame_len = pkt_len - ath5k_pad_size(hdr_len) + FCS_LEN;
 
 	if (frame_len & ~AR5K_2W_TX_DESC_CTL0_FRAME_LEN)
 		return -EINVAL;
@@ -202,7 +202,7 @@ static int ath5k_hw_setup_4word_tx_desc(struct ath5k_hw *ah,
 	/* Verify and set frame length */
 
 	/* remove padding we might have added before */
-	frame_len = pkt_len - (hdr_len & 3) + FCS_LEN;
+	frame_len = pkt_len - ath5k_pad_size(hdr_len) + FCS_LEN;
 
 	if (frame_len & ~AR5K_4W_TX_DESC_CTL0_FRAME_LEN)
 		return -EINVAL;
-- 
1.6.0.4

-- 
Bob Copeland %% www.bobcopeland.com



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

end of thread, other threads:[~2008-12-19 15:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-12-15 14:34 [PATCH] ath5k: Updated padding stuff for the RX and TX side. TX side has been 100% Benoit Papillault
2008-12-15 17:06 ` Jouni Malinen
2008-12-15 21:06   ` Benoit PAPILLAULT
2008-12-16 15:22     ` Bob Copeland
2008-12-16 15:42       ` Bob Copeland
2008-12-17 15:55       ` John W. Linville
2008-12-17 17:40         ` Bob Copeland
2008-12-17 18:35           ` John W. Linville
2008-12-19 15:53             ` Bob Copeland
2008-12-15 17:07 ` Bob Copeland

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.