LinuxLists.cc - pull request: wireless-2.6 2008-08-26

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wed, Aug 27, 2008 at 4:30 AM, John W. Linville
<[email protected]> wrote:
> Dave,
>
> Here is the latest round of fixes intended for 2.6.27. I think
> the non-iwlwifi ones are pretty solid. As for the iwlwifi ones,
> the Intel guys assure me that these are important changes.
>
> You will notice that (despite my exhortations) no less than two of
> the iwlwifi patches declare themselves as cleanups. I have included
> them anyway because the later fixes depend on them and they seemed
> harmless enough. Plus, they were posted (or at least written) before
> the recent fireworks about what we should be merging after -rc1...
>
PCI cleanup patch is badly named. It fixes PCI registers sizes and ads
one forgotten hw workaround
I called it cleanup because it went through all PCI accesses we have
in the driver.

Scan cleanup was requested because of dependencies but it's harmless,
we can rebase the patches
prefer not it's just waste of time. Afther massive ath9k merge I
didn't see it was a conceptual problem.

Thanks
Tomas

2008-08-27 08:40:59

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

From: "Tomas Winkler" <[email protected]>
Date: Wed, 27 Aug 2008 10:38:56 +0300

> Afther massive ath9k merge I didn't see it was a conceptual problem.

Stop right there.

You don't see it as a problem because you're not the one who has to
explain all of this shit to Linus and ends up getting flamed by him
when inappropriate changes are in my tree outside of the merge window.

Think for just one second about the people who are upstream from you
who have to deal with all of these issues, and not some selfish aspect
such as what is convenient and nice for you and your driver, and what
other people "got away with".

2008-08-27 09:10:39

by Marcel Holtmann

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

Hi Dave,

> > Afther massive ath9k merge I didn't see it was a conceptual problem.
>
> Stop right there.
>
> You don't see it as a problem because you're not the one who has to
> explain all of this shit to Linus and ends up getting flamed by him
> when inappropriate changes are in my tree outside of the merge window.

I did a quick review of the offending patch queue. And some of them
should go in since they are fixing real bugs (as far as I can tell from
a quick review). Others have to clearly wait for the next merge window.

Thomas, please go through the list of patches and pick the real bug
fixes. The cleanup patches can't go in since they are after the merge
window. And being one of the offenders why Dave got flamed by Linus,
there is no way to get these through. And my big offender was a
regression that broke userspace and came with a lengthy commit message.

Speaking of which, you better have a more convincing commit message if
you wanna get something in after the merge window. It should make clear
why this is a bug or a regression and why it can't wait until the next
merge window.

Please take this into account for any future patches since Linus now set
a really high bar for patches after the merge window.

Regards

Marcel

2008-08-27 09:13:46

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wed, Aug 27, 2008 at 11:40 AM, David Miller <[email protected]> wrote:
> From: "Tomas Winkler" <[email protected]>
> Date: Wed, 27 Aug 2008 10:38:56 +0300
>
>> Afther massive ath9k merge I didn't see it was a conceptual problem.
>
> Stop right there.
>
> You don't see it as a problem because you're not the one who has to
> explain all of this shit to Linus and ends up getting flamed by him
> when inappropriate changes are in my tree outside of the merge window.

> Think for just one second about the people who are upstream from you
> who have to deal with all of these issues, and not some selfish aspect
> such as what is convenient and nice for you and your driver, and what
> other people "got away with".

Don't get excited, this was a cynical remarks rather then anything
else. I understand all the aspects of this. Upstream problems are not
bigger then down here, Without offense you also see your world from
your selfish perspective.
Tomas

2008-08-27 10:05:41

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wed, Aug 27, 2008 at 2:10 PM, Marcel Holtmann
<[email protected]> wrote:
> Hi Dave,
>
>> > Afther massive ath9k merge I didn't see it was a conceptual problem.
>>
>> Stop right there.
>>
>> You don't see it as a problem because you're not the one who has to
>> explain all of this shit to Linus and ends up getting flamed by him
>> when inappropriate changes are in my tree outside of the merge window.
>
> I did a quick review of the offending patch queue. And some of them
> should go in since they are fixing real bugs (as far as I can tell from
> a quick review). Others have to clearly wait for the next merge window.
>
> Thomas, please go through the list of patches and pick the real bug
> fixes. The cleanup patches can't go in since they are after the merge
> window.

