Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gluon-core: add support for configuring the beacon interval #2028

Merged
merged 1 commit into from May 29, 2020

Conversation

blocktrron
Copy link
Member

This adds support for the beacon interval to be set on a per-band base.
This has the potential to reduce the amount of airtime used up for
sending beacon frames.

@rotanid rotanid added the 0. type: enhancement The changeset is an enhancement label May 21, 2020
@rotanid
Copy link
Member

rotanid commented May 21, 2020

thanks, are there also downsides besides the mentioned advantage/potential?

@blocktrron
Copy link
Member Author

blocktrron commented May 21, 2020

@rotanid devices at the edge of the coverage zone might not display the network immediately / at all if there are a lot of packets lost.

Whether or not it produces issues in close proximity depends on the onfigured value. I was testing it with ~ 1 beacon per second and did not experience issues at all.

It also decreases the interval in which other mesh nodes see the beacon / are able to establish first contact.

@blocktrron blocktrron requested a review from mweinelt May 22, 2020 09:29
@neocturne
Copy link
Member

Is this something a community can reasonable decide, or should we just hardcode values in Gluon?

@mweinelt
Copy link
Contributor

Pretty sure this should default to a well-tested and understood value. No point in trying to save airtime if it results in clients having a hard time seeing you.

@blocktrron
Copy link
Member Author

blocktrron commented May 22, 2020

@NeoRaider From my POV, this is a setting we can expose without issues. Values up to 500 ms should be fine for most use cases.

@mweinelt We already use the preset from OpenWrt (which is 100ms). I think exposing this setting brings a huge benefit for "Event" domains (as some communities have). When there are many devices broadcasting beacons, the difference becomes less noticeable even at larger intervals.

Remember, it's only big of a deal if there's already a large amount loss between your end-device and the access point. The access point will still send a beacon frame in case your device sends a probe request.

@mweinelt
Copy link
Contributor

mweinelt commented May 22, 2020

I don't seen an issue with exposing this value, I'm just not sure I have an immediate use case while lacking experience with regard to this setting.

@rotanid rotanid mentioned this pull request May 25, 2020
2 tasks
package/gluon-core/luasrc/lib/gluon/upgrade/200-wireless Outdated Show resolved Hide resolved
docs/multidomain-site-example/site.conf Outdated Show resolved Hide resolved
docs/user/site.rst Outdated Show resolved Hide resolved
docs/user/site.rst Outdated Show resolved Hide resolved
docs/user/site.rst Outdated Show resolved Hide resolved
This adds support for the beacon interval to be set on a per-band base.
This has the potential to reduce the amount of airtime used up for
sending beacon frames.
Copy link
Contributor

@mweinelt mweinelt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, lgtm.

@mweinelt mweinelt merged commit e951ff6 into freifunk-gluon:master May 29, 2020
@blocktrron blocktrron deleted the beacon-int branch May 30, 2020 07:55
@rotanid rotanid added this to the 2020.2 milestone Jun 1, 2020
@CodeFetch
Copy link
Contributor

@blocktrron I don't have a problem with the "event" use-case itself, but I think that it does not work this way and your statement is misleading. While there is a foreign transmission the AP won't send beacons either way because of CSMA/CA if it senses this transmission. Don't suggest to try to fix a bad network setup with that. It won't reduce the majority of collisions in the hidden node case and if there is no hidden node, the mentioned problem does not exists except the network setup was planned very very badly (e.g. using too many APs at one place).

@blocktrron
Copy link
Member Author

Reducing the beacon rate allows more time to to be used for client traffic. I did not claim it reduces the collision probability on the shared medium.

The point with the event domain was about reducing the regular beacon interval as clients will perform periodic probe requests nevertheless.

@CodeFetch
Copy link
Contributor

@blocktrron I understood your intention, but as far as I know it's a myth. A beacon ordinarily fits into less than 4 TUs (which really is the worst case). As we have 2 SSIDs I assumed a maximum of 10% bandwidth consumption. As we have set 54 Mbps as a minimum rate now, the time decreases drastically. Here's someone who actually measured it: http://wifinigel.blogspot.com/2013/08/its-well-known-rule-of-thumb-when.html

For getting the air clear buffered packets should be delivered as fast as possible and power-saving stations will have a higher probability to request them timely when you have a standard beacon interval. Otherwise you'll have all power-saving stations contend for PS-polling in the worst case causing a higher collision probability.

@blocktrron
Copy link
Member Author

I feel this is becoming arguing about a solution which clearly does not fit all (hence it's optional).

The traffic indication map is often sent with beacons directed to the STA, thus the collision probability is reduced (and the beacon can be transmitted at a much higher rate). How this is handled is not consistent, as fullmac drivers implement their own handling. Possible timeouts are still determined using the beacon interval.

So configuring the beacon interval might or might not give you a benefit here, ultimately it depends on your environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. type: enhancement The changeset is an enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants