Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCH] brcmfmac: fix incorrect event channel deduction
@ 2017-01-17 22:29 gavinli
  2017-01-17 22:41 ` Rafał Miłecki
  0 siblings, 1 reply; 9+ messages in thread
From: gavinli @ 2017-01-17 22:29 UTC (permalink / raw)
  To: arend.vanspriel, franky.lin, hante.meuleman, linux-wireless,
	brcm80211-dev-list.pdl
  Cc: Gavin Li

From: Gavin Li <git@thegavinli.com>

brcmf_sdio_fromevntchan() was being called on the the hardware header
rather than the software header, causing some frames to be
mischaracterized as on the event channel rather than the data channel.
This fixes the performance regression introduced in commit c56caa9d
where event processing is done separately.

Signed-off-by: Gavin Li <git@thegavinli.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index dfb0658713d9..d2219885071f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -1661,7 +1661,7 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 					   pfirst->len, pfirst->next,
 					   pfirst->prev);
 			skb_unlink(pfirst, &bus->glom);
-			if (brcmf_sdio_fromevntchan(pfirst->data))
+			if (brcmf_sdio_fromevntchan(&dptr[SDPCM_HWHDR_LEN]))
 				brcmf_rx_event(bus->sdiodev->dev, pfirst);
 			else
 				brcmf_rx_frame(bus->sdiodev->dev, pfirst,
-- 
2.11.0

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

* Re: [PATCH] brcmfmac: fix incorrect event channel deduction
  2017-01-17 22:29 [PATCH] brcmfmac: fix incorrect event channel deduction gavinli
@ 2017-01-17 22:41 ` Rafał Miłecki
  2017-01-17 22:41   ` Rafał Miłecki
  0 siblings, 1 reply; 9+ messages in thread
From: Rafał Miłecki @ 2017-01-17 22:41 UTC (permalink / raw)
  To: gavinli
  Cc: Arend Van Spriel, Franky Lin, Hante Meuleman,
	linux-wireless@vger.kernel.org,
	open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
	Gavin Li

On 17 January 2017 at 23:29,  <gavinli@thegavinli.com> wrote:
> From: Gavin Li <git@thegavinli.com>
>
> brcmf_sdio_fromevntchan() was being called on the the hardware header
> rather than the software header, causing some frames to be
> mischaracterized as on the event channel rather than the data channel.
> This fixes the performance regression introduced in commit c56caa9d
> where event processing is done separately.

This should be:
in commit c56caa9db8ab ("brcmfmac: screening firmware event packet") where (...)

You should also use
Fixes: c56caa9db8ab ("brcmfmac: screening firmware event packet") where (...)


> Signed-off-by: Gavin Li <git@thegavinli.com>

Please add also:
Cc: stable@vger.kernel.org # 4.6+

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

* Re: [PATCH] brcmfmac: fix incorrect event channel deduction
  2017-01-17 22:41 ` Rafał Miłecki
@ 2017-01-17 22:41   ` Rafał Miłecki
  0 siblings, 0 replies; 9+ messages in thread
From: Rafał Miłecki @ 2017-01-17 22:41 UTC (permalink / raw)
  To: gavinli
  Cc: Arend Van Spriel, Franky Lin, Hante Meuleman,
	linux-wireless@vger.kernel.org,
	open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
	Gavin Li

On 17 January 2017 at 23:41, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com> wro=
te:
> On 17 January 2017 at 23:29,  <gavinli@thegavinli.com> wrote:
>> From: Gavin Li <git@thegavinli.com>
>>
>> brcmf_sdio_fromevntchan() was being called on the the hardware header
>> rather than the software header, causing some frames to be
>> mischaracterized as on the event channel rather than the data channel.
>> This fixes the performance regression introduced in commit c56caa9d
>> where event processing is done separately.
>
> This should be:
> in commit c56caa9db8ab ("brcmfmac: screening firmware event packet") wher=
e (...)
>
> You should also use
> Fixes: c56caa9db8ab ("brcmfmac: screening firmware event packet") where (=
...)

Oops, copy & paste mistake. Just:
Fixes: c56caa9db8ab ("brcmfmac: screening firmware event packet")

--=20
Rafa=C5=82

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

* [PATCH] brcmfmac: fix incorrect event channel deduction
@ 2017-01-17 22:50 gavinli
  0 siblings, 0 replies; 9+ messages in thread
From: gavinli @ 2017-01-17 22:50 UTC (permalink / raw)
  To: arend.vanspriel, franky.lin, hante.meuleman, linux-wireless,
	brcm80211-dev-list.pdl
  Cc: stable, Gavin Li

From: Gavin Li <git@thegavinli.com>

brcmf_sdio_fromevntchan() was being called on the the data frame
rather than the software header, causing some frames to be
mischaracterized as on the event channel rather than the data channel.

This fixes a major performance regression (due to dropped packets).

Fixes: c56caa9db8ab ("brcmfmac: screening firmware event packet")
Signed-off-by: Gavin Li <git@thegavinli.com>
Cc: <stable@vger.kernel.org> [4.6+]
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index dfb0658713d9..d2219885071f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -1661,7 +1661,7 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 					   pfirst->len, pfirst->next,
 					   pfirst->prev);
 			skb_unlink(pfirst, &bus->glom);
-			if (brcmf_sdio_fromevntchan(pfirst->data))
+			if (brcmf_sdio_fromevntchan(&dptr[SDPCM_HWHDR_LEN]))
 				brcmf_rx_event(bus->sdiodev->dev, pfirst);
 			else
 				brcmf_rx_frame(bus->sdiodev->dev, pfirst,
-- 
2.11.0

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

* [PATCH] brcmfmac: fix incorrect event channel deduction
@ 2017-01-17 22:55 gavinli
  2017-01-17 23:09 ` Rafał Miłecki
  0 siblings, 1 reply; 9+ messages in thread
