Add support for resource abstraction

From: Jiri Pirko <[email protected]>

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.

Quoted 15 lines · Jiri Pirko

From: Jiri Pirko <[email protected]>

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.

Quoted 4 lines

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,

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.

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?

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.

IMO the above output is not user friendly and having to keep a PRM on hand for each device model is not a realistic solution.

Quoted 8 lines

$ 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

Quoted 4 lines

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?

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.

Quoted 8 lines

Quoted 8 lines

$ 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

Quoted 7 lines

Quoted 4 lines

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?

The fact you have decided to expose them means you expect people to change them. So yes, they need to be documented. Maybe add something to Documentation/ABI/stable/

Quoted 4 lines

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.

Documenting the ABI is good practice.

Andrew

This is misunderstanding I believe. This is not about ABI. That is well defined by the netlink attributes. This is about meaning of particular ASIC-specific internal resources.

Quoted 3 lines

This is misunderstanding I believe. This is not about ABI. That is well defined by the netlink attributes. This is about meaning of particular ASIC-specific internal resources.

I would agree that the netlink attributed are clearly defined. But the meta information, what this ASIC specific internal resource means when you combine these attributes, is unclear. This meta information is also part of the ABI, and documenting giving users a hit what it means, and why they should change it, would be good practice.

Look at sysfs. open/read/write are clearly defined, which is the equivalent of the netlink attributes. The meta information we document in Documentation/ABI/, what a file name means, what a value means, what other values it can take, etc.

Quoted 10 lines · Wed, Dec 27, 2017 at 02:08:03PM CET, [email protected] wrote:

Quoted 3 lines

This is misunderstanding I believe. This is not about ABI. That is well defined by the netlink attributes. This is about meaning of particular ASIC-specific internal resources.

I would agree that the netlink attributed are clearly defined. But the meta information, what this ASIC specific internal resource means when you combine these attributes, is unclear. This meta information is also part of the ABI, and documenting giving users a hit what it means, and why they should change it, would be good practice.

Quoted 4 lines

Look at sysfs. open/read/write are clearly defined, which is the equivalent of the netlink attributes. The meta information we document in Documentation/ABI/, what a file name means, what a value means, what other values it can take, etc.

Hmm. That documents mainly sysfs. No mention of Netlink at all. But maybe I missed it. Also, that defines the interface as is. However we are talking about the data exchanged over the interface, not the interface itself. I don't see how ASIC/HW specific thing, like for example KVD in our case could be part of kernel ABI. That makes 0 sense to me, sorry.

Hmm. That documents mainly sysfs. No mention of Netlink at all. But maybe I missed it. Also, that defines the interface as is. However we are talking about the data exchanged over the interface, not the interface itself. I don't see how ASIC/HW specific thing, like for example KVD in our case could be part of kernel ABI.

Quoted 5 lines

Hmm. That documents mainly sysfs. No mention of Netlink at all. But maybe I missed it. Also, that defines the interface as is. However we are talking about the data exchanged over the interface, not the interface itself. I don't see how ASIC/HW specific thing, like for example KVD in our case could be part of kernel ABI.

You need to be very careful here. As soon as somebody starts using it, it might become an ABI. Or you need to clearly document it is not ABI, there is no guarantee it will not disappear or change its meaning in the next kernel, and it should be used with extreme caution.

Quoted 2 lines

Quoted 2 lines

Many of the ASIC's internal resources are limited and are shared between several hardware procedures. For example, unified hash-based memory

can

Quoted 1 line

Quoted 1 line

be used for many lookup purposes, like FDB and LPM. In many cases the

user

Quoted 5 lines

Quoted 5 lines

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.

Quoted 7 lines

Quoted 7 lines

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.

Quoted 1 line

Quoted 1 line

Furthermore, monitoring the resource occupancy can be done in real-time

and

Quoted 14 lines

Quoted 1 line

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

Quoted 4 lines

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?

Quoted 3 lines

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.

Quoted 36 lines

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.

Quoted 3 lines

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.

Quoted 8 lines

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.

Quoted 3 lines

IMO the above output is not user friendly and having to keep a PRM on hand for each device model is not a realistic solution.

Quoted 33 lines · Wed, Dec 27, 2017 at 05:05:09AM CET, [email protected] wrote:

Quoted 15 lines · Jiri Pirko

From: Jiri Pirko <[email protected]>

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.

Quoted 4 lines

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

Quoted 4 lines

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?

Quoted 5 lines

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.

Quoted 38 lines

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.

Quoted 3 lines

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.

Quoted 8 lines

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.

Quoted 39 lines · Jiri Pirko

Quoted 37 lines · Wed, Dec 27, 2017 at 05:05:09AM CET, [email protected] wrote:

Quoted 15 lines · Jiri Pirko

From: Jiri Pirko <[email protected]>

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.

Quoted 4 lines

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,

Quoted 2 lines

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?

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Many of the ASIC's internal resources are limited and are shared

between

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

several hardware procedures. For example, unified hash-based memory

can

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

be used for many lookup purposes, like FDB and LPM. In many cases the

user

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

can provide a partitioning scheme for such a resource in order to

perform

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

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

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

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

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

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

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

limitations of the hardware, and receive notification regarding its

occupancy.

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Furthermore, monitoring the resource occupancy can be done in real-

time and

Quoted 14 lines

Quoted 14 lines

Quoted 14 lines

Quoted 1 line

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

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

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

Quoted 10 lines

Quoted 4 lines

Quoted 1 line

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?

Arkadi, on what david says above, can the resource names and ids not be driver specific, but moved up to the switchdev layer and just map to fdb, host routes, nexthops table sizes etc ?.
Can these generic networking resources then in-turn be mapped to kvd sizes by the driver ?

I think it goes the other way around. The dpipe tables are the ones that can be translated to functionality; The resources are internal and HW-specific representing the possible internal division of resources -
but a given resource sn't necessarily mapped to a single networking feature. [It might be in some cases, but not in the general case]

Quoted 5 lines · Yuval Mintz

I think it goes the other way around. The dpipe tables are the ones that can be translated to functionality; The resources are internal and HW-specific representing the possible internal division of resources -
but a given resource sn't necessarily mapped to a single networking feature. [It might be in some cases, but not in the general case]

This is what I am getting at -- a single resource /kvd/linear is used for multiple networking features, and those networking features do map to well known entities -- fdb entries, ACL entries, ipv4/v6 host entries, LPM entries, etc.

Nothing about the output from devlink helps the user in any way to understand how to change the resource values. Saying that these

resources, what they mean and how they are used is MLX proprietary and is known only to MLX employees and those with MLX agreements is not acceptable. Likewise, requiring some network admin to deep dive into the mlxsw driver to piece together how kvd/linear (for example) is used is not acceptable.

The cover letter touts "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."

Great, now give the user some indication of how to do that. Is setting /kvd/linear to 0 acceptable? If not, why? What functionality is lost? (Apparently, everything [1].)