I just repeat myself again, for the current series except the scanning
cleaning patches these patches
are must. This particular patch is there just because the hidden AP
connectivity fixes were stacked above it.

And being one of the offenders why Dave got flamed by Linus,
> there is no way to get these through. And my big offender was a
> regression that broke userspace and came with a lengthy commit message.

There is a central problem here, since I cannot really adjust driver
development progress from obvious reason with Linux merging window.
Upstream kernel is unfortunately not the only customer I report to. So
actually stable versions got half backed drivers depending on some
arbitrary time cut from my (selfish) development point of view.
In this particular point we are really only cleaning bug database, our
mistake here is that we didn't export these bug to external bugzilla.

> Speaking of which, you better have a more convincing commit message if
> you wanna get something in after the merge window. It should make clear
> why this is a bug or a regression and why it can't wait until the next
> merge window.

Yes, I've already got this message on 3 channels already :)

> Please take this into account for any future patches since Linus now set
> a really high bar for patches after the merge window.

Understood.
Tomas

2008-08-27 10:11:06

by Johannes Berg

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wed, 2008-08-27 at 13:05 +0300, Tomas Winkler wrote:

> There is a central problem here, since I cannot really adjust driver
> development progress from obvious reason with Linux merging window.
> Upstream kernel is unfortunately not the only customer I report to. So
> actually stable versions got half backed drivers depending on some
> arbitrary time cut from my (selfish) development point of view.

FWIW, many people including myself think you should work the other way
around, that is develop things in the mainline/wireless-testing tree
rather than developing a stable version internally and then dumping a
big pile of patches whenever it's convenient for you. IOW, we think you
should use Linux upstream as your development tree and then branch off
of that for the internal stabilisation trees you need for other
customers, rather than having some sort of internal development tree and
branching out Linux upstream as you appear to work.

That would also avoid the nasty surprises you've gotten a few times
where other people managed to get patches to iwlwifi drivers into the
Linux tree without you noticing.

johannes

Attachments:

signature.asc (836.00 B)
This is a digitally signed message part

2008-08-27 10:32:54

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

From: "Tomas Winkler" <[email protected]>
Date: Wed, 27 Aug 2008 13:05:23 +0300

> There is a central problem here, since I cannot really adjust driver
> development progress from obvious reason with Linux merging window.

Indeed, that's exactly what your core problem is.

You think it's OK to contaminate upstream just because of the
way you have choosen to maintain your driver.

Well guess what, it's not OK to do that, and it's absolutely not our
problem that you choose to do your development in the most anti-social
way possible wrt. upstream.

Anyways, keep defending yourself.

If you persist, I can certainly make things _really_ easy, such that
if there isn't a regression list entry for the bug you are fixing and
it isn't mentioned explicitly in your commit message, I simply will
refuse to take your changes outside of the merge window.

It's that simple.

Would you like things to work that way, where I'm a hard ass, or would
you like to stop defending yourself endlessly, and instead start to be
reasonable?

Thanks.

2008-08-27 10:33:36

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

From: Johannes Berg <[email protected]>
Date: Wed, 27 Aug 2008 12:10:28 +0200

> FWIW, many people including myself think you should work the other way
> around, that is develop things in the mainline/wireless-testing tree
> rather than developing a stable version internally and then dumping a
> big pile of patches whenever it's convenient for you.

Absolutely.

2008-08-27 11:34:56

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wed, Aug 27, 2008 at 1:10 PM, Johannes Berg
<[email protected]> wrote:
> On Wed, 2008-08-27 at 13:05 +0300, Tomas Winkler wrote:
>
>> There is a central problem here, since I cannot really adjust driver
>> development progress from obvious reason with Linux merging window.
>> Upstream kernel is unfortunately not the only customer I report to. So
>> actually stable versions got half backed drivers depending on some
>> arbitrary time cut from my (selfish) development point of view.
>
> FWIW, many people including myself think you should work the other way
> around, that is develop things in the mainline/wireless-testing tree
> rather than developing a stable version internally
> big pile of patches whenever it's convenient for you.

We put big pile once the driver was functional and nobody expected it
to be merged into stable kernel from that point
we post everything that is mergable. The patches are sent out each
Friday after they got through internal review there is nothing, there
is nothing magical about it.

IOW, we think you
> should use Linux upstream as your development tree and then branch off
> of that for the internal stabilisation trees you need for other
> customers, rather than having some sort of internal development tree and
> branching out Linux upstream as you appear to work.

