DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jerin Jacob <jerin.jacob@caviumnetworks.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 3/4] eal/arm: Enable lpm/table/pipeline libs
Date: Thu, 3 Dec 2015 18:50:34 +0530	[thread overview]
Message-ID: <20151203131949.GA19561@localhost.localdomain> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB97725836AD02F0@irsmsx105.ger.corp.intel.com>

On Thu, Dec 03, 2015 at 12:42:13PM +0000, Ananyev, Konstantin wrote:
>
>
> > -----Original Message-----
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Thursday, December 03, 2015 12:17 PM
> > To: Ananyev, Konstantin
> > Cc: Thomas Monjalon; dev@dpdk.org; viktorin@rehivetech.com
> > Subject: Re: [dpdk-dev] [PATCH 3/4] eal/arm: Enable lpm/table/pipeline libs
> >
> > On Thu, Dec 03, 2015 at 11:02:07AM +0000, Ananyev, Konstantin wrote:
> >
> > Hi Konstantin,
> >
> > > Hi Jerin,
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jerin Jacob
> > > > Sent: Thursday, December 03, 2015 9:34 AM
> > > > To: Thomas Monjalon
> > > > Cc: dev@dpdk.org
> > > > Subject: Re: [dpdk-dev] [PATCH 3/4] eal/arm: Enable lpm/table/pipeline libs
> > > >
> > > > On Wed, Dec 02, 2015 at 05:57:10PM +0100, Thomas Monjalon wrote:
> > > > > 2015-12-02 22:23, Jerin Jacob:
> > > > > > On Wed, Dec 02, 2015 at 05:40:13PM +0100, Thomas Monjalon wrote:
> > > > > > > 2015-12-02 20:04, Jerin Jacob:
> > > > > > > > On Wed, Dec 02, 2015 at 09:13:51PM +0800, Jianbo Liu wrote:
> > > > > > > > > On 2 December 2015 at 18:39, Jerin Jacob <jerin.jacob@caviumnetworks.com> wrote:
> > > > > > > > > > AND they include "rte_lpm.h"(it internally includes rte_vect.h)
> > > > > > > > > > that lead to multiple definition and its not good.
> > > > > > > > > >
> > > > > > > > > But you will have similar issue since "typedef int32x4_t __m128i"
> > > > > > > > > appears in both your patch and this header file.
> > > > > > > >
> > > > > > > > I just tested it, it won't break, back to back "typedef int32x4_t __m128i"
> > > > > > > > is fine(unlike inline function).
> > > > > > > >
> > > > > > > > my intention to keep __m128i "as is"  because changing the __m128i to rte_???
> > > > > > > > something would break the ABI.
> > > > > > >
> > > > > > > Isn't it already broken in 2.2?
> > > > > >
> > > > > > Does it mean, You would like to have rte_128i(or similar) kind of
> > > > > > abstraction to represent 128bit SIMD variable in DPDK?
> > > > >
> > > > > If you are convinced that it is the best way to write a generic code, yes.
> > > >
> > > > I grep-ed through DPDK API list to see the dependency with SIMD in API
> > > > definition.I see only rte_lpm_lookupx4 API has SIMD dependency in API
> > > > definition.
> > > >
> > > > I believe that's the root cause of the problem. IMO, The
> > > > better way to fix this would be to remove __m128i from API and have more
> > > > general representation to remove the architecture dependency from API
> > > >
> > > > something like this,
> > > >
> > > > rte_lpm_lookupx4(const struct rte_lpm *lpm, uint32_t *ip, uint16_t
> > > > hop[4], uint16_t defv)
> > > >
> > > > instead of
> > > >
> > > > rte_lpm_lookupx4(const struct rte_lpm *lpm, __m128i ip, uint16_t
> > > > hop[4],  uint16_t defv)
> > >
> > > The idea for that function was that rte_lpm_lookupx4() accepts 4 IPv4 addresses that are:
> > > 1. already in 128bit register
> > > 2. 'prepared' - byte swap is already done for them if needed.
> > >
> > > About ways to fix  __m128i dependency: as I can see x86 and arm DPDK code
> > > already has xmm_t typedef:
> > >
> > > $ find lib -type f | xargs grep xmm_t | grep typedef
> > > lib/librte_eal/common/include/arch/x86/rte_vect.h:typedef __m128i xmm_t;
> > > lib/librte_eal/common/include/arch/arm/rte_vect.h:typedef int32x4_t xmm_t;
> > >
> > > Why not to  change rte_lpm_lookupx4() to accept xmm_t as input parameter.
> > > As I understand it would solve the problem, and wouldn't introduce any API/ABI breakage, right?
> >
> > Yes, If we have API/ABI breakage concerns.
> >
> > IMO, Now this would call for some kind of rte_vect_* abstraction load,
> > store, set kind of SIMD operation on xmm_t in common test code to
> > aviod #ifdef's in app/test/test_lpm.c
>
> Yes, seems so.
>
> >
> > I guess we may not need those abstractions in
> > lib/librte_eal/common/include/arch/ directory.
> > keeping in app/test/xmmt_ops.h should be enough, right?
>
> That sounds ok to me.
> At least for now.
> For future the more generic question - do we like to have some
> generic layer abstraction for similar vector instrincts across different archs?
> From one side it might help people writing/using vector implementation of some stuff,
> from other side - there would be extra hassle creating/supporting it.

There are few such libaries avilable on web. example:

NEON -> SSE
https://software.intel.com/sites/default/files/managed/cf/f6/NEONvsSSE.h

SSE -> NEON
https://github.com/jratcliff63367/sse2neon/blob/master/SSE2NEON.h