The dpipe tables list some correlation between the kvd resources and tables but that is not a complete list and again there is nothing to tell a user that it is only a partial list of how a kvd resource is

used. For example, it shows ipv4 host is in /kvd/hash_single and that is all it shows. So if I have an ipv6 only deployment can I conclude that I can set /kvd/hash_single to 0? Or the reverse, can I set hash_double to 0 for an ipv4 only deployment? From the limited information given, it is reasonable for a user to assume yes and has to learn through trial and error what can be done. [2]

-----

[1] This is allowed by the current patch set and perhaps it should not be:

$ ip ro ls vrf vrf1101
unreachable default metric 8192
11.2.51.0/24 dev swp1s0.51 proto kernel scope link src 11.2.51.1 offload
11.3.51.0/24 dev swp1s1.51 proto kernel scope link src 11.3.51.1 offload
11.4.51.0/24 dev swp1s2.51 proto kernel scope link src 11.4.51.1 offload
11.5.51.0/24 dev swp1s3.51 proto kernel scope link src 11.5.51.1 offload
11.6.51.0/24 dev swp3s0.51 proto kernel scope link src 11.6.51.1 offload
11.7.51.0/24 dev swp3s1.51 proto kernel scope link src 11.7.51.1 offload
11.8.51.0/24 dev swp3s2.51 proto kernel scope link src 11.8.51.1 offload
11.9.51.0/24 dev swp3s3.51 proto kernel scope link src 11.9.51.1 offload

$ devlink resource set pci/0000:03:00.0 path /kvd/linear size 0

$ devlink reload pci/0000:03:00.0
$ ip ro ls vrf vrf1101
unreachable default metric 8192

[2] Same exact result for setting hash_double to 0:

$ ip ro ls vrf vrf1101
unreachable default metric 8192
11.2.51.0/24 dev swp1s0.51 proto kernel scope link src 11.2.51.1 offload
11.3.51.0/24 dev swp1s1.51 proto kernel scope link src 11.3.51.1 offload
11.4.51.0/24 dev swp1s2.51 proto kernel scope link src 11.4.51.1 offload
11.5.51.0/24 dev swp1s3.51 proto kernel scope link src 11.5.51.1 offload
11.6.51.0/24 dev swp3s0.51 proto kernel scope link src 11.6.51.1 offload
11.7.51.0/24 dev swp3s1.51 proto kernel scope link src 11.7.51.1 offload
11.8.51.0/24 dev swp3s2.51 proto kernel scope link src 11.8.51.1 offload
11.9.51.0/24 dev swp3s3.51 proto kernel scope link src 11.9.51.1 offload

$ devlink resource set pci/0000:03:00.0 path /kvd/hash_double size 0
$ devlink reload pci/0000:03:00.0
$ ip ro ls vrf vrf1101
unreachable default metric 8192

Quoted 13 lines

[1] This is allowed by the current patch set and perhaps it should not be:

$ ip ro ls vrf vrf1101
unreachable default metric 8192
11.2.51.0/24 dev swp1s0.51 proto kernel scope link src 11.2.51.1 offload
11.3.51.0/24 dev swp1s1.51 proto kernel scope link src 11.3.51.1 offload
11.4.51.0/24 dev swp1s2.51 proto kernel scope link src 11.4.51.1 offload
11.5.51.0/24 dev swp1s3.51 proto kernel scope link src 11.5.51.1 offload
11.6.51.0/24 dev swp3s0.51 proto kernel scope link src 11.6.51.1 offload
11.7.51.0/24 dev swp3s1.51 proto kernel scope link src 11.7.51.1 offload
11.8.51.0/24 dev swp3s2.51 proto kernel scope link src 11.8.51.1 offload
11.9.51.0/24 dev swp3s3.51 proto kernel scope link src 11.9.51.1 offload

Quoted 1 line

$ devlink resource set pci/0000:03:00.0 path /kvd/linear size 0

This line actually did nothing, because size zero is not acceptable see patch 6. This is pure userpsace problem that error is not shown.

You can verify it by dumping the resources and see that there is no pending change (only size and not size_new).

Quoted 4 lines

$ devlink reload pci/0000:03:00.0
$ ip ro ls vrf vrf1101
unreachable default metric 8192

So you just performed full reload of the driver which includes unregistration of all the netdevs and full init. KVD update requires full teardown of the driver.

The system will not get back to the same state after reloading, It's should be done on init. But it doesn't have to be like this this, each driver provides his own reload devlink op implementation so in our case full blown reset is required.

Quoted 13 lines

[2] Same exact result for setting hash_double to 0:

$ ip ro ls vrf vrf1101
unreachable default metric 8192
11.2.51.0/24 dev swp1s0.51 proto kernel scope link src 11.2.51.1 offload
11.3.51.0/24 dev swp1s1.51 proto kernel scope link src 11.3.51.1 offload
11.4.51.0/24 dev swp1s2.51 proto kernel scope link src 11.4.51.1 offload
11.5.51.0/24 dev swp1s3.51 proto kernel scope link src 11.5.51.1 offload
11.6.51.0/24 dev swp3s0.51 proto kernel scope link src 11.6.51.1 offload
11.7.51.0/24 dev swp3s1.51 proto kernel scope link src 11.7.51.1 offload
11.8.51.0/24 dev swp3s2.51 proto kernel scope link src 11.8.51.1 offload
11.9.51.0/24 dev swp3s3.51 proto kernel scope link src 11.9.51.1 offload

$ devlink resource set pci/0000:03:00.0 path /kvd/hash_double size 0

Quoted 13 lines · Arkadi Sharshevsky

Quoted 13 lines

[1] This is allowed by the current patch set and perhaps it should not be:

$ ip ro ls vrf vrf1101
unreachable default metric 8192
11.2.51.0/24 dev swp1s0.51 proto kernel scope link src 11.2.51.1 offload
11.3.51.0/24 dev swp1s1.51 proto kernel scope link src 11.3.51.1 offload
11.4.51.0/24 dev swp1s2.51 proto kernel scope link src 11.4.51.1 offload
11.5.51.0/24 dev swp1s3.51 proto kernel scope link src 11.5.51.1 offload
11.6.51.0/24 dev swp3s0.51 proto kernel scope link src 11.6.51.1 offload
11.7.51.0/24 dev swp3s1.51 proto kernel scope link src 11.7.51.1 offload
11.8.51.0/24 dev swp3s2.51 proto kernel scope link src 11.8.51.1 offload
11.9.51.0/24 dev swp3s3.51 proto kernel scope link src 11.9.51.1 offload

Quoted 4 lines

Quoted 1 line

$ devlink resource set pci/0000:03:00.0 path /kvd/linear size 0

This line actually did nothing, because size zero is not acceptable see patch 6. This is pure userpsace problem that error is not shown.

Then perhaps you have a kernel side bug. After the reload I get this:

$ devlink resource show pci/0000:03:00.0
pci/0000:03:00.0:
  name kvd size 245760 size_valid true
  resources:
    name linear size 0 occ 0
    name hash_double size 60416
    name hash_single size 87040

Quoted 5 lines

You can verify it by dumping the resources and see that there is no pending change (only size and not size_new).

Quoted 8 lines

Quoted 4 lines

$ devlink reload pci/0000:03:00.0
$ ip ro ls vrf vrf1101
unreachable default metric 8192

So you just performed full reload of the driver which includes unregistration of all the netdevs and full init. KVD update requires full teardown of the driver.

you are right, I forgot to do a networking reload. Because of the above (0 was actually taken) all kinds of errors are spewed on 'ifreload -av' and there is no change to the ro ls:

$  ip ro ls vrf vrf1101
unreachable default metric 8192

Quoted 20 lines

The system will not get back to the same state after reloading, It's should be done on init. But it doesn't have to be like this this, each driver provides his own reload devlink op implementation so in our case full blown reset is required.

Quoted 13 lines

[2] Same exact result for setting hash_double to 0:

$ ip ro ls vrf vrf1101
unreachable default metric 8192
11.2.51.0/24 dev swp1s0.51 proto kernel scope link src 11.2.51.1 offload
11.3.51.0/24 dev swp1s1.51 proto kernel scope link src 11.3.51.1 offload
11.4.51.0/24 dev swp1s2.51 proto kernel scope link src 11.4.51.1 offload
11.5.51.0/24 dev swp1s3.51 proto kernel scope link src 11.5.51.1 offload
11.6.51.0/24 dev swp3s0.51 proto kernel scope link src 11.6.51.1 offload
11.7.51.0/24 dev swp3s1.51 proto kernel scope link src 11.7.51.1 offload
11.8.51.0/24 dev swp3s2.51 proto kernel scope link src 11.8.51.1 offload
11.9.51.0/24 dev swp3s3.51 proto kernel scope link src 11.9.51.1 offload

$ devlink resource set pci/0000:03:00.0 path /kvd/hash_double size 0

On this command you are correct, 0 is not taken:

$ devlink resource set pci/0000:03:00.0 path /kvd/hash_double size 0
$ devlink resource show pci/0000:03:00.0
pci/0000:03:00.0:
  name kvd size 245760 size_valid true
  resources:
    name linear size 0 occ 0
    name hash_double size 60416
    name hash_single size 87040

but the 'set' command did not fail with a proper extack based error message, so consider this another a bug report.

Quoted 34 lines · Jiri Pirko

Quoted 33 lines · Wed, Dec 27, 2017 at 05:05:09AM CET, [email protected] wrote:

Quoted 15 lines · Jiri Pirko

From: Jiri Pirko <[email protected]>

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.

Quoted 4 lines

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

Quoted 7 lines

Quoted 4 lines

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?

Quoted 8 lines

Quoted 5 lines

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.

Quoted 42 lines

Quoted 41 lines

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?

Quoted 3 lines

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.

Quoted 9 lines

Quoted 8 lines

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.

Quoted 4 lines

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.

Quoted 1 line

Quoted 1 line

Quoted 1 line

Many of the ASIC's internal resources are limited and are shared

between

Quoted 1 line

Quoted 1 line

Quoted 1 line

several hardware procedures. For example, unified hash-based memory

can

Quoted 1 line

Quoted 1 line

Quoted 1 line

be used for many lookup purposes, like FDB and LPM. In many cases the

user

Quoted 1 line

Quoted 1 line

Quoted 1 line

can provide a partitioning scheme for such a resource in order to

perform

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

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

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

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.

Quoted 1 line

Quoted 1 line

Quoted 1 line

Furthermore, monitoring the resource occupancy can be done in real-

time and

Quoted 14 lines

Quoted 14 lines

Quoted 1 line

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

Quoted 3 lines

Quoted 3 lines

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

Quoted 4 lines

Quoted 1 line

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?

Quoted 8 lines

Quoted 5 lines

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.

Quoted 38 lines

Quoted 38 lines

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.

Quoted 7 lines

Quoted 3 lines

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.

Quoted 9 lines

Quoted 8 lines

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.

Quoted 4 lines

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.

Quoted 11 lines

Quoted 10 lines

Quoted 5 lines

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.

Quoted 5 lines

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.

Perhaps we can simply have devlink utility output the dpipe table[s] associated with the resource when showing the resource? It would contain live information as well as prevent the need for 'mental jumping jacks'.

Quoted 17 lines · Yuval Mintz

Quoted 11 lines

Quoted 10 lines

Quoted 5 lines

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.

Quoted 5 lines

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.

Quoted 5 lines

Perhaps we can simply have devlink utility output the dpipe table[s] associated with the resource when showing the resource? It would contain live information as well as prevent the need for 'mental jumping jacks'.

My primary contention for this static partitioning is that the proposal is not giving the user the information they need to make decisions.

As I mentioned earlier, the resource show command gives this:

$ 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

the paths /kvd/linear, /kvd/hash_single and /kvd/hash_double are essentially random names (nothing related to industry standard names)

and the listed sizes are random numbers (no units)[1]. There is nothing there to tell a user what they can adjust or why they would want to make an adjustment.

Looking at 'dpipe table show':

$ devlink dpipe table show pci/0000:03:00.0
pci/0000:03:00.0:
 name mlxsw_erif size 1000 counters_enabled false
 match:
   type field_exact header mlxsw_meta field erif_port mapping ifindex
 action:
   type field_modify header mlxsw_meta field l3_forward
   type field_modify header mlxsw_meta field l3_drop

resource_path /kvd/hash_single name mlxsw_host4 size 62
counters_enabled false
match:
type field_exact header mlxsw_meta field erif_port mapping ifindex type field_exact header ipv4 field destination ip
action:
type field_modify header ethernet field destination mac

resource_path /kvd/hash_double name mlxsw_host6 size 0
counters_enabled false
match:
type field_exact header mlxsw_meta field erif_port mapping ifindex type field_exact header ipv6 field destination ip
action:
type field_modify header ethernet field destination mac

resource_path /kvd/linear name mlxsw_adj size 0 counters_enabled false match:
type field_exact header mlxsw_meta field adj_index
type field_exact header mlxsw_meta field adj_size
type field_exact header mlxsw_meta field adj_hash_index
action:
type field_modify header ethernet field destination mac type field_modify header mlxsw_meta field erif_port mapping ifindex

So there are 4 tables exported to userspace:

1. mlxsw_erif table which is not in any of the kvd regions (no resource path is given) and it has a size of 1000. Does mlxsw_erif mean a rif as in Router Interfaces? So the switch supports up to 1000 router interfaces.

2. mlxsw_host4 in /kvd/hash_single with a size of 62. Based on the

fields mlxsw_host4 means IPv4 host entries (neighbor entries). Why is the size set at 62? Seems really low.

