Skip to content

container related fixes and additions - #85

Open
dangowrt wants to merge 6 commits into
openwrt:masterfrom
dangowrt:netifd-ubus-device-add
Open

container related fixes and additions#85
dangowrt wants to merge 6 commits into
openwrt:masterfrom
dangowrt:netifd-ubus-device-add

Conversation

@dangowrt

@dangowrt dangowrt commented Jun 8, 2026

Copy link
Copy Markdown
Member

Fix bugs affecting container network setup and expose the remaining ubus surface an external manager needs to wire up containers without taking the (problematic) detour via UCI:

  • create_device and delete_device on the network object allow assembling ephemeral devices such as veth pairs or bridges entirely over ubus, with no persistent config device section. Runtime-created devices are exempt from the config reload sweep, mirroring how add_dynamic interfaces survive a reload.
  • A persistent flag on add_dynamic (default off) keeps a dynamic interface across bare carrier loss; it is only removed once its device actually goes away. Without it, a veth feeding a container whose peer has not gained carrier yet is destroyed while still needed, and an external manager re-adding it churns.
  • add_dynamic now accepts an optional data table, so an interface can be created with e.g. its firewall zone membership and rule data attached atomically. Previously the caller had to follow up with set_data, and a firewall reload landing between the two calls observed the interface without any data.
  • Bug fixes for the jail netns move paths and for a crash when freeing a runtime-created bridge whose vlan_aliases kvlist was never initialised.

v2:

  • rebased onto current master
  • dropped "interface: defer freeing of removed interfaces" and "device: do not leak the config blob when device creation fails", superseded by e2f28e5 ("interface: defer interface removal to avoid use-after-free") and 684dc2d ("device: fix error handling in device_create") respectively
  • reworked "interface: keep persistent dynamic interfaces on carrier loss" on top of the deferred interface removal machinery
  • added "ubus: allow supplying interface data in add_dynamic"

@dangowrt
dangowrt requested a review from nbd168 June 8, 2026 03:58
@dangowrt
dangowrt force-pushed the netifd-ubus-device-add branch 2 times, most recently from 21489fa to 2d319b5 Compare June 11, 2026 23:50
@dangowrt
dangowrt requested review from hauke and lynxis June 11, 2026 23:52
system_link_netns_move() built its RTM_NEWLINK message with whatever
system_if_resolve() returned for the source device. When the device does
not exist (e.g. a jailed interface whose veth has not been created yet),
system_if_resolve() returns 0, and the message goes out with
ifi_index = 0 while IFLA_IFNAME is set to target_ifname (the jail_device
name).

The kernel's __rtnl_newlink() then selects the target device by name when
ifi_index is 0 (net/core/rtnetlink.c: rtnl_dev_get -> __dev_get_by_name),
so the move operates on whatever host device happens to be named like the
jail_device. With a jail_device of "eth0" this moves the host's real eth0
into the container's network namespace, knocking the host off the
network.

Bail out when the source device cannot be resolved to a real ifindex so
the request is never sent with a zero index.

Fixes: d93126d ("interface: allow renaming interface when moving to jail netns")
Signed-off-by: Daniel Golle <[email protected]>
interface_start_jail() moved each jailed interface's main device into the
container netns at the moment the jail's netns was registered. If the
device did not exist yet (e.g. a veth whose creation had not been
triggered), the move was a no-op and the container came up with no
network device; the in-jail netifd then failed DHCP with
"udhcpc: SIOCGIFINDEX: No such device" until the operator manually
brought the host interface up.

Keep a duplicated reference to the jail netns on the interface when the
move cannot be performed yet, and complete it from interface_main_dev_cb
once the device appears (DEV_EVENT_ADD). Any stale pending reference from
an earlier jail incarnation is dropped on the next interface_start_jail()
invocation, so a successful immediate move can never leave a dangling
netns reference behind that would later divert the device into a dead
namespace. interface_stop_jail and interface_free drop any still-pending
reference as well.

