libifconfig: Add function to get bridge status

[freqlabs]

Summary

The new function operates similarly to ifconfig_lagg_get_lagg_status and likewise is accompanied by a function to free the bridge status data structure.

I have included in this patch the relocation of some strings describing STP parameters and the PV2ID macro from ifconfig into net/if_bridgevar.h as they are useful for consumers of libifconfig.

Test Plan

I added bindings for the new functions to my Lua wrapper for libifconfig and produced the following json:

{
  "address_cache_size": 2000,
  "address_cache_lifetime": 1200,
  "forward_delay": 15,
  "hello_time": 2,
  "hold_count": 6,
  "id": "00:00:00:00:00:00",
  "max_age": 20,
  "members": {
    "lagg0": {
      "flags": [
        "LEARNING",
        "DISCOVER",
        "AUTOEDGE",
        "AUTOPTP"
      ],
      "ifmaxaddr": 0,
      "path_cost": 2000000,
      "port": 6,
      "priority": 128
    }
  },
  "priority": 32768,
  "protocol": 2,
  "root_id": "00:00:00:00:00:00",
  "root_path_cost": 0,
  "root_port": 0,
  "root_priority": 32768
}

[kp]

lib/libifconfig/libifconfig_bridge.c

73 ↗

(On Diff #73685)

Would it be worth doing *bridge_status = NULL here? I expect that'd reduce the likelihood of bugs in users of this function (e.g. if they fail to check the return status and try to use *bridge_status it'll be a lot more obvious).

133 ↗

(On Diff #73685)

The function documentation says this does nothing if bs is NULL, but ...

[melifaro]

lib/libifconfig/libifconfig.h

64 ↗

(On Diff #73685)

(again) Sorry, any chance of having more descriptive name? (Comment would also help).

65 ↗

(On Diff #73685)

As far as I understand, here we return the structure with size& list of struct ifbreq.
Any chance it can be simplified by storing the number of member interfaces in `
ifconfig_bridge_status` and pointing directly to the array of struct ifbreq*?

lib/libifconfig/libifconfig_bridge.c

46 ↗

(On Diff #73685)

Any chance of having a bit more descriptive field name? status, br_status ?

92 ↗

(On Diff #73685)

Maybe it's worth renaming param to something different ? :-)
It looks a bit strange when it's used everywhere except BRDGPARAM ioctl.

sys/net/if_bridgevar.h

272 ↗

(On Diff #73685)

Sorry, can you please clarify what is the benefit of moving from the const array of strings containing the relevant data for the states/protos/etc to these kind of defines?

[melifaro]

Thank you for adding the bridge support!
Please see some minor comments inline.

[freqlabs]

Address feedback

[freqlabs]

sys/net/if_bridgevar.h

272 ↗

(On Diff #73685)

I'm moving the useful information from the source for ifconfig into a header where it can be reused by other code. This same convention is used for example to define CARP_STATES and similarly LAGG_PROTOS already.

[melifaro]

Thanks for making the changes!
LGTM.

lib/libifconfig/libifconfig.h

64 ↗

(On Diff #73811)

(nit) members_count to follow the pattern of object_property used in 2 variables names below?

[freqlabs]

Rename num_members to members_count for consistency.

Thanks for putting a fresh set of eyes on this :)