From: gavinli @ 2017-01-17 22:55 UTC (permalink / raw)
  To: arend.vanspriel, franky.lin, hante.meuleman, linux-wireless,
	brcm80211-dev-list.pdl
  Cc: stable, Gavin Li

From: Gavin Li <git@thegavinli.com>

brcmf_sdio_fromevntchan() was being called on the the data frame
rather than the software header, causing some frames to be
mischaracterized as on the event channel rather than the data channel.

This fixes a major performance regression (due to dropped packets).

Fixes: c56caa9db8ab ("brcmfmac: screening firmware event packet")
Signed-off-by: Gavin Li <git@thegavinli.com>
Cc: <stable@vger.kernel.org> [4.6+]
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index dfb0658713d9..d2219885071f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -1661,7 +1661,7 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 					   pfirst->len, pfirst->next,
 					   pfirst->prev);
 			skb_unlink(pfirst, &bus->glom);
-			if (brcmf_sdio_fromevntchan(pfirst->data))
+			if (brcmf_sdio_fromevntchan(&dptr[SDPCM_HWHDR_LEN]))
 				brcmf_rx_event(bus->sdiodev->dev, pfirst);
 			else
 				brcmf_rx_frame(bus->sdiodev->dev, pfirst,
-- 
2.11.0

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

* Re: [PATCH] brcmfmac: fix incorrect event channel deduction
  2017-01-17 22:55 gavinli
@ 2017-01-17 23:09 ` Rafał Miłecki
  0 siblings, 0 replies; 9+ messages in thread
From: Rafał Miłecki @ 2017-01-17 23:09 UTC (permalink / raw)
  To: gavinli
  Cc: Arend Van Spriel, Franky Lin, Hante Meuleman,
	linux-wireless@vger.kernel.org,
	open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER, Stable,
	Gavin Li

On 17 January 2017 at 23:55,  <gavinli@thegavinli.com> wrote:
> From: Gavin Li <git@thegavinli.com>
>
> brcmf_sdio_fromevntchan() was being called on the the data frame
> rather than the software header, causing some frames to be
> mischaracterized as on the event channel rather than the data channel.
>
> This fixes a major performance regression (due to dropped packets).
>
> Fixes: c56caa9db8ab ("brcmfmac: screening firmware event packet")
> Signed-off-by: Gavin Li <git@thegavinli.com>
> Cc: <stable@vger.kernel.org> [4.6+]

Oh, that was supposed to be 4.7+, I gave you a wrong hint, sorry!

You may send V3, or maybe ask Kalle to fix it by hand when applying the pat=
ch.

--=20
Rafa=C5=82

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

* [PATCH] brcmfmac: fix incorrect event channel deduction
@ 2017-01-17 23:15 gavinli
  2017-01-18 10:27 ` Kalle Valo
  0 siblings, 1 reply; 9+ messages in thread
From: gavinli @ 2017-01-17 23:15 UTC (permalink / raw)
  To: arend.vanspriel, franky.lin, hante.meuleman, linux-wireless,
	brcm80211-dev-list.pdl
  Cc: stable, Gavin Li

From: Gavin Li <git@thegavinli.com>

brcmf_sdio_fromevntchan() was being called on the the data frame
rather than the software header, causing some frames to be
mischaracterized as on the event channel rather than the data channel.

This fixes a major performance regression (due to dropped packets).

Fixes: c56caa9db8ab ("brcmfmac: screening firmware event packet")
Signed-off-by: Gavin Li <git@thegavinli.com>
Cc: <stable@vger.kernel.org> [4.7+]
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index dfb0658713d9..d2219885071f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -1661,7 +1661,7 @@ static u8 brcmf_sdio_rxglom(struct brcmf_sdio *bus, u8 rxseq)
 					   pfirst->len, pfirst->next,
 					   pfirst->prev);
 			skb_unlink(pfirst, &bus->glom);
