On Mon, 21 Aug 2006 09:41:14 +0200, Johannes Berg wrote:
> I think this is not correct if a STA is removed for which packets
> are buffered, but if it is still wrong then that case was never
> correct to start with if the hw has a set_tim callback.

You're right, good catch.

Just minor things:

> [...]
> -     if (num_bits) {
> +     if (have_bits) {
>               /* Find largest even number N1 so that bits numbered 1 through
>                * (N1 x 8) - 1 in the bitmap are 0 and number N2 so that bits
>                * (N2 + 1) x 8 through 2007 are 0. */
>               n1 = 0;
> -             for (i = 0; i < sizeof(bitmap); i++) {
> -                     if (bitmap[i]) {
> +             /* 251 = max size of tim bitmap in beacon */
> +             for (i = 0; i < 251; i++) {

Please, use a constant here.

> [...]
> @@ -211,13 +213,10 @@ struct ieee80211_if_ap {
>       u8 *generic_elem;
>       size_t generic_elem_len;
>  
> -     /* TODO: sta_aid could be replaced by 2008-bit large bitfield of
> -      * that could be used in TIM element generation. This would also
> -      * make TIM element generation a bit faster. */
> -     /* AID mapping to station data. NULL, if AID is free. AID is in the
> -      * range 1..2007 and sta_aid[i] corresponds to AID i+1. */
> -     struct sta_info *sta_aid[MAX_AID_TABLE_SIZE];
> -     int max_aid; /* largest aid currently in use */
> +     /* yes, this looks ugly, but guarantees that we can later use
> +      * bitmap_empty :)
> +      * NB: don't ever use set_bit, use bss_tim_set/bss_tim_clear! */
> +     u8 tim[sizeof(unsigned long)*BITS_TO_LONGS(MAX_AID_TABLE_SIZE+1)];

Hm, adding spaces here would extend the line above 80 characters... But
this way it doesn't look good. What to do here? I'd prefer leaving the
line a little over 80 chars in this case. What do you think?

> [...]
> --- wireless-dev.orig/net/d80211/sta_info.c   2006-08-20 14:56:17.418192788 
> +0200
> +++ wireless-dev/net/d80211/sta_info.c        2006-08-20 14:56:20.588192788 
> +0200
> @@ -424,13 +424,6 @@ void sta_info_remove_aid_ptr(struct sta_
>       sdata = IEEE80211_DEV_TO_SUB_IF(sta->dev);
>       if (sta->aid <= 0 || !sdata->bss)
>               return;
> -
> -     sdata->bss->sta_aid[sta->aid - 1] = NULL;
> -     if (sta->aid == sdata->bss->max_aid) {
> -             while (sdata->bss->max_aid > 0 &&
> -                    !sdata->bss->sta_aid[sdata->bss->max_aid - 1])
> -                     sdata->bss->max_aid--;
> -     }
>  }

Why are you not calling bss_tim_clear here? Am I missing something?

Also, adding hw->set_tim call here should fix the problem you described
at the beginning of the mail.

Thanks,

 Jiri

-- 
Jiri Benc
SUSE Labs
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to