Re: [patch net-next RFC v2 07/11] mlxsw: spectrum: Register KVD resources with devlink

2017-11-19 Thread Arkadi Sharshevsky 


On 11/18/2017 09:18 PM, David Ahern wrote:
> On 11/14/17 9:18 AM, Jiri Pirko wrote:
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c 
>> b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>> index d02c130..f0cbd67 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
>> @@ -3927,6 +3927,173 @@ static const struct mlxsw_config_profile 
>> mlxsw_sp_config_profile = {
>>  .resource_query_enable  = 1,
>>  };
>>  
>> +static bool
>> +mlxsw_sp_resource_kvd_granularity_validate(struct netlink_ext_ack *extack,
>> +   u64 size)
>> +{
>> +const struct mlxsw_config_profile *profile;
>> +
>> +profile = _sp_config_profile;
>> +if (size % profile->kvd_hash_granularity) {
>> +NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "resource set with wrong 
>> granularity");
>> +return false;
>> +}
>> +return true;
>> +}
>> +
>> +static int
>> +mlxsw_sp_resource_kvd_size_validate(struct devlink *devlink, u64 size,
>> +struct list_head *resource_list,
>> +struct netlink_ext_ack *extack)
>> +{
>> +struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>> +u32 kvd_size, single_size, double_size, linear_size;
>> +struct devlink_resource *resource;
>> +
>> +kvd_size = MLXSW_CORE_RES_GET(mlxsw_core, KVD_SIZE);
>> +if (kvd_size != size) {
>> +NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "kvd size cannot be 
>> chagned");
> 
> s/chagned/changed/
> 
>> +return -EINVAL;
>> +}
>> +
>> +list_for_each_entry(resource, resource_list, list) {
>> +switch (resource->id) {
>> +case MLXSW_SP_RESOURCE_KVD_LINEAR:
>> +linear_size = resource->size_new;
>> +break;
>> +case MLXSW_SP_RESOURCE_KVD_HASH_SINGLE:
>> +single_size = resource->size_new;
>> +break;
>> +case MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE:
>> +double_size = resource->size_new;
>> +break;
>> +}
>> +}
>> +
>> +/* Overlap is not supported */
>> +if (linear_size + single_size + double_size > kvd_size) {
>> +NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "Overlap is not 
>> supported");
> 
> Overlap? Isn't that sum of the partitions are greater than total size?
> 

In case sum of the partitions is greater than the kvd tot size, the
hash single/double will be set in an overlapping state, which we do
not support currently.

> 
>> +return -EINVAL;
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +static int
>> +mlxsw_sp_resource_kvd_linear_size_validate(struct devlink *devlink, u64 
>> size,
>> +   struct list_head *resource_list,
>> +   struct netlink_ext_ack *extack)
>> +{
>> +if (!mlxsw_sp_resource_kvd_granularity_validate(extack, size))
>> +return -EINVAL;
>> +
>> +return 0;
>> +}
>> +
>> +static int
>> +mlxsw_sp_resource_kvd_hash_single_size_validate(struct devlink *devlink, 
>> u64 size,
>> +struct list_head *resource_list,
>> +struct netlink_ext_ack *extack)
>> +{
>> +struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>> +
>> +if (!mlxsw_sp_resource_kvd_granularity_validate(extack, size))
>> +return -EINVAL;
>> +
>> +if (size < MLXSW_CORE_RES_GET(mlxsw_core, KVD_SINGLE_MIN_SIZE)) {
>> +NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "hash single size is 
>> smaller then min");
> 
> s/then min/than minimium/
> 
>> +return -EINVAL;
>> +}
>> +return 0;
>> +}
>> +
>> +static int
>> +mlxsw_sp_resource_kvd_hash_double_size_validate(struct devlink *devlink, 
>> u64 size,
>> +struct list_head *resource_list,
>> +struct netlink_ext_ack *extack)
>> +{
>> +struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>> +
>> +if (!mlxsw_sp_resource_kvd_granularity_validate(extack, size))
>> +return -EINVAL;
>> +
>> +if (size < MLXSW_CORE_RES_GET(mlxsw_core, KVD_DOUBLE_MIN_SIZE)) {
>> +NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "hash double size is 
>> smaller then min");
> 
> s/then min/than minimium/
> 
> How does the user learn the minimum size and the granularity for the KVD
> resources? Seems like those could be read-only attributes in the
> resource dump to make it easier for the user.
> 

This seems to me as too case specific and I didn't want to add
UAPI attributes for this stuff..

The resource shouldn't be define as only memory based hardware blocks.
I actually plane expose the rifs as resource as well.

I think that if the user try to configure and receives an such error
it is very clear

Re: [patch net-next RFC v2 07/11] mlxsw: spectrum: Register KVD resources with devlink

2017-11-18 Thread David Ahern 
On 11/14/17 9:18 AM, Jiri Pirko wrote:
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c 
> b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> index d02c130..f0cbd67 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> @@ -3927,6 +3927,173 @@ static const struct mlxsw_config_profile 
> mlxsw_sp_config_profile = {
>   .resource_query_enable  = 1,
>  };
>  
> +static bool
> +mlxsw_sp_resource_kvd_granularity_validate(struct netlink_ext_ack *extack,
> +u64 size)
> +{
> + const struct mlxsw_config_profile *profile;
> +
> + profile = _sp_config_profile;
> + if (size % profile->kvd_hash_granularity) {
> + NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "resource set with wrong 
> granularity");
> + return false;
> + }
> + return true;
> +}
> +
> +static int
> +mlxsw_sp_resource_kvd_size_validate(struct devlink *devlink, u64 size,
> + struct list_head *resource_list,
> + struct netlink_ext_ack *extack)
> +{
> + struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> + u32 kvd_size, single_size, double_size, linear_size;
> + struct devlink_resource *resource;
> +
> + kvd_size = MLXSW_CORE_RES_GET(mlxsw_core, KVD_SIZE);
> + if (kvd_size != size) {
> + NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "kvd size cannot be 
> chagned");

s/chagned/changed/

> + return -EINVAL;
> + }
> +
> + list_for_each_entry(resource, resource_list, list) {
> + switch (resource->id) {
> + case MLXSW_SP_RESOURCE_KVD_LINEAR:
> + linear_size = resource->size_new;
> + break;
> + case MLXSW_SP_RESOURCE_KVD_HASH_SINGLE:
> + single_size = resource->size_new;
> + break;
> + case MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE:
> + double_size = resource->size_new;
> + break;
> + }
> + }
> +
> + /* Overlap is not supported */
> + if (linear_size + single_size + double_size > kvd_size) {
> + NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "Overlap is not 
> supported");

Overlap? Isn't that sum of the partitions are greater than total size?


> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +mlxsw_sp_resource_kvd_linear_size_validate(struct devlink *devlink, u64 size,
> +struct list_head *resource_list,
> +struct netlink_ext_ack *extack)
> +{
> + if (!mlxsw_sp_resource_kvd_granularity_validate(extack, size))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int
> +mlxsw_sp_resource_kvd_hash_single_size_validate(struct devlink *devlink, u64 
> size,
> + struct list_head *resource_list,
> + struct netlink_ext_ack *extack)
> +{
> + struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> +
> + if (!mlxsw_sp_resource_kvd_granularity_validate(extack, size))
> + return -EINVAL;
> +
> + if (size < MLXSW_CORE_RES_GET(mlxsw_core, KVD_SINGLE_MIN_SIZE)) {
> + NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "hash single size is 
> smaller then min");

s/then min/than minimium/

> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int
> +mlxsw_sp_resource_kvd_hash_double_size_validate(struct devlink *devlink, u64 
> size,
> + struct list_head *resource_list,
> + struct netlink_ext_ack *extack)
> +{
> + struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> +
> + if (!mlxsw_sp_resource_kvd_granularity_validate(extack, size))
> + return -EINVAL;
> +
> + if (size < MLXSW_CORE_RES_GET(mlxsw_core, KVD_DOUBLE_MIN_SIZE)) {
> + NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "hash double size is 
> smaller then min");

s/then min/than minimium/

How does the user learn the minimum size and the granularity for the KVD
resources? Seems like those could be read-only attributes in the
resource dump to make it easier for the user.

Re: [patch net-next RFC v2 07/11] mlxsw: spectrum: Register KVD resources with devlink

2017-11-15 Thread Arkadi Sharshevsky 


On 11/15/2017 02:15 AM, David Ahern wrote:
> On 11/14/17 9:18 AM, Jiri Pirko wrote:
>> +static int
>> +mlxsw_sp_resource_kvd_size_validate(struct devlink *devlink, u64 size,
>> +struct list_head *resource_list,
>> +struct netlink_ext_ack *extack)
>> +{
>> +struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
>> +u32 kvd_size, single_size, double_size, linear_size;
>> +struct devlink_resource *resource;
>> +
>> +kvd_size = MLXSW_CORE_RES_GET(mlxsw_core, KVD_SIZE);
>> +if (kvd_size != size) {
>> +NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "kvd size cannot be 
>> chagned");
>> +return -EINVAL;
>> +}
>> +
>> +list_for_each_entry(resource, resource_list, list) {
>> +switch (resource->id) {
>> +case MLXSW_SP_RESOURCE_KVD_LINEAR:
>> +linear_size = resource->size_new;
>> +break;
>> +case MLXSW_SP_RESOURCE_KVD_HASH_SINGLE:
>> +single_size = resource->size_new;
>> +break;
>> +case MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE:
>> +double_size = resource->size_new;
>> +break;
>> +}
>> +}
>> +
>> +/* Overlap is not supported */
>> +if (linear_size + single_size + double_size > kvd_size) {
>> +NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "Overlap is not 
>> supported");
>> +return -EINVAL;
>> +}
>> +
>> +return 0;
>> +}
>> +
> 
> gcc warnings due to the above:
> 
> /home/dsa/kernel-3.git/drivers/net/ethernet/mellanox/mlxsw/spectrum.c:
> In function ‘mlxsw_sp_resource_kvd_size_validate’:
> /home/dsa/kernel-3.git/drivers/net/ethernet/mellanox/mlxsw/spectrum.c:3974:32:
> warning: ‘linear_size’ may be used uninitialized in this function
> [-Wmaybe-uninitialized]
>   if (linear_size + single_size + double_size > kvd_size) {
> ^
> /home/dsa/kernel-3.git/drivers/net/ethernet/mellanox/mlxsw/spectrum.c:3950:29:
> warning: ‘double_size’ may be used uninitialized in this function
> [-Wmaybe-uninitialized]
>   u32 kvd_size, single_size, double_size, linear_size;
>  ^
> /home/dsa/kernel-3.git/drivers/net/ethernet/mellanox/mlxsw/spectrum.c:3950:16:
> warning: ‘single_size’ may be used uninitialized in this function
> [-Wmaybe-uninitialized]
>   u32 kvd_size, single_size, double_size, linear_size;
> ^
> 
Thanks, will fix

Re: [patch net-next RFC v2 07/11] mlxsw: spectrum: Register KVD resources with devlink

2017-11-14 Thread David Ahern 
On 11/14/17 9:18 AM, Jiri Pirko wrote:
> +static int
> +mlxsw_sp_resource_kvd_size_validate(struct devlink *devlink, u64 size,
> + struct list_head *resource_list,
> + struct netlink_ext_ack *extack)
> +{
> + struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> + u32 kvd_size, single_size, double_size, linear_size;
> + struct devlink_resource *resource;
> +
> + kvd_size = MLXSW_CORE_RES_GET(mlxsw_core, KVD_SIZE);
> + if (kvd_size != size) {
> + NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "kvd size cannot be 
> chagned");
> + return -EINVAL;
> + }
> +
> + list_for_each_entry(resource, resource_list, list) {
> + switch (resource->id) {
> + case MLXSW_SP_RESOURCE_KVD_LINEAR:
> + linear_size = resource->size_new;
> + break;
> + case MLXSW_SP_RESOURCE_KVD_HASH_SINGLE:
> + single_size = resource->size_new;
> + break;
> + case MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE:
> + double_size = resource->size_new;
> + break;
> + }
> + }
> +
> + /* Overlap is not supported */
> + if (linear_size + single_size + double_size > kvd_size) {
> + NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "Overlap is not 
> supported");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +

gcc warnings due to the above:

/home/dsa/kernel-3.git/drivers/net/ethernet/mellanox/mlxsw/spectrum.c:
In function ‘mlxsw_sp_resource_kvd_size_validate’:
/home/dsa/kernel-3.git/drivers/net/ethernet/mellanox/mlxsw/spectrum.c:3974:32:
warning: ‘linear_size’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
  if (linear_size + single_size + double_size > kvd_size) {
^
/home/dsa/kernel-3.git/drivers/net/ethernet/mellanox/mlxsw/spectrum.c:3950:29:
warning: ‘double_size’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
  u32 kvd_size, single_size, double_size, linear_size;
 ^
/home/dsa/kernel-3.git/drivers/net/ethernet/mellanox/mlxsw/spectrum.c:3950:16:
warning: ‘single_size’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
  u32 kvd_size, single_size, double_size, linear_size;
^

[patch net-next RFC v2 07/11] mlxsw: spectrum: Register KVD resources with devlink

2017-11-14 Thread Jiri Pirko 
From: Arkadi Sharshevsky 

Register the KVD resources with devlink. The KVD is a memory resource
which is subdivided into three partitions which are the linear, hash
single and hash double.

Signed-off-by: Arkadi Sharshevsky 
Signed-off-by: Jiri Pirko 
---
 drivers/net/ethernet/mellanox/mlxsw/core.c |   9 ++
 drivers/net/ethernet/mellanox/mlxsw/core.h |   1 +
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 168 +
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h |  12 ++
 4 files changed, 190 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c 
b/drivers/net/ethernet/mellanox/mlxsw/core.c
index f3315bc..2488662 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1012,6 +1012,12 @@ int mlxsw_core_bus_device_register(const struct 
mlxsw_bus_info *mlxsw_bus_info,
if (err)
goto err_bus_init;
 
+   if (mlxsw_driver->resources_register) {
+   err = mlxsw_driver->resources_register(mlxsw_core);
+   if (err)
+   goto err_register_resources;
+   }
+
err = mlxsw_ports_init(mlxsw_core);
if (err)
goto err_ports_init;
@@ -1067,6 +1073,8 @@ int mlxsw_core_bus_device_register(const struct 
mlxsw_bus_info *mlxsw_bus_info,
 err_ports_init:
mlxsw_bus->fini(bus_priv);
 err_bus_init:
+   devlink_resources_unregister(devlink, NULL);
+err_register_resources:
devlink_free(devlink);
 err_devlink_alloc:
mlxsw_core_driver_put(device_kind);
@@ -1086,6 +1094,7 @@ void mlxsw_core_bus_device_unregister(struct mlxsw_core 
*mlxsw_core)
mlxsw_emad_fini(mlxsw_core);
kfree(mlxsw_core->lag.mapping);
mlxsw_ports_fini(mlxsw_core);
+   devlink_resources_unregister(devlink, NULL);
mlxsw_core->bus->fini(mlxsw_core->bus_priv);
devlink_free(devlink);
mlxsw_core_driver_put(device_kind);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h 
b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 34dda96..e23f83b 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -308,6 +308,7 @@ struct mlxsw_driver {
   u32 *p_cur, u32 *p_max);
void (*txhdr_construct)(struct sk_buff *skb,
const struct mlxsw_tx_info *tx_info);
+   int (*resources_register)(struct mlxsw_core *mlxsw_core);
u8 txhdr_len;
const struct mlxsw_config_profile *profile;
 };
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c 
b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index d02c130..f0cbd67 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3927,6 +3927,173 @@ static const struct mlxsw_config_profile 
mlxsw_sp_config_profile = {
.resource_query_enable  = 1,
 };
 
+static bool
+mlxsw_sp_resource_kvd_granularity_validate(struct netlink_ext_ack *extack,
+  u64 size)
+{
+   const struct mlxsw_config_profile *profile;
+
+   profile = _sp_config_profile;
+   if (size % profile->kvd_hash_granularity) {
+   NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "resource set with wrong 
granularity");
+   return false;
+   }
+   return true;
+}
+
+static int
+mlxsw_sp_resource_kvd_size_validate(struct devlink *devlink, u64 size,
+   struct list_head *resource_list,
+   struct netlink_ext_ack *extack)
+{
+   struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
+   u32 kvd_size, single_size, double_size, linear_size;
+   struct devlink_resource *resource;
+
+   kvd_size = MLXSW_CORE_RES_GET(mlxsw_core, KVD_SIZE);
+   if (kvd_size != size) {
+   NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "kvd size cannot be 
chagned");
+   return -EINVAL;
+   }
+
+   list_for_each_entry(resource, resource_list, list) {
+   switch (resource->id) {
+   case MLXSW_SP_RESOURCE_KVD_LINEAR:
+   linear_size = resource->size_new;
+   break;
+   case MLXSW_SP_RESOURCE_KVD_HASH_SINGLE:
+   single_size = resource->size_new;
+   break;
+   case MLXSW_SP_RESOURCE_KVD_HASH_DOUBLE:
+   double_size = resource->size_new;
+   break;
+   }
+   }
+
+   /* Overlap is not supported */
+   if (linear_size + single_size + double_size > kvd_size) {
+   NL_SET_ERR_MSG(extack, MLXSW_SP_PREFIX "Overlap is not 
supported");
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+static int
+mlxsw_sp_resource_kvd_linear_size_validate(struct devlink *devlink, u64