All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] ath10k:  support msdu chaining.
@ 2014-02-28  1:20 ` greearb
  0 siblings, 0 replies; 10+ messages in thread
From: greearb @ 2014-02-28  1:20 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Ben Greear

From: Ben Greear <greearb@candelatech.com>

Consolidate the list of msdu skbs into the msdu-head skb, delete the
rest of the skbs, pass the msdu-head skb on up the stack as normal.

Tested with high-speed TCP and UDP traffic on modified firmware that
supports raw-rx.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

v2:  Add note about skb_try_coalesce, though I am not sure it will
     work or not.
     Only grow skb once, which should save on some memory (re)allocation
     and copying when we have more than one chained skb.

 drivers/net/wireless/ath/ath10k/htt_rx.c | 60 +++++++++++++++++++++++++++++---
 1 file changed, 56 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 9f3def7..a5070a9 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -401,6 +401,7 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 		msdu_len = MS(__le32_to_cpu(rx_desc->msdu_start.info0),
 			      RX_MSDU_START_INFO0_MSDU_LENGTH);
 		msdu_chained = rx_desc->frag_info.ring2_more_count;
+		msdu_chaining = msdu_chained;
 
 		if (msdu_len_invalid)
 			msdu_len = 0;
@@ -428,7 +429,6 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 
 			msdu->next = next;
 			msdu = next;
-			msdu_chaining = 1;
 		}
 
 		last_msdu = __le32_to_cpu(rx_desc->msdu_end.info0) &
@@ -1008,14 +1008,66 @@ continue_on:
 				continue;
 			}
 
-			/* FIXME: we do not support chaining yet.
-			 * this needs investigation */
 			if (msdu_chaining) {
-				ath10k_warn("htt rx msdu_chaining is true\n");
+				struct sk_buff *next = msdu_head->next;
+				struct sk_buff *to_free = next;
+				static int do_once = 1;
+				int space;
+				int total_len = 0;
+
+				/* TODO:  Might could optimize this by using
+				 * skb_try_coalesce or similar method to]
+				 * decrease copying.
+				 */
+
+				msdu_head->next = NULL;
+
+				if (unlikely(do_once)) {
+					ath10k_warn("htt rx msdu_chaining detected %d\n",
+						    msdu_chaining);
+					do_once = 0;
+				}
+
+				/* Allocate total length all at once. */
+				while (next) {
+					total_len += next->len;
+					next = next->next;
+				}
+
+				space = total_len - skb_tailroom(msdu_head);
+				if ((space > 0) &&
+				    (pskb_expand_head(msdu_head, 0, space, GFP_ATOMIC) < 0)) {
+					/* TODO:  bump some rx-oom error stat */
+					goto outside_continue;
+				}
+
+				/* Walk list again, copying contents into
+				 * msdu_head
+				 */
+				next = to_free;
+				while (next) {
+					skb_copy_from_linear_data(next, skb_put(msdu_head, next->len),
+								  next->len);
+					next = next->next;
+				}
+
+				/* If here, we have consolidated skb.  Free the
+				 * fragments and pass the main skb on up the
+				 * stack.
+				 */
+				ath10k_htt_rx_free_msdu_chain(to_free);
+				goto have_good_msdu;
+
+			outside_continue:
+				/* put it back together so we can free the
+				 * whole list at once.
+				 */
+				msdu_head->next = to_free;
 				ath10k_htt_rx_free_msdu_chain(msdu_head);
 				continue;
 			}
 
+		have_good_msdu:
 			info.skb     = msdu_head;
 			info.fcs_err = ath10k_htt_rx_has_fcs_err(msdu_head);
 			info.mic_err = ath10k_htt_rx_has_mic_err(msdu_head);
-- 
1.7.11.7


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

* [PATCH v2] ath10k:  support msdu chaining.
@ 2014-02-28  1:20 ` greearb
  0 siblings, 0 replies; 10+ messages in thread
From: greearb @ 2014-02-28  1:20 UTC (permalink / raw)
  To: ath10k; +Cc: Ben Greear, linux-wireless

From: Ben Greear <greearb@candelatech.com>

Consolidate the list of msdu skbs into the msdu-head skb, delete the
rest of the skbs, pass the msdu-head skb on up the stack as normal.

