mbox series

[v6,0/6] implement common rte bit operation APIs in PMDs

Message ID 1576648808-24765-1-git-send-email-joyce.kong@arm.com (mailing list archive)
Headers
Series implement common rte bit operation APIs in PMDs |

Message

Joyce Kong Dec. 18, 2019, 6 a.m. UTC
  There are a lot functions of bit operations scattered in PMDs, consolidate
them into a common API family and applied in different PMDs to reduce code
duplication.

v6:
 Trim 'unsigned long' in PMDs down to 'uint32_t', as on mainstream 64-bit OS,
 'unsigned long' is 64-bit in size, but the 32-bit OS expects 32-bit 'unsigned
 long' argument.

v5:
 Correct the spelling mistake in test_bitops.c

v4:
  Introduce uint32_t/uint64_t *addr when definiting bit operation APIs(suggested by
  Morten Brørup).

v3:
  1. Change the API's head file back to rte_bitops.h, then implement both 32-bit and
     64-bit operations with and without C11 atomic memory ordering.
  2. Add multi-core test case for bit operations which implemented with memory ordering.
  3. Modify the doc of both APIs and test cases.

v2:
  1. Add doxygen comments for the rte bit operation API(suggested by Stephen Hemminger).
  2. Add test cases for common rte bit operation API(suggested by Stephen Hemminger).
  3. Change the header file to rte_io_bitops.h and the operation to rte_io_set_bit()etc.,
     as the API uses barriers inside and the barriers are only needed for IO operations
     (suggested by Jerin Jacob).
  4. Use an well defined uint_NN_t type(suggested by Morten Brørup).

Joyce Kong (6):
  lib/eal: implement the family of rte bit operation APIs
  test/bitops: add bit operation test case
  net/axgbe: use common rte bit operation APIs instead
  net/bnx2x: use common rte bit operation APIs instead
  net/qede: use common rte bit operation APIs instead
  net/hinic: use common rte bit operation APIs instead

 MAINTAINERS                                |   5 +
 app/test/Makefile                          |   1 +
 app/test/autotest_data.py                  |   6 +
 app/test/meson.build                       |   2 +
 app/test/test_bitops.c                     | 305 +++++++++++++++++++
 doc/api/doxy-api-index.md                  |   5 +-
 drivers/net/axgbe/axgbe_common.h           |  29 +-
 drivers/net/axgbe/axgbe_ethdev.c           |  14 +-
 drivers/net/axgbe/axgbe_ethdev.h           |   2 +-
 drivers/net/axgbe/axgbe_mdio.c             |  14 +-
 drivers/net/bnx2x/bnx2x.c                  | 232 +++++++-------
 drivers/net/bnx2x/bnx2x.h                  |  10 +-
 drivers/net/bnx2x/ecore_sp.h               |  47 +--
 drivers/net/hinic/Makefile                 |   1 +
 drivers/net/hinic/base/hinic_compat.h      |  33 +-
 drivers/net/hinic/hinic_pmd_ethdev.c       |  16 +-
 drivers/net/hinic/hinic_pmd_ethdev.h       |   2 +-
 drivers/net/hinic/meson.build              |   2 +
 drivers/net/qede/base/bcm_osal.c           |  22 +-
 drivers/net/qede/base/bcm_osal.h           |  14 +-
 drivers/net/qede/base/ecore.h              |   6 +-
 drivers/net/qede/base/ecore_cxt.c          |   6 +-
 drivers/net/qede/base/ecore_dcbx.c         |   8 +-
 drivers/net/qede/base/ecore_dev.c          |  38 +--
 drivers/net/qede/base/ecore_dev_api.h      |   2 +-
 drivers/net/qede/base/ecore_l2.c           |   6 +-
 drivers/net/qede/base/ecore_mcp.c          |   4 +-
 drivers/net/qede/base/ecore_sp_commands.c  |  12 +-
 drivers/net/qede/base/ecore_spq.c          |   2 +-
 drivers/net/qede/base/ecore_spq.h          |  10 +-
 drivers/net/qede/qede_main.c               |   4 +-
 lib/librte_eal/common/Makefile             |   1 +
 lib/librte_eal/common/include/rte_bitops.h | 474 +++++++++++++++++++++++++++++
 lib/librte_eal/common/meson.build          |   3 +-
 34 files changed, 1015 insertions(+), 323 deletions(-)
 create mode 100644 app/test/test_bitops.c
 create mode 100644 lib/librte_eal/common/include/rte_bitops.h
  

