Re: [PATCH] dma/idxd: add generic option for queue config

[Bruce Richardson <bruce.richardson@intel.com>]

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