dynamic ps + offchannel mgmt_tx + HW RoC

dynamic ps + offchannel mgmt_tx + HW RoC

[Krishna Chaitanya]

Hi,

When the HW is in PS and Dynamic PS is enabled we are dropping
all the offchannel frames. Sequence is as below

if hw is in PS: stop_queues reason=PS, queue dynamic_ps_disable_work
send Offchannel mgmt_tx
drop the packet because offchanel + stop_reasons[q] !=0

How should we handle this? SW RoC is handling this
by checking for offchannel and bringing the HW out of PS before
handing the packet to TX path, so it works fine.

-- 
Thanks,
Regards,
Chaitanya T K.

Re: dynamic ps + offchannel mgmt_tx + HW RoC

[Johannes Berg]

On Tue, 2016-05-03 at 18:23 +0530, Krishna Chaitanya wrote:
> Hi,
> 
> When the HW is in PS and Dynamic PS is enabled we are dropping
> all the offchannel frames. Sequence is as below
> 
> if hw is in PS: stop_queues reason=PS, queue dynamic_ps_disable_work
> send Offchannel mgmt_tx
> drop the packet because offchanel + stop_reasons[q] !=0
> 
> How should we handle this? SW RoC is handling this
> by checking for offchannel and bringing the HW out of PS before
> handing the packet to TX path, so it works fine.

You should handle this by not doing such a non-sensical thing and
finally implementing powersave properly in the driver :)

johannes

Re: dynamic ps + offchannel mgmt_tx + HW RoC

[Krishna Chaitanya]

On Tue, May 3, 2016 at 6:44 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2016-05-03 at 18:23 +0530, Krishna Chaitanya wrote:
>> Hi,
>>
>> When the HW is in PS and Dynamic PS is enabled we are dropping
>> all the offchannel frames. Sequence is as below
>>
>> if hw is in PS: stop_queues reason=PS, queue dynamic_ps_disable_work
>> send Offchannel mgmt_tx
>> drop the packet because offchanel + stop_reasons[q] !=0
>>
>> How should we handle this? SW RoC is handling this
>> by checking for offchannel and bringing the HW out of PS before
>> handing the packet to TX path, so it works fine.
>
> You should handle this by not doing such a non-sensical thing and
> finally implementing powersave properly in the driver :)
i don't see any issues in the powersave w.r.t driver. Isn't it a
valid case? you meant implementing dynamic_ps?

Re: dynamic ps + offchannel mgmt_tx + HW RoC

[Krishna Chaitanya]

On Tue, May 3, 2016 at 7:46 PM, Krishna Chaitanya
<chaitanya.mgit@gmail.com> wrote:
> On Tue, May 3, 2016 at 6:44 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
>> On Tue, 2016-05-03 at 18:23 +0530, Krishna Chaitanya wrote:
>>> Hi,
>>>
>>> When the HW is in PS and Dynamic PS is enabled we are dropping
>>> all the offchannel frames. Sequence is as below
>>>
>>> if hw is in PS: stop_queues reason=PS, queue dynamic_ps_disable_work
>>> send Offchannel mgmt_tx
>>> drop the packet because offchanel + stop_reasons[q] !=0
>>>
>>> How should we handle this? SW RoC is handling this
>>> by checking for offchannel and bringing the HW out of PS before
>>> handing the packet to TX path, so it works fine.
>>
>> You should handle this by not doing such a non-sensical thing and
>> finally implementing powersave properly in the driver :)
> i don't see any issues in the powersave w.r.t driver. Isn't it a
> valid case? you meant implementing dynamic_ps?
If it advertise HW_SUPPORTS_DYNAMIC_PS it works fine.
so may be we should queue those frames and send it to
HW once it is out of powersave?

Re: dynamic ps + offchannel mgmt_tx + HW RoC

[Johannes Berg]

On Tue, 2016-05-03 at 20:02 +0530, Krishna Chaitanya wrote:

> > i don't see any issues in the powersave w.r.t driver. Isn't it a
> > valid case? you meant implementing dynamic_ps?

No, I really did mean implementing the entire PS logic in the driver,
instead of having mac80211 do it.

> If it advertise HW_SUPPORTS_DYNAMIC_PS it works fine.
> so may be we should queue those frames and send it to
> HW once it is out of powersave?

We could, but I *really* don't want to patch over the messy and broken
powersave code in mac80211 now.

I really do think that the only way out of this mess is to implement
powersave entirely outside of mac80211; perhaps mac80211 could provide
helpers for it, but tying it into the MLME implementation and having
all the PS-Poll stuff be global etc. is simply wrong today.

johannes

Re: dynamic ps + offchannel mgmt_tx + HW RoC

[Krishna Chaitanya]

On Tue, May 3, 2016 at 9:31 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2016-05-03 at 20:02 +0530, Krishna Chaitanya wrote:
>
>> > i don't see any issues in the powersave w.r.t driver. Isn't it a
>> > valid case? you meant implementing dynamic_ps?
>
> No, I really did mean implementing the entire PS logic in the driver,
> instead of having mac80211 do it.
>
>> If it advertise HW_SUPPORTS_DYNAMIC_PS it works fine.
>> so may be we should queue those frames and send it to
>> HW once it is out of powersave?
>
> We could, but I *really* don't want to patch over the messy and broken
> powersave code in mac80211 now.
>
> I really do think that the only way out of this mess is to implement
> powersave entirely outside of mac80211; perhaps mac80211 could provide
> helpers for it, but tying it into the MLME implementation and having
> all the PS-Poll stuff be global etc. is simply wrong today.
Okay, i understand. This again points to our discussion about mac80211
ps a while ago. May be we should document this somewhere so that
device driver developers are aware of this?



