[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BY5PR12MB4322A28E6678CBB8A6544026DC4F9@BY5PR12MB4322.namprd12.prod.outlook.com>
Date: Tue, 13 Apr 2021 17:36:36 +0000
From: Parav Pandit <parav@...dia.com>
To: "Saleem, Shiraz" <shiraz.saleem@...el.com>,
Jason Gunthorpe <jgg@...dia.com>
CC: "dledford@...hat.com" <dledford@...hat.com>,
"kuba@...nel.org" <kuba@...nel.org>,
"davem@...emloft.net" <davem@...emloft.net>,
"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
"Lacombe, John S" <john.s.lacombe@...el.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"Ertman, David M" <david.m.ertman@...el.com>,
"Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
"Williams, Dan J" <dan.j.williams@...el.com>,
"Hefty, Sean" <sean.hefty@...el.com>,
"Keller, Jacob E" <jacob.e.keller@...el.com>
Subject: RE: [PATCH v4 05/23] ice: Add devlink params support
> From: Saleem, Shiraz <shiraz.saleem@...el.com>
> Sent: Tuesday, April 13, 2021 8:11 PM
[..]
> > > > Parav is talking about generic ways to customize the aux devices
> > > > created and that would seem to serve the same function as this.
> > >
> > > Is there an RFC or something posted for us to look at?
> > I do not have polished RFC content ready yet.
> > But coping the full config sequence snippet from the internal draft
> > (changed for ice
> > example) here as I like to discuss with you in this context.
>
> Thanks Parav! Some comments below.
>
> >
> > # (1) show auxiliary device types supported by a given devlink device.
> > # applies to pci pf,vf,sf. (in general at devlink instance).
> > $ devlink dev auxdev show pci/0000:06.00.0
> > pci/0000:06.00.0:
> > current:
> > roce eth
> > new:
> > supported:
> > roce eth iwarp
> >
> > # (2) enable iwarp and ethernet type of aux devices and disable roce.
> > $ devlink dev auxdev set pci/0000:06:00.0 roce off iwarp on
> >
> > # (3) now see which aux devices will be enable on next reload.
> > $ devlink dev auxdev show pci/0000:06:00.0
> > pci/0000:06:00.0:
> > current:
> > roce eth
> > new:
> > eth iwarp
> > supported:
> > roce eth iwarp
> >
> > # (4) now reload the device and see which aux devices are created.
> > At this point driver undergoes reconfig for removal of roce and adding
> iwarp.
> > $ devlink reload pci/0000:06:00.0
>
> I see this is modeled like devlink resource.
>
> Do we really to need a PCI driver re-init to switch the type of the auxdev
> hanging off the PCI dev?
>
I don't see a need to re-init the whole PCI driver. Since only aux device config is changed only that piece to get reloaded.
> Why not just allow the setting to apply dynamically during a 'set' itself with an
> unplug/plug of the auxdev with correct type.
>
This suggestion came up in the internal discussion too.
However such task needs to synchronize with devlink reload command and also with driver remove() sequence.
So locking wise and depending on amount of config change, it is close to what reload will do.
For example other resource config or other params setting also to take effect.
So to avoid defining multiple config sequence, doing as part of already existing devlink reload, it brings simple sequence to user.
For example,
1. enable/disable desired aux devices
2. configure device resources
3. set some device params
4. do devlink reload and apply settings done in #1 to #3
Powered by blists - more mailing lists