[rt2x00-users] [RFC] rt2x00: implement PS broadcast/multicast frame buffering
Helmut Schaa
helmut.schaa at googlemail.com
Tue Jun 22 20:28:32 UTC 2010
Am Dienstag 22 Juni 2010 schrieb Gertjan van Wingerde:
> On 06/21/10 18:42, Helmut Schaa wrote:
> > In AP mode mac80211 will buffer multi- and broadcast frames for us. rt2x00
> > should retrieve the buffered frames via ieee80211_get_buffered_bc after a
> > beacon was sent out. Since we already have the beacondone callback that is
> > called after each beacon we can simply set another flag to trigger the
> > buffered frames to be sent out in rt2x00lib_intf_scheduled_iter.
> >
> > Signed-off-by: Helmut Schaa <helmut.schaa at googlemail.com>
> > ---
> >
> > I'm not sure if it is enough to send the buffered frames from within the
> > workqueue as 802.11 tells us:
> >
> > "After a DTIM, the AP shall send out the buffered broadcast/multicast MSDUs
> > using normal frame transmission rules, before transmitting any unicast frames."
> >
> > But even in interrupt context I don't think we can ensure that no unicast
> > frames are sent out before the buffered frames? Any ideas?
> >
> > Could we put the buffered frames in a different queue? Maybe the mgmt queue?
> >
> > However, this patch at least sends multicast and broadcast frames out after
> > a DTIM beacon. That's better then not sening them out at all, right?
> >
> > Helmut
> >
> > drivers/net/wireless/rt2x00/rt2400pci.c | 2 +-
> > drivers/net/wireless/rt2x00/rt2500pci.c | 2 +-
> > drivers/net/wireless/rt2x00/rt2800pci.c | 2 +-
> > drivers/net/wireless/rt2x00/rt2x00.h | 4 +++-
> > drivers/net/wireless/rt2x00/rt2x00dev.c | 20 ++++++++++++++++++--
> > drivers/net/wireless/rt2x00/rt2x00mac.c | 2 +-
> > drivers/net/wireless/rt2x00/rt61pci.c | 2 +-
> > 7 files changed, 26 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
> > index 1eb882e..ca80b60 100644
> > --- a/drivers/net/wireless/rt2x00/rt2400pci.c
> > +++ b/drivers/net/wireless/rt2x00/rt2400pci.c
> > @@ -1261,7 +1261,7 @@ static irqreturn_t rt2400pci_interrupt(int irq, void *dev_instance)
> > * 1 - Beacon timer expired interrupt.
> > */
> > if (rt2x00_get_field32(reg, CSR7_TBCN_EXPIRE))
> > - rt2x00lib_beacondone(rt2x00dev);
> > + rt2x00lib_beacondone(rt2x00dev, true);
> >
> > /*
> > * 2 - Rx ring done interrupt.
> > diff --git a/drivers/net/wireless/rt2x00/rt2500pci.c b/drivers/net/wireless/rt2x00/rt2500pci.c
> > index a29cb21..49ec41e 100644
> > --- a/drivers/net/wireless/rt2x00/rt2500pci.c
> > +++ b/drivers/net/wireless/rt2x00/rt2500pci.c
> > @@ -1397,7 +1397,7 @@ static irqreturn_t rt2500pci_interrupt(int irq, void *dev_instance)
> > * 1 - Beacon timer expired interrupt.
> > */
> > if (rt2x00_get_field32(reg, CSR7_TBCN_EXPIRE))
> > - rt2x00lib_beacondone(rt2x00dev);
> > + rt2x00lib_beacondone(rt2x00dev, true);
> >
> > /*
> > * 2 - Rx ring done interrupt.
> > diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
> > index d51a080..a84e80b 100644
> > --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> > +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> > @@ -945,7 +945,7 @@ static irqreturn_t rt2800pci_interrupt(int irq, void *dev_instance)
> > * Current beacon was sent out, fetch the next one
> > */
> > if (rt2x00_get_field32(reg, INT_SOURCE_CSR_TBTT))
> > - rt2x00lib_beacondone(rt2x00dev);
> > + rt2x00lib_beacondone(rt2x00dev, true);
> >
> > if (rt2x00_get_field32(reg, INT_SOURCE_CSR_AUTO_WAKEUP))
> > rt2800pci_wakeup(rt2x00dev);
> > diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
> > index e7acc6a..7523797 100644
> > --- a/drivers/net/wireless/rt2x00/rt2x00.h
> > +++ b/drivers/net/wireless/rt2x00/rt2x00.h
> > @@ -373,6 +373,7 @@ struct rt2x00_intf {
> > */
> > unsigned int delayed_flags;
> > #define DELAYED_UPDATE_BEACON 0x00000001
> > +#define DELAYED_SEND_BUFFERED_BC 0x00000002
> >
> > /*
> > * Software sequence counter, this is only required
> > @@ -1053,7 +1054,8 @@ static inline void rt2x00debug_dump_frame(struct rt2x00_dev *rt2x00dev,
> > /*
> > * Interrupt context handlers.
> > */
> > -void rt2x00lib_beacondone(struct rt2x00_dev *rt2x00dev);
> > +void rt2x00lib_beacondone(struct rt2x00_dev *rt2x00dev,
> > + bool send_buffered_bc);
> > void rt2x00lib_txdone(struct queue_entry *entry,
> > struct txdone_entry_desc *txdesc);
> > void rt2x00lib_rxdone(struct rt2x00_dev *rt2x00dev,
> > diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
> > index e684698..0456f8d 100644
> > --- a/drivers/net/wireless/rt2x00/rt2x00dev.c
> > +++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
> > @@ -124,6 +124,7 @@ static void rt2x00lib_intf_scheduled_iter(void *data, u8 *mac,
> > {
> > struct rt2x00_dev *rt2x00dev = data;
> > struct rt2x00_intf *intf = vif_to_intf(vif);
> > + struct sk_buff *skb;
> > int delayed_flags;
> >
> > /*
> > @@ -147,6 +148,18 @@ static void rt2x00lib_intf_scheduled_iter(void *data, u8 *mac,
> > if (!test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
> > return;
> >
> > + /*
> > + * Send out buffered broad- and multicast frames when
> > + * DELAYED_SEND_BUFFERED_BC is set.
> > + */
> > + if (delayed_flags & DELAYED_SEND_BUFFERED_BC) {
> > + skb = ieee80211_get_buffered_bc(rt2x00dev->hw, vif);
> > + while (skb) {
> > + rt2x00mac_tx(rt2x00dev->hw, skb);
> > + skb = ieee80211_get_buffered_bc(rt2x00dev->hw, vif);
> > + }
> > + }
> > +
> > if (delayed_flags & DELAYED_UPDATE_BEACON)
> > rt2x00queue_update_beacon(rt2x00dev, vif, true);
> > }
>
> Do we really need this additional flag?
> I guess the flag is only disabled when beaconing is enabled. What are the chances
> that then there are buffered bc/mc frames already. Wouldn't the while-loop be a
> nop in that case?
>
> Adding the boolean to the beacondone function and this flag seems like a lot of
> overkill for a situation that might not happen in the first place.
The only reason why I've added the boolean and the flag is because beacon_done
is also called from rt2x00mac_set_tim and we don't want to send buffered bc/mc
frames out just because the TIM for the next beacon was updated without having
sent a beacon.
I also thought about scheduling a tasklet for sending out the buffered frames,
and since that would have a dedicated function we could live without the flag.
> > @@ -172,6 +185,7 @@ static void rt2x00lib_beacondone_iter(void *data, u8 *mac,
> > struct ieee80211_vif *vif)
> > {
> > struct rt2x00_intf *intf = vif_to_intf(vif);
> > + bool send_buffered_bc = *(bool *)data;
> >
> > if (vif->type != NL80211_IFTYPE_AP &&
> > vif->type != NL80211_IFTYPE_ADHOC &&
> > @@ -181,17 +195,19 @@ static void rt2x00lib_beacondone_iter(void *data, u8 *mac,
> >
> > spin_lock(&intf->lock);
> > intf->delayed_flags |= DELAYED_UPDATE_BEACON;
> > + if (send_buffered_bc)
> > + intf->delayed_flags |= DELAYED_SEND_BUFFERED_BC;
> > spin_unlock(&intf->lock);
> > }
> >
> > -void rt2x00lib_beacondone(struct rt2x00_dev *rt2x00dev)
> > +void rt2x00lib_beacondone(struct rt2x00_dev *rt2x00dev, bool send_buffered_bc)
> > {
> > if (!test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
> > return;
> >
> > ieee80211_iterate_active_interfaces_atomic(rt2x00dev->hw,
> > rt2x00lib_beacondone_iter,
> > - rt2x00dev);
> > + &send_buffered_bc);
> >
> > ieee80211_queue_work(rt2x00dev->hw, &rt2x00dev->intf_work);
> > }
> > diff --git a/drivers/net/wireless/rt2x00/rt2x00mac.c b/drivers/net/wireless/rt2x00/rt2x00mac.c
> > index abbd857..7813f39 100644
> > --- a/drivers/net/wireless/rt2x00/rt2x00mac.c
> > +++ b/drivers/net/wireless/rt2x00/rt2x00mac.c
> > @@ -435,7 +435,7 @@ int rt2x00mac_set_tim(struct ieee80211_hw *hw, struct ieee80211_sta *sta,
> > {
> > struct rt2x00_dev *rt2x00dev = hw->priv;
> >
> > - rt2x00lib_beacondone(rt2x00dev);
> > + rt2x00lib_beacondone(rt2x00dev, false);
> > return 0;
> > }
> > EXPORT_SYMBOL_GPL(rt2x00mac_set_tim);
> > diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
> > index ee7d434..25239ce 100644
> > --- a/drivers/net/wireless/rt2x00/rt61pci.c
> > +++ b/drivers/net/wireless/rt2x00/rt61pci.c
> > @@ -2204,7 +2204,7 @@ static irqreturn_t rt61pci_interrupt(int irq, void *dev_instance)
> > * 5 - Beacon done interrupt.
> > */
> > if (rt2x00_get_field32(reg, INT_SOURCE_CSR_BEACON_DONE))
> > - rt2x00lib_beacondone(rt2x00dev);
> > + rt2x00lib_beacondone(rt2x00dev, true);
> >
> > return IRQ_HANDLED;
> > }
>
>
More information about the users
mailing list