d80211-drivers pull request (week-48)

d80211-drivers pull request (week-48)

[Michael Wu]

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

Hi John,

Please pull
git://git.kernel.org/pub/scm/linux/kernel/git/mwu/d80211-drivers.git week-48
for these patches:

Michael Wu (3):
      zd1211rw-d80211: check IEEE80211_TXCTL_USE_CTS_PROTECT
      zd1211rw-d80211: Use ieee80211_tx_status
      zd1211rw-d80211: Use LED class

 zd_chip.c |    2 
 zd_chip.h |    5 -
 zd_mac.c  |  200 ++++++++++++++++++++++++++++++++++++++++++--------------------
 zd_mac.h  |   25 +++++--
 zd_usb.c  |   27 +++-----
 zd_usb.h  |    2 
 6 files changed, 169 insertions(+), 92 deletions(-)

This list excludes 8 zd1211rw patch "ports" and 4 patches from the previous pull
request. Note that the LED class patch eliminates the use of workqueues in
zd1211rw-d80211. adm8211 and p54 do not use workqueues either.

Thanks,
-Michael Wu

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: d80211-drivers pull request (week-48)

[Daniel Drake]

Michael Wu wrote:
>       zd1211rw-d80211: Use ieee80211_tx_status

I've thought some more about this and I'm not so sure that this is the 
right approach.

Can't devicescape be taught that the ZD1211 handles retries in hardware 
and the stack doesn't need to worry about it?

What does devicescape do in response to not getting an ack? Does it 
retransmit? If that is all then it doesn't need to be at stack level, 
since the hardware handles that on its own, and we can configure that to 
behave the same way as the stack would.

I think I remember reading that devicescape uses failed transmission 
rate in the rate adjustment calculations. Even without this racy ack 
system we can still achieve that - the device tells us every time it 
retries a transmit, and then it sends a special interrupt at the end 
saying that all retries failed.

Daniel

Re: d80211-drivers pull request (week-48)

[Michael Wu]

[-- Attachment #1: Type: text/plain, Size: 1623 bytes --]

On Monday 11 December 2006 20:07, Daniel Drake wrote:
> Michael Wu wrote:
> >       zd1211rw-d80211: Use ieee80211_tx_status
>
> I've thought some more about this and I'm not so sure that this is the
> right approach.
>
> Can't devicescape be taught that the ZD1211 handles retries in hardware
> and the stack doesn't need to worry about it?
>
All 802.11 devices have to be able to handle retries in hardware to do random 
backoff properly. Still, the stack wants to know what happened.

> I think I remember reading that devicescape uses failed transmission
> rate in the rate adjustment calculations. Even without this racy ack
> system we can still achieve that - the device tells us every time it
> retries a transmit, and then it sends a special interrupt at the end
> saying that all retries failed.
>
Yes, but it also uses successful transmissions in rate adjustment.

I don't think this race is such a big deal. It will only happen when someone 
is really trying to mess with the link, and would cause the rate control to 
jump to the highest speed. However, if someone is really trying to mess with 
the link this way, the stability of the link is in trouble anyways. Wait for 
stations to send frames, and send an ack for every unicast frame - everyone 
will get confused. To actually mess with this code, the attacker would have 
to transmit acks nearly continuously as it can't tell exactly when is a good 
time to screw things up, and the driver recovers as soon as the queue is 
emptied. Someone transmitting all the time is a problem for all wireless 
cards. :)

-Michael Wu

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: d80211-drivers pull request (week-48)

[Daniel Drake]

Michael Wu wrote:
> I don't think this race is such a big deal. It will only happen when someone 
> is really trying to mess with the link, and would cause the rate control to 
> jump to the highest speed. However, if someone is really trying to mess with 
> the link this way, the stability of the link is in trouble anyways. Wait for 
> stations to send frames, and send an ack for every unicast frame - everyone 
> will get confused. To actually mess with this code, the attacker would have 
> to transmit acks nearly continuously as it can't tell exactly when is a good 
> time to screw things up, and the driver recovers as soon as the queue is 
> emptied. Someone transmitting all the time is a problem for all wireless 
> cards. :)