Comments

Gavin Hu Dec. 18, 2019, 6:55 a.m. UTC | #1
Hi Maintainers, 

This series of patches is to consolidate the rte bitops APIs(to reduce duplication) and aim for use by all PMDs. 
In this stage, a few of PMDs you maintained were piloted to stabilize the APIs.

Before expansion to all PMDs, could you please shout out your opinions? 
The APIs have already evolved over community feedback, have a look to know more the background.  

Best Regards,
Gavin

> -----Original Message-----
> From: Joyce Kong <joyce.kong@arm.com>
> Sent: Wednesday, December 18, 2019 2:00 PM
> To: thomas@monjalon.net; stephen@networkplumber.org;
> david.marchand@redhat.com; mb@smartsharesystems.com;
> jerinj@marvell.com; bruce.richardson@intel.com; ravi1.kumar@amd.com;
> rmody@marvell.com; shshaikh@marvell.com; xuanziyang2@huawei.com;
> cloud.wangxiaoyun@huawei.com; zhouguoyang@huawei.com; Honnappa
> Nagarahalli <Honnappa.Nagarahalli@arm.com>; Phil Yang
> <Phil.Yang@arm.com>; Gavin Hu <Gavin.Hu@arm.com>
> Cc: nd <nd@arm.com>; dev@dpdk.org
> Subject: [PATCH v6 0/6] implement common rte bit operation APIs in PMDs
> 
> There are a lot functions of bit operations scattered in PMDs, consolidate
> them into a common API family and applied in different PMDs to reduce
> code
> duplication.
> 
> v6:
>  Trim 'unsigned long' in PMDs down to 'uint32_t', as on mainstream 64-bit OS,
>  'unsigned long' is 64-bit in size, but the 32-bit OS expects 32-bit 'unsigned
>  long' argument.
> 
> v5:
>  Correct the spelling mistake in test_bitops.c
> 
> v4:
>   Introduce uint32_t/uint64_t *addr when definiting bit operation
> APIs(suggested by
>   Morten Brørup).
> 
> v3:
>   1. Change the API's head file back to rte_bitops.h, then implement both 32-
> bit and
>      64-bit operations with and without C11 atomic memory ordering.
>   2. Add multi-core test case for bit operations which implemented with
> memory ordering.
>   3. Modify the doc of both APIs and test cases.
> 
> v2:
>   1. Add doxygen comments for the rte bit operation API(suggested by
> Stephen Hemminger).
>   2. Add test cases for common rte bit operation API(suggested by Stephen
> Hemminger).
>   3. Change the header file to rte_io_bitops.h and the operation to
> rte_io_set_bit()etc.,
>      as the API uses barriers inside and the barriers are only needed for IO
> operations
>      (suggested by Jerin Jacob).
>   4. Use an well defined uint_NN_t type(suggested by Morten Brørup).
> 
> Joyce Kong (6):
>   lib/eal: implement the family of rte bit operation APIs
>   test/bitops: add bit operation test case
>   net/axgbe: use common rte bit operation APIs instead
>   net/bnx2x: use common rte bit operation APIs instead
>   net/qede: use common rte bit operation APIs instead
>   net/hinic: use common rte bit operation APIs instead
> 
>  MAINTAINERS                                |   5 +
>  app/test/Makefile                          |   1 +
>  app/test/autotest_data.py                  |   6 +
>  app/test/meson.build                       |   2 +
>  app/test/test_bitops.c                     | 305 +++++++++++++++++++
>  doc/api/doxy-api-index.md                  |   5 +-
>  drivers/net/axgbe/axgbe_common.h           |  29 +-
>  drivers/net/axgbe/axgbe_ethdev.c           |  14 +-
>  drivers/net/axgbe/axgbe_ethdev.h           |   2 +-
>  drivers/net/axgbe/axgbe_mdio.c             |  14 +-
>  drivers/net/bnx2x/bnx2x.c                  | 232 +++++++-------
>  drivers/net/bnx2x/bnx2x.h                  |  10 +-
>  drivers/net/bnx2x/ecore_sp.h               |  47 +--
>  drivers/net/hinic/Makefile                 |   1 +
>  drivers/net/hinic/base/hinic_compat.h      |  33 +-
>  drivers/net/hinic/hinic_pmd_ethdev.c       |  16 +-
>  drivers/net/hinic/hinic_pmd_ethdev.h       |   2 +-
>  drivers/net/hinic/meson.build              |   2 +
>  drivers/net/qede/base/bcm_osal.c           |  22 +-
>  drivers/net/qede/base/bcm_osal.h           |  14 +-
>  drivers/net/qede/base/ecore.h              |   6 +-
>  drivers/net/qede/base/ecore_cxt.c          |   6 +-
>  drivers/net/qede/base/ecore_dcbx.c         |   8 +-
>  drivers/net/qede/base/ecore_dev.c          |  38 +--
>  drivers/net/qede/base/ecore_dev_api.h      |   2 +-
>  drivers/net/qede/base/ecore_l2.c           |   6 +-
>  drivers/net/qede/base/ecore_mcp.c          |   4 +-
>  drivers/net/qede/base/ecore_sp_commands.c  |  12 +-
>  drivers/net/qede/base/ecore_spq.c          |   2 +-
>  drivers/net/qede/base/ecore_spq.h          |  10 +-
>  drivers/net/qede/qede_main.c               |   4 +-
>  lib/librte_eal/common/Makefile             |   1 +
>  lib/librte_eal/common/include/rte_bitops.h | 474
> +++++++++++++++++++++++++++++
>  lib/librte_eal/common/meson.build          |   3 +-
>  34 files changed, 1015 insertions(+), 323 deletions(-)
>  create mode 100644 app/test/test_bitops.c
>  create mode 100644 lib/librte_eal/common/include/rte_bitops.h
> 
> --
> 2.7.4
  
