> -----Original Message-----
> From: Jakub Kicinski <jakub.kicin...@netronome.com>
> Sent: Saturday, July 6, 2019 1:09 AM
> To: Sudarsana Reddy Kalluru <skall...@marvell.com>
> Cc: da...@davemloft.net; netdev@vger.kernel.org; Michal Kalderon
> <mkalde...@marvell.com>; Ariel Elior <ael...@marvell.com>; Jiri Pirko
> <j...@resnulli.us>
> Subject: Re: [EXT] Re: [PATCH net-next v2 4/4] qed*: Add devlink support for
> configuration attributes.
> 
> On Fri, 5 Jul 2019 08:22:41 +0000, Sudarsana Reddy Kalluru wrote:
> > > On Thu, 4 Jul 2019 06:20:11 -0700, Sudarsana Reddy Kalluru wrote:
> > > > This patch adds implementation for devlink callbacks for reading
> > > > and configuring the device attributes.
> > > >
> > > > Signed-off-by: Sudarsana Reddy Kalluru <skall...@marvell.com>
> > > > Signed-off-by: Ariel Elior <ael...@marvell.com>
> 
> > > > diff --git a/Documentation/networking/devlink-params-qede.txt
> > > > b/Documentation/networking/devlink-params-qede.txt
> > > > new file mode 100644
> > > > index 0000000..f78a993
> > > > --- /dev/null
> > > > +++ b/Documentation/networking/devlink-params-qede.txt
> > > > @@ -0,0 +1,72 @@
> > > > +enable_sriov           [DEVICE, GENERIC]
> > > > +                       Configuration mode: Permanent
> > > > +
> > > > +iwarp_cmt              [DEVICE, DRIVER-SPECIFIC]
> > > > +                       Enable iWARP support over 100G device (CMT
> mode).
> > > > +                       Type: Boolean
> > > > +                       Configuration mode: runtime
> > > > +
> > > > +entity_id              [DEVICE, DRIVER-SPECIFIC]
> > > > +                       Set the entity ID value to be used for this 
> > > > device
> > > > +                       while reading/configuring the devlink 
> > > > attributes.
> > > > +                       Type: u8
> > > > +                       Configuration mode: runtime
> > >
> > > Can you explain what this is?
> >
> > Hardware/mfw provides the option to modify/read the config of other
> > PFs. A non-zero entity id represents a partition number (or simply a
> > PF-id) for which the config need to be read/updated.
> 
> Having a parameter which changes the interpretation of other parameters
> makes me quite uncomfortable :(  Could it be a better idea, perhaps, to use
> PCI ports?  We have been discussing PCI ports for a while now, and they will
> probably become a reality soon.  You could then hang the per-PF parameters
> off of the PF ports rather than the device instance?
> 
Agree with you, thanks.

> > > > +device_capabilities    [DEVICE, DRIVER-SPECIFIC]
> > > > +                       Set the entity ID value to be used for this 
> > > > device
> > > > +                       while reading/configuring the devlink 
> > > > attributes.
> > > > +                       Type: u8
> > > > +                       Configuration mode: runtime
> > >
> > > Looks like you copied the previous text here.
> > Will update it, thanks.
> >
> > >
> > > > +mf_mode                        [DEVICE, DRIVER-SPECIFIC]
> > > > +                       Configure Multi Function mode for the device.
> > > > +                       Supported MF modes and the assoicated values 
> > > > are,
> > > > +                           MF allowed(0), Default(1), SPIO4(2), 
> > > > NPAR1.0(3),
> > > > +                           NPAR1.5(4), NPAR2.0(5), BD(6) and UFP(7)
> > >
> > > NPAR should have a proper API in devlink port, what are the other
> modes?
> > >
> > These are the different modes supported by the Marvell NIC. In our
> > case the mf_mode is per adapter basis, e.g., it's not possible to
> > configure one port in NPAR mode and the other in Default mode.
> 
> Jiri, what are your thoughts on the NPAR support?  It is effectively a PCI 
> split.
> If we are going to support mdev split, should we perhaps have a "depth" or
> "type" of split and allow for users to configure it using the same API?
> 
> > > > +                       Type: u8
> > > > +                       Configuration mode: Permanent
> > > > +
> > > > +dcbx_mode              [PORT, DRIVER-SPECIFIC]
> > > > +                       Configure DCBX mode for the device.
> > > > +                       Supported dcbx modes are,
> > > > +                           Disabled(0), IEEE(1), CEE(2) and
> > > > Dynamic(3)
> > > > +                       Type: u8
> > > > +                       Configuration mode: Permanent
> > >
> > > Why is this a permanent parameter?
> > >
> > This specifies the dcbx_mode to be configured in non-volatile memory.
> > The value is persistent and is used in the next load of OS or the mfw.
> 
> And it can't be changed at runtime?
Run time dcbx params are not affected via this interface, it only updates 
config on permanent storage of the port.

Reply via email to