IfAPI: Add some more accessors
ClosedPublic
Actions

Authored by jhibbits on Jan 25 2023, 9:31 PM.

Details

Reviewers

glebius

Group Reviewers

network

Commits

rG0d2684e15e41: IfAPI: Add some more accessors

Summary

if_setreassignfn for wireguard.
if_getinputfn() and if_getstartfn() for various drivers. Use the function descriptor typedefs for these and the setters.
vlantrunk accessor. This is used by VLAN_CAPABILITIES() used by several drivers, as well as directly by mxge(4).
if_pcp member accessor, used by cxgbe.
accessors for netmap adapter.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jhibbits created this revision.Jan 25 2023, 9:31 PM

Herald added subscribers: melifaro, ae, imp. · View Herald TranscriptJan 25 2023, 9:31 PM

jhibbits requested review of this revision.Jan 25 2023, 9:31 PM

Harbormaster completed remote builds in B49235: Diff 115738.Jan 25 2023, 9:31 PM

melifaro added inline comments.Jan 25 2023, 9:37 PM

sys/net/if.c
4775	Nit: ‘const if_t’ for all accessors?

jhibbits added inline comments.Jan 25 2023, 9:40 PM

sys/net/if.c
4775	Thought about it, but 'const if_t' and 'if_t' mean the same thing, really. The 'const' goes on the pointer, not on the struct. The const makes no difference.

melifaro added inline comments.Jan 26 2023, 2:19 PM

sys/net/if.c
4775	I agree - it's certainly true for the drivers part, where there is no visibility over the `struct ifnet` internals. For the netstack situation may look a bit different. Personally I think it would make sense to use available accessors there as well - at least for consistency. Then, there will be situations where 'const ifnet *ifp' will be passed to the function which then need to access one of the fields. The programmer will have to either do field access directly, or do the typecast. Both approaches doesn't look fancy to me. Finally, apart from helping the compiler to enforce read-only boundaries, it can also signal the intent to the human. What do you think?

No objection from me. Don't you mind that later we come up with a kobj(9) or a kobj-like (see r281151 of projects/ifnet) for all these various softcs?

This revision is now accepted and ready to land.Jan 31 2023, 4:51 PM

Do you have by chance any plans on merging accessors to 13/ to ease other merges?

In D38202#870121, @glebius wrote:

No objection from me. Don't you mind that later we come up with a kobj(9) or a kobj-like (see r281151 of projects/ifnet) for all these various softcs?

I hope we do come up with a better API than this. These accessors just decouple the precise layout, but that's it. For Juniper this is sufficient, but it's far from ideal for the general case.