Tested with high-speed TCP and UDP traffic on modified firmware that
supports raw-rx.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---

v2:  Add note about skb_try_coalesce, though I am not sure it will
     work or not.
     Only grow skb once, which should save on some memory (re)allocation
     and copying when we have more than one chained skb.

 drivers/net/wireless/ath/ath10k/htt_rx.c | 60 +++++++++++++++++++++++++++++---
 1 file changed, 56 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 9f3def7..a5070a9 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -401,6 +401,7 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 		msdu_len = MS(__le32_to_cpu(rx_desc->msdu_start.info0),
 			      RX_MSDU_START_INFO0_MSDU_LENGTH);
 		msdu_chained = rx_desc->frag_info.ring2_more_count;
+		msdu_chaining = msdu_chained;
 
 		if (msdu_len_invalid)
 			msdu_len = 0;
@@ -428,7 +429,6 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 
 			msdu->next = next;
 			msdu = next;
-			msdu_chaining = 1;
 		}
 
 		last_msdu = __le32_to_cpu(rx_desc->msdu_end.info0) &
@@ -1008,14 +1008,66 @@ continue_on:
 				continue;
 			}
 
-			/* FIXME: we do not support chaining yet.
-			 * this needs investigation */
 			if (msdu_chaining) {
-				ath10k_warn("htt rx msdu_chaining is true\n");
+				struct sk_buff *next = msdu_head->next;
+				struct sk_buff *to_free = next;
+				static int do_once = 1;
+				int space;
+				int total_len = 0;
+
+				/* TODO:  Might could optimize this by using
+				 * skb_try_coalesce or similar method to]
+				 * decrease copying.
+				 */
+
+				msdu_head->next = NULL;
+
+				if (unlikely(do_once)) {
+					ath10k_warn("htt rx msdu_chaining detected %d\n",
+						    msdu_chaining);
+					do_once = 0;
+				}
+
+				/* Allocate total length all at once. */
+				while (next) {
+					total_len += next->len;
+					next = next->next;
+				}
+
+				space = total_len - skb_tailroom(msdu_head);
+				if ((space > 0) &&
+				    (pskb_expand_head(msdu_head, 0, space, GFP_ATOMIC) < 0)) {
+					/* TODO:  bump some rx-oom error stat */
+					goto outside_continue;
+				}
+
+				/* Walk list again, copying contents into
+				 * msdu_head
+				 */
+				next = to_free;
+				while (next) {
+					skb_copy_from_linear_data(next, skb_put(msdu_head, next->len),
+								  next->len);
+					next = next->next;
+				}
+
+				/* If here, we have consolidated skb.  Free the
+				 * fragments and pass the main skb on up the
+				 * stack.
+				 */
+				ath10k_htt_rx_free_msdu_chain(to_free);
+				goto have_good_msdu;
+
+			outside_continue:
+				/* put it back together so we can free the
+				 * whole list at once.
+				 */
+				msdu_head->next = to_free;
 				ath10k_htt_rx_free_msdu_chain(msdu_head);
 				continue;
 			}
 
+		have_good_msdu:
 			info.skb     = msdu_head;
 			info.fcs_err = ath10k_htt_rx_has_fcs_err(msdu_head);
 			info.mic_err = ath10k_htt_rx_has_mic_err(msdu_head);
-- 
1.7.11.7


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

* Re: [PATCH v2] ath10k:  support msdu chaining.
  2014-02-28  1:20 ` greearb
@ 2014-02-28  7:38   ` Kalle Valo
  -1 siblings, 0 replies; 10+ messages in thread
From: Kalle Valo @ 2014-02-28  7:38 UTC (permalink / raw)
  To: greearb; +Cc: ath10k, linux-wireless

greearb@candelatech.com writes:

> From: Ben Greear <greearb@candelatech.com>
>
> Consolidate the list of msdu skbs into the msdu-head skb, delete the
> rest of the skbs, pass the msdu-head skb on up the stack as normal.
>
> Tested with high-speed TCP and UDP traffic on modified firmware that
> supports raw-rx.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>
> v2:  Add note about skb_try_coalesce, though I am not sure it will
>      work or not.
>      Only grow skb once, which should save on some memory (re)allocation
>      and copying when we have more than one chained skb.