It's ugly, in my mind not necessary, and it will kill performance. We 
haven't had to make such compromises in a long time. We got a large TX 
speed boost when the driver was modified to queue up multiple transmit 
URBs (i.e. don't wait for URB completion of the first) at the same time 
early during driver development. And even with that we're still a fair 
distance from the performance of the vendor driver.

While the stack isn't so well suited for this device I'd much prefer to 
see a more simplistic workaround. For example, assume all packets were 
successful but then report a failure when an interrupt comes in. Or, if 
the stack won't accept out-of-the-blue notifications like that, then 
maintain a counter which is incremented when a failure is reported, and 
when transmitting the next few frames report them as failed and 
decrement the counter (while counter > 0). Maybe disable rate control 
until we can come up with a nicer solution.

Daniel

Re: d80211-drivers pull request (week-48)

[Michael Wu]

[-- Attachment #1: Type: text/plain, Size: 1828 bytes --]

On Monday 11 December 2006 22:51, Daniel Drake wrote:
> It's ugly, in my mind not necessary, and it will kill performance. We
> haven't had to make such compromises in a long time. We got a large TX
> speed boost when the driver was modified to queue up multiple transmit
> URBs (i.e. don't wait for URB completion of the first) at the same time
> early during driver development. And even with that we're still a fair
> distance from the performance of the vendor driver.
>
Yes, synchronous anything tends to kill performance. I do not see what about 
this patch kills performance, however. It merely frees skbs at a later point. 
The additional code in the RX path should be negligible. Also, every other 
d80211 driver has some sort of similar mechanism, the only difference being 
that  the one for zd1211rw-d80211 isn't as reliable. I do not think it is 
that ugly.

> While the stack isn't so well suited for this device I'd much prefer to
> see a more simplistic workaround. For example, assume all packets were
> successful but then report a failure when an interrupt comes in. Or, if
> the stack won't accept out-of-the-blue notifications like that, then
> maintain a counter which is incremented when a failure is reported, and
> when transmitting the next few frames report them as failed and
> decrement the counter (while counter > 0).
I don't think these solutions are much prettier than a tx_queue.

> Maybe disable rate control 
> until we can come up with a nicer solution.
>
I don't think that's gonna happen unless changes are made in the firmware, or 
there's some bit somewhere to flip that does exactly what we need. And 
surely, having some working rate control is better than requiring manual tx 
rate tuning, right? (which isn't an option in d80211 anyway)

-Michael Wu

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: d80211-drivers pull request (week-48)

[Michael Buesch]

On Tuesday 12 December 2006 02:07, Daniel Drake wrote:
> Michael Wu wrote:
> >       zd1211rw-d80211: Use ieee80211_tx_status
> 
> I've thought some more about this and I'm not so sure that this is the 
> right approach.
> 
> Can't devicescape be taught that the ZD1211 handles retries in hardware 
> and the stack doesn't need to worry about it?
> 
> What does devicescape do in response to not getting an ack?

It does ratecontrol based on that.
Basically: No ACK == failed packet. If too many failures,
lower the rate.

-- 
Greetings Michael.

Re: d80211-drivers pull request (week-48)

[Ulrich Kunitz]

On 06-12-11 21:20 Michael Wu wrote:

> On Monday 11 December 2006 20:07, Daniel Drake wrote:
> > Michael Wu wrote:
> > >       zd1211rw-d80211: Use ieee80211_tx_status
> >
> > I've thought some more about this and I'm not so sure that this is the
> > right approach.
> >
> > Can't devicescape be taught that the ZD1211 handles retries in hardware
> > and the stack doesn't need to worry about it?
> >
> All 802.11 devices have to be able to handle retries in hardware to do random 
> backoff properly. Still, the stack wants to know what happened.
> 
> > I think I remember reading that devicescape uses failed transmission
> > rate in the rate adjustment calculations. Even without this racy ack
> > system we can still achieve that - the device tells us every time it
> > retries a transmit, and then it sends a special interrupt at the end
> > saying that all retries failed.
> >
> Yes, but it also uses successful transmissions in rate adjustment.
> 
> I don't think this race is such a big deal. It will only happen when someone 
> is really trying to mess with the link, and would cause the rate control to 
> jump to the highest speed. However, if someone is really trying to mess with 
> the link this way, the stability of the link is in trouble anyways. Wait for 
> stations to send frames, and send an ack for every unicast frame - everyone 
> will get confused. To actually mess with this code, the attacker would have 
> to transmit acks nearly continuously as it can't tell exactly when is a good 
> time to screw things up, and the driver recovers as soon as the queue is 
> emptied. Someone transmitting all the time is a problem for all wireless 
> cards. :)

Just two observations:

1) The retry-failed-interrupt message contains the destination
   MAC address of the transmitted message. 
2) We could get easily two acks for the same transmission.
3) We could get ACKs or retry-failed-interrupts for packets, for
   which no status has been requested by the upper layers.

