On Tue, 2014-06-03 at 19:05 -0700, Luis R. Rodriguez wrote:
> > +++ b/net/mac80211/tx.c
> > @@ -478,6 +478,19 @@ ieee80211_tx_h_unicast_ps_buf(struct ieee80211_tx_data *tx)
> > sta->sta.addr, sta->sta.aid, ac);
> > if (tx->local->total_ps_buffered >= TOTAL_MAX_TX_BUFFER)
> > purge_old_ps_buffers(tx->local);
> > +
> > + /* sync with ieee80211_sta_ps_deliver_wakeup */
> > + spin_lock(&sta->ps_lock);
> > + /*
> > + * STA woke up the meantime and all the frames on ps_tx_buf have
> > + * been queued to pending queue. No reordering can happen, go
> > + * ahead and Tx the packet.
> > + */
> > + if (!test_sta_flag(sta, WLAN_STA_PS_STA)) {
> > + spin_unlock(&sta->ps_lock);
> > + return TX_CONTINUE;
> > + }
> > +
> > if (skb_queue_len(&sta->ps_tx_buf[ac]) >= STA_MAX_TX_BUFFER) {
> > struct sk_buff *old = skb_dequeue(&sta->ps_tx_buf[ac]);
> > ps_dbg(tx->sdata,
>
> This fixes the race for the case where the API introduced via commit
> af81858172cc but that code also *moved* code, the API added simply
> added a new way to let driver to hit a trigger to send buffered
> frames, are we sure that through mechanisms that don't use this API
> this race doesn't exist? That is can we not race between
> ieee80211_rx_h_sta_process() and ieee80211_tx_h_unicast_ps_buf()?
Err, I have no idea what you're trying to say.
Did you mean sta_ps_end() vs. ieee80211_tx_h_unicast_ps_buf()? Those can
*race*, but any outcome will be valid, afaict, given the station flag
tricks. It all goes back to the same (locked) code for manipulating the
queues.
I recently fixed a bug where mac80211 would get into a confused state
and not buffer packets when it would really be required, but that's just
a bug that can be observed over the air, it has no bad consequences
except for that station losing traffic.
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