I didn't look at the patch itself yet, but I see checkpatch warnings:

drivers/net/wireless/ath/ath10k/htt_rx.c:1022: WARNING: line over 80 characters
drivers/net/wireless/ath/ath10k/htt_rx.c:1032: WARNING: line over 80 characters
drivers/net/wireless/ath/ath10k/htt_rx.c:1044: WARNING: labels should not be indented
drivers/net/wireless/ath/ath10k/htt_rx.c:1053: WARNING: labels should not be indented

-- 
Kalle Valo

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

* Re: [PATCH v2] ath10k:  support msdu chaining.
@ 2014-02-28  7:38   ` Kalle Valo
  0 siblings, 0 replies; 10+ messages in thread
From: Kalle Valo @ 2014-02-28  7:38 UTC (permalink / raw)
  To: greearb; +Cc: linux-wireless, ath10k

greearb@candelatech.com writes:

> From: Ben Greear <greearb@candelatech.com>
>
> Consolidate the list of msdu skbs into the msdu-head skb, delete the
> rest of the skbs, pass the msdu-head skb on up the stack as normal.
>
> Tested with high-speed TCP and UDP traffic on modified firmware that
> supports raw-rx.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>
> v2:  Add note about skb_try_coalesce, though I am not sure it will
>      work or not.
>      Only grow skb once, which should save on some memory (re)allocation
>      and copying when we have more than one chained skb.

I didn't look at the patch itself yet, but I see checkpatch warnings:

drivers/net/wireless/ath/ath10k/htt_rx.c:1022: WARNING: line over 80 characters
drivers/net/wireless/ath/ath10k/htt_rx.c:1032: WARNING: line over 80 characters
drivers/net/wireless/ath/ath10k/htt_rx.c:1044: WARNING: labels should not be indented
drivers/net/wireless/ath/ath10k/htt_rx.c:1053: WARNING: labels should not be indented

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

* Re: [PATCH v2] ath10k: support msdu chaining.
  2014-02-28  1:20 ` greearb
