> From: Jacob Keller <jacob.e.kel...@intel.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); }
> > +
