Message-ID: <ae70d810-8277-899b-b2a9-6b2dbdd5eb21@cumulusnetworks.com>
Date:   Wed, 27 Dec 2017 10:34:08 -0600
From:   David Ahern <dsa@...ulusnetworks.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, arkadis@...lanox.com,
        mlxsw@...lanox.com, andrew@...n.ch,
        vivien.didelot@...oirfairelinux.com, f.fainelli@...il.com,
        michael.chan@...adcom.com, ganeshgr@...lsio.com,
        saeedm@...lanox.com, matanb@...lanox.com, leonro@...lanox.com,
        idosch@...lanox.com, jakub.kicinski@...ronome.com, ast@...nel.org,
        daniel@...earbox.net, simon.horman@...ronome.com,
        pieter.jansenvanvuuren@...ronome.com, john.hurley@...ronome.com,
        alexander.h.duyck@...el.com, linville@...driver.com,
        gospo@...adcom.com, steven.lin1@...adcom.com, yuvalm@...lanox.com,
        ogerlitz@...lanox.com, roopa@...ulusnetworks.com
Subject: Re: [patch net-next v2 00/10] Add support for resource abstraction

On 12/27/17 2:09 AM, Jiri Pirko wrote:
> Wed, Dec 27, 2017 at 05:05:09AM CET, dsa@...ulusnetworks.com wrote:
>> On 12/26/17 5:23 AM, Jiri Pirko wrote:
>>> From: Jiri Pirko <jiri@...lanox.com>
>>>
>>> Many of the ASIC's internal resources are limited and are shared between
>>> several hardware procedures. For example, unified hash-based memory can
>>> be used for many lookup purposes, like FDB and LPM. In many cases the user
>>> can provide a partitioning scheme for such a resource in order to perform
>>> fine tuning for his application. In such cases performing driver reload is
>>> needed for the changes to take place, thus this patchset also adds support
>>> for hot reload.
>>>
>>> Such an abstraction can be coupled with devlink's dpipe interface, which
>>> models the ASIC's pipeline as a graph of match/action tables. By modeling
>>> the hardware resource object, and by coupling it to several dpipe tables,
>>> further visibility can be achieved in order to debug ASIC-wide issues.
>>>
>>> The proposed interface will provide the user the ability to understand the
>>> limitations of the hardware, and receive notification regarding its occupancy.
>>> Furthermore, monitoring the resource occupancy can be done in real-time and
>>> can be useful in many cases.
>>
>> In the last RFC (not v1, but RFC) I asked for some kind of description
>> for each resource, and you and Arkadi have pushed back. Let's walk
>> through an example to see what I mean:
>>
>> $ devlink resource show pci/0000:03:00.0
>> pci/0000:03:00.0:
>>  name kvd size 245760 size_valid true
>>  resources:
>>    name linear size 98304 occ 0
>>    name hash_double size 60416
>>    name hash_single size 87040
>>
>> So this 2700 has 3 resources that can be managed -- some table or
>> resource or something named 'kvd' with linear, hash_double and
>> hash_single sub-resources. What are these names referring too? The above
>> output gives no description, and 'kvd' is not an industry term. Further,
> 
> This are internal resources specific to the ASIC. Would you like some
> description to each or something like that?

devlink has some nice self-documenting capabilities. What's missing here
is a description of what the resource is used for in standard terms --
ipv4 host routes, fdb, nexthops, rifs, etc. Even if the description is a
short list versus an exhaustive list of everything it is used for. e.g.,
Why would a user decrease linear and increase hash_single or vice versa?

> 
> 
>> what are these sizes that a user can control? The output contains no
>> units, no description, nothing. In short, the above output provides
>> random numbers associated with random names.
> 
> Units are now exposed from kernel, just this version of iproute2 patch
> does not display it.

please provide an iproute2 patch that does so the full context if this
patch set can be reviewed from a user perspective.

> 
> 
>>
>> I can see dpipe tables exported by this device:
>>
>> $ devlink dpipe header show pci/0000:03:00.0
>>
>> pci/0000:03:00.0:
>>  name mlxsw_meta
>>  field:
>>    name erif_port bitwidth 32 mapping_type ifindex
>>    name l3_forward bitwidth 1
>>    name l3_drop bitwidth 1
>>    name adj_index bitwidth 32
>>    name adj_size bitwidth 32
>>    name adj_hash_index bitwidth 32
>>
>>  name ipv6
>>  field:
>>    name destination ip bitwidth 128
>>
>>  name ipv4
>>  field:
>>    name destination ip bitwidth 32
>>
>>  name ethernet
>>  field:
>>    name destination mac bitwidth 48
>>
>> but none mention 'kvd' or 'linear' or 'hash" and none of the other
>> various devlink options:
>>
>> $ devlink
>> Usage: devlink [ OPTIONS ] OBJECT { COMMAND | help }
>> where  OBJECT := { dev | port | sb | monitor | dpipe }
>>
>> seem to related to resources.
>>
>> So how does a user know what they are controlling by this 'resource'
>> option? Is the user expected to have a PRM or user guide on hand for the
>> specific device model that is being configured?
> 
> The relation of specific dpipe table to specific resource is exposed by
> the kernel as well. Probably the iproute2 patch just does not display
> it.

please provide an iproute2 patch that does so the full context if this
patch set can be reviewed from a user perspective.

> 
> 
>>
>> Again, I have no objections to kvd, linear, hash, etc terms as they do
>> relate to Mellanox products. But kvd/linear, for example, does correlate
>> to industry standard concepts in some way. My request is that the
>> resource listing guide the user in some way, stating what these
>> resources mean.
> 
> So the showed relation to dpipe table would be enougn or you would still
> like to see some description? I don't like the description concept here
> as the relations to dpipe table should tell user exactly what he needs
> to know.

I believe it is useful to have a 1-line, short description that gives
the user some memory jogger as to what the resource is used for. It does
not have to be an exhaustive list, but the user should not have to do
mental jumping jacks running a bunch of commands to understand the
resources for vendor specific asics.

Powered by blists - more mailing lists