netdev - RE: [PATCH net-next 1/3] devlink: Introduce PCI PF port flavour and port attribute

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]

Message-ID: <AM0PR05MB4866675A8CB5BDB2890E14BBD1FB0@AM0PR05MB4866.eurprd05.prod.outlook.com>
Date:   Wed, 3 Jul 2019 13:49:51 +0000
From:   Parav Pandit <parav@...lanox.com>
To:     Jiri Pirko <jiri@...nulli.us>
CC:     Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Jiri Pirko <jiri@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Saeed Mahameed <saeedm@...lanox.com>,
        "vivien.didelot@...il.com" <vivien.didelot@...il.com>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>
Subject: RE: [PATCH net-next 1/3] devlink: Introduce PCI PF port flavour and
 port attribute



> -----Original Message-----
> From: Jiri Pirko <jiri@...nulli.us>
> Sent: Wednesday, July 3, 2019 4:07 PM
> To: Parav Pandit <parav@...lanox.com>
> Cc: Jakub Kicinski <jakub.kicinski@...ronome.com>; Jiri Pirko
> <jiri@...lanox.com>; netdev@...r.kernel.org; Saeed Mahameed
> <saeedm@...lanox.com>; vivien.didelot@...il.com; andrew@...n.ch;
> f.fainelli@...il.com
> Subject: Re: [PATCH net-next 1/3] devlink: Introduce PCI PF port flavour and
> port attribute
> 
> Wed, Jul 03, 2019 at 06:46:13AM CEST, parav@...lanox.com wrote:
> >
> >
> >> -----Original Message-----
> >> From: Jakub Kicinski <jakub.kicinski@...ronome.com>
> >> Sent: Wednesday, July 3, 2019 7:46 AM
> >> To: Parav Pandit <parav@...lanox.com>
> >> Cc: Jiri Pirko <jiri@...lanox.com>; netdev@...r.kernel.org; Saeed
> >> Mahameed <saeedm@...lanox.com>
> >> Subject: Re: [PATCH net-next 1/3] devlink: Introduce PCI PF port
> >> flavour and port attribute
> >>
> >> On Wed, 3 Jul 2019 02:08:39 +0000, Parav Pandit wrote:
> >> > > If you want to expose some device specific eswitch port ID please
> >> > > add a new attribute for that.
> >> > > The fact that that ID may match port_number for your device today
> >> > > is coincidental.  port_number, and split attributes should not be
> >> > > exposed for PCI ports.
> >> >
> >> > So your concern is non mellanox hw has eswitch but there may not be
> >> > a unique handle to identify a eswitch port?
> >>
> >> That's not a concern, no.  Like any debug attribute it should be optional.
> >>
> >> > Or that handle may be wider than 32-bit?
> >>
> >> 64 bit would probably be better, yes, although that wasn't my initial
> >> concern.
> >>
> >Why 32-bit is not enough?
> >
> >> > And instead of treating port_number as handle, there should be
> >> > different attribute, is that the ask?
> >>
> >> Yes, the ask, as always, is to not abuse existing attributes to carry
> >> tangentially related information.
> >
> >Why it is tangential?
> >Devlink_port has got a port_number. Depending on flavour this port_number
> represents a port.
> >If it is floavour=PHYSICAL, its physical port number.
> >If it is eswitch pf/vf ports, it represents eswitch port.
> >
> >Why you see it only as physical_port_number?
> 
> The original intention was like that. See the desc of
> devlink_port_attrs_set():
> 
>  *      @port_number: number of the port that is facing user, for example
>  *                    the front panel port number
> 
> For vf/pf representors, this is not applicable and should be indeed avoided.
> 
Physical port number is not applicable but this is useful information that completes the eswitch picture.
Because eswitch has this termination end point anyway.
Instead of inventing some new vendor specific field, I see value in using existing port_number field.
Will wait for others inputs.

> However, we expose it for DEVLINK_PORT_FLAVOUR_CPU and
> DEVLINK_PORT_FLAVOUR_DSA. Not sure if it makes sense there either.
> Ccing Florian, Andrew and Vivien.
> What do you guys think?
> 
> Perhaps we should have:
> 	if (attrs->flavour == DEVLINK_PORT_FLAVOUR_PHYSICAL &&
> 	    nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER, attrs-
> >port_number))
>                 return -EMSGSIZE;
> in devlink_nl_port_attrs_put()