| Message ID | 20200917172020.26484-1-parav@nvidia.com |
|---|---|
| Headers | show |
| Series | devlink: Add SF add/delete devlink ops | expand |
On Thu, 17 Sep 2020 20:20:12 +0300 Parav Pandit wrote: > Hi Dave, Jakub, > > Similar to PCI VF, PCI SF represents portion of the device. > PCI SF is represented using a new devlink port flavour. > > This short series implements small part of the RFC described in detail at [1] and [2]. > > It extends > (a) devlink core to expose new devlink port flavour 'pcisf'. > (b) Expose new user interface to add/delete devlink port. > (c) Extends netdevsim driver to simulate PCI PF and SF ports > (d) Add port function state attribute Is this an RFC? It doesn't add any in-tree users.
> From: Jakub Kicinski <kuba@kernel.org> > Sent: Friday, September 18, 2020 10:22 PM > > On Thu, 17 Sep 2020 20:20:12 +0300 Parav Pandit wrote: > > Hi Dave, Jakub, > > > > Similar to PCI VF, PCI SF represents portion of the device. > > PCI SF is represented using a new devlink port flavour. > > > > This short series implements small part of the RFC described in detail at [1] > and [2]. > > > > It extends > > (a) devlink core to expose new devlink port flavour 'pcisf'. > > (b) Expose new user interface to add/delete devlink port. > > (c) Extends netdevsim driver to simulate PCI PF and SF ports > > (d) Add port function state attribute > > Is this an RFC? It doesn't add any in-tree users. It is not an RFC. devlink + mlx5 + netdevsim is crossing 25+ patches on eswitch side. So splitting it to logical piece as devlink + netdevsim. After which mlx5 eswitch side come close to 15 + 4 patches which can run as two separate patchset. What do you suggest?
On Fri, 18 Sep 2020 17:08:15 +0000 Parav Pandit wrote: > > From: Jakub Kicinski <kuba@kernel.org> > > Sent: Friday, September 18, 2020 10:22 PM > > > > On Thu, 17 Sep 2020 20:20:12 +0300 Parav Pandit wrote: > > > Hi Dave, Jakub, > > > > > > Similar to PCI VF, PCI SF represents portion of the device. > > > PCI SF is represented using a new devlink port flavour. > > > > > > This short series implements small part of the RFC described in detail at [1] > > and [2]. > > > > > > It extends > > > (a) devlink core to expose new devlink port flavour 'pcisf'. > > > (b) Expose new user interface to add/delete devlink port. > > > (c) Extends netdevsim driver to simulate PCI PF and SF ports > > > (d) Add port function state attribute > > > > Is this an RFC? It doesn't add any in-tree users. > It is not an RFC. > devlink + mlx5 + netdevsim is crossing 25+ patches on eswitch side. > So splitting it to logical piece as devlink + netdevsim. > After which mlx5 eswitch side come close to 15 + 4 patches which can > run as two separate patchset. > > What do you suggest? Start with real patches, not netdevsim.
> From: Jakub Kicinski <kuba@kernel.org> > Sent: Friday, September 18, 2020 11:07 PM > > On Fri, 18 Sep 2020 17:08:15 +0000 Parav Pandit wrote: > > > From: Jakub Kicinski <kuba@kernel.org> > > > Sent: Friday, September 18, 2020 10:22 PM > > > > > > On Thu, 17 Sep 2020 20:20:12 +0300 Parav Pandit wrote: > > > > Hi Dave, Jakub, > > > > > > > > Similar to PCI VF, PCI SF represents portion of the device. > > > > PCI SF is represented using a new devlink port flavour. > > > > > > > > This short series implements small part of the RFC described in > > > > detail at [1] > > > and [2]. > > > > > > > > It extends > > > > (a) devlink core to expose new devlink port flavour 'pcisf'. > > > > (b) Expose new user interface to add/delete devlink port. > > > > (c) Extends netdevsim driver to simulate PCI PF and SF ports > > > > (d) Add port function state attribute > > > > > > Is this an RFC? It doesn't add any in-tree users. > > It is not an RFC. > > devlink + mlx5 + netdevsim is crossing 25+ patches on eswitch side. > > So splitting it to logical piece as devlink + netdevsim. > > After which mlx5 eswitch side come close to 15 + 4 patches which can > > run as two separate patchset. > > > > What do you suggest? > > Start with real patches, not netdevsim. Hmm. Shall I split the series below, would that be ok? First patchset, (a) devlink piece to add/delete port (b) mlx5 counter part Second patchset, (a) devlink piece to set the state (b) mlx5 counter part Follow on patchset to create/delete sf's netdev on virtbus in mlx5 + devlink plumbing. Netdevsim after that.
On Fri, 18 Sep 2020 17:47:24 +0000 Parav Pandit wrote: > > > What do you suggest? > > > > Start with real patches, not netdevsim. > > Hmm. Shall I split the series below, would that be ok? > > First patchset, > (a) devlink piece to add/delete port > (b) mlx5 counter part > > Second patchset, > (a) devlink piece to set the state > (b) mlx5 counter part > > Follow on patchset to create/delete sf's netdev on virtbus in mlx5 + devlink plumbing. > Netdevsim after that. I'd start from the virtbus part so we can see what's being created.
> From: Jakub Kicinski <kuba@kernel.org> > Sent: Friday, September 18, 2020 11:58 PM > > On Fri, 18 Sep 2020 17:47:24 +0000 Parav Pandit wrote: > > > > What do you suggest? > > > > > > Start with real patches, not netdevsim. > > > > Hmm. Shall I split the series below, would that be ok? > > > > First patchset, > > (a) devlink piece to add/delete port > > (b) mlx5 counter part > > > > Second patchset, > > (a) devlink piece to set the state > > (b) mlx5 counter part > > > > Follow on patchset to create/delete sf's netdev on virtbus in mlx5 + devlink > plumbing. > > Netdevsim after that. > > I'd start from the virtbus part so we can see what's being created. How do you reach there without a user interface?
On Fri, 18 Sep 2020 20:09:24 +0000 Parav Pandit wrote: > > From: Jakub Kicinski <kuba@kernel.org> > > Sent: Friday, September 18, 2020 11:58 PM > > > > On Fri, 18 Sep 2020 17:47:24 +0000 Parav Pandit wrote: > > > > > What do you suggest? > > > > > > > > Start with real patches, not netdevsim. > > > > > > Hmm. Shall I split the series below, would that be ok? > > > > > > First patchset, > > > (a) devlink piece to add/delete port > > > (b) mlx5 counter part > > > > > > Second patchset, > > > (a) devlink piece to set the state > > > (b) mlx5 counter part > > > > > > Follow on patchset to create/delete sf's netdev on virtbus in mlx5 + devlink > > plumbing. > > > Netdevsim after that. > > > > I'd start from the virtbus part so we can see what's being created. > > How do you reach there without a user interface? Well.. why do you have a user interface which doesn't cause anything to happen (devices won't get created)? You're splitting the submission, it's obvious the implementation won't be complete after the first one. My expectation is that your implementation of the devlink commands will just hand them off to FW, so there won't be anything interesting there to review. Start with the hard / risky parts - I consider the virtbus to be that, since the conversation there includes multiple vendors, use cases and subsystems.
> From: Jakub Kicinski <kuba@kernel.org> > Sent: Tuesday, September 22, 2020 3:32 AM > > On Fri, 18 Sep 2020 20:09:24 +0000 Parav Pandit wrote: > > > From: Jakub Kicinski <kuba@kernel.org> > > > Sent: Friday, September 18, 2020 11:58 PM > > > > > > On Fri, 18 Sep 2020 17:47:24 +0000 Parav Pandit wrote: > > > > > > What do you suggest? > > > > > > > > > > Start with real patches, not netdevsim. > > > > > > > > Hmm. Shall I split the series below, would that be ok? > > > > > > > > First patchset, > > > > (a) devlink piece to add/delete port > > > > (b) mlx5 counter part > > > > > > > > Second patchset, > > > > (a) devlink piece to set the state > > > > (b) mlx5 counter part > > > > > > > > Follow on patchset to create/delete sf's netdev on virtbus in mlx5 > > > > + devlink > > > plumbing. > > > > Netdevsim after that. > > > > > > I'd start from the virtbus part so we can see what's being created. > > > > How do you reach there without a user interface? > > Well.. why do you have a user interface which doesn't cause anything to happen > (devices won't get created)? You're splitting the submission, it's obvious the > implementation won't be complete after the first one. > > My expectation is that your implementation of the devlink commands will just > hand them off to FW, so there won't be anything interesting there to review. Correct. Since handing off to firmware and processing event from firmware is creating fairly more patches, In first series I will just go inline where devlink command would create/remove virtbus device on state active/inactive. This way it should be possible to have minimal working series under 20 patches. This way in one patchset, I prefer to cover (a) devlink plumbing for port add/del, port state (b) mlx5 eswitch refactor and devlink callback handling (c) devlink plumbing around (d) virtbus device and their netdev creation This also gives complete view from user interface to netdev device creation. Post this series, will improve the internals of mlx5 via events etc which doesn't need multi-vendor, virtbus and devlink involvement. Will differ netdevsim to later date. > > Start with the hard / risky parts - I consider the virtbus to be that, since the > conversation there includes multiple vendors, use cases and subsystems.