Re: [PATCH V2 net 00/11] Extending the ena driver to support new features and enhance performance

[Jakub Kicinski <jakub.kicinski@netronome.com>]

On Thu, 6 Jun 2019 21:40:19 +0000, Bshara, Nafea wrote:
> On 6/6/19, 10:16 AM, "Jakub Kicinski" <jakub.kicinski@netronome.com> wrote:
> 
>     Hi Samih!
>     
>     Please don't top post on Linux kernel mailing lists.
>     
>     On Thu, 6 Jun 2019 10:23:40 +0000, Jubran, Samih wrote:
>     > As of today there are no flags exposed by ENA NIC device, however, we
>     > are planning to use them in the near future. We want to provide
>     > customers with extra methods to identify (and differentiate) multiple
>     > network interfaces that can be attached to a single VM. Currently,
>     > customers can identify a specific network interface (ENI) only by MAC
>     > address, and later look up this MAC among other multiple ENIs that
>     > they have. In some cases it might not be convenient. Using these
>     > flags will let us inform the customer about ENI`s specific
>     > properties.  
>     
>     Oh no :(  You're using private _feature_ flags as labels or tags.
>     
>     > It's not finalized, but tentatively it can look like this: 
>     > primary-eni: on /* Differentiate between primary and secondary ENIs */  
>     
>     Did you consider using phys_port_name for this use case?
>     
>     If the intent is to have those interfaces bonded, even better would
>     be to extend the net_failover module or work on user space ABI for VM
>     failover.  That might be a bigger effort, though.

Someone else will address this point?

>     > has-associated-efa: on /* Will specify ENA device has another associated EFA device */  
>     
>     IDK how your ENA/EFA thing works, but sounds like something that should
>     be solved in the device model.
> 
> [NB] ENA is a netDev,  EFA is an ib_dev.   Both share the same physical network
> All previous attempt to make them share at device level leads to a
> lot of complexity at the OS level, with inter-dependencies that are
> pronged to error Not to mention that OS distribution that backport
> from mainline would backport different subset of each driver,  let
> alone they belong to two subtrees with two different maintainers and
> we don’t want to be in a place where we have to coordinate releases
> between two subgroups
>
> As such, we selected a much safer path, that operational, development and deployment of two different drivers (netdev vs ib_dev) much easier but exposing the nic as two different Physical functions/devices
> 
> Only cost we have is that we need this extra propriety to help user identify which two devices are related hence Samih's comments

I think you're arguing with a different point than the one I made :)
Do you think I said they should use the same PCI device?  I haven't.

IIUC you are exposing a "tag" through the feature API on the netdev to
indicate that there is another PCI device present on the system?

What I said is if there is a relation between PCI devices the best
level to expose this relation at is at the device model level.  IOW
have some form on "link" in sysfs, not in a random place on a netdev.

Having said that, it's entirely unclear to me what the user scenario is
here.  You say "which two devices related", yet you only have one bit,
so it can indicate that there is another device, not _which_ device is
related.  Information you can full well get from running lspci 🤷
Do the devices have the same PCI ID/vendor:model?