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

hostapd: fix IEEE 802.11r (fast roaming) defaults #940

Closed
wants to merge 1 commit into from

Conversation

dwmw2
Copy link
Contributor

@dwmw2 dwmw2 commented May 13, 2018

Use ft_psk_generate_local=1 by default, as it makes everything else fairly
trivial. All of the r0kh/r1kh and key management stuff goes away and hostapd
fairly much does it all for us.

We do need to provide nas_identifier, which can be derived from the BSSID,
and we need to generate a mobility_domain, for which we default to the first
four chars of the md5sum of the SSID.

The complex manual setup should also still work, but the defaults also
now work easily out of the box. Verified by manually running hostapd
(with the autogenerated config) and watching the debug output:

wlan2: STA ac:37:43:a0:a6:ae WPA: FT authentication already completed - do not start 4-way handshake

This was previous submitted to LEDE in
lede-project/source#1382

[dwmw2: Rewrote commit message]
Signed-off-by: Gospod Nassa devianca@gmail.com
Signed-off-by: David Woodhouse dwmw2@infradead.org

@dwmw2
Copy link
Contributor Author

dwmw2 commented May 13, 2018

Thanks @devianceluka for doing the real work here; I'm just tidying up the commit message a bit and making sure it doesn't get lost.

@pkgadd
Copy link
Contributor

pkgadd commented May 13, 2018

I've been using this (the original) patchset since november 2017, it would be great to get this merged.

@dedeckeh dedeckeh added the core packages pull request/issue for core (in-tree) packages label May 14, 2018
@dwmw2
Copy link
Contributor Author

dwmw2 commented May 17, 2018

Also enable 11r support in wpad-mini since it is fairly small.

@hauke
Copy link
Member

hauke commented May 21, 2018

Could you please rebase this on top of current master as the first part was already applied to master.

@dwmw2
Copy link
Contributor Author

dwmw2 commented May 21, 2018

Done, and added LTO to claw back some of the size increase. Not that it was massive anyway.

@juppin
Copy link

juppin commented Jun 8, 2018

Also enable 11r support in wpad-mini since it is fairly small.

Thats a great idea... I hate it to install full wpad for only 802.11r

@@ -133,6 +133,10 @@ ifneq ($(LOCAL_TYPE),hostapd)
ifdef CONFIG_WPA_SUPPLICANT_NO_TIMESTAMP_CHECK
TARGET_CFLAGS += -DNO_TIMESTAMP_CHECK
endif
ifeq ($(LOCAL_VARIANT),mini)
TARGET_CFLAGS += -flto -Os
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove -Os, it's already in CFLAGS by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

With so many devices having dual 2.4GHz + 5GHz radios, this isn't a
particularly esoteric use case any more. Luci offers it by default too.

On my test build the size increase is relatively modest:
   text	   data	    bss	    dec	    hex	filename
 425067	   2704	    356	 428127	  6885f	wpad-mini
 452003	   2684	    372	 455059	  6f193	wpad-mini+11r
 717339	   2796	    588	 720723	  aff53	wpad-full

Let's just enable it by default. We can offset some of the size increase
by enabling LTO:
 437615	   2496	    372	 440483	  6b8a3	wpad-mini+11r+lto

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
@ldir-EDB0
Copy link
Contributor

I hate to see this dying a death. The stumbling block here is the size (and functionality) increase for what is a mini(malist) package with attendant concerns that the size increase breaks the bank on 'legacy' routers that are short on space. The way forward that I can certainly vote for is to create (yet another) variant, I suggest 'wpad-basic' which gives you the mini(malist) defaults but adds a few finesse functionality points, such as 11r & 11w. Make wpad-basic the new default, so that 11r/w works out of the box with luci.

I'm happy to work with whoever to get this going, though wpad/hostapd is going to be a voyage of discovery for me :-)

@mkresin
Copy link
Member

mkresin commented Jul 9, 2018 via email

@diizzyy
Copy link
Contributor

diizzyy commented Jul 9, 2018

As mips is the concern, what's the difference in size between with 11r (incl -flto) and without?
I think it's a bad and confusing idea to have the same name for a package which is different between arches/platforms.

@ldir-EDB0
Copy link
Contributor

that is not what is being proposed at all.

@diizzyy
Copy link
Contributor

diizzyy commented Jul 9, 2018

Ahh, so there's going to be yet another variant?
Are we talking about 10kbyte in the end or what is the difference in size?

@ldir-EDB0
Copy link
Contributor

yes

@diizzyy
Copy link
Contributor

diizzyy commented Jul 9, 2018

Wouldn't it be a good idea to look at the size first? If it's ends up being the ~same as the current version/variant is it really worth bothering making another one?

@ldir-EDB0
Copy link
Contributor

The commit has some size info included, also with -lto for comparison - I doubt the mips numbers are significantly different in % terms. I've seen pressure (in IRC) that an increase in size caused by an increase in functionality on 'mini' packages is not desirable. The only way out of that is to create a new package. There was talk of renaming wpad-mini to wpad-tiny and have a new wpad-mini. Personally, if mini is short for minimalist, then it should provide the minimum required functionality without finesse features for the smallest size. Basic can take that as a starting point and add some finesse features, obviously with a size penalty.

@nbd168
Copy link
Member

nbd168 commented Jul 10, 2018

I just pushed a binutils update + upstream backport, which fixes some LTO issues in my build, along with some changes to enable full LTO for all hostapd/wpa_supplicant variants, including make jobserver support

@diizzyy
Copy link
Contributor

diizzyy commented Jul 10, 2018

@ldir-EDB0
Ok, that makes sense. I agree that naming is a bit confusing however it's probably going to be a hard to align all packages. Perhaps worth looking at other projects how they handle it? https://www.freebsd.org/doc/en/books/porters-handbook/flavors.html for instance

@nbd168
Copy link
Member

nbd168 commented Jul 12, 2018

With LTO enabled for both the 11r and non-11r wpad-mini build, the size difference for the full gzip-compressed .ipk file is around 25k. I'd estimate that this feature costs around 15-20k of flash space after LZMA.

@diizzyy
Copy link
Contributor

diizzyy commented Jul 12, 2018

@nbd168
Thanks for providing more information

@blogic
Copy link
Contributor

blogic commented Aug 1, 2018

duplicate of what we have pending in patchwork

@blogic blogic closed this Aug 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core packages pull request/issue for core (in-tree) packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants