Page MenuHomeFreeBSD

net80211: Enable netdump support
AcceptedPublic

Authored by cem on Sep 24 2018, 11:46 PM.

Details

Reviewers
markj
adrian
Summary

Allow net80211 drivers to register a small vtable of netdump-related
methods.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 20675
Build 20090: arc lint + arc unit

Event Timeline

cem created this revision.Sep 24 2018, 11:46 PM
adrian added inline comments.Sep 25 2018, 12:04 AM
sys/net80211/ieee80211.c
710 ↗(On Diff #48416)

Hi!

This stuff should go into the freebsd specific header file, as other OSes are using this wifi stack now!

cem marked an inline comment as done.Sep 25 2018, 12:18 AM
cem added inline comments.
sys/net80211/ieee80211.c
710 ↗(On Diff #48416)

Hi Adrian,

How would I hook vap_attach from the freebsd header? Thanks!

(The stuff below can move, no problem.)

adrian added inline comments.Sep 25 2018, 12:22 AM
sys/net80211/ieee80211.c
710 ↗(On Diff #48416)

Create a separate function, say ieee80211_netdump_attach(), and then the non-freebsd OSes can stub it out.

cem marked an inline comment as done and an inline comment as not done.Sep 25 2018, 12:22 AM
cem added inline comments.
sys/net80211/ieee80211.c
710 ↗(On Diff #48416)

I guess I could hang something off of ieee80211_sysctl_vattach but that seems kind of ugly.

cem marked an inline comment as done.Sep 25 2018, 12:22 AM
cem added inline comments.
sys/net80211/ieee80211.c
710 ↗(On Diff #48416)

Can do, thanks

cem updated this revision to Diff 48417.Sep 25 2018, 12:46 AM

Move netdump attach logic to freebsd-specific net80211 files

markj accepted this revision.Sep 25 2018, 1:35 AM
markj added inline comments.
sys/net80211/ieee80211_var.h
376

I strongly prefer to avoid the ifdef here: the bloat is not significant, and it ensures that KLDs have the same view of the struct layout. I think it's not uncommon for people to compile wifi drivers out-of-tree.

This revision is now accepted and ready to land.Sep 25 2018, 1:35 AM
cem marked an inline comment as done.Sep 25 2018, 1:39 AM
cem added inline comments.
sys/net80211/ieee80211_var.h
376

Yeah, I agree. Will fix.

Yeah please don't ifdef the structure definitions. I need to go do a pass at some point to make that not be a problem; it's 2018 :)

cem updated this revision to Diff 48419.Sep 25 2018, 1:46 AM
cem marked an inline comment as done.

Remove gratuitous ABI difference ifdef NETDUMP

This revision now requires review to proceed.Sep 25 2018, 1:46 AM
cem added a comment.Sep 25 2018, 1:53 AM

Yeah please don't ifdef the structure definitions. I need to go do a pass at some point to make that not be a problem; it's 2018 :)

Are you ok with #if FreeBSD or do you think we should leave the field in on other OS as well? I don't have any preference. Thanks!

markj added a comment.Sep 25 2018, 1:57 AM
In D17308#369025, @cem wrote:

Yeah please don't ifdef the structure definitions. I need to go do a pass at some point to make that not be a problem; it's 2018 :)

Are you ok with #if FreeBSD or do you think we should leave the field in on other OS as well? I don't have any preference. Thanks!

Since you can get away with a forward declaration of the struct, I can't imagine that the field would cause problems on other OS'.

cem added a comment.Sep 25 2018, 2:00 AM

Since you can get away with a forward declaration of the struct, I can't imagine that the field would cause problems on other OS'.

Sure, just struct bloat there.

cem updated this revision to Diff 50106.Nov 7 2018, 7:47 AM

De-conditionalize 80211dump vtable pointer on FreeBSD per markj.

markj accepted this revision.Nov 7 2018, 9:41 PM
This revision is now accepted and ready to land.Nov 7 2018, 9:41 PM