On Sat, Jun 08, 2024 at 11:47:24AM +1000, John Thomson wrote:
> 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)?
Why not just _not_ use the 'label' device tree property, and bring
a decent udev implementation into OpenWrt which can handle persistent
naming according to the labels on the box? Even within DSA, it is
considered better practice to use udev rather than 'label'. Not to
mention that once available, udev is a uniform solution for all network
interfaces, unlike 'label'.
Full disclosure: I myself tried for about 30 minutes to convert the udev
rules below into an /etc/hotplug.d script that procd would run, before
getting the impression it's never going to work as intended, because by
the time all relevant "add" actions run (built-in drivers), user space
hasn't even loaded, and thus hasn't got a chance to run any hooks.
I haven't actually opened the source code to compare how other uevent
handlers deal with this.
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:00.5", DRIVERS=="mscc_felix", ATTR{phys_port_name}=="p0", NAME="swp0"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:00.5", DRIVERS=="mscc_felix", ATTR{phys_port_name}=="p1", NAME="swp1"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:00.5", DRIVERS=="mscc_felix", ATTR{phys_port_name}=="p2", NAME="swp2"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:00.5", DRIVERS=="mscc_felix", ATTR{phys_port_name}=="p3", NAME="swp3"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:00.5", DRIVERS=="mscc_felix", ATTR{phys_port_name}=="p4", NAME="swp4"
ACTION=="add", SUBSYSTEM=="net", KERNELS=="0000:00:00.5", DRIVERS=="mscc_felix", ATTR{phys_port_name}=="p5", NAME="swp5"