Fixes: 1321c1b ("add basic support for jail network namespaces")
Signed-off-by: Daniel Golle <[email protected]>
The vlan_aliases kvlist is initialised lazily in device_vlan_update(),
which only runs during configuration load. A bridge that is created and
freed without going through a config load (e.g. one created at runtime
over ubus) reaches bridge_free() with the kvlist still zero-filled, and
kvlist_free() then walks an uninitialised avl_tree whose list head
contains NULL pointers, crashing netifd. Guard the call on get_len, the
same marker device_vlan_update() uses to tell an initialised kvlist
apart.

Fixes: 4544f02 ("bridge-vlan: add support for defining aliases for vlan ids")
Signed-off-by: Daniel Golle <[email protected]>
Add create_device {name, type, ...} and delete_device {name} ubus methods
on the network object, the device-level analog of add_dynamic. create_device
resolves the device type and creates the device, marking it dynamic;
delete_device finds the device, clears its dynamic and current_config flags
and frees it once unused. A new bool dynamic on struct device exempts these
runtime-created devices from the reload sweep (device_reset_config leaves the
current_config of dynamic devices alone), mirroring how add_dynamic
interfaces survive a reload.

This lets an external manager assemble ephemeral devices such as a veth pair
entirely over ubus, with no persistent UCI 'config device' section.

Signed-off-by: Daniel Golle <[email protected]>
A dynamic interface is removed (IFC_REMOVE) whenever it tears down, including
on a bare carrier loss. For a veth feeding a container whose peer has not
gained carrier yet this destroys the interface and tears down the veth pair
while it is still needed; an external manager re-adding it then churns.

Add a per-interface 'persistent' flag, set at add_dynamic time and carried
across reloads, that suppresses the self-removal on carrier loss: a persistent
dynamic interface is only removed once its device actually goes away. As the
suppression means such an interface can now sit in IFS_DOWN or IFS_TEARDOWN
when its device disappears, handle the removal from interface_main_dev_cb on
DEV_EVENT_REMOVE for those states. Other add_dynamic users keep the current
behaviour (default off).

Signed-off-by: Daniel Golle <[email protected]>
An external manager wiring up a dynamic interface for a container has
to call add_dynamic first and set_data afterwards to attach the data
consumers such as fw4 read from the interface. A firewall reload
landing between the two calls sees the interface with no data at all,
which for zone membership is a security-relevant window rather than a
cosmetic one.

Accept an optional 'data' table in the add_dynamic call and attach it
to the freshly added interface before replying. netifd serves ubus
requests from a single thread, so the interface can never be observed
without its data. Entries follow the same semantics as set_data: each
top-level key is inserted or replaced individually, other keys are left
untouched. The table is validated before the interface is created so a
malformed call does not leave a half-configured interface behind.

Switch interface_parse_data() to blobmsg_for_each_attr so it can walk
a nested table attribute as well as the raw set_data message: the
nested attribute carries a blobmsg name header which blob_for_each_attr
does not skip, making the iteration come up empty.

The firewall zone can already be supplied at creation through the
'zone' attribute; the data table generalises this to every key
consumers read from interface data, such as fw4's per-interface rules.

Signed-off-by: Daniel Golle <[email protected]>
@dangowrt
dangowrt force-pushed the netifd-ubus-device-add branch from 2d319b5 to 17cca8a Compare August 23, 2026 02:38
@dangowrt

Copy link
Copy Markdown
Member Author

Rebased onto current master. Two commits are gone as they were superseded by e2f28e5 and 684dc2d; the persistent interface handling now sits on top of the deferred interface removal. New in v2 is "ubus: allow supplying interface data in add_dynamic", which closes the window between add_dynamic and set_data during which a firewall reload would observe the interface without its data. The whole series has been tested on an x86/64 VM: create_device/delete_device lifecycle, persistent interfaces across carrier loss and config reload, and fw4 picking up zone membership supplied at interface creation.

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.

1 participant