Re: [PATCH net-next 2/3] devlink: Consider other controller while building phys_port_name

[Jiri Pirko <jiri@resnulli.us>]

Sun, Sep 06, 2020 at 05:08:45AM CEST, parav@nvidia.com wrote:
>
>
>> From: Jiri Pirko <jiri@resnulli.us>
>> Sent: Friday, September 4, 2020 2:13 PM
>> 
>> Thu, Sep 03, 2020 at 09:31:23PM CEST, kuba@kernel.org wrote:
>> >On Thu, 3 Sep 2020 07:54:39 +0200 Jiri Pirko wrote:
>> >> Wed, Sep 02, 2020 at 05:23:58PM CEST, kuba@kernel.org wrote:
>> >> >On Wed, 2 Sep 2020 10:00:11 +0200 Jiri Pirko wrote:
>> >> >>>> I didn't quite get the fact that you want to not show controller ID on the
>> local
>> >> >>>> port, initially.
>> >> >>> Mainly to not_break current users.
>> >> >>
>> >> >> You don't have to take it to the name, unless "external" flag is set.
>> >> >>
>> >> >> But I don't really see the point of showing !external, cause such
>> >> >> controller number would be always 0. Jakub, why do you think it is
>> >> >> needed?
>> >> >
>> >> >It may seem reasonable for a smartNIC where there are only two
>> >> >controllers, and all you really need is that external flag.
>> >> >
>> >> >In a general case when users are trying to figure out the topology
>> >> >not knowing which controller they are sitting at looks like a
>> >> >serious limitation.
>> >>
>> >> I think we misunderstood each other. I never proposed just "external"
>> >> flag.
>> >
>> >Sorry, I was just saying that assuming a single host SmartNIC the
>> >controller ID is not necessary at all. You never suggested that, I did.
>> >Looks like I just confused everyone with that comment :(
>> >
>> >Different controller ID for different PFs but the same PCIe link would
>> >be very wrong. So please clarify - if I have a 2 port smartNIC, with on
>> >PCIe link to the host, and the embedded controller - what would I see?
>> 
>> Parav?
>>
>One controller id for both such PFs.
>I liked the idea of putting controller number for all the ports but not embedded for local ports.
>
>> 
>> >
>> >> What I propose is either:
>> >> 1) ecnum attribute absent for local
>> >>    ecnum attribute absent set to 0 for external controller X
>> >>    ecnum attribute absent set to 1 for external controller Y
>> >>    ...
>> >>
>> >> or:
>> >> 2) ecnum attribute absent for local, external flag set to false
>> >>    ecnum attribute absent set to 0 for external controller X, external flag set
>> to true
>> >>    ecnum attribute absent set to 1 for external controller Y,
>> >> external flag set to true
>> >
>> >I'm saying that I do want to see the the controller ID for all ports.
>> >
>> >So:
>> >
>> >3) local:   { "controller ID": x }
>> >   remote1: { "controller ID": y, "external": true }
>> >   remote1: { "controller ID": z, "external": true }
>> >
>> >We don't have to put the controller ID in the name for local ports, but
>> >the attribute should be reported. AFAIU name was your main concern, no?
>> 
>> Okay. Sounds fine. Let's put the controller number there for all ports.
>> ctrlnum X external true
>> ctrlnum Y external false
>> 
>> if (!external)
>> 	ignore the ctrlnum when generating the name
>> 
>
>Putting little more realistic example for Jakub's and your suggestion below.
>
>Below is the output for 3 controllers. ( 2 external + 1 local )
>Each external controller consist of 2 PCI PFs for a external host via single PCIe cable.
>Each local controller consist of 1 PCI PF.
>
>$ devlink port show
>pci/0000:00:08.0/0: type eth netdev enp0s8f0_pf0 flavour pcipf pfnum 0 cnum 0 external false
>pci/0000:00:08.0/1: type eth netdev enp0s8f0_c1pf0 flavour pcipf pfnum 0 cnum 1 external true
>pci/0000:00:08.1/1: type eth netdev enp0s8f1_c1pf1 flavour pcipf pfnum 1 cnum 1 external true

I see cnum 0 and cnum 1, yet you talk about 3 controllers. What did I
miss?


>
>Looks ok?
>
>> 
>> >
>> >> >Example - multi-host system and you want to know which controller
>> >> >you are to run power cycle from the BMC side.
>> >> >
>> >> >We won't be able to change that because it'd change the names for you.