3. mlxsw_host6 in /kvd/hash_double with a size of 0. Based on the fields mlxsw_host6 means IPv6 host entries (neighbor entries). The size of 0 is concerning. I guess the switch is not configured to do IPv6?

4. mlxsw_adj in /kvd/linear with a size of 0. Based on the fields I am going to guess it is an fdb entry????

Quoted 7 lines

So there are 4 tables exported to userspace:

1. mlxsw_erif table which is not in any of the kvd regions (no resource path is given) and it has a size of 1000. Does mlxsw_erif mean a rif as in Router Interfaces? So the switch supports up to 1000 router interfaces.

2. mlxsw_host4 in /kvd/hash_single with a size of 62. Based on the

Size tells you the actual size. It cannot give you max size. The reason is simple. The resources are shared among multiple tables. That is exactly what this resource patch makes visible.

Quoted 6 lines · Jiri Pirko

Quoted 6 lines

So there are 4 tables exported to userspace:

1. mlxsw_erif table which is not in any of the kvd regions (no resource path is given) and it has a size of 1000. Does mlxsw_erif mean a rif as in Router Interfaces? So the switch supports up to 1000 router interfaces.

Quoted 6 lines

Quoted 1 line

2. mlxsw_host4 in /kvd/hash_single with a size of 62. Based on the

Size tells you the actual size. It cannot give you max size. The reason is simple. The resources are shared among multiple tables. That is exactly what this resource patch makes visible.

In the erif table, the 1000 is the max not current usage. I do not have

1000 interfaces:

$ ip -br li sh | wc -l
597

$ devlink dpipe table dump pci/0000:03:00.0 name mlxsw_erif
...
 index 503
 match_value:
   type field_exact header mlxsw_meta field erif_port mapping ifindex
mapping_value 601 value 503
 action_value:
   type field_modify header mlxsw_meta field l3_forward value 1

The host4 table it is current size with no maximum.

The meaning of table size needs to be consistent across tables.

Quoted 7 lines · Jiri Pirko

Quoted 7 lines

So there are 4 tables exported to userspace:

1. mlxsw_erif table which is not in any of the kvd regions (no resource path is given) and it has a size of 1000. Does mlxsw_erif mean a rif as in Router Interfaces? So the switch supports up to 1000 router interfaces.

Quoted 7 lines

Quoted 2 lines

2. mlxsw_host4 in /kvd/hash_single with a size of 62. Based on the

Size tells you the actual size. It cannot give you max size. The reason is simple. The resources are shared among multiple tables. That is exactly what this resource patch makes visible.

In the erif table, the 1000 is the max not current usage. I do not have 1000 interfaces:

$ ip -br li sh | wc -l 597

$ devlink dpipe table dump pci/0000:03:00.0 name mlxsw_erif ... index
503 match_value: type field_exact header mlxsw_meta field erif_port
mapping ifindex mapping_value 601 value 503 action_value: type
field_modify header mlxsw_meta field l3_forward value 1

The host4 table it is current size with no maximum.

The meaning of table size needs to be consistent across tables.

1. Both dpipe and devlink resource are abstraction models for hardware entities, and as a result they true to provide generic objects. Each driver/ASIC should register his own and it absolutely proprietary implementation. There is absolutely NO industry standard here, the only thing that resembles a standard is that dpipe looks a bit like P4 only because its proved to be useful for describing packet forwarding pipelines. The host4 table is just a hardware process in the mellanox spectrum ASIC pipeline and it should not be part of ABI, sorry I clearly don't understand how this even came up.

Quoted 9 lines

1. Both dpipe and devlink resource are abstraction models for hardware entities, and as a result they true to provide generic objects. Each driver/ASIC should register his own and it absolutely proprietary implementation. There is absolutely NO industry standard here, the only thing that resembles a standard is that dpipe looks a bit like P4 only because its proved to be useful for describing packet forwarding pipelines. The host4 table is just a hardware process in the mellanox spectrum ASIC pipeline and it should not be part of ABI, sorry I clearly don't understand how this even came up.

Why should it not be part of the ABI? Are you saying it will change from version to version? Are you trying to warning people not to write scripts using it, because its meaning will change and what works now will break in the next kernel version?

In my opinion it should not change. Unless there is a bug (like the one DaveA found in mlxsw erif table). Existing tables and resources should be only added. It is the driver's maintainer responsibility to not to break user scripts.

From: Jiri Pirko <[email protected]>

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.
---
Userspace part prototype can be found at https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Farkadis%2Fiproute2%2F&data=02%7C01%7Carkadis%40mellanox.com%7C1ae3d8b4854a454e21e008d54c5329e3%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636498842440762657&sdata=7MC2BFQFxjnmHqy2sOOL9VEa4ZGq6e5Z2n2WvuNgyFk%3D&reserved=0
at resource_dev branch.

v1->v2
- Add resource size attribute.
- Fix split bug.

Arkadi Sharshevsky (10):
devlink: Add per devlink instance lock
devlink: Add support for resource abstraction
devlink: Add support for reload
devlink: Add relation between dpipe and resource
mlxsw: pci: Add support for performing bus reset
mlxsw: spectrum: Register KVD resources with devlink
mlxsw: spectrum_dpipe: Connect dpipe tables to resources
mlxsw: spectrum: Add support for getting kvdl occupancy
mlxsw: pci: Add support for getting resource through devlink
mlxsw: core: Add support for reload

 drivers/net/ethernet/mellanox/mlxsw/core.c         |  85 ++-
 drivers/net/ethernet/mellanox/mlxsw/core.h         |  16 +-
 drivers/net/ethernet/mellanox/mlxsw/i2c.c          |   5 +-
 drivers/net/ethernet/mellanox/mlxsw/pci.c          |  98 ++--
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     | 205 ++++++++
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |  13 +
 .../net/ethernet/mellanox/mlxsw/spectrum_dpipe.c   |  72 ++-
 .../net/ethernet/mellanox/mlxsw/spectrum_kvdl.c    |  26 +
 include/net/devlink.h                              |  97 ++++
 include/uapi/linux/devlink.h                       |  21 +
 net/core/devlink.c                                 | 573 ++++++++++++++++++---
 11 files changed, 1079 insertions(+), 132 deletions(-)

Quoted 23 lines · Jiri Pirko

From: Jiri Pirko <[email protected]>

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.
---
Userspace part prototype can be found at https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Farkadis%2Fiproute2%2F&data=02%7C01%7Carkadis%40mellanox.com%7C1ae3d8b4854a454e21e008d54c5329e3%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636498842440762657&sdata=7MC2BFQFxjnmHqy2sOOL9VEa4ZGq6e5Z2n2WvuNgyFk%3D&reserved=0
at resource_dev branch.

Quoted 4 lines

v1->v2
- Add resource size attribute.
- Fix split bug.

Just to summarize the current fixes required:

