On 2021/6/10 0:40, Jakub Kicinski wrote:
On Wed, 9 Jun 2021 17:16:06 +0800 Yunsheng Lin wrote:
On 2021/6/9 1:29, Jakub Kicinski wrote:
On Tue, 8 Jun 2021 20:10:37 +0800 Yunsheng Lin wrote:
Afer discussion with Parav in other thread, I undersood it was the current practice, but I am not sure I understand why it is current *best* practice.
If we allow all PF of a ASCI to register to the same devlink instance, does it not make sense that all VF under one PF also register to the same devlink instance that it's PF is registering to when they are in the same host?
For eswitch legacy mode, whether VF and PF are the same host or not, the VF can also provide the serial number of a ASIC to register to the devlink instance, if that devlink instance does not exist yet, just create that devlink instance according to the serial number, just like PF does.
For eswitch DEVLINK_ESWITCH_MODE_SWITCHDEV mode, the flavour type for devlink port instance representing the netdev of VF function is FLAVOUR_VIRTUAL, the flavour type for devlink port instance representing the representor netdev of VF is FLAVOUR_PCI_VF, which are different type, so they can register to the same devlink instance even when both of the devlink port instance is in the same host?
Is there any reason why VF use its own devlink instance?
Primary use case for VFs is virtual environments where guest isn't trusted, so tying the VF to the main devlink instance, over which guest should have no control is counter productive.
The security is mainly about VF using in container case, right? Because VF using in VM, it is different host, it means a different devlink instance for VF, so there is no security issue for VF using in VM case? But it might not be the case for VF using in container?
How do you differentiate from the device perspective VF being assigned to the host vs VM? Presumably PFs and VFs have a similar API to talk to the FW, if VF can "join" the devlink instance of the PF that'd suggest to me it has access to privileged FW commands.
I was thinking info/param/health that is specfic to a VF is only registered to the devlink port instance of that VF, same for resource that is specific to PF. And it seems the param is already able to registered based on devlink instance(devlink_params_register()) or based on devlink port instance( devlink_port_params_register()).
Only PF will register privileged common resource based on devlink instance, we may need to ensure only one PF register the privileged common resource (maybe the PF probed first do the the privileged common resource registering, I am not sure how to ensure that or implement it yet).
When user access the common resource in devlink instance, I think it is ok to pass it through one of the PF(suppose all PF is in the same privilege level)?
When user access the resource in devlink port instance of PHYSICAL/VIRTUAL, the access is through the specific function(PF/VF) corresponds to that devlink port instance?
When user access the resource in devlink port instance of PCI_PF/PCI_VF/PCI_SF, the access is through the function where the eswitch is located?
so if a devlink instance only have devlink port instance of VF, that devlink instance has not privileged common resource registered, so the user is not able to access the privileged common resource?
If the PF and VF is in the same host and in the same net namespace, I suppose it is ok to have the PF and VF to share the same devlink instance with the privileged common resource registered?
Also I read about the devlink disscusion betwwen you and jiri in [1]: "I think we agree that all objects of an ASIC should be under one devlink instance, the question remains whether both ends of the pipe for PCI devices (subdevs or not) should appear under ports or does the "far end" (from ASICs perspective)/"host end" get its own category."
I am not sure if there is already any conclusion about the latter part (I did not find the conclusion in that thread)?
"far end" (from ASICs perspective)/"host end" means PF/VF, right? Which seems to correspond to port flavor of FLAVOUR_PHYSICAL and FLAVOUR_VIRTUAL if we try to represent PF/VF using devlink port instance?
No, no, PHYSICAL is a physical port on the adapter, like an SFP port. There wasn't any conclusion to that discussion. Mellanox views devlink ports as eswitch ports, I view them as device ports which is hard to reconcile.
I suppose eswitch ports only exist when DEVLINK_ESWITCH_MODE_SWITCHDEV mode is enabled, right? Does "Mellanox views devlink ports as eswitch ports" means mlx driver will not create any devlink port instance when DEVLINK_ESWITCH_MODE_LEGACY mode is enabled? It does not seems to be the case any more, because the PF is registered as a devlink port instance of FLAVOUR_PHYSICAL and VF is registered as a devlink port instance of FLAVOUR__VIRTUAL in mlx5e_devlink_port_register(), unless mlx5e_devlink_port_register() is only called in SWITCHDEV mode too.
From discussion in other thread with parav in [1], it seems: 1. Whenever there is a pcie function(PF/VF, maybe SF too?), there is a devlink instance corresponds to that pcie function. 2. Whenever there is a netdev(netdev of PF/VF, or representor netdev), there is a devlink port instance corresponds to that netdev.
It seems we only need to change (1) to enable "all objects of an ASIC should be under one devlink instance" as below: Whenever there is a ASIC(or switch), there is a devlink instance corresponds to that ASIC(or switch)?
I am not sure I understand what it means by "device ports"? netdev? or "physical port on the adapter, like an SFP port"? or "pcie function like PF/VF"? Let's suppose it is in MODE_LEGACY mode.
1. https://patchwork.kernel.org/project/netdevbpf/patch/20210603111901.9888-1-p...
It seems the conclusion is very important to our disscusion in this thread, as we are trying to represent PF/VF as devlink port instance in this thread(at least that is what I think, hns3 does not support eswitch SWITCHDEV mode yet).
Also, there is a "switch_id" concept from jiri's example, which seems to be not implemented yet? pci/0000:05:00.0/10000: type eth netdev enp5s0npf0s0 flavour pci_pf pf 0 subport 0 switch_id 00154d130d2f
I am not sure I understand what does it mean by "devlink instances with multiple names"?
Does that mean whenever a devlink port instance is registered to a devlink instance, that devlink instance get a new name according to the PCI device which the just registered devlink port instance corresponds to?
Not devlink port, new PCI device. Multiple ports may reside on the same PCI function, some ports don't have a function (e.g. Ethernet ports).
Multiple ports on the same mainly PCI function means subfunction from mlx, right?
Not necessarily, there are older devices out there (older NFPs, mlx4) which have one PF which is logically divided by the driver to service multiple ports.
“some ports don't have a function (e.g. Ethernet ports)” does not seem exist yet? For now devlink port instance of FLAVOUR_PHYSICAL represents both PF and Ethernet ports?
It does. I think Mellanox cards are incapable of divorcing PFs from Ethernet ports, but the NFP driver represents the Ethernet port/SFP as one netdev and devlink port (PHYSICAL) and the host port by another netdev and devlink port (PCI_PF). Which allows forwarding frames between PFs and between Ethernet ports directly (again, something not supported efficiently by simpler cards, but supported by NFPs).
If "Whenever there is a netdev(netdev of PF/VF, or representor netdev), there is a devlink port instance corresponds to that netdev." rule apply to the above case, as there is one netdev for PF and one netdev for Ethernet port, then we have two devlink port instance too, one for netdev of PF, one for the netdev of Ethernet port, which is different from Mellanox having one netdev for both PF and Ethernet port，hence one devlink port for both PF and Ethernet port.
It seems it is needed to clarify the FLAVOUR_PHYSICAL and FLAVOUR_PCI_PF maybe having different semantic between NFP and Mellanox?
we might need to add another flavour type to indicate the netdev of PF, if FLAVOUR_PHYSICAL indicates netdev of Ethernet port(if that netdev exists) and FLAVOUR_PCI_PF indicates representor netdev of PF, as the comment in definiation of flavour type:
DEVLINK_PORT_FLAVOUR_PHYSICAL, /* Any kind of a port physically * facing the user. */
DEVLINK_PORT_FLAVOUR_PCI_PF, /* Represents eswitch port for * the PCI PF. It is an internal * port that faces the PCI PF. */
.