On Mon, 8 Jul 2019 23:17:34 -0500, Parav Pandit wrote: > This patchset carry forwards the work initiated in [1] and discussion > futher concluded at [2]. > > To improve visibility of representor netdevice, its association with > PF or VF, physical port, two new devlink port flavours are added as > PCI PF and PCI VF ports. > > A sample eswitch view can be seen below, which will be futher extended to > mdev subdevices of a PCI function in future. > > Patch-1 moves physical port's attribute to new structure > Patch-2 enhances netlink response to consider port flavour > Patch-3,4 extends devlink port attributes and port flavour > Patch-5 extends mlx5 driver to register devlink ports for PF, VF and > physical link.
The coding leaves something to be desired: 1) flavour handling in devlink_nl_port_attrs_put() really calls for a switch statement, 2) devlink_port_attrs_.*set() can take a pointer to flavour specific structure instead of attr structure for setting the parameters, 3) the "ret" variable there is unnecessary, 4) there is inconsistency in whether there is an empty line between if (ret) return; after __devlink_port_attrs_set() and attr setting, 5) /* Associated PCI VF for of the PCI PF for this port. */ doesn't read great; 6) mlx5 functions should preferably have an appropriate prefix - f.e. register_devlink_port() or is_devlink_port_supported(). But I'll leave it to Jiri and Dave to decide if its worth a respin :) Functionally I think this is okay. Reviewed-by: Jakub Kicinski <jakub.kicin...@netronome.com>
