[lng-odp] Change in lng/odp[master]: validation: enable bt for scheduling test

2016-07-07 Thread lava-bot (Code Review) 
lava-bot has posted comments on this change.

Change subject: validation: enable bt for scheduling test
..


Patch Set 1: Code-Review-1

"Build Failed 

https://ci.linaro.org/jenkins/job/odp-api-check-gerrit/7/ : [OUTPUT]:"

-- 
To view, visit https://review.linaro.org/13135
To unsubscribe, visit https://review.linaro.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3479b518df39be9ee7beab1ebf98a64f9895457e
Gerrit-PatchSet: 1
Gerrit-Project: lng/odp
Gerrit-Branch: master
Gerrit-Owner: Maxim Uvarov 
Gerrit-Reviewer: lava-bot 
Gerrit-HasComments: No

[lng-odp] Change in lng/odp[master]: validation: enable bt for scheduling test

2016-07-07 Thread lava-bot (Code Review) 
lava-bot has posted comments on this change.

Change subject: validation: enable bt for scheduling test
..


Patch Set 1:

"Build Started https://ci.linaro.org/jenkins/job/odp-api-check-gerrit/7/ "

-- 
To view, visit https://review.linaro.org/13135
To unsubscribe, visit https://review.linaro.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3479b518df39be9ee7beab1ebf98a64f9895457e
Gerrit-PatchSet: 1
Gerrit-Project: lng/odp
Gerrit-Branch: master
Gerrit-Owner: Maxim Uvarov 
Gerrit-Reviewer: lava-bot 
Gerrit-HasComments: No

[lng-odp] Change in lng/odp[master]: linux-generic: add stacktrace

2016-07-07 Thread lava-bot (Code Review) 
lava-bot has posted comments on this change.

Change subject: linux-generic: add stacktrace
..


Patch Set 1: Code-Review-1

"Build Failed 

https://ci.linaro.org/jenkins/job/odp-api-check-gerrit/6/ : [OUTPUT]:"

-- 
To view, visit https://review.linaro.org/13134
To unsubscribe, visit https://review.linaro.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id9df73633583e93dc4fb39f0a90fdf6c7c56a6cc
Gerrit-PatchSet: 1
Gerrit-Project: lng/odp
Gerrit-Branch: master
Gerrit-Owner: Maxim Uvarov 
Gerrit-Reviewer: lava-bot 
Gerrit-HasComments: No

Re: [lng-odp] [PATCH] doc: driver-api-guide: initial revision

2016-07-07 Thread Christophe Milard 
Yes! I DID send a mail!!! It is in my mail history!

I re-include it here again:

On 2016-07-05 08:47, Mike Holmes wrote:
> Add an initial driver API document structure for the existing driver
> framework.
>
here again we have a vocabulary issue: the term "API" is used for "interface",
but actually stands for "Application Programming Interface":
If we talk about "driver API" we should also talk about "application API"
(which would stand for Application Application Programing interface" :-( .

I would prefer "Add an initial driver interface document..."
API is then the name of the north interface and DRV the name of the south
interface: there is not such thing as a "application API", there is just the
API (north) interface and the DRV (south) interface.
This is the choice we already have made in the repo (api and drv prefix)
should eventually be: []--guide, i.e:
DX_INIT_DOXYGEN($PACKAGE_NAME,
${srcdir}/doc/specification-api-guide/Doxyfile,
${builddir}/doc/specification-api-guide/output,
${srcdir}/doc/helper-guide/Doxyfile,
${builddir}/doc/helper-guide/output,
${srcdir}/doc/platform-api-guide/Doxyfile,
${builddir}/doc/platform-api-guide/output,
${srcdir}/doc/specification-drv-guide/Doxyfile,
${builddir}/doc/specification-drv-guide/output)

>
>  ##
>  # Enable/disable ODP_DEBUG_PRINT
> diff --git a/doc/Makefile.am b/doc/Makefile.am
> index d49d84b..59d6a6c 100644
> --- a/doc/Makefile.am
> +++ b/doc/Makefile.am
> @@ -1,4 +1,8 @@
> -SUBDIRS = application-api-guide helper-guide platform-api-guide
> +SUBDIRS = \
> + application-api-guide \
> + helper-guide \
> + platform-api-guide \
> + driver-api-guide

should eventually be:
specification-api-guide
helper-guide
platform-api-guide
specification-drv-guide

>
>  if user_guide
>  SUBDIRS += implementers-guide users-guide process-guide
> diff --git a/doc/driver-api-guide/.gitignore b/doc/driver-api-guide/.gitignore
> new file mode 100644
> index 000..53752db
> --- /dev/null
> +++ b/doc/driver-api-guide/.gitignore
> @@ -0,0 +1 @@
> +output
> diff --git a/doc/driver-api-guide/Doxyfile b/doc/driver-api-guide/Doxyfile
> new file mode 100644
> index 000..eff3285
> --- /dev/null
> +++ b/doc/driver-api-guide/Doxyfile
> @@ -0,0 +1,14 @@
> +@INCLUDE = $(SRCDIR)/doc/Doxyfile_common
> +
> +PROJECT_NAME = "Driver Reference Manual"

Should be "Driver Interface (drv) Reference Manual"
You can ommit the "(drv)" but this is the doc for the intervace, not a driver.
should be:
AC_CONFIG_FILES([doc/specification-api-guide/Makefile
 doc/helper-guide/Makefile
 doc/implementers-guide/Makefile
 doc/Makefile
 doc/platform-api-guide/Makefile
 doc/process-guide/Makefile
 doc/users-guide/Makefile
 doc/specification-drv-guide/Makefile])

I am aware that this patch should not touch old names (otherwise it will go
outside what its commit msg says), so part of the naming inconsistency will
remain, but if you agree with the suggested names, maybe you can change the
patch so it goes in the right direction.
If you don't agree, I think I could cope with any other suggestion as long as
it remain consistent. Remember as well that the interface prefixes in the repo
are "api" and "drv".

Thanks for helping the driver iinterface to be :-)

On 7 July 2016 at 15:49, Mike Holmes  wrote:
>
>
> On 6 July 2016 at 21:29, Bill Fischofer  wrote:
>>
>> This looks good, but I think it makes more sense for this to be API-NEXT
>> and considered part of Tiger Moth since we're not doing anything at all with
>> drivers in Monarch and this inclusion could be confusing to readers.
>
>
> I agree, it can be one of the first TigerMoth changes.
> I think Christophe has comments but I have not seen a mail - Christophe
> anything to add since this is your domain ?
>
>>
>>
>>
>> On Tue, Jul 5, 2016 at 7:47 AM, Mike Holmes 
>> wrote:
>>>
>>> Add an initial driver API document structure for the existing driver
>>> framework.
>>>
>>> Signed-off-by: Mike Holmes 
>>
>>
>> Reviewed-and-tested-by: Bill Fischofer 
>>
>>>
>>> ---
>>>  configure.ac |  4 +++-
>>>  doc/Makefile.am  |  6 +-
>>>  doc/driver-api-guide/.gitignore  |  1 +
>>>  doc/driver-api-guide/Doxyfile| 14 ++
>>>  doc/driver-api-guide/Makefile.am |  5 +
>>>  doc/driver-api-guide/odp.dox | 20 
>>>  doc/m4/configure.m4  |  3 ++-
>>>  7 files changed, 50 insertions(+), 3 deletions(-)
>>>  create mode 100644 doc/driver-api-guide/.gitignore
>>>  create mode 100644 doc/driver-api-guide/Doxyfile
>>>

[lng-odp] [PATCH v2] test: l2fwd: prefetch packet data

2016-07-07 Thread Matias Elo 
Prefetch ethernet addresses. This is a no-op on odp-linux but improves
performance significantly on odp-dpdk implementation.

Signed-off-by: Matias Elo 
---

V2:
- Use ODPH_ETHHDR_LEN define (Maxim)

 test/performance/odp_l2fwd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/test/performance/odp_l2fwd.c b/test/performance/odp_l2fwd.c
index 164f7f5..e296b94 100644
--- a/test/performance/odp_l2fwd.c
+++ b/test/performance/odp_l2fwd.c
@@ -255,6 +255,9 @@ static inline void fill_eth_addrs(odp_packet_t pkt_tbl[],
 
for (i = 0; i < num; ++i) {
pkt = pkt_tbl[i];
+
+   odp_packet_prefetch(pkt, 0, ODPH_ETHHDR_LEN);
+
if (odp_packet_has_eth(pkt)) {
eth = (odph_ethhdr_t *)odp_packet_l2_ptr(pkt, NULL);
 
-- 
1.9.1

Re: [lng-odp] [PATCH] test: l2fwd: prefetch packet data

2016-07-07 Thread Maxim Uvarov 

On 07/07/16 16:56, Matias Elo wrote:

Prefetch ethernet addresses. This is a no-op on odp-linux but improves
performance significantly on odp-dpdk implementation.

Signed-off-by: Matias Elo 
---
  test/performance/odp_l2fwd.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/test/performance/odp_l2fwd.c b/test/performance/odp_l2fwd.c
index 164f7f5..cd4789f 100644
--- a/test/performance/odp_l2fwd.c
+++ b/test/performance/odp_l2fwd.c
@@ -255,6 +255,10 @@ static inline void fill_eth_addrs(odp_packet_t pkt_tbl[],
  
  	for (i = 0; i < num; ++i) {

pkt = pkt_tbl[i];
+
+   /* Prefetch ethernet addresses */
+   odp_packet_prefetch(pkt, 0, 12);
+

_ODP_ETHHDR_LEN ?



if (odp_packet_has_eth(pkt)) {
eth = (odph_ethhdr_t *)odp_packet_l2_ptr(pkt, NULL);

Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield order doxygen omissions

2016-07-07 Thread Savolainen, Petri (Nokia - FI/Espoo) 


From: Bill Fischofer [mailto:bill.fischo...@linaro.org] 
Sent: Thursday, July 07, 2016 5:08 PM
To: Mike Holmes 
Cc: Savolainen, Petri (Nokia - FI/Espoo) 
; lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield order 
doxygen omissions



On Thu, Jul 7, 2016 at 9:05 AM, Mike Holmes  wrote:
If it can't be done in a backwards compatible way this has to go to TigerMoth.

Do we patch in Monarch and fix properly there ?

This should be fixed in Monarch. I'd prefer to fix this without reverting 
Petri's header restructure patch series, but that's what introduced this 
problem.


<>

Only way to fix it, is to remove doxygen documentation of 
ODP_LITTLE_ENDIAN_BITFIELD when it is not defined, and vice versa with 
ODP_BIG_ENDIAN_BITFIELD. I'm not sure why doxygen was happy before ... both BIG 
and LITTLE were documented, but only one of those  should have been #defined.

I'd prefer API fix to Monarch, so that ENDIAN and BITFIELD defines work the 
same way. 

-Petri

Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield order doxygen omissions

2016-07-07 Thread Bill Fischofer 
On Thu, Jul 7, 2016 at 9:05 AM, Mike Holmes  wrote:

> If it can't be done in a backwards compatible way this has to go to
> TigerMoth.
>
> Do we patch in Monarch and fix properly there ?
>

This should be fixed in Monarch. I'd prefer to fix this without reverting
Petri's header restructure patch series, but that's what introduced this
problem.


>
> Mike
>
> On 7 July 2016 at 09:48, Bill Fischofer  wrote:
>
>> On Thu, Jul 7, 2016 at 8:38 AM, Savolainen, Petri (Nokia - FI/Espoo) <
>> petri.savolai...@nokia-bell-labs.com> wrote:
>>
>> >
>> >
>> > From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
>> > Sent: Thursday, July 07, 2016 3:01 PM
>> > To: Savolainen, Petri (Nokia - FI/Espoo) <
>> > petri.savolai...@nokia-bell-labs.com>
>> > Cc: lng-odp@lists.linaro.org
>> > Subject: Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield
>> > order doxygen omissions
>> >
>> >
>> >
>> > On Thu, Jul 7, 2016 at 6:57 AM, Savolainen, Petri (Nokia - FI/Espoo) <
>> > petri.savolai...@nokia-bell-labs.com> wrote:
>> >
>> >
>> > From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
>> > Sent: Thursday, July 07, 2016 2:42 PM
>> > To: Savolainen, Petri (Nokia - FI/Espoo) <
>> > petri.savolai...@nokia-bell-labs.com>
>> > Cc: lng-odp@lists.linaro.org
>> > Subject: Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield
>> > order doxygen omissions
>> >
>> >
>> > On Thu, Jul 7, 2016 at 2:46 AM, Savolainen, Petri (Nokia - FI/Espoo) <
>> > petri.savolai...@nokia-bell-labs.com> wrote:
>> > I'm OK with the change, but didn't do it as part of my patch set since
>> > it's an API change. I think the change is not a big deal and should be
>> done
>> > to align the usage of byte/bitfield defines. The patch should be tagged
>> > with api: prefix and go through api-next.
>> >
>> > Also, API spec could be more explicit about the byte/bitfield defines
>> and
>> > how application should use/test those. I think bitfield defines should
>> work
>> > the same way as the current byte order defines.
>> >
>> > #if (ODP_BYTE_ORDER == ODP_BIG_ENDIAN)
>> > // big endian byte order
>> > #else
>> > // little endian byte order
>> > #endif
>> >
>> >
>> > -Petri
>> >
>> > Not making an API change was the reason I did this change this way. This
>> > is now an implementation change only.  The change that would involve an
>> API
>> > change would have been to define an ODP_BITFIELD_ORDER variable to
>> mirror
>> > the ODP_BYTE_ORDER variable and set that to either
>> > ODP_LITTLE_ENDIAN_BITFIELD or ODP_BIG_ENDIAN_BITFIELD as needed.
>> >
>> >
>> > > --- a/platform/linux-generic/include/protocols/tcp.h
>> > > +++ b/platform/linux-generic/include/protocols/tcp.h
>> > > @@ -34,7 +34,7 @@ typedef struct ODP_PACKED {
>> > >   odp_u32be_t ack_no;   /**< Acknowledgment number */
>> > >   union {
>> > >   odp_u16be_t doffset_flags;
>> > > -#if defined(ODP_BIG_ENDIAN_BITFIELD)
>> > > +#if ODP_BIG_ENDIAN_BITFIELD
>> >
>> >
>> > The change is non-backward, API change for application code. Tcp helper
>> is
>> > also application code.
>> >
>> > Today, application may do this half of the time ...
>> >
>> > #if defined(ODP_BIG_ENDIAN_BITFIELD)
>> > // foo
>> > #else
>> > // bar
>> > #endif
>> >
>> > ... and this the other half ...
>> >
>> > #if defined(ODP_LITTLE_ENDIAN_BITFIELD)
>> > // foo
>> > #else
>> > // bar
>> > #endif
>> >
>> >
>> > After the change, application would break since both
>> > ODP_LITTLE_ENDIAN_BITFIELD and ODP_BIG_ENDIAN_BITFIELD are defined
>> always.
>> >
>> > Yes, which is why the patch includes an update to both copies of tcp.h,
>> > which are currently the only files that reference those #defines.
>> >
>> > The issue is that the recent change seems to have broken doxygen, so the
>> > question is what's the best way to fix that?
>> >
>> >
>> > << Reply to HTML mail>>
>> > You must consider helpers as part of application code. There may be
>> other
>> > application code out there which does exactly the same thing (the #if
>> > defined example above). This change would break *any* application code
>> that
>> > uses the defines.
>> >
>>
>> So short of reverting your latest header restructure patch, how would you
>> suggest fixing this?
>>
>>
>> >
>> > -Petri
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>>
>
>
>
> --
> Mike Holmes
> Technical Manager - Linaro Networking Group
> Linaro.org  *│ *Open source software for ARM SoCs
> "Work should be fun and collaborative, the rest follows"
>
>
>

Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield order doxygen omissions

2016-07-07 Thread Mike Holmes 
If it can't be done in a backwards compatible way this has to go to
TigerMoth.

Do we patch in Monarch and fix properly there ?

Mike

On 7 July 2016 at 09:48, Bill Fischofer  wrote:

> On Thu, Jul 7, 2016 at 8:38 AM, Savolainen, Petri (Nokia - FI/Espoo) <
> petri.savolai...@nokia-bell-labs.com> wrote:
>
> >
> >
> > From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
> > Sent: Thursday, July 07, 2016 3:01 PM
> > To: Savolainen, Petri (Nokia - FI/Espoo) <
> > petri.savolai...@nokia-bell-labs.com>
> > Cc: lng-odp@lists.linaro.org
> > Subject: Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield
> > order doxygen omissions
> >
> >
> >
> > On Thu, Jul 7, 2016 at 6:57 AM, Savolainen, Petri (Nokia - FI/Espoo) <
> > petri.savolai...@nokia-bell-labs.com> wrote:
> >
> >
> > From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
> > Sent: Thursday, July 07, 2016 2:42 PM
> > To: Savolainen, Petri (Nokia - FI/Espoo) <
> > petri.savolai...@nokia-bell-labs.com>
> > Cc: lng-odp@lists.linaro.org
> > Subject: Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield
> > order doxygen omissions
> >
> >
> > On Thu, Jul 7, 2016 at 2:46 AM, Savolainen, Petri (Nokia - FI/Espoo) <
> > petri.savolai...@nokia-bell-labs.com> wrote:
> > I'm OK with the change, but didn't do it as part of my patch set since
> > it's an API change. I think the change is not a big deal and should be
> done
> > to align the usage of byte/bitfield defines. The patch should be tagged
> > with api: prefix and go through api-next.
> >
> > Also, API spec could be more explicit about the byte/bitfield defines and
> > how application should use/test those. I think bitfield defines should
> work
> > the same way as the current byte order defines.
> >
> > #if (ODP_BYTE_ORDER == ODP_BIG_ENDIAN)
> > // big endian byte order
> > #else
> > // little endian byte order
> > #endif
> >
> >
> > -Petri
> >
> > Not making an API change was the reason I did this change this way. This
> > is now an implementation change only.  The change that would involve an
> API
> > change would have been to define an ODP_BITFIELD_ORDER variable to mirror
> > the ODP_BYTE_ORDER variable and set that to either
> > ODP_LITTLE_ENDIAN_BITFIELD or ODP_BIG_ENDIAN_BITFIELD as needed.
> >
> >
> > > --- a/platform/linux-generic/include/protocols/tcp.h
> > > +++ b/platform/linux-generic/include/protocols/tcp.h
> > > @@ -34,7 +34,7 @@ typedef struct ODP_PACKED {
> > >   odp_u32be_t ack_no;   /**< Acknowledgment number */
> > >   union {
> > >   odp_u16be_t doffset_flags;
> > > -#if defined(ODP_BIG_ENDIAN_BITFIELD)
> > > +#if ODP_BIG_ENDIAN_BITFIELD
> >
> >
> > The change is non-backward, API change for application code. Tcp helper
> is
> > also application code.
> >
> > Today, application may do this half of the time ...
> >
> > #if defined(ODP_BIG_ENDIAN_BITFIELD)
> > // foo
> > #else
> > // bar
> > #endif
> >
> > ... and this the other half ...
> >
> > #if defined(ODP_LITTLE_ENDIAN_BITFIELD)
> > // foo
> > #else
> > // bar
> > #endif
> >
> >
> > After the change, application would break since both
> > ODP_LITTLE_ENDIAN_BITFIELD and ODP_BIG_ENDIAN_BITFIELD are defined
> always.
> >
> > Yes, which is why the patch includes an update to both copies of tcp.h,
> > which are currently the only files that reference those #defines.
> >
> > The issue is that the recent change seems to have broken doxygen, so the
> > question is what's the best way to fix that?
> >
> >
> > << Reply to HTML mail>>
> > You must consider helpers as part of application code. There may be other
> > application code out there which does exactly the same thing (the #if
> > defined example above). This change would break *any* application code
> that
> > uses the defines.
> >
>
> So short of reverting your latest header restructure patch, how would you
> suggest fixing this?
>
>
> >
> > -Petri
> >
> >
> >
> >
> >
> >
> >
> >
>



-- 
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org  *│ *Open source software for ARM SoCs
"Work should be fun and collaborative, the rest follows"

[lng-odp] [PATCH] test: l2fwd: prefetch packet data

2016-07-07 Thread Matias Elo 
Prefetch ethernet addresses. This is a no-op on odp-linux but improves
performance significantly on odp-dpdk implementation.

Signed-off-by: Matias Elo 
---
 test/performance/odp_l2fwd.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/test/performance/odp_l2fwd.c b/test/performance/odp_l2fwd.c
index 164f7f5..cd4789f 100644
--- a/test/performance/odp_l2fwd.c
+++ b/test/performance/odp_l2fwd.c
@@ -255,6 +255,10 @@ static inline void fill_eth_addrs(odp_packet_t pkt_tbl[],
 
for (i = 0; i < num; ++i) {
pkt = pkt_tbl[i];
+
+   /* Prefetch ethernet addresses */
+   odp_packet_prefetch(pkt, 0, 12);
+
if (odp_packet_has_eth(pkt)) {
eth = (odph_ethhdr_t *)odp_packet_l2_ptr(pkt, NULL);
 
-- 
1.9.1

Re: [lng-odp] [PATCH] doc: driver-api-guide: initial revision

2016-07-07 Thread Mike Holmes 
On 6 July 2016 at 21:29, Bill Fischofer  wrote:

> This looks good, but I think it makes more sense for this to be API-NEXT
> and considered part of Tiger Moth since we're not doing anything at all
> with drivers in Monarch and this inclusion could be confusing to readers.
>

I agree, it can be one of the first TigerMoth changes.
I think Christophe has comments but I have not seen a mail - Christophe
anything to add since this is your domain ?


>
>
> On Tue, Jul 5, 2016 at 7:47 AM, Mike Holmes 
> wrote:
>
>> Add an initial driver API document structure for the existing driver
>> framework.
>>
>> Signed-off-by: Mike Holmes 
>>
>
> Reviewed-and-tested-by: Bill Fischofer 
>
>
>> ---
>>  configure.ac |  4 +++-
>>  doc/Makefile.am  |  6 +-
>>  doc/driver-api-guide/.gitignore  |  1 +
>>  doc/driver-api-guide/Doxyfile| 14 ++
>>  doc/driver-api-guide/Makefile.am |  5 +
>>  doc/driver-api-guide/odp.dox | 20 
>>  doc/m4/configure.m4  |  3 ++-
>>  7 files changed, 50 insertions(+), 3 deletions(-)
>>  create mode 100644 doc/driver-api-guide/.gitignore
>>  create mode 100644 doc/driver-api-guide/Doxyfile
>>  create mode 100644 doc/driver-api-guide/Makefile.am
>>  create mode 100644 doc/driver-api-guide/odp.dox
>>
>> diff --git a/configure.ac b/configure.ac
>> index c0eb207..d1e410a 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -188,7 +188,9 @@ DX_INIT_DOXYGEN($PACKAGE_NAME,
>> ${srcdir}/doc/helper-guide/Doxyfile,
>> ${builddir}/doc/helper-guide/output,
>> ${srcdir}/doc/platform-api-guide/Doxyfile,
>> -   ${builddir}/doc/platform-api-guide/output)
>> +   ${builddir}/doc/platform-api-guide/output,
>> +   ${srcdir}/doc/driver-api-guide/Doxyfile,
>> +   ${builddir}/doc/driver-api-guide/output)
>>
>>
>>  ##
>>  # Enable/disable ODP_DEBUG_PRINT
>> diff --git a/doc/Makefile.am b/doc/Makefile.am
>> index d49d84b..59d6a6c 100644
>> --- a/doc/Makefile.am
>> +++ b/doc/Makefile.am
>> @@ -1,4 +1,8 @@
>> -SUBDIRS = application-api-guide helper-guide platform-api-guide
>> +SUBDIRS = \
>> +   application-api-guide \
>> +   helper-guide \
>> +   platform-api-guide \
>> +   driver-api-guide
>>
>>  if user_guide
>>  SUBDIRS += implementers-guide users-guide process-guide
>> diff --git a/doc/driver-api-guide/.gitignore
>> b/doc/driver-api-guide/.gitignore
>> new file mode 100644
>> index 000..53752db
>> --- /dev/null
>> +++ b/doc/driver-api-guide/.gitignore
>> @@ -0,0 +1 @@
>> +output
>> diff --git a/doc/driver-api-guide/Doxyfile b/doc/driver-api-guide/Doxyfile
>> new file mode 100644
>> index 000..eff3285
>> --- /dev/null
>> +++ b/doc/driver-api-guide/Doxyfile
>> @@ -0,0 +1,14 @@
>> +@INCLUDE = $(SRCDIR)/doc/Doxyfile_common
>> +
>> +PROJECT_NAME = "Driver Reference Manual"
>> +PROJECT_NUMBER = $(VERSION)
>> +PROJECT_LOGO = $(SRCDIR)/doc/images/ODP-Logo-HQ.svg
>> +INPUT = $(SRCDIR)/doc/driver-api-guide \
>> +   $(SRCDIR)/include/odp/drv \
>> +   $(SRCDIR)/include/odp_drv.h
>> +EXCLUDE_PATTERNS = drv* odp_drv.h
>> +EXAMPLE_PATH = $(SRCDIR)/example $(SRCDIR)
>> +PREDEFINED = __GNUC__ \
>> +"ODP_HANDLE_T(type)=odp_handle_t type" \
>> +odpdrv_bool_t=int
>> +WARNINGS = NO
>> diff --git a/doc/driver-api-guide/Makefile.am
>> b/doc/driver-api-guide/Makefile.am
>> new file mode 100644
>> index 000..4fc4755
>> --- /dev/null
>> +++ b/doc/driver-api-guide/Makefile.am
>> @@ -0,0 +1,5 @@
>> +EXTRA_DIST = \
>> +odp.dox
>> +
>> +clean-local:
>> +   rm -rf output
>> diff --git a/doc/driver-api-guide/odp.dox b/doc/driver-api-guide/odp.dox
>> new file mode 100644
>> index 000..687a79e
>> --- /dev/null
>> +++ b/doc/driver-api-guide/odp.dox
>> @@ -0,0 +1,20 @@
>> +/* Copyright (c) 2016, Linaro Limited
>> + * All rights reserved
>> + *
>> + * SPDX-License-Identifier: BSD-3-Clause
>> + */
>> +
>> +/**
>> + * @mainpage
>> + *
>> + * @section sec_1 Introduction
>> + *
>> + * OpenDataPlane (ODP) provides a driver interface
>> +
>> + *
>> + * @section contact Contact Details
>> + * - The main web site is http://www.opendataplane.org/
>> + * - The git repo is https://git.linaro.org/lng/odp.git
>> + * - Bug tracking is
>> https://bugs.linaro.org/buglist.cgi?product=OpenDataPlane
>> + *
>> + */
>> diff --git a/doc/m4/configure.m4 b/doc/m4/configure.m4
>> index ed9451d..6e02f76 100644
>> --- a/doc/m4/configure.m4
>> +++ b/doc/m4/configure.m4
>> @@ -42,4 +42,5 @@ AC_CONFIG_FILES([doc/application-api-guide/Makefile
>>  doc/Makefile
>>  doc/platform-api-guide/Makefile
>>  doc/process-guide/Makefile
>> -doc/users-guide/Makefile])
>> +

Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield order doxygen omissions

2016-07-07 Thread Bill Fischofer 
On Thu, Jul 7, 2016 at 8:38 AM, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolai...@nokia-bell-labs.com> wrote:

>
>
> From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
> Sent: Thursday, July 07, 2016 3:01 PM
> To: Savolainen, Petri (Nokia - FI/Espoo) <
> petri.savolai...@nokia-bell-labs.com>
> Cc: lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield
> order doxygen omissions
>
>
>
> On Thu, Jul 7, 2016 at 6:57 AM, Savolainen, Petri (Nokia - FI/Espoo) <
> petri.savolai...@nokia-bell-labs.com> wrote:
>
>
> From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
> Sent: Thursday, July 07, 2016 2:42 PM
> To: Savolainen, Petri (Nokia - FI/Espoo) <
> petri.savolai...@nokia-bell-labs.com>
> Cc: lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield
> order doxygen omissions
>
>
> On Thu, Jul 7, 2016 at 2:46 AM, Savolainen, Petri (Nokia - FI/Espoo) <
> petri.savolai...@nokia-bell-labs.com> wrote:
> I'm OK with the change, but didn't do it as part of my patch set since
> it's an API change. I think the change is not a big deal and should be done
> to align the usage of byte/bitfield defines. The patch should be tagged
> with api: prefix and go through api-next.
>
> Also, API spec could be more explicit about the byte/bitfield defines and
> how application should use/test those. I think bitfield defines should work
> the same way as the current byte order defines.
>
> #if (ODP_BYTE_ORDER == ODP_BIG_ENDIAN)
> // big endian byte order
> #else
> // little endian byte order
> #endif
>
>
> -Petri
>
> Not making an API change was the reason I did this change this way. This
> is now an implementation change only.  The change that would involve an API
> change would have been to define an ODP_BITFIELD_ORDER variable to mirror
> the ODP_BYTE_ORDER variable and set that to either
> ODP_LITTLE_ENDIAN_BITFIELD or ODP_BIG_ENDIAN_BITFIELD as needed.
>
>
> > --- a/platform/linux-generic/include/protocols/tcp.h
> > +++ b/platform/linux-generic/include/protocols/tcp.h
> > @@ -34,7 +34,7 @@ typedef struct ODP_PACKED {
> >   odp_u32be_t ack_no;   /**< Acknowledgment number */
> >   union {
> >   odp_u16be_t doffset_flags;
> > -#if defined(ODP_BIG_ENDIAN_BITFIELD)
> > +#if ODP_BIG_ENDIAN_BITFIELD
>
>
> The change is non-backward, API change for application code. Tcp helper is
> also application code.
>
> Today, application may do this half of the time ...
>
> #if defined(ODP_BIG_ENDIAN_BITFIELD)
> // foo
> #else
> // bar
> #endif
>
> ... and this the other half ...
>
> #if defined(ODP_LITTLE_ENDIAN_BITFIELD)
> // foo
> #else
> // bar
> #endif
>
>
> After the change, application would break since both
> ODP_LITTLE_ENDIAN_BITFIELD and ODP_BIG_ENDIAN_BITFIELD are defined always.
>
> Yes, which is why the patch includes an update to both copies of tcp.h,
> which are currently the only files that reference those #defines.
>
> The issue is that the recent change seems to have broken doxygen, so the
> question is what's the best way to fix that?
>
>
> << Reply to HTML mail>>
> You must consider helpers as part of application code. There may be other
> application code out there which does exactly the same thing (the #if
> defined example above). This change would break *any* application code that
> uses the defines.
>

So short of reverting your latest header restructure patch, how would you
suggest fixing this?


>
> -Petri
>
>
>
>
>
>
>
>

Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield order doxygen omissions

2016-07-07 Thread Savolainen, Petri (Nokia - FI/Espoo) 


From: Bill Fischofer [mailto:bill.fischo...@linaro.org] 
Sent: Thursday, July 07, 2016 3:01 PM
To: Savolainen, Petri (Nokia - FI/Espoo) 
Cc: lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield order 
doxygen omissions



On Thu, Jul 7, 2016 at 6:57 AM, Savolainen, Petri (Nokia - FI/Espoo) 
 wrote:


From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
Sent: Thursday, July 07, 2016 2:42 PM
To: Savolainen, Petri (Nokia - FI/Espoo) 
Cc: lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield order 
doxygen omissions


On Thu, Jul 7, 2016 at 2:46 AM, Savolainen, Petri (Nokia - FI/Espoo) 
 wrote:
I'm OK with the change, but didn't do it as part of my patch set since it's an 
API change. I think the change is not a big deal and should be done to align 
the usage of byte/bitfield defines. The patch should be tagged with api: prefix 
and go through api-next.

Also, API spec could be more explicit about the byte/bitfield defines and how 
application should use/test those. I think bitfield defines should work the 
same way as the current byte order defines.

#if (ODP_BYTE_ORDER == ODP_BIG_ENDIAN)
// big endian byte order
#else
// little endian byte order
#endif


-Petri

Not making an API change was the reason I did this change this way. This is now 
an implementation change only.  The change that would involve an API change 
would have been to define an ODP_BITFIELD_ORDER variable to mirror the 
ODP_BYTE_ORDER variable and set that to either ODP_LITTLE_ENDIAN_BITFIELD or 
ODP_BIG_ENDIAN_BITFIELD as needed.


> --- a/platform/linux-generic/include/protocols/tcp.h
> +++ b/platform/linux-generic/include/protocols/tcp.h
> @@ -34,7 +34,7 @@ typedef struct ODP_PACKED {
>       odp_u32be_t ack_no;   /**< Acknowledgment number */
>       union {
>               odp_u16be_t doffset_flags;
> -#if defined(ODP_BIG_ENDIAN_BITFIELD)
> +#if ODP_BIG_ENDIAN_BITFIELD


The change is non-backward, API change for application code. Tcp helper is also 
application code.

Today, application may do this half of the time ...

#if defined(ODP_BIG_ENDIAN_BITFIELD)
// foo
#else
// bar
#endif

... and this the other half ...

#if defined(ODP_LITTLE_ENDIAN_BITFIELD)
// foo
#else
// bar
#endif


After the change, application would break since both ODP_LITTLE_ENDIAN_BITFIELD 
and ODP_BIG_ENDIAN_BITFIELD are defined always.

Yes, which is why the patch includes an update to both copies of tcp.h, which 
are currently the only files that reference those #defines.

The issue is that the recent change seems to have broken doxygen, so the 
question is what's the best way to fix that?


<< Reply to HTML mail>>
You must consider helpers as part of application code. There may be other 
application code out there which does exactly the same thing (the #if defined 
example above). This change would break *any* application code that uses the 
defines.
 
-Petri

[lng-odp] [Bug 2309] Timer validation test failure

2016-07-07 Thread bugzilla-daemon 
https://bugs.linaro.org/show_bug.cgi?id=2309

Mike Holmes  changed:

   What|Removed |Added

 CC||petri.savolai...@linaro.org

--- Comment #4 from Mike Holmes  ---
Petri can you retest with the latest master ?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are the assignee for the bug.

[lng-odp] [Bug 2282] timer fails

2016-07-07 Thread bugzilla-daemon 
https://bugs.linaro.org/show_bug.cgi?id=2282

Mike Holmes  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |NON REPRODUCIBLE
 CC||mike.hol...@linaro.org

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are on the CC list for the bug.

Re: [lng-odp] [PATCH v5] example: introducing l3fwd

2016-07-07 Thread Elo, Matias (Nokia - FI/Espoo) 
Comments below.

-Matias

> > > +#define MAX_NB_RX_QUEUE  (MAX_NB_WORKER)
> > > +#define MAX_NB_TX_QUEUE  (MAX_NB_WORKER)
> > > +#define MAX_NB_QCONF_PER_CORE(MAX_NB_PKTIO)
> >
> > You could remove the MAX_NB_RX_QUEUE & MAX_NB_TX_QUEUE &
> > MAX_NB_QCONF_PER_CORE defines altogether and use the
> > MAX_NB_WORKER & MAX_NB_PKTIO defines directly.
> >
> 
> Maybe these numbers are not same as MAX_NB_WORKER

In this case you should add separate checks for index out of bounds
for arrays which use these defines.

Btw. you should add a check to make sure that
'args->worker_count < MAX_NB_WORKER'.

> > > +
> > > + eth->dst = global.eth_dest_mac[dif];
> >
> > It seems like the destination MAC is not read from the routing table.
> 
> The dest mac in routing table is populated into this array while using lpm.
> In this case, not find entry in routing table. Just doing "dest ip > dest
> port" mapping.

Okay, so you can have only one destination mac per interface when using lpm?

> > > +static void l3fwd_one_queue(uint32_t sif, uint8_t rxq_id) {
> > > + struct l3fwd_pktio_s *port;
> > > + odp_packet_t *tbl;
> > > + odp_pktout_queue_t outq;
> > > + odp_packet_t pkt_tbl[MAX_PKT_BURST];
> > > + int pkts, drop, sent;
> > > + int dif, dst_port;
> > > + int i;
> > > +
> > > + port = _pktios[sif];
> > > + pkts = odp_pktin_recv(port->ifin[rxq_id], pkt_tbl, MAX_PKT_BURST);
> > > + if (pkts <= 0)
> > > + return;
> > > +
> > > + drop = drop_err_pkts(pkt_tbl, pkts);
> > > + pkts -= drop;
> > > +
> > > + dif = global.fwd_func(pkt_tbl[0], sif);
> > > + tbl = _tbl[0];
> > > + while (pkts) {
> > > + dst_port = dif;
> > > + for (i = 1; i < pkts; i++) {
> > > + dif = global.fwd_func(tbl[i], sif);
> > > + if (dif != dst_port)
> > > + break;
> > > + }
> > > + outq = global.l3fwd_pktios[dst_port].ifout[rxq_id];
> > > + sent = odp_pktout_send(outq, tbl, i);
> >
> >
> > This looks like it will break if multiple worker threads are sending to the
> same
> > interface at the same time. Instead of using 'rxq_id' you should use
> > odp_thread_id() and allocate a private tx queue for each worker.
> >
> 
> The threads are distributed among queues, no two threads handle same queue.
> If share queue, different rxq_id is bound to the same queue.

This works in the receive side but not in the transmit side. Threads may 
transmit packets
to any interface based on the routing decision. Because of this each thread 
requires a
private tx queue for each interface to enable safe lockless operation.

> > > + if (args->qconf_count == 0) {
> > > + if (nb_worker > args->if_count) {
> > > + printf("Warning: too much threads, "
> > > +"truncate to port number: %d\n",
> args->if_count);
> > > + nb_worker = args->if_count;
> > > + }
> >
> > You could allocate multiple rx queues per interface if necessary. This way
> the
> > application would scale automatically when adding more processing threads.
> It
> > is cumbersome for the user to manually do the port/queue/thread mapping.
> 
> The port/queue/core mapping is used by dpdk-l3fwd, I was told to provide an
> equivalent.
> However, it could be discussed to support or not in odp.

Simply modifying the default config should do the trick (+ remove the check 
above the loop):

E.g.
>+  for (i = 0; i < nb_worker; i++) {
>+  q = >qconf_config[i];
>+  q->core_idx = i;
>+  q->rxq_idx = i / args->if_count;
>+  q->if_idx = i % args->if_count;
>+  }

I tested this and the performance still doesn't scale when thread count is 
increased, so there
must be some other issue somewhere.

> 
> >
> > > +
> > > + for (i = 0; i < nb_worker; i++) {
> > > + q = >qconf_config[i];
> > > + q->core_idx = i;
> > > + q->rxq_idx = 0;
> > > + q->if_idx = i;
> > > + }
> > > +
> >
> > If 'nb_worker < args->if_count' traffic is not handled from all interfaces.
> >
> Will be fixed.
> 
> > > +void resolve_fwd_db(char *intf, int portid, uint8_t *mac) {
> > > + fwd_db_entry_t *entry;
> > > +
> > > + /* Walk the list and attempt to set output and MAC */
> > > + for (entry = fwd_db->list; NULL != entry; entry = entry->next) {
> > > + if (strcmp(intf, entry->oif))
> > > + continue;
> > > +
> > > + entry->oif_id = portid;
> > > + memcpy(entry->src_mac.addr, mac, ODPH_ETHADDR_LEN);
> >
> > If no destination MAC address was given as argument you should pick one. For
> > example 02:00:00:00:00:XX where XX is entry->oif_id.
> >
> > > +char *mac_addr_str(char *b, odph_ethaddr_t *mac) {
> > > + uint8_t *byte;
> > > +
> > > + byte = mac->addr;
> > > + sprintf(b, "%02X.%02X.%02X.%02X.%02X.%02X",
> > > + byte[0], byte[1], byte[2], byte[3], byte[4], byte[5]);
>

[lng-odp] Change in lng/odp[master]: linux-generic: add stacktrace

2016-07-07 Thread Maxim Uvarov (Code Review) 
Maxim Uvarov has uploaded a new change for review.

  https://review.linaro.org/13134

Change subject: linux-generic: add stacktrace
..

linux-generic: add stacktrace

Add initial version for more verbose stacktrace for linux-
generic odp platform.
Usage:

export ODP_STACKTRACE_ENABLE=y

Thread 0 terminated with signal=11 (SIGSEGV)
./test/performance/odp_scheduling[0x4250b3]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x10330)[0x7f7ae5cc1330]
./test/performance/odp_scheduling[0x40a064]
./test/performance/odp_scheduling(main+0x2c1)[0x40a5dc]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5)[0x7f7ae590df45]
./test/performance/odp_scheduling[0x408ed9]
odp_stacktrace.c:57:fault_handler():Program interrupted
Aborted

Then convert address to function symbol with add2line:

addr2line  -e ./test/performance/odp_scheduling 0x4250b3
/opt/Linaro/odp3.git/platform/linux-generic/odp_stacktrace.c:49

Change-Id: Id9df73633583e93dc4fb39f0a90fdf6c7c56a6cc
Signed-off-by: Maxim Uvarov 
---
M platform/linux-generic/Makefile.am
M platform/linux-generic/include/odp_debug_internal.h
M platform/linux-generic/odp_init.c
A platform/linux-generic/odp_stacktrace.c
4 files changed, 81 insertions(+), 0 deletions(-)


  git pull ssh://review.linaro.org:29418/lng/odp refs/changes/34/13134/1

diff --git a/platform/linux-generic/Makefile.am 
b/platform/linux-generic/Makefile.am
index c8fd8cb..1a7be32 100644
--- a/platform/linux-generic/Makefile.am
+++ b/platform/linux-generic/Makefile.am
@@ -174,6 +174,7 @@
   odp_spinlock.c \
   odp_spinlock_recursive.c \
   odp_system_info.c \
+  odp_stacktrace.c \
   odp_thread.c \
   odp_thrmask.c \
   odp_ticketlock.c \
diff --git a/platform/linux-generic/include/odp_debug_internal.h 
b/platform/linux-generic/include/odp_debug_internal.h
index 02ae87a..a43705f 100644
--- a/platform/linux-generic/include/odp_debug_internal.h
+++ b/platform/linux-generic/include/odp_debug_internal.h
@@ -83,6 +83,8 @@
 #define ODP_PRINT(fmt, ...) \
odp_global_data.log_fn(ODP_LOG_PRINT, " " fmt, ##__VA_ARGS__)
 
+void _odp_stracktrace_init(void);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/platform/linux-generic/odp_init.c 
b/platform/linux-generic/odp_init.c
index f534759..875381f 100644
--- a/platform/linux-generic/odp_init.c
+++ b/platform/linux-generic/odp_init.c
@@ -33,6 +33,9 @@
odp_global_data.abort_fn = params->abort_fn;
}
 
+   if (getenv("ODP_STACKTRACE_ENABLE"))
+   _odp_stracktrace_init();
+
if (odp_cpumask_init_global(params)) {
ODP_ERR("ODP cpumask init failed.\n");
goto init_failed;
diff --git a/platform/linux-generic/odp_stacktrace.c 
b/platform/linux-generic/odp_stacktrace.c
new file mode 100644
index 000..76a8803
--- /dev/null
+++ b/platform/linux-generic/odp_stacktrace.c
@@ -0,0 +1,75 @@
+/* Copyright (c) 2016, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#include 
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static odp_spinlock_t print_lock;
+
+static void fault_handler(int signal)
+{
+   size_t num_stack_frames;
+   const char  *signal_name;
+   void  *bt_array[4096];
+   sigset_t sigset;
+
+   /* disable all signals */
+   sigfillset();
+   sigprocmask(SIG_BLOCK, , NULL);
+
+   switch (signal) {
+   case SIGILL:
+   signal_name = "SIGILL";   break;
+   case SIGFPE:
+   signal_name = "SIGFPE";   break;
+   case SIGSEGV:
+   signal_name = "SIGSEGV";  break;
+   case SIGTERM:
+   signal_name = "SIGTERM";  break;
+   case SIGBUS:
+   signal_name = "SIGBUS";   break;
+   default:
+   signal_name = "UNKNOWN";  break;
+   }
+
+   odp_spinlock_lock(_lock);
+   num_stack_frames = backtrace(bt_array, 100);
+   fprintf(stderr, "\nThread %d terminated with signal=%u (%s)\n",
+   odp_thread_id(), signal, signal_name);
+   backtrace_symbols_fd(bt_array, num_stack_frames, fileno(stderr));
+   fflush(NULL);
+   sync();
+   odp_spinlock_unlock(_lock);
+
+   ODP_ABORT("Program interrupted\n");
+}
+
+void _odp_stracktrace_init(void)
+{
+   struct sigaction signal_action;
+
+   odp_spinlock_init(_lock);
+
+   memset(_action, 0, sizeof(signal_action));
+   signal_action.sa_handler = fault_handler;
+   sigfillset(_action.sa_mask);
+
+   sigaction(SIGILL,  _action, NULL);
+   sigaction(SIGFPE,  _action, NULL);
+   sigaction(SIGSEGV, _action, NULL);
+   sigaction(SIGTERM, _action, NULL);
+   sigaction(SIGBUS,  _action, NULL);
+}

-- 
To view,

[lng-odp] Change in lng/odp[master]: linux-generic: add stacktrace

2016-07-07 Thread lava-bot (Code Review) 
lava-bot has posted comments on this change.

Change subject: linux-generic: add stacktrace
..


Patch Set 1:

"Build Started https://ci.linaro.org/jenkins/job/odp-api-check-gerrit/6/ "

-- 
To view, visit https://review.linaro.org/13134
To unsubscribe, visit https://review.linaro.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id9df73633583e93dc4fb39f0a90fdf6c7c56a6cc
Gerrit-PatchSet: 1
Gerrit-Project: lng/odp
Gerrit-Branch: master
Gerrit-Owner: Maxim Uvarov 
Gerrit-Reviewer: lava-bot 
Gerrit-HasComments: No

[lng-odp] Change in lng/odp[master]: validation: enable bt for scheduling test

2016-07-07 Thread Maxim Uvarov (Code Review) 
Maxim Uvarov has uploaded a new change for review.

  https://review.linaro.org/13135

Change subject: validation: enable bt for scheduling test
..

validation: enable bt for scheduling test

Change-Id: I3479b518df39be9ee7beab1ebf98a64f9895457e
Signed-off-by: Maxim Uvarov 
---
M test/performance/odp_scheduling_run.sh
1 file changed, 1 insertion(+), 0 deletions(-)


  git pull ssh://review.linaro.org:29418/lng/odp refs/changes/35/13135/1

diff --git a/test/performance/odp_scheduling_run.sh 
b/test/performance/odp_scheduling_run.sh
index 755b0c1..fd81d78 100755
--- a/test/performance/odp_scheduling_run.sh
+++ b/test/performance/odp_scheduling_run.sh
@@ -8,6 +8,7 @@
 # Script that passes command line arguments to odp_scheduling test when
 # launched by 'make check'
 
+export ODP_STACKTRACE_ENABLE=y
 TEST_DIR="${TEST_DIR:-$(dirname $0)}"
 ret=0
 

-- 
To view, visit https://review.linaro.org/13135
To unsubscribe, visit https://review.linaro.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3479b518df39be9ee7beab1ebf98a64f9895457e
Gerrit-PatchSet: 1
Gerrit-Project: lng/odp
Gerrit-Branch: master
Gerrit-Owner: Maxim Uvarov

Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield order doxygen omissions

2016-07-07 Thread Bill Fischofer 
On Thu, Jul 7, 2016 at 6:57 AM, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolai...@nokia-bell-labs.com> wrote:

>
>
> From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
> Sent: Thursday, July 07, 2016 2:42 PM
> To: Savolainen, Petri (Nokia - FI/Espoo) <
> petri.savolai...@nokia-bell-labs.com>
> Cc: lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield
> order doxygen omissions
>
>
> On Thu, Jul 7, 2016 at 2:46 AM, Savolainen, Petri (Nokia - FI/Espoo) <
> petri.savolai...@nokia-bell-labs.com> wrote:
> I'm OK with the change, but didn't do it as part of my patch set since
> it's an API change. I think the change is not a big deal and should be done
> to align the usage of byte/bitfield defines. The patch should be tagged
> with api: prefix and go through api-next.
>
> Also, API spec could be more explicit about the byte/bitfield defines and
> how application should use/test those. I think bitfield defines should work
> the same way as the current byte order defines.
>
> #if (ODP_BYTE_ORDER == ODP_BIG_ENDIAN)
> // big endian byte order
> #else
> // little endian byte order
> #endif
>
>
> -Petri
>
> Not making an API change was the reason I did this change this way. This
> is now an implementation change only.  The change that would involve an API
> change would have been to define an ODP_BITFIELD_ORDER variable to mirror
> the ODP_BYTE_ORDER variable and set that to either
> ODP_LITTLE_ENDIAN_BITFIELD or ODP_BIG_ENDIAN_BITFIELD as needed.
>
>
> > --- a/platform/linux-generic/include/protocols/tcp.h
> > +++ b/platform/linux-generic/include/protocols/tcp.h
> > @@ -34,7 +34,7 @@ typedef struct ODP_PACKED {
> >   odp_u32be_t ack_no;   /**< Acknowledgment number */
> >   union {
> >   odp_u16be_t doffset_flags;
> > -#if defined(ODP_BIG_ENDIAN_BITFIELD)
> > +#if ODP_BIG_ENDIAN_BITFIELD
>
>
> The change is non-backward, API change for application code. Tcp helper is
> also application code.
>
> Today, application may do this half of the time ...
>
> #if defined(ODP_BIG_ENDIAN_BITFIELD)
> // foo
> #else
> // bar
> #endif
>
> ... and this the other half ...
>
> #if defined(ODP_LITTLE_ENDIAN_BITFIELD)
> // foo
> #else
> // bar
> #endif
>
>
> After the change, application would break since both
> ODP_LITTLE_ENDIAN_BITFIELD and ODP_BIG_ENDIAN_BITFIELD are defined always.
>
>
Yes, which is why the patch includes an update to both copies of tcp.h,
which are currently the only files that reference those #defines.

The issue is that the recent change seems to have broken doxygen, so the
question is what's the best way to fix that?


> -Petri
>
>
>

Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield order doxygen omissions

2016-07-07 Thread Savolainen, Petri (Nokia - FI/Espoo) 


From: Bill Fischofer [mailto:bill.fischo...@linaro.org] 
Sent: Thursday, July 07, 2016 2:42 PM
To: Savolainen, Petri (Nokia - FI/Espoo) 
Cc: lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield order 
doxygen omissions


On Thu, Jul 7, 2016 at 2:46 AM, Savolainen, Petri (Nokia - FI/Espoo) 
 wrote:
I'm OK with the change, but didn't do it as part of my patch set since it's an 
API change. I think the change is not a big deal and should be done to align 
the usage of byte/bitfield defines. The patch should be tagged with api: prefix 
and go through api-next.

Also, API spec could be more explicit about the byte/bitfield defines and how 
application should use/test those. I think bitfield defines should work the 
same way as the current byte order defines.

#if (ODP_BYTE_ORDER == ODP_BIG_ENDIAN)
// big endian byte order
#else
// little endian byte order
#endif


-Petri

Not making an API change was the reason I did this change this way. This is now 
an implementation change only.  The change that would involve an API change 
would have been to define an ODP_BITFIELD_ORDER variable to mirror the 
ODP_BYTE_ORDER variable and set that to either ODP_LITTLE_ENDIAN_BITFIELD or 
ODP_BIG_ENDIAN_BITFIELD as needed.


> --- a/platform/linux-generic/include/protocols/tcp.h
> +++ b/platform/linux-generic/include/protocols/tcp.h
> @@ -34,7 +34,7 @@ typedef struct ODP_PACKED {
>   odp_u32be_t ack_no;   /**< Acknowledgment number */
>   union {
>   odp_u16be_t doffset_flags;
> -#if defined(ODP_BIG_ENDIAN_BITFIELD)
> +#if ODP_BIG_ENDIAN_BITFIELD


The change is non-backward, API change for application code. Tcp helper is also 
application code.

Today, application may do this half of the time ...

#if defined(ODP_BIG_ENDIAN_BITFIELD)
// foo
#else
// bar
#endif

... and this the other half ...

#if defined(ODP_LITTLE_ENDIAN_BITFIELD)
// foo
#else
// bar
#endif


After the change, application would break since both ODP_LITTLE_ENDIAN_BITFIELD 
and ODP_BIG_ENDIAN_BITFIELD are defined always.

-Petri

[lng-odp] [RFC OPNESSL-ODP 2/2] Adding Async ODP engine for AES cipher

2016-07-07 Thread Nikhil Agarwal 
---
 README   |  21 +++
 engine/eng_odp.c | 437 +++
 2 files changed, 458 insertions(+)
 create mode 100644 README
 create mode 100644 engine/eng_odp.c

diff --git a/README b/README
new file mode 100644
index 000..b13393a
--- /dev/null
+++ b/README
@@ -0,0 +1,21 @@
+Copyright (c) 2013-2014, Linaro Limited
+All rights reserved.
+
+SPDX-License-Identifier:BSD-3-Clause
+
+OpenDataPlane (ODP) project source code.
+http://www.opendataplane.org/
+
+How to build:
+Following are the steep to build this object:
+./bootstrap
+./configure --with-openssl-path= 
--with-odp-path=
+make
+
+How to run.
+Copy the shared object engine/.libs/libsslodp.so to "opnessl 
path"/lib/engines/.
+Execute any openssl application with arguments  "-engine libsslodp".
+
+Currently, we have tested openssl speed application with NXP platform.
+
+
diff --git a/engine/eng_odp.c b/engine/eng_odp.c
new file mode 100644
index 000..91151ae
--- /dev/null
+++ b/engine/eng_odp.c
@@ -0,0 +1,437 @@
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+odp_pool_t pool;
+int num_queue = 8;
+odp_queue_t out_queue[8];
+
+typedef struct ossl_odp_status {
+   bool is_complete;
+   bool is_successful;
+} ossl_odp_status_t;
+
+/* Engine Id and Name */
+static const char *engine_odp_id = "libsslodp";
+static const char *engine_odp_name = "ODP based engine";
+
+/* Engine Lifetime functions */
+static int ossl_odp_destroy(ENGINE *e);
+static int ossl_odp_init(ENGINE *e);
+static int ossl_odp_finish(ENGINE *e);
+
+/* Set up digests. Just SHA1 for now */
+static int ossl_odp_digests(ENGINE *e, const EVP_MD **digest,
+   const int **nids, int nid);
+
+/*
+ * Holds the EVP_MD object for sha1 in this engine. Set up once only during
+ * engine bind and can then be reused many times.
+ */
+static EVP_MD *_hidden_sha1_md = NULL;
+static const EVP_MD *ossl_odp_sha1(void)
+{
+   return _hidden_sha1_md;
+}
+static void destroy_digests(void)
+{
+   EVP_MD_meth_free(_hidden_sha1_md);
+   _hidden_sha1_md = NULL;
+}
+
+
+static int ossl_odp_digest_nids(const int **nids)
+{
+   static int digest_nids[2] = { 0, 0 };
+   static int pos = 0;
+   static int init = 0;
+
+   if (!init) {
+   const EVP_MD *md;
+   if ((md = ossl_odp_sha1()) != NULL)
+   digest_nids[pos++] = EVP_MD_type(md);
+   digest_nids[pos] = 0;
+   init = 1;
+   }
+   *nids = digest_nids;
+   return pos;
+}
+
+/* AES */
+
+static int ossl_odp_aes128_init_key(EVP_CIPHER_CTX *ctx,
+   const unsigned char *key, const unsigned char *iv, int enc);
+static int ossl_odp_aes128_cbc_cipher(EVP_CIPHER_CTX *ctx, unsigned char *out,
+   const unsigned char *in, size_t inl);
+static int ossl_odp_aes128_cbc_cleanup(EVP_CIPHER_CTX *ctx);
+
+struct ossl_odp_ctx {
+   odp_crypto_session_t session;
+};
+
+/*
+ * Holds the EVP_CIPHER object for aes_128_cbc in this engine. Set up once only
+ * during engine bind and can then be reused many times.
+ */
+static EVP_CIPHER *_hidden_aes_128_cbc;
+static const EVP_CIPHER *ossl_odp_aes_128_cbc(void)
+{
+   return _hidden_aes_128_cbc;
+}
+
+static void destroy_ciphers(void)
+{
+   EVP_CIPHER_meth_free(_hidden_aes_128_cbc);
+   _hidden_aes_128_cbc = NULL;
+}
+
+static int ossl_odp_ciphers(ENGINE *e, const EVP_CIPHER **cipher,
+   const int **nids, int nid);
+
+static int ossl_odp_cipher_nids[] = {
+   NID_aes_128_cbc,
+   0
+};
+
+
+static int bind_odp(ENGINE *e)
+{
+   odp_queue_param_t qparam;
+   odp_pool_param_t params;
+   int i;
+   if (0 != odp_init_global(NULL, NULL)) {
+   printf("error: odp_init_global() failed.\n");
+   return -1;
+   }
+
+   if (0 != odp_init_local(ODP_THREAD_WORKER)) {
+   printf("error: odp_init_local() failed.\n");
+   return -1;
+   }
+
+   memset(, 0, sizeof(params));
+   params.pkt.seg_len = 20480;
+   params.pkt.len = 20480;
+   params.pkt.num = 4096;
+   params.type = ODP_POOL_PACKET;
+
+   pool = odp_pool_create("ossl_odp_pool", );
+
+   if (ODP_POOL_INVALID == pool) {
+   printf("Packet pool creation failed.\n");
+   odp_term_local();
+   odp_term_global();
+   return -1;
+   }
+   odp_queue_param_init();
+   qparam.sched.prio = ODP_SCHED_PRIO_HIGHEST;
+   qparam.sched.sync = ODP_SCHED_SYNC_ATOMIC;
+   qparam.sched.group = ODP_SCHED_GROUP_ALL;
+   for (i = 0; i < num_queue; i++) {
+   char queue_name[256] = {0};
+   sprintf(queue_name, "%s%d", "ossl_out_queue_", i);
+   out_queue[i] = odp_queue_create(queue_name,
+

[lng-odp] [RFC OPNESSL-ODP 1/2] Initial Commit for Makeflies and configure scripts.

2016-07-07 Thread Nikhil Agarwal 
From: Nikhil Agarwal 

Added Directory structure. Makeflies and configure scripts.

Signed-off-by: Nikhil Agarwal 
---
 .gitignore |  25 ++
 Makefile.am|   2 +
 bootstrap  |   7 +++
 configure.ac   | 136 +
 engine/Makefile.am |   5 ++
 5 files changed, 175 insertions(+)
 create mode 100644 .gitignore
 create mode 100644 Makefile.am
 create mode 100755 bootstrap
 create mode 100644 configure.ac
 create mode 100644 engine/Makefile.am

diff --git a/.gitignore b/.gitignore
new file mode 100644
index 000..1f844a8
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,25 @@
+Makefile
+Makefile.in
+aclocal.m4
+ar-lib
+autom4te.cache/
+compile
+config.guess
+config.log
+config.status
+config.sub
+configure
+depcomp
+engine/.deps/
+engine/.libs/
+engine/Makefile
+engine/Makefile.in
+engine/eng_odp.lo
+engine/eng_odp.o
+engine/libsslodp.la
+install-sh
+libtool
+ltmain.sh
+m4/
+missing
+
diff --git a/Makefile.am b/Makefile.am
new file mode 100644
index 000..0536da9
--- /dev/null
+++ b/Makefile.am
@@ -0,0 +1,2 @@
+ACLOCAL_AMFLAGS=-I m4
+SUBDIRS = engine
diff --git a/bootstrap b/bootstrap
new file mode 100755
index 000..ce37658
--- /dev/null
+++ b/bootstrap
@@ -0,0 +1,7 @@
+#! /bin/sh
+set -x
+#mkdir m4
+aclocal
+libtoolize --copy
+automake --add-missing --copy --warnings=all
+autoconf
diff --git a/configure.ac b/configure.ac
new file mode 100644
index 000..19249c9
--- /dev/null
+++ b/configure.ac
@@ -0,0 +1,136 @@
+AC_PREREQ([2.5])
+AC_INIT([ossl-odp], [0.1], [lng-odp@lists.linaro.org])
+AM_INIT_AUTOMAKE([foreign])
+AC_CONFIG_SRCDIR([engine/eng_odp.c])
+
+AC_USE_SYSTEM_EXTENSIONS
+AC_SYS_LARGEFILE
+AC_CONFIG_MACRO_DIR([m4])
+AM_SILENT_RULES([yes])
+
+# Checks for programs.
+AC_PROG_CC
+AM_PROG_CC_C_O
+
+AC_PROG_CXX
+AC_PROG_INSTALL
+AC_PROG_MAKE_SET
+
+AM_PROG_AR
+#Use libtool
+LT_INIT([])
+AC_SUBST([LIBTOOL_DEPS])
+AM_PROG_LIBTOOL
+
+# Checks for library functions.
+AC_FUNC_MALLOC
+
+# Checks for typedefs, structures, and compiler characteristics.
+AC_HEADER_STDBOOL
+AC_C_INLINE
+AC_TYPE_SIZE_T
+AC_TYPE_SSIZE_T
+AC_TYPE_UINT8_T
+AC_TYPE_UINT16_T
+AC_TYPE_INT32_T
+AC_TYPE_UINT32_T
+AC_TYPE_UINT64_T
+
+##
+# Which architecture optimizations will we use
+##
+AS_CASE([$host],
+  [x86*], [ARCH=x86],
+  [mips64*], [ARCH=mips64],
+  [aarch64*], [ARCH=aarch64],
+  [powerpc*], [ARCH=powerpc],
+  [ARCH=linux]
+)
+AC_SUBST([ARCH])
+
+##
+# Save and set temporary compilation flags
+##
+OLD_LDFLAGS=$LDFLAGS
+OLD_CPPFLAGS=$CPPFLAGS
+LDFLAGS="$AM_LDFLAGS $LDFLAGS"
+CPPFLAGS="$AM_CPPFLAGS $CPPFLAGS"
+
+
+##
+# Default include setup
+##
+AM_CFLAGS="$AM_CFLAGS $ODP_CFLAGS"
+AM_CXXFLAGS="-std=c++11"
+
+
+
+##
+# Set optional OpenSSL path
+##
+AC_ARG_WITH([openssl-path],
+   AC_HELP_STRING([--with-openssl-path=DIR path to openssl libs and 
headers],
+   [(or in the default path if not specified).]),
+   [OPENSSL_PATH=$withval
+   AM_CPPFLAGS="$AM_CPPFLAGS -I$OPENSSL_PATH/include"
+   AM_LDFLAGS="$AM_LDFLAGS -L$OPENSSL_PATH/lib"
+   ],[])
+
+##
+# Set optional ODP path
+##
+AC_ARG_WITH([odp-path],
+   AC_HELP_STRING([--with-odp-path=DIR path to odp libs and headers],
+   [(or in the default path if not specified).]),
+   [ODP_PATH=$withval
+   AM_CPPFLAGS="$AM_CPPFLAGS -I$ODP_PATH/include"
+   AM_LDFLAGS="$AM_LDFLAGS -L$ODP_PATH/lib"
+   ],[])
+
+
+##
+# Save and set temporary compilation flags
+##
+OLD_LDFLAGS=$LDFLAGS
+OLD_CPPFLAGS=$CPPFLAGS
+LDFLAGS="$AM_LDFLAGS $LDFLAGS"
+CPPFLAGS="$AM_CPPFLAGS $CPPFLAGS"
+
+AC_CHECK_LIB([odp], [odp_init_global],[ODP_LIB=-lodp],
+   [AC_MSG_FAILURE([ODP library required])])
+
+
+AC_CONFIG_FILES([Makefile
+   engine/Makefile
+   ])
+
+AC_SUBST([ODP_PATH])
+AC_SUBST([ODP_LIB])
+AC_SUBST([OPENSSL_PATH])
+AC_SUBST([LIBS])
+AC_SUBST([AM_CPPFLAGS])
+AC_SUBST([CPPFLAGS])
+AC_SUBST([AM_CFLAGS])
+AC_SUBST([CFLAGS])
+AC_SUBST([AM_LDFLAGS])

Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield order doxygen omissions

2016-07-07 Thread Bill Fischofer 
On Thu, Jul 7, 2016 at 2:46 AM, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolai...@nokia-bell-labs.com> wrote:

> I'm OK with the change, but didn't do it as part of my patch set since
> it's an API change. I think the change is not a big deal and should be done
> to align the usage of byte/bitfield defines. The patch should be tagged
> with api: prefix and go through api-next.
>
> Also, API spec could be more explicit about the byte/bitfield defines and
> how application should use/test those. I think bitfield defines should work
> the same way as the current byte order defines.
>
> #if (ODP_BYTE_ORDER == ODP_BIG_ENDIAN)
> // big endian byte order
> #else
> // little endian byte order
> #endif
>
>
> -Petri
>

Not making an API change was the reason I did this change this way. This is
now an implementation change only.  The change that would involve an API
change would have been to define an ODP_BITFIELD_ORDER variable to mirror
the ODP_BYTE_ORDER variable and set that to either
ODP_LITTLE_ENDIAN_BITFIELD or ODP_BIG_ENDIAN_BITFIELD as needed.


>
>
> > -Original Message-
> > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of
> Bill
> > Fischofer
> > Sent: Thursday, July 07, 2016 4:28 AM
> > To: lng-odp@lists.linaro.org
> > Subject: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield order
> > doxygen omissions
> >
> > Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2402 by assigning
> > explicit values to ODP_LITTLE_ENDIAN_BITFIELD and ODP_BIT_ENDIAN_BITFIELD
> > to avoid Doxygen warnings. This makes these consistent with the usage for
> > ODP_BIG_ENDIAN and ODP_LITTLE_ENDIAN.
> >
> > Note that this requires tests of these fields to change from #ifdef to
> > #if.
> >
> > Signed-off-by: Bill Fischofer 
> > ---
> >  helper/include/odp/helper/tcp.h   | 4 ++--
> >  platform/linux-generic/include/odp/api/plat/byteorder_types.h | 6 --
> >  platform/linux-generic/include/protocols/tcp.h| 4 ++--
> >  3 files changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/helper/include/odp/helper/tcp.h
> > b/helper/include/odp/helper/tcp.h
> > index cabef90..fd234e5 100644
> > --- a/helper/include/odp/helper/tcp.h
> > +++ b/helper/include/odp/helper/tcp.h
> > @@ -34,7 +34,7 @@ typedef struct ODP_PACKED {
> >   odp_u32be_t ack_no;   /**< Acknowledgment number */
> >   union {
> >   odp_u16be_t doffset_flags;
> > -#if defined(ODP_BIG_ENDIAN_BITFIELD)
> > +#if ODP_BIG_ENDIAN_BITFIELD
> >   struct {
> >   odp_u16be_t rsvd1:8;
> >   odp_u16be_t flags:8; /**< TCP flags as a byte */
> > @@ -51,7 +51,7 @@ typedef struct ODP_PACKED {
> >   odp_u16be_t syn:1;
> >   odp_u16be_t fin:1;
> >   };
> > -#elif defined(ODP_LITTLE_ENDIAN_BITFIELD)
> > +#elif ODP_LITTLE_ENDIAN_BITFIELD
> >   struct {
> >   odp_u16be_t flags:8;
> >   odp_u16be_t rsvd1:8; /**< TCP flags as a byte */
> > diff --git
> a/platform/linux-generic/include/odp/api/plat/byteorder_types.h
> > b/platform/linux-generic/include/odp/api/plat/byteorder_types.h
> > index 679d4cf..acc899a 100644
> > --- a/platform/linux-generic/include/odp/api/plat/byteorder_types.h
> > +++ b/platform/linux-generic/include/odp/api/plat/byteorder_types.h
> > @@ -52,12 +52,14 @@ extern "C" {
> >   #define ODP_LITTLE_ENDIAN   1
> >   #define ODP_BIG_ENDIAN  0
> >   #define ODP_BYTE_ORDER  ODP_LITTLE_ENDIAN
> > - #define ODP_LITTLE_ENDIAN_BITFIELD
> > + #define ODP_LITTLE_ENDIAN_BITFIELD  1
> > + #define ODP_BIG_ENDIAN_BITFIELD 0
> >  #else
> >   #define ODP_LITTLE_ENDIAN   0
> >   #define ODP_BIG_ENDIAN  1
> >   #define ODP_BYTE_ORDER  ODP_BIG_ENDIAN
> > - #define ODP_BIG_ENDIAN_BITFIELD
> > + #define ODP_LITTLE_ENDIAN_BITFIELD  0
> > + #define ODP_BIG_ENDIAN_BITFIELD 1
> >  #endif
> >
> >  typedef uint16_t __odp_bitwise   odp_u16le_t;
> > diff --git a/platform/linux-generic/include/protocols/tcp.h
> > b/platform/linux-generic/include/protocols/tcp.h
> > index 4e92e4b..114262e 100644
> > --- a/platform/linux-generic/include/protocols/tcp.h
> > +++ b/platform/linux-generic/include/protocols/tcp.h
> > @@ -34,7 +34,7 @@ typedef struct ODP_PACKED {
> >   odp_u32be_t ack_no;   /**< Acknowledgment number */
> >   union {
> >   odp_u16be_t doffset_flags;
> > -#if defined(ODP_BIG_ENDIAN_BITFIELD)
> > +#if ODP_BIG_ENDIAN_BITFIELD
> >   struct {
> >   odp_u16be_t rsvd1:8;
> >   odp_u16be_t flags:8; /**< TCP flags as a byte */
> > @@ -51,7 +51,7 @@ typedef struct ODP_PACKED {
> >   odp_u16be_t syn:1;
> >   odp_u16be_t fin:1;
> >   };
> > -#elif

Re: [lng-odp] [PATCH v5] example: introducing l3fwd

2016-07-07 Thread forrest.shi 
Hi Matias,

Thanks for your review. My comments inline.

BR,
Forrest

> -Original Message-
> From: Elo, Matias (Nokia - FI/Espoo) [mailto:matias@nokia-bell-labs.com]
> Sent: Wednesday, July 06, 2016 20:00
> To: forrest@linaro.org
> Cc: lng-odp@lists.linaro.org
> Subject: RE: [lng-odp][PATCH v5] example: introducing l3fwd
> 
> Hi Forrest,
> 
> The destination MAC address doesn't seem to work (tried also with colons as
> separators):
> 
> $ sudo ./example/l3fwd/odp_l3fwd -i 0,2 -r 1.1.1.0/24:0:02.01.03.04.05.01 -r
> 2.2.2.0/24:2:0:02.01.03.04.05.02 -t 2 ...
> Routing table
> -
> subnet  next_hopdest_mac
> 2.2.2.0/24  2   00.00.00.00.00.00
> 1.1.1.0/24  0   00.00.00.00.00.00
> 

Cmdline requires dot mac, helper odph_eth_addr_parse requires colon mac,
that's the problem.
Will be changed to colon mac,  comma separator in -r string.

> 
> When adding a third route the application doesn't act as expected.
> 
> $ sudo ./example/l3fwd/odp_l3fwd -i 0,2 -r 1.1.1.0/25:0 -r 2.2.2.0/24:2 -r
> 1.1.1.128/25:0 -t 2
> 
> Routing table
> -
> subnet  next_hopdest_mac
> 1.1.1.128/250   00.00.00.00.00.00
> 2.2.2.0/24  2   00.00.00.00.00.00
> 1.1.1.0/25  0   00.00.00.00.00.00
> 
> My test setup:
> 
> IF0 < dst: 2.2.2.0/24
> IF2 < dst: 1.1.1.0/24
> 
> When I send traffic only to IF0 it is correctly routed to IF2. However, when
I
> send traffic to IF2 the traffic is routed to both of the interfaces (most to
IF2).
> When I send test traffic to both interfaces at the same time, nothing is
received
> from IF2 anymore (possible explanation below).
> 


Subnet mask is not correct, will be fixed.



> Further comments below.
> 
> -Matias
> 
> 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> 
> Move up with the other system headers.
> 
> > +#define MAX_NB_RX_QUEUE(MAX_NB_WORKER)
> > +#define MAX_NB_TX_QUEUE(MAX_NB_WORKER)
> > +#define MAX_NB_QCONF_PER_CORE  (MAX_NB_PKTIO)
> 
> You could remove the MAX_NB_RX_QUEUE & MAX_NB_TX_QUEUE &
> MAX_NB_QCONF_PER_CORE defines altogether and use the
> MAX_NB_WORKER & MAX_NB_PKTIO defines directly.
>

Maybe these numbers are not same as MAX_NB_WORKER

 
> > +#define CPU_ID_INVALID (-1)
> 
> Unused define.
> 
> > +struct thread_arg_s {
> > +   int cpu;/* initialized as -1 */
> > +   struct l3fwd_qconf_s *qconf_args[MAX_NB_QCONF_PER_CORE];
> > +   int count;
> >+};
> 
> The cpu member seems unnecessary.
> 
> > +   rc = odp_pktio_capability(pktio, );
> > +   if (rc) {
> > +   printf("pktio %s: unable to read capabilities, "
> > +  "set to 1 rx and 1 tx queue\n", name);
> > +   fwd_pktio->nb_rxq = 1;
> > +   fwd_pktio->nb_txq = 1;
> > +   }
> 
> Simply return here if odp_pktio_capability() fails. You are using 'capa'
later on
> and its contents are undefined if odp_pktio_capability() fails.
> 
> You can use LOG_ERR() macro to print error messages.
> 
> > +   if (odp_packet_has_udp(pkt) ||
> > +   odp_packet_has_tcp(pkt)) {
> > +   /* UDP or TCP*/
> > +   void *ptr = odp_packet_l4_ptr(pkt, NULL);
> > +
> > +   udp = (odph_udphdr_t *)ptr;
> 
> Simply:   udp = odp_packet_l4_ptr(pkt, NULL);
> 
> 
> > +   if (entry) {
> > +   eth->src = entry->src_mac;
> > +   eth->dst = entry->dst_mac;
> > +   dif = entry->oif_id;
> > +   } else {
> > +   /* no route, send by src port */
> > +   eth->dst = eth->src;
> > +   dif = sif;
> > +   }
> 
> If no route is found the packet should in my opinion be dropped.
> 
> > +   /* network byte order maybe different from host */
> > +   ret = fib_tbl_lookup(odp_be_to_cpu_32(ip->dst_addr), );
> > +   if (ret)
> > +   dif = sif;
> 
> Same as above.
> 
> > +
> > +   eth->dst = global.eth_dest_mac[dif];
> 
> It seems like the destination MAC is not read from the routing table.

The dest mac in routing table is populated into this array while using lpm. 
In this case, not find entry in routing table. Just doing "dest ip > dest
port" mapping.

> 
> > +static void l3fwd_one_queue(uint32_t sif, uint8_t rxq_id) {
> > +   struct l3fwd_pktio_s *port;
> > +   odp_packet_t *tbl;
> > +   odp_pktout_queue_t outq;
> > +   odp_packet_t pkt_tbl[MAX_PKT_BURST];
> > +   int pkts, drop, sent;
> > +   int dif, dst_port;
> > +   int i;
> > +
> > +   port = _pktios[sif];
> > +   pkts = odp_pktin_recv(port->ifin[rxq_id], pkt_tbl, MAX_PKT_BURST);
> > +   if (pkts <= 0)
> > +   return;
> > +
> > +   drop = drop_err_pkts(pkt_tbl, pkts);
> > +   pkts -= drop;
> > +
> > +   dif = global.fwd_func(pkt_tbl[0], sif);
> > +   tbl = _tbl[0];
> > +   while (pkts) {
> > +   dst_port = dif;
> > +   for (i = 1; i < pkts; i++)

[lng-odp] test directory structure

2016-07-07 Thread Christophe Milard 
Hi,

I need to move things around to include test for the driver interface
in the test directory.
This is what I can thing about.

Mike, Does this match what you had in mind?

test
├── platform//platform dependant stuff and launchers
│   └── linux-generic
│   ├── api
│   ├── drv
│   ├── miscellaneous
│   └── performance
└── validation  //platform agnostic stuff
├── api
│   ├── atomic
│   ├── barrier
│   ├── buffer
│   └── classification
├── drv
├── miscellaneous
└── performance

15 directories, 0 files

Re: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield order doxygen omissions

2016-07-07 Thread Savolainen, Petri (Nokia - FI/Espoo) 
I'm OK with the change, but didn't do it as part of my patch set since it's an 
API change. I think the change is not a big deal and should be done to align 
the usage of byte/bitfield defines. The patch should be tagged with api: prefix 
and go through api-next.

Also, API spec could be more explicit about the byte/bitfield defines and how 
application should use/test those. I think bitfield defines should work the 
same way as the current byte order defines.

#if (ODP_BYTE_ORDER == ODP_BIG_ENDIAN)
// big endian byte order
#else
// little endian byte order
#endif 


-Petri



> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Bill
> Fischofer
> Sent: Thursday, July 07, 2016 4:28 AM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [PATCH] linux-generic: byteorder: avoid bitfield order
> doxygen omissions
> 
> Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2402 by assigning
> explicit values to ODP_LITTLE_ENDIAN_BITFIELD and ODP_BIT_ENDIAN_BITFIELD
> to avoid Doxygen warnings. This makes these consistent with the usage for
> ODP_BIG_ENDIAN and ODP_LITTLE_ENDIAN.
> 
> Note that this requires tests of these fields to change from #ifdef to
> #if.
> 
> Signed-off-by: Bill Fischofer 
> ---
>  helper/include/odp/helper/tcp.h   | 4 ++--
>  platform/linux-generic/include/odp/api/plat/byteorder_types.h | 6 --
>  platform/linux-generic/include/protocols/tcp.h| 4 ++--
>  3 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/helper/include/odp/helper/tcp.h
> b/helper/include/odp/helper/tcp.h
> index cabef90..fd234e5 100644
> --- a/helper/include/odp/helper/tcp.h
> +++ b/helper/include/odp/helper/tcp.h
> @@ -34,7 +34,7 @@ typedef struct ODP_PACKED {
>   odp_u32be_t ack_no;   /**< Acknowledgment number */
>   union {
>   odp_u16be_t doffset_flags;
> -#if defined(ODP_BIG_ENDIAN_BITFIELD)
> +#if ODP_BIG_ENDIAN_BITFIELD
>   struct {
>   odp_u16be_t rsvd1:8;
>   odp_u16be_t flags:8; /**< TCP flags as a byte */
> @@ -51,7 +51,7 @@ typedef struct ODP_PACKED {
>   odp_u16be_t syn:1;
>   odp_u16be_t fin:1;
>   };
> -#elif defined(ODP_LITTLE_ENDIAN_BITFIELD)
> +#elif ODP_LITTLE_ENDIAN_BITFIELD
>   struct {
>   odp_u16be_t flags:8;
>   odp_u16be_t rsvd1:8; /**< TCP flags as a byte */
> diff --git a/platform/linux-generic/include/odp/api/plat/byteorder_types.h
> b/platform/linux-generic/include/odp/api/plat/byteorder_types.h
> index 679d4cf..acc899a 100644
> --- a/platform/linux-generic/include/odp/api/plat/byteorder_types.h
> +++ b/platform/linux-generic/include/odp/api/plat/byteorder_types.h
> @@ -52,12 +52,14 @@ extern "C" {
>   #define ODP_LITTLE_ENDIAN   1
>   #define ODP_BIG_ENDIAN  0
>   #define ODP_BYTE_ORDER  ODP_LITTLE_ENDIAN
> - #define ODP_LITTLE_ENDIAN_BITFIELD
> + #define ODP_LITTLE_ENDIAN_BITFIELD  1
> + #define ODP_BIG_ENDIAN_BITFIELD 0
>  #else
>   #define ODP_LITTLE_ENDIAN   0
>   #define ODP_BIG_ENDIAN  1
>   #define ODP_BYTE_ORDER  ODP_BIG_ENDIAN
> - #define ODP_BIG_ENDIAN_BITFIELD
> + #define ODP_LITTLE_ENDIAN_BITFIELD  0
> + #define ODP_BIG_ENDIAN_BITFIELD 1
>  #endif
> 
>  typedef uint16_t __odp_bitwise   odp_u16le_t;
> diff --git a/platform/linux-generic/include/protocols/tcp.h
> b/platform/linux-generic/include/protocols/tcp.h
> index 4e92e4b..114262e 100644
> --- a/platform/linux-generic/include/protocols/tcp.h
> +++ b/platform/linux-generic/include/protocols/tcp.h
> @@ -34,7 +34,7 @@ typedef struct ODP_PACKED {
>   odp_u32be_t ack_no;   /**< Acknowledgment number */
>   union {
>   odp_u16be_t doffset_flags;
> -#if defined(ODP_BIG_ENDIAN_BITFIELD)
> +#if ODP_BIG_ENDIAN_BITFIELD
>   struct {
>   odp_u16be_t rsvd1:8;
>   odp_u16be_t flags:8; /**< TCP flags as a byte */
> @@ -51,7 +51,7 @@ typedef struct ODP_PACKED {
>   odp_u16be_t syn:1;
>   odp_u16be_t fin:1;
>   };
> -#elif defined(ODP_LITTLE_ENDIAN_BITFIELD)
> +#elif ODP_LITTLE_ENDIAN_BITFIELD
>   struct {
>   odp_u16be_t flags:8;
>   odp_u16be_t rsvd1:8; /**< TCP flags as a byte */
> --
> 2.7.4