On Thu, Oct 03, 2019 at 04:31:32PM +0000, Jason Gunthorpe wrote: > On Sat, Sep 28, 2019 at 07:54:16PM +0300, Leon Romanovsky wrote: > > On Thu, Sep 26, 2019 at 01:58:38PM -0400, Jonathan Toppins wrote: > > > On 09/26/2019 08:34 AM, Jason Gunthorpe wrote: > > > > On Thu, Sep 26, 2019 at 12:42:53PM +0300, Leon Romanovsky wrote: > > > >> From: Leon Romanovsky <leonro@xxxxxxxxxxxx> > > > >> > > > >> Virtual devices like SIW or RXE don't set FW version because > > > >> they don't have one, use that fact to rely on having empty > > > >> fw_ver file to sense such virtual devices. > > > > > > > > Have you checked that every physical device does set fw version? > > > > > > > > Seems hacky > > > > > > agreed, how are tuntap devices handled, is there a similar handling that > > > can be applied here? > > > > Unfortunately, we can't do the same, RDMA doesn't have notion of stacked devices. > > > > 1. > > TUN devices are initialized with ARPHRD_NONE type. > > https://elixir.bootlin.com/linux/latest/source/drivers/net/tun.c#L1396 > > > > It causes for systemd-udev to skip their rename. > > https://github.com/systemd/systemd/blob/master/src/udev/udev-builtin-net_id.c#L781 > > > > 2. > > TAP devices are skipped due to the fact that iflink != ifindex on such devices. > > https://github.com/systemd/systemd/blob/master/src/udev/udev-builtin-net_id.c#L810 > > > > So, yes hacky, but the solution is tailored to RDMA subsystem where ALL > > devices have FW and we can ensure that ALL future devices will report any > > sort of string through fw_ver file. > > It still seems really hacky, why not add some device flag or something > instead? Is this better because it works with old kernels? Yes, I'm trying to find a way to do such discovery without changing kernel, because we have only two virtual devices and I don't expect to see any new ones in forth coming future. However, this patch is wrong because there are at least two drivers (hns and EFA) didn't implement .get_dev_fw_str() and they will have empty fw_ver. 805 void ib_get_device_fw_str(struct ib_device *dev, char *str) 806 { 807 if (dev->ops.get_dev_fw_str) 808 dev->ops.get_dev_fw_str(dev, str); 809 else 810 str[0] = '\0'; 811 } 812 EXPORT_SYMBOL(ib_get_device_fw_str); Special flag seems too much for me, what about writing special string to fw_ver to indicate virtual device? For example "virtual". Thanks > > Jason
