netdev - RE: [PATCH net-next v2 2/8] devlink: Support add and delete devlink 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-next>] [day] [month] [year] [list]

Message-ID: <BY5PR12MB43227952D5E596D50CD0D74DDC3F0@BY5PR12MB4322.namprd12.prod.outlook.com>
Date:   Fri, 18 Sep 2020 04:25:09 +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 2/8] devlink: Support add and delete devlink
 port

> From: Jacob Keller <jacob.e.keller@...el.com>
> Sent: Friday, September 18, 2020 12:13 AM
> 
> 
> On 9/17/2020 10:20 AM, Parav Pandit wrote:
> > Extended devlink interface for the user to add and delete port.
> > Extend devlink to connect user requests to driver to add/delete such
> > port in the device.
> >
> > When driver routines are invoked, devlink instance lock is not held.
> > This enables driver to perform several devlink objects registration,
> > unregistration such as (port, health reporter, resource etc) by using
> > exising devlink APIs.
> > This also helps to uniformly used the code for port registration
> > during driver unload and during port deletion initiated by user.
> >
> 
> Ok. Seems like a good goal to be able to share code uniformly between driver
> load and new port creation.
>
Yes.
 
> > +static int devlink_nl_cmd_port_new_doit(struct sk_buff *skb, struct
> > +genl_info *info) {
> > +	struct netlink_ext_ack *extack = info->extack;
> > +	struct devlink_port_new_attrs new_attrs = {};
> > +	struct devlink *devlink = info->user_ptr[0];
> > +
> > +	if (!info->attrs[DEVLINK_ATTR_PORT_FLAVOUR] ||
> > +	    !info->attrs[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]) {
> > +		NL_SET_ERR_MSG_MOD(extack, "Port flavour or PCI PF are not
> specified");
> > +		return -EINVAL;
> > +	}
> > +	new_attrs.flavour = nla_get_u16(info-
> >attrs[DEVLINK_ATTR_PORT_FLAVOUR]);
> > +	new_attrs.pfnum =
> > +nla_get_u16(info->attrs[DEVLINK_ATTR_PORT_PCI_PF_NUMBER]);
> > +
> 
> Presuming that the device supports it, this could be used to allow creating other
> types of ports bsides subfunctions?
>
This series is creating PCI PF and subfunction ports.
Jiri's RFC [1] explained a possibility for VF representors to follow the similar scheme if device supports it.

I am not sure creating other port flavours are useful enough such as CPU, PHYSICAL etc.
I do not have enough knowledge about use case for creating CPU ports, if at all it exists.
Usually physical ports are linked to a card hardware on how many physical ports present on circuit.
So I find it odd if a device support physical port creation, but again its my limited view at the moment.
 
> > +	if (info->attrs[DEVLINK_ATTR_PORT_INDEX]) {
> > +		new_attrs.port_index = nla_get_u32(info-
> >attrs[DEVLINK_ATTR_PORT_INDEX]);
> > +		new_attrs.port_index_valid = true;
> > +	}
> 
> So if the userspace doesn't provide a port index, drivers are responsible for
> choosing one? Same for the other attributes I suppose?
Yes.

> 
> > +	if (info->attrs[DEVLINK_ATTR_PORT_CONTROLLER_NUMBER]) {
> > +		new_attrs.controller =
> > +			nla_get_u16(info-
> >attrs[DEVLINK_ATTR_PORT_CONTROLLER_NUMBER]);
> > +		new_attrs.controller_valid = true;
> > +	}
> > +	if (info->attrs[DEVLINK_ATTR_PORT_PCI_SF_NUMBER]) {
> > +		new_attrs.sfnum = nla_get_u32(info-
> >attrs[DEVLINK_ATTR_PORT_PCI_SF_NUMBER]);
> > +		new_attrs.sfnum_valid = true;
> > +	}
> > +
> > +	if (!devlink->ops->port_new)
> > +		return -EOPNOTSUPP;
> > +
> > +	return devlink->ops->port_new(devlink, &new_attrs, extack); }
> > +