| Message ID | 20171114161852.6633-9-jiri@resnulli.us |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | Add support for resource abstraction | expand |
On 11/14/17 9:18 AM, Jiri Pirko wrote: > From: Arkadi Sharshevsky <arkadis@mellanox.com> > > Connect current dpipe tables to resources. The tables are connected > in the following fashion: > 1. IPv4 host - KVD hash single > 2. IPv6 host - KVD hash double > 3. Adjacency - KVD linear Those descriptions would be helpful to the user. A description attribute for the resources?
On 11/18/2017 09:19 PM, David Ahern wrote: > On 11/14/17 9:18 AM, Jiri Pirko wrote: >> From: Arkadi Sharshevsky <arkadis@mellanox.com> >> >> Connect current dpipe tables to resources. The tables are connected >> in the following fashion: >> 1. IPv4 host - KVD hash single >> 2. IPv6 host - KVD hash double >> 3. Adjacency - KVD linear > > Those descriptions would be helpful to the user. A description attribute > for the resources? > As described in the cover letter this resources are used by the majority of the ASICs lookup processes. So currently there is one to one mapping but is should increase as more tables are exposed, so I don't think its a good idea to maintain such an attribute.
On 11/19/17 2:16 AM, Arkadi Sharshevsky wrote: > > > On 11/18/2017 09:19 PM, David Ahern wrote: >> On 11/14/17 9:18 AM, Jiri Pirko wrote: >>> From: Arkadi Sharshevsky <arkadis@mellanox.com> >>> >>> Connect current dpipe tables to resources. The tables are connected >>> in the following fashion: >>> 1. IPv4 host - KVD hash single >>> 2. IPv6 host - KVD hash double >>> 3. Adjacency - KVD linear >> >> Those descriptions would be helpful to the user. A description attribute >> for the resources? >> > > As described in the cover letter this resources are used by the > majority of the ASICs lookup processes. So currently there is one > to one mapping but is should increase as more tables are exposed, > so I don't think its a good idea to maintain such an attribute. > 'IPv4 host' yes, but I mean the term 'KVD hash single'? Is it the same across all h/w vendors? I have only seen that in the context of MLX. If it is a MLX term then a description to the user that KVD hash single == IPv4 host is warranted.
On 11/19/2017 05:58 PM, David Ahern wrote: > On 11/19/17 2:16 AM, Arkadi Sharshevsky wrote: >> >> >> On 11/18/2017 09:19 PM, David Ahern wrote: >>> On 11/14/17 9:18 AM, Jiri Pirko wrote: >>>> From: Arkadi Sharshevsky <arkadis@mellanox.com> >>>> >>>> Connect current dpipe tables to resources. The tables are connected >>>> in the following fashion: >>>> 1. IPv4 host - KVD hash single >>>> 2. IPv6 host - KVD hash double >>>> 3. Adjacency - KVD linear >>> >>> Those descriptions would be helpful to the user. A description attribute >>> for the resources? >>> >> >> As described in the cover letter this resources are used by the >> majority of the ASICs lookup processes. So currently there is one >> to one mapping but is should increase as more tables are exposed, >> so I don't think its a good idea to maintain such an attribute. >> > > 'IPv4 host' yes, but I mean the term 'KVD hash single'? Is it the same > across all h/w vendors? I have only seen that in the context of MLX. If > it is a MLX term then a description to the user that KVD hash single == > IPv4 host is warranted. > But this relation is wrong, there is no equality here. The LPM, FDB and VID to FID mapping are all can be modeled as lookup tables (via dpipe) that use KVD hash single resource. This description string will grow very long. I dont think this is the right place to document such thing, eitherway, the user can dump the dpipe tables and see which is mapped to what resource.
On 11/23/17 6:40 AM, Arkadi Sharshevsky wrote: > > > On 11/19/2017 05:58 PM, David Ahern wrote: >> On 11/19/17 2:16 AM, Arkadi Sharshevsky wrote: >>> >>> >>> On 11/18/2017 09:19 PM, David Ahern wrote: >>>> On 11/14/17 9:18 AM, Jiri Pirko wrote: >>>>> From: Arkadi Sharshevsky <arkadis@mellanox.com> >>>>> >>>>> Connect current dpipe tables to resources. The tables are connected >>>>> in the following fashion: >>>>> 1. IPv4 host - KVD hash single >>>>> 2. IPv6 host - KVD hash double >>>>> 3. Adjacency - KVD linear >>>> >>>> Those descriptions would be helpful to the user. A description attribute >>>> for the resources? >>>> >>> >>> As described in the cover letter this resources are used by the >>> majority of the ASICs lookup processes. So currently there is one >>> to one mapping but is should increase as more tables are exposed, >>> so I don't think its a good idea to maintain such an attribute. >>> >> >> 'IPv4 host' yes, but I mean the term 'KVD hash single'? Is it the same >> across all h/w vendors? I have only seen that in the context of MLX. If >> it is a MLX term then a description to the user that KVD hash single == >> IPv4 host is warranted. >> > > But this relation is wrong, there is no equality here. The LPM, FDB and > VID to FID mapping are all can be modeled as lookup tables (via dpipe) > that use KVD hash single resource. > > This description string will grow very long. I dont think this is the > right place to document such thing, eitherway, the user can dump the > dpipe tables and see which is mapped to what resource. Users should not have to find a PRM or user guide for *each version of their hardware* to program something so fundamental. This is software. We can make it user friendly. Use of vendor specific terms is fine -- allows correlation to vendor docs. But there should also be text to help the user correlate vendor terms to generic industry terms.
On 11/27/2017 06:12 PM, David Ahern wrote: > On 11/23/17 6:40 AM, Arkadi Sharshevsky wrote: >> >> >> On 11/19/2017 05:58 PM, David Ahern wrote: >>> On 11/19/17 2:16 AM, Arkadi Sharshevsky wrote: >>>> >>>> >>>> On 11/18/2017 09:19 PM, David Ahern wrote: >>>>> On 11/14/17 9:18 AM, Jiri Pirko wrote: >>>>>> From: Arkadi Sharshevsky <arkadis@mellanox.com> >>>>>> >>>>>> Connect current dpipe tables to resources. The tables are connected >>>>>> in the following fashion: >>>>>> 1. IPv4 host - KVD hash single >>>>>> 2. IPv6 host - KVD hash double >>>>>> 3. Adjacency - KVD linear >>>>> >>>>> Those descriptions would be helpful to the user. A description attribute >>>>> for the resources? >>>>> >>>> >>>> As described in the cover letter this resources are used by the >>>> majority of the ASICs lookup processes. So currently there is one >>>> to one mapping but is should increase as more tables are exposed, >>>> so I don't think its a good idea to maintain such an attribute. >>>> >>> >>> 'IPv4 host' yes, but I mean the term 'KVD hash single'? Is it the same >>> across all h/w vendors? I have only seen that in the context of MLX. If >>> it is a MLX term then a description to the user that KVD hash single == >>> IPv4 host is warranted. >>> >> >> But this relation is wrong, there is no equality here. The LPM, FDB and >> VID to FID mapping are all can be modeled as lookup tables (via dpipe) >> that use KVD hash single resource. >> >> This description string will grow very long. I dont think this is the >> right place to document such thing, eitherway, the user can dump the >> dpipe tables and see which is mapped to what resource. > > Users should not have to find a PRM or user guide for *each version of > their hardware* to program something so fundamental. This is software. I still don't understand, you can dump the dpipe table to see which tables use which resource. Why I need this redundant documentation string in the kernel? > We can make it user friendly. Use of vendor specific terms is fine -- > allows correlation to vendor docs. But there should also be text to help IMHO such documentation strings should not be in the kernel. > the user correlate vendor terms to generic industry terms. > Really you want to add DEVLINK_RESOURCE_DESCRIPTION string attributed? " Used by the following hardware tables - VID-to-FID - LPM - FDB - HOST ... " Again I think it is redundant, just dump those tables. No need to use user-guides nor PRM.
Mon, Nov 27, 2017 at 05:12:47PM CET, dsa@cumulusnetworks.com wrote: >On 11/23/17 6:40 AM, Arkadi Sharshevsky wrote: >> >> >> On 11/19/2017 05:58 PM, David Ahern wrote: >>> On 11/19/17 2:16 AM, Arkadi Sharshevsky wrote: >>>> >>>> >>>> On 11/18/2017 09:19 PM, David Ahern wrote: >>>>> On 11/14/17 9:18 AM, Jiri Pirko wrote: >>>>>> From: Arkadi Sharshevsky <arkadis@mellanox.com> >>>>>> >>>>>> Connect current dpipe tables to resources. The tables are connected >>>>>> in the following fashion: >>>>>> 1. IPv4 host - KVD hash single >>>>>> 2. IPv6 host - KVD hash double >>>>>> 3. Adjacency - KVD linear >>>>> >>>>> Those descriptions would be helpful to the user. A description attribute >>>>> for the resources? >>>>> >>>> >>>> As described in the cover letter this resources are used by the >>>> majority of the ASICs lookup processes. So currently there is one >>>> to one mapping but is should increase as more tables are exposed, >>>> so I don't think its a good idea to maintain such an attribute. >>>> >>> >>> 'IPv4 host' yes, but I mean the term 'KVD hash single'? Is it the same >>> across all h/w vendors? I have only seen that in the context of MLX. If >>> it is a MLX term then a description to the user that KVD hash single == >>> IPv4 host is warranted. >>> >> >> But this relation is wrong, there is no equality here. The LPM, FDB and >> VID to FID mapping are all can be modeled as lookup tables (via dpipe) >> that use KVD hash single resource. >> >> This description string will grow very long. I dont think this is the >> right place to document such thing, eitherway, the user can dump the >> dpipe tables and see which is mapped to what resource. > >Users should not have to find a PRM or user guide for *each version of >their hardware* to program something so fundamental. This is software. >We can make it user friendly. Use of vendor specific terms is fine -- >allows correlation to vendor docs. But there should also be text to help >the user correlate vendor terms to generic industry terms. I have to be missing something. You can easily see the relation between each dpipe table and resources already as a part of this patchset. The string you suggest shows the same thing, therefore it is completely redundant. What am I missing?
On 11/28/17 6:27 AM, Jiri Pirko wrote: > > I have to be missing something. You can easily see the relation between > each dpipe table and resources already as a part of this patchset. The > string you suggest shows the same thing, therefore it is completely > redundant. What am I missing? > At this point let's see a working RFC v3 and we can discuss from there.
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c index 96fdba7..282fe82 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_dpipe.c @@ -774,11 +774,27 @@ static struct devlink_dpipe_table_ops mlxsw_sp_host4_ops = { static int mlxsw_sp_dpipe_host4_table_init(struct mlxsw_sp *mlxsw_sp) { struct devlink *devlink = priv_to_devlink(mlxsw_sp->core); + int err; - return devlink_dpipe_table_register(devlink, - MLXSW_SP_DPIPE_TABLE_NAME_HOST4, - &mlxsw_sp_host4_ops, - mlxsw_sp, false); + err = devlink_dpipe_table_register(devlink, + MLXSW_SP_DPIPE_TABLE_NAME_HOST4, + &mlxsw_sp_host4_ops, + mlxsw_sp, false); + if (err) + return err; + + err = devlink_dpipe_table_resource_set(devlink, + MLXSW_SP_DPIPE_TABLE_NAME_HOST4, + MLXSW_SP_RESOURCE_KVD_HASH_SINGLE); + if (err) + goto err_resource_set; + + return 0; + +err_resource_set: + devlink_dpipe_table_unregister(devlink, + MLXSW_SP_DPIPE_TABLE_NAME_HOST4); + return err; } static void mlxsw_sp_dpipe_host4_table_fini(struct mlxsw_sp *mlxsw_sp) @@ -832,11 +848,27 @@ static struct devlink_dpipe_table_ops mlxsw_sp_host6_ops = { static int mlxsw_sp_dpipe_host6_table_init(struct mlxsw_sp *mlxsw_sp) { struct devlink *devlink = priv_to_devlink(mlxsw_sp->core); + int err; - return devlink_dpipe_table_register(devlink, - MLXSW_SP_DPIPE_TABLE_NAME_HOST6, - &mlxsw_sp_host6_ops, - mlxsw_sp, false); + err = devlink_dpipe_table_register(devlink, + MLXSW_SP_DPIPE_TABLE_NAME_HOST6, + &mlxsw_sp_host6_ops, + mlxsw_sp, false); + if (err) + return err; + + err = devlink_dpipe_table_resource_set(devlink, + MLXSW_SP_DPIPE_TABLE_NAME_HOST6, + MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE); + if (err) + goto err_resource_set; + + return 0; + +err_resource_set: + devlink_dpipe_table_unregister(devlink, + MLXSW_SP_DPIPE_TABLE_NAME_HOST6); + return err; } static void mlxsw_sp_dpipe_host6_table_fini(struct mlxsw_sp *mlxsw_sp) @@ -1216,11 +1248,27 @@ static struct devlink_dpipe_table_ops mlxsw_sp_dpipe_table_adj_ops = { static int mlxsw_sp_dpipe_adj_table_init(struct mlxsw_sp *mlxsw_sp) { struct devlink *devlink = priv_to_devlink(mlxsw_sp->core); + int err; - return devlink_dpipe_table_register(devlink, - MLXSW_SP_DPIPE_TABLE_NAME_ADJ, - &mlxsw_sp_dpipe_table_adj_ops, - mlxsw_sp, false); + err = devlink_dpipe_table_register(devlink, + MLXSW_SP_DPIPE_TABLE_NAME_ADJ, + &mlxsw_sp_dpipe_table_adj_ops, + mlxsw_sp, false); + if (err) + return err; + + err = devlink_dpipe_table_resource_set(devlink, + MLXSW_SP_DPIPE_TABLE_NAME_ADJ, + MLXSW_SP_RESOURCE_KVD_LINEAR); + if (err) + goto err_resource_set; + + return 0; + +err_resource_set: + devlink_dpipe_table_unregister(devlink, + MLXSW_SP_DPIPE_TABLE_NAME_ADJ); + return err; } static void mlxsw_sp_dpipe_adj_table_fini(struct mlxsw_sp *mlxsw_sp)