mbox series

[v12,0/7] bbdev changes for 22.11

Message ID 20221004171656.17967-1-nicolas.chautru@intel.com (mailing list archive)
Headers show
Series bbdev changes for 22.11 | expand

Message

Nicolas Chautru Oct. 4, 2022, 5:16 p.m. UTC
Hi Akhil, Thomas, 

v12: minor change to fix misaligned comment on patch 6 raised by Thomas. Thanks. 
v11: updated based on Thomas review notably on comments through the serie and ordering. Thanks. I have also updated rel_notes and deprecation through the serie this time.
v10: replacing the _PADDED_MAX enum to _SIZE_MAX macro based on suggestion from Ferruh/Maxime/Akhil. Thanks
v9: removing code snippet from documentation in 5/7 requested by Akhil. Thanks. 
v8: edit based on review by Akhil : typos, coding guidelines. No functional change. Thanks
v7: couple of typos in documentation spotted by Maxime. Thanks.
v6: added one comment in commit 2/7 suggested by Maxime.
v5: update base on review from Tom Rix. Number of typos reported and resolved,
removed the commit related to rw_lock for now, added a commit for
code clean up from review, resolved one rebase issue between 2 commits, used size of array for some bound check implementation. Thanks. 
v4: update to the last 2 commits to include function to print the queue status and a fix to the rte_lock within the wrong structure
v3: update to device status info to also use padded size for the related array.
Adding also 2 additionals commits to allow the API struc to expose more information related to queues corner cases/warning as well as an optional rw lock.
Hemant, Maxime, this is planned for DPDK 21.11 but would like review/ack early is possible to get this applied earlier and due to time off this summer.
Thanks
Nic

Nicolas Chautru (7):
  bbdev: allow operation type enum for growth
  bbdev: add device status info
  bbdev: add device info on queue topology
  drivers/baseband: update PMDs to expose queue per operation
  bbdev: add new operation for FFT processing
  bbdev: add queue related warning and status information
  bbdev: remove unnecessary if-check

 app/test-bbdev/test_bbdev.c                   |   2 +-
 app/test-bbdev/test_bbdev_perf.c              |   6 +-
 doc/guides/prog_guide/bbdev.rst               | 103 +++++++++++
 doc/guides/rel_notes/deprecation.rst          |  13 --
 doc/guides/rel_notes/release_22_11.rst        |  14 ++
 drivers/baseband/acc100/rte_acc100_pmd.c      |  30 ++--
 .../fpga_5gnr_fec/rte_fpga_5gnr_fec.c         |   9 +
 drivers/baseband/fpga_lte_fec/fpga_lte_fec.c  |   9 +
 drivers/baseband/la12xx/bbdev_la12xx.c        |  10 +-
 drivers/baseband/null/bbdev_null.c            |   1 +
 .../baseband/turbo_sw/bbdev_turbo_software.c  |  13 ++
 examples/bbdev_app/main.c                     |   2 +-
 lib/bbdev/rte_bbdev.c                         |  57 +++++-
 lib/bbdev/rte_bbdev.h                         | 158 +++++++++++++++-
 lib/bbdev/rte_bbdev_op.h                      | 169 ++++++++++++++++--
 lib/bbdev/version.map                         |  12 ++
 16 files changed, 560 insertions(+), 48 deletions(-)

Comments

