Add support for devlink DPIPE tables #3
Conversation
Extend this function arguments list in order to support passing netlink data (data []byte). Signed-off-by: Alexander Petrovskiy <alexpe@mellanox.com>
Add DpipeTables() function to retrieve a list of all supported DPIPE tables for a specified device. The following information is collected per DPIPE table: - devlink bus - devlink device - table name - table size - table counters_enabled flag More info about DPIPE: https://github.com/Mellanox/mlxsw/wiki/Pipeline-Debugging-(DPIPE) Signed-off-by: Alexander Petrovskiy <alexpe@mellanox.com>
Signed-off-by: Alexander Petrovskiy <alexpe@mellanox.com>
|
I also added an integration test for DPIPE just to verify that it's working. Looking forward to hear your feedback about this. Should I just remove this test or rework it somehow? |
There was a problem hiding this comment.
We should also have a unit test for DPIPE tables just like what TestLinuxClientDevicesOK is doing. You can copy that for the structure. It simulates a netlink connection and verifies the correct encoding and decoding of attributes as a sanity check for environments that you don't want to manipulate your actual operating system via netlink.
client_linux_integration_test.go
Outdated
| tables = append(tables, tt...) | ||
| } | ||
|
|
||
| if len(tables) == 0 { |
There was a problem hiding this comment.
I think it's okay to remove this check and just print the tables that were found. These "integration" tests are more so just a sanity check of the actual devlink interface at the moment. If there is a simulator kernel module of some sort we could use in CI though, that would be very helpful. Do you know of anything like that?
There was a problem hiding this comment.
Yes, there is a netdevsim module available, but it doesn't support DPIPE yet.
We can probably use it to test some other devlink features like devlink trap, for example.
More details on netdevsim and devlink trap:
https://cateee.net/lkddb/web-lkddb/NETDEVSIM.html
https://github.com/torvalds/linux/blob/master/Documentation/networking/devlink-trap.rst
https://github.com/jeffmerkey/leaf-linux/blob/master/tools/testing/selftests/drivers/net/netdevsim/devlink_trap.sh
There was a problem hiding this comment.
Awesome! I'll open an issue to look into this.
Signed-off-by: Alexander Petrovskiy <alexpe@mellanox.com>
Signed-off-by: Alexander Petrovskiy <alexpe@mellanox.com>
Change it to accept 'ae *netlink.AttributeEncoder' as an optional argument to pass 'ae' with data to encode for netlink message instead of data itself. Signed-off-by: Alexander Petrovskiy <alexpe@mellanox.com>
Signed-off-by: Alexander Petrovskiy <alexpe@mellanox.com>
Signed-off-by: Alexander Petrovskiy <alexpe@mellanox.com>
Just print all DPIPE tables that are available. Signed-off-by: Alexander Petrovskiy <alexpe@mellanox.com>
|
Thank you for your review! I updated the PR. |
There was a problem hiding this comment.
Looking good overall! See my previous comment about adding a unit test for parsing out the DPIPE structures: #3 (review).
Add DpipeTables() function to retrieve a list of all supported DPIPE tables for a specified device.
The following information is collected per DPIPE table:
More info about DPIPE:
https://github.com/Mellanox/mlxsw/wiki/Pipeline-Debugging-(DPIPE)