Hi Jiri,
On Thu, Feb 16, 2017 at 04:22:37PM +0100, Jiri Pirko wrote:
> From: Arkadi Sharshevsky <arka...@mellanox.com>
>
> The pipeline debug is used to export the pipeline abstractions
> for the main objects - tables, headers and entries. The only support for
> set is for changing the counter parameter on specific table.
>
> The basic structures:
>
> Header - can represent a real protocol header information or internal
> metadata. Generic protocol headers like IPv4 can be shared
> between drivers. Each driver can add local headers.
>
> Field - part of a header. Can represent protocol field or specific
> ASIC metadata field. Hardware special metadata fields can
> be mapped to different resources, for example switch ASIC
> ports can have internal number which from the systems
> point of view is mapped to netdeivce ifindex.
>
> Hfield - Specific combination of header:field. This object is used
> to represent the table behavior in terms of match/action.
>
> Hfield_val - Represents value of specific Hfield.
>
> Table - represents a hardware block which can be described with
> match/action behavior. The match/action can be done on the
> packets data or on the internal metadata that it gathered
> along the packets traversal throw the pipeline which is vendor
> specific and should be exported in order to provide
> understanding of ASICs behavior.
>
> Entry - represents single record in a specific table. The entry is
> identified by specific combination of Hfield_vals for match
> /action.
>
> Prior to accessing the tables/entries the drivers provide the header/
> field data base which is used by driver to user-space. The data base
> is split between the shared headers and unique headers.
Thanks for posting this. In general I think it looks quite promising.
After a first pass over the code I have the following
specific comments:
>
> Signed-off-by: Arkadi Sharshevsky <arka...@mellanox.com>
> Signed-off-by: Jiri Pirko <j...@mellanox.com>
> ---
> include/net/devlink.h | 224 ++++++++++++-
> include/uapi/linux/devlink.h | 50 ++-
> net/core/devlink.c | 747
> +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1019 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/devlink.h b/include/net/devlink.h
...
> +/**
> + * struct devlink_dpipe_entry - table entry object
> + * @index: index of the entry in the table
> + * @matches: tuple match values
> + * @matches_count: count of matches tuples
> + * @actions: tuple actions values
> + * @actions_count: count of actions values
> + * @counter: value of counter
> + * @counter_valid: Specify if value is valid from hardware
> + */
> +struct devlink_dpipe_entry {
> + unsigned int index;
I'm not sure what I understand what index is but I assume that it is a
unique identifier for the flow within the table. From the point of view
of having enough indexes for all entries in the table an unsigned int seems
adequate. But I see use-cases that have significantly wider identifiers.
I'm wondering what your thoughts are on supporting wider identifiers.
Perhaps they belong in the match?
> + struct devlink_dpipe_hfield_val *matches;
> + unsigned int matches_count;
> + struct devlink_dpipe_hfield_val *actions;
> + unsigned int actions_count;
> + u64 counter;
I'm unclear on what counter is. Is it the number of times the action entry
has been used (hit)? If so I think some provision for richer per-hit
counters for entries would be useful. At least number of hits and number of
bytes. But perhaps it would be useful to allow hardware to describe its
per-entry counters?
> + bool counter_valid;
> +};
...