Unfortunately fixing bugs on stable branch take precedence of
adjusting to new API on development branch that someone decided to do.
I wanted to work directly on wireless testing but it was broken over
an over and I have only limited resources more in testing then in
development I just had to branch out to be ready with the driver when
HW is out. People just check the immediate impact of they fix the
don't test for collateral damage and this is understandable an
individual developer doesn't have lab with IBSS, BSS, AP, etc setups.

> That would also avoid the nasty surprises you've gotten a few times
> where other people managed to get patches to iwlwifi drivers into the
> Linux tree without you noticing.

I've noticed just deferred to fixing it.

In summary
This was my first Linux project, it was first project that was done on
new HW (nobody could test it but us) and I don't deny I did a lot of
mistake in development process I don't deny that and we are learning
from them.
Tomas

2008-08-27 11:40:00

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

From: "John W. Linville" <[email protected]>
Date: Tue, 26 Aug 2008 21:30:10 -0400

> I would appreciate it if we could merge this request as-is. If you
> are not comfortable with that, then I would ask you to pull from the
> 'no-iwlwifi' branch instead. In that case, at least we can get the
> less questionable patches pushed upstream ASAP.

I've pulled the no-iwlwifi branch, thanks John.

2008-08-27 11:42:58

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wed, Aug 27, 2008 at 1:32 PM, David Miller <[email protected]> wrote:
> From: "Tomas Winkler" <[email protected]>
> Date: Wed, 27 Aug 2008 13:05:23 +0300
>
>> There is a central problem here, since I cannot really adjust driver
>> development progress from obvious reason with Linux merging window.
>
> Indeed, that's exactly what your core problem is.
>
> You think it's OK to contaminate upstream just because of the
> way you have choosen to maintain your driver.
>

I don't think I contaminate upstream I'm just trying to fix bugs so
people can work with the HW.

> Well guess what, it's not OK to do that, and it's absolutely not our
> problem that you choose to do your development in the most anti-social
> way possible wrt. upstream.
>
> Anyways, keep defending yourself.

I'm not defending myself I'm explaining myself.

> If you persist, I can certainly make things _really_ easy, such that
> if there isn't a regression list entry for the bug you are fixing and
> it isn't mentioned explicitly in your commit message, I simply will
> refuse to take your changes outside of the merge window.
>
> It's that simple.
>
> Would you like things to work that way, where I'm a hard ass, or would
> you like to stop defending yourself endlessly, and instead start to be
> reasonable?

I'm not sure why you are so militant, you cut out form my email sentences
like 'Yes' and 'Understood'
Is this personal ?

Tomas

2008-08-27 11:46:15

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

From: "Tomas Winkler" <[email protected]>
Date: Wed, 27 Aug 2008 14:34:28 +0300

> Unfortunately fixing bugs on stable branch take precedence of
> adjusting to new API on development branch that someone decided to do.
> I wanted to work directly on wireless testing but it was broken over
> an over and I have only limited resources more in testing then in
> development I just had to branch out to be ready with the driver when
> HW is out. People just check the immediate impact of they fix the
> don't test for collateral damage and this is understandable an
> individual developer doesn't have lab with IBSS, BSS, AP, etc setups.

But think about this from the other perspective.

When you queue up tons of things, especially in infrastructure level
code such as mac80211, and on top of it you do your work on the stable
branch and do not do you work against the development tree, guess what
happens?

You show up with accumulated piles of non-trivial patches for people
to review. And then you'll get upset when they suggest that things be
implemented differently.

It's all because of the gap in time.

And during this time, if you had submitted earlier, you would end up
doing smaller and mode gradual modifications to your design. And
you'd take care of them before they effect subsequent pieces of work
you want to do which depend upon the earlier bits.

The longer you queue stuff up, the more painful having to change stuff
at the beginning of that queue becomes. It can invalidate everything
else you worked on.

The only sane way to operate is to post your work early and often, or
else you'll live in a world of hurt, and it will be nobody's fault but
your own.