-- 
Thanks,
Regards,
Chaitanya T K.

Re: dynamic ps + offchannel mgmt_tx + HW RoC

[Janusz Dziedzic]

On 3 May 2016 at 18:58, Krishna Chaitanya <chaitanya.mgit@gmail.com> wrote:
> On Tue, May 3, 2016 at 9:31 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
>> On Tue, 2016-05-03 at 20:02 +0530, Krishna Chaitanya wrote:
>>
>>> > i don't see any issues in the powersave w.r.t driver. Isn't it a
>>> > valid case? you meant implementing dynamic_ps?
>>
>> No, I really did mean implementing the entire PS logic in the driver,
>> instead of having mac80211 do it.
>>
>>> If it advertise HW_SUPPORTS_DYNAMIC_PS it works fine.
>>> so may be we should queue those frames and send it to
>>> HW once it is out of powersave?
>>
>> We could, but I *really* don't want to patch over the messy and broken
>> powersave code in mac80211 now.
>>
>> I really do think that the only way out of this mess is to implement
>> powersave entirely outside of mac80211; perhaps mac80211 could provide
>> helpers for it, but tying it into the MLME implementation and having
>> all the PS-Poll stuff be global etc. is simply wrong today.
> Okay, i understand. This again points to our discussion about mac80211
> ps a while ago. May be we should document this somewhere so that
> device driver developers are aware of this?
>

Why not remove PS code from the mac80211 then, while is broken and
should not be used?

BR
Janusz

>
>
> --
> Thanks,
> Regards,
> Chaitanya T K.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: dynamic ps + offchannel mgmt_tx + HW RoC

[Johannes Berg]

On Tue, 2016-05-03 at 19:21 +0200, Janusz Dziedzic wrote:
> 
> Why not remove PS code from the mac80211 then, while is broken and
> should not be used?
> 

It'd still be a regression since some drivers can kinda use it.

johannes

Re: dynamic ps + offchannel mgmt_tx + HW RoC

[Johannes Berg]

There is, btw, perhaps a different way - just fix the damn stuff.

Requires moving everything into ifmgd rather than local, and then
perhaps if only a single managed interface exists mirroring its state
into the existing driver calls etc.

It'd still be a big task, and I don't see much advantage over just
reimplementing it.

johannes

Re: dynamic ps + offchannel mgmt_tx + HW RoC

[Krishna Chaitanya]

On Wed, May 4, 2016 at 1:05 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> There is, btw, perhaps a different way - just fix the damn stuff.
>
> Requires moving everything into ifmgd rather than local, and then
> perhaps if only a single managed interface exists mirroring its state
> into the existing driver calls etc.
>
> It'd still be a big task, and I don't see much advantage over just
> reimplementing it.

Yes, requires quite an effort and also need to handle lot of cases.
Instead, why don't we make the default as complete PS offload
and force the drivers who need to use mac80211 PS negate these
in the register_hw? That way we are clear about the intentions.

-- 
Thanks,
Regards,
Chaitanya T K.

Re: dynamic ps + offchannel mgmt_tx + HW RoC

[Johannes Berg]

On Wed, 2016-05-04 at 01:20 +0530, Krishna Chaitanya wrote:
> On Wed, May 4, 2016 at 1:05 AM, Johannes Berg <johannes@sipsolutions.
> net> wrote:
> > 
> > There is, btw, perhaps a different way - just fix the damn stuff.
> > 
> > Requires moving everything into ifmgd rather than local, and then
> > perhaps if only a single managed interface exists mirroring its
> > state
> > into the existing driver calls etc.
> > 
> > It'd still be a big task, and I don't see much advantage over just
> > reimplementing it.
> Yes, requires quite an effort and also need to handle lot of cases.
> Instead, why don't we make the default as complete PS offload
> and force the drivers who need to use mac80211 PS negate these
> in the register_hw? That way we are clear about the intentions.

I don't really see a big advantage, and it'd create churn in a lot of
drivers. Maybe documenting it with the flags that we really think you
should set them or something, or making the powersave DOC section
mention this would be good, but beyond that I don't think it'd be a
good idea.

johannes

Re: dynamic ps + offchannel mgmt_tx + HW RoC

[Krishna Chaitanya]

On Wed, May 4, 2016 at 1:33 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Wed, 2016-05-04 at 01:20 +0530, Krishna Chaitanya wrote:
>> On Wed, May 4, 2016 at 1:05 AM, Johannes Berg <johannes@sipsolutions.
>> net> wrote:
>> >
>> > There is, btw, perhaps a different way - just fix the damn stuff.
>> >
>> > Requires moving everything into ifmgd rather than local, and then
>> > perhaps if only a single managed interface exists mirroring its
>> > state
>> > into the existing driver calls etc.
>> >
>> > It'd still be a big task, and I don't see much advantage over just
>> > reimplementing it.
>> Yes, requires quite an effort and also need to handle lot of cases.
>> Instead, why don't we make the default as complete PS offload
>> and force the drivers who need to use mac80211 PS negate these
>> in the register_hw? That way we are clear about the intentions.
>
> I don't really see a big advantage, and it'd create churn in a lot of
> drivers. Maybe documenting it with the flags that we really think you
> should set them or something, or making the powersave DOC section
> mention this would be good, but beyond that I don't think it'd be a
> good idea.
that's fine with me.