Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Jiri Pirko <jiri@resnulli.us>
Cc: Parav Pandit <parav@nvidia.com>,
	Parav Pandit <parav@mellanox.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"roid@mellanox.com" <roid@mellanox.com>,
	"saeedm@mellanox.com" <saeedm@mellanox.com>,
	Jiri Pirko <jiri@nvidia.com>
Subject: Re: [PATCH net-next 2/3] devlink: Consider other controller while building phys_port_name
Date: Thu, 3 Sep 2020 12:31:23 -0700	[thread overview]
Message-ID: <20200903123123.7e6025ec@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <20200903055439.GA2997@nanopsycho.orion>

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?

> 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?

> >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.  

  reply	other threads:[~2020-09-03 19:31 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-25 13:58 [PATCH net-next 0/3] devlink show controller info Parav Pandit
2020-08-25 13:58 ` [PATCH net-next 1/3] devlink: Add comment block for missing port attributes Parav Pandit
2020-08-25 13:58 ` [PATCH net-next 2/3] devlink: Consider other controller while building phys_port_name Parav Pandit
2020-08-26  0:32   ` Jakub Kicinski
2020-08-26  4:27     ` Parav Pandit
2020-08-26 20:07       ` Jakub Kicinski
2020-08-27  4:31         ` Parav Pandit
2020-08-27 18:32           ` Jakub Kicinski
2020-08-27 20:15             ` Parav Pandit
2020-08-27 21:42               ` Jakub Kicinski
2020-08-28  4:27                 ` Parav Pandit
2020-08-28  5:08                   ` Parav Pandit
2020-08-28 16:43                   ` Jakub Kicinski
2020-08-29  3:43                     ` Parav Pandit
2020-09-01  8:19                       ` Jiri Pirko
2020-09-01  8:53                         ` Parav Pandit
2020-09-01  9:17                           ` Jiri Pirko
2020-09-01 21:28                             ` Jakub Kicinski
2020-09-02  4:26                               ` Parav Pandit
2020-09-02  4:44                                 ` Parav Pandit
2020-09-02  8:00                                 ` Jiri Pirko
2020-09-02 15:23                                   ` Jakub Kicinski
2020-09-02 16:18                                     ` Parav Pandit
2020-09-02 20:10                                       ` Parav Pandit
2020-09-03  5:54                                     ` Jiri Pirko
2020-09-03 19:31                                       ` Jakub Kicinski [this message]
2020-09-04  8:43                                         ` Jiri Pirko
2020-09-06  3:08                                           ` Parav Pandit
2020-09-06 16:46                                             ` Jakub Kicinski
2020-09-07  7:21                                             ` Jiri Pirko
2020-09-07 16:18                                               ` Jakub Kicinski
2020-08-25 13:58 ` [PATCH net-next 3/3] net/mlx5: E-switch, Set controller attribute for PCI PF and VF ports Parav Pandit
2020-09-08 14:42 ` [PATCH net-next v2 0/6] devlink show controller number Parav Pandit
2020-09-08 14:42   ` [PATCH net-next v2 1/6] net/mlx5: E-switch, Read controller number from device Parav Pandit
2020-09-08 14:42   ` [PATCH net-next v2 2/6] devlink: Add comment block for missing port attributes Parav Pandit
2020-09-08 14:42   ` [PATCH net-next v2 3/6] devlink: Move structure comments outside of structure Parav Pandit
2020-09-08 14:42   ` [PATCH net-next v2 4/6] devlink: Introduce external controller flag Parav Pandit
2020-09-08 14:42   ` [PATCH net-next v2 5/6] devlink: Introduce controller number Parav Pandit
2020-09-08 18:50     ` Jakub Kicinski
2020-09-09  3:06       ` Parav Pandit
2020-09-08 14:42   ` [PATCH net-next v2 6/6] devlink: Use controller while building phys_port_name Parav Pandit
2020-09-09  4:50 ` [PATCH net-next v3 0/6] devlink show controller number Parav Pandit
2020-09-09  4:50   ` [PATCH net-next v3 1/6] net/mlx5: E-switch, Read controller number from device Parav Pandit
2020-09-09  4:50   ` [PATCH net-next v3 2/6] devlink: Add comment block for missing port attributes Parav Pandit
2020-09-09  4:50   ` [PATCH net-next v3 3/6] devlink: Move structure comments outside of structure Parav Pandit
2020-09-09  4:50   ` [PATCH net-next v3 4/6] devlink: Introduce external controller flag Parav Pandit
2020-09-09  4:50   ` [PATCH net-next v3 5/6] devlink: Introduce controller number Parav Pandit
2020-09-09  4:50   ` [PATCH net-next v3 6/6] devlink: Use controller while building phys_port_name Parav Pandit
2020-09-10 15:02     ` David Ahern
2020-09-09 15:34   ` [PATCH net-next v3 0/6] devlink show controller number Jakub Kicinski
2020-09-09 21:20     ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200903123123.7e6025ec@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
    --to=kuba@kernel.org \
    --cc=davem@davemloft.net \
    --cc=jiri@nvidia.com \
    --cc=jiri@resnulli.us \
    --cc=netdev@vger.kernel.org \
    --cc=parav@mellanox.com \
    --cc=parav@nvidia.com \
    --cc=roid@mellanox.com \
    --cc=saeedm@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).