2008-08-27 12:26:30

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wed, Aug 27, 2008 at 2:45 PM, David Miller <[email protected]> wrote:
> From: "Tomas Winkler" <[email protected]>
> Date: Wed, 27 Aug 2008 14:34:28 +0300
>
>> Unfortunately fixing bugs on stable branch take precedence of
>> adjusting to new API on development branch that someone decided to do.
>> I wanted to work directly on wireless testing but it was broken over
>> an over and I have only limited resources more in testing then in
>> development I just had to branch out to be ready with the driver when
>> HW is out. People just check the immediate impact of they fix the
>> don't test for collateral damage and this is understandable an
>> individual developer doesn't have lab with IBSS, BSS, AP, etc setups.
>
> But think about this from the other perspective.
>
> When you queue up tons of things, especially in infrastructure level
> code such as mac80211, and on top of it you do your work on the stable
> branch and do not do you work against the development tree, guess what
> happens?

> You show up with accumulated piles of non-trivial patches for people
> to review. And then you'll get upset when they suggest that things be
> implemented differently.

I never worked that way I've published immediately what I had. I don't
know where did you get this impression
If I didn't publish something it's just too reasons.
The code was made dirty and I didn't like design myself (for example
our 11h implementation), even though it's publicly available.
or just simply didn't have time because I have to satisfy first thous
who pays my check ( everybody to feed their children after all:) )
(e.g. rfkill fixes)
Almost every driver has it's own development branch, we didn't do
something different here, this is just classical divide and conquer
strategy.

> It's all because of the gap in time.
>
> And during this time, if you had submitted earlier, you would end up
> doing smaller and mode gradual modifications to your design. And
> you'd take care of them before they effect subsequent pieces of work
> you want to do which depend upon the earlier bits.
>
> The longer you queue stuff up, the more painful having to change stuff
> at the beginning of that queue becomes. It can invalidate everything
> else you worked on.
>
> The only sane way to operate is to post your work early and often, or
> else you'll live in a world of hurt, and it will be nobody's fault but
> your own.

This is all understood and this would be indeed the ideal case.
Tomas

2008-08-27 13:11:28

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wednesday 27 August 2008, Tomas Winkler wrote:
> > should use Linux upstream as your development tree and then branch off
> > of that for the internal stabilisation trees you need for other
> > customers, rather than having some sort of internal development tree and
> > branching out Linux upstream as you appear to work.
>
> Unfortunately fixing bugs on stable branch take precedence of
> adjusting to new API on development branch that someone decided to do.
> I wanted to work directly on wireless testing but it was broken over
> an over and I have only limited resources more in testing then in
> development I just had to branch out to be ready with the driver when
> HW is out. People just check the immediate impact of they fix the
> don't test for collateral damage and this is understandable an
> individual developer doesn't have lab with IBSS, BSS, AP, etc setups.

I think it would be an advantage for you to develop in wireless-testing
and use wireless-2.6 and probably additionally some internal trees as
stable trees. Like everybody else does it.

The advantage is simple: You get more people testing your code.

2008-08-27 14:09:20

by Arjan van de Ven

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wed, 27 Aug 2008 13:05:23 +0300
"Tomas Winkler" <[email protected]> wrote:

>
> There is a central problem here, since I cannot really adjust driver
> development progress from obvious reason with Linux merging window.
> Upstream kernel is unfortunately not the only customer I report to.

yes it is. You really don't have any other customers of this driver...
kernel.org kernel is *it*.

(if you're talking about backporting this to distros.. that's the
distros problem ;-)

--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2008-08-27 14:55:33

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wed, Aug 27, 2008 at 4:10 PM, Michael Buesch <[email protected]> wrote:
> On Wednesday 27 August 2008, Tomas Winkler wrote:
>> > should use Linux upstream as your development tree and then branch off
>> > of that for the internal stabilisation trees you need for other
>> > customers, rather than having some sort of internal development tree and
>> > branching out Linux upstream as you appear to work.
>>
>> Unfortunately fixing bugs on stable branch take precedence of
>> adjusting to new API on development branch that someone decided to do.
>> I wanted to work directly on wireless testing but it was broken over
>> an over and I have only limited resources more in testing then in
>> development I just had to branch out to be ready with the driver when
>> HW is out. People just check the immediate impact of they fix the
>> don't test for collateral damage and this is understandable an
>> individual developer doesn't have lab with IBSS, BSS, AP, etc setups.
>
> I think it would be an advantage for you to develop in wireless-testing
> and use wireless-2.6 and probably additionally some internal trees as
> stable trees. Like everybody else does it.

I'm not sure what you are taking about, we did post patches to
wireless-testing, the driver for 5000 HW is working in 2.6.27-rc4 I
personally use it every day. I'm sending bug fixes, like this seris
of patches is direct answer to bugs that were discovered very
recently, I'm not sure what's everybody problem here.

