In the case where a DSA port (via DTB label) had an interface name that collided with an existing netdev name, register_netdevice failed with -EEXIST, and the port was not usable. While this did correctly identify a configuration error in DTB, rather bringup the port with an enumerated interface name, which can be renamed later from userspace where required. While this does change the implicit expectation that it is an error if the DSA port cannot use it's predictable (DTS label) name, there is no functionality to stop netdev from allocating one of these (perhaps poorly selected) DSA port names to a non-DSA device before the DSA device can. While at it, also test that the port name is a valid interface name, before doing the work to setup the device, and use an enumerated name otherwise. This was seen recently (for the EdgeRouter X device) in OpenWrt when a downstream hack [1] was removed, which had used DTS label for ifname in an ethernet device driver, in favour of renaming ifnames in userspace. At the time the device was added to OpenWrt, it only used one network device driver interface, plus the switch ports, so eth1 (matching physical labelling) was used as a switch port label. Since, this device has been adjusted to use phy muxing, exposing a switch port instead as the second network device, so at bringup for this DSA port, eth1 (which is later renamed in userspace) exists, and the eth1 labelled DSA port cannot be used. [1]: https://lore.kernel.org/netdev/20210419154659.44096-3-ilya.lipnitskiy@xxxxxxxxx/ Signed-off-by: John Thomson <git@xxxxxxxxxxxxxxxxxxxxxxxxxxx> --- RFC: Not a full solution. Not sure if supported, I cannot see any users in tree DTS, but I guess I would need to skip these checks (and should mark as NEM_NAME_ENUM) if port->name contains '%'. name is also used in alloc_netdev_mqs, and I have not worked out if any of the functionality between alloc_netdev_mqs and the register_netdevice uses name, so I added these test early, but believe without a rntl lock, a colliding name could still be allocated to another device between this introduced test, and where this device does lock and register_netdevice near the end of this function. To deal with this looks to require moving the rntl_lock before these tests, which would lock around significantly more. As an alternative, could we possibly always register an enumerated name, then (if name valid) dev_change_name (not exported), while still within the lock after register_netdevice? Or could we introduce a parameter or switch-level DTS property that forces DSA to ignore port labels, so that all network devices names can be managed from userspace (using the existing port DSA label as intended name, as this still seems the best place to define device labels, even if the driver does not use this label)? Cheers --- net/dsa/user.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/net/dsa/user.c b/net/dsa/user.c index 867c5fe9a4da..347d2d8eb219 100644 --- a/net/dsa/user.c +++ b/net/dsa/user.c @@ -2684,6 +2684,7 @@ int dsa_user_create(struct dsa_port *port) struct dsa_switch *ds = port->ds; struct net_device *user_dev; struct dsa_user_priv *p; + bool valid_name = false; const char *name; int assign_type; int ret; @@ -2692,6 +2693,20 @@ int dsa_user_create(struct dsa_port *port) ds->num_tx_queues = 1; if (port->name) { + if (!netdev_name_in_use(&init_net, port->name)) + valid_name = true; + else + netdev_warn(conduit, "port %d set name: %s: already in use\n", + port->index, port->name); + if (dev_valid_name(port->name)) { + valid_name &= true; + } else { + valid_name = false; + netdev_warn(conduit, "port %d set name: %s: is invalid\n", + port->index, port->name); + } + } + if (valid_name) { name = port->name; assign_type = NET_NAME_PREDICTABLE; } else { -- 2.45.1