@ 2014-02-28  7:51   ` Michal Kazior
  -1 siblings, 0 replies; 10+ messages in thread
From: Michal Kazior @ 2014-02-28  7:51 UTC (permalink / raw)
  To: Ben Greear; +Cc: ath10k, linux-wireless

On 28 February 2014 02:20,  <greearb@candelatech.com> wrote:
> From: Ben Greear <greearb@candelatech.com>
>
> Consolidate the list of msdu skbs into the msdu-head skb, delete the
> rest of the skbs, pass the msdu-head skb on up the stack as normal.
>
> Tested with high-speed TCP and UDP traffic on modified firmware that
> supports raw-rx.

As for an umodified firmware I expect ath10k will just re-assemble
corrupted frames just to have them dropped by mac80211. It might be
worth checking:

 if (chaining && fcs_error)
    drop();

..to avoid processing junk and flushing d-cache (this matters for less
powerful host systems).


>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>
> v2:  Add note about skb_try_coalesce, though I am not sure it will
>      work or not.
>      Only grow skb once, which should save on some memory (re)allocation
>      and copying when we have more than one chained skb.
>
>  drivers/net/wireless/ath/ath10k/htt_rx.c | 60 +++++++++++++++++++++++++++++---
>  1 file changed, 56 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 9f3def7..a5070a9 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -401,6 +401,7 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
>                 msdu_len = MS(__le32_to_cpu(rx_desc->msdu_start.info0),
>                               RX_MSDU_START_INFO0_MSDU_LENGTH);
>                 msdu_chained = rx_desc->frag_info.ring2_more_count;
> +               msdu_chaining = msdu_chained;
>
>                 if (msdu_len_invalid)
>                         msdu_len = 0;
> @@ -428,7 +429,6 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
>
>                         msdu->next = next;
>                         msdu = next;
> -                       msdu_chaining = 1;
>                 }
>
>                 last_msdu = __le32_to_cpu(rx_desc->msdu_end.info0) &
> @@ -1008,14 +1008,66 @@ continue_on:
>                                 continue;
>                         }
>
> -                       /* FIXME: we do not support chaining yet.
> -                        * this needs investigation */
>                         if (msdu_chaining) {
> -                               ath10k_warn("htt rx msdu_chaining is true\n");
> +                               struct sk_buff *next = msdu_head->next;
> +                               struct sk_buff *to_free = next;
> +                               static int do_once = 1;
> +                               int space;
> +                               int total_len = 0;
> +
> +                               /* TODO:  Might could optimize this by using
> +                                * skb_try_coalesce or similar method to]
> +                                * decrease copying.
> +                                */

It'd be great if we could use kernel-provided function to merge skbs
instead of rolling out our own ;-)


> +
> +                               msdu_head->next = NULL;
> +
> +                               if (unlikely(do_once)) {
> +                                       ath10k_warn("htt rx msdu_chaining detected %d\n",
> +                                                   msdu_chaining);
> +                                       do_once = 0;
> +                               }

I don't really like this do_once thing. I wouldn't even bother with a
warning, since, well, chaining parsing is implemented. This should be
a simple debug print if anything.


> +
> +                               /* Allocate total length all at once. */
> +                               while (next) {
> +                                       total_len += next->len;
> +                                       next = next->next;
> +                               }
> +
> +                               space = total_len - skb_tailroom(msdu_head);
> +                               if ((space > 0) &&
> +                                   (pskb_expand_head(msdu_head, 0, space, GFP_ATOMIC) < 0)) {
> +                                       /* TODO:  bump some rx-oom error stat */
> +                                       goto outside_continue;
> +                               }
> +
> +                               /* Walk list again, copying contents into
> +                                * msdu_head
> +                                */
> +                               next = to_free;
> +                               while (next) {
> +                                       skb_copy_from_linear_data(next, skb_put(msdu_head, next->len),
> +                                                                 next->len);
> +                                       next = next->next;
> +                               }
> +
> +                               /* If here, we have consolidated skb.  Free the
> +                                * fragments and pass the main skb on up the
> +                                * stack.
> +                                */
> +                               ath10k_htt_rx_free_msdu_chain(to_free);
> +                               goto have_good_msdu;
> +
> +                       outside_continue:
> +                               /* put it back together so we can free the
> +                                * whole list at once.
> +                                */
> +                               msdu_head->next = to_free;
>                                 ath10k_htt_rx_free_msdu_chain(msdu_head);
>                                 continue;
>                         }
>
> +               have_good_msdu:
>                         info.skb     = msdu_head;
>                         info.fcs_err = ath10k_htt_rx_has_fcs_err(msdu_head);
>                         info.mic_err = ath10k_htt_rx_has_mic_err(msdu_head);

If anything, please, make it into a separate function. The rx handler
is already huge and it needs shrinking down, not expanding it more.


Michał

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

* Re: [PATCH v2] ath10k: support msdu chaining.
@ 2014-02-28  7:51   ` Michal Kazior
  0 siblings, 0 replies; 10+ messages in thread
From: Michal Kazior @ 2014-02-28  7:51 UTC (permalink / raw)
  To: Ben Greear; +Cc: linux-wireless, ath10k

On 28 February 2014 02:20,  <greearb@candelatech.com> wrote:
> From: Ben Greear <greearb@candelatech.com>
>
> Consolidate the list of msdu skbs into the msdu-head skb, delete the
> rest of the skbs, pass the msdu-head skb on up the stack as normal.
>
> Tested with high-speed TCP and UDP traffic on modified firmware that
> supports raw-rx.

As for an umodified firmware I expect ath10k will just re-assemble
corrupted frames just to have them dropped by mac80211. It might be
worth checking:

 if (chaining && fcs_error)
    drop();

..to avoid processing junk and flushing d-cache (this matters for less
powerful host systems).


