| Message ID | 1548678108-9526-1-git-send-email-sgruszka@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mt76x02: Beacon support for USB | expand |
Stanislaw Gruszka <sgruszka@redhat.com> writes: > We can configure beaconing, but without TBTT interrupt we > can not support PS buffering. This can be added later using > kernel hrtimer, if we can keep it in sycn with device timer. > > I tested AP and IBSS modes. So how does this work reliably so that there's no packet loss with clients using power save?
On 2019-01-29 12:47, Kalle Valo wrote: > Stanislaw Gruszka <sgruszka@redhat.com> writes: > >> We can configure beaconing, but without TBTT interrupt we >> can not support PS buffering. This can be added later using >> kernel hrtimer, if we can keep it in sycn with device timer. >> >> I tested AP and IBSS modes. > > So how does this work reliably so that there's no packet loss with > clients using power save? There will be multicast packet loss for clients using power save. - Felix
Felix Fietkau <nbd@nbd.name> writes: > On 2019-01-29 12:47, Kalle Valo wrote: >> Stanislaw Gruszka <sgruszka@redhat.com> writes: >> >>> We can configure beaconing, but without TBTT interrupt we >>> can not support PS buffering. This can be added later using >>> kernel hrtimer, if we can keep it in sycn with device timer. >>> >>> I tested AP and IBSS modes. >> >> So how does this work reliably so that there's no packet loss with >> clients using power save? > > There will be multicast packet loss for clients using power save. Isn't that a problem? At least as a normal user I would very frustrated if sometimes my connection work and sometimes not, for example if I'm trying discover devices from my network. Hopefully nobody won't use USB devices for any real AP stuff, but still enabling something which we know doesn't work realiably is concerning.
On 2019-01-29 13:07, Kalle Valo wrote: > Felix Fietkau <nbd@nbd.name> writes: > >> On 2019-01-29 12:47, Kalle Valo wrote: >>> Stanislaw Gruszka <sgruszka@redhat.com> writes: >>> >>>> We can configure beaconing, but without TBTT interrupt we >>>> can not support PS buffering. This can be added later using >>>> kernel hrtimer, if we can keep it in sycn with device timer. >>>> >>>> I tested AP and IBSS modes. >>> >>> So how does this work reliably so that there's no packet loss with >>> clients using power save? >> >> There will be multicast packet loss for clients using power save. > > Isn't that a problem? At least as a normal user I would very frustrated > if sometimes my connection work and sometimes not, for example if I'm > trying discover devices from my network. Hopefully nobody won't use USB > devices for any real AP stuff, but still enabling something which we > know doesn't work realiably is concerning. I agree. Maybe we should leave out the flag for AP mode in this patch until we have PS buffering and leave the rest of the code intact. - Felix
Felix Fietkau <nbd@nbd.name> writes: > On 2019-01-29 13:07, Kalle Valo wrote: >> Felix Fietkau <nbd@nbd.name> writes: >> >>> On 2019-01-29 12:47, Kalle Valo wrote: >>>> Stanislaw Gruszka <sgruszka@redhat.com> writes: >>>> >>>>> We can configure beaconing, but without TBTT interrupt we >>>>> can not support PS buffering. This can be added later using >>>>> kernel hrtimer, if we can keep it in sycn with device timer. >>>>> >>>>> I tested AP and IBSS modes. >>>> >>>> So how does this work reliably so that there's no packet loss with >>>> clients using power save? >>> >>> There will be multicast packet loss for clients using power save. >> >> Isn't that a problem? At least as a normal user I would very frustrated >> if sometimes my connection work and sometimes not, for example if I'm >> trying discover devices from my network. Hopefully nobody won't use USB >> devices for any real AP stuff, but still enabling something which we >> know doesn't work realiably is concerning. > > I agree. Maybe we should leave out the flag for AP mode in this patch > until we have PS buffering and leave the rest of the code intact. At least for me that sounds good.
> Felix Fietkau <nbd@nbd.name> writes: > > > On 2019-01-29 13:07, Kalle Valo wrote: > >> Felix Fietkau <nbd@nbd.name> writes: > >> > >>> On 2019-01-29 12:47, Kalle Valo wrote: > >>>> Stanislaw Gruszka <sgruszka@redhat.com> writes: > >>>> > >>>>> We can configure beaconing, but without TBTT interrupt we > >>>>> can not support PS buffering. This can be added later using > >>>>> kernel hrtimer, if we can keep it in sycn with device timer. > >>>>> > >>>>> I tested AP and IBSS modes. > >>>> > >>>> So how does this work reliably so that there's no packet loss with > >>>> clients using power save? > >>> > >>> There will be multicast packet loss for clients using power save. > >> > >> Isn't that a problem? At least as a normal user I would very frustrated > >> if sometimes my connection work and sometimes not, for example if I'm > >> trying discover devices from my network. Hopefully nobody won't use USB > >> devices for any real AP stuff, but still enabling something which we > >> know doesn't work realiably is concerning. > > > > I agree. Maybe we should leave out the flag for AP mode in this patch > > until we have PS buffering and leave the rest of the code intact. > > At least for me that sounds good. We can support ps buffering in AP as well using a hrtimer. In this way we can reuse most of the existing code Regards, Lorenzo > > -- > Kalle Valo
On Tue, Jan 29, 2019 at 01:10:08PM +0100, Felix Fietkau wrote: > On 2019-01-29 13:07, Kalle Valo wrote: > > Felix Fietkau <nbd@nbd.name> writes: > > > >> On 2019-01-29 12:47, Kalle Valo wrote: > >>> Stanislaw Gruszka <sgruszka@redhat.com> writes: > >>> > >>>> We can configure beaconing, but without TBTT interrupt we > >>>> can not support PS buffering. This can be added later using > >>>> kernel hrtimer, if we can keep it in sycn with device timer. > >>>> > >>>> I tested AP and IBSS modes. > >>> > >>> So how does this work reliably so that there's no packet loss with > >>> clients using power save? > >> > >> There will be multicast packet loss for clients using power save. > > > > Isn't that a problem? At least as a normal user I would very frustrated > > if sometimes my connection work and sometimes not, for example if I'm > > trying discover devices from my network. Hopefully nobody won't use USB > > devices for any real AP stuff, but still enabling something which we > > know doesn't work realiably is concerning. > I agree. Maybe we should leave out the flag for AP mode in this patch > until we have PS buffering and leave the rest of the code intact. But how serious problem of dropping multicast frames for PS stations is? I don't think from user perspective this is "sometimes my connection work and sometimes not", but something much less annoying. Another thing is that this (D)TIM PS is not reliable by design, that why UAPSD was introduced. Moreover in the tree we have already bunch of drivers that do advertise AP mode support without HOST_BROADCAST_PS_BUFFERING like iwlwifi or brcm80211 . So I don't think we should drop AP flag, AP mode works with this patch set quite well. Thanks Stanislaw
On Tue, Jan 29, 2019 at 01:40:57PM +0100, Lorenzo Bianconi wrote: > > Felix Fietkau <nbd@nbd.name> writes: > > > > > On 2019-01-29 13:07, Kalle Valo wrote: > > >> Felix Fietkau <nbd@nbd.name> writes: > > >> > > >>> On 2019-01-29 12:47, Kalle Valo wrote: > > >>>> Stanislaw Gruszka <sgruszka@redhat.com> writes: > > >>>> > > >>>>> We can configure beaconing, but without TBTT interrupt we > > >>>>> can not support PS buffering. This can be added later using > > >>>>> kernel hrtimer, if we can keep it in sycn with device timer. > > >>>>> > > >>>>> I tested AP and IBSS modes. > > >>>> > > >>>> So how does this work reliably so that there's no packet loss with > > >>>> clients using power save? > > >>> > > >>> There will be multicast packet loss for clients using power save. > > >> > > >> Isn't that a problem? At least as a normal user I would very frustrated > > >> if sometimes my connection work and sometimes not, for example if I'm > > >> trying discover devices from my network. Hopefully nobody won't use USB > > >> devices for any real AP stuff, but still enabling something which we > > >> know doesn't work realiably is concerning. > > > > > > I agree. Maybe we should leave out the flag for AP mode in this patch > > > until we have PS buffering and leave the rest of the code intact. > > > > At least for me that sounds good. > > We can support ps buffering in AP as well using a hrtimer. In this way we > can reuse most of the existing code Yes, but there is issue to address, since kernel timer and device TBT timer are independed, they possibly can get out of sync after some time, for example few hours or days. So there is need to prevent/fix this somehow. Thanks Stanislaw
On 2019-01-30 09:37, Stanislaw Gruszka wrote: > On Tue, Jan 29, 2019 at 01:40:57PM +0100, Lorenzo Bianconi wrote: >> > Felix Fietkau <nbd@nbd.name> writes: >> > >> > > On 2019-01-29 13:07, Kalle Valo wrote: >> > >> Felix Fietkau <nbd@nbd.name> writes: >> > >> >> > >>> On 2019-01-29 12:47, Kalle Valo wrote: >> > >>>> Stanislaw Gruszka <sgruszka@redhat.com> writes: >> > >>>> >> > >>>>> We can configure beaconing, but without TBTT interrupt we >> > >>>>> can not support PS buffering. This can be added later using >> > >>>>> kernel hrtimer, if we can keep it in sycn with device timer. >> > >>>>> >> > >>>>> I tested AP and IBSS modes. >> > >>>> >> > >>>> So how does this work reliably so that there's no packet loss with >> > >>>> clients using power save? >> > >>> >> > >>> There will be multicast packet loss for clients using power save. >> > >> >> > >> Isn't that a problem? At least as a normal user I would very frustrated >> > >> if sometimes my connection work and sometimes not, for example if I'm >> > >> trying discover devices from my network. Hopefully nobody won't use USB >> > >> devices for any real AP stuff, but still enabling something which we >> > >> know doesn't work realiably is concerning. >> > > >> > > I agree. Maybe we should leave out the flag for AP mode in this patch >> > > until we have PS buffering and leave the rest of the code intact. >> > >> > At least for me that sounds good. >> >> We can support ps buffering in AP as well using a hrtimer. In this way we >> can reuse most of the existing code > > Yes, but there is issue to address, since kernel timer and device TBT > timer are independed, they possibly can get out of sync after some time, > for example few hours or days. So there is need to prevent/fix this > somehow. We could read the TSF timer value from the hardware and sync the hrtimer against that. - Felix
On 2019-01-30 09:29, Stanislaw Gruszka wrote: > On Tue, Jan 29, 2019 at 01:10:08PM +0100, Felix Fietkau wrote: >> On 2019-01-29 13:07, Kalle Valo wrote: >> > Felix Fietkau <nbd@nbd.name> writes: >> > >> >> On 2019-01-29 12:47, Kalle Valo wrote: >> >>> Stanislaw Gruszka <sgruszka@redhat.com> writes: >> >>> >> >>>> We can configure beaconing, but without TBTT interrupt we >> >>>> can not support PS buffering. This can be added later using >> >>>> kernel hrtimer, if we can keep it in sycn with device timer. >> >>>> >> >>>> I tested AP and IBSS modes. >> >>> >> >>> So how does this work reliably so that there's no packet loss with >> >>> clients using power save? >> >> >> >> There will be multicast packet loss for clients using power save. >> > >> > Isn't that a problem? At least as a normal user I would very frustrated >> > if sometimes my connection work and sometimes not, for example if I'm >> > trying discover devices from my network. Hopefully nobody won't use USB >> > devices for any real AP stuff, but still enabling something which we >> > know doesn't work realiably is concerning. >> I agree. Maybe we should leave out the flag for AP mode in this patch >> until we have PS buffering and leave the rest of the code intact. > > But how serious problem of dropping multicast frames for PS stations is? > I don't think from user perspective this is "sometimes my connection > work and sometimes not", but something much less annoying. Actually, if you're considering two stations on an AP trying to connect to each other, it is exactly "sometimes my connection work and sometimes not", because of ARP. Clients won't see the ARP requests sent to each other, if the client being asked is in powersave mode. > Another thing is that this (D)TIM PS is not reliable by design, > that why UAPSD was introduced. UAPSD doesn't replace TIM based powersave indication, and I'm pretty sure it doesn't handle multicast either. > Moreover in the tree we have already bunch of drivers that do > advertise AP mode support without HOST_BROADCAST_PS_BUFFERING > like iwlwifi or brcm80211 . I haven't looked at the code, but they may be handling buffered multicast in a different way, possibly with firmware involvement. - Felix
On Wed, Jan 30, 2019 at 10:16:34AM +0100, Felix Fietkau wrote: > On 2019-01-30 09:37, Stanislaw Gruszka wrote: > > On Tue, Jan 29, 2019 at 01:40:57PM +0100, Lorenzo Bianconi wrote: > >> > Felix Fietkau <nbd@nbd.name> writes: > >> > > >> > > On 2019-01-29 13:07, Kalle Valo wrote: > >> > >> Felix Fietkau <nbd@nbd.name> writes: > >> > >> > >> > >>> On 2019-01-29 12:47, Kalle Valo wrote: > >> > >>>> Stanislaw Gruszka <sgruszka@redhat.com> writes: > >> > >>>> > >> > >>>>> We can configure beaconing, but without TBTT interrupt we > >> > >>>>> can not support PS buffering. This can be added later using > >> > >>>>> kernel hrtimer, if we can keep it in sycn with device timer. > >> > >>>>> > >> > >>>>> I tested AP and IBSS modes. > >> > >>>> > >> > >>>> So how does this work reliably so that there's no packet loss with > >> > >>>> clients using power save? > >> > >>> > >> > >>> There will be multicast packet loss for clients using power save. > >> > >> > >> > >> Isn't that a problem? At least as a normal user I would very frustrated > >> > >> if sometimes my connection work and sometimes not, for example if I'm > >> > >> trying discover devices from my network. Hopefully nobody won't use USB > >> > >> devices for any real AP stuff, but still enabling something which we > >> > >> know doesn't work realiably is concerning. > >> > > > >> > > I agree. Maybe we should leave out the flag for AP mode in this patch > >> > > until we have PS buffering and leave the rest of the code intact. > >> > > >> > At least for me that sounds good. > >> > >> We can support ps buffering in AP as well using a hrtimer. In this way we > >> can reuse most of the existing code > > > > Yes, but there is issue to address, since kernel timer and device TBT > > timer are independed, they possibly can get out of sync after some time, > > for example few hours or days. So there is need to prevent/fix this > > somehow. > We could read the TSF timer value from the hardware and sync the hrtimer > against that. Ok then. I'll implement hrtimer solution and repost this set. Patch 6 will no longer be necessery. Regards Stanislaw
Stanislaw Gruszka <sgruszka@redhat.com> writes: > On Tue, Jan 29, 2019 at 01:10:08PM +0100, Felix Fietkau wrote: >> On 2019-01-29 13:07, Kalle Valo wrote: >> > Felix Fietkau <nbd@nbd.name> writes: >> > >> >> On 2019-01-29 12:47, Kalle Valo wrote: >> >>> Stanislaw Gruszka <sgruszka@redhat.com> writes: >> >>> >> >>>> We can configure beaconing, but without TBTT interrupt we >> >>>> can not support PS buffering. This can be added later using >> >>>> kernel hrtimer, if we can keep it in sycn with device timer. >> >>>> >> >>>> I tested AP and IBSS modes. >> >>> >> >>> So how does this work reliably so that there's no packet loss with >> >>> clients using power save? >> >> >> >> There will be multicast packet loss for clients using power save. >> > >> > Isn't that a problem? At least as a normal user I would very frustrated >> > if sometimes my connection work and sometimes not, for example if I'm >> > trying discover devices from my network. Hopefully nobody won't use USB >> > devices for any real AP stuff, but still enabling something which we >> > know doesn't work realiably is concerning. >> I agree. Maybe we should leave out the flag for AP mode in this patch >> until we have PS buffering and leave the rest of the code intact. > > But how serious problem of dropping multicast frames for PS stations > is? It's _both_ broadcast and multicast frames. Felix already mentioned ARP but there are also other protocols which rely on broadcast/multicast frames. > I don't think from user perspective this is "sometimes my connection > work and sometimes not", but something much less annoying. So basically you are saying that we should just depend on lock and hope that users don't notice the packet loss. I don't think that's really good design philosophy. > Moreover in the tree we have already bunch of drivers that do > advertise AP mode support without HOST_BROADCAST_PS_BUFFERING > like iwlwifi or brcm80211 . After seeing how much those drivers are tested I would be very surprised to see that their broadcast and multicast packet handling is broken. > So I don't think we should drop AP flag, AP mode works with > this patch set quite well. "Works" is a very broad statement and depends on what you have tested. Sure, if you tested tested with ping and iperf from a client to AP without using power save mode that will work. But if you try to connect from another device on the network to another client using aggressive power saving it's a totally different situation. Back in the Nokia N800 days I wasted a lot of time debugging all sort of buggy APs which didn't work correctly with power save, it was really annoying and very frustrating for the users. Let's not do the same mistakes, we are better than that.
On Wed, Jan 30, 2019 at 11:07:48AM +0100, Stanislaw Gruszka wrote: > > >> We can support ps buffering in AP as well using a hrtimer. In this way we > > >> can reuse most of the existing code > > > > > > Yes, but there is issue to address, since kernel timer and device TBT > > > timer are independed, they possibly can get out of sync after some time, > > > for example few hours or days. So there is need to prevent/fix this > > > somehow. > > We could read the TSF timer value from the hardware and sync the hrtimer > > against that. > > Ok then. I'll implement hrtimer solution and repost this set. Patch > 6 will no longer be necessery. This require more changes than I expected to allow programming registers from atomic context for mt76-usb. So I'll repost without AP support for now. Stanislaw
On Wed, Jan 30, 2019 at 04:22:55PM +0100, Stanislaw Gruszka wrote: > On Wed, Jan 30, 2019 at 11:07:48AM +0100, Stanislaw Gruszka wrote: > > > >> We can support ps buffering in AP as well using a hrtimer. In this way we > > > >> can reuse most of the existing code > > > > > > > > Yes, but there is issue to address, since kernel timer and device TBT > > > > timer are independed, they possibly can get out of sync after some time, > > > > for example few hours or days. So there is need to prevent/fix this > > > > somehow. > > > We could read the TSF timer value from the hardware and sync the hrtimer > > > against that. > > > > Ok then. I'll implement hrtimer solution and repost this set. Patch > > 6 will no longer be necessery. > > This require more changes than I expected to allow programming registers > from atomic context for mt76-usb. So I'll repost without AP support for > now. There are more obstacles here. On USB we do not have PSD queue available via endpoinds. I have some solutions to this problem, but non of them work satisfactory so far. Still experimenting ... Stanislaw