1. ERIF dpipe table size is reporting wrong size. More precisely the ERIF table does not take rifs, so it should not be linked to the rif bank resource (is not part of this patchset, future extension).
2. Extended ACK user-space bug.
3. ABI documentation- Not sure we agreed upon it, Jiri?

If I missed something please respond. Nothing of the fixes mentioned above is relevant for this patchset actually.

Couple of key-points:

1. Constrains\trade off about setting the sizes - this can be obtained trivially from the resource tree nested structure.
2. Dpipe provides the mapping of hardware processes to resources.
3. Units - each resource specifies his units, if dpipe table's size is X and its related to some resource its size is normalized to that resources basic unit.

IMO this is the most hardware exact interaction, and this is the way it should be exported from the kernel, if something is not presented in 'user' convenient way some utilities can be implemented in userspace to easily do it. Furthermore, some examples will be provided for the whole kvd tree partition for different cases (IPv6 heavy etc..). Advanced user will be able to tweak it as they like.

Regarding the 'switchdev' layer I think that kernel's software tables like nexthops/neigh/routes should be mapped to dpipe tables and not to resources directly:

kernel_fdb--> dpipe_fdb -->/kvd/hash_single.

Quoted 25 lines

Arkadi Sharshevsky (10):
devlink: Add per devlink instance lock
devlink: Add support for resource abstraction
devlink: Add support for reload
devlink: Add relation between dpipe and resource
mlxsw: pci: Add support for performing bus reset
mlxsw: spectrum: Register KVD resources with devlink
mlxsw: spectrum_dpipe: Connect dpipe tables to resources
mlxsw: spectrum: Add support for getting kvdl occupancy
mlxsw: pci: Add support for getting resource through devlink
mlxsw: core: Add support for reload

 drivers/net/ethernet/mellanox/mlxsw/core.c         |  85 ++-
 drivers/net/ethernet/mellanox/mlxsw/core.h         |  16 +-
 drivers/net/ethernet/mellanox/mlxsw/i2c.c          |   5 +-
 drivers/net/ethernet/mellanox/mlxsw/pci.c          |  98 ++--
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c     | 205 ++++++++
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h     |  13 +
 .../net/ethernet/mellanox/mlxsw/spectrum_dpipe.c   |  72 ++-
 .../net/ethernet/mellanox/mlxsw/spectrum_kvdl.c    |  26 +
 include/net/devlink.h                              |  97 ++++
 include/uapi/linux/devlink.h                       |  21 +
 net/core/devlink.c                                 | 573 ++++++++++++++++++---
 11 files changed, 1079 insertions(+), 132 deletions(-)

Question is where to put it. It is mlxsw-specific thing, moreover, Spectrum-specific thing, same as dpipe tables etc. Not sure. Perhaps Documentation/networking/mlxsw.txt ?

Quoted 3 lines

Question is where to put it. It is mlxsw-specific thing, moreover, Spectrum-specific thing, same as dpipe tables etc. Not sure. Perhaps Documentation/networking/mlxsw.txt ?

Hi Jiri

Documentation/ABI seems like the correct place. There is nothing in the README which says it is limited to files. You could use an name like devlink-driver-mlxsw.

Just to summarize the current fixes required:

1. ERIF dpipe table size is reporting wrong size. More precisely the ERIF table does not take rifs, so it should not be linked to the rif bank resource (is not part of this patchset, future extension).
2. Extended ACK user-space bug.
3. ABI documentation- Not sure we agreed upon it, Jiri?

If I missed something please respond. Nothing of the fixes mentioned above is relevant for this patchset actually.

Quoted 12 lines · Arkadi Sharshevsky

Just to summarize the current fixes required:

1. ERIF dpipe table size is reporting wrong size. More precisely the ERIF table does not take rifs, so it should not be linked to the rif bank resource (is not part of this patchset, future extension).
2. Extended ACK user-space bug.
3. ABI documentation- Not sure we agreed upon it, Jiri?

If I missed something please respond. Nothing of the fixes mentioned above is relevant for this patchset actually.

Can you fix the userspace command and then we come back to what else is needed? Right now, it is hard to tell what is a user space bug and what is a kernel space bug.

For example:

$ devlink resource set pci/0000:03:00.0 path /kvd/linear size 10000
$ 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

The set command did not fail, yet there is no size_new arg in the output like there is for this change:

$ devlink resource set pci/0000:03:00.0 path /kvd/linear size 0
$ 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 size_new 0 occ 0
    name hash_double size 60416
    name hash_single size 87040

As I stated this is a user-space bug which I fixed, and updated my repo so please pull. Devlink uses mnl,and currently mnl does not support extended ack. I added support for this in my local ver of libmnl:

https://github.com/arkadis/libmnl.git

On branch master, so you can check it out. Besides this bugs, which were userspace, can please specify what are the pending problems from your point of view? Thanks!

Quoted 4 lines · Arkadi Sharshevsky

As I stated this is a user-space bug which I fixed, and updated my repo so please pull. Devlink uses mnl,and currently mnl does not support extended ack. I added support for this in my local ver of libmnl:

Quoted 5 lines

https://github.com/arkadis/libmnl.git

On branch master, so you can check it out. Besides this bugs, which were userspace, can please specify what are the pending problems from your point of view? Thanks!

devlink is in iproute2 package and it has extack support. See 'git log lib/libnetlink.c'

Quoted 7 lines · Wed, Jan 03, 2018 at 07:14:16PM CET, [email protected] wrote:

Quoted 6 lines · Arkadi Sharshevsky

As I stated this is a user-space bug which I fixed, and updated my repo so please pull. Devlink uses mnl,and currently mnl does not support extended ack. I added support for this in my local ver of libmnl:

https://github.com/arkadis/libmnl.git

Quoted 6 lines

Quoted 3 lines

On branch master, so you can check it out. Besides this bugs, which were userspace, can please specify what are the pending problems from your point of view? Thanks!

devlink is in iproute2 package and it has extack support. See 'git log lib/libnetlink.c'

Dave, devlink uses libmnl.

Quoted 15 lines · Jiri Pirko

Quoted 13 lines · Wed, Jan 03, 2018 at 07:14:16PM CET, [email protected] wrote:

Quoted 9 lines · Arkadi Sharshevsky

As I stated this is a user-space bug which I fixed, and updated my repo so please pull. Devlink uses mnl,and currently mnl does not support extended ack. I added support for this in my local ver of libmnl:

https://github.com/arkadis/libmnl.git

On branch master, so you can check it out. Besides this bugs, which were userspace, can please specify what are the pending problems from your point of view? Thanks!

devlink is in iproute2 package and it has extack support. See 'git log lib/libnetlink.c'

Quoted 2 lines

Dave, devlink uses libmnl.

Now I remember. You wrote it independently and but needed iproute2 be a delivery vehicle. It uses none of the common infrastructure from iproute2. Could we make this more difficult ....

