MIME-Version: 1.0
In-Reply-To: <20161103095938.84054-1-nbd@nbd.name>
References: <20161103095938.84054-1-nbd@nbd.name>
From: Emmanuel Grumbach <egrumbach@gmail.com>
Date: Thu, 3 Nov 2016 12:51:11 +0200
Message-ID: <CANUX_P0xv4a1O7tqBYABsXbRvRQm_3Va888EopKDz433cwjXew@mail.gmail.com> (sfid-20161103_115117_294061_79C4A645)
Subject: Re: [PATCH] mac80211: fix broken AP mode handling of powersave clients
To: Felix Fietkau <nbd@nbd.name>
Cc: linux-wireless <linux-wireless@vger.kernel.org>,
        Johannes Berg <johannes@sipsolutions.net>,
        Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Content-Type: text/plain; charset=UTF-8
Sender: linux-wireless-owner@vger.kernel.org

On Thu, Nov 3, 2016 at 11:59 AM, Felix Fietkau <nbd@nbd.name> wrote:
> Commit c68df2e7be0c ("mac80211: allow using AP_LINK_PS with
> mac80211-generated TIM IE") introduced a logic error, where
> __sta_info_recalc_tim turns into a no-op if local->ops->set_tim is not
> set. This prevents the beacon TIM bit from being set for all drivers
> that do not implement this op (almost all of them), thus thoroughly
> essential AP mode powersave functionality.
>
> Cc: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> Fixes: c68df2e7be0c ("mac80211: allow using AP_LINK_PS with mac80211-generated TIM IE")
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
>  net/mac80211/sta_info.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
> index 236d47e..621734e 100644
> --- a/net/mac80211/sta_info.c
> +++ b/net/mac80211/sta_info.c
> @@ -688,7 +688,7 @@ static void __sta_info_recalc_tim(struct sta_info *sta, bool ignore_pending)
>         }
>
>         /* No need to do anything if the driver does all */
> -       if (!local->ops->set_tim)
> +       if (local->ops->set_tim)
>                 return;

but ... then, you'll need call to drv_set_tim below...
Apparently, we can't rely on the ops pointer to enter or not enter this flow.