From: Rafał Miłecki <[email protected]>
Flowrings contain skbs waiting for transmission that were passed to us
by netif. It means we checked every one of them looking for 802.1x
Ethernet type. When deleting flowring we have to use freeing function
that will check for 802.1x type as well.
Freeing skbs without a proper check was leading to counter not being
properly decreased. This was triggering a WARNING every time
brcmf_netdev_wait_pend8021x was called.
Signed-off-by: Rafał Miłecki <[email protected]>
---
Kalle: this isn't important enough for 4.8 as it's too late for that.
I'd like to get it for 4.9 however, as this fixes bug that could lead
to WARNING on every add_key/del_key call. We was struggling with these
WARNINGs for some time and this fixes one of two problems causing them.
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
index b16b367..d0b738d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
@@ -234,13 +234,20 @@ static void brcmf_flowring_block(struct brcmf_flowring *flow, u16 flowid,
void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 flowid)
{
+ struct brcmf_bus *bus_if = dev_get_drvdata(flow->dev);
struct brcmf_flowring_ring *ring;
+ struct brcmf_if *ifp;
u16 hash_idx;
+ u8 ifidx;
struct sk_buff *skb;
ring = flow->rings[flowid];
if (!ring)
return;
+
+ ifidx = brcmf_flowring_ifidx_get(flow, flowid);
+ ifp = brcmf_get_ifp(bus_if->drvr, ifidx);
+
brcmf_flowring_block(flow, flowid, false);
hash_idx = ring->hash_id;
flow->hash[hash_idx].ifidx = BRCMF_FLOWRING_INVALID_IFIDX;
@@ -249,7 +256,7 @@ void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 flowid)
skb = skb_dequeue(&ring->skblist);
while (skb) {
- brcmu_pkt_buf_free_skb(skb);
+ brcmf_txfinalize(ifp, skb, false);
skb = skb_dequeue(&ring->skblist);
}
--
2.9.3
On 27 September 2016 at 13:44, Rafa=C5=82 Mi=C5=82ecki <[email protected]> w=
rote:
> On 27 September 2016 at 13:27, Kalle Valo <[email protected]> wrote:
>> Arend Van Spriel <[email protected]> writes:
>>
>>> On 27-9-2016 11:14, Rafa=C5=82 Mi=C5=82ecki wrote:
>>>> From: Rafa=C5=82 Mi=C5=82ecki <[email protected]>
>>>>
>>>> Flowrings contain skbs waiting for transmission that were passed to us
>>>> by netif. It means we checked every one of them looking for 802.1x
>>>> Ethernet type. When deleting flowring we have to use freeing function
>>>> that will check for 802.1x type as well.
>>>>
>>>> Freeing skbs without a proper check was leading to counter not being
>>>> properly decreased. This was triggering a WARNING every time
>>>> brcmf_netdev_wait_pend8021x was called.
>>>
>>> Acked-by: Arend van Spriel <[email protected]>
>>>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <[email protected]>
>>>> ---
>>>> Kalle: this isn't important enough for 4.8 as it's too late for that.
>>>>
>>>> I'd like to get it for 4.9 however, as this fixes bug that could lead
>>>> to WARNING on every add_key/del_key call. We was struggling with these
>>>> WARNINGs for some time and this fixes one of two problems causing them=
.
>>
>> Ok, I'll queue this for 4.9.
>>
>>> Please mark it for stable as well.
>>
>> I can add that. Any ideas how old releases stable releases should this
>> go to?
>
> I was analyzing this.
> 1) This patch uses brcmf_get_ifp which is available in 4.4+ only.
> 2) It applies cleanly to 4.5+ only due to 32f90caa7debd ("brcmfmac:
> Increase nr of supported flowrings.")
> 3) 4.4 would also require applying to the patch without broadcom/ subdir
>
> That said I suggest 4.5+. Any objections?
Let me see if patchwork with pick Cc tag as it does for others.
Cc: [email protected] # 4.5+
This may be worth backporting to 4.4 as well (as it's longterm), but
I'll do it separately due to patch not applying cleanly.
--=20
Rafa=C5=82
On 27 September 2016 at 14:04, Arend Van Spriel
<[email protected]> wrote:
> On 27-9-2016 13:58, Rafa=C5=82 Mi=C5=82ecki wrote:
>> On 27 September 2016 at 13:44, Rafa=C5=82 Mi=C5=82ecki <[email protected]=
> wrote:
>>> On 27 September 2016 at 13:27, Kalle Valo <[email protected]> wrote:
>>>> Arend Van Spriel <[email protected]> writes:
>>>>
>>>>> On 27-9-2016 11:14, Rafa=C5=82 Mi=C5=82ecki wrote:
>>>>>> From: Rafa=C5=82 Mi=C5=82ecki <[email protected]>
>>>>>>
>>>>>> Flowrings contain skbs waiting for transmission that were passed to =
us
>>>>>> by netif. It means we checked every one of them looking for 802.1x
>>>>>> Ethernet type. When deleting flowring we have to use freeing functio=
n
>>>>>> that will check for 802.1x type as well.
>>>>>>
>>>>>> Freeing skbs without a proper check was leading to counter not being
>>>>>> properly decreased. This was triggering a WARNING every time
>>>>>> brcmf_netdev_wait_pend8021x was called.
>>>>>
>>>>> Acked-by: Arend van Spriel <[email protected]>
>>>>>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <[email protected]>
>>>>>> ---
>>>>>> Kalle: this isn't important enough for 4.8 as it's too late for that=
.
>>>>>>
>>>>>> I'd like to get it for 4.9 however, as this fixes bug that could lea=
d
>>>>>> to WARNING on every add_key/del_key call. We was struggling with the=
se
>>>>>> WARNINGs for some time and this fixes one of two problems causing th=
em.
>>>>
>>>> Ok, I'll queue this for 4.9.
>>>>
>>>>> Please mark it for stable as well.
>>>>
>>>> I can add that. Any ideas how old releases stable releases should this
>>>> go to?
>>>
>>> I was analyzing this.
>>> 1) This patch uses brcmf_get_ifp which is available in 4.4+ only.
>>> 2) It applies cleanly to 4.5+ only due to 32f90caa7debd ("brcmfmac:
>>> Increase nr of supported flowrings.")
>>> 3) 4.4 would also require applying to the patch without broadcom/ subdi=
r
>>>
>>> That said I suggest 4.5+. Any objections?
>
> No objections. Just a tip. I tend to look at kernel.org main page to see
> the stable and long-term kernel listed. So 4.7+ and 4.5+ have same
> meaning as 4.5 and 4.6 are not stable/long-term kernels.
Some projects may work on their own stable kernels, e.g. Ubuntu, see:
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable
That's why I don't always look strictly at upstream stable releases only.
--=20
Rafa=C5=82
From: Rafał Miłecki <[email protected]>
Flowrings contain skbs waiting for transmission that were passed to us
by netif. It means we checked every one of them looking for 802.1x
Ethernet type. When deleting flowring we have to use freeing function
that will check for 802.1x type as well.
Freeing skbs without a proper check was leading to counter not being
properly decreased. This was triggering a WARNING every time
brcmf_netdev_wait_pend8021x was called.
Signed-off-by: Rafał Miłecki <[email protected]>
Acked-by: Arend van Spriel <[email protected]>
Cc: [email protected] # 4.5+
---
V2: Add Cc for stable 4.5+. It doesn't apply cleanly to 4.4 and is not
possible for 4.3- due to missing brcmf_get_ifp.
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
index b16b367..d0b738d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
@@ -234,13 +234,20 @@ static void brcmf_flowring_block(struct brcmf_flowring *flow, u16 flowid,
void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 flowid)
{
+ struct brcmf_bus *bus_if = dev_get_drvdata(flow->dev);
struct brcmf_flowring_ring *ring;
+ struct brcmf_if *ifp;
u16 hash_idx;
+ u8 ifidx;
struct sk_buff *skb;
ring = flow->rings[flowid];
if (!ring)
return;
+
+ ifidx = brcmf_flowring_ifidx_get(flow, flowid);
+ ifp = brcmf_get_ifp(bus_if->drvr, ifidx);
+
brcmf_flowring_block(flow, flowid, false);
hash_idx = ring->hash_id;
flow->hash[hash_idx].ifidx = BRCMF_FLOWRING_INVALID_IFIDX;
@@ -249,7 +256,7 @@ void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 flowid)
skb = skb_dequeue(&ring->skblist);
while (skb) {
- brcmu_pkt_buf_free_skb(skb);
+ brcmf_txfinalize(ifp, skb, false);
skb = skb_dequeue(&ring->skblist);
}
--
2.9.3
Arend Van Spriel <[email protected]> writes:
> On 27-9-2016 11:14, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <[email protected]>
>>=20
>> Flowrings contain skbs waiting for transmission that were passed to us
>> by netif. It means we checked every one of them looking for 802.1x
>> Ethernet type. When deleting flowring we have to use freeing function
>> that will check for 802.1x type as well.
>>=20
>> Freeing skbs without a proper check was leading to counter not being
>> properly decreased. This was triggering a WARNING every time
>> brcmf_netdev_wait_pend8021x was called.
>
> Acked-by: Arend van Spriel <[email protected]>
>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <[email protected]>
>> ---
>> Kalle: this isn't important enough for 4.8 as it's too late for that.
>>=20
>> I'd like to get it for 4.9 however, as this fixes bug that could lead
>> to WARNING on every add_key/del_key call. We was struggling with these
>> WARNINGs for some time and this fixes one of two problems causing them.
Ok, I'll queue this for 4.9.
> Please mark it for stable as well.
I can add that. Any ideas how old releases stable releases should this
go to?
--=20
Kalle Valo
On 27-9-2016 13:27, Kalle Valo wrote:
> Arend Van Spriel <[email protected]> writes:
>
>> On 27-9-2016 11:14, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <[email protected]>
>>>
>>> Flowrings contain skbs waiting for transmission that were passed to us
>>> by netif. It means we checked every one of them looking for 802.1x
>>> Ethernet type. When deleting flowring we have to use freeing function
>>> that will check for 802.1x type as well.
>>>
>>> Freeing skbs without a proper check was leading to counter not being
>>> properly decreased. This was triggering a WARNING every time
>>> brcmf_netdev_wait_pend8021x was called.
>>
>> Acked-by: Arend van Spriel <[email protected]>
>>> Signed-off-by: Rafał Miłecki <[email protected]>
>>> ---
>>> Kalle: this isn't important enough for 4.8 as it's too late for that.
>>>
>>> I'd like to get it for 4.9 however, as this fixes bug that could lead
>>> to WARNING on every add_key/del_key call. We was struggling with these
>>> WARNINGs for some time and this fixes one of two problems causing them.
>
> Ok, I'll queue this for 4.9.
>
>> Please mark it for stable as well.
>
> I can add that. Any ideas how old releases stable releases should this
> go to?
Not sure if the vendor directory move causes issues as stable can not
fallback to three-way merge. I assumed it would so my last stable tag
was only for 4.7 and I took care of older kernels at later time with
backported patch. I can do that for this one as well.
Regards,
Arend
On 27-9-2016 13:58, Rafał Miłecki wrote:
> On 27 September 2016 at 13:44, Rafał Miłecki <[email protected]> wrote:
>> On 27 September 2016 at 13:27, Kalle Valo <[email protected]> wrote:
>>> Arend Van Spriel <[email protected]> writes:
>>>
>>>> On 27-9-2016 11:14, Rafał Miłecki wrote:
>>>>> From: Rafał Miłecki <[email protected]>
>>>>>
>>>>> Flowrings contain skbs waiting for transmission that were passed to us
>>>>> by netif. It means we checked every one of them looking for 802.1x
>>>>> Ethernet type. When deleting flowring we have to use freeing function
>>>>> that will check for 802.1x type as well.
>>>>>
>>>>> Freeing skbs without a proper check was leading to counter not being
>>>>> properly decreased. This was triggering a WARNING every time
>>>>> brcmf_netdev_wait_pend8021x was called.
>>>>
>>>> Acked-by: Arend van Spriel <[email protected]>
>>>>> Signed-off-by: Rafał Miłecki <[email protected]>
>>>>> ---
>>>>> Kalle: this isn't important enough for 4.8 as it's too late for that.
>>>>>
>>>>> I'd like to get it for 4.9 however, as this fixes bug that could lead
>>>>> to WARNING on every add_key/del_key call. We was struggling with these
>>>>> WARNINGs for some time and this fixes one of two problems causing them.
>>>
>>> Ok, I'll queue this for 4.9.
>>>
>>>> Please mark it for stable as well.
>>>
>>> I can add that. Any ideas how old releases stable releases should this
>>> go to?
>>
>> I was analyzing this.
>> 1) This patch uses brcmf_get_ifp which is available in 4.4+ only.
>> 2) It applies cleanly to 4.5+ only due to 32f90caa7debd ("brcmfmac:
>> Increase nr of supported flowrings.")
>> 3) 4.4 would also require applying to the patch without broadcom/ subdir
>>
>> That said I suggest 4.5+. Any objections?
No objections. Just a tip. I tend to look at kernel.org main page to see
the stable and long-term kernel listed. So 4.7+ and 4.5+ have same
meaning as 4.5 and 4.6 are not stable/long-term kernels.
Regards,
Arend
> Let me see if patchwork with pick Cc tag as it does for others.
>
> Cc: [email protected] # 4.5+
>
> This may be worth backporting to 4.4 as well (as it's longterm), but
> I'll do it separately due to patch not applying cleanly.
Rafa=C5=82 Mi=C5=82ecki <[email protected]> writes:
>>>>> Kalle: this isn't important enough for 4.8 as it's too late for that.
>>>>>
>>>>> I'd like to get it for 4.9 however, as this fixes bug that could lead
>>>>> to WARNING on every add_key/del_key call. We was struggling with these
>>>>> WARNINGs for some time and this fixes one of two problems causing the=
m.
>>>
>>> Ok, I'll queue this for 4.9.
>>>
>>>> Please mark it for stable as well.
>>>
>>> I can add that. Any ideas how old releases stable releases should this
>>> go to?
>>
>> I was analyzing this.
>> 1) This patch uses brcmf_get_ifp which is available in 4.4+ only.
>> 2) It applies cleanly to 4.5+ only due to 32f90caa7debd ("brcmfmac:
>> Increase nr of supported flowrings.")
>> 3) 4.4 would also require applying to the patch without broadcom/ subdir
>>
>> That said I suggest 4.5+. Any objections?
>
> Let me see if patchwork with pick Cc tag as it does for others.
>
> Cc: [email protected] # 4.5+
An excellent idea but no luck:
Signed-off-by: Rafa? Mi?ecki <[email protected]>
Acked-by: Arend van Spriel <[email protected]>
I'll add this to my patchwork wishlist though, I think it would be a
really useful feature to have.
(The question marks are because of my buggy copy paste, ignore those)
--=20
Kalle Valo
On 27 September 2016 at 13:27, Kalle Valo <[email protected]> wrote:
> Arend Van Spriel <[email protected]> writes:
>
>> On 27-9-2016 11:14, Rafa=C5=82 Mi=C5=82ecki wrote:
>>> From: Rafa=C5=82 Mi=C5=82ecki <[email protected]>
>>>
>>> Flowrings contain skbs waiting for transmission that were passed to us
>>> by netif. It means we checked every one of them looking for 802.1x
>>> Ethernet type. When deleting flowring we have to use freeing function
>>> that will check for 802.1x type as well.
>>>
>>> Freeing skbs without a proper check was leading to counter not being
>>> properly decreased. This was triggering a WARNING every time
>>> brcmf_netdev_wait_pend8021x was called.
>>
>> Acked-by: Arend van Spriel <[email protected]>
>>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <[email protected]>
>>> ---
>>> Kalle: this isn't important enough for 4.8 as it's too late for that.
>>>
>>> I'd like to get it for 4.9 however, as this fixes bug that could lead
>>> to WARNING on every add_key/del_key call. We was struggling with these
>>> WARNINGs for some time and this fixes one of two problems causing them.
>
> Ok, I'll queue this for 4.9.
>
>> Please mark it for stable as well.
>
> I can add that. Any ideas how old releases stable releases should this
> go to?
I was analyzing this.
1) This patch uses brcmf_get_ifp which is available in 4.4+ only.
2) It applies cleanly to 4.5+ only due to 32f90caa7debd ("brcmfmac:
Increase nr of supported flowrings.")
3) 4.4 would also require applying to the patch without broadcom/ subdir
That said I suggest 4.5+. Any objections?
--=20
Rafa=C5=82
Rafał Miłecki wrote:
> From: Rafał Miłecki <[email protected]>
>
> Flowrings contain skbs waiting for transmission that were passed to us
> by netif. It means we checked every one of them looking for 802.1x
> Ethernet type. When deleting flowring we have to use freeing function
> that will check for 802.1x type as well.
>
> Freeing skbs without a proper check was leading to counter not being
> properly decreased. This was triggering a WARNING every time
> brcmf_netdev_wait_pend8021x was called.
>
> Signed-off-by: Rafał Miłecki <[email protected]>
> Acked-by: Arend van Spriel <[email protected]>
> Cc: [email protected] # 4.5+
Patch applied to wireless-drivers-next.git, thanks.
7f00ee2bbc63 brcmfmac: use correct skb freeing helper when deleting flowring
--
https://patchwork.kernel.org/patch/9351797/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 27-9-2016 11:14, Rafał Miłecki wrote:
> From: Rafał Miłecki <[email protected]>
>
> Flowrings contain skbs waiting for transmission that were passed to us
> by netif. It means we checked every one of them looking for 802.1x
> Ethernet type. When deleting flowring we have to use freeing function
> that will check for 802.1x type as well.
>
> Freeing skbs without a proper check was leading to counter not being
> properly decreased. This was triggering a WARNING every time
> brcmf_netdev_wait_pend8021x was called.
Acked-by: Arend van Spriel <[email protected]>
> Signed-off-by: Rafał Miłecki <[email protected]>
> ---
> Kalle: this isn't important enough for 4.8 as it's too late for that.
>
> I'd like to get it for 4.9 however, as this fixes bug that could lead
> to WARNING on every add_key/del_key call. We was struggling with these
> WARNINGs for some time and this fixes one of two problems causing them.
Please mark it for stable as well.
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
> index b16b367..d0b738d 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
> @@ -234,13 +234,20 @@ static void brcmf_flowring_block(struct brcmf_flowring *flow, u16 flowid,
>
> void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 flowid)
> {
> + struct brcmf_bus *bus_if = dev_get_drvdata(flow->dev);
> struct brcmf_flowring_ring *ring;
> + struct brcmf_if *ifp;
> u16 hash_idx;
> + u8 ifidx;
> struct sk_buff *skb;
>
> ring = flow->rings[flowid];
> if (!ring)
> return;
> +
> + ifidx = brcmf_flowring_ifidx_get(flow, flowid);
> + ifp = brcmf_get_ifp(bus_if->drvr, ifidx);
> +
> brcmf_flowring_block(flow, flowid, false);
> hash_idx = ring->hash_id;
> flow->hash[hash_idx].ifidx = BRCMF_FLOWRING_INVALID_IFIDX;
I am not very familiar with flowring code, but I suppose this is just
initializing the entry for later use, right?
> @@ -249,7 +256,7 @@ void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 flowid)
>
> skb = skb_dequeue(&ring->skblist);
> while (skb) {
> - brcmu_pkt_buf_free_skb(skb);
> + brcmf_txfinalize(ifp, skb, false);
> skb = skb_dequeue(&ring->skblist);
> }
>
>