> The advantage is simple: You get more people testing your code.

We fixed bugs in wireless-testing/mac80211 that were there for months
just because we are only one that do any comprehensive validation. I
don't want insult anyone and I really exaggeration here but
sometimes I have feeling that more people are breaking the code.

Tomas

2008-08-27 15:23:41

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wednesday 27 August 2008 16:55:15 Tomas Winkler wrote:
> I'm not sure what you are taking about, we did post patches to
> wireless-testing, the driver for 5000 HW is working in 2.6.27-rc4 I
> personally use it every day. I'm sending bug fixes, like this seris
> of patches is direct answer to bugs that were discovered very
> recently, I'm not sure what's everybody problem here.

The problem is that these patches are posted in huge batches.
There are three problems (probably more) with that:
1) "Oh, one of these 15 patches recently merged broke my hw"
instead of
"This patch XXX recently merged broke my hw"
2) It's a _lot_ harder to review. I do not have the time to review
a lot of patches at once. I bet other people won't either.
3) If somebody has any objections against one patch, you'll probably
have to rebase them all (See recent pull request vs cleanups).
That's additional work for you.

What's the problem with sending patches upstream as soon as you finished
them, instead of piling them up locally and sending them alltogether?

> We fixed bugs in wireless-testing/mac80211 that were there for months
> just because we are only one that do any comprehensive validation. I
> don't want insult anyone and I really exaggeration here but
> sometimes I have feeling that more people are breaking the code.

If you work directly upstream, you will discover these bugs even faster.
If you maintain your local fork of mac80211, you will have a delay. But
you will have to handle them _anyway_. It's just delayed.

And please, don't act like you don't add bugs to mac80211. There were several
bugs that broke connections on b43 (and probably other hw, too) in the past.

--
Greetings Michael.

2008-08-27 15:45:52

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wed, Aug 27, 2008 at 6:22 PM, Michael Buesch <[email protected]> wrote:
> On Wednesday 27 August 2008 16:55:15 Tomas Winkler wrote:
>> I'm not sure what you are taking about, we did post patches to
>> wireless-testing, the driver for 5000 HW is working in 2.6.27-rc4 I
>> personally use it every day. I'm sending bug fixes, like this seris
>> of patches is direct answer to bugs that were discovered very
>> recently, I'm not sure what's everybody problem here.
>
> The problem is that these patches are posted in huge batches.
> There are three problems (probably more) with that:
> 1) "Oh, one of these 15 patches recently merged broke my hw"
> instead of
> "This patch XXX recently merged broke my hw"
> 2) It's a _lot_ harder to review. I do not have the time to review
> a lot of patches at once. I bet other people won't either.
> 3) If somebody has any objections against one patch, you'll probably
> have to rebase them all (See recent pull request vs cleanups).
> That's additional work for you.
>
> What's the problem with sending patches upstream as soon as you finished
> them, instead of piling them up locally and sending them alltogether?

First of all, mac patches are send immediately to wireless-dev for
review just because we don't want to break anyone works (just look at
the archives). iwlwifi patches are sent first to internal mailing
list for review and internal testing because nobody is really
reviewing these patches in wireless-dev and this is common practices
(see latest orinoco 19 patches dump) Internal mailing list is
really the only place I'm getting any feedback on iwlwifi code, they
are few exception of course.
Second I'm not sure why Yi is sending them only once a week I thought
he had some agreement with John, really don't know.

>> We fixed bugs in wireless-testing/mac80211 that were there for months
>> just because we are only one that do any comprehensive validation. I
>> don't want insult anyone and I really exaggeration here but
>> sometimes I have feeling that more people are breaking the code.
>
> If you work directly upstream, you will discover these bugs even faster.
> If you maintain your local fork of mac80211, you will have a delay. But
> you will have to handle them _anyway_. It's just delayed.

For exact reason we delay patches to 2.6.28, too keep stable.

>
> And please, don't act like you don't add bugs to mac80211. There were several
> bugs that broke connections on b43 (and probably other hw, too) in the past.

Not sure what exact patches are you referring to but I do a lot of
bugs no doubt and I really test only iwlwifi but we do broad features
testing IIRC I did IBSS test with b43 it's broken same way is iwlwifi
(look for TSF bug fix)
Thanks
Tomas

2008-08-27 19:26:46

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

> John W. Linville (1):
> mac80211: quiet chatty IBSS merge message