David Marchand Jan. 17, 2020, 1:03 p.m. UTC | #2
On Wed, Dec 18, 2019 at 7:00 AM Joyce Kong <joyce.kong@arm.com> wrote:
>
> There are a lot functions of bit operations scattered in PMDs, consolidate
> them into a common API family and applied in different PMDs to reduce code
> duplication.
>
> v6:
>  Trim 'unsigned long' in PMDs down to 'uint32_t', as on mainstream 64-bit OS,
>  'unsigned long' is 64-bit in size, but the 32-bit OS expects 32-bit 'unsigned
>  long' argument.
>
> v5:
>  Correct the spelling mistake in test_bitops.c
>
> v4:
>   Introduce uint32_t/uint64_t *addr when definiting bit operation APIs(suggested by
>   Morten Brørup).
>
> v3:
>   1. Change the API's head file back to rte_bitops.h, then implement both 32-bit and
>      64-bit operations with and without C11 atomic memory ordering.
>   2. Add multi-core test case for bit operations which implemented with memory ordering.
>   3. Modify the doc of both APIs and test cases.
>
> v2:
>   1. Add doxygen comments for the rte bit operation API(suggested by Stephen Hemminger).
>   2. Add test cases for common rte bit operation API(suggested by Stephen Hemminger).
>   3. Change the header file to rte_io_bitops.h and the operation to rte_io_set_bit()etc.,
>      as the API uses barriers inside and the barriers are only needed for IO operations
>      (suggested by Jerin Jacob).
>   4. Use an well defined uint_NN_t type(suggested by Morten Brørup).


I did not wait for travis to finish but afaics, this series only
builds on 32bits and AARCH64.
https://travis-ci.com/david-marchand/dpdk/builds/144927372