I suggest that the skbs as buffer for the URB transmissions. Check
ACK/NAK for all packets, but still prepare the status only for
packets, which requested it. We could add a timeout value to each
packet to make sure, that we don't ACK or NAK packets, which have
been overdue.

-- 
Uli Kunitz

Re: d80211-drivers pull request (week-48)

[Michael Wu]

[-- Attachment #1: Type: text/plain, Size: 1196 bytes --]

On Tuesday 12 December 2006 18:39, Ulrich Kunitz wrote:
> Just two observations:
>
> 1) The retry-failed-interrupt message contains the destination
>    MAC address of the transmitted message.
Hm, that might be useful to check in master or adhoc mode to make sure the tx 
queue isn't messed up.

> 2) We could get easily two acks for the same transmission.
How would this occur?

> 3) We could get ACKs or retry-failed-interrupts for packets, for
>    which no status has been requested by the upper layers.
>
We report status for every frame which will get an ACK, which is every unicast 
frame. Coincidentally, d80211 uses the same logic to determine when it wants 
the status to be reported (IIRC).

> I suggest that the skbs as buffer for the URB transmissions.
Yes, copying the entire frame to another buffer isn't very performance 
friendly. This was one of the things that bothered me when I ported zd1211rw.

> We could add a timeout value to each 
> packet to make sure, that we don't ACK or NAK packets, which have
> been overdue.
Some sort of watchdog to kick things when things stall could be useful, though 
I'm not sure how to go about it.

-Michael Wu

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

RE: d80211-drivers pull request (week-48)

[Simon Barber]

Devicescape does understant that the hardware can do retries - but it
adds software retries on top. This allows higher reliability, as well as
correct handling of the powersave state machine. (PS bit from a STA is
supposed to stop APs transmission immediately).

Simon

-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
On Behalf Of Michael Buesch
Sent: Tuesday, December 12, 2006 1:35 AM
To: Daniel Drake
Cc: Michael Wu; John Linville; netdev@vger.kernel.org; Ulrich Kunitz
Subject: Re: d80211-drivers pull request (week-48)

On Tuesday 12 December 2006 02:07, Daniel Drake wrote:
> Michael Wu wrote:
> >       zd1211rw-d80211: Use ieee80211_tx_status
> 
> I've thought some more about this and I'm not so sure that this is the

> right approach.
> 
> Can't devicescape be taught that the ZD1211 handles retries in 
> hardware and the stack doesn't need to worry about it?
> 
> What does devicescape do in response to not getting an ack?

It does ratecontrol based on that.
Basically: No ACK == failed packet. If too many failures, lower the
rate.

--
Greetings Michael.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in the
body of a message to majordomo@vger.kernel.org More majordomo info at
http://vger.kernel.org/majordomo-info.html