This patch is correct yet it suppresses an important warning, meaning
that you have constant IBSS reconnection, remove all connected station
and adding them again, This greatly degraded performance. This is
caused by inability to adjust to TSF of the IBSS leader

<snipt>
static int ieee80211_sta_join_ibss(struct net_device *dev,
struct ieee80211_if_sta *ifsta,
struct ieee80211_sta_bss *bss)
.....
/* Remove possible STA entries from other IBSS networks. */
sta_info_flush_delayed(sdata);
</snip>

> Assaf Krauss (1):
> iwlwifi: W/A for the TSF correction in IBSS
>

This patch fixes this problem in iwlwifi

We've observed same problem with the broadcom card

Tomas

2008-08-27 20:25:57

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wednesday 27 August 2008, Tomas Winkler wrote:
> > John W. Linville (1):
> > mac80211: quiet chatty IBSS merge message
>
> This patch is correct yet it suppresses an important warning, meaning
> that you have constant IBSS reconnection, remove all connected station
> and adding them again, This greatly degraded performance. This is
> caused by inability to adjust to TSF of the IBSS leader
>
> <snipt>
> static int ieee80211_sta_join_ibss(struct net_device *dev,
> struct ieee80211_if_sta *ifsta,
> struct ieee80211_sta_bss *bss)
> .....
> /* Remove possible STA entries from other IBSS networks. */
> sta_info_flush_delayed(sdata);
> </snip>

I fail to see how the TSF could be related to an ever reconnecting
station. Can you elaborate on what happens?

I was under the impression that the firmware would handle TSF stuff.
Also the "IBSS leader" is a new thing to me. I remember from the specs
that the device should accept the TSF from _any_ beacon. Not just a
"leader". Am I mislead? :)

I also fail to see how we could _ever_ set the TSF to something remotely
correct from the driver because of the FIFO delay.

>
> > Assaf Krauss (1):
> > iwlwifi: W/A for the TSF correction in IBSS

I cannot find this patch in wireless-testing and I don't
have a copy of wireless-2.6 here. Can you send me the patch, or
explain what it does?

2008-08-27 23:11:31

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wed, Aug 27, 2008 at 11:25 PM, Michael Buesch <[email protected]> wrote:
> On Wednesday 27 August 2008, Tomas Winkler wrote:
>> > John W. Linville (1):
>> > mac80211: quiet chatty IBSS merge message
>>
>> This patch is correct yet it suppresses an important warning, meaning
>> that you have constant IBSS reconnection, remove all connected station
>> and adding them again, This greatly degraded performance. This is
>> caused by inability to adjust to TSF of the IBSS leader
>>
>> <snipt>
>> static int ieee80211_sta_join_ibss(struct net_device *dev,
>> struct ieee80211_if_sta *ifsta,
>> struct ieee80211_sta_bss *bss)
>> .....
>> /* Remove possible STA entries from other IBSS networks. */
>> sta_info_flush_delayed(sdata);
>> </snip>
>
> I fail to see how the TSF could be related to an ever reconnecting
> station. Can you elaborate on what happens?
>
> I was under the impression that the firmware would handle TSF stuff.
> Also the "IBSS leader" is a new thing to me. I remember from the specs
> that the device should accept the TSF from _any_ beacon. Not just a
> "leader". Am I mislead? :)

What is happening that IBSS station should adopt TSF of the oldest
station i.e. with highest
TSF. This is also leader of the IBSS (this is not spec definition just
local jargon) Adaptation
mean we adjust to the same clock

if (beacon_timestamp > rx_timestamp)
merge

beacon_timestamp (what STA advertise in the beacon ) is bigger then
time reception of the beacon according our local TSF. If this is true
then there is merging
meaing we call reset_tsf, remove all the stations form the list and
add the leader.

In IBSS all the station race on sending beacon. so adjusting clock is
important for power save.

> I also fail to see how we could _ever_ set the TSF to something remotely
> correct from the driver because of the FIFO delay.
>
Exactly also iwlwifi driver is not able to really adjust TSF and
always lag behind we've really tried to make it work...no success.
The result is you are constantly removing and joining the same station.

The patch from Assaf just disable reporting RX timestamp to mac and
thus disabling merging which gives incorrect spec behavior but smooth
traffic.
Actually we've checked few cards including broadcom and various
windows NICs and non of them implements this correctly so this WA is
probably the solution.

Other solution would be to mark leader with highest TSF and not
reconnecting to the same station again and again.

