netdev - RE: [PATCH net-next 2/3] devlink: Consider other controller while building phys_port

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 6 Sep 2020 03:08:45 +0000
From:   Parav Pandit <parav@...dia.com>
To:     Jiri Pirko <jiri@...nulli.us>, Jakub Kicinski <kuba@...nel.org>
CC:     Parav Pandit <parav@...lanox.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "roid@...lanox.com" <roid@...lanox.com>,
        "saeedm@...lanox.com" <saeedm@...lanox.com>,
        Jiri Pirko <jiri@...dia.com>
Subject: RE: [PATCH net-next 2/3] devlink: Consider other controller while
 building phys_port_name



> From: Jiri Pirko <jiri@...nulli.us>
> Sent: Friday, September 4, 2020 2:13 PM
> 
> Thu, Sep 03, 2020 at 09:31:23PM CEST, kuba@...nel.org wrote:
> >On Thu, 3 Sep 2020 07:54:39 +0200 Jiri Pirko wrote:
> >> Wed, Sep 02, 2020 at 05:23:58PM CEST, kuba@...nel.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

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.