[Accel-config] Re: [PATCH v4 03/10] accel-config: Add completion record size setup for accfg_device

On 4/12/2022 12:20 AM, Zhang, Li4 wrote:
> Hi @Thomas, Ramesh <mailto:ramesh.thomas(a)intel.com>,
> 
> Thanks for your comments.
> 
> The following is my questions?
> 
>  > Please make this dynamic. See how basenames is used. No need for this 
> function.
> 
> The new function device_parse_compl_size() is invoked in add_device() 
> for initializing "device->compl_size".
> 
> If abandon device_parse_compl_size(), Shall I initialize 
> "device->compl_size" in funcrion device_parse_type()?
> 
>  > Please create a new version block
> 
> Could you please guide me how to do it?
> 
> Should I append
> 
> /               LIBACCFG_14 {/
> 
> /               global:/
> 
> /                              accfg_device_get_compl_size;/
> 
> /               } LIBACCFG_13;/
> 
> at the end of file accfg/lib/libaccel-config.sym?

Yes, that is right.

> 
> ----------------------------
> 
> Best wishes,
> 
> Rex Zhang
> 
> li4.zhang(a)intel.com
> 
> -----Original Message-----
> From: Thomas, Ramesh <ramesh.thomas(a)intel.com>
> Sent: Tuesday, April 12, 2022 12:24 PM
> To: Zhang, Li4 <li4.zhang(a)intel.com>; Jiang, Dave 
> <dave.jiang(a)intel.com>; Sun, Yi <yi.sun(a)intel.com>; 
> yi.sun(a)linux.intel.com; Zhu, Tony <tony.zhu(a)intel.com>; 
> accel-config(a)lists.01.org
> Subject: Re: [PATCH v4 03/10] accel-config: Add completion record size 
> setup for accfg_device
> 
> On 4/9/2022 7:14 PM, Li Zhang wrote:
> 
>  > DSA and IAA test need to get completion record size for allocating and
> 
>  > dumping completion record. DSA completion record is 32 bytes while IAA
> 
>  > completion record is 64 bytes. So add compl_size field in struct
> 
>  > accfg_device for using by DSA and IAA test code.
> 
>  >
> 
>  > Signed-off-by: Li Zhang <li4.zhang(a)intel.com 
> <mailto:li4.zhang(a)intel.com>>
> 
>  > Reviewed-by: Dave Jiang <dave.jiang(a)intel.com 
> <mailto:dave.jiang(a)intel.com>>
> 
>  > ---
> 
>  >   accfg/lib/libaccfg.c    | 17 +++++++++++++++++
> 
>  >   accfg/lib/private.h     |  1 +
> 
>  >   accfg/libaccel_config.h |  5 +++++
> 
>  >   3 files changed, 23 insertions(+)
> 
>  >
> 
>  > diff --git a/accfg/lib/libaccfg.c b/accfg/lib/libaccfg.c index
> 
>  > 8d883b1..200bd70 100644
> 
>  > --- a/accfg/lib/libaccfg.c
> 
>  > +++ b/accfg/lib/libaccfg.c
> 
>  > @@ -547,6 +547,19 @@ static int device_parse_type(struct accfg_device 
> *device)
> 
>  >             return -ENODEV;
> 
>  >   }
> 
>  >
> 
>  > +/*set completion record size*/
> 
>  > +static int device_parse_compl_size(struct accfg_device *device) {
> 
>  > +          if (device->type == ACCFG_DEVICE_DSA)
> 
>  > +                         device->compl_size = 
> ACCFG_DEVICE_COMPL_SIZE_DSA;
> 
>  > +          else if (device->type == ACCFG_DEVICE_IAX)
> 
>  > +                         device->compl_size = 
> ACCFG_DEVICE_COMPL_SIZE_IAX;
> 
> Please make this dynamic. See how basenames is used. No need for this 
> function.
> 
>  > +          else
> 
>  > +                         return -ENODEV;
> 
>  > +
> 
>  > +          return 0;
> 
>  > +}
> 
>  > +
> 
>  >   static int mdev_str_to_type(char *mdev_type_str)
> 
>  >   {
> 
>  >             char **b;
> 
>  > @@ -706,6 +719,10 @@ static void *add_device(void *parent, int id, 
> const char *ctl_base,
> 
>  >             if (rc < 0)
> 
>  >                            goto err_dev_path;
> 
>  >
> 
>  > +          rc = device_parse_compl_size(device);
> 
> No need to call function. Assign from array indexed by device type.
> 
>  > +          if (rc < 0)
> 
>  > +                         goto err_dev_path;
> 
>  > +
> 
>  >             device->bus_type_str = bus_type;
> 
>  >
> 
>  >             if (is_mdev_registered(device) && add_device_mdevs(ctx, 
> device))
> 
>  > diff --git a/accfg/lib/private.h b/accfg/lib/private.h index
> 
>  > f88044c..773aba2 100644
> 
>  > --- a/accfg/lib/private.h
> 
>  > +++ b/accfg/lib/private.h
> 
>  > @@ -46,6 +46,7 @@ struct accfg_device {
> 
>  >             int numa_node;
> 
>  >             int ims_size;
> 
>  >             int max_batch_size;
> 
>  > +          int compl_size;
> 
>  >             int configurable;
> 
>  >             int max_read_buffers;
> 
>  >             unsigned int read_buffer_limit;
> 
>  > diff --git a/accfg/libaccel_config.h b/accfg/libaccel_config.h index
> 
>  > 9e952a7..ef7c8ae 100644
> 
>  > --- a/accfg/libaccel_config.h
> 
>  > +++ b/accfg/libaccel_config.h
> 
>  > @@ -45,6 +45,11 @@ enum accfg_device_state {
> 
>  >             ACCFG_DEVICE_UNKNOWN = -1,
> 
>  >   };
> 
>  >
> 
>  > +enum accfg_device_compl_size {
> 
>  > +          ACCFG_DEVICE_COMPL_SIZE_DSA = 32,
> 
>  > +          ACCFG_DEVICE_COMPL_SIZE_IAX = 64,
> 
>  > +};
> 
>  > +
> 
> No need for this enum. Create an array like basenames and store the 
> corresponding value for each type
> 
>  >   enum accfg_wq_mode {
> 
>  >             ACCFG_WQ_SHARED = 0,
> 
>  >             ACCFG_WQ_DEDICATED,
>