bridge: full IFLA_BR_* support, filter defaults from show #181

vklimovs commented 2026年05月11日 23:53:41 +02:00

Bridge: full IFLA_BR_* support, filter defaults from show

Summary

Implement the complete set of settable kernel bridge attributes and make ifstate show emit a usable, schema-valid config. A bridge at kernel defaults now shows as kind: bridge + state: up instead of dumping 40+ runtime attrs.

Changes

New attributes (30 in schema/2/ifstate.conf.schema.json)

• STP: br_forward_delay, br_hello_time, br_max_age, br_stp_state, br_priority
• Forwarding: br_group_fwd_mask, br_group_addr
• Multicast: br_mcast_snooping, br_mcast_router, br_mcast_querier, hash/cnt/intvl knobs, IGMP/MLD versions, stats
• Netfilter: br_nf_call_iptables/ip6tables/arptables
• Boolopt trio (via IFLA_BR_MULTI_BOOLOPT): br_no_linklocal_learn, br_mcast_vlan_snooping, br_mst_enabled
• FDB cap: br_fdb_max_learned

New module libifstate/link/bridge.py provides:

• BridgeLink subclass — collapses the three boolopt booleans into the packed kernel struct on apply, decodes them on read
• pyroute2 bridge_data.nla_map extension for the five IFLA_BR_* attrs missing from pyroute2 ≤ 0.9.5 (all-or-nothing patch — bails if pyroute2 has any of them, since type ids are derived from nla_map position)
• HZ-aware tolerant compare for USER_HZ timer attrs (br_forward_delay, br_hello_time, br_max_age, br_ageing_time, all mcast_*_intvl). The kernel rounds USER_HZ → jiffies → USER_HZ lossily on HZ=300/250/...; without tolerance every re-apply would chase its tail
• is_schema_default(), split_multi_boolopt(), bridge_boolean_attrs() consumed by the show path

Show path (libifstate/__init__.py)

• Suppresses attrs matching the schema default (timer attrs match both raw and HZ-rounded variants)
• Suppresses read-only/runtime attrs (br_root_id, br_bridge_id, *_timer, br_topology_change*, br_fdb_n_learned, br_pad, br_mcast_querier_state)
• Coerces kernel uint8 0/1 → Python True/False for attrs declared type: boolean, so show output validates against the schema and ifstate show | ifstate apply round-trips clean
• showall=True still dumps everything for debugging

Tests

• Every attr in the full config lands in kernel state
• Second apply records zero link changes (idempotent)
• Individual boolopt toggle preserves the other bits
• Read-only attrs filtered from show; boolopt struct split into individual bools
• External flip of an unmanaged boolopt survives ifstate apply (optmask only covers user keys)
• External timer-attr clobber is detected and corrected — HZ tolerance doesn't mask real drift
• vlan_protocol switch 802.1q→802.1ad on a vlan-filtering bridge preserves brvlan entries on the port
• show | apply round-trip is a no-op
• Schema rejects 7 malformed values (priority overflow, igmp/mld version range, malformed MAC, ...)
• Kernel constraint surface: mcast_vlan_snooping without vlan_filtering fails at netlink and the error is logged
• show(showall=False) AND show(showall=True) both validate against the schema

Notes

• Tested on HZ=300; HZ detection falls back to /proc/config.gz / /boot/config-$(uname -r). On unknown HZ the tolerance is disabled (re-apply will re-set timer attrs every run, but kernel state stays correct).
• pyroute2 patch is global per Python process — documented in the patch's docstring.

