netdev - RE: [PATCH net-next v2 1/8] devlink: Introduce PCI SF 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-next>] [day] [month] [year] [list]

Message-ID: <BY5PR12MB4322441DBA23EB8F5B8D3B90DC3F0@BY5PR12MB4322.namprd12.prod.outlook.com>
Date:   Fri, 18 Sep 2020 03:54:39 +0000
From:   Parav Pandit <parav@...dia.com>
To:     Jacob Keller <jacob.e.keller@...el.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     Jiri Pirko <jiri@...dia.com>
Subject: RE: [PATCH net-next v2 1/8] devlink: Introduce PCI SF port flavour
 and port attribute



> From: Jacob Keller <jacob.e.keller@...el.com>
> Sent: Friday, September 18, 2020 12:00 AM
> 
> 
> On 9/17/2020 10:20 AM, Parav Pandit wrote:
> > A PCI sub-function (SF) represents a portion of the device similar to
> > PCI VF.
> >
> > In an eswitch, PCI SF may have port which is normally represented
> > using a representor netdevice.
> > To have better visibility of eswitch port, its association with SF,
> > and its representor netdevice, introduce a PCI SF port flavour.
> >
> > When devlink port flavour is PCI SF, fill up PCI SF attributes of the
> > port.
> >
> > Extend port name creation using PCI PF and SF number scheme on best
> > effort basis, so that vendor drivers can skip defining their own
> > scheme.
> 
> What does this mean? What's the scheme used? 
>
Scheme used is equivalent as what is used for PCI VF ports. pfNvfM.
It is pfNsfM.
Below example shows the representor netdevice name as 'eni10npf0sf44' built by systemd/udev using phys_port_name.

> Do drivers still have the option to make their own scheme? If so, why?
Today we have two types of drivers (mlx5_core, netdevsim) which uses devlink core which creates the name.
Or other drivers (bnxt, nfp) which doesn't yet migrated to use devlink infra for PCI PF, VF ports.
Such drivers are phys_port_name and other ndos.
It is not the role of this patch to block those drivers, but any new implementation doesn't need to hand code switch_id and phys_port_name related ndos for SF.
For example, bnxt_vf_rep_get_phys_port_name().

> It's not obvious to me in this patch where the numbering scheme comes from. It
> looks like it's still up to the caller to set the numbers.
>
Naming scheme for PCI PF and PCI VF port flavours already exist.
Scheme is equivalent for PCI SF flavour.

I thought example is good enough to show that, but I will update commit message to describe this scheme to make it clear. pfNsfM.
 
> >
> > An example view of a PCI SF port.
> >
> > $ devlink port show netdevsim/netdevsim10/2
> > netdevsim/netdevsim10/2: type eth netdev eni10npf0sf44 flavour pcisf
> controller 0 pfnum 0 sfnum 44 external false splittable false
> >   function:
> >     hw_addr 00:00:00:00:00:00
> >
> > devlink port show netdevsim/netdevsim10/2 -jp {
> >     "port": {
> >         "netdevsim/netdevsim10/2": {
> >             "type": "eth",
> >             "netdev": "eni10npf0sf44",
> >             "flavour": "pcisf",
> >             "controller": 0,
> >             "pfnum": 0,
> >             "sfnum": 44,
> >             "external": false,
> >             "splittable": false,
> >             "function": {
> >                 "hw_addr": "00:00:00:00:00:00"
> >             }
> >         }
> >     }
> > }
> >
> > Signed-off-by: Parav Pandit <parav@...dia.com>
> > Reviewed-by: Jiri Pirko <jiri@...dia.com>
> > ---
> >  include/net/devlink.h        | 17 +++++++++++++++++
> >  include/uapi/linux/devlink.h |  7 +++++++
> >  net/core/devlink.c           | 37 ++++++++++++++++++++++++++++++++++++
> >  3 files changed, 61 insertions(+)
> >


> >  static int __devlink_port_phys_port_name_get(struct devlink_port
> *devlink_port,
> >  					     char *name, size_t len)
> >  {
> > @@ -7855,6 +7889,9 @@ static int
> __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
> >  		n = snprintf(name, len, "pf%uvf%u",
> >  			     attrs->pci_vf.pf, attrs->pci_vf.vf);
> >  		break;
> > +	case DEVLINK_PORT_FLAVOUR_PCI_SF:
> > +		n = snprintf(name, len, "pf%usf%u", attrs->pci_sf.pf, attrs-
> >pci_sf.sf);
> > +		break;
> >  	}
> >
This is where the naming scheme is done, like pcipf and pcivf port flavours.

> >  	if (n >= len)
> >