Last solution would be to remove this merging all together but then
I'm not sure if Bruno added this code just implement the spec or
really tested it with any hardware. I'm not sure if any vendor
implements PS in IBSS so this merging is probably not important
anyway.

>>
>> > Assaf Krauss (1):
>> > iwlwifi: W/A for the TSF correction in IBSS
>
> I cannot find this patch in wireless-testing and I don't
> have a copy of wireless-2.6 here. Can you send me the patch, or
> explain what it does?
>

Explained above just disable get_tsf and also report to mac
by removing line rx_status.flag |= RX_FLAG_TSFT;

Tomas

2008-08-27 23:31:23

by Luis R. Rodriguez

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wed, Aug 27, 2008 at 04:11:15PM -0700, Tomas Winkler wrote:
> if (beacon_timestamp > rx_timestamp)
> merge
>
> The patch from Assaf just disable reporting RX timestamp to mac and
> thus disabling merging which gives incorrect spec behavior but smooth
> traffic.

We *should follow the spec*.

> Actually we've checked few cards including broadcom and various
> windows NICs

What are windows NICs?

> and non of them implements this correctly so this WA is
> probably the solution.

Disagreed! If there is hardware which is not capable of handling this
we should simply have a HW flag which specifies this to handle this as a
work around (WA). Just because some cards are not capable it doesn't
mean it should impose that on the rest.

> Other solution would be to mark leader with highest TSF and not
> reconnecting to the same station again and again.
>
> Last solution would be to remove this merging all together but then
> I'm not sure if Bruno added this code just implement the spec or
> really tested it with any hardware. I'm not sure if any vendor
> implements PS in IBSS so this merging is probably not important
> anyway.

Absolutely not! IBSS merge is per spec, otherwise you don't really have
a real IBSS.

Luis

2008-08-28 00:19:50

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Thu, Aug 28, 2008 at 2:31 AM, Luis R. Rodriguez
<[email protected]> wrote:
> On Wed, Aug 27, 2008 at 04:11:15PM -0700, Tomas Winkler wrote:
>> if (beacon_timestamp > rx_timestamp)
>> merge
>>
>> The patch from Assaf just disable reporting RX timestamp to mac and
>> thus disabling merging which gives incorrect spec behavior but smooth
>> traffic.
>
> We *should follow the spec*.

I prefer to be able transfer files over blindly following spec. This
doesn't really break interoperability in practice.

>> Actually we've checked few cards including broadcom and various
>> windows NICs
>
> What are windows NICs?
>
>> and non of them implements this correctly so this WA is
>> probably the solution.
>
> Disagreed! If there is hardware which is not capable of handling this
> we should simply have a HW flag which specifies this to handle this as a
> work around (WA). Just because some cards are not capable it doesn't
> mean it should impose that on the rest.

This is w/a in the specific driver not in the mac so this is de facto
what you have suggested. This patch is marked as W/A and should have
probably
some cleaner solution in 2.6.28

>> Other solution would be to mark leader with highest TSF and not
>> reconnecting to the same station again and again.
>>
>> Last solution would be to remove this merging all together but then
>> I'm not sure if Bruno added this code just implement the spec or
>> really tested it with any hardware. I'm not sure if any vendor
>> implements PS in IBSS so this merging is probably not important
>> anyway.
>
> Absolutely not! IBSS merge is per spec, otherwise you don't really have
> a real IBSS.

I'm all for following spec and we've tried really hard to make it work.
Anyhow I'm not sure what you call real IBSS. nobody has checked it for
1.5 year this patch is in and now is suddenly so important...
b43 was effected and probably also something that John worked with (as
he issued his anti-chatty patch)
I would like to see if there is any HW under mac80211 that actually
work with this.

Tomas

> Luis
>

2008-08-28 01:30:52

by Luis R. Rodriguez

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wed, Aug 27, 2008 at 5:19 PM, Tomas Winkler <[email protected]> wrote:
> This patch is marked as W/A and should have
> probably some cleaner solution in 2.6.28

OK

>>> Last solution would be to remove this merging all together
>>
>> Absolutely not! IBSS merge is per spec, otherwise you don't really have
>> a real IBSS.
>
> I'm all for following spec and we've tried really hard to make it work.
> Anyhow I'm not sure what you call real IBSS. nobody has checked it for
> 1.5 year this patch is in and now is suddenly so important...

