Re: [PATCH for-next] RDMA/nldev: Add parent bdf to device information dump

Gal Pressman
2020-11-09T09:03:46Z

On 09/11/2020 7:09, Leon Romanovsky wrote:
> On Sun, Nov 08, 2020 at 07:49:35PM -0400, Jason Gunthorpe wrote:
>> On Sun, Nov 08, 2020 at 03:03:45PM +0200, Gal Pressman wrote:
>>> On 05/11/2020 22:00, Jason Gunthorpe wrote:
>>>> On Tue, Nov 03, 2020 at 05:45:26PM +0200, Gal Pressman wrote:
>>>>> On 03/11/2020 16:22, Jason Gunthorpe wrote:
>>>>>> On Tue, Nov 03, 2020 at 04:11:19PM +0200, Gal Pressman wrote:
>>>>>>> On 03/11/2020 15:57, Leon Romanovsky wrote:
>>>>>>>> On Tue, Nov 03, 2020 at 09:45:22AM -0400, Jason Gunthorpe wrote:
>>>>>>>>> On Tue, Nov 03, 2020 at 03:26:27PM +0200, Gal Pressman wrote:
>>>>>>>>>> Add the ability to query the device's bdf through rdma tool netlink
>>>>>>>>>> command (in addition to the sysfs infra).
>>>>>>>>>>
>>>>>>>>>> In case of virtual devices (rxe/siw), the netdev bdf will be shown.
>>>>>>>>>
>>>>>>>>> Why? What is the use case?
>>>>>>>>
>>>>>>>> Right, and why isn't netdev (RDMA_NLDEV_ATTR_NDEV_NAME) enough?
>>>>>>>
>>>>>>> When taking system topology into consideration you need some way to pair the
>>>>>>> ibdev and bdf, especially when working with multiple devices.
>>>>>>> The netdev name doesn't exist on devices with no netdevs (IB, EFA).
>>>>>>
>>>>>> You are supposed to use sysfs
>>>>>>
>>>>>> /sys/class/infiniband/ibp0s9/device
>>>>>>
>>>>>> Should always be the physical device
>>>>>>
>>>>>>> Why rdma tool? Because it's more intuitive than sysfs.
>>>>>>
>>>>>> But we generally don't put this information into netlink BDF is just
>>>>>> the start, you need all the other topology information to make sense
>>>>>> of it, and all that is in sysfs only already
>>>>>
>>>>> As the commit message says, it's in addition to the device sysfs.
>>>>>
>>>>> Many (if not most) of the existing rdma netlink commands are duplicates of some
>>>>> sysfs entries, but show it in a more "modern" way.
>>>>> I'm not convinced that bdf should be treated differently.
>>>>
>>>> Why did you call it BDF anyhow? it has nothing to do with PCI BDF
>>>> other than it happens to be the PDF for PCI devices. Netdev called
>>>> this bus_info
>>>
>>> Are there non pci devices in the subsystem?
>>
>> Yes, HNS uses non-pci devices
>>
>>> I can rename to a more fitting name, will change to bus_info unless
>>> someone has a better idea.
>>
>> The thing is, is is still useless. You have to consult sysfs to
>> understand what bus it is scoped on to do anything further with
>> it. Can't just assume it is PCI.
>
> Can anyone please remind me why are we doing it?
> What problem do you solve here by adding new nldev attributes?