Akhil Goyal Oct. 6, 2022, 5:31 p.m. UTC | #1
> Hi Akhil, Thomas,
> 
> v12: minor change to fix misaligned comment on patch 6 raised by Thomas.
> Thanks.
> v11: updated based on Thomas review notably on comments through the serie
> and ordering. Thanks. I have also updated rel_notes and deprecation through
> the serie this time.
> v10: replacing the _PADDED_MAX enum to _SIZE_MAX macro based on
> suggestion from Ferruh/Maxime/Akhil. Thanks
> v9: removing code snippet from documentation in 5/7 requested by Akhil.
> Thanks.
> v8: edit based on review by Akhil : typos, coding guidelines. No functional
> change. Thanks
> v7: couple of typos in documentation spotted by Maxime. Thanks.
> v6: added one comment in commit 2/7 suggested by Maxime.
> v5: update base on review from Tom Rix. Number of typos reported and
> resolved,
> removed the commit related to rw_lock for now, added a commit for
> code clean up from review, resolved one rebase issue between 2 commits, used
> size of array for some bound check implementation. Thanks.
> v4: update to the last 2 commits to include function to print the queue status
> and a fix to the rte_lock within the wrong structure
> v3: update to device status info to also use padded size for the related array.
> Adding also 2 additionals commits to allow the API struc to expose more
> information related to queues corner cases/warning as well as an optional rw
> lock.
> Hemant, Maxime, this is planned for DPDK 21.11 but would like review/ack early
> is possible to get this applied earlier and due to time off this summer.
> Thanks
> Nic
> 
> Nicolas Chautru (7):
>   bbdev: allow operation type enum for growth
>   bbdev: add device status info
>   bbdev: add device info on queue topology
>   drivers/baseband: update PMDs to expose queue per operation
>   bbdev: add new operation for FFT processing
>   bbdev: add queue related warning and status information
>   bbdev: remove unnecessary if-check
> 
>  app/test-bbdev/test_bbdev.c                   |   2 +-
>  app/test-bbdev/test_bbdev_perf.c              |   6 +-
>  doc/guides/prog_guide/bbdev.rst               | 103 +++++++++++
>  doc/guides/rel_notes/deprecation.rst          |  13 --
>  doc/guides/rel_notes/release_22_11.rst        |  14 ++
>  drivers/baseband/acc100/rte_acc100_pmd.c      |  30 ++--
>  .../fpga_5gnr_fec/rte_fpga_5gnr_fec.c         |   9 +
>  drivers/baseband/fpga_lte_fec/fpga_lte_fec.c  |   9 +
>  drivers/baseband/la12xx/bbdev_la12xx.c        |  10 +-
>  drivers/baseband/null/bbdev_null.c            |   1 +
>  .../baseband/turbo_sw/bbdev_turbo_software.c  |  13 ++
>  examples/bbdev_app/main.c                     |   2 +-
>  lib/bbdev/rte_bbdev.c                         |  57 +++++-
>  lib/bbdev/rte_bbdev.h                         | 158 +++++++++++++++-
>  lib/bbdev/rte_bbdev_op.h                      | 169 ++++++++++++++++--
>  lib/bbdev/version.map                         |  12 ++
>  16 files changed, 560 insertions(+), 48 deletions(-)
> 
Hi Nicolas,

There were many formatting issues in this patch. This has been a practice in whole of bbdev.
Please take a diff of what is applied on the tree and what was submitted to take care of the
formatting in future patches and please plan to fix the rest of bbdev documentation (API + prog_guide).

Series applied to dpdk-next-crypto

Thanks.
Nicolas Chautru Oct. 6, 2022, 10:28 p.m. UTC | #2
Thanks Akhil, 

A couple of things I miss in term of guidelines for my benefit and that I don't see in documentation:
- May I ask what rule we should use for documentation line breaking? I am unclear of the reason for some of the changes you made whose origin version looked legit to me. Were you trying to fit into 80 chars (not always the case) or was this to split the sentence by phrase as opposed to breaking simply just before 100 chars? Can you clarify the recommendation? I see in the doc it suggests 80 chars but unsure whether this is because this was not updated when moving to 100 chars. 
- Do we really need a dot at the end of every text in a comment even when not an actual sentence? I don't personally see the point nor do I believe this is followed in much of DPDK existing code (it depends at best), nor was this case in the file where new structure was added (keeping consistency).  Is that really a recommendation for DPDK guideline for all new code to add a dot at the end of all text? 

Thanks for kindly clarifying, I can then update the other serie based on such guidelines. 

Thanks for the other formatting issues and commit message changes. 

Nic

> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Thursday, October 6, 2022 10:32 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> thomas@monjalon.net
> Cc: maxime.coquelin@redhat.com; trix@redhat.com; mdr@ashroe.eu;
> Richardson, Bruce <bruce.richardson@intel.com>;
> david.marchand@redhat.com; stephen@networkplumber.org; Zhang,
> Mingshan <mingshan.zhang@intel.com>; hemant.agrawal@nxp.com
> Subject: RE: [EXT] [PATCH v12 0/7] bbdev changes for 22.11
> 
> > Hi Akhil, Thomas,
> >
> > v12: minor change to fix misaligned comment on patch 6 raised by Thomas.
> > Thanks.
> > v11: updated based on Thomas review notably on comments through the
> > serie and ordering. Thanks. I have also updated rel_notes and
> > deprecation through the serie this time.
> > v10: replacing the _PADDED_MAX enum to _SIZE_MAX macro based on
> > suggestion from Ferruh/Maxime/Akhil. Thanks
> > v9: removing code snippet from documentation in 5/7 requested by Akhil.
> > Thanks.
> > v8: edit based on review by Akhil : typos, coding guidelines. No
> > functional change. Thanks
> > v7: couple of typos in documentation spotted by Maxime. Thanks.
> > v6: added one comment in commit 2/7 suggested by Maxime.
> > v5: update base on review from Tom Rix. Number of typos reported and
> > resolved, removed the commit related to rw_lock for now, added a
> > commit for code clean up from review, resolved one rebase issue
> > between 2 commits, used size of array for some bound check
> > implementation. Thanks.
> > v4: update to the last 2 commits to include function to print the
> > queue status and a fix to the rte_lock within the wrong structure
> > v3: update to device status info to also use padded size for the related
> array.
> > Adding also 2 additionals commits to allow the API struc to expose
> > more information related to queues corner cases/warning as well as an
> > optional rw lock.
> > Hemant, Maxime, this is planned for DPDK 21.11 but would like
> > review/ack early is possible to get this applied earlier and due to time off
> this summer.
> > Thanks
> > Nic
> >
> > Nicolas Chautru (7):
> >   bbdev: allow operation type enum for growth
> >   bbdev: add device status info
> >   bbdev: add device info on queue topology
> >   drivers/baseband: update PMDs to expose queue per operation
> >   bbdev: add new operation for FFT processing
> >   bbdev: add queue related warning and status information
> >   bbdev: remove unnecessary if-check
> >
> >  app/test-bbdev/test_bbdev.c                   |   2 +-
> >  app/test-bbdev/test_bbdev_perf.c              |   6 +-
> >  doc/guides/prog_guide/bbdev.rst               | 103 +++++++++++
> >  doc/guides/rel_notes/deprecation.rst          |  13 --
> >  doc/guides/rel_notes/release_22_11.rst        |  14 ++
> >  drivers/baseband/acc100/rte_acc100_pmd.c      |  30 ++--
> >  .../fpga_5gnr_fec/rte_fpga_5gnr_fec.c         |   9 +
> >  drivers/baseband/fpga_lte_fec/fpga_lte_fec.c  |   9 +
> >  drivers/baseband/la12xx/bbdev_la12xx.c        |  10 +-
> >  drivers/baseband/null/bbdev_null.c            |   1 +
> >  .../baseband/turbo_sw/bbdev_turbo_software.c  |  13 ++
> >  examples/bbdev_app/main.c                     |   2 +-
> >  lib/bbdev/rte_bbdev.c                         |  57 +++++-
> >  lib/bbdev/rte_bbdev.h                         | 158 +++++++++++++++-
> >  lib/bbdev/rte_bbdev_op.h                      | 169 ++++++++++++++++--
> >  lib/bbdev/version.map                         |  12 ++
> >  16 files changed, 560 insertions(+), 48 deletions(-)
> >
> Hi Nicolas,
> 
> There were many formatting issues in this patch. This has been a practice in
> whole of bbdev.
> Please take a diff of what is applied on the tree and what was submitted to
> take care of the formatting in future patches and please plan to fix the rest
> of bbdev documentation (API + prog_guide).
> 
> Series applied to dpdk-next-crypto
> 
> Thanks.
Akhil Goyal Oct. 7, 2022, 4:46 a.m. UTC | #3
> Thanks Akhil,
> 
> A couple of things I miss in term of guidelines for my benefit and that I don't see
> in documentation:
> - May I ask what rule we should use for documentation line breaking? I am
> unclear of the reason for some of the changes you made whose origin version
> looked legit to me. Were you trying to fit into 80 chars (not always the case) or
> was this to split the sentence by phrase as opposed to breaking simply just
> before 100 chars? Can you clarify the recommendation? I see in the doc it
> suggests 80 chars but unsure whether this is because this was not updated when
> moving to 100 chars.

