libifconfig: Add initial support for if_bridge

[marieheleneka_gmail.com]

Summary

Implements support for adding and removing members to/from bridges.
Fixed typo in some docxml comments
Submitted by: Marie Helene Kvello-Aune ([e-mail])
MFC after: 30d

Not to be committed until after 12 has branched.

[kp]

I’ll try to review in the next day or two, but it needs tests ;) (in a separte commit)
I think there are currently no functional tests for if_bridge, but it should be pretty straightforward to spin up a vnet jail, create a bridge in it and then use libifconfig to add/remove interfaces.
It’d have the very nice side-effect of getting us some basic bridge tests too.

[kp]

lib/libifconfig/Makefile

19

(Style) You've got trailing spaces here.

lib/libifconfig/libifconfig.h

262

(Style) You've got trailing spaces here.

lib/libifconfig/libifconfig_bridge.c

55

(Style) This line is too long.

66

(Style) This line is too long.

(Idle musing) How much detail do we get from the bridge ioctl code? Could we, for example, distinguish 'interface to add doesn't exist' from 'bridge doesn't exist' from 'access denied'?

78

I wonder if it's worth checking that the user isn't doing silly things, like passing in NULL pointers, or overly long interface names (> IFNAMSIZ).

That wasn't really a concern in ifconfig, but when others start using this we may want more validation.

(Certainly not a blocking issue.)

[marieheleneka_gmail.com]

lib/libifconfig/Makefile

19

Made the list one entry per line, to reduce size of future diffs.

lib/libifconfig/libifconfig_bridge.c

66

I'm not sure exactly how much detail we can get from this, but the error code is stored as-is by ifconfig_ioctlwrap, and can be retrieved using the ifconfig_err_* functions.

78

We might want to do that. This should probably also be done for various other parameters, like description length. I'll add it to the todo list.

[marieheleneka_gmail.com]

Update according to feedback

[marieheleneka_gmail.com]

More trailing whitespace

[marieheleneka_gmail.com]

Some minor corrections. Turns out it helps if I run tests on the right tree. :)

[kp]

lib/libifconfig/libifconfig_bridge.c

72

This can probably be a bool. It's only ever going to be true or false.

It might also be helpful to make this part of the public API of libifconfig. I can certainly imagine situations where I'd rather use this than if (add) ifconfig_bridge_add_member(); else ifconfig_bridge_remove_member().
The counterargument to that is that it might be better to have a smaller API. Your call :)

[marieheleneka_gmail.com]

lib/libifconfig/libifconfig_bridge.c

72

I contemplated not implementing the ifconfig_bridge_add_member() and ifconfig_bridge_remove_member(), and rather have the static ifconfig_bridge_addrem_member() be part of the API as ifconfig_bridge_modify_member(), with the current arguments. IIRC, there are a few other parts of the API which could benefit from such consolidation as well; I'll have to look closer at it.

I like the idea of using bool, but in that case, it should probably be used for every situation where it's really just a boolean.

I'll ponder some on the two above points and make a decision on how to do this. At least it's easy to implement the decision, as libifconfig is marked private and therefore doesn't need API/ABI compatibility between releases.