## Bridge: full IFLA_BR_* support, filter defaults from show ### Summary Implement the complete set of settable kernel bridge attributes and make `ifstate show` emit a usable, schema-valid config. A bridge at kernel defaults now shows as `kind: bridge` + `state: up` instead of dumping 40+ runtime attrs. ### Changes **New attributes** (30 in `schema/2/ifstate.conf.schema.json`) - STP: `br_forward_delay`, `br_hello_time`, `br_max_age`, `br_stp_state`, `br_priority` - Forwarding: `br_group_fwd_mask`, `br_group_addr` - Multicast: `br_mcast_snooping`, `br_mcast_router`, `br_mcast_querier`, hash/cnt/intvl knobs, IGMP/MLD versions, stats - Netfilter: `br_nf_call_iptables`/`ip6tables`/`arptables` - Boolopt trio (via `IFLA_BR_MULTI_BOOLOPT`): `br_no_linklocal_learn`, `br_mcast_vlan_snooping`, `br_mst_enabled` - FDB cap: `br_fdb_max_learned` **New module** `libifstate/link/bridge.py` provides: - `BridgeLink` subclass — collapses the three boolopt booleans into the packed kernel struct on apply, decodes them on read - pyroute2 `bridge_data.nla_map` extension for the five `IFLA_BR_*` attrs missing from pyroute2 ≤ 0.9.5 (all-or-nothing patch — bails if pyroute2 has any of them, since type ids are derived from `nla_map` position) - HZ-aware tolerant compare for USER_HZ timer attrs (`br_forward_delay`, `br_hello_time`, `br_max_age`, `br_ageing_time`, all `mcast_*_intvl`). The kernel rounds USER_HZ → jiffies → USER_HZ lossily on HZ=300/250/...; without tolerance every re-apply would chase its tail - `is_schema_default()`, `split_multi_boolopt()`, `bridge_boolean_attrs()` consumed by the show path **Show path** (`libifstate/__init__.py`) - Suppresses attrs matching the schema default (timer attrs match both raw and HZ-rounded variants) - Suppresses read-only/runtime attrs (`br_root_id`, `br_bridge_id`, `*_timer`, `br_topology_change*`, `br_fdb_n_learned`, `br_pad`, `br_mcast_querier_state`) - Coerces kernel uint8 0/1 → Python `True`/`False` for attrs declared `type: boolean`, so `show` output validates against the schema and `ifstate show | ifstate apply` round-trips clean - `showall=True` still dumps everything for debugging ### Tests - Every attr in the full config lands in kernel state - Second apply records zero link changes (idempotent) - Individual boolopt toggle preserves the other bits - Read-only attrs filtered from show; boolopt struct split into individual bools - External flip of an unmanaged boolopt survives ifstate apply (optmask only covers user keys) - External timer-attr clobber is detected and corrected — HZ tolerance doesn't mask real drift - `vlan_protocol` switch 802.1q→802.1ad on a vlan-filtering bridge preserves brvlan entries on the port - `show | apply` round-trip is a no-op - Schema rejects 7 malformed values (priority overflow, igmp/mld version range, malformed MAC, ...) - Kernel constraint surface: `mcast_vlan_snooping` without `vlan_filtering` fails at netlink and the error is logged - `show(showall=False)` AND `show(showall=True)` both validate against the schema ### Notes - Tested on HZ=300; HZ detection falls back to `/proc/config.gz` / `/boot/config-$(uname -r)`. On unknown HZ the tolerance is disabled (re-apply will re-set timer attrs every run, but kernel state stays correct). - pyroute2 patch is global per Python process — documented in the patch's docstring.

vklimovs commented 2026年06月14日 08:03:56 +02:00

I'd like to check which direction you'd prefer before I rework this.

The cleanest way I've found to get defaults without the schema is to probe the kernel: briefly create a throwaway bridge in the current netns, read back the IFLA_BR_* values the kernel initialises it with, then delete it (cached once per process). It returns the defaults in the same user-facing form show uses.

Advantages:

• No second source of truth. The defaults are always correct for the running kernel and version — nothing to keep in sync with kernel headers/source by hand. (Worth noting: the default values aren't actually in the uapi headers. Only the NLA names are; the values live in kernel C source, and several are HZ-derived, so a hardcoded table is more fragile than it first looks.)
• It sidesteps the USER_HZ rounding problem entirely. The probe reads back the same rounded value a real bridge would report, so default-matching becomes plain equality and the HZ machinery can go away.
• It generalises to your second point: a Link.kernel_defaults() helper could probe any kind, so other link types could get the same default-filtering later without a per-kind defaults table. (The boolean/read-only attr sets would still need explicit frozensets per kind as those can't be probed, which also matches your "use a frozenset from kernel headers" suggestion.)

The cost is that this introduces a side effect into show: it creates and removes an interface, so it needs CAP_NET_ADMIN and the relevant kernel module to be loadable. If the probe can't run it degrades gracefully (filters nothing).

Are you comfortable with show creating a throwaway interface to learn defaults? If you'd rather show stay strictly read-only/side-effect-free, I'll go with hardcoded frozensets/dicts from the kernel sources instead. Just let me know which trade-off you prefer.

I'd like to check which direction you'd prefer before I rework this. The cleanest way I've found to get defaults *without* the schema is to **probe the kernel**: briefly create a throwaway bridge in the current netns, read back the `IFLA_BR_*` values the kernel initialises it with, then delete it (cached once per process). It returns the defaults in the same user-facing form `show` uses. Advantages: - **No second source of truth.** The defaults are always correct for the running kernel and version — nothing to keep in sync with kernel headers/source by hand. (Worth noting: the default *values* aren't actually in the uapi headers. Only the NLA names are; the values live in kernel C source, and several are HZ-derived, so a hardcoded table is more fragile than it first looks.) - It sidesteps the USER_HZ rounding problem entirely. The probe reads back the same rounded value a real bridge would report, so default-matching becomes plain equality and the HZ machinery can go away. - It **generalises** to your second point: a `Link.kernel_defaults()` helper could probe any kind, so other link types could get the same default-filtering later without a per-kind defaults table. (The boolean/read-only attr *sets* would still need explicit frozensets per kind as those can't be probed, which also matches your "use a frozenset from kernel headers" suggestion.) The cost is that this introduces a **side effect into `show`**: it creates and removes an interface, so it needs `CAP_NET_ADMIN` and the relevant kernel module to be loadable. If the probe can't run it degrades gracefully (filters nothing). Are you comfortable with `show` creating a throwaway interface to learn defaults? If you'd rather `show` stay strictly read-only/side-effect-free, I'll go with hardcoded frozensets/dicts from the kernel sources instead. Just let me know which trade-off you prefer.

liske commented 2026年06月16日 23:11:00 +02:00

I had to ponder about it... sorry for the long delay and long reply.

@vklimovs wrote in #181 (comment):

I'd like to check which direction you'd prefer before I rework this.

The cleanest way I've found to get defaults without the schema is to probe the kernel: briefly create a throwaway bridge in the current netns, read back the IFLA_BR_* values the kernel initialises it with, then delete it (cached once per process). It returns the defaults in the same user-facing form show uses.

I did not like to read the schema for two reasons:

1. on NixOS the schema is not required during runtime since it is already checked during host deployment #112, so I would not like to add any hard dependencies on the schema
2. parsing the schema repeats extracting the default values everytime ifstate is run... which creates a (small) unnecessary overhead while it could be precalculated once and put in some python structures

Creating a throwaway bridge is an even larger effort than 2)... and it creates a (temporary) configuration that the user did not configure... I do not want to go in this direction. Recently the first helper script was added in #182 to extract the ethtool feature list from the kernel git repo and update the schema file accordingly. Maybe we could using something similar here?

1. script creates throwaway bridge
2. gathers the NLA defaults
   • builds a python file containing the defaults as dict
   • updates the default values in the schema file

Advantages:

* **No second source of truth.** The defaults are always correct for the running kernel and version — nothing to keep in sync with kernel headers/source by hand. (Worth noting: the default _values_ aren't actually in the uapi headers. Only the NLA names are; the values live in kernel C source, and several are HZ-derived, so a hardcoded table is more fragile than it first looks.)

Based on the kernel's no regression promise I'm OK with copying the defaults once - they should not change in the kernel afterwards. Since the defaults are well-documented in the schema this also gives some reliability to the user (which is missing in the kernel docs).

* It sidesteps the USER_HZ rounding problem entirely. The probe reads back the same rounded value a real bridge would report, so default-matching becomes plain equality and the HZ machinery can go away.

I'm not sure if I fully understand the issue here. Is the problem only related to the default value or also to custom values? Is the _kernel_round reliable?

* It **generalises** to your second point: a `Link.kernel_defaults()` helper could probe any kind, so other link types could get the same default-filtering later without a per-kind defaults table. (The boolean/read-only attr _sets_ would still need explicit frozensets per kind as those can't be probed, which also matches your "use a frozenset from kernel headers" suggestion.)

The cost is that this introduces a side effect into show: it creates and removes an interface, so it needs CAP_NET_ADMIN and the relevant kernel module to be loadable. If the probe can't run it degrades gracefully (filters nothing).

Are you comfortable with show creating a throwaway interface to learn defaults? If you'd rather show stay strictly read-only/side-effect-free, I'll go with hardcoded frozensets/dicts from the kernel sources instead. Just let me know which trade-off you prefer.

The script is somewhat in between:

• run only by a developer who commits the changes to git
• can be used to update the frozensets/dicts later in case the kernel gets new features (NLAs)
• does not increase the costs for running ifstate by evaluating files or creating thoaway interfaces
• should still match the real kernel defaults... at least we have some well documented defaults for the user 😉

I had to ponder about it... sorry for the long delay and long reply. @vklimovs wrote in https://codeberg.org/routerkit/ifstate/pulls/181#issuecomment-17429129: > I'd like to check which direction you'd prefer before I rework this. > > The cleanest way I've found to get defaults _without_ the schema is to **probe the kernel**: briefly create a throwaway bridge in the current netns, read back the `IFLA_BR_*` values the kernel initialises it with, then delete it (cached once per process). It returns the defaults in the same user-facing form `show` uses. I did not like to read the schema for two reasons: 1) on NixOS the schema is not required during runtime since it is already checked during host deployment #112, so I would not like to add any hard dependencies on the schema 2) parsing the schema repeats extracting the default values everytime ifstate is run... which creates a (small) unnecessary overhead while it could be precalculated once and put in some python structures Creating a throwaway bridge is an even larger effort than 2)... and it creates a (temporary) configuration that the user did not configure... I do not want to go in this direction. Recently the first helper script was added in #182 to extract the ethtool feature list from the kernel git repo and update the schema file accordingly. Maybe we could using something similar here? 1) script creates throwaway bridge 2) gathers the NLA defaults - builds a python file containing the defaults as dict - updates the default values in the schema file > Advantages: > > * **No second source of truth.** The defaults are always correct for the running kernel and version — nothing to keep in sync with kernel headers/source by hand. (Worth noting: the default _values_ aren't actually in the uapi headers. Only the NLA names are; the values live in kernel C source, and several are HZ-derived, so a hardcoded table is more fragile than it first looks.) Based on the kernel's no regression promise I'm OK with copying the defaults once - they should not change in the kernel afterwards. Since the defaults are well-documented in the schema this also gives some reliability to the user (which is missing in the kernel docs). > * It sidesteps the USER_HZ rounding problem entirely. The probe reads back the same rounded value a real bridge would report, so default-matching becomes plain equality and the HZ machinery can go away. I'm not sure if I fully understand the issue here. Is the problem only related to the default value or also to custom values? Is the `_kernel_round` reliable? > * It **generalises** to your second point: a `Link.kernel_defaults()` helper could probe any kind, so other link types could get the same default-filtering later without a per-kind defaults table. (The boolean/read-only attr _sets_ would still need explicit frozensets per kind as those can't be probed, which also matches your "use a frozenset from kernel headers" suggestion.) > > > The cost is that this introduces a **side effect into `show`**: it creates and removes an interface, so it needs `CAP_NET_ADMIN` and the relevant kernel module to be loadable. If the probe can't run it degrades gracefully (filters nothing). > > Are you comfortable with `show` creating a throwaway interface to learn defaults? If you'd rather `show` stay strictly read-only/side-effect-free, I'll go with hardcoded frozensets/dicts from the kernel sources instead. Just let me know which trade-off you prefer. The script is somewhat in between: - run only by a developer who commits the changes to git - can be used to update the frozensets/dicts later in case the kernel gets new features (NLAs) - does not increase the costs for running ifstate by evaluating files or creating thoaway interfaces - should still match the real kernel defaults... at least we have some well documented defaults for the user 😉

This pull request has changes conflicting with the target branch.