The guideline for is for 80 characters, but if the sentence is going to be split across two
Lines, we should break at logical points. This improves readability.

> - Do we really need a dot at the end of every text in a comment even when not
> an actual sentence? I don't personally see the point nor do I believe this is
> followed in much of DPDK existing code (it depends at best), nor was this case in
> the file where new structure was added (keeping consistency).  Is that really a
> recommendation for DPDK guideline for all new code to add a dot at the end of
> all text?

Dots should be added for each of the comment wherever we have a sentence.
They may be skipped for a couple of words.

> 
> Thanks for kindly clarifying, I can then update the other serie based on such
> guidelines.
> 
> Thanks for the other formatting issues and commit message changes.
> 
> Nic
> 
> > -----Original Message-----
> > From: Akhil Goyal <gakhil@marvell.com>
> > Sent: Thursday, October 6, 2022 10:32 AM
> > To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
> > thomas@monjalon.net
> > Cc: maxime.coquelin@redhat.com; trix@redhat.com; mdr@ashroe.eu;
> > Richardson, Bruce <bruce.richardson@intel.com>;
> > david.marchand@redhat.com; stephen@networkplumber.org; Zhang,
> > Mingshan <mingshan.zhang@intel.com>; hemant.agrawal@nxp.com
> > Subject: RE: [EXT] [PATCH v12 0/7] bbdev changes for 22.11
> >
> > > Hi Akhil, Thomas,
> > >
> > > v12: minor change to fix misaligned comment on patch 6 raised by Thomas.
> > > Thanks.
> > > v11: updated based on Thomas review notably on comments through the
> > > serie and ordering. Thanks. I have also updated rel_notes and
> > > deprecation through the serie this time.
> > > v10: replacing the _PADDED_MAX enum to _SIZE_MAX macro based on
> > > suggestion from Ferruh/Maxime/Akhil. Thanks
> > > v9: removing code snippet from documentation in 5/7 requested by Akhil.
> > > Thanks.
> > > v8: edit based on review by Akhil : typos, coding guidelines. No
> > > functional change. Thanks
> > > v7: couple of typos in documentation spotted by Maxime. Thanks.
> > > v6: added one comment in commit 2/7 suggested by Maxime.
> > > v5: update base on review from Tom Rix. Number of typos reported and
> > > resolved, removed the commit related to rw_lock for now, added a
> > > commit for code clean up from review, resolved one rebase issue
> > > between 2 commits, used size of array for some bound check
> > > implementation. Thanks.
> > > v4: update to the last 2 commits to include function to print the
> > > queue status and a fix to the rte_lock within the wrong structure
> > > v3: update to device status info to also use padded size for the related
> > array.
> > > Adding also 2 additionals commits to allow the API struc to expose
> > > more information related to queues corner cases/warning as well as an
> > > optional rw lock.
> > > Hemant, Maxime, this is planned for DPDK 21.11 but would like
> > > review/ack early is possible to get this applied earlier and due to time off
> > this summer.
> > > Thanks
> > > Nic
> > >
> > > Nicolas Chautru (7):
> > >   bbdev: allow operation type enum for growth
> > >   bbdev: add device status info
> > >   bbdev: add device info on queue topology
> > >   drivers/baseband: update PMDs to expose queue per operation
> > >   bbdev: add new operation for FFT processing
> > >   bbdev: add queue related warning and status information
> > >   bbdev: remove unnecessary if-check
> > >
> > >  app/test-bbdev/test_bbdev.c                   |   2 +-
> > >  app/test-bbdev/test_bbdev_perf.c              |   6 +-
> > >  doc/guides/prog_guide/bbdev.rst               | 103 +++++++++++
> > >  doc/guides/rel_notes/deprecation.rst          |  13 --
> > >  doc/guides/rel_notes/release_22_11.rst        |  14 ++
> > >  drivers/baseband/acc100/rte_acc100_pmd.c      |  30 ++--
> > >  .../fpga_5gnr_fec/rte_fpga_5gnr_fec.c         |   9 +
> > >  drivers/baseband/fpga_lte_fec/fpga_lte_fec.c  |   9 +
> > >  drivers/baseband/la12xx/bbdev_la12xx.c        |  10 +-
> > >  drivers/baseband/null/bbdev_null.c            |   1 +
> > >  .../baseband/turbo_sw/bbdev_turbo_software.c  |  13 ++
> > >  examples/bbdev_app/main.c                     |   2 +-
> > >  lib/bbdev/rte_bbdev.c                         |  57 +++++-
> > >  lib/bbdev/rte_bbdev.h                         | 158 +++++++++++++++-
> > >  lib/bbdev/rte_bbdev_op.h                      | 169 ++++++++++++++++--
> > >  lib/bbdev/version.map                         |  12 ++
> > >  16 files changed, 560 insertions(+), 48 deletions(-)
> > >
> > Hi Nicolas,
> >
> > There were many formatting issues in this patch. This has been a practice in
> > whole of bbdev.
> > Please take a diff of what is applied on the tree and what was submitted to
> > take care of the formatting in future patches and please plan to fix the rest
> > of bbdev documentation (API + prog_guide).
> >
> > Series applied to dpdk-next-crypto
> >
> > Thanks.
Thomas Monjalon Oct. 10, 2022, 7:35 a.m. UTC | #4
06/10/2022 19:31, Akhil Goyal:
> There were many formatting issues in this patch. This has been a practice in whole of bbdev.
> Please take a diff of what is applied on the tree and what was submitted to take care of the
> formatting in future patches and please plan to fix the rest of bbdev documentation (API + prog_guide).
> 
> Series applied to dpdk-next-crypto

