2010/5/25 Weedy <[email protected]>:
> 2010/5/23 G?bor Stefanik <[email protected]>:
>> In the meantime, one thing to test: Add a printk of sc->opmode.
>
> May 24 22:04:20 tiny-h4x kernel: [41147.243149] sc->opmode: 02 (over9000 times)
>
> So i'm guessing I did it wrong (I don't know C).
> ? ? ? ?printk(KERN_NOTICE "sc->opmode: %02x\n", sc->opmode);
>
No, that is correct, and proves my theory (2 is NL80211_IFTYPE_STATION
- it should be 6 for monitor mode).
BTW, please use "Reply to all".
--
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
On Thu, May 27, 2010 at 02:31:12PM -0400, Bob Copeland wrote:
> Honestly I don't know what reported-by policy is (other than I should
> get your consent first). I guess John can weigh in whether:
>
> Reported-by: [email protected]
>
> is acceptable. For S-o-b, the rule is no anonymous or pseudonymous
> contributions. Of course, you don't have to be credited, but your
> bisection was a huge help in pinning it down :)
Reported-by: Weedy ([email protected])
Is fine. (And I don't know C so I doubt I'll need to use my real name)
2010/5/26 Bob Copeland <[email protected]>:
> 2010/5/25 Gábor Stefanik <[email protected]>:
>> On Tue, May 25, 2010 at 4:53 PM, Bob Copeland <[email protected]> wrote:
>>> 2010/5/25 Weedy <[email protected]>:
>>>> 2010/5/24 Gábor Stefanik <[email protected]>:
>>>>> 2010/5/25 Weedy <[email protected]>:
>>>>>> 2010/5/23 Gábor Stefanik <[email protected]>:
>>>>>>> In the meantime, one thing to test: Add a printk of sc->opmode.
>>>>>>
>>>>>> May 24 22:04:20 tiny-h4x kernel: [41147.243149] sc->opmode: 02 (over9000 times)
>>>>>>
>>>>>> So i'm guessing I did it wrong (I don't know C).
>>>>>> printk(KERN_NOTICE "sc->opmode: %02x\n", sc->opmode);
>>>>>>
>>>>>
>>>>> No, that is correct, and proves my theory (2 is NL80211_IFTYPE_STATION
>>>>> - it should be 6 for monitor mode).
>>>>>
>>>>> BTW, please use "Reply to all".
>>>>>
>>>> gmail got rid of the "Reply to all by default" option :<
>>>>
>>>> When you have a patch I will be waiting.
>>>
>>> Sorry, I missed this thread somehow. Thanks for the detective
>>> work and apologies for my stupid goof. Gábor, are you prepping
>>> a patch? I can fix it if you like.
>>>
>>
>> If you can, please fix it - I know what the bug is, but have no solid
>> idea about a fix.
>
> Ok, it should be enough to look at the filter flags instead of
> the opmode -- I knew in the back of my mind that the monitor
> stuff was bogus (part of the reason I did the patch in the first
> place) but just got confused by what was already there I guess.
>
>
I await your patch with open arms.
2010/5/24 Gábor Stefanik <[email protected]>:
> 2010/5/25 Weedy <[email protected]>:
>> 2010/5/23 Gábor Stefanik <[email protected]>:
>>> In the meantime, one thing to test: Add a printk of sc->opmode.
>>
>> May 24 22:04:20 tiny-h4x kernel: [41147.243149] sc->opmode: 02 (over9000 times)
>>
>> So i'm guessing I did it wrong (I don't know C).
>> printk(KERN_NOTICE "sc->opmode: %02x\n", sc->opmode);
>>
>
> No, that is correct, and proves my theory (2 is NL80211_IFTYPE_STATION
> - it should be 6 for monitor mode).
>
> BTW, please use "Reply to all".
>
gmail got rid of the "Reply to all by default" option :<
When you have a patch I will be waiting.
Bob Copeland wrote:
> On Wed, May 26, 2010 at 11:49 PM, Richard Farina <[email protected]> wrote:
>
>> Bob Copeland wrote:
>>
>>> Ok, it should be enough to look at the filter flags instead of
>>> the opmode -- I knew in the back of my mind that the monitor
>>> stuff was bogus (part of the reason I did the patch in the first
>>> place) but just got confused by what was already there I guess.
>>>
>> I've got a lot of people very interested in this fix. Let me know what kind
>> of support you need to make this happen. You know where to find me on irc
>> ;-)
>>
>
> Ok, can you and Weedy try this patch?
>
> Use the attachment -- gmail will screw up the whitespace, but I included
> it inline for reference.
>
> Weedy, if you want reported-by credit can you give your full name and
> preferred email address?
>
> From: Bob Copeland <[email protected]>
> Date: Thu, 27 May 2010 08:54:38 -0400
> Subject: [PATCH] ath5k: retain promiscuous setting
>
> Commit 56d1de0a21db28e41741cfa0a66e18bc8d920554, "ath5k: clean up
> filter flags setting" introduced a regression in monitor mode such
> that the promisc filter flag would get lost.
>
> Although we set the promisc flag when it changed, we did not
> preserve it across subsequent calls to configure_filter. This patch
> restores the original functionality.
>
> Cc: [email protected]
> Signed-off-by: Bob Copeland <[email protected]>
> ---
>
> Note, a better fix would be to just unconditionally look at new_flags,
> but this is the minimal change for stable. I'll add fixing all this
> stuff up to my todo.
>
> drivers/net/wireless/ath/ath5k/base.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/base.c
> b/drivers/net/wireless/ath/ath5k/base.c
> index 9c27623..9e023b8 100644
> --- a/drivers/net/wireless/ath/ath5k/base.c
> +++ b/drivers/net/wireless/ath/ath5k/base.c
> @@ -3153,13 +3153,15 @@ static void ath5k_configure_filter(struct
> ieee80211_hw *hw,
>
> if (changed_flags & (FIF_PROMISC_IN_BSS | FIF_OTHER_BSS)) {
> if (*new_flags & FIF_PROMISC_IN_BSS) {
> - rfilt |= AR5K_RX_FILTER_PROM;
> __set_bit(ATH_STAT_PROMISC, sc->status);
> } else {
> __clear_bit(ATH_STAT_PROMISC, sc->status);
> }
> }
>
> + if (test_bit(ATH_STAT_PROMISC, sc->status))
> + rfilt |= AR5K_RX_FILTER_PROM;
> +
> /* Note, AR5K_RX_FILTER_MCAST is already enabled */
> if (*new_flags & FIF_ALLMULTI) {
> mfilt[0] = ~0;
>
Tested-By: Rick Farina
This patch fixes the problem, data packets are now captured while in
monitor mode. Incidentally when I tried to chase this back to see when
it started I found the same behavior in 2.6.32_rc and even in 2.6.29
(which is funny since Weedy bisected the change to some time during
2.6.31_rc5). This fix should be pushed out to as much of stable as
possible as soon as possible. Thanks for the patch Bob!
-Rick Farina
2010/5/25 G?bor Stefanik <[email protected]>:
> On Tue, May 25, 2010 at 4:53 PM, Bob Copeland <[email protected]> wrote:
>> 2010/5/25 Weedy <[email protected]>:
>>> 2010/5/24 G?bor Stefanik <[email protected]>:
>>>> 2010/5/25 Weedy <[email protected]>:
>>>>> 2010/5/23 G?bor Stefanik <[email protected]>:
>>>>>> In the meantime, one thing to test: Add a printk of sc->opmode.
>>>>>
>>>>> May 24 22:04:20 tiny-h4x kernel: [41147.243149] sc->opmode: 02 (over9000 times)
>>>>>
>>>>> So i'm guessing I did it wrong (I don't know C).
>>>>> ? ? ? ?printk(KERN_NOTICE "sc->opmode: %02x\n", sc->opmode);
>>>>>
>>>>
>>>> No, that is correct, and proves my theory (2 is NL80211_IFTYPE_STATION
>>>> - it should be 6 for monitor mode).
>>>>
>>>> BTW, please use "Reply to all".
>>>>
>>> gmail got rid of the "Reply to all by default" option :<
>>>
>>> When you have a patch I will be waiting.
>>
>> Sorry, I missed this thread somehow. ?Thanks for the detective
>> work and apologies for my stupid goof. ?G?bor, are you prepping
>> a patch? ?I can fix it if you like.
>>
>
> If you can, please fix it - I know what the bug is, but have no solid
> idea about a fix.
Ok, it should be enough to look at the filter flags instead of
the opmode -- I knew in the back of my mind that the monitor
stuff was bogus (part of the reason I did the patch in the first
place) but just got confused by what was already there I guess.
--
Bob Copeland %% http://www.bobcopeland.com
On Thu, May 27, 2010 at 02:31:12PM -0400, Bob Copeland wrote:
> On Thu, May 27, 2010 at 1:40 PM, Weedy <[email protected]> wrote:
>
> >> Weedy, if you want reported-by credit can you give your full name and
> >> preferred email address?
> > Do I have to? I don't really want to tie my name to this email :/
>
> Honestly I don't know what reported-by policy is (other than I should
> get your consent first). I guess John can weigh in whether:
>
> Reported-by: [email protected]
>
> is acceptable. For S-o-b, the rule is no anonymous or pseudonymous
> contributions. Of course, you don't have to be credited, but your
> bisection was a huge help in pinning it down :)
It isn't necessary, and I suspect that someone that doesn't want his
name attached doesn't wanted the addition of any further unnecessary
evidence in the git changelogs either. :-)
John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
On Tue, May 25, 2010 at 4:53 PM, Bob Copeland <[email protected]> wrote:
> 2010/5/25 Weedy <[email protected]>:
>> 2010/5/24 G?bor Stefanik <[email protected]>:
>>> 2010/5/25 Weedy <[email protected]>:
>>>> 2010/5/23 G?bor Stefanik <[email protected]>:
>>>>> In the meantime, one thing to test: Add a printk of sc->opmode.
>>>>
>>>> May 24 22:04:20 tiny-h4x kernel: [41147.243149] sc->opmode: 02 (over9000 times)
>>>>
>>>> So i'm guessing I did it wrong (I don't know C).
>>>> ? ? ? ?printk(KERN_NOTICE "sc->opmode: %02x\n", sc->opmode);
>>>>
>>>
>>> No, that is correct, and proves my theory (2 is NL80211_IFTYPE_STATION
>>> - it should be 6 for monitor mode).
>>>
>>> BTW, please use "Reply to all".
>>>
>> gmail got rid of the "Reply to all by default" option :<
>>
>> When you have a patch I will be waiting.
>
> Sorry, I missed this thread somehow. ?Thanks for the detective
> work and apologies for my stupid goof. ?G?bor, are you prepping
> a patch? ?I can fix it if you like.
>
If you can, please fix it - I know what the bug is, but have no solid
idea about a fix.
(The long-term fix of course would be to make mac80211 say
NL80211_IFTYPE_MONITOR for monitor interfaces, but AFAIK it is
impossible or unwanted - Johannes can probably shed more light on
this. One thing is sure: saying NL80211_IFTYPE_STATION when only a
monitor interface is active is wrong; it should be IFTYPE_MONITOR, or
if that is impossible, IFTYPE_INVALID.)
--
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
On Thu, May 27, 2010 at 10:31 AM, Bob Copeland <[email protected]> wrote:
>
> Ok, can you and Weedy try this patch?
Works for me but I only have 2 active APs around me at work. When I
get home I'll have access to a cloud.
> Weedy, if you want reported-by credit can you give your full name and
> preferred email address?
Do I have to? I don't really want to tie my name to this email :/
Bob Copeland wrote:
> 2010/5/25 G?bor Stefanik <[email protected]>:
>
>> On Tue, May 25, 2010 at 4:53 PM, Bob Copeland <[email protected]> wrote:
>>
>>> 2010/5/25 Weedy <[email protected]>:
>>>
>>>> 2010/5/24 G?bor Stefanik <[email protected]>:
>>>>
>>>>> 2010/5/25 Weedy <[email protected]>:
>>>>>
>>>>>> 2010/5/23 G?bor Stefanik <[email protected]>:
>>>>>>
>>>>>>> In the meantime, one thing to test: Add a printk of sc->opmode.
>>>>>>>
>>>>>> May 24 22:04:20 tiny-h4x kernel: [41147.243149] sc->opmode: 02 (over9000 times)
>>>>>>
>>>>>> So i'm guessing I did it wrong (I don't know C).
>>>>>> printk(KERN_NOTICE "sc->opmode: %02x\n", sc->opmode);
>>>>>>
>>>>>>
>>>>> No, that is correct, and proves my theory (2 is NL80211_IFTYPE_STATION
>>>>> - it should be 6 for monitor mode).
>>>>>
>>>>> BTW, please use "Reply to all".
>>>>>
>>>>>
>>>> gmail got rid of the "Reply to all by default" option :<
>>>>
>>>> When you have a patch I will be waiting.
>>>>
>>> Sorry, I missed this thread somehow. Thanks for the detective
>>> work and apologies for my stupid goof. G?bor, are you prepping
>>> a patch? I can fix it if you like.
>>>
>>>
>> If you can, please fix it - I know what the bug is, but have no solid
>> idea about a fix.
>>
>
> Ok, it should be enough to look at the filter flags instead of
> the opmode -- I knew in the back of my mind that the monitor
> stuff was bogus (part of the reason I did the patch in the first
> place) but just got confused by what was already there I guess.
>
>
I've got a lot of people very interested in this fix. Let me know what
kind of support you need to make this happen. You know where to find me
on irc ;-)
Thanks,
Rick
On Wed, May 26, 2010 at 11:49 PM, Richard Farina <[email protected]> wrote:
> Bob Copeland wrote:
>> Ok, it should be enough to look at the filter flags instead of
>> the opmode -- I knew in the back of my mind that the monitor
>> stuff was bogus (part of the reason I did the patch in the first
>> place) but just got confused by what was already there I guess.
>
> I've got a lot of people very interested in this fix. Let me know what kind
> of support you need to make this happen. ?You know where to find me on irc
> ;-)
Ok, can you and Weedy try this patch?
Use the attachment -- gmail will screw up the whitespace, but I included
it inline for reference.
Weedy, if you want reported-by credit can you give your full name and
preferred email address?
From: Bob Copeland <[email protected]>
Date: Thu, 27 May 2010 08:54:38 -0400
Subject: [PATCH] ath5k: retain promiscuous setting
Commit 56d1de0a21db28e41741cfa0a66e18bc8d920554, "ath5k: clean up
filter flags setting" introduced a regression in monitor mode such
that the promisc filter flag would get lost.
Although we set the promisc flag when it changed, we did not
preserve it across subsequent calls to configure_filter. This patch
restores the original functionality.
Cc: [email protected]
Signed-off-by: Bob Copeland <[email protected]>
---
Note, a better fix would be to just unconditionally look at new_flags,
but this is the minimal change for stable. I'll add fixing all this
stuff up to my todo.
drivers/net/wireless/ath/ath5k/base.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath/ath5k/base.c
b/drivers/net/wireless/ath/ath5k/base.c
index 9c27623..9e023b8 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -3153,13 +3153,15 @@ static void ath5k_configure_filter(struct
ieee80211_hw *hw,
if (changed_flags & (FIF_PROMISC_IN_BSS | FIF_OTHER_BSS)) {
if (*new_flags & FIF_PROMISC_IN_BSS) {
- rfilt |= AR5K_RX_FILTER_PROM;
__set_bit(ATH_STAT_PROMISC, sc->status);
} else {
__clear_bit(ATH_STAT_PROMISC, sc->status);
}
}
+ if (test_bit(ATH_STAT_PROMISC, sc->status))
+ rfilt |= AR5K_RX_FILTER_PROM;
+
/* Note, AR5K_RX_FILTER_MCAST is already enabled */
if (*new_flags & FIF_ALLMULTI) {
mfilt[0] = ~0;
--
1.6.3.3
--
Bob Copeland %% http://www.bobcopeland.com
On Thu, May 27, 2010 at 1:40 PM, Weedy <[email protected]> wrote:
>> Weedy, if you want reported-by credit can you give your full name and
>> preferred email address?
> Do I have to? I don't really want to tie my name to this email :/
Honestly I don't know what reported-by policy is (other than I should
get your consent first). I guess John can weigh in whether:
Reported-by: [email protected]
is acceptable. For S-o-b, the rule is no anonymous or pseudonymous
contributions. Of course, you don't have to be credited, but your
bisection was a huge help in pinning it down :)
--
Bob Copeland %% http://www.bobcopeland.com
2010/5/25 Weedy <[email protected]>:
> 2010/5/24 G?bor Stefanik <[email protected]>:
>> 2010/5/25 Weedy <[email protected]>:
>>> 2010/5/23 G?bor Stefanik <[email protected]>:
>>>> In the meantime, one thing to test: Add a printk of sc->opmode.
>>>
>>> May 24 22:04:20 tiny-h4x kernel: [41147.243149] sc->opmode: 02 (over9000 times)
>>>
>>> So i'm guessing I did it wrong (I don't know C).
>>> ? ? ? ?printk(KERN_NOTICE "sc->opmode: %02x\n", sc->opmode);
>>>
>>
>> No, that is correct, and proves my theory (2 is NL80211_IFTYPE_STATION
>> - it should be 6 for monitor mode).
>>
>> BTW, please use "Reply to all".
>>
> gmail got rid of the "Reply to all by default" option :<
>
> When you have a patch I will be waiting.
Sorry, I missed this thread somehow. Thanks for the detective
work and apologies for my stupid goof. G?bor, are you prepping
a patch? I can fix it if you like.
--
Bob Copeland %% http://www.bobcopeland.com