Page MenuHomeFreeBSD

netgraph/ng_bridge: Reestablish old ABI
AcceptedPublic

Authored by lutz_donnerhacke.de on Wed, Oct 9, 9:35 PM.

Details

Reviewers
julian
Group Reviewers
network
Summary

In https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=240787#c15
Julian noted, that the ABI change should be downward compatible.

Test Plan

Compile an binary using the old API to generate the netgraph messages.
Compile the kernel with the new version of ng_bridge.
The old binary should work and the new node should react accordingly.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 26968
Build 25268: arc lint + arc unit

Event Timeline

It should be backward compatible for 12 at least. Probably not needed for 13 but no harm in carrying it for one release.

Can you use both binaries? e.g. a script uses one and manually you use a newer one?

julian accepted this revision.Thu, Oct 10, 12:54 AM
This revision is now accepted and ready to land.Thu, Oct 10, 12:54 AM

That's why I added the cookies to the API.

Can you use both binaries? e.g. a script uses one and manually you use a newer one?

Nope. I'm currently on vacation and do not have a full test bed available.

It's quite hard to build such (user land) binaries, because there was never a need for.
Usually the node is used in ngctl-scripts which is agnostic to the cookies (it's detecting the control structures by introspection)

It's possible, that kernel (netgraph) modules have compiled messages for the bridge node. In this case, the precompiled netgraph module would be able to communicate with the new code using the old ABI.

But I'm not aware of any such thing. If I'd had such an alien module, I'd be prepared to recompile it after changing the kernel. With is patch the alien module would still work even if the kernel is updated by freebsd-update (without going through the source).

It should be backward compatible for 12 at least. Probably not needed for 13 but no harm in carrying it for one release.

I'd propose to change the "#define NGM_BRIDGE_TABLE_API" into "/* set ... in OPTIONS if you need it */" for 13 and remove the code in 14.

I wasn't saing you shoudl force yourself to do the test.. just whether it is theoretically possible,, anyhow I'm happy with the result. Don't let me stop you from committing it and I would say that the only case for keeping it in 13 is for the brief period while an upgrade is occurring. (for example I know people who have their interface connected into a bridge so that if the bridge wasn't configured correctly they'd be off the air.. It would basically be for compatibility up the 12 line rather than being needed in the 13 line.

I wasn't saing you shoudl force yourself to do the test.. just whether it is theoretically possible,, anyhow I'm happy with the result. Don't let me stop you from committing it and I would say that the only case for keeping it in 13 is for the brief period while an upgrade is occurring. (for example I know people who have their interface connected into a bridge so that if the bridge wasn't configured correctly they'd be off the air.. It would basically be for compatibility up the 12 line rather than being needed in the 13 line.

Because there is an API change, their scripts might fail anyways.
If they explicitly set "ipfw" config (which was useless), the script will fail.
If they explicitly parse the output of "gettable", the script will fail (due to reporting hook names instead of numbers).
For all other cases (incl. the examples in the man page) the API is compatible.

Don't let me stop you from committing it ...

Don't worry, I do not have commiting rights.

emaste added a subscriber: emaste.Fri, Oct 11, 12:46 PM