Please could you check this build issue?
https://bugs.dpdk.org/show_bug.cgi?id=1095

Ideally I'd like to see it fixed today, thanks.
Nicolas Chautru Oct. 10, 2022, 5:07 p.m. UTC | #5
Hi Thomas, Akhil, 

I put a fix here : 
https://patches.dpdk.org/project/dpdk/patch/20221010170403.21201-1-nicolas.chautru@intel.com/

I did not reproduce that issue on gcc or recent clang, only with clang 3.4.2 on Centos7.

Thanks
Nic

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, October 10, 2022 12:36 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>
> Cc: dev@dpdk.org; maxime.coquelin@redhat.com; trix@redhat.com;
> mdr@ashroe.eu; Richardson, Bruce <bruce.richardson@intel.com>;
> david.marchand@redhat.com; stephen@networkplumber.org; Zhang,
> Mingshan <mingshan.zhang@intel.com>; hemant.agrawal@nxp.com; Akhil
> Goyal <gakhil@marvell.com>; alialnu@nvidia.com
> Subject: Re: [EXT] [PATCH v12 0/7] bbdev changes for 22.11
> 
> 06/10/2022 19:31, Akhil Goyal:
> > There were many formatting issues in this patch. This has been a practice in
> whole of bbdev.
> > Please take a diff of what is applied on the tree and what was
> > submitted to take care of the formatting in future patches and please plan
> to fix the rest of bbdev documentation (API + prog_guide).
> >
> > Series applied to dpdk-next-crypto
> 
> Please could you check this build issue?
> https://bugs.dpdk.org/show_bug.cgi?id=1095
> 
> Ideally I'd like to see it fixed today, thanks.
>