dma/idxd: add generic option for queue config

  The device config script currently uses some defaults to configure
devices in a generic way.

With the addition of this option, users have more control over how
queues are configured.

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
---
 drivers/dma/idxd/dpdk_idxd_cfg.py | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)
  

On Wed, Mar 30, 2022 at 04:07:00PM +0100, Kevin Laatz wrote:
> The device config script currently uses some defaults to configure
> devices in a generic way.
> 
> With the addition of this option, users have more control over how
> queues are configured.
> 
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> ---
>  drivers/dma/idxd/dpdk_idxd_cfg.py | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma/idxd/dpdk_idxd_cfg.py b/drivers/dma/idxd/dpdk_idxd_cfg.py
> index 3f5d5ee752..9ac724e7a8 100755
> --- a/drivers/dma/idxd/dpdk_idxd_cfg.py
> +++ b/drivers/dma/idxd/dpdk_idxd_cfg.py
> @@ -62,9 +62,25 @@ def get_dsa_id(pci):
>                  return int(dir[3:])
>      sys.exit(f"Could not get device ID for device {pci}")
>  
> -
> -def configure_dsa(dsa_id, queues, prefix):
> +def parse_wq_opts(dsa_id, q, wq_opts):
> +    "Parse the additional user-specified queue configuration"
> +    wq_dir = SysfsDir(f'/sys/bus/dsa/devices/dsa{dsa_id}/wq{dsa_id}.{q}')
> +    for wq_opt in wq_opts:
> +        try:
> +            opt, val = wq_opt.split("=")
> +        except ValueError:
> +            sys.exit("Invalid format, use format 'option=value'")
> +        if not os.path.exists(os.path.join(wq_dir.path, f'{opt}')):
> +            sys.exit(f"Invalid sysfs node '{opt}', path does not exist")
> +        wq_dir.write_values({opt: val})
> +
> +
> +def configure_dsa(dsa_id, args):
>      "Configure the DSA instance with appropriate number of queues"
> +    queues = args.q
> +    prefix = args.prefix
> +    wq_opts = args.wq_option
> +
>      dsa_dir = SysfsDir(f"/sys/bus/dsa/devices/dsa{dsa_id}")
>  
>      max_groups = dsa_dir.read_int("max_groups")
> @@ -92,6 +108,11 @@ def configure_dsa(dsa_id, queues, prefix):
>                               "max_batch_size": 1024,
>                               "size": int(max_work_queues_size / nb_queues)})
>  
> +    # parse additional user-spcified queue configuration
> +    if wq_opts:
> +        for q in range(nb_queues):
> +            parse_wq_opts(dsa_id, q, wq_opts)
> +

I think this may be better to have the parse function only parse the
options and split them. If that is done before the actual queue
configuration function is called, then the additional options could be
passed in there, and merged with the existing config settings. This avoids
duplicating things and doing two sets of configs.

/Bruce
  

On 31/03/2022 15:57, Bruce Richardson wrote:
> On Wed, Mar 30, 2022 at 04:07:00PM +0100, Kevin Laatz wrote:
>> The device config script currently uses some defaults to configure
>> devices in a generic way.
>>
>> With the addition of this option, users have more control over how
>> queues are configured.
>>
>> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
>> ---
>>   drivers/dma/idxd/dpdk_idxd_cfg.py | 29 ++++++++++++++++++++++++++---
>>   1 file changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/dma/idxd/dpdk_idxd_cfg.py b/drivers/dma/idxd/dpdk_idxd_cfg.py
>> index 3f5d5ee752..9ac724e7a8 100755
>> --- a/drivers/dma/idxd/dpdk_idxd_cfg.py
>> +++ b/drivers/dma/idxd/dpdk_idxd_cfg.py
>> @@ -62,9 +62,25 @@ def get_dsa_id(pci):
>>                   return int(dir[3:])
>>       sys.exit(f"Could not get device ID for device {pci}")
>>   
>> -
>> -def configure_dsa(dsa_id, queues, prefix):
>> +def parse_wq_opts(dsa_id, q, wq_opts):
>> +    "Parse the additional user-specified queue configuration"
>> +    wq_dir = SysfsDir(f'/sys/bus/dsa/devices/dsa{dsa_id}/wq{dsa_id}.{q}')
>> +    for wq_opt in wq_opts:
>> +        try:
>> +            opt, val = wq_opt.split("=")
>> +        except ValueError:
>> +            sys.exit("Invalid format, use format 'option=value'")
>> +        if not os.path.exists(os.path.join(wq_dir.path, f'{opt}')):
>> +            sys.exit(f"Invalid sysfs node '{opt}', path does not exist")
>> +        wq_dir.write_values({opt: val})
>> +
>> +
>> +def configure_dsa(dsa_id, args):
>>       "Configure the DSA instance with appropriate number of queues"
>> +    queues = args.q
>> +    prefix = args.prefix
>> +    wq_opts = args.wq_option
>> +
>>       dsa_dir = SysfsDir(f"/sys/bus/dsa/devices/dsa{dsa_id}")
>>   
>>       max_groups = dsa_dir.read_int("max_groups")
>> @@ -92,6 +108,11 @@ def configure_dsa(dsa_id, queues, prefix):
>>                                "max_batch_size": 1024,
>>                                "size": int(max_work_queues_size / nb_queues)})
>>   
>> +    # parse additional user-spcified queue configuration
>> +    if wq_opts:
>> +        for q in range(nb_queues):
>> +            parse_wq_opts(dsa_id, q, wq_opts)
>> +
> I think this may be better to have the parse function only parse the
> options and split them. If that is done before the actual queue
> configuration function is called, then the additional options could be
> passed in there, and merged with the existing config settings. This avoids
> duplicating things and doing two sets of configs.

Thanks for the suggestion, Bruce. I'll look into it and send a v2.

/Kevin