Move the A-MSDU handling code from mac80211 to cfg80211 so that more
drivers can use it. The new created function ieee80211_asmdu_to_8023s
converts an A-MSDU frame to a list of 802.3 frames.
Signed-off-by: Zhu Yi <[email protected]>
---
include/net/cfg80211.h | 12 +++++
net/mac80211/rx.c | 107 ++++++++++-------------------------------------
net/wireless/util.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 140 insertions(+), 84 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 0884b9a..ad31b55 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1593,6 +1593,18 @@ int ieee80211_data_from_8023(struct sk_buff *skb, u8 *addr,
enum nl80211_iftype iftype, u8 *bssid, bool qos);
/**
+ * ieee80211_asmdu_to_8023s - decode an IEEE 802.11n A-MSDU frame
+ *
+ * @skb: The input IEEE 802.11n A-MSDU frame.
+ * @list: The output list of 802.3 frames. It must be allocated and
+ * initialized by by the caller.
+ * @addr: The device MAC address.
+ * @iftype: The device interface type.
+ */
+int ieee80211_amsdu_to_8023s(struct sk_buff *skb, struct sk_buff_head *list,
+ u8 *addr, enum nl80211_iftype iftype);
+
+/**
* cfg80211_classify8021d - determine the 802.1p/1d tag for a data frame
* @skb: the data frame
*/
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 57b8a0a..34ab25d 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1556,16 +1556,11 @@ static ieee80211_rx_result debug_noinline
ieee80211_rx_h_amsdu(struct ieee80211_rx_data *rx)
{
struct net_device *dev = rx->sdata->dev;
- struct ieee80211_local *local = rx->local;
- u16 ethertype;
- u8 *payload;
- struct sk_buff *skb = rx->skb, *frame = NULL;
+ struct sk_buff *skb = rx->skb;
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
__le16 fc = hdr->frame_control;
- const struct ethhdr *eth;
- int remaining, err;
- u8 dst[ETH_ALEN];
- u8 src[ETH_ALEN];
+ int err;
+ struct sk_buff_head frame_list;
if (unlikely(!ieee80211_is_data(fc)))
return RX_CONTINUE;
@@ -1576,92 +1571,36 @@ ieee80211_rx_h_amsdu(struct ieee80211_rx_data *rx)
if (!(rx->flags & IEEE80211_RX_AMSDU))
return RX_CONTINUE;
- err = __ieee80211_data_to_8023(rx);
- if (unlikely(err))
+ if (ieee80211_has_a4(hdr->frame_control) &&
+ rx->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
+ !rx->sdata->u.vlan.sta)
return RX_DROP_UNUSABLE;
- skb->dev = dev;
-
- dev->stats.rx_packets++;
- dev->stats.rx_bytes += skb->len;
-
- /* skip the wrapping header */
- eth = (struct ethhdr *) skb_pull(skb, sizeof(struct ethhdr));
- if (!eth)
+ if (is_multicast_ether_addr(hdr->addr1) &&
+ ((rx->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
+ rx->sdata->u.vlan.sta) ||
+ (rx->sdata->vif.type == NL80211_IFTYPE_STATION &&
+ rx->sdata->u.mgd.use_4addr)))
return RX_DROP_UNUSABLE;
- while (skb != frame) {
- u8 padding;
- __be16 len = eth->h_proto;
- unsigned int subframe_len = sizeof(struct ethhdr) + ntohs(len);
-
- remaining = skb->len;
- memcpy(dst, eth->h_dest, ETH_ALEN);
- memcpy(src, eth->h_source, ETH_ALEN);
-
- padding = ((4 - subframe_len) & 0x3);
- /* the last MSDU has no padding */
- if (subframe_len > remaining)
- return RX_DROP_UNUSABLE;
-
- skb_pull(skb, sizeof(struct ethhdr));
- /* if last subframe reuse skb */
- if (remaining <= subframe_len + padding)
- frame = skb;
- else {
- /*
- * 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->dev = dev;
+ skb_queue_head_init(&frame_list);
- skb_reserve(frame,
- ALIGN(local->hw.extra_tx_headroom, 4) +
- sizeof(struct ethhdr) + 2);
- memcpy(skb_put(frame, ntohs(len)), skb->data,
- ntohs(len));
+ err = ieee80211_amsdu_to_8023s(skb, &frame_list, dev->dev_addr,
+ rx->sdata->vif.type);
+ if (err)
+ return RX_DROP_UNUSABLE;
- eth = (struct ethhdr *) skb_pull(skb, ntohs(len) +
- padding);
- if (!eth) {
- dev_kfree_skb(frame);
- return RX_DROP_UNUSABLE;
- }
- }
+ dev->stats.rx_packets++;
+ dev->stats.rx_bytes += skb->len;
- skb_reset_network_header(frame);
- frame->dev = dev;
- frame->priority = skb->priority;
- rx->skb = frame;
-
- payload = frame->data;
- ethertype = (payload[6] << 8) | payload[7];
-
- if (likely((compare_ether_addr(payload, rfc1042_header) == 0 &&
- ethertype != ETH_P_AARP && ethertype != ETH_P_IPX) ||
- compare_ether_addr(payload,
- bridge_tunnel_header) == 0)) {
- /* remove RFC1042 or Bridge-Tunnel
- * encapsulation and replace EtherType */
- skb_pull(frame, 6);
- memcpy(skb_push(frame, ETH_ALEN), src, ETH_ALEN);
- memcpy(skb_push(frame, ETH_ALEN), dst, ETH_ALEN);
- } else {
- memcpy(skb_push(frame, sizeof(__be16)),
- &len, sizeof(__be16));
- memcpy(skb_push(frame, ETH_ALEN), src, ETH_ALEN);
- memcpy(skb_push(frame, ETH_ALEN), dst, ETH_ALEN);
- }
+ while (!skb_queue_empty(&frame_list)) {
+ rx->skb = skb_dequeue(&frame_list);
if (!ieee80211_frame_allowed(rx, fc)) {
- if (skb == frame) /* last frame */
+ if (skb == rx->skb) /* last frame */
return RX_DROP_UNUSABLE;
- dev_kfree_skb(frame);
+ dev_kfree_skb(rx->skb);
continue;
}
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 59361fd..8ed98d7 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -497,6 +497,111 @@ int ieee80211_data_from_8023(struct sk_buff *skb, u8 *addr,
}
EXPORT_SYMBOL(ieee80211_data_from_8023);
+/**
+ * ieee80211_asmdu_to_8023s - decode an IEEE 802.11n A-MSDU frame
+ *
+ * Decode an IEEE 802.11n A-MSDU frame and convert it to a list of
+ * 802.3 frames. This function returns 0 on succeess. In this case,
+ * we optimize the code to reuse the @skb to hold the last 802.3
+ * frame in the @list.
+ *
+ * @skb: The input IEEE 802.11n A-MSDU frame.
+ * @list: The output list of 802.3 frames. It must be allocated and
+ * initialized by by the caller.
+ * @addr: The device MAC address.
+ * @iftype: The device interface type.
+ */
+int ieee80211_amsdu_to_8023s(struct sk_buff *skb, struct sk_buff_head *list,
+ u8 *addr, enum nl80211_iftype iftype)
+{
+ struct sk_buff *frame = NULL;
+ u16 ethertype;
+ u8 *payload;
+ const struct ethhdr *eth;
+ int remaining, err;
+ u8 dst[ETH_ALEN], src[ETH_ALEN];
+
+ err = ieee80211_data_to_8023(skb, addr, iftype);
+ if (err)
+ return err;
+
+ /* skip the wrapping header */
+ eth = (struct ethhdr *) skb_pull(skb, sizeof(struct ethhdr));
+ if (!eth)
+ return -1;
+
+ while (skb != frame) {
+ u8 padding;
+ __be16 len = eth->h_proto;
+ unsigned int subframe_len = sizeof(struct ethhdr) + ntohs(len);
+
+ remaining = skb->len;
+ memcpy(dst, eth->h_dest, ETH_ALEN);
+ memcpy(src, eth->h_source, ETH_ALEN);
+
+ padding = (4 - subframe_len) & 0x3;
+ /* the last MSDU has no padding */
+ if (subframe_len > remaining)
+ goto purge;
+
+ skb_pull(skb, sizeof(struct ethhdr));
+ /* reuse skb for the last subframe */
+ if (remaining <= subframe_len + padding)
+ frame = skb;
+ else {
+ /*
+ * Allocate and reserve two bytes more for payload
+ * alignment since sizeof(struct ethhdr) is 14.
+ */
+ frame = dev_alloc_skb(subframe_len + 2);
+ if (!frame)
+ goto purge;
+
+ skb_reserve(frame, sizeof(struct ethhdr) + 2);
+ memcpy(skb_put(frame, ntohs(len)), skb->data,
+ ntohs(len));
+
+ eth = (struct ethhdr *)skb_pull(skb, ntohs(len) +
+ padding);
+ if (!eth) {
+ dev_kfree_skb(frame);
+ goto purge;
+ }
+ }
+
+ skb_reset_network_header(frame);
+ frame->dev = skb->dev;
+ frame->priority = skb->priority;
+
+ payload = frame->data;
+ ethertype = (payload[6] << 8) | payload[7];
+
+ if (likely((compare_ether_addr(payload, rfc1042_header) == 0 &&
+ ethertype != ETH_P_AARP && ethertype != ETH_P_IPX) ||
+ compare_ether_addr(payload,
+ bridge_tunnel_header) == 0)) {
+ /* remove RFC1042 or Bridge-Tunnel
+ * encapsulation and replace EtherType */
+ skb_pull(frame, 6);
+ memcpy(skb_push(frame, ETH_ALEN), src, ETH_ALEN);
+ memcpy(skb_push(frame, ETH_ALEN), dst, ETH_ALEN);
+ } else {
+ memcpy(skb_push(frame, sizeof(__be16)), &len,
+ sizeof(__be16));
+ memcpy(skb_push(frame, ETH_ALEN), src, ETH_ALEN);
+ memcpy(skb_push(frame, ETH_ALEN), dst, ETH_ALEN);
+ }
+ skb_queue_tail(list, frame);
+ }
+
+ return 0;
+
+ purge:
+ skb_queue_purge(list);
+ return -1;
+}
+EXPORT_SYMBOL(ieee80211_amsdu_to_8023s);
+
/* Given a data frame determine the 802.1p/1d tag to use. */
unsigned int cfg80211_classify8021d(struct sk_buff *skb)
{
--
1.6.0.4
When the device receives an A-MSDU frame (indicated by flag
IWM_RX_TICKET_AMSDU_MSK), use ieee80211_amsdu_to_8023s to convert
it to a list of 802.3 frames and handled them to upper layer.
Signed-off-by: Zhu Yi <[email protected]>
---
drivers/net/wireless/iwmc3200wifi/rx.c | 55 +++++++++++++++++++++++++++-----
1 files changed, 47 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/iwmc3200wifi/rx.c b/drivers/net/wireless/iwmc3200wifi/rx.c
index 72c27a3..99f4f8a 100644
--- a/drivers/net/wireless/iwmc3200wifi/rx.c
+++ b/drivers/net/wireless/iwmc3200wifi/rx.c
@@ -1534,6 +1534,34 @@ static void classify8023(struct sk_buff *skb)
}
}
+static int iwm_rx_process_amsdu(struct iwm_priv *iwm, struct sk_buff *skb)
+{
+ struct wireless_dev *wdev = iwm_to_wdev(iwm);
+ struct net_device *ndev = iwm_to_ndev(iwm);
+ struct sk_buff_head list;
+ struct sk_buff *frame;
+ int ret;
+
+ IWM_HEXDUMP(iwm, DBG, RX, "A-MSDU: ", skb->data, skb->len);
+
+ skb_queue_head_init(&list);
+ ret = ieee80211_amsdu_to_8023s(skb, &list, ndev->dev_addr,
+ wdev->iftype);
+ if (ret) {
+ IWM_ERR(iwm, "decode A-MSDU frame failed\n");
+ return -EINVAL;
+ }
+
+ while ((frame = skb_dequeue(&list))) {
+ if (netif_rx_ni(frame) == NET_RX_DROP) {
+ IWM_ERR(iwm, "Packet dropped\n");
+ ndev->stats.rx_dropped++;
+ }
+ }
+
+ return 0;
+}
+
static void iwm_rx_process_packet(struct iwm_priv *iwm,
struct iwm_rx_packet *packet,
struct iwm_rx_ticket_node *ticket_node)
@@ -1548,25 +1576,36 @@ static void iwm_rx_process_packet(struct iwm_priv *iwm,
switch (le16_to_cpu(ticket_node->ticket->action)) {
case IWM_RX_TICKET_RELEASE:
IWM_DBG_RX(iwm, DBG, "RELEASE packet\n");
+
+ skb->dev = iwm_to_ndev(iwm);
+ skb->protocol = eth_type_trans(skb, ndev);
+ skb->ip_summed = CHECKSUM_NONE;
+ memset(skb->cb, 0, sizeof(skb->cb));
+
+ ndev->stats.rx_packets++;
+ ndev->stats.rx_bytes += skb->len;
+
classify8023(skb);
iwm_rx_adjust_packet(iwm, packet, ticket_node);
+
+ if (le16_to_cpu(ticket_node->ticket->flags) &
+ IWM_RX_TICKET_AMSDU_MSK) {
+ ret = iwm_rx_process_amsdu(iwm, skb);
+ if (ret < 0)
+ kfree_skb(packet->skb);
+ break;
+ }
+
ret = ieee80211_data_to_8023(skb, ndev->dev_addr, wdev->iftype);
if (ret < 0) {
IWM_DBG_RX(iwm, DBG, "Couldn't convert 802.11 header - "
"%d\n", ret);
+ kfree_skb(packet->skb);
break;
}
IWM_HEXDUMP(iwm, DBG, RX, "802.3: ", skb->data, skb->len);
- skb->dev = iwm_to_ndev(iwm);
- skb->protocol = eth_type_trans(skb, ndev);
- skb->ip_summed = CHECKSUM_NONE;
- memset(skb->cb, 0, sizeof(skb->cb));
-
- ndev->stats.rx_packets++;
- ndev->stats.rx_bytes += skb->len;
-
if (netif_rx_ni(skb) == NET_RX_DROP) {
IWM_ERR(iwm, "Packet dropped\n");
ndev->stats.rx_dropped++;
--
1.6.0.4
On Fri, 2009-11-27 at 17:30 +0800, Zhu Yi wrote:
> > No, davem and I optimised that away a long time ago via using
> > netdev->needed_headroom and netdev->needed_tailroom. It even works
> for bridges and their slave devices, iirc.
>
> I missed this. Will check it. If so, I'll add another parameter to
> pass the extra hw tx headroom to ieee80211_asmdu_to_8023s.
Now I see your changes to LL_RESERVED_SPACE*(). Thanks!
-yi
On Fri, 2009-11-27 at 17:20 +0800, Johannes Berg wrote:
> Is it? I don't think so. Many drivers go up beyond that as far as I
> know. Then some do different things like putting it in a different DMA
> block.
>
> > While for those drivers really need a bigger
> > extra headroom and support Rx aggregation, this probably means
> > ieee80211_skb_resize. But the resize should always happen for every
> > packet from the IP stack, right?
>
> No, davem and I optimised that away a long time ago via using
> netdev->needed_headroom and netdev->needed_tailroom. It even works for
> bridges and their slave devices, iirc.
I missed this. Will check it. If so, I'll add another parameter to pass
the extra hw tx headroom to ieee80211_asmdu_to_8023s.
Thanks,
-yi
On Thu, 2009-11-26 at 14:03 +0800, Zhu Yi wrote:
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -1593,6 +1593,18 @@ int ieee80211_data_from_8023(struct sk_buff *skb, u8 *addr,
> enum nl80211_iftype iftype, u8 *bssid, bool qos);
>
> /**
> + * ieee80211_asmdu_to_8023s - decode an IEEE 802.11n A-MSDU frame
> + *
> + * @skb: The input IEEE 802.11n A-MSDU frame.
> + * @list: The output list of 802.3 frames. It must be allocated and
> + * initialized by by the caller.
> + * @addr: The device MAC address.
> + * @iftype: The device interface type.
> + */
> +int ieee80211_amsdu_to_8023s(struct sk_buff *skb, struct sk_buff_head *list,
> + u8 *addr, enum nl80211_iftype iftype);
Oh one more thing, can you make addr const?
Also -- you lost the extra TX headroom which I think mac80211 as an AP
requires since it could forward these frames? Or does that not happen?
Not sure right now why that was there to start with.
johannes
On Fri, 2009-11-27 at 10:52 +0800, Zhu Yi wrote:
> > Also -- you lost the extra TX headroom which I think mac80211 as an AP
> > requires since it could forward these frames? Or does that not happen?
> > Not sure right now why that was there to start with.
>
> Yes, I'd like to hear more feedback on this. I think it's a trade off
> between performance optimization and clean interface. As we already use
> dev_alloc_skb to reserve 32 bytes headroom, it should be enough for most
> of the current drivers.
Is it? I don't think so. Many drivers go up beyond that as far as I
know. Then some do different things like putting it in a different DMA
block.
> While for those drivers really need a bigger
> extra headroom and support Rx aggregation, this probably means
> ieee80211_skb_resize. But the resize should always happen for every
> packet from the IP stack, right?
No, davem and I optimised that away a long time ago via using
netdev->needed_headroom and netdev->needed_tailroom. It even works for
bridges and their slave devices, iirc.
johannes
On Fri, 2009-11-27 at 10:33 +0100, Johannes Berg wrote:
> The thing is that I'm not even sure if we can possibly forward a frame
> after this deaggregation. I'll poke at the code.
Ah, yes, it is possible -- the destination of the A-MSDU subframes might
be associated to mac80211 acting as an AP. I was thinking multicast
frames can't be encapsulated that way so it can't happen. So yes, adding
a headroom parameter would be good I think.
johannes
On Thu, 2009-11-26 at 18:05 +0800, Johannes Berg wrote:
> Oh one more thing, can you make addr const?
Sure.
> Also -- you lost the extra TX headroom which I think mac80211 as an AP
> requires since it could forward these frames? Or does that not happen?
> Not sure right now why that was there to start with.
Yes, I'd like to hear more feedback on this. I think it's a trade off
between performance optimization and clean interface. As we already use
dev_alloc_skb to reserve 32 bytes headroom, it should be enough for most
of the current drivers. While for those drivers really need a bigger
extra headroom and support Rx aggregation, this probably means
ieee80211_skb_resize. But the resize should always happen for every
packet from the IP stack, right?
Thanks,
-yi
On Thu, 2009-11-26 at 14:03 +0800, Zhu Yi wrote:
> Move the A-MSDU handling code from mac80211 to cfg80211 so that more
> drivers can use it. The new created function ieee80211_asmdu_to_8023s
> converts an A-MSDU frame to a list of 802.3 frames.
>
> Signed-off-by: Zhu Yi <[email protected]>
> ---
> include/net/cfg80211.h | 12 +++++
> net/mac80211/rx.c | 107 ++++++++++-------------------------------------
> net/wireless/util.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 140 insertions(+), 84 deletions(-)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 0884b9a..ad31b55 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -1593,6 +1593,18 @@ int ieee80211_data_from_8023(struct sk_buff *skb, u8 *addr,
> enum nl80211_iftype iftype, u8 *bssid, bool qos);
>
> /**
> + * ieee80211_asmdu_to_8023s - decode an IEEE 802.11n A-MSDU frame
^ typo
> + skb->dev = dev;
> + skb_queue_head_init(&frame_list);
^ use __ versions please, no need for locking since it's on stack
> + dev->stats.rx_packets++;
> + dev->stats.rx_bytes += skb->len;
Shouldn't rx_packets be per sub-frame?
> + while (!skb_queue_empty(&frame_list)) {
> + rx->skb = skb_dequeue(&frame_list);
__skb_dequeue
> +/**
> + * ieee80211_asmdu_to_8023s - decode an IEEE 802.11n A-MSDU frame
^ copy/paste :)
Why do you have the kernel-doc twice anyway?
> + skb_queue_tail(list, frame);
__
> + purge:
> + skb_queue_purge(list);
__ (does that exist? guess it must?)
johannes
On Thu, 2009-11-26 at 17:49 +0800, Johannes Berg wrote:
> > + skb->dev = dev;
> > + skb_queue_head_init(&frame_list);
>
> ^ use __ versions please, no need for locking since it's on stack
You are right. Good catch!
> > + dev->stats.rx_packets++;
> > + dev->stats.rx_bytes += skb->len;
>
> Shouldn't rx_packets be per sub-frame?
Not very sure. Maybe it's one frame from the device's perspective? The
original code does this so I didn't change the behavior. Should I?
Thanks,
-yi
On Fri, 2009-11-27 at 09:06 +0800, Zhu Yi wrote:
> > > + dev->stats.rx_packets++;
> > > + dev->stats.rx_bytes += skb->len;
> >
> > Shouldn't rx_packets be per sub-frame?
>
> Not very sure. Maybe it's one frame from the device's perspective? The
> original code does this so I didn't change the behavior. Should I?
Ah, yes, I remember now -- had been wondering about this before. What
does everybody else think?
johannes
On Fri, 2009-11-27 at 17:30 +0800, Zhu Yi wrote:
> On Fri, 2009-11-27 at 17:20 +0800, Johannes Berg wrote:
> > Is it? I don't think so. Many drivers go up beyond that as far as I
> > know. Then some do different things like putting it in a different DMA
> > block.
> >
> > > While for those drivers really need a bigger
> > > extra headroom and support Rx aggregation, this probably means
> > > ieee80211_skb_resize. But the resize should always happen for every
> > > packet from the IP stack, right?
> >
> > No, davem and I optimised that away a long time ago via using
> > netdev->needed_headroom and netdev->needed_tailroom. It even works for
> > bridges and their slave devices, iirc.
>
> I missed this. Will check it. If so, I'll add another parameter to pass
> the extra hw tx headroom to ieee80211_asmdu_to_8023s.
The thing is that I'm not even sure if we can possibly forward a frame
after this deaggregation. I'll poke at the code.
johannes