Skip to content

vrf: add fixes and sync changes with bonding.c/bridge.c#79

Open
Linaro1985 wants to merge 6 commits into
openwrt:masterfrom
Linaro1985:vrf_fixes
Open

vrf: add fixes and sync changes with bonding.c/bridge.c#79
Linaro1985 wants to merge 6 commits into
openwrt:masterfrom
Linaro1985:vrf_fixes

Conversation

@Linaro1985

@Linaro1985 Linaro1985 commented May 6, 2026

Copy link
Copy Markdown
Contributor

VRF support was originally based on bonding.c/bridge.c. Since being added to netifd, fixes have been made that did not make it into vrf.c. Therefore, let's bring in the missing changes.

Commits taken as a basis:

Also this PR contains next fixes:

  • vrf: remove incorrect IPv6 disable on VRF ports
  • vrf: remove unused vrf_empty field
  • vrf: rename vrf_state_type to vrf_device_type
  • vrf: add license header
  • system-linux: fix system_vrf_addif retry loop

@Linaro1985 Linaro1985 force-pushed the vrf_fixes branch 2 times, most recently from 3320822 to 01708db Compare May 6, 2026 06:10
@Linaro1985 Linaro1985 changed the title vrf: sync changes with bonding.c/bridge.c vrf: fixes and sync changes with bonding.c/bridge.c May 6, 2026
@Linaro1985

Copy link
Copy Markdown
Contributor Author

ping @systemcrash @howels

@systemcrash systemcrash left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look like clean additions/changes.

@Linaro1985 Linaro1985 changed the title vrf: fixes and sync changes with bonding.c/bridge.c vrf: add fixes and sync changes with bonding.c/bridge.c May 7, 2026
Commits taken as a basis:
- device: fix build error on 32 bit systems
- debug: remove newline from debug messages
- bridge: skip present toggle in bridge_free_member() when device is active
- bridge: remove kernel member on teardown regardless of device claim state
- bridge: attempt delbr unconditionally on bridge destroy

Signed-off-by: Maxim Anisimov <maxim.anisimov.ua@gmail.com>
Unlike bridge members, VRF enslaved interfaces remain full L3 devices.
Disabling IPv6 on them was copied from bridge.c but is semantically
wrong for VRF: link-local addresses on VRF ports are needed for IPv6
routing protocols (BGP, OSPFv3) to function within the VRF domain.

The Linux kernel VRF documentation confirms that enslaved interfaces
participate in IPv4 and IPv6 stacks normally, with their routes
automatically moved to the VRF-associated routing table.

Signed-off-by: Maxim Anisimov <maxim.anisimov.ua@gmail.com>
vrf_empty is set unconditionally to true in vrf_apply_settings() and
has no corresponding UCI attribute, so it can never be false. Its only
observable effect is to always trigger force_active=true and
device_set_present() in vrf_config_init(), and to make vrf_remove_member()
return early so that force_active is never cleared and device_set_present(false)
is never called when all ports are removed.

This reflects the intended semantics: a VRF without ports is a valid
and useful configuration. Replace the dead conditional with direct
unconditional assignments, and drop the now-unreachable
force_active/device_set_present block from vrf_remove_member()

Signed-off-by: Maxim Anisimov <maxim.anisimov.ua@gmail.com>
Rename the static device_type descriptor from vrf_state_type to
vrf_device_type for consistency with bridge_device_type and
bonding_device_type. The _state suffix is misleading as it implies
a connection to struct vrf_state, while this is a static type
descriptor unrelated to per-instance state.

Also remove a spurious blank line in vrf_member_update().

Signed-off-by: Maxim Anisimov <maxim.anisimov.ua@gmail.com>
Add the standard GPLv2 license header consistent with other netifd
modules. Co-authored-by Felix Fietkau is noted as the bridge and
bonding device management patterns were derived from his bridge.c
and bonding.c.

Signed-off-by: Maxim Anisimov <maxim.anisimov.ua@gmail.com>
The previous implementation using goto had three bugs:

1. The debug message was printed unconditionally, before checking ret,
   so "Failed to add" was logged even on success.

2. After a successful system_vrf_if() call (ret == 0), the code still
   jumped back to retry instead of exiting, causing up to 3 unnecessary
   redundant kernel calls.

3. The off-by-one in the tries guard (tries <= 3 after post-increment)
   allowed 4 iterations instead of 3.

Rewrite using a for loop with break on success, mirroring the structure
of system_bridge_addif().

Signed-off-by: Maxim Anisimov <maxim.anisimov.ua@gmail.com>
@Linaro1985

Linaro1985 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

@robimarko, @nbd168, are there any additional changes needed for this to be accepted?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants