On Sun, Nov 15, 2015 at 08:27:28PM +0100, Martin Pieuchot wrote:
> I'm really concerned about the use of tasks in iwm_ampdu_rx_{start,stop}
>
> By looking at iwn(4) which also has a lot of bits for handling A-MPDU
> and A-MSDU frames, it seems to me that this go against damien@'s design.
> This design might have its own set of problems, but I fear that working
> around them might add another set of problems on top.
iwm(4) runs all fw commands in process context and checks for errors
after tsleep(). This means we cannot return errors to the interrupt
context which triggered the commands.
Remember this? http://marc.info/?l=openbsd-tech&m=143463744006189&w=2
iwn(4) triggers fw commands in interrupt context and ignores errors.
I'm still not sure which is better. Perhaps iwm(4) should do what iwn(4)
does. Perhaps not. But this is a general problem with iwm(4) and it's not
restricted to just this task. I'm simply following the current pattern.
> Are you sure that when these functions are called it is safe to modify
> the soft states of the Block Ack agreements before notifying the
> hardware?
The soft state is one layer removed from the real action.
Based on my understanding so far, I believe it works like this:
Once switched to block up mode, the hardware passes A-MPDUs
up to the driver, with failed A-MPDU subframes stripped, and
sends acks for non-failed frames in batches when it receives
an explicit or implicit block ack request (implicit ones are
piggy-backed onto headers in the A-MPDU, explicit ones are
the ones where the AP sends a special frame to ask for acks).
net80211 doesn't really deal with acks. It just reorders frames
passed to it from the driver based on sequence number.
These frames have already been ack'ed or will be ack'ed soon.
It only looks at block ack requests to adjust its idea of the
current sequence number window. If it is totally off it just
dials the window forward and drops buffered frames.
If the hardware fails to set up a block ack we agreed to, or if
for some other reason the hardware isn't set up for block ack yet
when we receive an A-MPDU, I think the worst case should be that
the sending AP will time out its BA agreement, and perhaps it will
see dropped frames and retry. net80211 will recognize duplicates
based on their sequence numbers and drop them. Non-aggregated frames
are always a valid fallback so the AP may choose to try negotiating
block ack again, or it might not.
> > Index: net80211/ieee80211_node.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net80211/ieee80211_node.c,v
> > retrieving revision 1.91
> > diff -u -p -r1.91 ieee80211_node.c
> > --- net80211/ieee80211_node.c 15 Nov 2015 11:14:17 -0000 1.91
> > +++ net80211/ieee80211_node.c 15 Nov 2015 13:06:28 -0000
> > @@ -1066,6 +1072,32 @@ ieee80211_find_node_for_beacon(struct ie
> > return (keep);
> > }
> >
> > +#ifndef IEEE80211_NO_HT
> > +void
> > +ieee80211_ba_del(struct ieee80211_node *ni)
> > +{
> > + int tid;
> > +
> > + for (tid = 0; tid < nitems(ni->ni_rx_ba); tid++) {
> > + struct ieee80211_rx_ba *ba = &ni->ni_rx_ba[tid];
> > + if (ba->ba_state == IEEE80211_BA_AGREED) {
> > + if (timeout_pending(&ba->ba_to))
> > + timeout_del(&ba->ba_to);
> > + ba->ba_state = IEEE80211_BA_INIT;
> > + }
>
> I wouldn't bother with timeout_pending(), well I would event reuse the
> idioms from ieee80211_recv_delba()/ieee80211_delba_request()
>
> What about the reordering buffer? Is it freed? (How) is this related to
> ieee80211_node_leave_ht()?
Ah, thanks. I'll check that out.
ieee80211_node_leave_ht() runs in 11n AP mode only.
We don't support that yet so I've left the function blank.
I'm not sure yet what it will do.
