From: Johannes Berg <johannes@sipsolutions.net>
To: John Linville <linville@tuxdriver.com>
Cc: Linux Wireless <linux-wireless@vger.kernel.org>
Subject: [PATCH] mac80211: clarify alignment docs, fix up alignment
Date: Wed, 07 Jan 2009 00:26:10 +0100 [thread overview]
Message-ID: <1231284370.22613.0.camel@johannes> (raw)
Not all drivers are capable of passing properly aligned frames,
in particular with mesh networking no hardware will support
completely aligning it correctly.
This patch adds code to align the data payload to a 4-byte
boundary in memory for those platforms that require this, or
when CONFIG_MAC80211_DEBUG_PACKET_ALIGNMENT is set.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
Documentation/DocBook/mac80211.tmpl | 4 -
net/mac80211/rx.c | 110 +++++++++++++++++++++++++-----------
2 files changed, 79 insertions(+), 35 deletions(-)
--- wireless-testing.orig/Documentation/DocBook/mac80211.tmpl 2009-01-06 23:26:47.000000000 +0100
+++ wireless-testing/Documentation/DocBook/mac80211.tmpl 2009-01-06 23:26:51.000000000 +0100
@@ -164,8 +164,8 @@ usage should require reading the full do
!Pinclude/net/mac80211.h Frame format
</sect1>
<sect1>
- <title>Alignment issues</title>
- <para>TBD</para>
+ <title>Packet alignment</title>
+!Pnet/mac80211/rx.c Packet alignment
</sect1>
<sect1>
<title>Calling into mac80211 from interrupts</title>
--- wireless-testing.orig/net/mac80211/rx.c 2009-01-06 23:18:13.000000000 +0100
+++ wireless-testing/net/mac80211/rx.c 2009-01-07 00:14:34.000000000 +0100
@@ -102,7 +102,7 @@ ieee80211_rx_radiotap_len(struct ieee802
return len;
}
-/**
+/*
* ieee80211_add_rx_radiotap_header - add radiotap header
*
* add a radiotap header containing all the fields which the hardware provided.
@@ -372,39 +372,50 @@ static void ieee80211_parse_qos(struct i
rx->skb->priority = (tid > 7) ? 0 : tid;
}
-static void ieee80211_verify_ip_alignment(struct ieee80211_rx_data *rx)
+/**
+ * DOC: Packet alignment
+ *
+ * Drivers always need to pass packets that are aligned to two-byte boundaries
+ * to the stack.
+ *
+ * Additionally, should, if possible, align the payload data in a way that
+ * guarantees that the contained IP header is aligned to a four-byte
+ * boundary. In the case of regular frames, this simply means aligning the
+ * payload to a four-byte boundary (because either the IP header is directly
+ * contained, or IV/RFC1042 headers that have a length divisible by four are
+ * in front of it).
+ *
+ * With A-MSDU frames, however, the payload data address must yield two modulo
+ * four because there are 14-byte 802.3 headers within the A-MSDU frames that
+ * push the IP header further back to a multiple of four again. Thankfully, the
+ * specs were sane enough this time around to require padding each A-MSDU
+ * subframe to a length that is a multiple of four.
+ *
+ * Padding like Atheros hardware adds which is inbetween the 802.11 header and
+ * the payload is not supported, the driver is required to move the 802.11
+ * header to be directly in front of the payload in that case.
+ */
+static void ieee80211_verify_alignment(struct ieee80211_rx_data *rx)
{
-#ifdef CONFIG_MAC80211_DEBUG_PACKET_ALIGNMENT
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)rx->skb->data;
int hdrlen;
+#ifndef CONFIG_MAC80211_DEBUG_PACKET_ALIGNMENT
+ return;
+#endif
+
+ if (WARN_ONCE((unsigned long)rx->skb->data & 1,
+ "unaligned packet at 0x%p\n", rx->skb->data))
+ return;
+
if (!ieee80211_is_data_present(hdr->frame_control))
return;
- /*
- * Drivers are required to align the payload data in a way that
- * guarantees that the contained IP header is aligned to a four-
- * byte boundary. In the case of regular frames, this simply means
- * aligning the payload to a four-byte boundary (because either
- * the IP header is directly contained, or IV/RFC1042 headers that
- * have a length divisible by four are in front of it.
- *
- * With A-MSDU frames, however, the payload data address must
- * yield two modulo four because there are 14-byte 802.3 headers
- * within the A-MSDU frames that push the IP header further back
- * to a multiple of four again. Thankfully, the specs were sane
- * enough this time around to require padding each A-MSDU subframe
- * to a length that is a multiple of four.
- *
- * Padding like atheros hardware adds which is inbetween the 802.11
- * header and the payload is not supported, the driver is required
- * to move the 802.11 header further back in that case.
- */
hdrlen = ieee80211_hdrlen(hdr->frame_control);
if (rx->flags & IEEE80211_RX_AMSDU)
hdrlen += ETH_HLEN;
- WARN_ON_ONCE(((unsigned long)(rx->skb->data + hdrlen)) & 3);
-#endif
+ WARN_ONCE(((unsigned long)(rx->skb->data + hdrlen)) & 3,
+ "unaligned IP payload at 0x%p\n", rx->skb->data + hdrlen);
}
@@ -1279,10 +1290,37 @@ ieee80211_deliver_skb(struct ieee80211_r
}
if (skb) {
- /* deliver to local stack */
- skb->protocol = eth_type_trans(skb, dev);
- memset(skb->cb, 0, sizeof(skb->cb));
- netif_rx(skb);
+ int align __maybe_unused;
+
+#if defined(CONFIG_MAC80211_DEBUG_PACKET_ALIGNMENT) || !defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+ /*
+ * 'align' will only take the values 0 or 2 here
+ * since all frames are required to be aligned
+ * to 2-byte boundaries when being passed to
+ * mac80211. That also explains the __skb_push()
+ * below.
+ */
+ align = (unsigned long)skb->data & 4;
+ if (align) {
+ if (WARN_ON(skb_headroom(skb) < 3)) {
+ dev_kfree_skb(skb);
+ skb = NULL;
+ } else {
+ u8 *data = skb->data;
+ size_t len = skb->len;
+ u8 *new = __skb_push(skb, align);
+ memmove(new, data, len);
+ __skb_trim(skb, len);
+ }
+ }
+#endif
+
+ if (skb) {
+ /* deliver to local stack */
+ skb->protocol = eth_type_trans(skb, dev);
+ memset(skb->cb, 0, sizeof(skb->cb));
+ netif_rx(skb);
+ }
}
if (xmit_skb) {
@@ -1352,14 +1390,20 @@ ieee80211_rx_h_amsdu(struct ieee80211_rx
if (remaining <= subframe_len + padding)
frame = skb;
else {
- frame = dev_alloc_skb(local->hw.extra_tx_headroom +
- subframe_len);
+ /*
+ * Allocate and reserve two bytes more for payload
+ * alignment since sizeof(struct ethhdr) is 14.
+ */
+ frame = dev_alloc_skb(
+ ALIGN(local->hw.extra_tx_headroom, 4) +
+ subframe_len + 2);
if (frame == NULL)
return RX_DROP_UNUSABLE;
- skb_reserve(frame, local->hw.extra_tx_headroom +
- sizeof(struct ethhdr));
+ skb_reserve(frame,
+ ALIGN(local->hw.extra_tx_headroom, 4) +
+ sizeof(struct ethhdr) + 2);
memcpy(skb_put(frame, ntohs(len)), skb->data,
ntohs(len));
@@ -1971,7 +2015,7 @@ static void __ieee80211_rx_handle_packet
rx.flags |= IEEE80211_RX_IN_SCAN;
ieee80211_parse_qos(&rx);
- ieee80211_verify_ip_alignment(&rx);
+ ieee80211_verify_alignment(&rx);
skb = rx.skb;
reply other threads:[~2009-01-06 23:26 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1231284370.22613.0.camel@johannes \
--to=johannes@sipsolutions.net \
--cc=linux-wireless@vger.kernel.org \
--cc=linville@tuxdriver.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox