All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s
@ 2009-11-26  6:03 Zhu Yi
  2009-11-26  6:03 ` [RFC 2/2] iwmc3200wifi: rx aggregation support Zhu Yi
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Zhu Yi @ 2009-11-26  6:03 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Zhu Yi

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 <yi.zhu@intel.com>
---
 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


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

* [RFC 2/2] iwmc3200wifi: rx aggregation support
  2009-11-26  6:03 [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s Zhu Yi
@ 2009-11-26  6:03 ` Zhu Yi
  2009-11-26  9:49 ` [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s Johannes Berg
  2009-11-26 10:05 ` Johannes Berg
  2 siblings, 0 replies; 12+ messages in thread
From: Zhu Yi @ 2009-11-26  6:03 UTC (permalink / raw)
  To: linville; +Cc: linux-wireless, Zhu Yi

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 <yi.zhu@intel.com>
---
 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


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

* Re: [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s
  2009-11-26  6:03 [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s Zhu Yi
  2009-11-26  6:03 ` [RFC 2/2] iwmc3200wifi: rx aggregation support Zhu Yi
@ 2009-11-26  9:49 ` Johannes Berg
  2009-11-27  1:06   ` Zhu Yi
  2009-11-26 10:05 ` Johannes Berg
  2 siblings, 1 reply; 12+ messages in thread
From: Johannes Berg @ 2009-11-26  9:49 UTC (permalink / raw)
  To: Zhu Yi; +Cc: linville, linux-wireless

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

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 <yi.zhu@intel.com>
> ---
>  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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s
  2009-11-26  6:03 [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s Zhu Yi
  2009-11-26  6:03 ` [RFC 2/2] iwmc3200wifi: rx aggregation support Zhu Yi
  2009-11-26  9:49 ` [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s Johannes Berg
@ 2009-11-26 10:05 ` Johannes Berg
  2009-11-27  2:52   ` Zhu Yi
  2 siblings, 1 reply; 12+ messages in thread
From: Johannes Berg @ 2009-11-26 10:05 UTC (permalink / raw)
  To: Zhu Yi; +Cc: linville, linux-wireless

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

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s
  2009-11-26  9:49 ` [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s Johannes Berg
@ 2009-11-27  1:06   ` Zhu Yi
  2009-11-27  9:18     ` Johannes Berg
  0 siblings, 1 reply; 12+ messages in thread
From: Zhu Yi @ 2009-11-27  1:06 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linville, linux-wireless

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


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

* Re: [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s
  2009-11-26 10:05 ` Johannes Berg
@ 2009-11-27  2:52   ` Zhu Yi
  2009-11-27  9:20     ` Johannes Berg
  0 siblings, 1 reply; 12+ messages in thread
From: Zhu Yi @ 2009-11-27  2:52 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linville, linux-wireless

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


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

* Re: [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s
  2009-11-27  1:06   ` Zhu Yi
@ 2009-11-27  9:18     ` Johannes Berg
  0 siblings, 0 replies; 12+ messages in thread
From: Johannes Berg @ 2009-11-27  9:18 UTC (permalink / raw)
  To: Zhu Yi; +Cc: linville, linux-wireless

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

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s
  2009-11-27  2:52   ` Zhu Yi
@ 2009-11-27  9:20     ` Johannes Berg
  2009-11-27  9:30       ` Zhu Yi
  0 siblings, 1 reply; 12+ messages in thread
From: Johannes Berg @ 2009-11-27  9:20 UTC (permalink / raw)
  To: Zhu Yi; +Cc: linville, linux-wireless

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

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s
  2009-11-27  9:20     ` Johannes Berg
@ 2009-11-27  9:30       ` Zhu Yi
  2009-11-27  9:33         ` Johannes Berg
  2009-11-30  2:34         ` Zhu Yi
  0 siblings, 2 replies; 12+ messages in thread
From: Zhu Yi @ 2009-11-27  9:30 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linville, linux-wireless

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


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

* Re: [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s
  2009-11-27  9:30       ` Zhu Yi
@ 2009-11-27  9:33         ` Johannes Berg
  2009-11-27  9:35           ` Johannes Berg
  2009-11-30  2:34         ` Zhu Yi
  1 sibling, 1 reply; 12+ messages in thread
From: Johannes Berg @ 2009-11-27  9:33 UTC (permalink / raw)
  To: Zhu Yi; +Cc: linville, linux-wireless

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

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s
  2009-11-27  9:33         ` Johannes Berg
@ 2009-11-27  9:35           ` Johannes Berg
  0 siblings, 0 replies; 12+ messages in thread
From: Johannes Berg @ 2009-11-27  9:35 UTC (permalink / raw)
  To: Zhu Yi; +Cc: linville, linux-wireless

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

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s
  2009-11-27  9:30       ` Zhu Yi
  2009-11-27  9:33         ` Johannes Berg
@ 2009-11-30  2:34         ` Zhu Yi
  1 sibling, 0 replies; 12+ messages in thread
From: Zhu Yi @ 2009-11-30  2:34 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linville, linux-wireless

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


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

end of thread, other threads:[~2009-11-30  2:34 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-26  6:03 [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s Zhu Yi
2009-11-26  6:03 ` [RFC 2/2] iwmc3200wifi: rx aggregation support Zhu Yi
2009-11-26  9:49 ` [RFC 1/2] wireless: add ieee80211_asmdu_to_8023s Johannes Berg
2009-11-27  1:06   ` Zhu Yi
2009-11-27  9:18     ` Johannes Berg
2009-11-26 10:05 ` Johannes Berg
2009-11-27  2:52   ` Zhu Yi
2009-11-27  9:20     ` Johannes Berg
2009-11-27  9:30       ` Zhu Yi
2009-11-27  9:33         ` Johannes Berg
2009-11-27  9:35           ` Johannes Berg
2009-11-30  2:34         ` Zhu Yi

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.