>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>
> v2:  Add note about skb_try_coalesce, though I am not sure it will
>      work or not.
>      Only grow skb once, which should save on some memory (re)allocation
>      and copying when we have more than one chained skb.
>
>  drivers/net/wireless/ath/ath10k/htt_rx.c | 60 +++++++++++++++++++++++++++++---
>  1 file changed, 56 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 9f3def7..a5070a9 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -401,6 +401,7 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
>                 msdu_len = MS(__le32_to_cpu(rx_desc->msdu_start.info0),
>                               RX_MSDU_START_INFO0_MSDU_LENGTH);
>                 msdu_chained = rx_desc->frag_info.ring2_more_count;
> +               msdu_chaining = msdu_chained;
>
>                 if (msdu_len_invalid)
>                         msdu_len = 0;
> @@ -428,7 +429,6 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
>
>                         msdu->next = next;
>                         msdu = next;
> -                       msdu_chaining = 1;
>                 }
>
>                 last_msdu = __le32_to_cpu(rx_desc->msdu_end.info0) &
> @@ -1008,14 +1008,66 @@ continue_on:
>                                 continue;
>                         }
>
> -                       /* FIXME: we do not support chaining yet.
> -                        * this needs investigation */
>                         if (msdu_chaining) {
> -                               ath10k_warn("htt rx msdu_chaining is true\n");
> +                               struct sk_buff *next = msdu_head->next;
> +                               struct sk_buff *to_free = next;
> +                               static int do_once = 1;
> +                               int space;
> +                               int total_len = 0;
> +
> +                               /* TODO:  Might could optimize this by using
> +                                * skb_try_coalesce or similar method to]
> +                                * decrease copying.
> +                                */

It'd be great if we could use kernel-provided function to merge skbs
instead of rolling out our own ;-)


> +
> +                               msdu_head->next = NULL;
> +
> +                               if (unlikely(do_once)) {
> +                                       ath10k_warn("htt rx msdu_chaining detected %d\n",
> +                                                   msdu_chaining);
> +                                       do_once = 0;
> +                               }

I don't really like this do_once thing. I wouldn't even bother with a
warning, since, well, chaining parsing is implemented. This should be
a simple debug print if anything.


> +
> +                               /* Allocate total length all at once. */
> +                               while (next) {
> +                                       total_len += next->len;
> +                                       next = next->next;
> +                               }
> +
> +                               space = total_len - skb_tailroom(msdu_head);
> +                               if ((space > 0) &&
> +                                   (pskb_expand_head(msdu_head, 0, space, GFP_ATOMIC) < 0)) {
> +                                       /* TODO:  bump some rx-oom error stat */
> +                                       goto outside_continue;
> +                               }
> +
> +                               /* Walk list again, copying contents into
> +                                * msdu_head
> +                                */
> +                               next = to_free;
> +                               while (next) {
> +                                       skb_copy_from_linear_data(next, skb_put(msdu_head, next->len),
> +                                                                 next->len);
> +                                       next = next->next;
> +                               }
> +
> +                               /* If here, we have consolidated skb.  Free the
> +                                * fragments and pass the main skb on up the
> +                                * stack.
> +                                */
> +                               ath10k_htt_rx_free_msdu_chain(to_free);
> +                               goto have_good_msdu;
> +
> +                       outside_continue:
> +                               /* put it back together so we can free the
> +                                * whole list at once.
> +                                */
> +                               msdu_head->next = to_free;
>                                 ath10k_htt_rx_free_msdu_chain(msdu_head);
>                                 continue;
>                         }
>
> +               have_good_msdu:
>                         info.skb     = msdu_head;
>                         info.fcs_err = ath10k_htt_rx_has_fcs_err(msdu_head);
>                         info.mic_err = ath10k_htt_rx_has_mic_err(msdu_head);

If anything, please, make it into a separate function. The rx handler
is already huge and it needs shrinking down, not expanding it more.


Michał

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

* Re: [PATCH v2] ath10k: support msdu chaining.
  2014-02-28  7:51   ` Michal Kazior
@ 2014-03-03 22:08     ` Ben Greear
  -1 siblings, 0 replies; 10+ messages in thread
From: Ben Greear @ 2014-03-03 22:08 UTC (permalink / raw)
  To: Michal Kazior; +Cc: ath10k, linux-wireless

On 02/27/2014 11:51 PM, Michal Kazior wrote:
> On 28 February 2014 02:20,  <greearb@candelatech.com> wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> Consolidate the list of msdu skbs into the msdu-head skb, delete the
>> rest of the skbs, pass the msdu-head skb on up the stack as normal.
>>
>> Tested with high-speed TCP and UDP traffic on modified firmware that
>> supports raw-rx.
> 
> As for an umodified firmware I expect ath10k will just re-assemble
> corrupted frames just to have them dropped by mac80211. It might be
> worth checking:
> 
>  if (chaining && fcs_error)
>     drop();

