Message ID | 1571799298-18873-1-git-send-email-joyce.kong@arm.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 764BD1BEB8; Wed, 23 Oct 2019 04:55:29 +0200 (CEST) Received: from foss.arm.com (unknown [217.140.110.172]) by dpdk.org (Postfix) with ESMTP id B00D949DF for <dev@dpdk.org>; Wed, 23 Oct 2019 04:55:27 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 31A941682; Tue, 22 Oct 2019 19:55:18 -0700 (PDT) Received: from net-arm-thunderx2-01.test.ast.arm.com (net-arm-thunderx2-01.shanghai.arm.com [10.169.40.40]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 3EF193F6C4; Tue, 22 Oct 2019 19:55:11 -0700 (PDT) From: Joyce Kong <joyce.kong@arm.com> To: dev@dpdk.org Cc: nd@arm.com, thomas@monjalon.net, jerinj@marvell.com, stephen@networkplumber.org, mb@smartsharesystems.com, honnappa.nagarahalli@arm.com, gavin.hu@arm.com, ravi1.kumar@amd.com, rmody@marvell.com, shshaikh@marvell.com, xuanziyang2@huawei.com, cloud.wangxiaoyun@huawei.com, zhouguoyang@huawei.com Date: Wed, 23 Oct 2019 10:54:52 +0800 Message-Id: <1571799298-18873-1-git-send-email-joyce.kong@arm.com> X-Mailer: git-send-email 2.7.4 MIME-Version: 1.0 In-Reply-To: <1571125801-45773-1-git-send-email-joyce.kong@arm.com> References: <1571125801-45773-1-git-send-email-joyce.kong@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: [dpdk-dev] [PATCH v2 0/6] implement common rte bit operation APIs in PMDs X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Series |
implement common rte bit operation APIs in PMDs
|
|
Message
Joyce Kong
Oct. 23, 2019, 2:54 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. 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/iobitops: add io bit operation test case net/axgbe: use common rte bit operation APIs instead net/bnx2x: use common rte bit operation APIs instead net/hinic: use common rte bit operation APIs instead net/qede: use common rte bit operation APIs instead app/test/Makefile | 1 + app/test/test_io_bitops.c | 86 +++++++++++ drivers/net/axgbe/axgbe_common.h | 29 +--- drivers/net/axgbe/axgbe_ethdev.c | 14 +- drivers/net/axgbe/axgbe_mdio.c | 14 +- drivers/net/bnx2x/bnx2x.c | 209 ++++++++++++-------------- drivers/net/bnx2x/bnx2x.h | 4 - drivers/net/bnx2x/ecore_sp.h | 9 +- drivers/net/hinic/base/hinic_compat.h | 35 +---- drivers/net/hinic/hinic_pmd_ethdev.c | 16 +- drivers/net/qede/base/bcm_osal.c | 20 --- drivers/net/qede/base/bcm_osal.h | 10 +- lib/librte_eal/common/Makefile | 1 + lib/librte_eal/common/include/rte_io_bitops.h | 112 ++++++++++++++ lib/librte_eal/common/meson.build | 1 + 15 files changed, 327 insertions(+), 234 deletions(-) create mode 100644 app/test/test_io_bitops.c create mode 100644 lib/librte_eal/common/include/rte_io_bitops.h
Comments
On Wed, Oct 23, 2019 at 4:55 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. > > 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). Thanks for working on this. This series is a cleanup and worth looking at, yet it came rather late. Discussion and enhancement can still continue, but it will be deferred to 20.02. -- David Marchand
23/10/2019 04:54, Joyce Kong: > 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. Please, could you look at what Adrien did in the Mellanox PMD? http://code.dpdk.org/dpdk/latest/source/drivers/net/mlx5/mlx5_utils.h#L28
Hi Thomas, > -----Original Message----- > From: Thomas Monjalon <thomas@monjalon.net> > Sent: Wednesday, October 30, 2019 12:43 AM > To: Joyce Kong (Arm Technology China) <Joyce.Kong@arm.com> > Cc: dev@dpdk.org; nd <nd@arm.com>; jerinj@marvell.com; > stephen@networkplumber.org; mb@smartsharesystems.com; Honnappa > Nagarahalli <Honnappa.Nagarahalli@arm.com>; Gavin Hu (Arm Technology > China) <Gavin.Hu@arm.com>; ravi1.kumar@amd.com; rmody@marvell.com; > shshaikh@marvell.com; xuanziyang2@huawei.com; > cloud.wangxiaoyun@huawei.com; zhouguoyang@huawei.com; > adrien.mazarguil@6wind.com > Subject: Re: [dpdk-dev] [PATCH v2 0/6] implement common rte bit operation > APIs in PMDs > > 23/10/2019 04:54, Joyce Kong: > > 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. > > Please, could you look at what Adrien did in the Mellanox PMD? > > http://code.dpdk.org/dpdk/latest/source/drivers/net/mlx5/mlx5_utils.h#L28 The code has less duplication, but it requires a less natural declaration of variables http://code.dpdk.org/dpdk/latest/source/drivers/net/mlx5/mlx5.h#L607 Should we take this way? /Gavin > >
30/10/2019 10:55, Gavin Hu (Arm Technology China): > Hi Thomas, > > From: Thomas Monjalon <thomas@monjalon.net> > > 23/10/2019 04:54, Joyce Kong: > > > 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. > > > > Please, could you look at what Adrien did in the Mellanox PMD? > > > > http://code.dpdk.org/dpdk/latest/source/drivers/net/mlx5/mlx5_utils.h#L28 > The code has less duplication, but it requires a less natural declaration of variables > http://code.dpdk.org/dpdk/latest/source/drivers/net/mlx5/mlx5.h#L607 > Should we take this way? I don't know which way is best. I suggested to read this code for 2 reasons: 1. we can be inspired 2. it may be replaced by the new common API as you did for other drivers
On Wed, Oct 30, 2019 at 3:25 PM Gavin Hu (Arm Technology China) <Gavin.Hu@arm.com> wrote: > > Hi Thomas, > > > -----Original Message----- > > From: Thomas Monjalon <thomas@monjalon.net> > > Sent: Wednesday, October 30, 2019 12:43 AM > > To: Joyce Kong (Arm Technology China) <Joyce.Kong@arm.com> > > Cc: dev@dpdk.org; nd <nd@arm.com>; jerinj@marvell.com; > > stephen@networkplumber.org; mb@smartsharesystems.com; Honnappa > > Nagarahalli <Honnappa.Nagarahalli@arm.com>; Gavin Hu (Arm Technology > > China) <Gavin.Hu@arm.com>; ravi1.kumar@amd.com; rmody@marvell.com; > > shshaikh@marvell.com; xuanziyang2@huawei.com; > > cloud.wangxiaoyun@huawei.com; zhouguoyang@huawei.com; > > adrien.mazarguil@6wind.com > > Subject: Re: [dpdk-dev] [PATCH v2 0/6] implement common rte bit operation > > APIs in PMDs > > > > 23/10/2019 04:54, Joyce Kong: > > > 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. > > > > Please, could you look at what Adrien did in the Mellanox PMD? > > > > http://code.dpdk.org/dpdk/latest/source/drivers/net/mlx5/mlx5_utils.h#L28 > The code has less duplication, but it requires a less natural declaration of variables > http://code.dpdk.org/dpdk/latest/source/drivers/net/mlx5/mlx5.h#L607 > Should we take this way? IMO, We need to consider the MACRO based scheme only as of the last resort. > /Gavin > > > > >
> -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jerin Jacob > Sent: Wednesday, October 30, 2019 1:33 PM > > On Wed, Oct 30, 2019 at 3:25 PM Gavin Hu (Arm Technology China) > <Gavin.Hu@arm.com> wrote: > > > > Hi Thomas, > > > > > -----Original Message----- > > > From: Thomas Monjalon <thomas@monjalon.net> > > > Sent: Wednesday, October 30, 2019 12:43 AM > > > > > > 23/10/2019 04:54, Joyce Kong: > > > > 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. > > > > > > Please, could you look at what Adrien did in the Mellanox PMD? > > > > > > > http://code.dpdk.org/dpdk/latest/source/drivers/net/mlx5/mlx5_utils.h#L28 > > The code has less duplication, but it requires a less natural declaration > of variables > > http://code.dpdk.org/dpdk/latest/source/drivers/net/mlx5/mlx5.h#L607 > > Should we take this way? > > > IMO, We need to consider the MACRO based scheme only as of the last resort. > I agree. The EAL library already has an I/O device memory access API, i.e. with functionality closely related to the proposed I/O device bit operation API: http://code.dpdk.org/dpdk/latest/source/lib/librte_eal/common/include/generic/rte_io.h I would prefer a similar approach, and API familiarity would be my strongest argument.
> -----Original Message----- > From: Morten Brørup <mb@smartsharesystems.com> > Sent: Wednesday, October 30, 2019 9:02 PM > To: Jerin Jacob <jerinjacobk@gmail.com>; Gavin Hu (Arm Technology China) > <Gavin.Hu@arm.com> > Cc: thomas@monjalon.net; Joyce Kong (Arm Technology China) > <Joyce.Kong@arm.com>; dev@dpdk.org; nd <nd@arm.com>; > jerinj@marvell.com; stephen@networkplumber.org; Honnappa Nagarahalli > <Honnappa.Nagarahalli@arm.com>; ravi1.kumar@amd.com; > rmody@marvell.com; shshaikh@marvell.com; xuanziyang2@huawei.com; > cloud.wangxiaoyun@huawei.com; zhouguoyang@huawei.com; > adrien.mazarguil@6wind.com > Subject: RE: [dpdk-dev] [PATCH v2 0/6] implement common rte bit operation > APIs in PMDs > > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jerin Jacob > > Sent: Wednesday, October 30, 2019 1:33 PM > > > > On Wed, Oct 30, 2019 at 3:25 PM Gavin Hu (Arm Technology China) > > <Gavin.Hu@arm.com> wrote: > > > > > > Hi Thomas, > > > > > > > -----Original Message----- > > > > From: Thomas Monjalon <thomas@monjalon.net> > > > > Sent: Wednesday, October 30, 2019 12:43 AM > > > > > > > > 23/10/2019 04:54, Joyce Kong: > > > > > 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. > > > > > > > > Please, could you look at what Adrien did in the Mellanox PMD? > > > > > > > > > > http://code.dpdk.org/dpdk/latest/source/drivers/net/mlx5/mlx5_utils.h#L28 > > > The code has less duplication, but it requires a less natural declaration > > of variables > > > http://code.dpdk.org/dpdk/latest/source/drivers/net/mlx5/mlx5.h#L607 > > > Should we take this way? > > > > > > IMO, We need to consider the MACRO based scheme only as of the last resort. > > > > I agree. > > The EAL library already has an I/O device memory access API, i.e. with > functionality closely related to the proposed I/O device bit operation API: > http://code.dpdk.org/dpdk/latest/source/lib/librte_eal/common/include/gene > ric/rte_io.h > > I would prefer a similar approach, and API familiarity would be my strongest > argument. Yes, this is a more natural way, and engineers are more familiar with the APIs. We will take this way as more people vote for this. Thanks Thomas also for your comment, we are inspired by this code, we will add assert() also to guarantee the 'bit' argument is in the valid range. We used this common API for some PMDs, but not extensively, the reason is we want to finalize the API firstly(with your comments coming) and then propagate later. /Gavin