Sometime in the next day I will jump through the hoops to get a proper devlink command.

Quoted 16 lines · Wed, Jan 03, 2018 at 07:29:46PM CET, [email protected] wrote:

Quoted 15 lines · Jiri Pirko

Quoted 13 lines · Wed, Jan 03, 2018 at 07:14:16PM CET, [email protected] wrote:

Quoted 9 lines · Arkadi Sharshevsky

As I stated this is a user-space bug which I fixed, and updated my repo so please pull. Devlink uses mnl,and currently mnl does not support extended ack. I added support for this in my local ver of libmnl:

https://github.com/arkadis/libmnl.git

On branch master, so you can check it out. Besides this bugs, which were userspace, can please specify what are the pending problems from your point of view? Thanks!

devlink is in iproute2 package and it has extack support. See 'git log lib/libnetlink.c'

Quoted 6 lines

Quoted 2 lines

Dave, devlink uses libmnl.

Now I remember. You wrote it independently and but needed iproute2 be a delivery vehicle. It uses none of the common infrastructure from iproute2. Could we make this more difficult ....

Feel free to rewrite it to use lib/libnetlink.c. Should not be that hard. Note that at the time I was pushing devlink userspace, tipc also used libmnl as a part of iproute2, so devlink was not the first one. That is why I decided not to rewrite.

As of the rest of the "common infrastructure", what exactly do you have in mind?

This is what I am getting at. Apparently, these resource patches for devlink require a patched libmnl to work properly. It is wrong for iproute2 to accept this patch and to build a devlink command that we know does not work. That means iproute2 needs a dependency on a specific version of libmnl - a version which does not yet exist because those changes have not been accepted and libmnl version released. Adding that dependency is going to inconvenience to all current users of the iproute2 repo.

Quoted 29 lines · Jiri Pirko

Quoted 16 lines · Wed, Jan 03, 2018 at 07:29:46PM CET, [email protected] wrote:

Quoted 15 lines · Jiri Pirko

Quoted 13 lines · Wed, Jan 03, 2018 at 07:14:16PM CET, [email protected] wrote:

Quoted 9 lines · Arkadi Sharshevsky

As I stated this is a user-space bug which I fixed, and updated my repo so please pull. Devlink uses mnl,and currently mnl does not support extended ack. I added support for this in my local ver of libmnl:

https://github.com/arkadis/libmnl.git

On branch master, so you can check it out. Besides this bugs, which were userspace, can please specify what are the pending problems from your point of view? Thanks!

devlink is in iproute2 package and it has extack support. See 'git log lib/libnetlink.c'

Quoted 6 lines

Quoted 2 lines

Dave, devlink uses libmnl.

Now I remember. You wrote it independently and but needed iproute2 be a delivery vehicle. It uses none of the common infrastructure from iproute2. Could we make this more difficult ....

Feel free to rewrite it to use lib/libnetlink.c. Should not be that hard. Note that at the time I was pushing devlink userspace, tipc also used libmnl as a part of iproute2, so devlink was not the first one. That is why I decided not to rewrite.

Quoted 3 lines

As of the rest of the "common infrastructure", what exactly do you have in mind?

This is what I am getting at. Apparently, these resource patches for devlink require a patched libmnl to work properly. It is wrong for

iproute2 to accept this patch and to build a devlink command that we know does not work. That means iproute2 needs a dependency on a specific version of libmnl - a version which does not yet exist because those changes have not been accepted and libmnl version released. Adding that dependency is going to inconvenience to all current users of the iproute2 repo.

Quoted 9 lines

Quoted 9 lines

Quoted 3 lines

Now I remember. You wrote it independently and but needed iproute2 be a delivery vehicle. It uses none of the common infrastructure from iproute2. Could we make this more difficult ....

Feel free to rewrite it to use lib/libnetlink.c. Should not be that hard. Note that at the time I was pushing devlink userspace, tipc also used libmnl as a part of iproute2, so devlink was not the first one. That is why I decided not to rewrite.

Quoted 6 lines

Quoted 3 lines

As of the rest of the "common infrastructure", what exactly do you have in mind?

This is what I am getting at. Apparently, these resource patches for devlink require a patched libmnl to work properly. It is wrong for

What do you mean, "work properly". If there is no patched libmnl, only thing what happens is that the extack won't be processed and shown. That is the same behaviour as if you compile iproute2 package without libmnl - all works, used just don't see extack messages.

I don't see any problem in that. Do you?

Quoted 21 lines · Jiri Pirko

Quoted 9 lines

Quoted 9 lines

Quoted 3 lines

Now I remember. You wrote it independently and but needed iproute2 be a delivery vehicle. It uses none of the common infrastructure from iproute2. Could we make this more difficult ....

Feel free to rewrite it to use lib/libnetlink.c. Should not be that hard. Note that at the time I was pushing devlink userspace, tipc also used libmnl as a part of iproute2, so devlink was not the first one. That is why I decided not to rewrite.

Quoted 6 lines

Quoted 3 lines

As of the rest of the "common infrastructure", what exactly do you have in mind?

This is what I am getting at. Apparently, these resource patches for devlink require a patched libmnl to work properly. It is wrong for

What do you mean, "work properly". If there is no patched libmnl, only thing what happens is that the extack won't be processed and shown. That is the same behaviour as if you compile iproute2 package without libmnl - all works, used just don't see extack messages.

Quoted 1 line

I don't see any problem in that. Do you?

I have libmnl installed. I build iproute2. I don't get extended ack messages on devlink failures. That is a problem. That is building a command that is known not to work as it should given the build dependencies.

Quoted 8 lines · Jiri Pirko

Quoted 7 lines · Wed, Jan 03, 2018 at 07:14:16PM CET, [email protected] wrote:

Quoted 6 lines · Arkadi Sharshevsky

As I stated this is a user-space bug which I fixed, and updated my repo so please pull. Devlink uses mnl,and currently mnl does not support extended ack. I added support for this in my local ver of libmnl:

https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Farkadis%2Flibmnl.git&data=02%7C01%7Carkadis%40mellanox.com%7C5c86b6240eb84459c6ae08d552d7f9a4%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636506009929977440&sdata=sgrNzMhPwe63BIVxexZTjl%2FXqW51kpuRiHVhTDNaa70%3D&reserved=0

Quoted 9 lines

Quoted 6 lines

Quoted 3 lines

On branch master, so you can check it out. Besides this bugs, which were userspace, can please specify what are the pending problems from your point of view? Thanks!

devlink is in iproute2 package and it has extack support. See 'git log lib/libnetlink.c'

Dave, devlink uses libmnl.

Now I remember. You wrote it independently and but needed iproute2 be a delivery vehicle. It uses none of the common infrastructure from iproute2. Could we make this more difficult ....

Sometime in the next day I will jump through the hoops to get a proper devlink command.

Quoted 31 lines · David Ahern

