iwx: migrate to using net80211 crypto key methods
Needs RevisionPublic
Actions

Authored by adrian on Sun, Sep 14, 6:45 PM.

Details

Reviewers

Group Reviewers

Summary

In preparation for further MFP key work, migrate iwx away from using
IEEE80211_KEY_GROUP and pointer arithmetic. This is a hold over from
ye olde days.

Locally tested:

(STA mode, CCMP/CCMP network)
iwx0: <Wi-Fi 6 AX210> mem 0x84c00000-0x84c03fff at device 0.0 on pci4

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 67013
Build 63896: arc lint + arc unit

Event Timeline

adrian created this revision.Sun, Sep 14, 6:45 PM

Herald added a subscriber: imp. · View Herald TranscriptSun, Sep 14, 6:45 PM

adrian requested review of this revision.Sun, Sep 14, 6:45 PM

Harbormaster completed remote builds in B67013: Diff 162000.Sun, Sep 14, 6:45 PM

adrian added a parent revision: D52302: [net80211] clean up M_SEQNO_SET and M_SEQNO_GET() to always limit to the sequence number range.Sun, Sep 14, 6:46 PM

adrian added a reviewer: wireless.

adrian added a project: wireless.

Logic seems fine to me.

The details are probably worth doing to avoid further copy and paste ...

sys/dev/iwx/if_iwx.c
10925	If you use net80211_vap_printf() you neither need ic or sc and we are not using device_printf in a wireless driver anymore directly (bonus point)
10927	return (0)
10962	Here too.
10963	Here () too.
10979	Here too.
10981	Here () too

This revision now requires changes to proceed.Sun, Sep 14, 7:30 PM

adrian added inline comments.Sun, Sep 14, 7:33 PM

sys/dev/iwx/if_iwx.c
10925	If you use net80211_vap_printf() you neither need ic or sc and we are not using device_printf in a wireless driver anymore directly (bonus point) Yeah, i really should use those routines I added. :-) Bonus points is a subsequent commit to convert the rest of device_printf() here to ic_printf / vap_printf where appropriate.

adrian added a child revision: D52524: iwx: add some missed beacon debugging.Sun, Sep 14, 7:38 PM

updates from bz

Harbormaster completed remote builds in B67020: Diff 162021.Sun, Sep 14, 10:34 PM

Sorry, nothing new from you but the original driver already seems to have gotten the return code wrong; can you please double-check?

sys/dev/iwx/if_iwx.c
10944	Can/MUST this even happen? Are we not checking the announced suites anywhere upfront in net80211? I know this was promised but did it happen? iwx only sets: ic->ic_cryptocaps \|= IEEE80211_CRYPTO_AES_CCM; for (*iv_key_set) [ieee80211_crypto_setkey]: 0 means error; != 0 means OK according to ieee80211_ioctl.c (no one else checks the return code it seems) I think all errors in this function are wrong. Question is "what should be an error"?
11012	Are we leaking keys here always telling net80211 we deleted it? XXX-TODO?

This revision now requires changes to proceed.Mon, Sep 15, 3:30 PM

adrian added inline comments.Tue, Sep 16, 5:13 AM

sys/dev/iwx/if_iwx.c
10944	cryptocaps just says "what we support in hardware", remember. That's why I added the methods to make it clearer what's supported in hardware and in software. net80211 expects all NICs to support WEP, TKIP, AES-CCM-128 in software unless you explicitly set the software cryptocaps via the calls I added earlier in the year. As for the errors - yeah, I don't like it at all either; and I'm currently working on adding comments to net80211 key lookup / type routines so it's clearer what the API is and what the driver expectations are. (The more I poke at cleaning up the crypto / cipher management stuff as part of MFP, the more I find gems like these hiding away :-) )

adrian added inline comments.Sun, Sep 21, 4:58 AM

sys/dev/iwx/if_iwx.c
11012	oh god this would be hilarious wouldn't it

adrian marked 2 inline comments as done.Sun, Sep 21, 4:59 AM

Revision Contents
Changeset List

Path

Size

sys/

dev/

iwx/

if_iwx.c

50 lines

Diff 162000

View Options

iwx: migrate to using net80211 crypto key methodsNeeds RevisionPublicActions