but coming up with common abstraction will  be difficult as it's not
one to one mapped all the time and performance criteria to choose
the instruction on given architecture to realize a certain logic etc

Jerin

>
> Konstantin
>
> >
> >
> > >
> > > Konstantin
> > >
> > > >
> > > > Now I am not sure why this API was created like this, from l3fwd.c
> > > > example, it looks to accommodate the IPV4 byte swap[1]. If it's true,
> > > > maybe we can have eal byte swap abstraction for optimized byte swap on
> > > > memory for 4 IP address in one shot
> > > >
> > > > or
> > > >
> > > > Have rte_lpm_lookupx4 take an argument for byte swap or not ?
> > > >
> > > > or
> > > >
> > > > something similar?
> > > >
> > > > Thoughts ?
> > > >
> > > > [1]
> > > > const  __m128i bswap_mask = _mm_set_epi8(12, 13, 14, 15, 8, 9, 10, 11,
> > > >                                                 4, 5, 6, 7, 0, 1, 2, 3);
> > > > /* Byte swap 4 IPV4 addresses. */
> > > > dip = _mm_shuffle_epi8(dip, bswap_mask);
> > > >
> > > > Jerin
> > > >
> > > > > I think the most important question is to know what is the best solution
> > > > > for performance and maintainability. The API/ABI questions will be considered
> > > > > after.
> > > > >
> > > > > Thanks for your involvement guys.

  reply	other threads:[~2015-12-03 13:20 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-01 18:41 [dpdk-dev] [PATCH 0/4] support acl/lpm/table/pipeline libs for armv7 and armv8 Jianbo Liu
2015-12-01 12:47 ` Jan Viktorin
2015-12-01 20:56   ` Jianbo Liu
2015-12-01 18:41 ` [dpdk-dev] [PATCH 1/4] eal/arm: use RTE_ARM_EAL_RDTSC_USE_PMU in rte_cycle_32.h Jianbo Liu
2015-12-01 12:41   ` Jan Viktorin
2015-12-01 12:43   ` Jan Viktorin
2015-12-01 18:41 ` [dpdk-dev] [PATCH 2/4] eal/acl: enable acl for armv7-a Jianbo Liu
2015-12-01 14:43   ` Jerin Jacob
2015-12-01 14:46     ` Jan Viktorin
2015-12-02  6:14       ` Jianbo Liu
2015-12-01 18:41 ` [dpdk-dev] [PATCH 3/4] eal/arm: Enable lpm/table/pipeline libs Jianbo Liu
2015-12-01 16:41   ` Jerin Jacob
2015-12-01 17:02     ` Jan Viktorin
2015-12-02  7:02     ` Jianbo Liu
     [not found]     ` <CAP4Qi3-5ofDU-2-4KsxFzMC1OpTsc5WjmxcFT2Eu_URA0UBzDw@mail.gmail.com>
2015-12-02  8:03       ` Jerin Jacob
2015-12-02  9:49         ` Jianbo Liu
2015-12-02 10:33           ` Ananyev, Konstantin
2015-12-02 10:48             ` Jerin Jacob
2015-12-02 13:06               ` Jan Viktorin
2015-12-02 10:39           ` Jerin Jacob
2015-12-02 13:05             ` Jan Viktorin
2015-12-02 13:13             ` Jianbo Liu
2015-12-02 14:34               ` Jerin Jacob
2015-12-02 16:40                 ` Thomas Monjalon
2015-12-02 16:53                   ` Jerin Jacob
2015-12-02 16:57                     ` Thomas Monjalon
2015-12-02 17:38                       ` Jerin Jacob
2015-12-03  9:33                       ` Jerin Jacob
2015-12-03 11:02                         ` Ananyev, Konstantin
2015-12-03 12:17                           ` Jerin Jacob
2015-12-03 12:42                             ` Ananyev, Konstantin
2015-12-03 13:20                               ` Jerin Jacob [this message]
2015-12-01 18:41 ` [dpdk-dev] [PATCH 4/4] maintainers: claim resposibility for ARMv7 and ARMv8 Jianbo Liu
2015-12-01 16:44   ` Jerin Jacob
2015-12-03 15:02 ` [dpdk-dev] [PATCH v2 0/3] support acl lib for armv7-a and a small fix Jianbo Liu
2015-12-03 15:02   ` [dpdk-dev] [PATCH v2 1/3] eal/arm: use RTE_ARM_EAL_RDTSC_USE_PMU in rte_cycle_32.h Jianbo Liu
2015-12-08  1:13     ` Thomas Monjalon
2015-12-03 15:02   ` [dpdk-dev] [PATCH v2 2/3] eal/acl: enable acl for armv7-a Jianbo Liu
2015-12-03 15:13     ` Jerin Jacob
2015-12-08  1:18     ` Thomas Monjalon
2015-12-08  1:50       ` Jianbo Liu
2015-12-08  2:23         ` Thomas Monjalon
2015-12-08  7:56           ` Jianbo Liu
2015-12-08 10:03             ` Thomas Monjalon
2015-12-08 10:21               ` Jianbo Liu
2015-12-08 10:38                 ` Thomas Monjalon
2015-12-08 11:27                   ` Jan Viktorin
2015-12-08 10:25               ` Jan Viktorin
2015-12-03 15:02   ` [dpdk-dev] [PATCH v2 3/3] maintainers: claim resposibility for ARMv7 and ARMv8 Jianbo Liu
2015-12-08  1:24   ` [dpdk-dev] [PATCH v2 0/3] support acl lib for armv7-a and a small fix Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151203131949.GA19561@localhost.localdomain \
    --to=jerin.jacob@caviumnetworks.com \
    --cc=dev@dpdk.org \
    --cc=konstantin.ananyev@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).