As far as I can tell, packets with bad FCS are dropped earlier in that
rx method?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


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

* Re: [PATCH v2] ath10k: support msdu chaining.
@ 2014-03-03 22:08     ` Ben Greear
  0 siblings, 0 replies; 10+ messages in thread
From: Ben Greear @ 2014-03-03 22:08 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

On 02/27/2014 11:51 PM, Michal Kazior wrote:
> On 28 February 2014 02:20,  <greearb@candelatech.com> wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> Consolidate the list of msdu skbs into the msdu-head skb, delete the
>> rest of the skbs, pass the msdu-head skb on up the stack as normal.
>>
>> Tested with high-speed TCP and UDP traffic on modified firmware that
>> supports raw-rx.
> 
> As for an umodified firmware I expect ath10k will just re-assemble
> corrupted frames just to have them dropped by mac80211. It might be
> worth checking:
> 
>  if (chaining && fcs_error)
>     drop();

As far as I can tell, packets with bad FCS are dropped earlier in that
rx method?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

* Re: [PATCH v2] ath10k: support msdu chaining.
  2014-03-03 22:08     ` Ben Greear
@ 2014-03-04  7:14       ` Michal Kazior
  -1 siblings, 0 replies; 10+ messages in thread
From: Michal Kazior @ 2014-03-04  7:14 UTC (permalink / raw)
  To: Ben Greear; +Cc: ath10k, linux-wireless

On 3 March 2014 23:08, Ben Greear <greearb@candelatech.com> wrote:
> On 02/27/2014 11:51 PM, Michal Kazior wrote:
>> On 28 February 2014 02:20,  <greearb@candelatech.com> wrote:
>>> From: Ben Greear <greearb@candelatech.com>
>>>
>>> Consolidate the list of msdu skbs into the msdu-head skb, delete the
>>> rest of the skbs, pass the msdu-head skb on up the stack as normal.
>>>
>>> Tested with high-speed TCP and UDP traffic on modified firmware that
>>> supports raw-rx.
>>
>> As for an umodified firmware I expect ath10k will just re-assemble
>> corrupted frames just to have them dropped by mac80211. It might be
>> worth checking:
>>
>>  if (chaining && fcs_error)
>>     drop();
>
> As far as I can tell, packets with bad FCS are dropped earlier in that
> rx method?

Ah, good point. We're dropping some frames based on `status` value already.


Michał

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

* Re: [PATCH v2] ath10k: support msdu chaining.
@ 2014-03-04  7:14       ` Michal Kazior
  0 siblings, 0 replies; 10+ messages in thread
From: Michal Kazior @ 2014-03-04  7:14 UTC (permalink / raw)
  To: Ben Greear; +Cc: linux-wireless, ath10k

On 3 March 2014 23:08, Ben Greear <greearb@candelatech.com> wrote:
> On 02/27/2014 11:51 PM, Michal Kazior wrote:
>> On 28 February 2014 02:20,  <greearb@candelatech.com> wrote:
>>> From: Ben Greear <greearb@candelatech.com>
>>>
>>> Consolidate the list of msdu skbs into the msdu-head skb, delete the
>>> rest of the skbs, pass the msdu-head skb on up the stack as normal.
>>>
>>> Tested with high-speed TCP and UDP traffic on modified firmware that
>>> supports raw-rx.
>>
>> As for an umodified firmware I expect ath10k will just re-assemble
>> corrupted frames just to have them dropped by mac80211. It might be
>> worth checking:
>>
>>  if (chaining && fcs_error)
>>     drop();
>
> As far as I can tell, packets with bad FCS are dropped earlier in that
> rx method?

Ah, good point. We're dropping some frames based on `status` value already.


Michał

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

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

end of thread, other threads:[~2014-03-04  7:15 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-28  1:20 [PATCH v2] ath10k: support msdu chaining greearb
2014-02-28  1:20 ` greearb
2014-02-28  7:38 ` Kalle Valo
2014-02-28  7:38   ` Kalle Valo
2014-02-28  7:51 ` Michal Kazior
2014-02-28  7:51   ` Michal Kazior
2014-03-03 22:08   ` Ben Greear
2014-03-03 22:08     ` Ben Greear
2014-03-04  7:14     ` Michal Kazior
2014-03-04  7:14       ` Michal Kazior

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.