Parav, please wrap your responses to at most 80 characters. This is hard to read.
On Mon, 4 Mar 2019 04:41:01 +0000, Parav Pandit wrote: > > -----Original Message----- > > From: Jakub Kicinski <jakub.kicin...@netronome.com> > > Sent: Friday, March 1, 2019 2:04 PM > > To: Parav Pandit <pa...@mellanox.com>; Or Gerlitz <gerlitz...@gmail.com> > > Cc: netdev@vger.kernel.org; linux-ker...@vger.kernel.org; > > michal.l...@markovi.net; da...@davemloft.net; > > gre...@linuxfoundation.org; Jiri Pirko <j...@mellanox.com> > > Subject: Re: [RFC net-next 0/8] Introducing subdev bus and devlink extension > > > > On Thu, 28 Feb 2019 23:37:44 -0600, Parav Pandit wrote: > > > Requirements for above use cases: > > > -------------------------------- > > > 1. We need a generic user interface & core APIs to create sub devices > > > from a parent pci device but should be generic enough for other parent > > > devices 2. Interface should be vendor agnostic 3. User should be able > > > to set device params at creation time 4. In future if needed, tool > > > should be able to create passthrough device to map to a virtual > > > machine > > > > Like a mediated device? > > Yes. > > > https://www.kernel.org/doc/Documentation/vfio-mediated-device.txt > > https://www.dpdk.org/wp-content/uploads/sites/35/2018/06/Mediated- > > Devices-Better-Userland-IO.pdf > > > > Other than pass-through it is entirely unclear to me why you'd need a bus. > > (Or should I say VM pass through or DPDK?) Could you clarify why the need > > for a bus? > > > A bus follow standard linux kernel device driver model to attach a > driver to specific device. Platform device with my limited > understanding looks a hack/abuse of it based on documentation [1], > but it can possibly be an alternative to bus if it looks fine to Greg > and others. I grok from this text that the main advantage you see is the ability to choose a driver for the subdevice. > > My thinking is that we should allow spawning subports in devlink > > and if user specifies "passthrough" the device spawned would be an > > mdev. > > devlink device is much more comprehensive way to create sub-devices > than sub-ports for at least below reasons. > > 1. devlink device already defines device->port relation which enables > to create multiport device. I presume that by devlink device you mean devlink instance? Yes, this part I'm following. > subport breaks that. Breaks what? The ability to create a devlink instance with multiple ports? > 2. With bus model, it enables us to load driver of same vendor or > generic one such a vfio in future. Yes, sorry, I'm not an expert on mdevs, but isn't that the goal of those? Could you go into more detail why not just use mdevs? > 3. Devices live on the bus, mapping a subport to 'struct device' is > not intuitive. Are you saying that the main devlink instance would not have any port information for the subdevices? Devices live on a bus. Software constructs - depend on how one wants to model them - don't have to. > 4. sub-device allows to use existing devlink port, > registers, health infrastructure to sub devices, which otherwise need > to be duplicated for ports. Health stuff is not tied to a port, I'm not following you. You can create a reporter per port, per ACL rule or per SB or per whatever your heart desires.. > 5. Even though current devlink devices are networking devices, there > is nothing restricts it to be that way. So subport is a restricted > view. > 6. devlink device already covers > port sub-object, hence creating devlink device is desired. > > > > 5. A device can have multiple ports > > > > What does this mean, in practice? You want to spawn a subdev which > > can access both ports? That'd be for RDMA use cases, more than > > Ethernet, right? (Just clarifying :)) > > > Yep, you got it right. :-) > > > > So how is it done? > > > ------------------ > > > (a) user in control > > > To address above requirements, a generic tool iproute2/devlink is > > > extended for sub device's life cycle. > > > However a devlink tool and its kernel counter part is not > > > sufficient to create protocol agnostic devices on a existing PCI > > > bus. > > > > "Protocol agnostic"?... What does that mean? > > > Devlink works on bus,device model. It doesn't matter what class of > device is. For example, for pci class can be anything. So newly > created sub-devices are not limited to netdev/rdma devices. Its > agnostic to protocol. More importantly, we don't want to create these > sub-devices who bus type is 'pci'. Because as described below, PCI > has its addressing scheme and pci bus must not have mix-n match > devices. > > So probably better wording should be, > 'a devlink tool and its kernel counterpart is not sufficient to > create sub-devices of same class as that of PCI device. Let me clarify - for networking devices the partition will most likely end up as a subport, but its not a requirement that each partition must be a subport.. The question was about the necessity to invent a new bus, and have every resource have a struct device.. > > > (b) subdev bus > > > A given bus defines well defined addressing scheme. Creating sub > > > devices on existing PCI bus with a different naming scheme is > > > just weird. So, creating well named devices on appropriate bus is > > > desired. > > > > What's that address scheme you're referring to, you seem to assign > > IDs in sequence? > > > Yes. a device on subdev bus follows standard linux driver model based > id assignment scheme = u32. And devices are well named as 'subdev0'. > Prefix + id as the default scheme of core driver model. I thought "well defined addressing scheme" means I can address subdevice X of device Y with your scheme. I can't, it's just an global ID. Thanks for clarifying. > > > Given that, these are user created devices for a given hardware > > > and in absence of a central entity like PCISIG to assign vendor > > > and device ids, A unique vendor and device id are maintained as > > > enum in include/linux/subdev_ids.h. > > > > Why do we need IDs? The sysfs hierarchy isn't sufficient? > > > Do we need a driver to match on those again? Is it going to be a > > different driver? > IDs are used to match driver against the created device. > It can be same or different driver. > Even in same driver case, it provides a clear code separation for > creating sub-devices and their respective one or more protocol > devices (netdev, rep-netdev, rdma ..) > > > > subdev bus device names follow default device naming scheme of > > > Linux kernel. It is done as 'subdev<instance_id>' such as, > > > subdev0, subdev3. > > > > > > System example view: > > > -------------------- > > > > > > $ devlink dev show > > > pci/0000:05:00.0 > > > > > > $ devlink dev add pci/0000:05:00.0 > > > > That does not look great. > > > Yes, It must return bus+device attributes in user output too > Code in existing patchset returns it, it is not shown here. > I will fix the cover-letter. > > > Also you have to return the id of the spawned device, otherwise > > this is very racy. > > > Yes, that is correct. It must return an devlink device id = > {bus+device} attr. I will update the example in v2. > > > > $ devlink dev show > > > pci/0000:05:00.0 > > > subdev/subdev0 > > > Please don't spawn devlink instances. Devlink instance is supposed > > to represent an ASIC. If we start spawning them willy nilly for > > whatever software construct we want to model the clarity of the > > ontology will suffer a lot. > Devlink devices not restricted to ASIC even though today it is > representing ASIC for one vendor. Today for one ASIC, it already > presents multiple devlink devices (128 or more) for PF and VFs, two > PFs on same ASIC etc. VF is just a sub-device which is well defined > by PCISIG, whereas sub-device is not. Sub-device do consume actual > ASIC resources (just like PFs and VFs), Hence point-(6) of > cover-letter indicate that the devlink capability to tell how many > such sub-devices can be created. > > In above example, they are created for a given bus-device following > existing devlink construct. > > > Please see the discussion on my recent patchset. I think Jiri CCed > > you. > I will review the discussion in short while after this reply, and > provide comments. > > > > Alternatives considered: > > > ------------------------ > > > Will discuss separately if needed to keep this RFC short. > > > > Please do discuss. > > > (a) subports instead of subdevices. > We dropped this option because its two restrictive; I explained above > the benefits of devlink device. > > (b) extending iproute2/ip link and iproute2/rdma tools to creating > sub-devices. But that is too limiting which doesn't provide all the > features we get using devlink. It also doesn't address the > passthrough needs and its just ugly to create and manage PCI level > devices using high level tools like 'ip' and 'rdma'. > > (c) creating platform device and platform driver instead of subdev bus > Our understanding is that - platform device for this purpose would be > an abuse/misuse, but our view is limited based on kernel > documentation in [2]. [1] says "platform devices typically appear as > autonomous entities" Sub-devices are well managed, created, > configurable by user. Most things of [1] -> "Platform devices" > section do not match with subdev. > > Greg suggested to use mfd framework (wrapper to platform), which also > needs extension. mfd_remove_devices() removes all the devices, while > here based on user request, we want to add/remove individual device. > Will wait if he is ok with subdev bus or he prefers to extend the > platform documentation and mfd for removing individual devices. > > (d) drivers/visorbus > This bus is limited to UUID/GUID based naming scheme and very > specific to s-Par standard and vendor. Additionally its guest drivers > are living in staging for more than year. So it doesn't appear the > right direction. > > (e) creating subdev as child objects of devlink device (such as port, > registers, health, etc). In this mode, a given devlink device has > multiport child device which is anchored using 'struct device' and > life cycled through devlink. Only difference with current proposal is > it doesn't follow standard driver model to bind to other driver. It > also doesn't show in unified way using devlink dev show. > > So instead of these alternatives, devlink device that matches PF, VF, > sub-device, + subdev bus seems better design. This follows all > standard constructs of 1. Devlink, 2. Linux driver model. It is not > limited to ports and generic enough for networking and not networking > devices. > > The things key thing for me on the netdev side is what is the > > forwarding model to this new entity. Is this basically VMDQ? > > Should we just go ahead and mandate "switchdev mode" here? > > > It will follow the switchdev mode, but it not limited to it. > Switchdev mode is for the eswitch functionality. There isn't a need > to combine this. rdma Infiniband will be able to use this without > switchdev mode. It's the devlink instance that's in "switchdev mode", regardless of type of any of its ports. > > Thanks for working on a common architecture and suffering through > > people's reviews rather than adding a debugfs interface that does > > this like a different vendor did :) > Oh yes, lets not do debugfs. > Thanks a lot Jakub for the review. > This common architecture should be able to address such common needs. > Please let me know if this needs more refinement, if I missed > something. > > [1] https://www.kernel.org/doc/Documentation/driver-model/platform.txt >