I don't care about your work around, I was replying due to your insane
comments about considering removing IBSS merge support because your
hardware does not support it well.

> I would like to see if there is any HW under mac80211 that actually
> work with this.

Atheros hardware supports IBSS merge, please consider a proper work
around instead for hardware which does not support this.

Luis

2008-08-28 07:59:38

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Thu, Aug 28, 2008 at 4:30 AM, Luis R. Rodriguez
<[email protected]> wrote:
> On Wed, Aug 27, 2008 at 5:19 PM, Tomas Winkler <[email protected]> wrote:
>> This patch is marked as W/A and should have
>> probably some cleaner solution in 2.6.28
>
> OK
>
>>>> Last solution would be to remove this merging all together
>>>
>>> Absolutely not! IBSS merge is per spec, otherwise you don't really have
>>> a real IBSS.
>>
>> I'm all for following spec and we've tried really hard to make it work.
>> Anyhow I'm not sure what you call real IBSS. nobody has checked it for
>> 1.5 year this patch is in and now is suddenly so important...
>
> I don't care about your work around, I was replying due to your insane
> comments about considering removing IBSS merge support because your
> hardware does not support it well.

Insane?!, watch your language mister I will catch you on next year OLS
:) This is definitely
not any highlighted proposal in my message

>> I would like to see if there is any HW under mac80211 that actually
>> work with this.
>
> Atheros hardware supports IBSS merge, please consider a proper work
> around instead for hardware which does not support this.

So how do set TSF from the beacon, I'm asking because there is
currently no interface from mac80211 to driver do that.
Thanks
Tomas

2008-08-28 08:31:49

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Thursday 28 August 2008, Tomas Winkler wrote:
> On Wed, Aug 27, 2008 at 11:25 PM, Michael Buesch <[email protected]> wrote:
> > On Wednesday 27 August 2008, Tomas Winkler wrote:
> >> > John W. Linville (1):
> >> > mac80211: quiet chatty IBSS merge message
> >>
> >> This patch is correct yet it suppresses an important warning, meaning
> >> that you have constant IBSS reconnection, remove all connected station
> >> and adding them again, This greatly degraded performance. This is
> >> caused by inability to adjust to TSF of the IBSS leader
> >>
> >> <snipt>
> >> static int ieee80211_sta_join_ibss(struct net_device *dev,
> >> struct ieee80211_if_sta *ifsta,
> >> struct ieee80211_sta_bss *bss)
> >> .....
> >> /* Remove possible STA entries from other IBSS networks. */
> >> sta_info_flush_delayed(sdata);
> >> </snip>
> >
> > I fail to see how the TSF could be related to an ever reconnecting
> > station. Can you elaborate on what happens?
> >
> > I was under the impression that the firmware would handle TSF stuff.
> > Also the "IBSS leader" is a new thing to me. I remember from the specs
> > that the device should accept the TSF from _any_ beacon. Not just a
> > "leader". Am I mislead? :)
>
> What is happening that IBSS station should adopt TSF of the oldest
> station i.e. with highest
> TSF. This is also leader of the IBSS (this is not spec definition just
> local jargon) Adaptation
> mean we adjust to the same clock
>
> if (beacon_timestamp > rx_timestamp)
> merge

Well, I was pretty sure the firmware did this for us.
I can recheck that later.

In any case, I will accept patches that are _well_ _tested_ to workaround this
issue. I don't have the time work work on IBSS by myself. My pile already is
big enough. :)

2008-08-28 10:42:36

by Bruno Randolf

[permalink] [raw]

Subject: Re: pull request: wireless-2.6 2008-08-26

On Wed, Aug 27, 2008 at 04:11:15PM -0700, Tomas Winkler wrote:
> Last solution would be to remove this merging all together but then
> I'm not sure if Bruno added this code just implement the spec or
> really tested it with any hardware.

of course i tested IBSS merge with atheros hardware when i sent in the patch
and it worked. this is about 6 month ago. recently i briefly tested IBSS,
with the same HW but it did not work any more - i didn't have the time to
investigate this issue...

please remember that there was a lengthy discussion about the standard
compliance and the reasons behind the need for IBSS merge before my patch was
accepted. i believe i had convincing arguments why IBSS mode does not
reliably work at all in practice if we don't support IBSS merge.

you might want to re-read this thread:
http://thread.gmane.org/gmane.linux.kernel.wireless.general/10278

greetings,
bruno

2008-08-28 10:52:53