Quoted 12 lines · Arkadi Sharshevsky

Just to summarize the current fixes required:

1. ERIF dpipe table size is reporting wrong size. More precisely the ERIF table does not take rifs, so it should not be linked to the rif bank resource (is not part of this patchset, future extension).
2. Extended ACK user-space bug.
3. ABI documentation- Not sure we agreed upon it, Jiri?

If I missed something please respond. Nothing of the fixes mentioned above is relevant for this patchset actually.

Can you fix the userspace command and then we come back to what else is needed? Right now, it is hard to tell what is a user space bug and what is a kernel space bug.

For example:

$ devlink resource set pci/0000:03:00.0 path /kvd/linear size 10000
$ 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

The set command did not fail, yet there is no size_new arg in the output like there is for this change:

Quoted 9 lines

$ devlink resource set pci/0000:03:00.0 path /kvd/linear size 0
$ 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 size_new 0 occ 0
    name hash_double size 60416
    name hash_single size 87040

As I stated this is a user-space bug which I fixed, and updated my repo so please pull. Devlink uses mnl,and currently mnl does not support extended ack. I added support for this in my local ver of libmnl:

https://github.com/arkadis/libmnl.git

On branch master, so you can check it out. Besides this bugs, which were userspace, can please specify what are the pending problems from your point of view? Thanks!

Quoted 48 lines · Arkadi Sharshevsky

Quoted 31 lines · David Ahern

Quoted 12 lines · Arkadi Sharshevsky

Just to summarize the current fixes required:

1. ERIF dpipe table size is reporting wrong size. More precisely the ERIF table does not take rifs, so it should not be linked to the rif bank resource (is not part of this patchset, future extension).
2. Extended ACK user-space bug.
3. ABI documentation- Not sure we agreed upon it, Jiri?

If I missed something please respond. Nothing of the fixes mentioned above is relevant for this patchset actually.

Can you fix the userspace command and then we come back to what else is needed? Right now, it is hard to tell what is a user space bug and what is a kernel space bug.

For example:

$ devlink resource set pci/0000:03:00.0 path /kvd/linear size 10000
$ 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

The set command did not fail, yet there is no size_new arg in the output like there is for this change:

Quoted 9 lines

$ devlink resource set pci/0000:03:00.0 path /kvd/linear size 0
$ 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 size_new 0 occ 0
    name hash_double size 60416
    name hash_single size 87040

As I stated this is a user-space bug which I fixed, and updated my repo so please pull. Devlink uses mnl,and currently mnl does not support extended ack. I added support for this in my local ver of libmnl:

Quoted 6 lines

https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Farkadis%2Flibmnl.git&data=02%7C01%7Carkadis%40mellanox.com%7C9a369b54cdec48a5e1d208d5531adbdb%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636506297202356822&sdata=6KOkBz5PHqu6z8nlexSdggZj42LE4geiYVFA%2BgcgaPE%3D&reserved=0

On branch master, so you can check it out. Besides this bugs, which were userspace, can please specify what are the pending problems from your point of view? Thanks!

Again, my comments all stem from user experience.

Can you explain what "double_word" means for a unit? I would expect a units to be kB or count (or items or entries).

$ devlink resource show pci/0000:03:00.0
pci/0000:03:00.0:
  name kvd size 245760 unit double_word size_valid true
  resources:
    name linear size 98304 occ 0 unit double_word
    name hash_double size 60416 unit double_word
    name hash_single size 87040 unit double_word

While that is confusing here from the userspace command it goes hand in hand with patch 2 and:

(quoted diff) +3 -0

+enum devlink_resource_unit {
+	DEVLINK_RESOURCE_UNIT_DOUBLE_WORD,
+};

Also, it seems like the occ of 0 is wrong since we know from past responses that if I set linear to 0 all of networking breaks.

How does a user learn the granularity of a resource:

$ devlink resource set pci/0000:03:00.0 path /kvd/hash_double size 50000
Error: mlxsw_spectrum: resource set with wrong granularity.

Try again with 51000 and then 52000 and ... Why not export the granularity read-only? I don't see it in the proposed UAPI.

And then on the reload:

$ devlink reload pci/0000:03:00.0
Error: devlink: resources size validation failed.

Since the reload is not doing any resource sizing that error message is confusing. Maybe something like "Sum of the resource components exceeds total size."

Finally, I still contend a 1-line description of each of the resources goes a long way to improving the user friendliness of this set.

Double word is 64 bit, dont understand why this is confusing.

Quoted 5 lines

Again, my comments all stem from user experience.

Can you explain what "double_word" means for a unit? I would expect a units to be kB or count (or items or entries).

Double word is 64 bit, dont understand why this is confusing.

Quoted 17 lines

$ devlink resource show pci/0000:03:00.0
pci/0000:03:00.0:
  name kvd size 245760 unit double_word size_valid true
  resources:
    name linear size 98304 occ 0 unit double_word
    name hash_double size 60416 unit double_word
    name hash_single size 87040 unit double_word

While that is confusing here from the userspace command it goes hand in hand with patch 2 and:

(quoted diff) +3 -0

+enum devlink_resource_unit {
+	DEVLINK_RESOURCE_UNIT_DOUBLE_WORD,
+};

Quoted 2 lines

Also, it seems like the occ of 0 is wrong since we know from past responses that if I set linear to 0 all of networking breaks.

You are clearly misunderstanding what is occupancy of the resource and what kvd linear is good for. As I mentioned in the last response kvd linear is mapped to adjacency table. So in case its 0 no nexthop routes could be configured, this information is provided by the dpipe<-->resource.

Occupancy means how much of the resource is used right now, why is this wrong? and how its related to the size 0 exactly?

Quoted 9 lines

How does a user learn the granularity of a resource:

$ devlink resource set pci/0000:03:00.0 path /kvd/hash_double size 50000
Error: mlxsw_spectrum: resource set with wrong granularity.

Quoted 3 lines

Try again with 51000 and then 52000 and ... Why not export the granularity read-only? I don't see it in the proposed UAPI.

I would like more adding the granularity size to the extack string instead of adding this to UAPI. The user will try to update once and will get the required granularity in the error message.

Quoted 11 lines

And then on the reload:

$ devlink reload pci/0000:03:00.0
Error: devlink: resources size validation failed.

Since the reload is not doing any resource sizing that error message is confusing. Maybe something like "Sum of the resource components exceeds total size."

No problem, sounds better.

Quoted 4 lines

Finally, I still contend a 1-line description of each of the resources goes a long way to improving the user friendliness of this set.

Strongly against it.

Quoted 7 lines · Arkadi Sharshevsky

Quoted 5 lines

Again, my comments all stem from user experience.

Can you explain what "double_word" means for a unit? I would expect a units to be kB or count (or items or entries).

Double word is 64 bit, dont understand why this is confusing.

As Andrew pointed out, double word can have a range of sizes.