-			if (brcmf_sdio_fromevntchan(pfirst->data))
+			if (brcmf_sdio_fromevntchan(&dptr[SDPCM_HWHDR_LEN]))
 				brcmf_rx_event(bus->sdiodev->dev, pfirst);
 			else
 				brcmf_rx_frame(bus->sdiodev->dev, pfirst,
-- 
2.11.0

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

* Re: [PATCH] brcmfmac: fix incorrect event channel deduction
  2017-01-17 23:15 gavinli
@ 2017-01-18 10:27 ` Kalle Valo
  2017-01-18 10:40   ` Kalle Valo
  0 siblings, 1 reply; 9+ messages in thread
From: Kalle Valo @ 2017-01-18 10:27 UTC (permalink / raw)
  To: gavinli
  Cc: arend.vanspriel, franky.lin, hante.meuleman, linux-wireless,
	brcm80211-dev-list.pdl, stable, Gavin Li

gavinli@thegavinli.com writes:

> From: Gavin Li <git@thegavinli.com>
>
> brcmf_sdio_fromevntchan() was being called on the the data frame
> rather than the software header, causing some frames to be
> mischaracterized as on the event channel rather than the data channel.
>
> This fixes a major performance regression (due to dropped packets).
>
> Fixes: c56caa9db8ab ("brcmfmac: screening firmware event packet")
> Signed-off-by: Gavin Li <git@thegavinli.com>
> Cc: <stable@vger.kernel.org> [4.7+]

In the future please add version to the subject so that maintainers can
easily find the latest version:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#subject

Arend, do you think this is safe enough for 4.10? Or better to get more
testing time and push this to 4.11 and from where to stable releases?

-- 
Kalle Valo

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

* Re: [PATCH] brcmfmac: fix incorrect event channel deduction
  2017-01-18 10:27 ` Kalle Valo
@ 2017-01-18 10:40   ` Kalle Valo
  0 siblings, 0 replies; 9+ messages in thread
From: Kalle Valo @ 2017-01-18 10:40 UTC (permalink / raw)
  To: gavinli
  Cc: arend.vanspriel, franky.lin, hante.meuleman, linux-wireless,
	brcm80211-dev-list.pdl, stable, Gavin Li

Kalle Valo <kvalo@codeaurora.org> writes:

> gavinli@thegavinli.com writes:
>
>> From: Gavin Li <git@thegavinli.com>
>>
>> brcmf_sdio_fromevntchan() was being called on the the data frame
>> rather than the software header, causing some frames to be
>> mischaracterized as on the event channel rather than the data channel.
>>
>> This fixes a major performance regression (due to dropped packets).
>>
>> Fixes: c56caa9db8ab ("brcmfmac: screening firmware event packet")
>> Signed-off-by: Gavin Li <git@thegavinli.com>
>> Cc: <stable@vger.kernel.org> [4.7+]
>
> In the future please add version to the subject so that maintainers can
> easily find the latest version:
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#subject

Ah, I missed that you had submitted v3:

https://patchwork.kernel.org/patch/9522185/

That's the version I'm planning to take (if Arend acks it).

-- 
Kalle Valo

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

end of thread, other threads:[~2017-01-18 10:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-17 22:29 [PATCH] brcmfmac: fix incorrect event channel deduction gavinli
2017-01-17 22:41 ` Rafał Miłecki
2017-01-17 22:41   ` Rafał Miłecki
  -- strict thread matches above, loose matches on Subject: below --
2017-01-17 22:50 gavinli
2017-01-17 22:55 gavinli
2017-01-17 23:09 ` Rafał Miłecki
2017-01-17 23:15 gavinli
2017-01-18 10:27 ` Kalle Valo
2017-01-18 10:40   ` Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox