2011-10-07 16:17:52

by Shahar Lev

[permalink] [raw]
Subject: [PATCH] wl12xx: remove warning message during IBSS Tx

mac80211 sets the carrier on an IBSS interface even when no network is
joined. Ignore garbage frames transmitted on a disconnected IBSS
interface without printing warnings.

Signed-off-by: Shahar Lev <[email protected]>
---
drivers/net/wireless/wl12xx/tx.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
index 1b3d8e3..a3b474b 100644
--- a/drivers/net/wireless/wl12xx/tx.c
+++ b/drivers/net/wireless/wl12xx/tx.c
@@ -431,7 +431,15 @@ static int wl1271_prepare_tx_frame(struct wl1271 *wl, struct sk_buff *skb,
}
hlid = wl1271_tx_get_hlid(wl, vif, skb);
if (hlid == WL12XX_INVALID_LINK_ID) {
- wl1271_error("invalid hlid. dropping skb 0x%p", skb);
+ if (wlvif->bss_type == BSS_TYPE_IBSS &&
+ !test_bit(WL1271_FLAG_IBSS_JOINED, &wl->flags)) {
+ /* It's ok to drop packets when not joined to IBSS */
+ wl1271_debug(DEBUG_TX, "dropping skb while IBSS not "
+ " joined");
+ } else {
+ wl1271_error("invalid hlid. dropping skb 0x%p", skb);
+ }
+
return -EINVAL;
}

--
1.7.4.1



2011-10-07 21:03:52

by Shahar Lev

[permalink] [raw]
Subject: Re: [PATCH] wl12xx: remove warning message during IBSS Tx

On Fri, Oct 7, 2011 at 6:20 PM, Johannes Berg <[email protected]> wrote:
> Maybe you should also change that? :-)

Consider it added to our collective TODO.

2011-10-07 16:20:43

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] wl12xx: remove warning message during IBSS Tx

On Fri, 2011-10-07 at 18:17 +0200, Shahar Lev wrote:
> mac80211 sets the carrier on an IBSS interface even when no network is
> joined.

Maybe you should also change that? :-)

johannes



2011-10-08 20:30:46

by Shahar Lev

[permalink] [raw]
Subject: Re: [PATCH] wl12xx: remove warning message during IBSS Tx

Part of a plot to increase the chances of my submissions getting
through by posing as Mr. Levi.

Thanks,
The Other Shahar

On Sat, Oct 8, 2011 at 10:02 PM, Luciano Coelho <[email protected]> wrote:
> On Sat, 2011-10-08 at 21:55 +0200, Arik Nemtsov wrote:
>> On Sat, Oct 8, 2011 at 21:45, Luciano Coelho <[email protected]> wrote:
>> >
>> > But what I meant with "the bug" was that, if this can be avoided in
>> > mac80211, why not do it there? Is it a a big effort to do it in
>> > mac80211?
>>
>> Not sure it's a big effort, but it requires testing etc. I'm afraid
>> we're all pretty busy with other stuff right now :)
>
> Not a very good excuse when it comes to upstreaming, but okay, I'll take
> this in anyway. :)
>
> Thanks, Shahar, and welcome! :)
>
> (BTW, Shahar Lev is not to be confused with Shahar Levi, as they are two
> different persons :)
>
> --
> Cheers,
> Luca.
>
>

2011-10-08 19:45:37

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH] wl12xx: remove warning message during IBSS Tx

On Sat, 2011-10-08 at 21:19 +0200, Arik Nemtsov wrote:
> On Fri, Oct 7, 2011 at 23:17, Luciano Coelho <[email protected]> wrote:
> > On Fri, 2011-10-07 at 23:03 +0200, Shahar Lev wrote:
> >> On Fri, Oct 7, 2011 at 6:20 PM, Johannes Berg <[email protected]> wrote:
> >> > Maybe you should also change that? :-)
> >>
> >> Consider it added to our collective TODO.
> >
> > Well, the fix is probably supposed to be done in mac80211, instead of
> > hiding the bug by removing the warning, isn't it?
>
> Sometimes data is sent when the IBSS interface is not connected.
> wl12xx handles this correctly by throwing the packets. In this sense
> there's no "bug".
> All this patch does is remove warning messages that confuse the users
> (and developers).
>
> I agree fixing mac80211 is the better thing to do, and we'll get to it
> when time permits.

Okay, not a big problem to do this in the driver.

But what I meant with "the bug" was that, if this can be avoided in
mac80211, why not do it there? Is it a a big effort to do it in
mac80211?


--
Cheers,
Luca.


2011-10-11 14:43:30

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH] wl12xx: remove warning message during IBSS Tx

On Fri, 2011-10-07 at 18:17 +0200, Shahar Lev wrote:
> mac80211 sets the carrier on an IBSS interface even when no network is
> joined. Ignore garbage frames transmitted on a disconnected IBSS
> interface without printing warnings.
>
> Signed-off-by: Shahar Lev <[email protected]>
> ---

Applied, thanks!

--
Cheers,
Luca.


2011-10-08 19:19:30

by Arik Nemtsov

[permalink] [raw]
Subject: Re: [PATCH] wl12xx: remove warning message during IBSS Tx

On Fri, Oct 7, 2011 at 23:17, Luciano Coelho <[email protected]> wrote:
> On Fri, 2011-10-07 at 23:03 +0200, Shahar Lev wrote:
>> On Fri, Oct 7, 2011 at 6:20 PM, Johannes Berg <[email protected]> wrote:
>> > Maybe you should also change that? :-)
>>
>> Consider it added to our collective TODO.
>
> Well, the fix is probably supposed to be done in mac80211, instead of
> hiding the bug by removing the warning, isn't it?

Sometimes data is sent when the IBSS interface is not connected.
wl12xx handles this correctly by throwing the packets. In this sense
there's no "bug".
All this patch does is remove warning messages that confuse the users
(and developers).

I agree fixing mac80211 is the better thing to do, and we'll get to it
when time permits.

Arik

2011-10-11 16:18:45

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH] wl12xx: remove warning message during IBSS Tx

On Tue, 2011-10-11 at 17:43 +0300, Luciano Coelho wrote:
> On Fri, 2011-10-07 at 18:17 +0200, Shahar Lev wrote:
> > mac80211 sets the carrier on an IBSS interface even when no network is
> > joined. Ignore garbage frames transmitted on a disconnected IBSS
> > interface without printing warnings.
> >
> > Signed-off-by: Shahar Lev <[email protected]>
> > ---
>
> Applied, thanks!

Actually I had to merge this with Eliad's vif patches, so I did this
before applying:

diff --git a/drivers/net/wireless/wl12xx/tx.c b/drivers/net/wireless/wl12xx/tx.c
index 1bc00ca..27a45e1 100644
--- a/drivers/net/wireless/wl12xx/tx.c
+++ b/drivers/net/wireless/wl12xx/tx.c
@@ -429,7 +429,7 @@ static int wl1271_prepare_tx_frame(struct wl1271 *wl, struct wl12xx_vif *wlvif,
hlid = wl12xx_tx_get_hlid(wl, wlvif, skb);
if (hlid == WL12XX_INVALID_LINK_ID) {
if (wlvif->bss_type == BSS_TYPE_IBSS &&
- !test_bit(WL1271_FLAG_IBSS_JOINED, &wl->flags)) {
+ !test_bit(WLVIF_FLAG_IBSS_JOINED, &wlvif->flags)) {
/* It's ok to drop packets when not joined to IBSS */
wl1271_debug(DEBUG_TX, "dropping skb while IBSS not "
" joined");


--
Cheers,
Luca.


2011-10-07 21:17:38

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH] wl12xx: remove warning message during IBSS Tx

On Fri, 2011-10-07 at 23:03 +0200, Shahar Lev wrote:
> On Fri, Oct 7, 2011 at 6:20 PM, Johannes Berg <[email protected]> wrote:
> > Maybe you should also change that? :-)
>
> Consider it added to our collective TODO.

Well, the fix is probably supposed to be done in mac80211, instead of
hiding the bug by removing the warning, isn't it?

--
Cheers,
Luca.


2011-10-08 20:02:50

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH] wl12xx: remove warning message during IBSS Tx

On Sat, 2011-10-08 at 21:55 +0200, Arik Nemtsov wrote:
> On Sat, Oct 8, 2011 at 21:45, Luciano Coelho <[email protected]> wrote:
> >
> > But what I meant with "the bug" was that, if this can be avoided in
> > mac80211, why not do it there? Is it a a big effort to do it in
> > mac80211?
>
> Not sure it's a big effort, but it requires testing etc. I'm afraid
> we're all pretty busy with other stuff right now :)

Not a very good excuse when it comes to upstreaming, but okay, I'll take
this in anyway. :)

Thanks, Shahar, and welcome! :)

(BTW, Shahar Lev is not to be confused with Shahar Levi, as they are two
different persons :)

--
Cheers,
Luca.


2011-10-08 19:55:48

by Arik Nemtsov

[permalink] [raw]
Subject: Re: [PATCH] wl12xx: remove warning message during IBSS Tx

On Sat, Oct 8, 2011 at 21:45, Luciano Coelho <[email protected]> wrote:
>
> But what I meant with "the bug" was that, if this can be avoided in
> mac80211, why not do it there? Is it a a big effort to do it in
> mac80211?

Not sure it's a big effort, but it requires testing etc. I'm afraid
we're all pretty busy with other stuff right now :)

Arik