To my point here, a 'unit' for a number should not be the number of bits it is represented by. I believe all of the kvd sizes are in entries (ie., a linear size of 98304 means I can have 98,304 entries in that resource).

Quoted 26 lines

Quoted 17 lines

$ devlink resource show pci/0000:03:00.0
pci/0000:03:00.0:
  name kvd size 245760 unit double_word size_valid true
  resources:
    name linear size 98304 occ 0 unit double_word
    name hash_double size 60416 unit double_word
    name hash_single size 87040 unit double_word

While that is confusing here from the userspace command it goes hand in hand with patch 2 and:

(quoted diff) +3 -0

+enum devlink_resource_unit {
+	DEVLINK_RESOURCE_UNIT_DOUBLE_WORD,
+};

Quoted 2 lines

Also, it seems like the occ of 0 is wrong since we know from past responses that if I set linear to 0 all of networking breaks.

You are clearly misunderstanding what is occupancy of the resource and what kvd linear is good for. As I mentioned in the last response kvd linear is mapped to adjacency table. So in case its 0 no nexthop routes could be configured, this information is provided by the dpipe<-->resource.

Quoted 2 lines

Occupancy means how much of the resource is used right now, why is this wrong? and how its related to the size 0 exactly?

The summary line above shows the current kvd/linear occupancy is '0'. That suggests my L3 only deployment is not using any kvd/linear which means I can set its allocation to 0 and devote more kvd resources to the hash regions.

But, as I pointed out in previous responses I can not set linear to 0. If I do all of networking breaks. That suggests that kvd/linear is used by more networking entities than you are currently reporting. Hence, telling me the kvd/linear occupancy is 0 is wrong.

I believe the stems from the how you are determining occupancy and the fact that not all tables have been implemented. Showing the current occupancy of a resource is very helpful, so it should be part of the API.

However, until mlxsw shows a proper value (either by implementing all dpipe tables or changing how it is calculated) it should not be returning that attribute. As it stands you are giving a user invalid data.

Quoted 9 lines

Quoted 9 lines

How does a user learn the granularity of a resource:

$ devlink resource set pci/0000:03:00.0 path /kvd/hash_double size 50000
Error: mlxsw_spectrum: resource set with wrong granularity.

Quoted 7 lines

Quoted 3 lines

Try again with 51000 and then 52000 and ... Why not export the granularity read-only? I don't see it in the proposed UAPI.

I would like more adding the granularity size to the extack string instead of adding this to UAPI. The user will try to update once and will get the required granularity in the error message.

A user should not have to run a command to get an error message to know essential data to running a command with the right value. Further, do not assume 'user' is a person or the devlink command.

Since granularity is essential to running a valid command, it should be an attribute for each resource.

Quoted 22 lines

Quoted 12 lines

And then on the reload:

$ devlink reload pci/0000:03:00.0
Error: devlink: resources size validation failed.

Since the reload is not doing any resource sizing that error message is confusing. Maybe something like "Sum of the resource components exceeds total size."

No problem, sounds better.

Quoted 4 lines

Finally, I still contend a 1-line description of each of the resources goes a long way to improving the user friendliness of this set.

Strongly against it.

Quoted 59 lines · David Ahern

Quoted 7 lines · Arkadi Sharshevsky

Quoted 5 lines

Again, my comments all stem from user experience.

Can you explain what "double_word" means for a unit? I would expect a units to be kB or count (or items or entries).

Double word is 64 bit, dont understand why this is confusing.

As Andrew pointed out, double word can have a range of sizes.

To my point here, a 'unit' for a number should not be the number of bits it is represented by. I believe all of the kvd sizes are in entries (ie., a linear size of 98304 means I can have 98,304 entries in that resource).

Quoted 26 lines

Quoted 17 lines

$ devlink resource show pci/0000:03:00.0
pci/0000:03:00.0:
  name kvd size 245760 unit double_word size_valid true
  resources:
    name linear size 98304 occ 0 unit double_word
    name hash_double size 60416 unit double_word
    name hash_single size 87040 unit double_word

While that is confusing here from the userspace command it goes hand in hand with patch 2 and:

(quoted diff) +3 -0

+enum devlink_resource_unit {
+	DEVLINK_RESOURCE_UNIT_DOUBLE_WORD,
+};

Quoted 2 lines

Also, it seems like the occ of 0 is wrong since we know from past responses that if I set linear to 0 all of networking breaks.

You are clearly misunderstanding what is occupancy of the resource and what kvd linear is good for. As I mentioned in the last response kvd linear is mapped to adjacency table. So in case its 0 no nexthop routes could be configured, this information is provided by the dpipe<-->resource.

Quoted 2 lines

Occupancy means how much of the resource is used right now, why is this wrong? and how its related to the size 0 exactly?

The summary line above shows the current kvd/linear occupancy is '0'. That suggests my L3 only deployment is not using any kvd/linear which means I can set its allocation to 0 and devote more kvd resources to the hash regions.

But, as I pointed out in previous responses I can not set linear to 0. If I do all of networking breaks. That suggests that kvd/linear is used by more networking entities than you are currently reporting. Hence, telling me the kvd/linear occupancy is 0 is wrong.

I believe the stems from the how you are determining occupancy and the fact that not all tables have been implemented. Showing the current occupancy of a resource is very helpful, so it should be part of the API.

Quoted 4 lines

However, until mlxsw shows a proper value (either by implementing all dpipe tables or changing how it is calculated) it should not be returning that attribute. As it stands you are giving a user invalid data.

Wait, the occupancy is very precise it goes directly to the linear allocator inside the driver and gives exact current usage. You assumed it goes to the dpipe tables which is incorrect.

The linear kvd memory is used by:
1. The adjacency table is responsible for the ECMP and nexthop resolution.
2. ACL flexible actions blocks (dpipe will contain table for this).

I dont know what you configured but in case you got some remote routes with a nexthop the occ should not be zero.

If your router contains only local routes and dont use ACL you can shrink it to zero, no problem.

Quoted 22 lines

Quoted 10 lines

Quoted 10 lines

How does a user learn the granularity of a resource:

$ devlink resource set pci/0000:03:00.0 path /kvd/hash_double size 50000
Error: mlxsw_spectrum: resource set with wrong granularity.

Quoted 7 lines

Quoted 3 lines

Try again with 51000 and then 52000 and ... Why not export the granularity read-only? I don't see it in the proposed UAPI.

I would like more adding the granularity size to the extack string instead of adding this to UAPI. The user will try to update once and will get the required granularity in the error message.

A user should not have to run a command to get an error message to know essential data to running a command with the right value. Further, do not assume 'user' is a person or the devlink command.

Quoted 3 lines

Since granularity is essential to running a valid command, it should be an attribute for each resource.

This is also your approach towards the min/max for resource sizes? Am I correct?

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Also, it seems like the occ of 0 is wrong since we know from past responses that if I set linear to 0 all of networking breaks.