[dpdk-dev] [PATCH v4 2/3] vhost: rework async configuration struct

[Xia, Chenbo]

Hi Jiayu,

> -----Original Message-----
> From: Hu, Jiayu <jiayu.hu at intel.com>
> Sent: Friday, July 16, 2021 2:18 PM
> To: Xia, Chenbo <chenbo.xia at intel.com>; dev at dpdk.org
> Cc: maxime.coquelin at redhat.com
> Subject: RE: [PATCH v4 2/3] vhost: rework async configuration struct
> 
> Thanks Chenbo for your comments.
> 
> Replies are inline.
> 
> > -----Original Message-----
> > From: Xia, Chenbo <chenbo.xia at intel.com>
> > Sent: Friday, July 16, 2021 2:03 PM
> > To: Hu, Jiayu <jiayu.hu at intel.com>; dev at dpdk.org
> > Cc: maxime.coquelin at redhat.com
> > Subject: RE: [PATCH v4 2/3] vhost: rework async configuration struct
> >
> > Hi Jiayu,
> >
> > > -----Original Message-----
> > > From: Hu, Jiayu <jiayu.hu at intel.com>
> > > Sent: Tuesday, July 13, 2021 3:46 PM
> > > To: dev at dpdk.org
> > > Cc: maxime.coquelin at redhat.com; Xia, Chenbo <chenbo.xia at intel.com>;
> > > Hu, Jiayu <jiayu.hu at intel.com>
> > > Subject: [PATCH v4 2/3] vhost: rework async configuration struct
> >
> > Struct -> structure
> >
> > >
> > > This patch reworks the async configuration structure to improve code
> > > readability. In addition, add preserved padding fields on the
> > > structure for future usage.
> > >
> > > Signed-off-by: Jiayu Hu <jiayu.hu at intel.com>
> > > ---
> > >  doc/guides/prog_guide/vhost_lib.rst | 19 +++++++++++--------
> > >  examples/vhost/main.c               |  8 ++++----
> > >  lib/vhost/rte_vhost_async.h         | 35 +++++++++++++++++---------------
> ---
> > >  lib/vhost/vhost.c                   | 13 +++++--------
> > >  4 files changed, 37 insertions(+), 38 deletions(-)
> > >

[snip]

> > > -vq->async_inorder = f.async_inorder;
> > > -vq->async_threshold = f.async_threshold;
> > > +vq->async_inorder = true;
> >
> > Do we still need this? It's never used.
> 
> I think we need to keep it, as we may support out-of-order channel in future.

We don't like to define things when it's not needed currently. If in future,
the definition should also be made in future :P. Just leave the definition
to future patch.

Thanks,
Chenbo

> 
> >
> > > +vq->async_threshold = config.async_threshold;
> >
> > vq->async_threshold is uint16_t and config.async_threshold is uint32_t.
> > They should be the same.
> 
> I will change vq->async_threshold to uint32_t.
> 
> Thanks,
> Jiayu
> >
> > Thanks,
> > Chenbo
> >
> > >
> > >  vq->async_registered = true;
> > >
> > > --
> > > 2.7.4
> >
>