Differential D38104

ifnet/API: Privatize the implementation of the drbr_* APIs
ClosedPublic
Actions

Authored by jhibbits on Jan 17 2023, 4:20 PM.

Details

Reviewers

glebius

Group Reviewers

network

Commits

rG1bfa548b1f16: ifnet/API: Privatize the implementation of the drbr_* APIs

Summary

When ALTQ is enabled ifnet accessors already need to be called, largely
defeating the purpose of the inline. To that extent, make the ALTQ form
functions in the netstack proper, and make them always available.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 49149
Build 46038: arc lint + arc unit

Event Timeline

jhibbits created this revision.Jan 17 2023, 4:20 PM

Herald added subscribers: melifaro, ae, imp. · View Herald TranscriptJan 17 2023, 4:20 PM

jhibbits requested review of this revision.Jan 17 2023, 4:20 PM

Harbormaster completed remote builds in B49149: Diff 115239.Jan 17 2023, 4:20 PM

glebius added inline comments.Jan 23 2023, 5:06 PM

sys/net/ifq.h
317
318–328	IMHO, these forward declarations should be under #ifdef ALTQ. Otherwise, with non-ALTQ case, we will have a forward declaration of a function as non-static, followed by a static implementation. Looks like our compiler doesn't care about that, but it could confuse other compilers. I could be wrong here, but better stay safe.
331	drbr_free() being the same for both cases should go down below (or may be higher up).
338

jhibbits added inline comments.Jan 23 2023, 5:27 PM

sys/net/ifq.h
318–328	Correct, my build testing was with LINT, which enables ALTQ so I didn't see any errors or warnings. Will fix it.
331	Moving this down below the block, since it needs drbr_flush().

Address @glebius's feedback.

Harbormaster completed remote builds in B49198: Diff 115459.Jan 23 2023, 5:32 PM

glebius accepted this revision as: glebius.Jan 23 2023, 7:09 PM

This revision is now accepted and ready to land.Jan 23 2023, 7:09 PM

Faied to build with ALTQ disabled, functions redefined. Explicitly define ALTQ to only get the prototypes, and also suggest this is intended only for ALTQ.

This revision now requires review to proceed.Jan 25 2023, 8:47 PM

Harbormaster completed remote builds in B49229: Diff 115732.Jan 25 2023, 8:47 PM

This revision was not accepted when it landed; it landed in state Needs Review.Jan 31 2023, 8:03 PM

Closed by commit rG1bfa548b1f16: ifnet/API: Privatize the implementation of the drbr_* APIs (authored by jhibbits). · Explain Why

This revision was automatically updated to reflect the committed changes.

jhibbits added a commit: rG1bfa548b1f16: ifnet/API: Privatize the implementation of the drbr_* APIs.