On Tue, Sep 11, 2018 at 5:25 PM Jiri Pirko [off-list ref] wrote:

Tue, Sep 11, 2018 at 01:33:51PM CEST, jakub.kicinski@netronome.com wrote:

quoted

On Tue, 11 Sep 2018 14:14:57 +0530, Vasundhara Volam wrote:

quoted

This patchset adds support for 4 generic and 1 driver-specific devlink
parameters.

Also, this patchset adds support to return proper error code if
HWRM_NVM_GET/SET_VARIABLE commands return error code
HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED.

Vasundhara Volam (8):
  devlink: Add generic parameter hw_tc_offload

Much like Jiri, I can't help but wonder why do you need this?

quoted

  devlink: Add generic parameter ignore_ari
  devlink: Add generic parameter msix_vec_per_pf_max
  devlink: Add generic parameter msix_vec_per_pf_min

IMHO more structured API would be preferable if possible.  The string
keys won't scale if you want to set the parameters per PF, and
creating more structured API for PCIe which is a relatively slow
moving HW spec seems tractable.

Not to mention the question Alex posed before about where this knobs
should actually live.  I'm personally fine with devlink.

quoted

  bnxt_en: Use hw_tc_offload and ignore_ari devlink parameters
  bnxt_en: return proper error when FW returns
    HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED
  bnxt_en: Use msix_vec_per_pf_max and msix_vec_per_pf_min devlink
    params.
  bnxt_en: Add a driver specific devlink parameter.

The details about the device specific devlink parameter are very much
lacking.  Why does the patch subject not mention any specifics?  What's
GRE in the first place?

You really need to provide more details and docs for all these
parameters.

We really need Documentation/networking/devlink-params.txt to describe
these.

Sure. I will add the documentation in my next version of the patchset
for all generic and driver specific parameters.