Re: [lng-odp] process thread and mixed mode

[Christophe Milard]

On 17 May 2016 at 02:09, Bill Fischofer  wrote:

>
>
> On Fri, May 13, 2016 at 6:32 AM, Savolainen, Petri (Nokia - FI/Espoo) <
> petri.savolai...@nokia.com> wrote:
>
>>
>>
>>
>>
>> *From:* Christophe Milard [mailto:christophe.mil...@linaro.org]
>> *Sent:* Friday, May 13, 2016 11:16 AM
>> *To:* Petri Savolainen ; Brian Brooks <
>> brian.bro...@linaro.org>; Bill Fischofer ;
>> Mike Holmes ; LNG ODP Mailman List <
>> lng-odp@lists.linaro.org>; Anders Roxell 
>> *Cc:* Tobias Lindquist 
>> *Subject:* process thread and mixed mode
>>
>>
>>
>> Hi,
>>
>>
>>
>> Let me start in a bit unusual way :-). I do not like my "running things
>> in process mode" patch series  that much...
>>
>> But I do think it should be merged. With the mixed mode.
>>
>>
>>
>> I think mixed mode is good addition, but would need more options: e.g.
>> --odph-num-proc  --odph-num-pthread-per-proc  and thus can be
>> added later. Pure proc mode is enough to get things started.
>>
>>
>>
>> The main goal of the patch series is to push the work regarding a proper
>> and consistent definition of some ODP low level concepts and objects. The
>> patch series makes things fail, hence pushing for defining and coding
>> things better, I hope.  Removing "provocative" things like the mixed mode
>> from it is just defeating the purpose of this series: I'll be happy to
>> remove it (and plenty of other things) when we know which well documented
>> definition/rule makes this code not relevant.
>>
>> As long as we keep saying that odp threads could be indifferently  linux
>> processes or pthreads, I'll be thinking that this series should work,
>> inclusive any kind of mixed mode.
>>
>>
>>
>> However, processes and threads are different things in linux, and I
>> believe this distinction should be kept within ODP: Trying to make
>> processes behave as pthreads for some ODP objects (such as addresses for
>> shmem objects) does not sound like a good idea to me. Yes, this can maybe
>> be done with linux -and even possibly without preallocating the memory-,
>> but still, linux creates 2 distinct virtual address spaces on fork() so why
>> would ODP put any exception to that?.
>> ...And If we do want to create such exceptions (which I don't like),
>> shouln't ODP define its own concept of process and thread as it obviously
>> wouldn't match 100% the definition of the OS?
>>
>>
>>
>>
>>
>> A lot of application internal state data may be directly managed with OS.
>> But I think we cannot depend 100% on OS shared data (== remove shm API all
>> together), since HW/implementation could have multiple ways optimize
>> where/how the memory is allocated/mapped (chip internal vs. external
>> memory, huge page mapped vs. normal page mapped, IO-MMU mapped, etc) … or
>> when passing pointers to ODP API, the API spec may require that the memory
>> is allocated from SHM (application would not know if the API function is
>> implemented as HW or SW).
>>
>>
>>
>>
>>
>> I do nevertheless definitively agree with Petri when saying that that ODP
>> applications should be given a way to "share pointers" between odpthreads.
>> Taking the "process only" behavior forcing every concurrent execution tasks
>> to use tricky offset calculation to find their data is too nasty and
>> inefficient.
>>
>>
>>
>> I think this will boil down to 2 choices:
>>
>> 1) Take these object definitions from the OS and accept the OS rules.
>>
>> 2) Define our own ODP objects (and possibly defines our own rules)
>>
>>
>>
>> 1)Taking the OS definitions:
>>
>> Choice 1) is consistent as long as we accept the OS rules. Taking that
>> choice would mean that any address remain valid within a process only:
>> shmem pointers will not be able to be shared between different processes: I
>> do not see that as a big problem as the applications can use pthreads if it
>> needs to share the same VA, or can fork() at a conveniant time. Moreover,
>> existing applications which should be ported to ODP are probably OK as they
>> would already be using pthreads/processes in the correct way.
>>
>>
>>
>> A more tricky aspect of choice 1) is the development of ODP itself:
>> Operations involving many "ODPthreads" will have to cope with both
>> processes and threads possibly without a clear view of what is done by the
>> application. ( I guess in linux if getpid()==gettid() when calling
>> odp_init_local() one can assume safely that the app forked...?).
>> I guess it is doable: assuming all odpthreads are descendant of the
>> instanciation process, ODP can initially reserve shared memory in
>> odp_init_global() for its own purpose. It has a price: within ODP (and
>> anything external to the app itself, such as drivers) a common VA cannot be
>> assumed. ODP/drivers will have be prepared for an app fork at any time and
>> hence do its internal stuff as if pointers could not be shared (except
>> within the initially allocated memory). This will cost ODP performance.
>>
>>
>>
>> Taking choice 1 probably also mean that shmen should no longer be

Re: [lng-odp] process thread and mixed mode

[Maxim Uvarov]

Bill, it looks reasonable, also do the same function for process as we have
for threads, like odp_thread_id() and odp_thread_count().

Maxim.

On 17 May 2016 at 03:09, Bill Fischofer  wrote:

> On Fri, May 13, 2016 at 6:32 AM, Savolainen, Petri (Nokia - FI/Espoo) <
> petri.savolai...@nokia.com> wrote:
>
> >
> >
> >
> >
> > *From:* Christophe Milard [mailto:christophe.mil...@linaro.org]
> > *Sent:* Friday, May 13, 2016 11:16 AM
> > *To:* Petri Savolainen ; Brian Brooks <
> > brian.bro...@linaro.org>; Bill Fischofer ;
> > Mike Holmes ; LNG ODP Mailman List <
> > lng-odp@lists.linaro.org>; Anders Roxell 
> > *Cc:* Tobias Lindquist 
> > *Subject:* process thread and mixed mode
> >
> >
> >
> > Hi,
> >
> >
> >
> > Let me start in a bit unusual way :-). I do not like my "running things
> in
> > process mode" patch series  that much...
> >
> > But I do think it should be merged. With the mixed mode.
> >
> >
> >
> > I think mixed mode is good addition, but would need more options: e.g.
> > --odph-num-proc  --odph-num-pthread-per-proc  and thus can be
> > added later. Pure proc mode is enough to get things started.
> >
> >
> >
> > The main goal of the patch series is to push the work regarding a proper
> > and consistent definition of some ODP low level concepts and objects. The
> > patch series makes things fail, hence pushing for defining and coding
> > things better, I hope.  Removing "provocative" things like the mixed mode
> > from it is just defeating the purpose of this series: I'll be happy to
> > remove it (and plenty of other things) when we know which well documented
> > definition/rule makes this code not relevant.
> >
> > As long as we keep saying that odp threads could be indifferently  linux
> > processes or pthreads, I'll be thinking that this series should work,
> > inclusive any kind of mixed mode.
> >
> >
> >
> > However, processes and threads are different things in linux, and I
> > believe this distinction should be kept within ODP: Trying to make
> > processes behave as pthreads for some ODP objects (such as addresses for
> > shmem objects) does not sound like a good idea to me. Yes, this can maybe
> > be done with linux -and even possibly without preallocating the memory-,
> > but still, linux creates 2 distinct virtual address spaces on fork() so
> why
> > would ODP put any exception to that?.
> > ...And If we do want to create such exceptions (which I don't like),
> > shouln't ODP define its own concept of process and thread as it obviously
> > wouldn't match 100% the definition of the OS?
> >
> >
> >
> >
> >
> > A lot of application internal state data may be directly managed with OS.
> > But I think we cannot depend 100% on OS shared data (== remove shm API
> all
> > together), since HW/implementation could have multiple ways optimize
> > where/how the memory is allocated/mapped (chip internal vs. external
> > memory, huge page mapped vs. normal page mapped, IO-MMU mapped, etc) … or
> > when passing pointers to ODP API, the API spec may require that the
> memory
> > is allocated from SHM (application would not know if the API function is
> > implemented as HW or SW).
> >
> >
> >
> >
> >
> > I do nevertheless definitively agree with Petri when saying that that ODP
> > applications should be given a way to "share pointers" between
> odpthreads.
> > Taking the "process only" behavior forcing every concurrent execution
> tasks
> > to use tricky offset calculation to find their data is too nasty and
> > inefficient.
> >
> >
> >
> > I think this will boil down to 2 choices:
> >
> > 1) Take these object definitions from the OS and accept the OS rules.
> >
> > 2) Define our own ODP objects (and possibly defines our own rules)
> >
> >
> >
> > 1)Taking the OS definitions:
> >
> > Choice 1) is consistent as long as we accept the OS rules. Taking that
> > choice would mean that any address remain valid within a process only:
> > shmem pointers will not be able to be shared between different
> processes: I
> > do not see that as a big problem as the applications can use pthreads if
> it
> > needs to share the same VA, or can fork() at a conveniant time. Moreover,
> > existing applications which should be ported to ODP are probably OK as
> they
> > would already be using pthreads/processes in the correct way.
> >
> >
> >
> > A more tricky aspect of choice 1) is the development of ODP itself:
> > Operations involving many "ODPthreads" will have to cope with both
> > processes and threads possibly without a clear view of what is done by
> the
> > application. ( I guess in linux if getpid()==gettid() when calling
> > odp_init_local() one can assume safely that the app forked...?).
> > I guess it is doable: assuming all odpthreads are descendant of the
> > instanciation process, ODP can initially reserve shared memory in
> > odp_init_global() for its own purpose. It has a price: within ODP (and
> > anything external to the app itself, such as drivers) a common VA cannot
> be
> > assumed. ODP/drivers will ha

Re: [lng-odp] [PATCHv2] doc: users-guide: add packet marking documentation

[Bill Fischofer]

Sorry about that. Fixed.

On Monday, May 16, 2016, Bala Manoharan  wrote:

> FYI
>
> @Bill: Looks like you did a reply instead of reply-all :)
>
> Regards,
> Bala
>
> -- Forwarded message --
> From: Bill Fischofer >
> Date: 16 May 2016 at 20:20
> Subject: Re: [PATCHv2] doc: users-guide: add packet marking documentation
> To: Balasubramanian Manoharan >
>
>
>
>
> On Mon, May 16, 2016 at 2:51 AM, Balasubramanian Manoharan
> > wrote:
> >
> > Updates packet marking api documentation to traffic manager user guide
> >
> > Signed-off-by: Balasubramanian Manoharan  >
>
>
> Reviewed-and-tested-by: Bill Fischofer  >
>
> >
> > ---
> > v2: document format update
> >  doc/users-guide/users-guide-tm.adoc | 73
> +
> >  1 file changed, 73 insertions(+)
> >
> > diff --git a/doc/users-guide/users-guide-tm.adoc
> b/doc/users-guide/users-guide-tm.adoc
> > index 12685b2..e02697b 100644
> > --- a/doc/users-guide/users-guide-tm.adoc
> > +++ b/doc/users-guide/users-guide-tm.adoc
> > @@ -263,6 +263,79 @@ settings for any TM system, though in most cases a
> TM system can (and should)
> >  be created/instantiated with smaller values, since lower values will
> often
> >  result in faster operation and/or less memory used.
> >
> > + Packet Marking
> > +
> > +The Packet Marking API is used to mark the packet based upon the final
> packet
> > +color assigned to it when it reaches the egress node.
> > +This is an optional feature and if available on the platform is used to
> reflect
> > +the packet color on IPv4/IPv6 DiffServ filed in accordance with
> https://www.ietf.org/rfc/rfc2474.txt[RFC 2474].
> > +There are three different packet marking fields supported they are,
> > +1). Assured forwarding in accordance with
> https://www.ietf.org/rfc/rfc2597.txt[RFC 2597], the DSCP is marked to
> > +set the packet Drop precedence in accordance with the color, i.e High
> Drop
> > +precedence for RED, Medium Drop precedence for YELLOW and leave the DSCP
> > +unchanged if the color is GREEN.
> > +2). Explicit Congestion Notification protocol per
> https://www.ietf.org/rfc/rfc3168.txt[RFC 3168], where a router
> > +encountering congestion can notify it by setting the lower 2 bits in
> > +DiffServ field to "11" Congestion Encountered code, which will
> ultimately
> > +reduce the transmission rate of the packet sender.
> > +3). In IEEE 802.1q VLAN tag header contains a DE - Drop Eligibility bit
> for
> > +marking a packet for Downstream switches, and is valid for Ethernet
> packet
> > +containing a VLAN tag.
> > +
> > +RFC 3168 is only valid for TCP packets whereas RFC 2597 is valid for
> IPv4/IPv6
> > +traffic.
> > +
> > +The values are set per color and hence the implementation may support
> these
> > +parameters only for a specific colors. marking_colors_supported field in
> > +capabilities structure can be used to check which colors are supported
> for
> > +marking.
> > +
> > + Vlan Marking.
> > +
> > +This vlan marking is used to enable the drop eligibility on the packet
> > +based on the packet color. If drop eligibility is enabled then the
> > +implementation will set the one bit VLAN Drop Eligibility Indicator
> (DEI)
> > +field (but only for packets that already carry a VLAN tag) of a packet
> based
> > +upon the final packet color assigned to the packet when it reaches the
> egress
> > +node.  When drop_eligible_enabled is false, then the given color has
> > +no effect on the VLAN fields.  See IEEE 802.1q for more details.
> > +`vlan_marking_supported` boolean in capability structure indicates
> whether this
> > +feature is supported by the implementation.
> > +
> > + Explicit Congestion Notification Marking.
> > +
> > +The `odp_tm_ecn_marking()` function allows one to configure the TM
> > +egress so that the two bit ECN subfield of the eight bit TOS field of an
> > +IPv4 packet OR the eight bit Traffic Class (TC) field of an IPv6 packet
> can be
> > +selectively modified based upon the final color assigned to the packet
> when it
> > +reaches the egress.  Note that the IPv4 header checksum will be updated
> -
> > +but only if the IPv4 TOS field actually changes as a result of this
> > +setting or the `odp_tm_drop_prec_marking()` setting.  For IPv6, since
> there is
> > +no header checksum, nothing needs to be done. If ECN is enabled for a
> > +particular color then ECN subfield will be set to _ECN_CE_  _i.e.,_
> congestion
> > +experienced.
> > +`ecn_marking_supported` boolean in capability structure indicates
> whether this
> > +feature is supported by the implementation.
> > +
> > + Drop Precedence Marking.
> > +
> > +The Drop precedence marking allows one to configure the TM
> > +egress to support Assured forwarding in accordance with
> https://www.ietf.org/rfc/rfc2597.txt[RFC 2597].
> > +The Drop Precedence bits are contained within the six bit Differentiated
> > +Services Code Point subfield of the IPv4 TOS field or the IPv6 Traffic
> > +Class (TC) field.  Specificall

Re: [lng-odp] [PATCHv2] doc: users-guide: add packet marking documentation

[Bill Fischofer]

On Monday, May 16, 2016, Balasubramanian Manoharan <
bala.manoha...@linaro.org> wrote:

> Updates packet marking api documentation to traffic manager user guide
>
> Signed-off-by: Balasubramanian Manoharan  >


Reviewed-and-tested-by: Bill Fischofer 

> ---
> v2: document format update
>  doc/users-guide/users-guide-tm.adoc | 73
> +
>  1 file changed, 73 insertions(+)
>
> diff --git a/doc/users-guide/users-guide-tm.adoc
> b/doc/users-guide/users-guide-tm.adoc
> index 12685b2..e02697b 100644
> --- a/doc/users-guide/users-guide-tm.adoc
> +++ b/doc/users-guide/users-guide-tm.adoc
> @@ -263,6 +263,79 @@ settings for any TM system, though in most cases a TM
> system can (and should)
>  be created/instantiated with smaller values, since lower values will often
>  result in faster operation and/or less memory used.
>
> + Packet Marking
> +
> +The Packet Marking API is used to mark the packet based upon the final
> packet
> +color assigned to it when it reaches the egress node.
> +This is an optional feature and if available on the platform is used to
> reflect
> +the packet color on IPv4/IPv6 DiffServ filed in accordance with
> https://www.ietf.org/rfc/rfc2474.txt[RFC 2474].
> +There are three different packet marking fields supported they are,
> +1). Assured forwarding in accordance with
> https://www.ietf.org/rfc/rfc2597.txt[RFC 2597], the DSCP is marked to
> +set the packet Drop precedence in accordance with the color, i.e High Drop
> +precedence for RED, Medium Drop precedence for YELLOW and leave the DSCP
> +unchanged if the color is GREEN.
> +2). Explicit Congestion Notification protocol per
> https://www.ietf.org/rfc/rfc3168.txt[RFC 3168], where a router
> +encountering congestion can notify it by setting the lower 2 bits in
> +DiffServ field to "11" Congestion Encountered code, which will ultimately
> +reduce the transmission rate of the packet sender.
> +3). In IEEE 802.1q VLAN tag header contains a DE - Drop Eligibility bit
> for
> +marking a packet for Downstream switches, and is valid for Ethernet packet
> +containing a VLAN tag.
> +
> +RFC 3168 is only valid for TCP packets whereas RFC 2597 is valid for
> IPv4/IPv6
> +traffic.
> +
> +The values are set per color and hence the implementation may support
> these
> +parameters only for a specific colors. marking_colors_supported field in
> +capabilities structure can be used to check which colors are supported for
> +marking.
> +
> + Vlan Marking.
> +
> +This vlan marking is used to enable the drop eligibility on the packet
> +based on the packet color. If drop eligibility is enabled then the
> +implementation will set the one bit VLAN Drop Eligibility Indicator (DEI)
> +field (but only for packets that already carry a VLAN tag) of a packet
> based
> +upon the final packet color assigned to the packet when it reaches the
> egress
> +node.  When drop_eligible_enabled is false, then the given color has
> +no effect on the VLAN fields.  See IEEE 802.1q for more details.
> +`vlan_marking_supported` boolean in capability structure indicates
> whether this
> +feature is supported by the implementation.
> +
> + Explicit Congestion Notification Marking.
> +
> +The `odp_tm_ecn_marking()` function allows one to configure the TM
> +egress so that the two bit ECN subfield of the eight bit TOS field of an
> +IPv4 packet OR the eight bit Traffic Class (TC) field of an IPv6 packet
> can be
> +selectively modified based upon the final color assigned to the packet
> when it
> +reaches the egress.  Note that the IPv4 header checksum will be updated -
> +but only if the IPv4 TOS field actually changes as a result of this
> +setting or the `odp_tm_drop_prec_marking()` setting.  For IPv6, since
> there is
> +no header checksum, nothing needs to be done. If ECN is enabled for a
> +particular color then ECN subfield will be set to _ECN_CE_  _i.e.,_
> congestion
> +experienced.
> +`ecn_marking_supported` boolean in capability structure indicates whether
> this
> +feature is supported by the implementation.
> +
> + Drop Precedence Marking.
> +
> +The Drop precedence marking allows one to configure the TM
> +egress to support Assured forwarding in accordance with
> https://www.ietf.org/rfc/rfc2597.txt[RFC 2597].
> +The Drop Precedence bits are contained within the six bit Differentiated
> +Services Code Point subfield of the IPv4 TOS field or the IPv6 Traffic
> +Class (TC) field.  Specifically the Drop Precedence sub-subfield can be
> +accessed with a DSCP bit mask of 0x06.  When enabled for a given color,
> +these two bits will be set to Medium Drop Precedence (value 0x4) if the
> +color is ODP_PACKET_YELLOW, set to High Drop Precedence (value 0x6) if
> +the color is ODP_PACKET_RED.
> +
> +Note that the IPv4 header checksum will be updated - but only if the
> +IPv4 TOS field actually changes as a result of this setting or the
> +`odp_tm_ecn_marking()` setting.  For IPv6, since there is no header
> checksum,
> +nothing else nee

[lng-odp] Fwd: [PATCHv2] doc: users-guide: add packet marking documentation

[Bala Manoharan]

FYI

@Bill: Looks like you did a reply instead of reply-all :)

Regards,
Bala

-- Forwarded message --
From: Bill Fischofer 
Date: 16 May 2016 at 20:20
Subject: Re: [PATCHv2] doc: users-guide: add packet marking documentation
To: Balasubramanian Manoharan 




On Mon, May 16, 2016 at 2:51 AM, Balasubramanian Manoharan
 wrote:
>
> Updates packet marking api documentation to traffic manager user guide
>
> Signed-off-by: Balasubramanian Manoharan 


Reviewed-and-tested-by: Bill Fischofer 

>
> ---
> v2: document format update
>  doc/users-guide/users-guide-tm.adoc | 73 
> +
>  1 file changed, 73 insertions(+)
>
> diff --git a/doc/users-guide/users-guide-tm.adoc 
> b/doc/users-guide/users-guide-tm.adoc
> index 12685b2..e02697b 100644
> --- a/doc/users-guide/users-guide-tm.adoc
> +++ b/doc/users-guide/users-guide-tm.adoc
> @@ -263,6 +263,79 @@ settings for any TM system, though in most cases a TM 
> system can (and should)
>  be created/instantiated with smaller values, since lower values will often
>  result in faster operation and/or less memory used.
>
> + Packet Marking
> +
> +The Packet Marking API is used to mark the packet based upon the final packet
> +color assigned to it when it reaches the egress node.
> +This is an optional feature and if available on the platform is used to 
> reflect
> +the packet color on IPv4/IPv6 DiffServ filed in accordance with 
> https://www.ietf.org/rfc/rfc2474.txt[RFC 2474].
> +There are three different packet marking fields supported they are,
> +1). Assured forwarding in accordance with 
> https://www.ietf.org/rfc/rfc2597.txt[RFC 2597], the DSCP is marked to
> +set the packet Drop precedence in accordance with the color, i.e High Drop
> +precedence for RED, Medium Drop precedence for YELLOW and leave the DSCP
> +unchanged if the color is GREEN.
> +2). Explicit Congestion Notification protocol per 
> https://www.ietf.org/rfc/rfc3168.txt[RFC 3168], where a router
> +encountering congestion can notify it by setting the lower 2 bits in
> +DiffServ field to "11" Congestion Encountered code, which will ultimately
> +reduce the transmission rate of the packet sender.
> +3). In IEEE 802.1q VLAN tag header contains a DE - Drop Eligibility bit for
> +marking a packet for Downstream switches, and is valid for Ethernet packet
> +containing a VLAN tag.
> +
> +RFC 3168 is only valid for TCP packets whereas RFC 2597 is valid for 
> IPv4/IPv6
> +traffic.
> +
> +The values are set per color and hence the implementation may support these
> +parameters only for a specific colors. marking_colors_supported field in
> +capabilities structure can be used to check which colors are supported for
> +marking.
> +
> + Vlan Marking.
> +
> +This vlan marking is used to enable the drop eligibility on the packet
> +based on the packet color. If drop eligibility is enabled then the
> +implementation will set the one bit VLAN Drop Eligibility Indicator (DEI)
> +field (but only for packets that already carry a VLAN tag) of a packet based
> +upon the final packet color assigned to the packet when it reaches the egress
> +node.  When drop_eligible_enabled is false, then the given color has
> +no effect on the VLAN fields.  See IEEE 802.1q for more details.
> +`vlan_marking_supported` boolean in capability structure indicates whether 
> this
> +feature is supported by the implementation.
> +
> + Explicit Congestion Notification Marking.
> +
> +The `odp_tm_ecn_marking()` function allows one to configure the TM
> +egress so that the two bit ECN subfield of the eight bit TOS field of an
> +IPv4 packet OR the eight bit Traffic Class (TC) field of an IPv6 packet can 
> be
> +selectively modified based upon the final color assigned to the packet when 
> it
> +reaches the egress.  Note that the IPv4 header checksum will be updated -
> +but only if the IPv4 TOS field actually changes as a result of this
> +setting or the `odp_tm_drop_prec_marking()` setting.  For IPv6, since there 
> is
> +no header checksum, nothing needs to be done. If ECN is enabled for a
> +particular color then ECN subfield will be set to _ECN_CE_  _i.e.,_ 
> congestion
> +experienced.
> +`ecn_marking_supported` boolean in capability structure indicates whether 
> this
> +feature is supported by the implementation.
> +
> + Drop Precedence Marking.
> +
> +The Drop precedence marking allows one to configure the TM
> +egress to support Assured forwarding in accordance with 
> https://www.ietf.org/rfc/rfc2597.txt[RFC 2597].
> +The Drop Precedence bits are contained within the six bit Differentiated
> +Services Code Point subfield of the IPv4 TOS field or the IPv6 Traffic
> +Class (TC) field.  Specifically the Drop Precedence sub-subfield can be
> +accessed with a DSCP bit mask of 0x06.  When enabled for a given color,
> +these two bits will be set to Medium Drop Precedence (value 0x4) if the
> +color is ODP_PACKET_YELLOW, set to High Drop Precedence (value 0x6) if
> +the color is O

Re: [lng-odp] [PATCH v4] doc: user-guide: Improve crypto section.

[Bala Manoharan]

On 17 May 2016 at 01:56, Bill Fischofer  wrote:
> On Mon, May 16, 2016 at 6:00 PM, Nikhil Agarwal 
> wrote:
>
>> Signed-off-by: Nikhil Agarwal 
>>
>
> Reviewed-and-tested-by: Bill Fischofer 
>
>

Reviewed-by: Balasubramanian Manoharan 

>> ---
>>  doc/users-guide/users-guide.adoc | 87
>> +++-
>>  1 file changed, 69 insertions(+), 18 deletions(-)
>>
>> diff --git a/doc/users-guide/users-guide.adoc
>> b/doc/users-guide/users-guide.adoc
>> index 0221634..b094802 100644
>> --- a/doc/users-guide/users-guide.adoc
>> +++ b/doc/users-guide/users-guide.adoc
>> @@ -909,24 +909,75 @@ include::users-guide-pktio.adoc[]
>>
>>  == Cryptographic services
>>
>> -ODP provides support for cryptographic operations required by various
>> security
>> -protocols (e.g. IPSec). To apply a cryptographic operation to a packet a
>> session
>> -must be created first. Packets processed by a session share the same
>> cryptographic
>> -parameters like algorithms, keys, initialization vectors. A session is
>> created with
>> -*odp_crypto_session_create()* call. After session creation a
>> cryptographic operation
>> -can be applied to a packet using *odp_crypto_operation()* call.
>> -Depending on the session type - synchronous or asynchronous the operation
>> returns
>> -when the operation completed or after the request has been submitted. In
>> the
>> -asynchronous case an operation completion event will be enqueued on the
>> session
>> -completion queue. The completion event conveys the status of the
>> operation and
>> -the result. The application has the responsibility to free the completion
>> event.
>> -The operation arguments specify for each packet the areas which are to be
>> encrypted
>> -or decrypted and authenticated. Also, in asynchronous case a context can
>> be
>> -associated with a given operation and when the operation completion event
>> is
>> -retrieved the associated context can be retrieved. An operation can be
>> executed
>> -in-place, when the output packet is the same as the input packet or the
>> output
>> -packet can be a new packet provided by the application or allocated by the
>> -implementation from the session output pool.
>> +ODP provides APIs to perform cryptographic operations required by various
>> +communication protocols (e.g. IPSec). ODP cryptographic APIs are session
>> based.
>> +
>> +ODP provides APIs for following cryptographic services:
>> +
>> +* Ciphering
>> +* Authentication/data integrity via Keyed-Hashing (HMAC)
>> +* Random number generation
>> +* Crypto capability inquiries
>> +
>> +=== Crypto Sessions
>> +
>> +To apply a cryptographic operation to a packet a session must be created.
>> All
>> +packets processed by a session share the parameters that define the
>> session.
>> +
>> +ODP supports synchronous and asynchronous crypto sessions. For
>> asynchronous
>> +sessions, the output of crypto operation is posted in a queue defined as
>> +the completion queue in its session parameters.
>> +
>> +ODP crypto APIs support chained operation sessions in which hashing and
>> ciphering
>> +both can be achieved using a single session and operation call. The order
>> of
>> +cipher and hashing can be controlled by the `auth_cipher_text` session
>> parameter.
>> +
>> +Other Session parameters include algorithms, keys, initialization vector
>> +(optional), encode or decode, output queue for async mode and output
>> packet pool
>> +for allocation of an output packet if required.
>> +
>> +=== Crypto operations
>> +
>> +After session creation, a cryptographic operation can be applied to a
>> packet
>> +using the `odp_crypto_operation()` API. Applications may indicate a
>> preference
>> +for synchronous or asynchronous processing in the session's `pref_mode`
>> parameter.
>> +However crypto operations may complete synchronously even if an
>> asynchronous
>> +preference is indicated, and applications must examine the `posted` output
>> +parameter from `odp_crypto_operation()` to determine whether the
>> operation has
>> +completed or if an `ODP_EVENT_CRYPTO_COMPL` notification is expected. In
>> the case
>> +of an async operation, the `posted` output parameter will be set to true.
>> +
>> +
>> +The operation arguments specify for each packet the areas that are to be
>> +encrypted or decrypted and authenticated. Also, there is an option of
>> overriding
>> +the initialization vector specified in session parameters.
>> +
>> +An operation can be executed in in-place, out-of-place or new buffer mode.
>> +In in-place mode output packet is same as the input packet.
>> +In case of out-of-place mode output packet is different from input packet
>> as
>> +specified by the application, while in new buffer mode implementation
>> allocates
>> +a new output buffer from the session’s output pool.
>> +
>> +The application can also specify a context associated with a given
>> operation that
>> +will be retained during async operation and can be retrieved via the
>> completion
>> +event.
>>

Re: [lng-odp] "Message too long" error message with Iperf testing

[gyanesh patra]

I am using the linux-generic with ODP  v1.9.0.0
I am just running Iperf with default settings. 
But i will try similar experiments by turning off the segment offloading on the 
target NIC. Thank you for the pointer.

Regards,
Gyanesh Patra

> On May 16, 2016, at 21:21, Bill Fischofer  wrote:
> 
> 
> 
> On Mon, May 16, 2016 at 6:45 PM, gyanesh patra  > wrote:
> I have a simple *standalone application* using ODP. It receives packets on
> one interface and do a broadcasting to all other interfaces. But many a
> times i see this error message when i try to calculate the throughput using
> IPerf tool.
> 
>   pktio/socket_mmap.c:263:pkt_mmap_v2_tx():sendto(pkt mmap):
> Message too long
> 
> What implementation of ODP are you running and at what level? linux-generic, 
> odp-dpdk, a vendor implementation?
> 
> ODP does not support Large Segment Offload (LSO) processing, so if the packet 
> you're trying to send exceeds the target pktio's MTU as returned by the 
> odp_pktio_mtu() API, then you'll see TX failures.
>  
> 
> I don't have jumbo frames enabled. Also i don't think IPerf is sending any
> jumbo frames. When i get this error, all packets were dropped and the
> throughput comes down to a very low number.
> 
> What seems to be a problem? Is there any specific configuration i need to
> do with respect to ODP?
> 
> I am using one thread for each interface. Each interface has one TX and one
> RX queue configured. I am using Burst mode to receive the packet type.
> 
> Thank you
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org 
> https://lists.linaro.org/mailman/listinfo/lng-odp 
> 
> 

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] "Message too long" error message with Iperf testing

[Bill Fischofer]

On Mon, May 16, 2016 at 6:45 PM, gyanesh patra 
wrote:

> I have a simple *standalone application* using ODP. It receives packets on
> one interface and do a broadcasting to all other interfaces. But many a
> times i see this error message when i try to calculate the throughput using
> IPerf tool.
>
>   pktio/socket_mmap.c:263:pkt_mmap_v2_tx():sendto(pkt mmap):
> Message too long
>

What implementation of ODP are you running and at what level?
linux-generic, odp-dpdk, a vendor implementation?

ODP does not support Large Segment Offload (LSO) processing, so if the
packet you're trying to send exceeds the target pktio's MTU as returned by
the odp_pktio_mtu() API, then you'll see TX failures.


>
> I don't have jumbo frames enabled. Also i don't think IPerf is sending any
> jumbo frames. When i get this error, all packets were dropped and the
> throughput comes down to a very low number.
>
> What seems to be a problem? Is there any specific configuration i need to
> do with respect to ODP?
>
> I am using one thread for each interface. Each interface has one TX and one
> RX queue configured. I am using Burst mode to receive the packet type.
>
> Thank you
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] process thread and mixed mode

[Bill Fischofer]

On Fri, May 13, 2016 at 6:32 AM, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolai...@nokia.com> wrote:

>
>
>
>
> *From:* Christophe Milard [mailto:christophe.mil...@linaro.org]
> *Sent:* Friday, May 13, 2016 11:16 AM
> *To:* Petri Savolainen ; Brian Brooks <
> brian.bro...@linaro.org>; Bill Fischofer ;
> Mike Holmes ; LNG ODP Mailman List <
> lng-odp@lists.linaro.org>; Anders Roxell 
> *Cc:* Tobias Lindquist 
> *Subject:* process thread and mixed mode
>
>
>
> Hi,
>
>
>
> Let me start in a bit unusual way :-). I do not like my "running things in
> process mode" patch series  that much...
>
> But I do think it should be merged. With the mixed mode.
>
>
>
> I think mixed mode is good addition, but would need more options: e.g.
> --odph-num-proc  --odph-num-pthread-per-proc  and thus can be
> added later. Pure proc mode is enough to get things started.
>
>
>
> The main goal of the patch series is to push the work regarding a proper
> and consistent definition of some ODP low level concepts and objects. The
> patch series makes things fail, hence pushing for defining and coding
> things better, I hope.  Removing "provocative" things like the mixed mode
> from it is just defeating the purpose of this series: I'll be happy to
> remove it (and plenty of other things) when we know which well documented
> definition/rule makes this code not relevant.
>
> As long as we keep saying that odp threads could be indifferently  linux
> processes or pthreads, I'll be thinking that this series should work,
> inclusive any kind of mixed mode.
>
>
>
> However, processes and threads are different things in linux, and I
> believe this distinction should be kept within ODP: Trying to make
> processes behave as pthreads for some ODP objects (such as addresses for
> shmem objects) does not sound like a good idea to me. Yes, this can maybe
> be done with linux -and even possibly without preallocating the memory-,
> but still, linux creates 2 distinct virtual address spaces on fork() so why
> would ODP put any exception to that?.
> ...And If we do want to create such exceptions (which I don't like),
> shouln't ODP define its own concept of process and thread as it obviously
> wouldn't match 100% the definition of the OS?
>
>
>
>
>
> A lot of application internal state data may be directly managed with OS.
> But I think we cannot depend 100% on OS shared data (== remove shm API all
> together), since HW/implementation could have multiple ways optimize
> where/how the memory is allocated/mapped (chip internal vs. external
> memory, huge page mapped vs. normal page mapped, IO-MMU mapped, etc) … or
> when passing pointers to ODP API, the API spec may require that the memory
> is allocated from SHM (application would not know if the API function is
> implemented as HW or SW).
>
>
>
>
>
> I do nevertheless definitively agree with Petri when saying that that ODP
> applications should be given a way to "share pointers" between odpthreads.
> Taking the "process only" behavior forcing every concurrent execution tasks
> to use tricky offset calculation to find their data is too nasty and
> inefficient.
>
>
>
> I think this will boil down to 2 choices:
>
> 1) Take these object definitions from the OS and accept the OS rules.
>
> 2) Define our own ODP objects (and possibly defines our own rules)
>
>
>
> 1)Taking the OS definitions:
>
> Choice 1) is consistent as long as we accept the OS rules. Taking that
> choice would mean that any address remain valid within a process only:
> shmem pointers will not be able to be shared between different processes: I
> do not see that as a big problem as the applications can use pthreads if it
> needs to share the same VA, or can fork() at a conveniant time. Moreover,
> existing applications which should be ported to ODP are probably OK as they
> would already be using pthreads/processes in the correct way.
>
>
>
> A more tricky aspect of choice 1) is the development of ODP itself:
> Operations involving many "ODPthreads" will have to cope with both
> processes and threads possibly without a clear view of what is done by the
> application. ( I guess in linux if getpid()==gettid() when calling
> odp_init_local() one can assume safely that the app forked...?).
> I guess it is doable: assuming all odpthreads are descendant of the
> instanciation process, ODP can initially reserve shared memory in
> odp_init_global() for its own purpose. It has a price: within ODP (and
> anything external to the app itself, such as drivers) a common VA cannot be
> assumed. ODP/drivers will have be prepared for an app fork at any time and
> hence do its internal stuff as if pointers could not be shared (except
> within the initially allocated memory). This will cost ODP performance.
>
>
>
> Taking choice 1 probably also mean that shmen should no longer be part of
> ODP: Allocating memory is as much an OS thing as forking a process... why
> would shmem objects be ODP objects while ODP thread would not be?
>
>
>
>
>
> Memory is a

[lng-odp] "Message too long" error message with Iperf testing

[gyanesh patra]

I have a simple *standalone application* using ODP. It receives packets on
one interface and do a broadcasting to all other interfaces. But many a
times i see this error message when i try to calculate the throughput using
IPerf tool.

  pktio/socket_mmap.c:263:pkt_mmap_v2_tx():sendto(pkt mmap):
Message too long

I don't have jumbo frames enabled. Also i don't think IPerf is sending any
jumbo frames. When i get this error, all packets were dropped and the
throughput comes down to a very low number.

What seems to be a problem? Is there any specific configuration i need to
do with respect to ODP?

I am using one thread for each interface. Each interface has one TX and one
RX queue configured. I am using Burst mode to receive the packet type.

Thank you
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH v2 5/5] linux-generic: packet: initialize only selected odp_packet_hdr_t members

[Bill Fischofer]

On Mon, May 16, 2016 at 9:25 AM, Mike Holmes  wrote:

> On 16 May 2016 at 05:50, Matias Elo  wrote:
>
> > Using memset to initialize struct odp_packet_hdr_t contents
> > to 0 has a significant overhead. Instead, initialize only
> > the required members of the struct.
> >
> > Signed-off-by: Matias Elo 
> > ---
> >  platform/linux-generic/include/odp_packet_internal.h | 13 +
> >  platform/linux-generic/odp_packet.c  | 18
> > ++
> >  2 files changed, 15 insertions(+), 16 deletions(-)
> >
> > diff --git a/platform/linux-generic/include/odp_packet_internal.h
> > b/platform/linux-generic/include/odp_packet_internal.h
> > index 1306b05..c87bc9f 100644
> > --- a/platform/linux-generic/include/odp_packet_internal.h
> > +++ b/platform/linux-generic/include/odp_packet_internal.h
> > @@ -4,7 +4,6 @@
> >   * SPDX-License-Identifier: BSD-3-Clause
> >   */
> >
> -
> >  /**
> >   * @file
> >   *
> > @@ -129,11 +128,16 @@ ODP_STATIC_ASSERT(sizeof(output_flags_t) ==
> > sizeof(uint32_t),
> >
> >  /**
> >   * Internal Packet header
> > + *
> > + * To optimize fast path performance this struct is not initialized to
> > zero in
> > + * packet_init(). Because of this any new fields added must be reviewed
> > for
> > + * initialization requirements.
> >   */
>
>
> This file will not contribute to the generated API documentation and I
>

It's not relevant to the API, only to the odp-linux implementation of the
API.


> think it should. This platform specific warning should appear there as a
> doxygen  @warning
> Perhaps we need to start a trend of adding a doxygen platform specific
> module called "PLATFORM SPECIFICS", then we can add information with the
> @addtogroup platform_specifics and users across platforms will know where
> to look for this sort of information.


The best place for this sort of implementation documentation is in the
comments in the C code itself. We just want a reminder to future
maintainers of the code of a shortcut that was taken for performance
reasons and the cautions that need to be observed as a result.

If we want to have these sort of notes be collated as part of a
"Maintainer's Guide" for odp-linux, that would seem a worthwhile project.
Something to consider adding for Tiger Moth?


>
>  typedef struct {
> > /* common buffer header */
> > odp_buffer_hdr_t buf_hdr;
> >
> > +   /* Following members are initialized by packet_init() */
> > input_flags_t  input_flags;
> > error_flags_t  error_flags;
> > output_flags_t output_flags;
> > @@ -142,15 +146,16 @@ typedef struct {
> > uint32_t l3_offset; /**< offset to L3 hdr, e.g. IPv4, IPv6 */
> > uint32_t l4_offset; /**< offset to L4 hdr (TCP, UDP, SCTP, also
> > ICMP) */
> >
> > -   uint32_t l3_len; /**< Layer 3 length */
> > -   uint32_t l4_len; /**< Layer 4 length */
> > -
> > uint32_t frame_len;
> > uint32_t headroom;
> > uint32_t tailroom;
> >
> > odp_pktio_t input;
> >
> > +   /* Members below are not initialized by packet_init() */
> > +   uint32_t l3_len; /**< Layer 3 length */
> > +   uint32_t l4_len; /**< Layer 4 length */
> > +
> > uint32_t flow_hash;  /**< Flow hash value */
> > odp_time_t timestamp;/**< Timestamp value */
> >
> > diff --git a/platform/linux-generic/odp_packet.c
> > b/platform/linux-generic/odp_packet.c
> > index 4f523c9..8dde404 100644
> > --- a/platform/linux-generic/odp_packet.c
> > +++ b/platform/linux-generic/odp_packet.c
> > @@ -49,19 +49,11 @@ void packet_parse_reset(odp_packet_hdr_t *pkt_hdr)
> >  static void packet_init(pool_entry_t *pool, odp_packet_hdr_t *pkt_hdr,
> > size_t size, int parse)
> >  {
> > -   /*
> > -   * Reset parser metadata.  Note that we clear via memset to make
> > -   * this routine indepenent of any additional adds to packet
> > metadata.
> > -   */
> > -   const size_t start_offset = ODP_FIELD_SIZEOF(odp_packet_hdr_t,
> > buf_hdr);
> > -   uint8_t *start;
> > -   size_t len;
> > +   pkt_hdr->input_flags.all  = 0;
> > +   pkt_hdr->output_flags.all = 0;
> > +   pkt_hdr->error_flags.all  = 0;
> >
> > -   start = (uint8_t *)pkt_hdr + start_offset;
> > -   len = sizeof(odp_packet_hdr_t) - start_offset;
> > -   memset(start, 0, len);
> > -
> > -   /* Set metadata items that initialize to non-zero values */
> > +   pkt_hdr->l2_offset = 0;
> > pkt_hdr->l3_offset = ODP_PACKET_OFFSET_INVALID;
> > pkt_hdr->l4_offset = ODP_PACKET_OFFSET_INVALID;
> >
> > @@ -79,6 +71,8 @@ static void packet_init(pool_entry_t *pool,
> > odp_packet_hdr_t *pkt_hdr,
> > pkt_hdr->tailroom  =
> > (pool->s.seg_size * pkt_hdr->buf_hdr.segcount) -
> > (pool->s.headroom + size);
> > +
> > +   pkt_hdr->input = ODP_PKTIO_INVALID;
> >  }
> >
> >  odp_packet_t packet_alloc(odp_pool_t 

Re: [lng-odp] [PATCHv2] linux-generic: timer: remove dead 128 bit optimizations

[Bill Fischofer]

On Mon, May 16, 2016 at 4:55 PM, Ola Liljedahl 
wrote:

> On 16 May 2016 at 23:31, Bill Fischofer  wrote:
>
>> Thanks Ola.  The original bug is that this fails compiling with clang on
>> 32-bit systems and the reason is that the struct in that environment winds
>> up being 24 bytes rather than 16 bytes long, so the ODP_STATIC_ASSERT()
>> fails.
>>
> The timer code should stop using odp_atomic_u64_t (which will include the
> spinlock for systems that do not support atomic operations on 64-bit
> locations, ARMv7A is one of the few 32-bit platforms which do support
> 64-bit atomics natively). Instead use a plain 64-bit datatype (uint64_t) on
> such systems. For systems which do not support 128-bit (or 64-bit?) atomic
> operations (which is the ideal but I think I managed to get something
> working with only 64-bit atomics), we should use locks instead. I think the
> code does use a separate array of locks which is indexed through a hash of
> the timer handle or something similar. This locks makes the additional lock
> in odp_atomic_u64_t unused.
>
> Which platforms use the 64-bit atomic code in the timer?
> ARMv7A (32-bit but with 64-bit atomics support - ldrexd/strexd)
> OCTEON/MIPS64 (with 64-bit atomics support - lld/scd)
> 64-bit POWER? (with 64--bit atomics support - ldarx/stdcx.)
>
> Se we need a couple of things:
> 1) 128-bit CAS support on e.g. x86 (requires that configure detects and
> enables -mcx16 flag).
> 2) 128-bit ldxp/stxp support on ARM64 (A64 ISA in AArch64 mode). I don't
> think common compilers support 128-bit CAS on ARM64 (it's a bit tricky) so
> need to use inline assembly.
> 3) Some cleanup in the timer code not to use odp_atomic_u64_t on systems
> which do not support 64-bit atomic operations (and instead uses the spin
> lock array). This will avoid the failed static assert.
>

Unless you'd like to submit that patch this week, we need to do something
on at least a temporary basis to close out Monarch RC3. Sounds like this is
an area we should revisit for restructure/cleanup for Tiger Moth. Given the
divergence between near-term need and the (better) longer-term solution you
outline, what do you recommend for now?


>
>
>> My original (simple) patch is at
>> http://patches.opendataplane.org/patch/5932/ however Maxim didn't like
>> it. Perhaps you can offer a compromise?
>>
>> On Mon, May 16, 2016 at 4:21 PM, Ola Liljedahl 
>> wrote:
>>
>>> I disagree. The 128-bit support is important because that's the
>>> lock-free timer implementation which was the whole idea. The lock-based
>>> code is just a work-around. We should rather add support in configure to
>>> detect compiler support for the -mcx16 flag which enables 128-bit CAS on
>>> x86-64. Now when I have an ARM64 target, I can actually write and test
>>> 128-bit CAS support on ARM (A64 ISA in AArch64 mode) and contribute that in
>>> another patch. I am not sure whether common compilers (e.g. GCC) properly
>>> supports 128-bit CAS on ARM64, might have to use LDXP/STXP (load/store
>>> exclusive pair) directly.
>>>
>>> -- Ola
>>>
>>> On 16 May 2016 at 22:43, Bill Fischofer 
>>> wrote:
>>>
 On Mon, May 16, 2016 at 2:15 PM, Maxim Uvarov 
 wrote:

 > Remove totally untested branch with 128 bit optimizations
 > for timer code.
 > This also fixes static assert bug:
 > https://bugs.linaro.org/show_bug.cgi?id=2211
 >
 > Signed-off-by: Maxim Uvarov 
 >

 Reviewed-and-tested-by: Bill Fischofer 


 > ---
 >  platform/linux-generic/odp_timer.c | 107
 > -
 >  1 file changed, 107 deletions(-)
 >
 > diff --git a/platform/linux-generic/odp_timer.c
 > b/platform/linux-generic/odp_timer.c
 > index 6b84309..be28da3 100644
 > --- a/platform/linux-generic/odp_timer.c
 > +++ b/platform/linux-generic/odp_timer.c
 > @@ -11,15 +11,7 @@
 >   *
 >   */
 >
 > -/* Check if compiler supports 16-byte atomics. GCC needs -mcx16 flag
 on
 > x86 */
 > -/* Using spin lock actually seems faster on Core2 */
 > -#ifdef ODP_ATOMIC_U128
 > -/* TB_NEEDS_PAD defined if sizeof(odp_buffer_t) != 8 */
 > -#define TB_NEEDS_PAD
 > -#define TB_SET_PAD(x) ((x).pad = 0)
 > -#else
 >  #define TB_SET_PAD(x) (void)(x)
 > -#endif
 >
 >  #include 
 >
 > @@ -70,11 +62,9 @@
 >   * Mutual exclusion in the absence of CAS16
 >
 >
 */
 >
 > -#ifndef ODP_ATOMIC_U128
 >  #define NUM_LOCKS 1024
 >  static _odp_atomic_flag_t locks[NUM_LOCKS]; /* Multiple locks per
 cache
 > line! */
 >  #define IDX2LOCK(idx) (&locks[(idx) % NUM_LOCKS])
 > -#endif
 >
 >
 >
 /**
 >   * Translation between timeout buffer and timeout header
 > @@ -98,17 +88,9 @@ static odp_timeout_hdr_t
 *timeout

Re: [lng-odp] [PATCHv2] linux-generic: timer: remove dead 128 bit optimizations

[Ola Liljedahl]

On 16 May 2016 at 23:31, Bill Fischofer  wrote:

> Thanks Ola.  The original bug is that this fails compiling with clang on
> 32-bit systems and the reason is that the struct in that environment winds
> up being 24 bytes rather than 16 bytes long, so the ODP_STATIC_ASSERT()
> fails.
>
The timer code should stop using odp_atomic_u64_t (which will include the
spinlock for systems that do not support atomic operations on 64-bit
locations, ARMv7A is one of the few 32-bit platforms which do support
64-bit atomics natively). Instead use a plain 64-bit datatype (uint64_t) on
such systems. For systems which do not support 128-bit (or 64-bit?) atomic
operations (which is the ideal but I think I managed to get something
working with only 64-bit atomics), we should use locks instead. I think the
code does use a separate array of locks which is indexed through a hash of
the timer handle or something similar. This locks makes the additional lock
in odp_atomic_u64_t unused.

Which platforms use the 64-bit atomic code in the timer?
ARMv7A (32-bit but with 64-bit atomics support - ldrexd/strexd)
OCTEON/MIPS64 (with 64-bit atomics support - lld/scd)
64-bit POWER? (with 64--bit atomics support - ldarx/stdcx.)

Se we need a couple of things:
1) 128-bit CAS support on e.g. x86 (requires that configure detects and
enables -mcx16 flag).
2) 128-bit ldxp/stxp support on ARM64 (A64 ISA in AArch64 mode). I don't
think common compilers support 128-bit CAS on ARM64 (it's a bit tricky) so
need to use inline assembly.
3) Some cleanup in the timer code not to use odp_atomic_u64_t on systems
which do not support 64-bit atomic operations (and instead uses the spin
lock array). This will avoid the failed static assert.


> My original (simple) patch is at
> http://patches.opendataplane.org/patch/5932/ however Maxim didn't like
> it. Perhaps you can offer a compromise?
>
> On Mon, May 16, 2016 at 4:21 PM, Ola Liljedahl 
> wrote:
>
>> I disagree. The 128-bit support is important because that's the lock-free
>> timer implementation which was the whole idea. The lock-based code is just
>> a work-around. We should rather add support in configure to detect compiler
>> support for the -mcx16 flag which enables 128-bit CAS on x86-64. Now when I
>> have an ARM64 target, I can actually write and test 128-bit CAS support on
>> ARM (A64 ISA in AArch64 mode) and contribute that in another patch. I am
>> not sure whether common compilers (e.g. GCC) properly supports 128-bit CAS
>> on ARM64, might have to use LDXP/STXP (load/store exclusive pair) directly.
>>
>> -- Ola
>>
>> On 16 May 2016 at 22:43, Bill Fischofer 
>> wrote:
>>
>>> On Mon, May 16, 2016 at 2:15 PM, Maxim Uvarov 
>>> wrote:
>>>
>>> > Remove totally untested branch with 128 bit optimizations
>>> > for timer code.
>>> > This also fixes static assert bug:
>>> > https://bugs.linaro.org/show_bug.cgi?id=2211
>>> >
>>> > Signed-off-by: Maxim Uvarov 
>>> >
>>>
>>> Reviewed-and-tested-by: Bill Fischofer 
>>>
>>>
>>> > ---
>>> >  platform/linux-generic/odp_timer.c | 107
>>> > -
>>> >  1 file changed, 107 deletions(-)
>>> >
>>> > diff --git a/platform/linux-generic/odp_timer.c
>>> > b/platform/linux-generic/odp_timer.c
>>> > index 6b84309..be28da3 100644
>>> > --- a/platform/linux-generic/odp_timer.c
>>> > +++ b/platform/linux-generic/odp_timer.c
>>> > @@ -11,15 +11,7 @@
>>> >   *
>>> >   */
>>> >
>>> > -/* Check if compiler supports 16-byte atomics. GCC needs -mcx16 flag
>>> on
>>> > x86 */
>>> > -/* Using spin lock actually seems faster on Core2 */
>>> > -#ifdef ODP_ATOMIC_U128
>>> > -/* TB_NEEDS_PAD defined if sizeof(odp_buffer_t) != 8 */
>>> > -#define TB_NEEDS_PAD
>>> > -#define TB_SET_PAD(x) ((x).pad = 0)
>>> > -#else
>>> >  #define TB_SET_PAD(x) (void)(x)
>>> > -#endif
>>> >
>>> >  #include 
>>> >
>>> > @@ -70,11 +62,9 @@
>>> >   * Mutual exclusion in the absence of CAS16
>>> >
>>> >
>>> */
>>> >
>>> > -#ifndef ODP_ATOMIC_U128
>>> >  #define NUM_LOCKS 1024
>>> >  static _odp_atomic_flag_t locks[NUM_LOCKS]; /* Multiple locks per
>>> cache
>>> > line! */
>>> >  #define IDX2LOCK(idx) (&locks[(idx) % NUM_LOCKS])
>>> > -#endif
>>> >
>>> >
>>> >
>>> /**
>>> >   * Translation between timeout buffer and timeout header
>>> > @@ -98,17 +88,9 @@ static odp_timeout_hdr_t *timeout_hdr(odp_timeout_t
>>> tmo)
>>> >  typedef struct tick_buf_s {
>>> > odp_atomic_u64_t exp_tck;/* Expiration tick or TMO_xxx */
>>> > odp_buffer_t tmo_buf;/* ODP_BUFFER_INVALID if timer not active
>>> */
>>> > -#ifdef TB_NEEDS_PAD
>>> > -   uint32_t pad;/* Need to be able to access padding for
>>> successful
>>> > CAS */
>>> > -#endif
>>> >  } tick_buf_t
>>> > -#ifdef ODP_ATOMIC_U128
>>> > -ODP_ALIGNED(16) /* 16-byte atomic operations need properly aligned
>>> > addresses */
>>> > -#endif
>>> >  ;
>>> >
>>> > -ODP_STATIC_ASSERT

Re: [lng-odp] [PATCHv3 2/2] linux-generic: drop odp_ prefix for internal cpuinfo

[Bill Fischofer]

I just did what Matias did and built myself a 32-bit Ubuntu VM to test
32-bit stuff with.  The -m32 compile option isn't quite the same and
obviously missed some cases.  I'm not sure how to incorporate that into
check-odp.

On Mon, May 16, 2016 at 10:49 AM, Mike Holmes 
wrote:

> Bill,
>
> Were you able to get the instructions / patch together to do a true 32bit
> check in check-odp ?
>
> If we are going to reject things that fail for 32bits we should describe
> how to recreate the test if you only have a 64 bit system.
>
> Mike
>
> On 12 May 2016 at 16:21, Bill Fischofer  wrote:
>
>> Compiling this on a 32-bit system I get:
>>
>>   CC   odp_system_info.lo
>> odp_system_info.c: In function ‘default_huge_page_size’:
>> odp_system_info.c:86:19: error: format ‘%lu’ expects argument of type
>> ‘long unsigned int *’, but argument 3 has type ‘uint64_t * {aka long long
>> unsigned int *}’ [-Werror=format=]
>>if (sscanf(str, "Hugepagesize: %8lu kB", &sz) == 1) {
>>^
>> cc1: all warnings being treated as errors
>>
>>
>> On Wed, May 11, 2016 at 8:02 AM, Maxim Uvarov 
>> wrote:
>>
>>> A little bit code clean up to drop odp_ prefix from internal things
>>> and rename huge_pages to default_huge_pages internal struct.
>>>
>>> Signed-off-by: Maxim Uvarov 
>>> ---
>>>  platform/linux-generic/arch/default/odp_sysinfo_parse.c |  2 +-
>>>  platform/linux-generic/arch/mips64/odp_sysinfo_parse.c  |  2 +-
>>>  platform/linux-generic/arch/powerpc/odp_sysinfo_parse.c |  2 +-
>>>  platform/linux-generic/arch/x86/odp_sysinfo_parse.c |  2 +-
>>>  platform/linux-generic/include/odp_internal.h   |  8 
>>>  platform/linux-generic/odp_system_info.c| 10 +-
>>>  6 files changed, 13 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/platform/linux-generic/arch/default/odp_sysinfo_parse.c
>>> b/platform/linux-generic/arch/default/odp_sysinfo_parse.c
>>> index 4dcd6d1..53e2aae 100644
>>> --- a/platform/linux-generic/arch/default/odp_sysinfo_parse.c
>>> +++ b/platform/linux-generic/arch/default/odp_sysinfo_parse.c
>>> @@ -8,7 +8,7 @@
>>>  #include 
>>>  #include 
>>>
>>> -int odp_cpuinfo_parser(FILE *file ODP_UNUSED, odp_system_info_t
>>> *sysinfo)
>>> +int cpuinfo_parser(FILE *file ODP_UNUSED, system_info_t *sysinfo)
>>>  {
>>> int i;
>>>
>>> diff --git a/platform/linux-generic/arch/mips64/odp_sysinfo_parse.c
>>> b/platform/linux-generic/arch/mips64/odp_sysinfo_parse.c
>>> index d45b420..407264b 100644
>>> --- a/platform/linux-generic/arch/mips64/odp_sysinfo_parse.c
>>> +++ b/platform/linux-generic/arch/mips64/odp_sysinfo_parse.c
>>> @@ -7,7 +7,7 @@
>>>  #include 
>>>  #include 
>>>
>>> -int odp_cpuinfo_parser(FILE *file, odp_system_info_t *sysinfo)
>>> +int cpuinfo_parser(FILE *file, system_info_t *sysinfo)
>>>  {
>>> char str[1024];
>>> char *pos;
>>> diff --git a/platform/linux-generic/arch/powerpc/odp_sysinfo_parse.c
>>> b/platform/linux-generic/arch/powerpc/odp_sysinfo_parse.c
>>> index 95200ee..3b88d55 100644
>>> --- a/platform/linux-generic/arch/powerpc/odp_sysinfo_parse.c
>>> +++ b/platform/linux-generic/arch/powerpc/odp_sysinfo_parse.c
>>> @@ -7,7 +7,7 @@
>>>  #include 
>>>  #include 
>>>
>>> -int odp_cpuinfo_parser(FILE *file, odp_system_info_t *sysinfo)
>>> +int cpuinfo_parser(FILE *file, system_info_t *sysinfo)
>>>  {
>>> char str[1024];
>>> char *pos;
>>> diff --git a/platform/linux-generic/arch/x86/odp_sysinfo_parse.c
>>> b/platform/linux-generic/arch/x86/odp_sysinfo_parse.c
>>> index c1e05c0..96127ec 100644
>>> --- a/platform/linux-generic/arch/x86/odp_sysinfo_parse.c
>>> +++ b/platform/linux-generic/arch/x86/odp_sysinfo_parse.c
>>> @@ -7,7 +7,7 @@
>>>  #include 
>>>  #include 
>>>
>>> -int odp_cpuinfo_parser(FILE *file, odp_system_info_t *sysinfo)
>>> +int cpuinfo_parser(FILE *file, system_info_t *sysinfo)
>>>  {
>>> char str[1024];
>>> char *pos;
>>> diff --git a/platform/linux-generic/include/odp_internal.h
>>> b/platform/linux-generic/include/odp_internal.h
>>> index 28a4fc4..19ab40a 100644
>>> --- a/platform/linux-generic/include/odp_internal.h
>>> +++ b/platform/linux-generic/include/odp_internal.h
>>> @@ -30,19 +30,19 @@ extern __thread int __odp_errno;
>>>
>>>  typedef struct {
>>> uint64_t cpu_hz_max[MAX_CPU_NUMBER];
>>> -   uint64_t huge_page_size;
>>> +   uint64_t default_huge_page_size;
>>> uint64_t page_size;
>>> int  cache_line_size;
>>> int  cpu_count;
>>> char cpu_arch_str[128];
>>> char model_str[MAX_CPU_NUMBER][128];
>>> -} odp_system_info_t;
>>> +} system_info_t;
>>>
>>>  struct odp_global_data_s {
>>> pid_t main_pid;
>>> odp_log_func_t log_fn;
>>> odp_abort_func_t abort_fn;
>>> -   odp_system_info_t system_info;
>>> +   system_info_t system_info;
>>> odp_cpumask_t control_cpus;
>>> odp_cpumask_t worker_cpus;
>>> int num_cpus_installed;
>>> @@ -126,7 +126,

Re: [lng-odp] [PATCH 2/2] time: fix invalid casting on a 32-bit host

[Bill Fischofer]

On Mon, May 16, 2016 at 9:27 AM, Ivan Khoronzhuk  wrote:

> Hi Matias,
>
> The odp_time_local and others functions are time sensitive functions,
> that's why it was decided to avoid copping as more as possible.
>
> The timespec is not simple "long type". Its type is arch dependent but is
> always 64bit.
> In case of 32 bit system it's defined as long long.
> The same for odp_time_t struct. So, at least for now it seems to be the
> same for
> both 32 and 64 bit systems. And I think Bill Fischofer knew about this
> while adding this
> ,at first glance, strange union, right Bill?
>

Yes. The original purpose of that union was not efficiency (though that's a
happy side-effect) but rather to remove the need for ODP applications to be
dependent on the variable expansion of the Linux timespec struct, which is
dependent on POSIX level. That was an earlier bug that caused much grief.


> Yes, it's not the best decision from style point of view, but it's fast
> and in case of an error
> is supposed to be caught by time validation tests.
>
>
> On 04.05.16 16:01, Matias Elo wrote:
>
>> The size of 'struct timespec' may vary on different host
>> architectures as it includes type long members. This breaks
>> time functions on a 32-bit x86 host. Fix this by
>> individually copying struct timespec members to odp_time_t.
>>
>> Signed-off-by: Matias Elo 
>> ---
>>   platform/linux-generic/odp_time.c | 26 +++---
>>   1 file changed, 15 insertions(+), 11 deletions(-)
>>
>> diff --git a/platform/linux-generic/odp_time.c
>> b/platform/linux-generic/odp_time.c
>> index 040f754..81e0522 100644
>> --- a/platform/linux-generic/odp_time.c
>> +++ b/platform/linux-generic/odp_time.c
>> @@ -11,11 +11,6 @@
>>   #include 
>>   #include 
>>
>> -typedef union {
>> -   odp_time_t  ex;
>> -   struct timespec in;
>> -} _odp_time_t;
>> -
>>   static odp_time_t start_time;
>>
>>   static inline
>> @@ -47,13 +42,17 @@ static inline odp_time_t time_diff(odp_time_t t2,
>> odp_time_t t1)
>>   static inline odp_time_t time_local(void)
>>   {
>> int ret;
>> -   _odp_time_t time;
>> +   odp_time_t time;
>> +   struct timespec sys_time;
>>
>> -   ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in);
>> +   ret = clock_gettime(CLOCK_MONOTONIC_RAW, &sys_time);
>> if (odp_unlikely(ret != 0))
>> ODP_ABORT("clock_gettime failed\n");
>>
>> -   return time_diff(time.ex, start_time);
>> +   time.tv_sec = sys_time.tv_sec;
>> +   time.tv_nsec = sys_time.tv_nsec;
>> +
>> +   return time_diff(time, start_time);
>>   }
>>
>>   static inline int time_cmp(odp_time_t t2, odp_time_t t1)
>> @@ -195,10 +194,15 @@ uint64_t odp_time_to_u64(odp_time_t time)
>>   int odp_time_init_global(void)
>>   {
>> int ret;
>> -   _odp_time_t time;
>> +   struct timespec time;
>>
>> -   ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in);
>> -   start_time = ret ? ODP_TIME_NULL : time.ex;
>> +   ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time);
>> +   if (ret) {
>> +   start_time = ODP_TIME_NULL;
>> +   } else {
>> +   start_time.tv_sec = time.tv_sec;
>> +   start_time.tv_nsec = time.tv_nsec;
>> +   }
>>
>> return ret;
>>   }
>>
>>
> --
> Regards,
> Ivan Khoronzhuk
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCHv2] linux-generic: timer: remove dead 128 bit optimizations

[Bill Fischofer]

Thanks Ola.  The original bug is that this fails compiling with clang on
32-bit systems and the reason is that the struct in that environment winds
up being 24 bytes rather than 16 bytes long, so the ODP_STATIC_ASSERT()
fails.

My original (simple) patch is at
http://patches.opendataplane.org/patch/5932/ however Maxim didn't like it.
Perhaps you can offer a compromise?

On Mon, May 16, 2016 at 4:21 PM, Ola Liljedahl 
wrote:

> I disagree. The 128-bit support is important because that's the lock-free
> timer implementation which was the whole idea. The lock-based code is just
> a work-around. We should rather add support in configure to detect compiler
> support for the -mcx16 flag which enables 128-bit CAS on x86-64. Now when I
> have an ARM64 target, I can actually write and test 128-bit CAS support on
> ARM (A64 ISA in AArch64 mode) and contribute that in another patch. I am
> not sure whether common compilers (e.g. GCC) properly supports 128-bit CAS
> on ARM64, might have to use LDXP/STXP (load/store exclusive pair) directly.
>
> -- Ola
>
> On 16 May 2016 at 22:43, Bill Fischofer  wrote:
>
>> On Mon, May 16, 2016 at 2:15 PM, Maxim Uvarov 
>> wrote:
>>
>> > Remove totally untested branch with 128 bit optimizations
>> > for timer code.
>> > This also fixes static assert bug:
>> > https://bugs.linaro.org/show_bug.cgi?id=2211
>> >
>> > Signed-off-by: Maxim Uvarov 
>> >
>>
>> Reviewed-and-tested-by: Bill Fischofer 
>>
>>
>> > ---
>> >  platform/linux-generic/odp_timer.c | 107
>> > -
>> >  1 file changed, 107 deletions(-)
>> >
>> > diff --git a/platform/linux-generic/odp_timer.c
>> > b/platform/linux-generic/odp_timer.c
>> > index 6b84309..be28da3 100644
>> > --- a/platform/linux-generic/odp_timer.c
>> > +++ b/platform/linux-generic/odp_timer.c
>> > @@ -11,15 +11,7 @@
>> >   *
>> >   */
>> >
>> > -/* Check if compiler supports 16-byte atomics. GCC needs -mcx16 flag on
>> > x86 */
>> > -/* Using spin lock actually seems faster on Core2 */
>> > -#ifdef ODP_ATOMIC_U128
>> > -/* TB_NEEDS_PAD defined if sizeof(odp_buffer_t) != 8 */
>> > -#define TB_NEEDS_PAD
>> > -#define TB_SET_PAD(x) ((x).pad = 0)
>> > -#else
>> >  #define TB_SET_PAD(x) (void)(x)
>> > -#endif
>> >
>> >  #include 
>> >
>> > @@ -70,11 +62,9 @@
>> >   * Mutual exclusion in the absence of CAS16
>> >
>> >
>> */
>> >
>> > -#ifndef ODP_ATOMIC_U128
>> >  #define NUM_LOCKS 1024
>> >  static _odp_atomic_flag_t locks[NUM_LOCKS]; /* Multiple locks per cache
>> > line! */
>> >  #define IDX2LOCK(idx) (&locks[(idx) % NUM_LOCKS])
>> > -#endif
>> >
>> >
>> >
>> /**
>> >   * Translation between timeout buffer and timeout header
>> > @@ -98,17 +88,9 @@ static odp_timeout_hdr_t *timeout_hdr(odp_timeout_t
>> tmo)
>> >  typedef struct tick_buf_s {
>> > odp_atomic_u64_t exp_tck;/* Expiration tick or TMO_xxx */
>> > odp_buffer_t tmo_buf;/* ODP_BUFFER_INVALID if timer not active
>> */
>> > -#ifdef TB_NEEDS_PAD
>> > -   uint32_t pad;/* Need to be able to access padding for successful
>> > CAS */
>> > -#endif
>> >  } tick_buf_t
>> > -#ifdef ODP_ATOMIC_U128
>> > -ODP_ALIGNED(16) /* 16-byte atomic operations need properly aligned
>> > addresses */
>> > -#endif
>> >  ;
>> >
>> > -ODP_STATIC_ASSERT(sizeof(tick_buf_t) == 16, "sizeof(tick_buf_t) ==
>> 16");
>> > -
>> >  typedef struct odp_timer_s {
>> > void *user_ptr;
>> > odp_queue_t queue;/* Used for free list when timer is free */
>> > @@ -378,34 +360,6 @@ static bool timer_reset(uint32_t idx,
>> > tick_buf_t *tb = &tp->tick_buf[idx];
>> >
>> > if (tmo_buf == NULL || *tmo_buf == ODP_BUFFER_INVALID) {
>> > -#ifdef ODP_ATOMIC_U128
>> > -   tick_buf_t new, old;
>> > -   do {
>> > -   /* Relaxed and non-atomic read of current
>> values */
>> > -   old.exp_tck.v = tb->exp_tck.v;
>> > -   old.tmo_buf = tb->tmo_buf;
>> > -   TB_SET_PAD(old);
>> > -   /* Check if there actually is a timeout buffer
>> > -* present */
>> > -   if (old.tmo_buf == ODP_BUFFER_INVALID) {
>> > -   /* Cannot reset a timer with neither old
>> > nor
>> > -* new timeout buffer */
>> > -   success = false;
>> > -   break;
>> > -   }
>> > -   /* Set up new values */
>> > -   new.exp_tck.v = abs_tck;
>> > -   new.tmo_buf = old.tmo_buf;
>> > -   TB_SET_PAD(new);
>> > -   /* Atomic CAS will fail if we experienced torn
>> > reads,
>> > -* retry update sequence until CAS succeeds */
>> > -   } while

Re: [lng-odp] [API-NEXT PATCH v4 2/2] helper: test: add test of cuckoo hash table

[Bill Fischofer]

Applying this part shows whitespace errors:

bill@Ubuntu15:~/linaro/ru$ git am ~/Mail/Incoming/Ru/2
Applying: helper: test: add test of cuckoo hash table
.git/rebase-apply/patch:16: indent with spaces.
  table$(EXEEXT)\
warning: 1 line adds whitespace errors.

Also, Linaro copyrights should be 2016, not 2015 since this is new code
being added this yearl.

Other than that the patch seems fine.

On Mon, May 16, 2016 at 12:28 AM, Ru Jia  wrote:

> This test program consists of basic validation tests
> and performance tests.
>
> Signed-off-by: Ru Jia 
> ---
>  helper/test/Makefile.am   |   4 +-
>  helper/test/cuckootable.c | 573
> ++
>  2 files changed, 576 insertions(+), 1 deletion(-)
>  create mode 100644 helper/test/cuckootable.c
>
> diff --git a/helper/test/Makefile.am b/helper/test/Makefile.am
> index 7f0b67d..0725405 100644
> --- a/helper/test/Makefile.am
> +++ b/helper/test/Makefile.am
> @@ -9,7 +9,8 @@ EXECUTABLES = chksum$(EXEEXT) \
>thread$(EXEEXT) \
>parse$(EXEEXT)\
>process$(EXEEXT)\
> -  table$(EXEEXT)
> +  table$(EXEEXT)\
> + cuckootable$(EXEEXT)
>
>  COMPILE_ONLY =
>
> @@ -31,3 +32,4 @@ dist_process_SOURCES = process.c
>  dist_parse_SOURCES = parse.c
>  process_LDADD = $(LIB)/libodphelper-linux.la $(LIB)/libodp-linux.la
>  dist_table_SOURCES = table.c
> +dist_cuckootable_SOURCES = cuckootable.c
> diff --git a/helper/test/cuckootable.c b/helper/test/cuckootable.c
> new file mode 100644
> index 000..e7ca63e
> --- /dev/null
> +++ b/helper/test/cuckootable.c
> @@ -0,0 +1,573 @@
> +/* Copyright (c) 2015, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier: BSD-3-Clause
> + */
> +
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + * * Redistributions of source code must retain the above copyright
> + *   notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + *   notice, this list of conditions and the following disclaimer in
> + *   the documentation and/or other materials provided with the
> + *   distribution.
> + * * Neither the name of Intel Corporation nor the names of its
> + *   contributors may be used to endorse or promote products derived
> + *   from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include <../odph_cuckootable.h>
> +
>
> +/***
> + * Hash function performance test configuration section.
> + *
> + * The five arrays below control what tests are performed. Every
> combination
> + * from the array entries is tested.
> + */
>
> +/**/
> +
> +/* 5-tuple key type */
> +struct flow_key {
> +   uint32_t ip_src;
> +   uint32_t ip_dst;
> +   uint16_t port_src;
> +   uint16_t port_dst;
> +   uint8_t proto;
> +} __packed;
> +
> +/*
> + * Print out result of unit test hash operation.
> + */
> +static void print_key_info(
> +   const char *msg, const struct flow_key *key)
> +{
> +   const uint8_t *p = (const uint8_t *)key;
> +   unsigned i;
> +
> +   printf("%s key:0x", msg);
> +   for (i = 0; i < sizeof(struct flow_key); i++)
> +   printf("%02X", p[i]);
> +   printf("\n");
> +}
> +
> +static double get_time_diff(struct timeval *start, struct timeval *end)
> +{
> +   int sec = end->tv_sec - start->tv_sec;
> +   int usec = end->tv_usec - start->tv_usec;
> +
> +   if (usec < 0) {
> +   sec--;
> + 

Re: [lng-odp] [PATCHv2] linux-generic: timer: remove dead 128 bit optimizations

[Ola Liljedahl]

I disagree. The 128-bit support is important because that's the lock-free
timer implementation which was the whole idea. The lock-based code is just
a work-around. We should rather add support in configure to detect compiler
support for the -mcx16 flag which enables 128-bit CAS on x86-64. Now when I
have an ARM64 target, I can actually write and test 128-bit CAS support on
ARM (A64 ISA in AArch64 mode) and contribute that in another patch. I am
not sure whether common compilers (e.g. GCC) properly supports 128-bit CAS
on ARM64, might have to use LDXP/STXP (load/store exclusive pair) directly.

-- Ola

On 16 May 2016 at 22:43, Bill Fischofer  wrote:

> On Mon, May 16, 2016 at 2:15 PM, Maxim Uvarov 
> wrote:
>
> > Remove totally untested branch with 128 bit optimizations
> > for timer code.
> > This also fixes static assert bug:
> > https://bugs.linaro.org/show_bug.cgi?id=2211
> >
> > Signed-off-by: Maxim Uvarov 
> >
>
> Reviewed-and-tested-by: Bill Fischofer 
>
>
> > ---
> >  platform/linux-generic/odp_timer.c | 107
> > -
> >  1 file changed, 107 deletions(-)
> >
> > diff --git a/platform/linux-generic/odp_timer.c
> > b/platform/linux-generic/odp_timer.c
> > index 6b84309..be28da3 100644
> > --- a/platform/linux-generic/odp_timer.c
> > +++ b/platform/linux-generic/odp_timer.c
> > @@ -11,15 +11,7 @@
> >   *
> >   */
> >
> > -/* Check if compiler supports 16-byte atomics. GCC needs -mcx16 flag on
> > x86 */
> > -/* Using spin lock actually seems faster on Core2 */
> > -#ifdef ODP_ATOMIC_U128
> > -/* TB_NEEDS_PAD defined if sizeof(odp_buffer_t) != 8 */
> > -#define TB_NEEDS_PAD
> > -#define TB_SET_PAD(x) ((x).pad = 0)
> > -#else
> >  #define TB_SET_PAD(x) (void)(x)
> > -#endif
> >
> >  #include 
> >
> > @@ -70,11 +62,9 @@
> >   * Mutual exclusion in the absence of CAS16
> >
> >
> */
> >
> > -#ifndef ODP_ATOMIC_U128
> >  #define NUM_LOCKS 1024
> >  static _odp_atomic_flag_t locks[NUM_LOCKS]; /* Multiple locks per cache
> > line! */
> >  #define IDX2LOCK(idx) (&locks[(idx) % NUM_LOCKS])
> > -#endif
> >
> >
> >
> /**
> >   * Translation between timeout buffer and timeout header
> > @@ -98,17 +88,9 @@ static odp_timeout_hdr_t *timeout_hdr(odp_timeout_t
> tmo)
> >  typedef struct tick_buf_s {
> > odp_atomic_u64_t exp_tck;/* Expiration tick or TMO_xxx */
> > odp_buffer_t tmo_buf;/* ODP_BUFFER_INVALID if timer not active */
> > -#ifdef TB_NEEDS_PAD
> > -   uint32_t pad;/* Need to be able to access padding for successful
> > CAS */
> > -#endif
> >  } tick_buf_t
> > -#ifdef ODP_ATOMIC_U128
> > -ODP_ALIGNED(16) /* 16-byte atomic operations need properly aligned
> > addresses */
> > -#endif
> >  ;
> >
> > -ODP_STATIC_ASSERT(sizeof(tick_buf_t) == 16, "sizeof(tick_buf_t) == 16");
> > -
> >  typedef struct odp_timer_s {
> > void *user_ptr;
> > odp_queue_t queue;/* Used for free list when timer is free */
> > @@ -378,34 +360,6 @@ static bool timer_reset(uint32_t idx,
> > tick_buf_t *tb = &tp->tick_buf[idx];
> >
> > if (tmo_buf == NULL || *tmo_buf == ODP_BUFFER_INVALID) {
> > -#ifdef ODP_ATOMIC_U128
> > -   tick_buf_t new, old;
> > -   do {
> > -   /* Relaxed and non-atomic read of current values
> */
> > -   old.exp_tck.v = tb->exp_tck.v;
> > -   old.tmo_buf = tb->tmo_buf;
> > -   TB_SET_PAD(old);
> > -   /* Check if there actually is a timeout buffer
> > -* present */
> > -   if (old.tmo_buf == ODP_BUFFER_INVALID) {
> > -   /* Cannot reset a timer with neither old
> > nor
> > -* new timeout buffer */
> > -   success = false;
> > -   break;
> > -   }
> > -   /* Set up new values */
> > -   new.exp_tck.v = abs_tck;
> > -   new.tmo_buf = old.tmo_buf;
> > -   TB_SET_PAD(new);
> > -   /* Atomic CAS will fail if we experienced torn
> > reads,
> > -* retry update sequence until CAS succeeds */
> > -   } while (!_odp_atomic_u128_cmp_xchg_mm(
> > -   (_odp_atomic_u128_t *)tb,
> > -   (_uint128_t *)&old,
> > -   (_uint128_t *)&new,
> > -   _ODP_MEMMODEL_RLS,
> > -   _ODP_MEMMODEL_RLX));
> > -#else
> >  #ifdef __ARM_ARCH
> > /* Since barriers are not good for C-A15, we take an
> >  * alternative approach using relaxed memory model */
> > @@ -453,7 +407,6 @@ st

Re: [lng-odp] [PATCH v3] doc: process-guide: convert CONTRIBUTING to asciidoc

[Bill Fischofer]

On Mon, May 16, 2016 at 11:29 AM, Mike Holmes 
wrote:

> Converting to asciidoc allows a tidy page to be added to the online
> documentation without cutting and pasting into wordpress.
> Being Asccidoc a tiny amount of clutter is added to show code snippets
> attractively when rendered that make it slightly hard to read as a raw
> document.
>
> Signed-off-by: Mike Holmes 
>

Reviewed-and-tested-by: Bill Fischofer 


> ---
>  CONTRIBUTING  | 110
> --
>  doc/process-guide/.gitignore  |   1 +
>  doc/process-guide/Makefile.am |  11 -
>  3 files changed, 74 insertions(+), 48 deletions(-)
>
> diff --git a/CONTRIBUTING b/CONTRIBUTING
> index f2f8947..a81fd8d 100644
> --- a/CONTRIBUTING
> +++ b/CONTRIBUTING
> @@ -1,28 +1,32 @@
> -Contributing to the OpenDataplane (ODP) API
> 
> -
> -Table of content:
> --
> -1. New Development
> -2. ODP patch expectations as an  open source project
> -3. Common Errors in Patch and Commit Messages
> -4. Documenting the code
> -5. Documenting the user docs
> -6. References
> -
> -1. New Development
> ---
> -ODP code shall be written with the kernel coding style [1].
> +:doctitle: OpenDataPlane (ODP) CONTRIBUTING
> +:description: This document is intended to guide a new application
> developer +
> +in understanding the  contributing requirements for ODP
> +:imagesdir: ../images
> +:toc:
> +:numbered!:
> +[abstract]
> +
> +== Abstract
> +
> +This document is intended to guide a new application developer in
> understanding
> +the  contributing requirements for ODP
> +
> +== New Development
> +
> +ODP code shall be written with the kernel coding style
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle[Kernel
> Coding Style]
> +
>  ODP code shall be documented using the doxygen style described in the
>  "Documenting the code" section.
>  Check patch script/checkpatch.pl shall be used before submitting a patch.
>
> -2. ODP patch expectations as an  open source project
> -
> +== ODP patch expectations as an  open source project
> +
>  While specific to the Linux kernel development, the following reference
> could
> -also be considered a general guide for any Open Source development [2]
> and is
> -relevant to ODP. Many of the guidelines in this ODP document are related
> to the
> -items in that information.
> +also be considered a general guide for any Open Source development
> +
> http://ldn.linuxfoundation.org/book/how-participate-linux-community[Participating
> in the Community]
> +and is relevant to ODP. Many of the guidelines in this ODP document are
> related
> +to the items in that information.
> +
>  Pay particular attention to section 5.3 that talks about patch
> preparation.
>  The key thing to remember is to break up your changes into logical
> sections.
>  Otherwise you run the risk of not being able to even explain the purpose
> of a
> @@ -34,11 +38,12 @@ Signed-off-by: tag line a copy of the description
> follows:
>  Signed-off-by: this is a developer's certification that he or she has
>  the right to submit the patch for inclusion into the [project].  It is
>  an agreement to the Developer's Certificate of Origin, the full text of
> -which can be found in [3] Documentation/SubmittingPatches.
> +which can be found in
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches[Submitting
> Patches]
> +
>  Code without a proper signoff cannot be merged into the mainline.
>
> -3. Common Errors in Patch and Commit Messages
> --
> +== Common Errors in Patch and Commit Messages
> +
>  - Avoid starting the summary line with a capital letter, unless the
> component
>being referred to also begins with a capital letter.
>  - Don't have one huge patch, split your change into logical subparts. It's
> @@ -83,8 +88,12 @@ Code without a proper signoff cannot be merged into the
> mainline.
>sources.
>  - Avoid punctuation in the short log.
>
> -4. Documenting the code
> 
> +== Documenting
> +
> +- References to wikipedia are not permitted.
> +
> +=== Documenting the code
> +
>  - Allow doxygen to use all its default behaviors to identify tagged
>information but where a doxygen tag must be specified use @
>  - The first line is by default the brief summary.
> @@ -93,49 +102,58 @@ Code without a proper signoff cannot be merged into
> the mainline.
>  - Normal comment sections should be before the code block and start with
>/** on its own line and finish with */ on its own line. The exception
>to this rule is a case where the comment is very small, for example
> -  /** macro description */
> -  #define SOME_MACRO 0
> +[source,doxygen]
> +
> +/** macro description */
> +#define SOME_MACRO 0
> +
>  - Commenting on the end 

Re: [lng-odp] [PATCHv4 1/2] linux-generic: use default huge page size

[Bill Fischofer]

For this series:

Reviewed-by: Bill Fischofer 
wrote:

> odp_shm_reserve() relays on huge page size to round up
> requested size. If 1 Gb pages present than parser takes
> it first as first alphabetical file name in sysfs. That
> lead to issue where all allocations wants 1 GB HP. This
> patch takes system default huge pages which are usually 2Mb,
> and has to be enough for existence odp example apps and
> validation tests.
>
> Reported-by: B.Nousilal 
> Signed-off-by: Maxim Uvarov 
> ---
>  platform/linux-generic/odp_system_info.c | 43
> +++-
>  1 file changed, 15 insertions(+), 28 deletions(-)
>
> diff --git a/platform/linux-generic/odp_system_info.c
> b/platform/linux-generic/odp_system_info.c
> index 0f1f3c7..8e95e05 100644
> --- a/platform/linux-generic/odp_system_info.c
> +++ b/platform/linux-generic/odp_system_info.c
> @@ -24,12 +24,9 @@
>  #include 
>  #include 
>
> -
>  #define CACHE_LNSZ_FILE \
> "/sys/devices/system/cpu/cpu0/cache/index0/coherency_line_size"
>
> -#define HUGE_PAGE_DIR "/sys/kernel/mm/hugepages"
> -
>  /*
>   * Report the number of logical CPUs detected at boot time
>   */
> @@ -77,37 +74,27 @@ static int systemcpu_cache_line_size(void)
>  #endif
>
>
> -static int huge_page_size(void)
> +static uint64_t default_huge_page_size(void)
>  {
> -   DIR *dir;
> -   struct dirent *dirent;
> -   int size = 0;
> +   char str[1024];
> +   unsigned long sz;
> +   FILE *file;
>
> -   dir = opendir(HUGE_PAGE_DIR);
> -   if (dir == NULL) {
> -   ODP_ERR("%s not found\n", HUGE_PAGE_DIR);
> -   return 0;
> -   }
> -
> -   while ((dirent = readdir(dir)) != NULL) {
> -   int temp = 0;
> -
> -   if (sscanf(dirent->d_name, "hugepages-%i", &temp) != 1)
> -   continue;
> -
> -   if (temp > size)
> -   size = temp;
> -   }
> +   file = fopen("/proc/meminfo", "rt");
>
> -   if (closedir(dir)) {
> -   ODP_ERR("closedir failed\n");
> -   return 0;
> +   while (fgets(str, sizeof(str), file) != NULL) {
> +   if (sscanf(str, "Hugepagesize:   %8lu kB", &sz) == 1) {
> +   ODP_DBG("defaut hp size is %" PRIu64 " kB\n", sz);
> +   fclose(file);
> +   return (uint64_t)sz * 1024;
> +   }
> }
>
> -   return size * 1024;
> +   ODP_ERR("unable to get default hp size\n");
> +   fclose(file);
> +   return 0;
>  }
>
> -
>  /*
>   * Analysis of /sys/devices/system/cpu/ files
>   */
> @@ -137,7 +124,7 @@ static int systemcpu(odp_system_info_t *sysinfo)
> return -1;
> }
>
> -   sysinfo->huge_page_size = huge_page_size();
> +   sysinfo->huge_page_size = default_huge_page_size();
>
> return 0;
>  }
> --
> 2.7.1.250.gff4ea60
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCHv2] linux-generic: timer: remove dead 128 bit optimizations

[Bill Fischofer]

On Mon, May 16, 2016 at 2:15 PM, Maxim Uvarov 
wrote:

> Remove totally untested branch with 128 bit optimizations
> for timer code.
> This also fixes static assert bug:
> https://bugs.linaro.org/show_bug.cgi?id=2211
>
> Signed-off-by: Maxim Uvarov 
>

Reviewed-and-tested-by: Bill Fischofer 


> ---
>  platform/linux-generic/odp_timer.c | 107
> -
>  1 file changed, 107 deletions(-)
>
> diff --git a/platform/linux-generic/odp_timer.c
> b/platform/linux-generic/odp_timer.c
> index 6b84309..be28da3 100644
> --- a/platform/linux-generic/odp_timer.c
> +++ b/platform/linux-generic/odp_timer.c
> @@ -11,15 +11,7 @@
>   *
>   */
>
> -/* Check if compiler supports 16-byte atomics. GCC needs -mcx16 flag on
> x86 */
> -/* Using spin lock actually seems faster on Core2 */
> -#ifdef ODP_ATOMIC_U128
> -/* TB_NEEDS_PAD defined if sizeof(odp_buffer_t) != 8 */
> -#define TB_NEEDS_PAD
> -#define TB_SET_PAD(x) ((x).pad = 0)
> -#else
>  #define TB_SET_PAD(x) (void)(x)
> -#endif
>
>  #include 
>
> @@ -70,11 +62,9 @@
>   * Mutual exclusion in the absence of CAS16
>
> */
>
> -#ifndef ODP_ATOMIC_U128
>  #define NUM_LOCKS 1024
>  static _odp_atomic_flag_t locks[NUM_LOCKS]; /* Multiple locks per cache
> line! */
>  #define IDX2LOCK(idx) (&locks[(idx) % NUM_LOCKS])
> -#endif
>
>
>  
> /**
>   * Translation between timeout buffer and timeout header
> @@ -98,17 +88,9 @@ static odp_timeout_hdr_t *timeout_hdr(odp_timeout_t tmo)
>  typedef struct tick_buf_s {
> odp_atomic_u64_t exp_tck;/* Expiration tick or TMO_xxx */
> odp_buffer_t tmo_buf;/* ODP_BUFFER_INVALID if timer not active */
> -#ifdef TB_NEEDS_PAD
> -   uint32_t pad;/* Need to be able to access padding for successful
> CAS */
> -#endif
>  } tick_buf_t
> -#ifdef ODP_ATOMIC_U128
> -ODP_ALIGNED(16) /* 16-byte atomic operations need properly aligned
> addresses */
> -#endif
>  ;
>
> -ODP_STATIC_ASSERT(sizeof(tick_buf_t) == 16, "sizeof(tick_buf_t) == 16");
> -
>  typedef struct odp_timer_s {
> void *user_ptr;
> odp_queue_t queue;/* Used for free list when timer is free */
> @@ -378,34 +360,6 @@ static bool timer_reset(uint32_t idx,
> tick_buf_t *tb = &tp->tick_buf[idx];
>
> if (tmo_buf == NULL || *tmo_buf == ODP_BUFFER_INVALID) {
> -#ifdef ODP_ATOMIC_U128
> -   tick_buf_t new, old;
> -   do {
> -   /* Relaxed and non-atomic read of current values */
> -   old.exp_tck.v = tb->exp_tck.v;
> -   old.tmo_buf = tb->tmo_buf;
> -   TB_SET_PAD(old);
> -   /* Check if there actually is a timeout buffer
> -* present */
> -   if (old.tmo_buf == ODP_BUFFER_INVALID) {
> -   /* Cannot reset a timer with neither old
> nor
> -* new timeout buffer */
> -   success = false;
> -   break;
> -   }
> -   /* Set up new values */
> -   new.exp_tck.v = abs_tck;
> -   new.tmo_buf = old.tmo_buf;
> -   TB_SET_PAD(new);
> -   /* Atomic CAS will fail if we experienced torn
> reads,
> -* retry update sequence until CAS succeeds */
> -   } while (!_odp_atomic_u128_cmp_xchg_mm(
> -   (_odp_atomic_u128_t *)tb,
> -   (_uint128_t *)&old,
> -   (_uint128_t *)&new,
> -   _ODP_MEMMODEL_RLS,
> -   _ODP_MEMMODEL_RLX));
> -#else
>  #ifdef __ARM_ARCH
> /* Since barriers are not good for C-A15, we take an
>  * alternative approach using relaxed memory model */
> @@ -453,7 +407,6 @@ static bool timer_reset(uint32_t idx,
> /* Release the lock */
> _odp_atomic_flag_clear(IDX2LOCK(idx));
>  #endif
> -#endif
> } else {
> /* We have a new timeout buffer which replaces any old one
> */
> /* Fill in some (constant) header fields for timeout
> events */
> @@ -468,19 +421,6 @@ static bool timer_reset(uint32_t idx,
> }
> /* Else ignore buffers of other types */
> odp_buffer_t old_buf = ODP_BUFFER_INVALID;
> -#ifdef ODP_ATOMIC_U128
> -   tick_buf_t new, old;
> -   new.exp_tck.v = abs_tck;
> -   new.tmo_buf = *tmo_buf;
> -   TB_SET_PAD(new);
> -   /* We are releasing the new timeout buffer to some other
> -* thread */
> -   _odp_atomic_u128_xchg_mm((_odp_atomic_

Re: [lng-odp] [PATCHv4 1/3] doc: images: add packet flow overview image

[Mike Holmes]

For the series

Reviewed-by: Mike Holmes 

On 12 May 2016 at 17:23, Bill Fischofer  wrote:

> From: Mike Holmes 
>
> Signed-off-by: Mike Holmes 
> Signed-off-by: Bill Fischofer 
> ---
>  doc/images/packet_flow.svg | 532
> +
>  1 file changed, 532 insertions(+)
>  create mode 100644 doc/images/packet_flow.svg
>
> diff --git a/doc/images/packet_flow.svg b/doc/images/packet_flow.svg
> new file mode 100644
> index 000..8d69074
> --- /dev/null
> +++ b/doc/images/packet_flow.svg
> @@ -0,0 +1,532 @@
> +
> +http://xml.openoffice.org/svg/export"; xmlns:dc="
> http://purl.org/dc/elements/1.1/"; xmlns:cc="http://creativecommons.org/ns#";
> xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"; xmlns:svg="
> http://www.w3.org/2000/svg"; xmlns="http://www.w3.org/2000/svg";
> xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd";
> xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape";
> version="1.2" width="471.92218mm" height="181.53999mm" viewBox="0 0
> 47192.219 18154" preserveAspectRatio="xMidYMid" xml:space="preserve"
> id="svg2" inkscape:version="0.91 r13725" sodipodi:docname="packet_flow.svg"
> style="fill-rule:evenodd;stroke-width:28.22200012;stroke-linejoin:round">
> +  
> +
> +  
> +image/svg+xml
> +http://purl.org/dc/dcmitype/StillImage"/>
> +
> +  
> +
> +  
> +   borderopacity="1" objecttolerance="10" gridtolerance="10"
> guidetolerance="10" inkscape:pageopacity="0" inkscape:pageshadow="2"
> inkscape:window-width="737" inkscape:window-height="480" id="namedview1126"
> showgrid="false" fit-margin-top="0" fit-margin-left="0"
> fit-margin-right="0" fit-margin-bottom="0" inkscape:zoom="0.14156002"
> inkscape:cx="881.02951" inkscape:cy="6.6614447" inkscape:window-x="65"
> inkscape:window-y="24" inkscape:window-maximized="0"
> inkscape:current-layer="svg2"/>
> +  
> +
> +  
> +
> +  
> +  
> + horiz-origin-y="0" vert-origin-x="45" vert-origin-y="90" vert-adv-y="90">
> +   units-per-em="2048" font-weight="normal" font-style="normal" ascent="1852"
> descent="423" id="font-face12"/>
> +  
> +  
> +  
> +  
> +  
> +   id="glyph24"/>
> +  
> +  
> +  
> +  
> +  
> +  
> +  
> +   id="glyph40"/>
> +  
> +  
> +  
> +  
> +  
> +   id="glyph52"/>
> +  
> +  
> +  
> +  
> +  
> +   id="glyph64"/>
> +  
> +  
> +  
> +   id="glyph72"/>
> +  
> +  
> +  
> +  
> +  
> +   id="glyph84"/>
> +  
> +  
> +  
> +
> +  
> +  
> +
> +  
> +  
> + transform="scale(4.8828125e-4,-4.8828125e-4)">
> +   inkscape:connector-curvature="0"/>
> +
> + transform="scale(4.8828125e-4,-4.8828125e-4)">
> +   inkscape:connector-curvature="0"/>
> +
> + transform="scale(4.8828125e-4,-4.8828125e-4)">
> +  
> +
> + transform="scale(4.8828125e-4,-4.8828125e-4)">
> +  
> +
> + transform="scale(4.8828125e-4,-4.8828125e-4)">
> +   id="path111" inkscape:connector-curvature="0"/>
> +
> + transform="scale(4.8828125e-4,-4.8828125e-4)">
> +   id="path114" inkscape:connector-curvature="0"/>
> +
> + transform="scale(4.8828125e-4,-4.8828125e-4)">
> +  
> +
> + transform="scale(4.8828125e-4,-4.8828125e-4)">
> +   inkscape:connector-curvature="0"/>
> +
> + transform="scale(4.8828125e-4,-4.8828125e-4)">
> +   inkscape:connector-curvature="0"/>
> +
> +  
> +  
> +  
> +
> +  
> +  
> +
> +  
> +   transform="translate(-4840.889,-3040.0017)">
> +
> +  
> +
> +  
> + width="3561" height="1654" id="rect142"
> style="fill:none;stroke:none"/> style="fill:#b6d7a8;stroke:none"/> inkscape:connector-curvature="0" style="fill:none;stroke:#3465a4"/> d="m 7531,7230 0,0 z" id="path148" inkscape:connector-curvature="0"
> style="fill:none;stroke:#3465a4"/> inkscape:connector-curvature="0" style="fill:none;stroke:#3465a4"/> class="TextShape" id="text152"> font-size="635px" font-weight="400" id="tspan154"
> style="font-weight:400;font-size:635px;font-family:'Liberation Sans',
> sans-serif"> id="tspan156">PKTIO
> API
> +
> +  
> +  
> + width="3561" height="1654" id="rect163"
> style="fill:none;stroke:none"/> style="fill:#b6d7a8;stroke:none"/> inkscape:connector-curvature="0" style="fill:none;stroke:#3465a4"/> d="m 7786,7612 0,0 z" id="path169" inkscape:connector-curvature="0"
> style="fill:none;stroke:#3465a4"/> inkscape:connector-curvature="0" style="fill:none;stroke:#3465a4"/> class="TextShape" id="text173"> font-size="635px" font-weight="400" id="tspan175"
> style="font-weight:400;font-size:635px;font-family:'Liberation Sans',
> sans-serif"> id="tspan177">PKTIO
> API
> +
> +  
> +  
> + width="3561" height="1654" id="rect184"
> style="fill:none;stroke:none"/> style="fill:#b6

Re: [lng-odp] [PATCH v4] doc: user-guide: Improve crypto section.

[Bill Fischofer]

On Mon, May 16, 2016 at 6:00 PM, Nikhil Agarwal 
wrote:

> Signed-off-by: Nikhil Agarwal 
>

Reviewed-and-tested-by: Bill Fischofer 


> ---
>  doc/users-guide/users-guide.adoc | 87
> +++-
>  1 file changed, 69 insertions(+), 18 deletions(-)
>
> diff --git a/doc/users-guide/users-guide.adoc
> b/doc/users-guide/users-guide.adoc
> index 0221634..b094802 100644
> --- a/doc/users-guide/users-guide.adoc
> +++ b/doc/users-guide/users-guide.adoc
> @@ -909,24 +909,75 @@ include::users-guide-pktio.adoc[]
>
>  == Cryptographic services
>
> -ODP provides support for cryptographic operations required by various
> security
> -protocols (e.g. IPSec). To apply a cryptographic operation to a packet a
> session
> -must be created first. Packets processed by a session share the same
> cryptographic
> -parameters like algorithms, keys, initialization vectors. A session is
> created with
> -*odp_crypto_session_create()* call. After session creation a
> cryptographic operation
> -can be applied to a packet using *odp_crypto_operation()* call.
> -Depending on the session type - synchronous or asynchronous the operation
> returns
> -when the operation completed or after the request has been submitted. In
> the
> -asynchronous case an operation completion event will be enqueued on the
> session
> -completion queue. The completion event conveys the status of the
> operation and
> -the result. The application has the responsibility to free the completion
> event.
> -The operation arguments specify for each packet the areas which are to be
> encrypted
> -or decrypted and authenticated. Also, in asynchronous case a context can
> be
> -associated with a given operation and when the operation completion event
> is
> -retrieved the associated context can be retrieved. An operation can be
> executed
> -in-place, when the output packet is the same as the input packet or the
> output
> -packet can be a new packet provided by the application or allocated by the
> -implementation from the session output pool.
> +ODP provides APIs to perform cryptographic operations required by various
> +communication protocols (e.g. IPSec). ODP cryptographic APIs are session
> based.
> +
> +ODP provides APIs for following cryptographic services:
> +
> +* Ciphering
> +* Authentication/data integrity via Keyed-Hashing (HMAC)
> +* Random number generation
> +* Crypto capability inquiries
> +
> +=== Crypto Sessions
> +
> +To apply a cryptographic operation to a packet a session must be created.
> All
> +packets processed by a session share the parameters that define the
> session.
> +
> +ODP supports synchronous and asynchronous crypto sessions. For
> asynchronous
> +sessions, the output of crypto operation is posted in a queue defined as
> +the completion queue in its session parameters.
> +
> +ODP crypto APIs support chained operation sessions in which hashing and
> ciphering
> +both can be achieved using a single session and operation call. The order
> of
> +cipher and hashing can be controlled by the `auth_cipher_text` session
> parameter.
> +
> +Other Session parameters include algorithms, keys, initialization vector
> +(optional), encode or decode, output queue for async mode and output
> packet pool
> +for allocation of an output packet if required.
> +
> +=== Crypto operations
> +
> +After session creation, a cryptographic operation can be applied to a
> packet
> +using the `odp_crypto_operation()` API. Applications may indicate a
> preference
> +for synchronous or asynchronous processing in the session's `pref_mode`
> parameter.
> +However crypto operations may complete synchronously even if an
> asynchronous
> +preference is indicated, and applications must examine the `posted` output
> +parameter from `odp_crypto_operation()` to determine whether the
> operation has
> +completed or if an `ODP_EVENT_CRYPTO_COMPL` notification is expected. In
> the case
> +of an async operation, the `posted` output parameter will be set to true.
> +
> +
> +The operation arguments specify for each packet the areas that are to be
> +encrypted or decrypted and authenticated. Also, there is an option of
> overriding
> +the initialization vector specified in session parameters.
> +
> +An operation can be executed in in-place, out-of-place or new buffer mode.
> +In in-place mode output packet is same as the input packet.
> +In case of out-of-place mode output packet is different from input packet
> as
> +specified by the application, while in new buffer mode implementation
> allocates
> +a new output buffer from the session’s output pool.
> +
> +The application can also specify a context associated with a given
> operation that
> +will be retained during async operation and can be retrieved via the
> completion
> +event.
> +
> +Results of an asynchronous session will be posted as completion events to
> the
> +session’s completion queue, which can be accessed directly or via the ODP
> +scheduler. The completion event contains the status of 

[lng-odp] [PATCH] validation: tm: add checks for adequate cpu resources

[Bill Fischofer]

The shaper and scheduler tests require a minimum of two CPUs to run
reliably. Make these tests conditional and skip them with a warning to
avoid spurious failures if test is run with only a single CPU.

Signed-off-by: Bill Fischofer 
---
 test/validation/traffic_mngr/traffic_mngr.c | 36 +++--
 test/validation/traffic_mngr/traffic_mngr.h |  2 ++
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/test/validation/traffic_mngr/traffic_mngr.c 
b/test/validation/traffic_mngr/traffic_mngr.c
index 305b91a..5ff58f9 100644
--- a/test/validation/traffic_mngr/traffic_mngr.c
+++ b/test/validation/traffic_mngr/traffic_mngr.c
@@ -3652,6 +3652,21 @@ void traffic_mngr_test_tm_create(void)
dump_tm_tree(0);
 }
 
+int traffic_mngr_check_shaper(void)
+{
+   odp_cpumask_t cpumask;
+   int cpucount = odp_cpumask_all_available(&cpumask);
+
+   if (cpucount < 2) {
+   LOG_DBG("\nSkipping shaper test because cpucount = %d "
+   "is less then min number 2 required\n", cpucount);
+   LOG_DBG("Rerun with more cpu resources\n");
+   return ODP_TEST_INACTIVE;
+   }
+
+   return ODP_TEST_ACTIVE;
+}
+
 void traffic_mngr_test_shaper(void)
 {
CU_ASSERT(test_shaper_bw("bw1",   "node_1_1_1", 0, 1   * MBPS) == 0);
@@ -3661,6 +3676,21 @@ void traffic_mngr_test_shaper(void)
CU_ASSERT(test_shaper_bw("bw100", "node_1_1_2", 0, 100 * MBPS) == 0);
 }
 
+int traffic_mngr_check_scheduler(void)
+{
+   odp_cpumask_t cpumask;
+   int cpucount = odp_cpumask_all_available(&cpumask);
+
+   if (cpucount < 2) {
+   LOG_DBG("\nSkipping scheduler test because cpucount = %d "
+   "is less then min number 2 required\n", cpucount);
+   LOG_DBG("Rerun with more cpu resources\n");
+   return ODP_TEST_INACTIVE;
+   }
+
+   return ODP_TEST_ACTIVE;
+}
+
 void traffic_mngr_test_scheduler(void)
 {
CU_ASSERT(test_sched_queue_priority("que_prio", "node_1_1_3", 10) == 0);
@@ -3818,8 +3848,10 @@ odp_testinfo_t traffic_mngr_suite[] = {
ODP_TEST_INFO(traffic_mngr_test_sched_profile),
ODP_TEST_INFO(traffic_mngr_test_threshold_profile),
ODP_TEST_INFO(traffic_mngr_test_wred_profile),
-   ODP_TEST_INFO(traffic_mngr_test_shaper),
-   ODP_TEST_INFO(traffic_mngr_test_scheduler),
+   ODP_TEST_INFO_CONDITIONAL(traffic_mngr_test_shaper,
+ traffic_mngr_check_shaper),
+   ODP_TEST_INFO_CONDITIONAL(traffic_mngr_test_scheduler,
+ traffic_mngr_check_scheduler),
ODP_TEST_INFO(traffic_mngr_test_thresholds),
ODP_TEST_INFO(traffic_mngr_test_byte_wred),
ODP_TEST_INFO(traffic_mngr_test_pkt_wred),
diff --git a/test/validation/traffic_mngr/traffic_mngr.h 
b/test/validation/traffic_mngr/traffic_mngr.h
index 0d50751..bdac221 100644
--- a/test/validation/traffic_mngr/traffic_mngr.h
+++ b/test/validation/traffic_mngr/traffic_mngr.h
@@ -17,7 +17,9 @@ void traffic_mngr_test_sched_profile(void);
 void traffic_mngr_test_threshold_profile(void);
 void traffic_mngr_test_wred_profile(void);
 void traffic_mngr_test_shaper(void);
+int traffic_mngr_check_shaper(void);
 void traffic_mngr_test_scheduler(void);
+int traffic_mngr_check_scheduler(void);
 void traffic_mngr_test_thresholds(void);
 void traffic_mngr_test_byte_wred(void);
 void traffic_mngr_test_pkt_wred(void);
-- 
2.7.4

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCHv2] linux-generic: timer: remove dead 128 bit optimizations

[Maxim Uvarov]

Remove totally untested branch with 128 bit optimizations
for timer code.
This also fixes static assert bug:
https://bugs.linaro.org/show_bug.cgi?id=2211

Signed-off-by: Maxim Uvarov 
---
 platform/linux-generic/odp_timer.c | 107 -
 1 file changed, 107 deletions(-)

diff --git a/platform/linux-generic/odp_timer.c 
b/platform/linux-generic/odp_timer.c
index 6b84309..be28da3 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -11,15 +11,7 @@
  *
  */
 
-/* Check if compiler supports 16-byte atomics. GCC needs -mcx16 flag on x86 */
-/* Using spin lock actually seems faster on Core2 */
-#ifdef ODP_ATOMIC_U128
-/* TB_NEEDS_PAD defined if sizeof(odp_buffer_t) != 8 */
-#define TB_NEEDS_PAD
-#define TB_SET_PAD(x) ((x).pad = 0)
-#else
 #define TB_SET_PAD(x) (void)(x)
-#endif
 
 #include 
 
@@ -70,11 +62,9 @@
  * Mutual exclusion in the absence of CAS16
  */
 
-#ifndef ODP_ATOMIC_U128
 #define NUM_LOCKS 1024
 static _odp_atomic_flag_t locks[NUM_LOCKS]; /* Multiple locks per cache line! 
*/
 #define IDX2LOCK(idx) (&locks[(idx) % NUM_LOCKS])
-#endif
 
 /**
  * Translation between timeout buffer and timeout header
@@ -98,17 +88,9 @@ static odp_timeout_hdr_t *timeout_hdr(odp_timeout_t tmo)
 typedef struct tick_buf_s {
odp_atomic_u64_t exp_tck;/* Expiration tick or TMO_xxx */
odp_buffer_t tmo_buf;/* ODP_BUFFER_INVALID if timer not active */
-#ifdef TB_NEEDS_PAD
-   uint32_t pad;/* Need to be able to access padding for successful CAS */
-#endif
 } tick_buf_t
-#ifdef ODP_ATOMIC_U128
-ODP_ALIGNED(16) /* 16-byte atomic operations need properly aligned addresses */
-#endif
 ;
 
-ODP_STATIC_ASSERT(sizeof(tick_buf_t) == 16, "sizeof(tick_buf_t) == 16");
-
 typedef struct odp_timer_s {
void *user_ptr;
odp_queue_t queue;/* Used for free list when timer is free */
@@ -378,34 +360,6 @@ static bool timer_reset(uint32_t idx,
tick_buf_t *tb = &tp->tick_buf[idx];
 
if (tmo_buf == NULL || *tmo_buf == ODP_BUFFER_INVALID) {
-#ifdef ODP_ATOMIC_U128
-   tick_buf_t new, old;
-   do {
-   /* Relaxed and non-atomic read of current values */
-   old.exp_tck.v = tb->exp_tck.v;
-   old.tmo_buf = tb->tmo_buf;
-   TB_SET_PAD(old);
-   /* Check if there actually is a timeout buffer
-* present */
-   if (old.tmo_buf == ODP_BUFFER_INVALID) {
-   /* Cannot reset a timer with neither old nor
-* new timeout buffer */
-   success = false;
-   break;
-   }
-   /* Set up new values */
-   new.exp_tck.v = abs_tck;
-   new.tmo_buf = old.tmo_buf;
-   TB_SET_PAD(new);
-   /* Atomic CAS will fail if we experienced torn reads,
-* retry update sequence until CAS succeeds */
-   } while (!_odp_atomic_u128_cmp_xchg_mm(
-   (_odp_atomic_u128_t *)tb,
-   (_uint128_t *)&old,
-   (_uint128_t *)&new,
-   _ODP_MEMMODEL_RLS,
-   _ODP_MEMMODEL_RLX));
-#else
 #ifdef __ARM_ARCH
/* Since barriers are not good for C-A15, we take an
 * alternative approach using relaxed memory model */
@@ -453,7 +407,6 @@ static bool timer_reset(uint32_t idx,
/* Release the lock */
_odp_atomic_flag_clear(IDX2LOCK(idx));
 #endif
-#endif
} else {
/* We have a new timeout buffer which replaces any old one */
/* Fill in some (constant) header fields for timeout events */
@@ -468,19 +421,6 @@ static bool timer_reset(uint32_t idx,
}
/* Else ignore buffers of other types */
odp_buffer_t old_buf = ODP_BUFFER_INVALID;
-#ifdef ODP_ATOMIC_U128
-   tick_buf_t new, old;
-   new.exp_tck.v = abs_tck;
-   new.tmo_buf = *tmo_buf;
-   TB_SET_PAD(new);
-   /* We are releasing the new timeout buffer to some other
-* thread */
-   _odp_atomic_u128_xchg_mm((_odp_atomic_u128_t *)tb,
-(_uint128_t *)&new,
-(_uint128_t *)&old,
-_ODP_MEMMODEL_ACQ_RLS);
-   old_buf = old.tmo_buf;
-#else
/* Take a related lock */
while (_odp_atomic_flag_tas(IDX2L

Re: [lng-odp] [PATCH] linux-generic: timer: remove dead 128 bit optimizations

[Maxim Uvarov]

there is one more place for #define NEED_PAD, will send v2

Maxim.

On 05/16/16 22:11, Maxim Uvarov wrote:

Remove totally untested branch with 128 bit optimizations
for timer code.
This also fixes static assert bug:
https://bugs.linaro.org/show_bug.cgi?id=2211

Signed-off-by: Maxim Uvarov 
---
  platform/linux-generic/odp_timer.c | 104 -
  1 file changed, 104 deletions(-)

diff --git a/platform/linux-generic/odp_timer.c 
b/platform/linux-generic/odp_timer.c
index 6b84309..bf2405c 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -11,15 +11,7 @@
   *
   */
  
-/* Check if compiler supports 16-byte atomics. GCC needs -mcx16 flag on x86 */

-/* Using spin lock actually seems faster on Core2 */
-#ifdef ODP_ATOMIC_U128
-/* TB_NEEDS_PAD defined if sizeof(odp_buffer_t) != 8 */
-#define TB_NEEDS_PAD
-#define TB_SET_PAD(x) ((x).pad = 0)
-#else
  #define TB_SET_PAD(x) (void)(x)
-#endif
  
  #include 
  
@@ -70,11 +62,9 @@

   * Mutual exclusion in the absence of CAS16
   
*/
  
-#ifndef ODP_ATOMIC_U128

  #define NUM_LOCKS 1024
  static _odp_atomic_flag_t locks[NUM_LOCKS]; /* Multiple locks per cache line! 
*/
  #define IDX2LOCK(idx) (&locks[(idx) % NUM_LOCKS])
-#endif
  
  /**

   * Translation between timeout buffer and timeout header
@@ -102,13 +92,8 @@ typedef struct tick_buf_s {
uint32_t pad;/* Need to be able to access padding for successful CAS */
  #endif
  } tick_buf_t
-#ifdef ODP_ATOMIC_U128
-ODP_ALIGNED(16) /* 16-byte atomic operations need properly aligned addresses */
-#endif
  ;
  
-ODP_STATIC_ASSERT(sizeof(tick_buf_t) == 16, "sizeof(tick_buf_t) == 16");

-
  typedef struct odp_timer_s {
void *user_ptr;
odp_queue_t queue;/* Used for free list when timer is free */
@@ -378,34 +363,6 @@ static bool timer_reset(uint32_t idx,
tick_buf_t *tb = &tp->tick_buf[idx];
  
  	if (tmo_buf == NULL || *tmo_buf == ODP_BUFFER_INVALID) {

-#ifdef ODP_ATOMIC_U128
-   tick_buf_t new, old;
-   do {
-   /* Relaxed and non-atomic read of current values */
-   old.exp_tck.v = tb->exp_tck.v;
-   old.tmo_buf = tb->tmo_buf;
-   TB_SET_PAD(old);
-   /* Check if there actually is a timeout buffer
-* present */
-   if (old.tmo_buf == ODP_BUFFER_INVALID) {
-   /* Cannot reset a timer with neither old nor
-* new timeout buffer */
-   success = false;
-   break;
-   }
-   /* Set up new values */
-   new.exp_tck.v = abs_tck;
-   new.tmo_buf = old.tmo_buf;
-   TB_SET_PAD(new);
-   /* Atomic CAS will fail if we experienced torn reads,
-* retry update sequence until CAS succeeds */
-   } while (!_odp_atomic_u128_cmp_xchg_mm(
-   (_odp_atomic_u128_t *)tb,
-   (_uint128_t *)&old,
-   (_uint128_t *)&new,
-   _ODP_MEMMODEL_RLS,
-   _ODP_MEMMODEL_RLX));
-#else
  #ifdef __ARM_ARCH
/* Since barriers are not good for C-A15, we take an
 * alternative approach using relaxed memory model */
@@ -453,7 +410,6 @@ static bool timer_reset(uint32_t idx,
/* Release the lock */
_odp_atomic_flag_clear(IDX2LOCK(idx));
  #endif
-#endif
} else {
/* We have a new timeout buffer which replaces any old one */
/* Fill in some (constant) header fields for timeout events */
@@ -468,19 +424,6 @@ static bool timer_reset(uint32_t idx,
}
/* Else ignore buffers of other types */
odp_buffer_t old_buf = ODP_BUFFER_INVALID;
-#ifdef ODP_ATOMIC_U128
-   tick_buf_t new, old;
-   new.exp_tck.v = abs_tck;
-   new.tmo_buf = *tmo_buf;
-   TB_SET_PAD(new);
-   /* We are releasing the new timeout buffer to some other
-* thread */
-   _odp_atomic_u128_xchg_mm((_odp_atomic_u128_t *)tb,
-(_uint128_t *)&new,
-(_uint128_t *)&old,
-_ODP_MEMMODEL_ACQ_RLS);
-   old_buf = old.tmo_buf;
-#else
/* Take a related lock */
while (_odp_atomic_flag_tas(IDX2LOCK(idx)))
/* While lock is taken, spin using relaxed loa

[lng-odp] [PATCH] linux-generic: timer: remove dead 128 bit optimizations

[Maxim Uvarov]

Remove totally untested branch with 128 bit optimizations
for timer code.
This also fixes static assert bug:
https://bugs.linaro.org/show_bug.cgi?id=2211

Signed-off-by: Maxim Uvarov 
---
 platform/linux-generic/odp_timer.c | 104 -
 1 file changed, 104 deletions(-)

diff --git a/platform/linux-generic/odp_timer.c 
b/platform/linux-generic/odp_timer.c
index 6b84309..bf2405c 100644
--- a/platform/linux-generic/odp_timer.c
+++ b/platform/linux-generic/odp_timer.c
@@ -11,15 +11,7 @@
  *
  */
 
-/* Check if compiler supports 16-byte atomics. GCC needs -mcx16 flag on x86 */
-/* Using spin lock actually seems faster on Core2 */
-#ifdef ODP_ATOMIC_U128
-/* TB_NEEDS_PAD defined if sizeof(odp_buffer_t) != 8 */
-#define TB_NEEDS_PAD
-#define TB_SET_PAD(x) ((x).pad = 0)
-#else
 #define TB_SET_PAD(x) (void)(x)
-#endif
 
 #include 
 
@@ -70,11 +62,9 @@
  * Mutual exclusion in the absence of CAS16
  */
 
-#ifndef ODP_ATOMIC_U128
 #define NUM_LOCKS 1024
 static _odp_atomic_flag_t locks[NUM_LOCKS]; /* Multiple locks per cache line! 
*/
 #define IDX2LOCK(idx) (&locks[(idx) % NUM_LOCKS])
-#endif
 
 /**
  * Translation between timeout buffer and timeout header
@@ -102,13 +92,8 @@ typedef struct tick_buf_s {
uint32_t pad;/* Need to be able to access padding for successful CAS */
 #endif
 } tick_buf_t
-#ifdef ODP_ATOMIC_U128
-ODP_ALIGNED(16) /* 16-byte atomic operations need properly aligned addresses */
-#endif
 ;
 
-ODP_STATIC_ASSERT(sizeof(tick_buf_t) == 16, "sizeof(tick_buf_t) == 16");
-
 typedef struct odp_timer_s {
void *user_ptr;
odp_queue_t queue;/* Used for free list when timer is free */
@@ -378,34 +363,6 @@ static bool timer_reset(uint32_t idx,
tick_buf_t *tb = &tp->tick_buf[idx];
 
if (tmo_buf == NULL || *tmo_buf == ODP_BUFFER_INVALID) {
-#ifdef ODP_ATOMIC_U128
-   tick_buf_t new, old;
-   do {
-   /* Relaxed and non-atomic read of current values */
-   old.exp_tck.v = tb->exp_tck.v;
-   old.tmo_buf = tb->tmo_buf;
-   TB_SET_PAD(old);
-   /* Check if there actually is a timeout buffer
-* present */
-   if (old.tmo_buf == ODP_BUFFER_INVALID) {
-   /* Cannot reset a timer with neither old nor
-* new timeout buffer */
-   success = false;
-   break;
-   }
-   /* Set up new values */
-   new.exp_tck.v = abs_tck;
-   new.tmo_buf = old.tmo_buf;
-   TB_SET_PAD(new);
-   /* Atomic CAS will fail if we experienced torn reads,
-* retry update sequence until CAS succeeds */
-   } while (!_odp_atomic_u128_cmp_xchg_mm(
-   (_odp_atomic_u128_t *)tb,
-   (_uint128_t *)&old,
-   (_uint128_t *)&new,
-   _ODP_MEMMODEL_RLS,
-   _ODP_MEMMODEL_RLX));
-#else
 #ifdef __ARM_ARCH
/* Since barriers are not good for C-A15, we take an
 * alternative approach using relaxed memory model */
@@ -453,7 +410,6 @@ static bool timer_reset(uint32_t idx,
/* Release the lock */
_odp_atomic_flag_clear(IDX2LOCK(idx));
 #endif
-#endif
} else {
/* We have a new timeout buffer which replaces any old one */
/* Fill in some (constant) header fields for timeout events */
@@ -468,19 +424,6 @@ static bool timer_reset(uint32_t idx,
}
/* Else ignore buffers of other types */
odp_buffer_t old_buf = ODP_BUFFER_INVALID;
-#ifdef ODP_ATOMIC_U128
-   tick_buf_t new, old;
-   new.exp_tck.v = abs_tck;
-   new.tmo_buf = *tmo_buf;
-   TB_SET_PAD(new);
-   /* We are releasing the new timeout buffer to some other
-* thread */
-   _odp_atomic_u128_xchg_mm((_odp_atomic_u128_t *)tb,
-(_uint128_t *)&new,
-(_uint128_t *)&old,
-_ODP_MEMMODEL_ACQ_RLS);
-   old_buf = old.tmo_buf;
-#else
/* Take a related lock */
while (_odp_atomic_flag_tas(IDX2LOCK(idx)))
/* While lock is taken, spin using relaxed loads */
@@ -496,7 +439,6 @@ static bool timer_reset(uint32_t idx,
 
/* Release the lock */
_odp_atomic_flag

[lng-odp] [Bug 2222] Coverity: uninitialized variables

[bugzilla-daemon]

https://bugs.linaro.org/show_bug.cgi?id=

Maxim Uvarov  changed:

   What|Removed |Added

 Status|CONFIRMED   |RESOLVED
 Resolution|--- |FIXED

--- Comment #2 from Maxim Uvarov  ---
commit 21dd40b6cab39aa359337cca56094eccdefc24c2
Author: Maxim Uvarov 
Date:   Fri May 13 15:44:51 2016 +0300

validation: pktio: initialize pkt_seq

Initialize to zero pkt_seq array first used
in recv_packets_tmo().
https://bugs.linaro.org/show_bug.cgi?id=

Signed-off-by: Maxim Uvarov 
Reviewed-and-tested-by: Matias Elo 

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCHv2] validation: pktio: initialize pkt_seq

[Maxim Uvarov]

Merged,
Maxim.

On 05/13/16 16:12, Elo, Matias (Nokia - FI/Espoo) wrote:

Reviewed-and-tested-by: Matias Elo 


-Original Message-
From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT Maxim
Uvarov
Sent: Friday, May 13, 2016 3:45 PM
To: lng-odp@lists.linaro.org
Subject: [lng-odp] [PATCHv2] validation: pktio: initialize pkt_seq

Initialize to zero pkt_seq array first used
in recv_packets_tmo().
https://bugs.linaro.org/show_bug.cgi?id=

Signed-off-by: Maxim Uvarov 
---
  test/validation/pktio/pktio.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
index a51e0b2..d035a26 100644
--- a/test/validation/pktio/pktio.c
+++ b/test/validation/pktio/pktio.c
@@ -919,9 +919,11 @@ static void test_recv_tmo(recv_tmo_mode_e mode)
ret = odp_pktout_queue(pktio_tx, &pktout_queue, 1);
CU_ASSERT_FATAL(ret > 0);

+   memset(pkt_seq, 0, sizeof(pkt_seq));
+
/* No packets sent yet, so should wait */
ns = 100 * ODP_TIME_MSEC_IN_NS;
-   ret = recv_packets_tmo(pktio_rx, pkt_tbl, pkt_seq, 1, mode,
+   ret = recv_packets_tmo(pktio_rx, &pkt_tbl[0], &pkt_seq[0], 1, mode,
   odp_pktin_wait_time(ns), ns);
CU_ASSERT(ret == 0);

--
2.7.1.250.gff4ea60

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCHv4 2/2] linux-generic: drop odp_ prefix for internal cpuinfo

[Maxim Uvarov]

A little bit code clean up to drop odp_ prefix from internal things
and rename huge_pages to default_huge_pages internal struct.

Signed-off-by: Maxim Uvarov 
---
 platform/linux-generic/arch/default/odp_sysinfo_parse.c |  2 +-
 platform/linux-generic/arch/mips64/odp_sysinfo_parse.c  |  2 +-
 platform/linux-generic/arch/powerpc/odp_sysinfo_parse.c |  2 +-
 platform/linux-generic/arch/x86/odp_sysinfo_parse.c |  2 +-
 platform/linux-generic/include/odp_internal.h   |  8 
 platform/linux-generic/odp_system_info.c| 10 +-
 6 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/platform/linux-generic/arch/default/odp_sysinfo_parse.c 
b/platform/linux-generic/arch/default/odp_sysinfo_parse.c
index 4dcd6d1..53e2aae 100644
--- a/platform/linux-generic/arch/default/odp_sysinfo_parse.c
+++ b/platform/linux-generic/arch/default/odp_sysinfo_parse.c
@@ -8,7 +8,7 @@
 #include 
 #include 
 
-int odp_cpuinfo_parser(FILE *file ODP_UNUSED, odp_system_info_t *sysinfo)
+int cpuinfo_parser(FILE *file ODP_UNUSED, system_info_t *sysinfo)
 {
int i;
 
diff --git a/platform/linux-generic/arch/mips64/odp_sysinfo_parse.c 
b/platform/linux-generic/arch/mips64/odp_sysinfo_parse.c
index d45b420..407264b 100644
--- a/platform/linux-generic/arch/mips64/odp_sysinfo_parse.c
+++ b/platform/linux-generic/arch/mips64/odp_sysinfo_parse.c
@@ -7,7 +7,7 @@
 #include 
 #include 
 
-int odp_cpuinfo_parser(FILE *file, odp_system_info_t *sysinfo)
+int cpuinfo_parser(FILE *file, system_info_t *sysinfo)
 {
char str[1024];
char *pos;
diff --git a/platform/linux-generic/arch/powerpc/odp_sysinfo_parse.c 
b/platform/linux-generic/arch/powerpc/odp_sysinfo_parse.c
index 95200ee..3b88d55 100644
--- a/platform/linux-generic/arch/powerpc/odp_sysinfo_parse.c
+++ b/platform/linux-generic/arch/powerpc/odp_sysinfo_parse.c
@@ -7,7 +7,7 @@
 #include 
 #include 
 
-int odp_cpuinfo_parser(FILE *file, odp_system_info_t *sysinfo)
+int cpuinfo_parser(FILE *file, system_info_t *sysinfo)
 {
char str[1024];
char *pos;
diff --git a/platform/linux-generic/arch/x86/odp_sysinfo_parse.c 
b/platform/linux-generic/arch/x86/odp_sysinfo_parse.c
index c1e05c0..96127ec 100644
--- a/platform/linux-generic/arch/x86/odp_sysinfo_parse.c
+++ b/platform/linux-generic/arch/x86/odp_sysinfo_parse.c
@@ -7,7 +7,7 @@
 #include 
 #include 
 
-int odp_cpuinfo_parser(FILE *file, odp_system_info_t *sysinfo)
+int cpuinfo_parser(FILE *file, system_info_t *sysinfo)
 {
char str[1024];
char *pos;
diff --git a/platform/linux-generic/include/odp_internal.h 
b/platform/linux-generic/include/odp_internal.h
index 28a4fc4..19ab40a 100644
--- a/platform/linux-generic/include/odp_internal.h
+++ b/platform/linux-generic/include/odp_internal.h
@@ -30,19 +30,19 @@ extern __thread int __odp_errno;
 
 typedef struct {
uint64_t cpu_hz_max[MAX_CPU_NUMBER];
-   uint64_t huge_page_size;
+   uint64_t default_huge_page_size;
uint64_t page_size;
int  cache_line_size;
int  cpu_count;
char cpu_arch_str[128];
char model_str[MAX_CPU_NUMBER][128];
-} odp_system_info_t;
+} system_info_t;
 
 struct odp_global_data_s {
pid_t main_pid;
odp_log_func_t log_fn;
odp_abort_func_t abort_fn;
-   odp_system_info_t system_info;
+   system_info_t system_info;
odp_cpumask_t control_cpus;
odp_cpumask_t worker_cpus;
int num_cpus_installed;
@@ -126,7 +126,7 @@ int _odp_int_name_tbl_term_global(void);
 
 void _odp_flush_caches(void);
 
-int odp_cpuinfo_parser(FILE *file, odp_system_info_t *sysinfo);
+int cpuinfo_parser(FILE *file, system_info_t *sysinfo);
 uint64_t odp_cpu_hz_current(int id);
 
 #ifdef __cplusplus
diff --git a/platform/linux-generic/odp_system_info.c 
b/platform/linux-generic/odp_system_info.c
index 8e95e05..fca35ce 100644
--- a/platform/linux-generic/odp_system_info.c
+++ b/platform/linux-generic/odp_system_info.c
@@ -98,7 +98,7 @@ static uint64_t default_huge_page_size(void)
 /*
  * Analysis of /sys/devices/system/cpu/ files
  */
-static int systemcpu(odp_system_info_t *sysinfo)
+static int systemcpu(system_info_t *sysinfo)
 {
int ret;
 
@@ -124,7 +124,7 @@ static int systemcpu(odp_system_info_t *sysinfo)
return -1;
}
 
-   sysinfo->huge_page_size = default_huge_page_size();
+   sysinfo->default_huge_page_size = default_huge_page_size();
 
return 0;
 }
@@ -137,7 +137,7 @@ int odp_system_info_init(void)
 {
FILE  *file;
 
-   memset(&odp_global_data.system_info, 0, sizeof(odp_system_info_t));
+   memset(&odp_global_data.system_info, 0, sizeof(system_info_t));
 
odp_global_data.system_info.page_size = ODP_PAGE_SIZE;
 
@@ -147,7 +147,7 @@ int odp_system_info_init(void)
return -1;
}
 
-   odp_cpuinfo_parser(file, &odp_global_data.system_info);
+   cpuinfo_parser(file, &odp_global_data.system_info);
 
 

[lng-odp] [PATCHv4 1/2] linux-generic: use default huge page size

[Maxim Uvarov]

odp_shm_reserve() relays on huge page size to round up
requested size. If 1 Gb pages present than parser takes
it first as first alphabetical file name in sysfs. That
lead to issue where all allocations wants 1 GB HP. This
patch takes system default huge pages which are usually 2Mb,
and has to be enough for existence odp example apps and
validation tests.

Reported-by: B.Nousilal 
Signed-off-by: Maxim Uvarov 
---
 platform/linux-generic/odp_system_info.c | 43 +++-
 1 file changed, 15 insertions(+), 28 deletions(-)

diff --git a/platform/linux-generic/odp_system_info.c 
b/platform/linux-generic/odp_system_info.c
index 0f1f3c7..8e95e05 100644
--- a/platform/linux-generic/odp_system_info.c
+++ b/platform/linux-generic/odp_system_info.c
@@ -24,12 +24,9 @@
 #include 
 #include 
 
-
 #define CACHE_LNSZ_FILE \
"/sys/devices/system/cpu/cpu0/cache/index0/coherency_line_size"
 
-#define HUGE_PAGE_DIR "/sys/kernel/mm/hugepages"
-
 /*
  * Report the number of logical CPUs detected at boot time
  */
@@ -77,37 +74,27 @@ static int systemcpu_cache_line_size(void)
 #endif
 
 
-static int huge_page_size(void)
+static uint64_t default_huge_page_size(void)
 {
-   DIR *dir;
-   struct dirent *dirent;
-   int size = 0;
+   char str[1024];
+   unsigned long sz;
+   FILE *file;
 
-   dir = opendir(HUGE_PAGE_DIR);
-   if (dir == NULL) {
-   ODP_ERR("%s not found\n", HUGE_PAGE_DIR);
-   return 0;
-   }
-
-   while ((dirent = readdir(dir)) != NULL) {
-   int temp = 0;
-
-   if (sscanf(dirent->d_name, "hugepages-%i", &temp) != 1)
-   continue;
-
-   if (temp > size)
-   size = temp;
-   }
+   file = fopen("/proc/meminfo", "rt");
 
-   if (closedir(dir)) {
-   ODP_ERR("closedir failed\n");
-   return 0;
+   while (fgets(str, sizeof(str), file) != NULL) {
+   if (sscanf(str, "Hugepagesize:   %8lu kB", &sz) == 1) {
+   ODP_DBG("defaut hp size is %" PRIu64 " kB\n", sz);
+   fclose(file);
+   return (uint64_t)sz * 1024;
+   }
}
 
-   return size * 1024;
+   ODP_ERR("unable to get default hp size\n");
+   fclose(file);
+   return 0;
 }
 
-
 /*
  * Analysis of /sys/devices/system/cpu/ files
  */
@@ -137,7 +124,7 @@ static int systemcpu(odp_system_info_t *sysinfo)
return -1;
}
 
-   sysinfo->huge_page_size = huge_page_size();
+   sysinfo->huge_page_size = default_huge_page_size();
 
return 0;
 }
-- 
2.7.1.250.gff4ea60

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCHv4 0/2] linux-generic: use default huge page size

[Maxim Uvarov]

v4: fix 32 build by scanning to unsigned long.
v3: return uint64_t, fix parser and add debug prints.

Maxim Uvarov (2):
  linux-generic: use default huge page size
  linux-generic: drop odp_ prefix for internal cpuinfo

 .../linux-generic/arch/default/odp_sysinfo_parse.c |  2 +-
 .../linux-generic/arch/mips64/odp_sysinfo_parse.c  |  2 +-
 .../linux-generic/arch/powerpc/odp_sysinfo_parse.c |  2 +-
 .../linux-generic/arch/x86/odp_sysinfo_parse.c |  2 +-
 platform/linux-generic/include/odp_internal.h  |  8 ++--
 platform/linux-generic/odp_system_info.c   | 51 --
 6 files changed, 27 insertions(+), 40 deletions(-)

-- 
2.7.1.250.gff4ea60

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCH v4] doc: user-guide: Improve crypto section.

[Nikhil Agarwal]

Signed-off-by: Nikhil Agarwal 
---
 doc/users-guide/users-guide.adoc | 87 +++-
 1 file changed, 69 insertions(+), 18 deletions(-)

diff --git a/doc/users-guide/users-guide.adoc b/doc/users-guide/users-guide.adoc
index 0221634..b094802 100644
--- a/doc/users-guide/users-guide.adoc
+++ b/doc/users-guide/users-guide.adoc
@@ -909,24 +909,75 @@ include::users-guide-pktio.adoc[]
 
 == Cryptographic services
 
-ODP provides support for cryptographic operations required by various security
-protocols (e.g. IPSec). To apply a cryptographic operation to a packet a 
session
-must be created first. Packets processed by a session share the same 
cryptographic
-parameters like algorithms, keys, initialization vectors. A session is created 
with
-*odp_crypto_session_create()* call. After session creation a cryptographic 
operation
-can be applied to a packet using *odp_crypto_operation()* call.
-Depending on the session type - synchronous or asynchronous the operation 
returns
-when the operation completed or after the request has been submitted. In the
-asynchronous case an operation completion event will be enqueued on the session
-completion queue. The completion event conveys the status of the operation and
-the result. The application has the responsibility to free the completion 
event.
-The operation arguments specify for each packet the areas which are to be 
encrypted
-or decrypted and authenticated. Also, in asynchronous case a context can be
-associated with a given operation and when the operation completion event is
-retrieved the associated context can be retrieved. An operation can be executed
-in-place, when the output packet is the same as the input packet or the output
-packet can be a new packet provided by the application or allocated by the
-implementation from the session output pool.
+ODP provides APIs to perform cryptographic operations required by various
+communication protocols (e.g. IPSec). ODP cryptographic APIs are session based.
+
+ODP provides APIs for following cryptographic services:
+
+* Ciphering
+* Authentication/data integrity via Keyed-Hashing (HMAC)
+* Random number generation
+* Crypto capability inquiries
+
+=== Crypto Sessions
+
+To apply a cryptographic operation to a packet a session must be created. All
+packets processed by a session share the parameters that define the session.
+
+ODP supports synchronous and asynchronous crypto sessions. For asynchronous
+sessions, the output of crypto operation is posted in a queue defined as
+the completion queue in its session parameters.
+
+ODP crypto APIs support chained operation sessions in which hashing and 
ciphering
+both can be achieved using a single session and operation call. The order of
+cipher and hashing can be controlled by the `auth_cipher_text` session 
parameter.
+
+Other Session parameters include algorithms, keys, initialization vector
+(optional), encode or decode, output queue for async mode and output packet 
pool
+for allocation of an output packet if required.
+
+=== Crypto operations
+
+After session creation, a cryptographic operation can be applied to a packet
+using the `odp_crypto_operation()` API. Applications may indicate a preference
+for synchronous or asynchronous processing in the session's `pref_mode` 
parameter.
+However crypto operations may complete synchronously even if an asynchronous
+preference is indicated, and applications must examine the `posted` output
+parameter from `odp_crypto_operation()` to determine whether the operation has
+completed or if an `ODP_EVENT_CRYPTO_COMPL` notification is expected. In the 
case
+of an async operation, the `posted` output parameter will be set to true.
+
+
+The operation arguments specify for each packet the areas that are to be
+encrypted or decrypted and authenticated. Also, there is an option of 
overriding
+the initialization vector specified in session parameters.
+
+An operation can be executed in in-place, out-of-place or new buffer mode.
+In in-place mode output packet is same as the input packet.
+In case of out-of-place mode output packet is different from input packet as
+specified by the application, while in new buffer mode implementation allocates
+a new output buffer from the session’s output pool.
+
+The application can also specify a context associated with a given operation 
that
+will be retained during async operation and can be retrieved via the completion
+event.
+
+Results of an asynchronous session will be posted as completion events to the
+session’s completion queue, which can be accessed directly or via the ODP
+scheduler. The completion event contains the status of the operation and the
+result. The application has the responsibility to free the completion event.
+
+=== Random number Generation
+
+ODP provides an API `odp_random_data()` to generate random data bytes. It has
+an argument to specify whether to use system entropy source for random number
+generation or not.
+
+=== Capability i

[lng-odp] [PATCH v3] doc: process-guide: convert CONTRIBUTING to asciidoc

[Mike Holmes]

Converting to asciidoc allows a tidy page to be added to the online
documentation without cutting and pasting into wordpress.
Being Asccidoc a tiny amount of clutter is added to show code snippets
attractively when rendered that make it slightly hard to read as a raw
document.

Signed-off-by: Mike Holmes 
---
 CONTRIBUTING  | 110 --
 doc/process-guide/.gitignore  |   1 +
 doc/process-guide/Makefile.am |  11 -
 3 files changed, 74 insertions(+), 48 deletions(-)

diff --git a/CONTRIBUTING b/CONTRIBUTING
index f2f8947..a81fd8d 100644
--- a/CONTRIBUTING
+++ b/CONTRIBUTING
@@ -1,28 +1,32 @@
-Contributing to the OpenDataplane (ODP) API

-
-Table of content:
--
-1. New Development
-2. ODP patch expectations as an  open source project
-3. Common Errors in Patch and Commit Messages
-4. Documenting the code
-5. Documenting the user docs
-6. References
-
-1. New Development
---
-ODP code shall be written with the kernel coding style [1].
+:doctitle: OpenDataPlane (ODP) CONTRIBUTING
+:description: This document is intended to guide a new application developer +
+in understanding the  contributing requirements for ODP
+:imagesdir: ../images
+:toc:
+:numbered!:
+[abstract]
+
+== Abstract
+
+This document is intended to guide a new application developer in understanding
+the  contributing requirements for ODP
+
+== New Development
+
+ODP code shall be written with the kernel coding style 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle[Kernel
 Coding Style]
+
 ODP code shall be documented using the doxygen style described in the
 "Documenting the code" section.
 Check patch script/checkpatch.pl shall be used before submitting a patch.
 
-2. ODP patch expectations as an  open source project
-
+== ODP patch expectations as an  open source project
+
 While specific to the Linux kernel development, the following reference could
-also be considered a general guide for any Open Source development [2] and is
-relevant to ODP. Many of the guidelines in this ODP document are related to the
-items in that information.
+also be considered a general guide for any Open Source development
+http://ldn.linuxfoundation.org/book/how-participate-linux-community[Participating
 in the Community]
+and is relevant to ODP. Many of the guidelines in this ODP document are related
+to the items in that information.
+
 Pay particular attention to section 5.3 that talks about patch preparation.
 The key thing to remember is to break up your changes into logical sections.
 Otherwise you run the risk of not being able to even explain the purpose of a
@@ -34,11 +38,12 @@ Signed-off-by: tag line a copy of the description follows:
 Signed-off-by: this is a developer's certification that he or she has
 the right to submit the patch for inclusion into the [project].  It is
 an agreement to the Developer's Certificate of Origin, the full text of
-which can be found in [3] Documentation/SubmittingPatches.
+which can be found in 
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches[Submitting
 Patches]
+
 Code without a proper signoff cannot be merged into the mainline.
 
-3. Common Errors in Patch and Commit Messages
--
+== Common Errors in Patch and Commit Messages
+
 - Avoid starting the summary line with a capital letter, unless the component
   being referred to also begins with a capital letter.
 - Don't have one huge patch, split your change into logical subparts. It's
@@ -83,8 +88,12 @@ Code without a proper signoff cannot be merged into the 
mainline.
   sources.
 - Avoid punctuation in the short log.
 
-4. Documenting the code

+== Documenting
+
+- References to wikipedia are not permitted.
+
+=== Documenting the code
+
 - Allow doxygen to use all its default behaviors to identify tagged
   information but where a doxygen tag must be specified use @
 - The first line is by default the brief summary.
@@ -93,49 +102,58 @@ Code without a proper signoff cannot be merged into the 
mainline.
 - Normal comment sections should be before the code block and start with
   /** on its own line and finish with */ on its own line. The exception
   to this rule is a case where the comment is very small, for example
-  /** macro description */
-  #define SOME_MACRO 0
+[source,doxygen]
+
+/** macro description */
+#define SOME_MACRO 0
+
 - Commenting on the end of a line for macros and struct members is allowed
-  using:
-  /**<  */ for example
-  #define SOME_MACRO 0 /**<  */
+  using: /**<  */ for example
+[source,doxygen]
+
+#define SOME_MACRO 0 /**<  */
+
 - Files should start with a files description using @file
 - Functions should specify their parameters with @param[in] and @param[out

Re: [lng-odp] [PATCH v2] doc: process-guide: convert CONTRIBUTING to asciidoc

[Mike Holmes]

On 14 May 2016 at 13:42, Bill Fischofer  wrote:

> Patch still has whitespace errors:
>
> bill@Ubuntu15:~/linaro/mikedoc2$ git am ~/Mail/Incoming/Mike/3
> Applying: doc: process-guide: convert CONTRIBUTING to asciidoc
> .git/rebase-apply/patch:133: indent with spaces.
> `odp_pktio_t`
>

The whole doc uses spaces not tabs, not sure why git am cares about this
line - maybe the necessary backticks ?, it is checkpatch is clear


> .git/rebase-apply/patch:159: indent with spaces.
> image::../.svg[align="center"]
>

again checkpatch is clear on this and the spaces look to be completely
consistent in this document



> .git/rebase-apply/patch:171: new blank line at EOF.
> +
> warning: 3 lines add whitespace errors.
>

I delete all whitespace in v3 and reorganize to accommodate a note on
today's ARCH call decision to not allow wiki references in the
documentation.



>
> It does format correctly, however.
>
> On Fri, May 13, 2016 at 10:55 AM, Mike Holmes 
> wrote:
>
>> Converting to asciidoc allows a tidy page to be added to the online
>> documentation without cutting and pasting into wordpress.
>> Being Asccidoc a tiny amount of clutter is added to show code snippets
>> attractively when rendered that make it slightly hard to read as a raw
>> document.
>>
>> Signed-off-by: Mike Holmes 
>> ---
>> v2
>>   [source,] formatting, ":", improve section on .svg (Bill)
>>
>>  CONTRIBUTING  | 108
>> ++
>>  doc/process-guide/.gitignore  |   1 +
>>  doc/process-guide/Makefile.am |  11 -
>>  3 files changed, 76 insertions(+), 44 deletions(-)
>>
>> diff --git a/CONTRIBUTING b/CONTRIBUTING
>> index f2f8947..a4ed988 100644
>> --- a/CONTRIBUTING
>> +++ b/CONTRIBUTING
>> @@ -1,28 +1,32 @@
>> -Contributing to the OpenDataplane (ODP) API
>> 
>> -
>> -Table of content:
>> --
>> -1. New Development
>> -2. ODP patch expectations as an  open source project
>> -3. Common Errors in Patch and Commit Messages
>> -4. Documenting the code
>> -5. Documenting the user docs
>> -6. References
>> -
>> -1. New Development
>> ---
>> -ODP code shall be written with the kernel coding style [1].
>> +:doctitle: OpenDataPlane (ODP) CONTRIBUTING
>> +:description: This document is intended to guide a new application
>> developer +
>> +in understanding the  contributing requirements for ODP
>> +:imagesdir: ../images
>> +:toc:
>> +:numbered!:
>> +[abstract]
>> +
>> +== Abstract
>> +
>> +This document is intended to guide a new application developer in
>> understanding
>> +the  contributing requirements for ODP
>> +
>> +== New Development
>> +
>> +ODP code shall be written with the kernel coding style
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle[Kernel
>> Coding Style]
>> +
>>  ODP code shall be documented using the doxygen style described in the
>>  "Documenting the code" section.
>>  Check patch script/checkpatch.pl shall be used before submitting a
>> patch.
>>
>> -2. ODP patch expectations as an  open source project
>> -
>> +== ODP patch expectations as an  open source project
>> +
>>  While specific to the Linux kernel development, the following reference
>> could
>> -also be considered a general guide for any Open Source development [2]
>> and is
>> -relevant to ODP. Many of the guidelines in this ODP document are related
>> to the
>> -items in that information.
>> +also be considered a general guide for any Open Source development
>> +
>> http://ldn.linuxfoundation.org/book/how-participate-linux-community[Participating
>> in the Community]
>> +and is relevant to ODP. Many of the guidelines in this ODP document are
>> related
>> +to the items in that information.
>> +
>>  Pay particular attention to section 5.3 that talks about patch
>> preparation.
>>  The key thing to remember is to break up your changes into logical
>> sections.
>>  Otherwise you run the risk of not being able to even explain the purpose
>> of a
>> @@ -34,11 +38,12 @@ Signed-off-by: tag line a copy of the description
>> follows:
>>  Signed-off-by: this is a developer's certification that he or she has
>>  the right to submit the patch for inclusion into the [project].  It is
>>  an agreement to the Developer's Certificate of Origin, the full text of
>> -which can be found in [3] Documentation/SubmittingPatches.
>> +which can be found in
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches[Submitting
>> Patches]
>> +
>>  Code without a proper signoff cannot be merged into the mainline.
>>
>> -3. Common Errors in Patch and Commit Messages
>> --
>> +== Common Errors in Patch and Commit Messages
>> +
>>  - Avoid starting the summary line with a capital letter, unless the
>> component
>>being referred to also begins with a capital letter.

Re: [lng-odp] [PATCHv3 2/2] linux-generic: drop odp_ prefix for internal cpuinfo

[Mike Holmes]

Bill,

Were you able to get the instructions / patch together to do a true 32bit
check in check-odp ?

If we are going to reject things that fail for 32bits we should describe
how to recreate the test if you only have a 64 bit system.

Mike

On 12 May 2016 at 16:21, Bill Fischofer  wrote:

> Compiling this on a 32-bit system I get:
>
>   CC   odp_system_info.lo
> odp_system_info.c: In function ‘default_huge_page_size’:
> odp_system_info.c:86:19: error: format ‘%lu’ expects argument of type
> ‘long unsigned int *’, but argument 3 has type ‘uint64_t * {aka long long
> unsigned int *}’ [-Werror=format=]
>if (sscanf(str, "Hugepagesize: %8lu kB", &sz) == 1) {
>^
> cc1: all warnings being treated as errors
>
>
> On Wed, May 11, 2016 at 8:02 AM, Maxim Uvarov 
> wrote:
>
>> A little bit code clean up to drop odp_ prefix from internal things
>> and rename huge_pages to default_huge_pages internal struct.
>>
>> Signed-off-by: Maxim Uvarov 
>> ---
>>  platform/linux-generic/arch/default/odp_sysinfo_parse.c |  2 +-
>>  platform/linux-generic/arch/mips64/odp_sysinfo_parse.c  |  2 +-
>>  platform/linux-generic/arch/powerpc/odp_sysinfo_parse.c |  2 +-
>>  platform/linux-generic/arch/x86/odp_sysinfo_parse.c |  2 +-
>>  platform/linux-generic/include/odp_internal.h   |  8 
>>  platform/linux-generic/odp_system_info.c| 10 +-
>>  6 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/platform/linux-generic/arch/default/odp_sysinfo_parse.c
>> b/platform/linux-generic/arch/default/odp_sysinfo_parse.c
>> index 4dcd6d1..53e2aae 100644
>> --- a/platform/linux-generic/arch/default/odp_sysinfo_parse.c
>> +++ b/platform/linux-generic/arch/default/odp_sysinfo_parse.c
>> @@ -8,7 +8,7 @@
>>  #include 
>>  #include 
>>
>> -int odp_cpuinfo_parser(FILE *file ODP_UNUSED, odp_system_info_t *sysinfo)
>> +int cpuinfo_parser(FILE *file ODP_UNUSED, system_info_t *sysinfo)
>>  {
>> int i;
>>
>> diff --git a/platform/linux-generic/arch/mips64/odp_sysinfo_parse.c
>> b/platform/linux-generic/arch/mips64/odp_sysinfo_parse.c
>> index d45b420..407264b 100644
>> --- a/platform/linux-generic/arch/mips64/odp_sysinfo_parse.c
>> +++ b/platform/linux-generic/arch/mips64/odp_sysinfo_parse.c
>> @@ -7,7 +7,7 @@
>>  #include 
>>  #include 
>>
>> -int odp_cpuinfo_parser(FILE *file, odp_system_info_t *sysinfo)
>> +int cpuinfo_parser(FILE *file, system_info_t *sysinfo)
>>  {
>> char str[1024];
>> char *pos;
>> diff --git a/platform/linux-generic/arch/powerpc/odp_sysinfo_parse.c
>> b/platform/linux-generic/arch/powerpc/odp_sysinfo_parse.c
>> index 95200ee..3b88d55 100644
>> --- a/platform/linux-generic/arch/powerpc/odp_sysinfo_parse.c
>> +++ b/platform/linux-generic/arch/powerpc/odp_sysinfo_parse.c
>> @@ -7,7 +7,7 @@
>>  #include 
>>  #include 
>>
>> -int odp_cpuinfo_parser(FILE *file, odp_system_info_t *sysinfo)
>> +int cpuinfo_parser(FILE *file, system_info_t *sysinfo)
>>  {
>> char str[1024];
>> char *pos;
>> diff --git a/platform/linux-generic/arch/x86/odp_sysinfo_parse.c
>> b/platform/linux-generic/arch/x86/odp_sysinfo_parse.c
>> index c1e05c0..96127ec 100644
>> --- a/platform/linux-generic/arch/x86/odp_sysinfo_parse.c
>> +++ b/platform/linux-generic/arch/x86/odp_sysinfo_parse.c
>> @@ -7,7 +7,7 @@
>>  #include 
>>  #include 
>>
>> -int odp_cpuinfo_parser(FILE *file, odp_system_info_t *sysinfo)
>> +int cpuinfo_parser(FILE *file, system_info_t *sysinfo)
>>  {
>> char str[1024];
>> char *pos;
>> diff --git a/platform/linux-generic/include/odp_internal.h
>> b/platform/linux-generic/include/odp_internal.h
>> index 28a4fc4..19ab40a 100644
>> --- a/platform/linux-generic/include/odp_internal.h
>> +++ b/platform/linux-generic/include/odp_internal.h
>> @@ -30,19 +30,19 @@ extern __thread int __odp_errno;
>>
>>  typedef struct {
>> uint64_t cpu_hz_max[MAX_CPU_NUMBER];
>> -   uint64_t huge_page_size;
>> +   uint64_t default_huge_page_size;
>> uint64_t page_size;
>> int  cache_line_size;
>> int  cpu_count;
>> char cpu_arch_str[128];
>> char model_str[MAX_CPU_NUMBER][128];
>> -} odp_system_info_t;
>> +} system_info_t;
>>
>>  struct odp_global_data_s {
>> pid_t main_pid;
>> odp_log_func_t log_fn;
>> odp_abort_func_t abort_fn;
>> -   odp_system_info_t system_info;
>> +   system_info_t system_info;
>> odp_cpumask_t control_cpus;
>> odp_cpumask_t worker_cpus;
>> int num_cpus_installed;
>> @@ -126,7 +126,7 @@ int _odp_int_name_tbl_term_global(void);
>>
>>  void _odp_flush_caches(void);
>>
>> -int odp_cpuinfo_parser(FILE *file, odp_system_info_t *sysinfo);
>> +int cpuinfo_parser(FILE *file, system_info_t *sysinfo);
>>  uint64_t odp_cpu_hz_current(int id);
>>
>>  #ifdef __cplusplus
>> diff --git a/platform/linux-generic/odp_system_info.c
>> b/platform/linux-generic/odp_system_info.c
>> index ff422f6..4801b7d 10064

Re: [lng-odp] [PATCHv6 1/2] doc: userguide: add timer/timeout state diagrams

[Maxim Uvarov]

Merged,
Maxim.

On 05/13/16 18:41, Christophe Milard wrote:

For the series:
Reviewed-by: Christophe Milard >


Christophe.


On 13 May 2016 at 17:01, Bill Fischofer > wrote:


Signed-off-by: Bill Fischofer mailto:bill.fischo...@linaro.org>>
Signed-off-by: Christophe Milard mailto:christophe.mil...@linaro.org>>

---
 doc/images/.gitignore   |  2 ++
 doc/images/timeout_fsm.gv   | 28 
 doc/images/timer_fsm.gv | 20 
 doc/users-guide/Makefile.am |  2 ++
 4 files changed, 52 insertions(+)
 create mode 100644 doc/images/timeout_fsm.gv
 create mode 100644 doc/images/timer_fsm.gv

diff --git a/doc/images/.gitignore b/doc/images/.gitignore
index a19aa75..003dbe6 100644
--- a/doc/images/.gitignore
+++ b/doc/images/.gitignore
@@ -1,2 +1,4 @@
 resource_management.svg
 pktio_fsm.svg
+timer_fsm.svg
+timeout_fsm.svg
diff --git a/doc/images/timeout_fsm.gv b/doc/images/timeout_fsm.gv
new file mode 100644
index 000..21ecb59
--- /dev/null
+++ b/doc/images/timeout_fsm.gv
@@ -0,0 +1,28 @@
+digraph timer_state_machine {
+   rankdir=LR;
+   size="12,20";
+   node [fontsize=28];
+   edge [fontsize=28];
+   node [shape=doublecircle]; TO_Unalloc;
+   node [shape=circle]; TO_Alloc TO_Pending TO_Delivered;
+   node [shape=rect]; TO_Enqueued;
+   TO_Unalloc -> TO_Alloc [label="odp_timeout_alloc()"];
+   TO_Alloc -> TO_Unalloc [label="odp_timeout_free()"];
+   TO_Alloc -> TO_Pending [fontcolor=green,
+ label="odp_timer_set_abs()"];
+   TO_Alloc -> TO_Pending [fontcolor=green,
+ label="odp_timer_set_rel()"];
+   TO_Pending -> TO_Alloc [fontcolor=green,
+ label="odp_timer_cancel()"];
+   TO_Pending -> TO_Enqueued [fontcolor=green, label="timer
expires"];
+   TO_Enqueued -> TO_Delivered [label="odp_schedule()"];
+   TO_Delivered -> TO_Pending [fontcolor=green,
+ label="odp_timer_set_abs()"];
+   TO_Delivered -> TO_Pending [fontcolor=green,
+ label="odp_timer_set_rel()"];
+   TO_Delivered -> TO_Delivered
[label="odp_timeout_from_event()"];
+   TO_Delivered -> TO_Delivered [label="odp_timeout_timer()"];
+   TO_Delivered -> TO_Unalloc
+   [label="odp_timeout_free() /
odp_event_free()"];
+
+}
diff --git a/doc/images/timer_fsm.gv b/doc/images/timer_fsm.gv
new file mode 100644
index 000..1798d31
--- /dev/null
+++ b/doc/images/timer_fsm.gv
@@ -0,0 +1,20 @@
+digraph timer_state_machine {
+   rankdir=LR;
+   size="12,20";
+   node [fontsize=28];
+   edge [fontsize=28];
+   node [shape=doublecircle]; Timer_Unalloc;
+   node [shape=circle]; Timer_Alloc Timer_Set Timer_Expired
+   Timer_Unalloc -> Timer_Alloc [label="odp_timer_alloc()"];
+   Timer_Alloc -> Timer_Unalloc [label="odp_timer_free()"];
+   Timer_Alloc -> Timer_Set
[fontcolor=green,label="odp_timer_set_abs()"];
+   Timer_Alloc -> Timer_Set
[fontcolor=green,label="odp_timer_set_rel()"];
+   Timer_Set -> Timer_Alloc
[fontcolor=green,label="odp_timer_cancel()"];
+   Timer_Set -> Timer_Expired [fontcolor=green,label="timer
expires"];
+   Timer_Expired -> Timer_Unalloc [label="odp_timer_free()"];
+   Timer_Expired -> Timer_Set [fontcolor=green,
+ label="odp_timer_set_abs()"];
+   Timer_Expired -> Timer_Set [fontcolor=green,
+ label="odp_timer_set_rel()"];
+
+}
diff --git a/doc/users-guide/Makefile.am b/doc/users-guide/Makefile.am
index 74caa96..2e1195e 100644
--- a/doc/users-guide/Makefile.am
+++ b/doc/users-guide/Makefile.am
@@ -30,6 +30,8 @@ IMAGES = $(top_srcdir)/doc/images/overview.svg \
 $(top_srcdir)/doc/images/release_git.svg \
 $(top_srcdir)/doc/images/segment.svg \
 $(top_srcdir)/doc/images/simple_release_git.svg \
+$(top_srcdir)/doc/images/timeout_fsm.svg \
+$(top_srcdir)/doc/images/timer_fsm.svg \
 $(top_srcdir)/doc/images/tm_hierarchy.svg \
 $(top_srcdir)/doc/images/tm_node.svg

--
2.7.4




___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [Bug 2227] classification_main: Conditional jump or move depends on uninitialised value

[bugzilla-daemon]

https://bugs.linaro.org/show_bug.cgi?id=2227

Bala Manoharan  changed:

   What|Removed |Added

 Status|IN_PROGRESS |RESOLVED
 Resolution|--- |FIXED

--- Comment #2 from Bala Manoharan  ---
commit 8387987f8c568f77d662aa8a79fc753c3d1a3dfe
Author: Balasubramanian Manoharan 
Date:   Fri May 13 17:21:51 2016 +0530

linux-generic: classification:fix uninitialized pmr param value

Fix memory leaked caused by uninitialized pmr param struct

Fixes: https://bugs.linaro.org/show_bug.cgi?id=2227

Signed-off-by: Balasubramanian Manoharan 
Reviewed-by: Bill Fischofer 
Signed-off-by: Maxim Uvarov 

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] validation: time: remove print and add verbose faults

[Ivan Khoronzhuk]

On 16.05.16 17:18, Maxim Uvarov wrote:

On 05/16/16 17:00, Ivan Khoronzhuk wrote:

Hi Maxim,

I know it's a little late, but printf was here not for testing wait until.
It was here to expose visually time for a second. As ODP doesn't have
other accurate enough time source the printf was added to visually demonstrate
1 second. It allows to see if at least frequency of counter was chosen 
correctly,
as it's most frequent error while time API implementation. It can be easily 
broken
even later while simple setting of some clock divider to wrong value, usually in
two times more or less. That's enough to see with human eye. I think this 
ability
is more needed then saving of some 0.1 second.



In my understanding it saves time between start() and end() measurements which
can be result of not passing border check.


The border check is was done with CU_ASSERT. Was it not enough?

I think that if test hangs for some long

time you anyway need to add some debug prints or stop it with gdb and see where
exactly it was stopped.


The idea not simply print every second...that's indicator that test is running 
also,
but also to demonstrate sens of 1 second. The *_until APIs are verified with 
odp_time APIs
checked previously, that is, one part of time API is checked with another part 
of time API ))...
and print real second while doing thisin case of error in both parts, you 
will never
catch an error with validation test w/o sense of time or another source. 
Another source is absent,
so only way - print itat least it allows to see some part of possible 
errors.

Maybe do add separate test which prints/verify odp_time and

linux system time?


not sure, ODP can run w/o linux, so validation test should be independent.
And adding new test only increases time of testing, so better to do it
in same cycle.



Maxim.


On 12.05.16 14:22, Maxim Uvarov wrote:

Having system call inside loop couse unpredictable delay
as the result wrong time diff calculation. Just removing
print make hole test execution shorter on 0.1 seconds
according to cunit stats. Also make verbose errors in
places where we out of limit in cunit. This should be
very helpful to understand DELAY_TOLERANCE value suitable
for all platforms.

Signed-off-by: Maxim Uvarov 
---
  test/validation/time/time.c | 35 +++
  1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/test/validation/time/time.c b/test/validation/time/time.c
index da456ea..f7f3d14 100644
--- a/test/validation/time/time.c
+++ b/test/validation/time/time.c
@@ -331,7 +331,6 @@ static void time_test_wait_until(time_cb time, 
time_from_ns_cb time_from_ns)
  for (i = 0; i < WAIT_SECONDS; i++) {
  wait = odp_time_sum(wait, second);
  odp_time_wait_until(wait);
-printf("%d..", i + 1);
  }
  end_time = time();

@@ -341,8 +340,19 @@ static void time_test_wait_until(time_cb time, 
time_from_ns_cb time_from_ns)
  upper_limit = time_from_ns(WAIT_SECONDS * ODP_TIME_SEC_IN_NS +
 DELAY_TOLERANCE);

-CU_ASSERT(odp_time_cmp(wait, lower_limit) >= 0);
-CU_ASSERT(odp_time_cmp(wait, upper_limit) <= 0);
+if (odp_time_cmp(wait, lower_limit) < 0) {
+fprintf(stderr, "Exceed lower limit: "
+"wait is %" PRIu64 ", lower_limit %" PRIu64 "\n",
+odp_time_to_ns(wait), odp_time_to_ns(lower_limit));
+CU_FAIL("Exceed lower limit\n");
+}
+
+if (odp_time_cmp(wait, upper_limit) > 0) {
+fprintf(stderr, "Exceed upper limit: "
+"wait is %" PRIu64 ", upper_limit %" PRIu64 "\n",
+odp_time_to_ns(wait), odp_time_to_ns(lower_limit));
+CU_FAIL("Exceed upper limit\n");
+}
  }

  void time_test_local_wait_until(void)
@@ -362,10 +372,8 @@ void time_test_wait_ns(void)
  odp_time_t start_time, end_time, diff;

  start_time = odp_time_local();
-for (i = 0; i < WAIT_SECONDS; i++) {
+for (i = 0; i < WAIT_SECONDS; i++)
  odp_time_wait_ns(ODP_TIME_SEC_IN_NS);
-printf("%d..", i + 1);
-}
  end_time = odp_time_local();

  diff = odp_time_diff(end_time, start_time);
@@ -375,8 +383,19 @@ void time_test_wait_ns(void)
  upper_limit = odp_time_local_from_ns(WAIT_SECONDS * ODP_TIME_SEC_IN_NS +
   DELAY_TOLERANCE);

-CU_ASSERT(odp_time_cmp(diff, lower_limit) >= 0);
-CU_ASSERT(odp_time_cmp(diff, upper_limit) <= 0);
+if (odp_time_cmp(diff, lower_limit) < 0) {
+fprintf(stderr, "Exceed lower limit: "
+"diff is %" PRIu64 ", lower_limit %" PRIu64 "\n",
+odp_time_to_ns(diff), odp_time_to_ns(lower_limit));
+CU_FAIL("Exceed lower limit\n");
+}
+
+if (odp_time_cmp(diff, upper_limit) > 0) {
+fprintf(stderr, "Exceed upper limit: "
+"diff is %" PRIu64 ", upper_limit %" PRIu64 "\n",
+odp_time_to_ns(diff), odp_time_to_ns(lower_limit));
+CU_FAIL("Exceed upper limit\n");
+}
  }

  static void 

Re: [lng-odp] [PATCH] validation: time: remove print and add verbose faults

[Maxim Uvarov]

On 05/16/16 17:00, Ivan Khoronzhuk wrote:

Hi Maxim,

I know it's a little late, but printf was here not for testing wait 
until.

It was here to expose visually time for a second. As ODP doesn't have
other accurate enough time source the printf was added to visually 
demonstrate
1 second. It allows to see if at least frequency of counter was chosen 
correctly,
as it's most frequent error while time API implementation. It can be 
easily broken
even later while simple setting of some clock divider to wrong value, 
usually in
two times more or less. That's enough to see with human eye. I think 
this ability

is more needed then saving of some 0.1 second.



In my understanding it saves time between start() and end() measurements 
which
can be result of not passing border check. I think that if test hangs 
for some long
time you anyway need to add some debug prints or stop it with gdb and 
see where
exactly it was stopped. Maybe do add separate test which prints/verify 
odp_time and

linux system time?

Maxim.


On 12.05.16 14:22, Maxim Uvarov wrote:

Having system call inside loop couse unpredictable delay
as the result wrong time diff calculation. Just removing
print make hole test execution shorter on 0.1 seconds
according to cunit stats. Also make verbose errors in
places where we out of limit in cunit. This should be
very helpful to understand DELAY_TOLERANCE value suitable
for all platforms.

Signed-off-by: Maxim Uvarov 
---
  test/validation/time/time.c | 35 +++
  1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/test/validation/time/time.c b/test/validation/time/time.c
index da456ea..f7f3d14 100644
--- a/test/validation/time/time.c
+++ b/test/validation/time/time.c
@@ -331,7 +331,6 @@ static void time_test_wait_until(time_cb time, 
time_from_ns_cb time_from_ns)

  for (i = 0; i < WAIT_SECONDS; i++) {
  wait = odp_time_sum(wait, second);
  odp_time_wait_until(wait);
-printf("%d..", i + 1);
  }
  end_time = time();

@@ -341,8 +340,19 @@ static void time_test_wait_until(time_cb time, 
time_from_ns_cb time_from_ns)

  upper_limit = time_from_ns(WAIT_SECONDS * ODP_TIME_SEC_IN_NS +
 DELAY_TOLERANCE);

-CU_ASSERT(odp_time_cmp(wait, lower_limit) >= 0);
-CU_ASSERT(odp_time_cmp(wait, upper_limit) <= 0);
+if (odp_time_cmp(wait, lower_limit) < 0) {
+fprintf(stderr, "Exceed lower limit: "
+"wait is %" PRIu64 ", lower_limit %" PRIu64 "\n",
+odp_time_to_ns(wait), odp_time_to_ns(lower_limit));
+CU_FAIL("Exceed lower limit\n");
+}
+
+if (odp_time_cmp(wait, upper_limit) > 0) {
+fprintf(stderr, "Exceed upper limit: "
+"wait is %" PRIu64 ", upper_limit %" PRIu64 "\n",
+odp_time_to_ns(wait), odp_time_to_ns(lower_limit));
+CU_FAIL("Exceed upper limit\n");
+}
  }

  void time_test_local_wait_until(void)
@@ -362,10 +372,8 @@ void time_test_wait_ns(void)
  odp_time_t start_time, end_time, diff;

  start_time = odp_time_local();
-for (i = 0; i < WAIT_SECONDS; i++) {
+for (i = 0; i < WAIT_SECONDS; i++)
  odp_time_wait_ns(ODP_TIME_SEC_IN_NS);
-printf("%d..", i + 1);
-}
  end_time = odp_time_local();

  diff = odp_time_diff(end_time, start_time);
@@ -375,8 +383,19 @@ void time_test_wait_ns(void)
  upper_limit = odp_time_local_from_ns(WAIT_SECONDS * 
ODP_TIME_SEC_IN_NS +

   DELAY_TOLERANCE);

-CU_ASSERT(odp_time_cmp(diff, lower_limit) >= 0);
-CU_ASSERT(odp_time_cmp(diff, upper_limit) <= 0);
+if (odp_time_cmp(diff, lower_limit) < 0) {
+fprintf(stderr, "Exceed lower limit: "
+"diff is %" PRIu64 ", lower_limit %" PRIu64 "\n",
+odp_time_to_ns(diff), odp_time_to_ns(lower_limit));
+CU_FAIL("Exceed lower limit\n");
+}
+
+if (odp_time_cmp(diff, upper_limit) > 0) {
+fprintf(stderr, "Exceed upper limit: "
+"diff is %" PRIu64 ", upper_limit %" PRIu64 "\n",
+odp_time_to_ns(diff), odp_time_to_ns(lower_limit));
+CU_FAIL("Exceed upper limit\n");
+}
  }

  static void time_test_to_u64(time_cb time)





___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH 2/2] time: fix invalid casting on a 32-bit host

[Ivan Khoronzhuk]

Hi Matias,

The odp_time_local and others functions are time sensitive functions,
that's why it was decided to avoid copping as more as possible.

The timespec is not simple "long type". Its type is arch dependent but is 
always 64bit.
In case of 32 bit system it's defined as long long.
The same for odp_time_t struct. So, at least for now it seems to be the same for
both 32 and 64 bit systems. And I think Bill Fischofer knew about this while 
adding this
,at first glance, strange union, right Bill?
Yes, it's not the best decision from style point of view, but it's fast and in 
case of an error
is supposed to be caught by time validation tests.

On 04.05.16 16:01, Matias Elo wrote:

The size of 'struct timespec' may vary on different host
architectures as it includes type long members. This breaks
time functions on a 32-bit x86 host. Fix this by
individually copying struct timespec members to odp_time_t.

Signed-off-by: Matias Elo 
---
  platform/linux-generic/odp_time.c | 26 +++---
  1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/platform/linux-generic/odp_time.c 
b/platform/linux-generic/odp_time.c
index 040f754..81e0522 100644
--- a/platform/linux-generic/odp_time.c
+++ b/platform/linux-generic/odp_time.c
@@ -11,11 +11,6 @@
  #include 
  #include 

-typedef union {
-   odp_time_t  ex;
-   struct timespec in;
-} _odp_time_t;
-
  static odp_time_t start_time;

  static inline
@@ -47,13 +42,17 @@ static inline odp_time_t time_diff(odp_time_t t2, 
odp_time_t t1)
  static inline odp_time_t time_local(void)
  {
int ret;
-   _odp_time_t time;
+   odp_time_t time;
+   struct timespec sys_time;

-   ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in);
+   ret = clock_gettime(CLOCK_MONOTONIC_RAW, &sys_time);
if (odp_unlikely(ret != 0))
ODP_ABORT("clock_gettime failed\n");

-   return time_diff(time.ex, start_time);
+   time.tv_sec = sys_time.tv_sec;
+   time.tv_nsec = sys_time.tv_nsec;
+
+   return time_diff(time, start_time);
  }

  static inline int time_cmp(odp_time_t t2, odp_time_t t1)
@@ -195,10 +194,15 @@ uint64_t odp_time_to_u64(odp_time_t time)
  int odp_time_init_global(void)
  {
int ret;
-   _odp_time_t time;
+   struct timespec time;

-   ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in);
-   start_time = ret ? ODP_TIME_NULL : time.ex;
+   ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time);
+   if (ret) {
+   start_time = ODP_TIME_NULL;
+   } else {
+   start_time.tv_sec = time.tv_sec;
+   start_time.tv_nsec = time.tv_nsec;
+   }

return ret;
  }



--
Regards,
Ivan Khoronzhuk
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH v2 5/5] linux-generic: packet: initialize only selected odp_packet_hdr_t members

[Mike Holmes]

On 16 May 2016 at 05:50, Matias Elo  wrote:

> Using memset to initialize struct odp_packet_hdr_t contents
> to 0 has a significant overhead. Instead, initialize only
> the required members of the struct.
>
> Signed-off-by: Matias Elo 
> ---
>  platform/linux-generic/include/odp_packet_internal.h | 13 +
>  platform/linux-generic/odp_packet.c  | 18
> ++
>  2 files changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/platform/linux-generic/include/odp_packet_internal.h
> b/platform/linux-generic/include/odp_packet_internal.h
> index 1306b05..c87bc9f 100644
> --- a/platform/linux-generic/include/odp_packet_internal.h
> +++ b/platform/linux-generic/include/odp_packet_internal.h
> @@ -4,7 +4,6 @@
>   * SPDX-License-Identifier: BSD-3-Clause
>   */
>
-
>  /**
>   * @file
>   *
> @@ -129,11 +128,16 @@ ODP_STATIC_ASSERT(sizeof(output_flags_t) ==
> sizeof(uint32_t),
>
>  /**
>   * Internal Packet header
> + *
> + * To optimize fast path performance this struct is not initialized to
> zero in
> + * packet_init(). Because of this any new fields added must be reviewed
> for
> + * initialization requirements.
>   */


This file will not contribute to the generated API documentation and I
think it should. This platform specific warning should appear there as a
doxygen  @warning
Perhaps we need to start a trend of adding a doxygen platform specific
module called "PLATFORM SPECIFICS", then we can add information with the
@addtogroup platform_specifics and users across platforms will know where
to look for this sort of information.

 typedef struct {
> /* common buffer header */
> odp_buffer_hdr_t buf_hdr;
>
> +   /* Following members are initialized by packet_init() */
> input_flags_t  input_flags;
> error_flags_t  error_flags;
> output_flags_t output_flags;
> @@ -142,15 +146,16 @@ typedef struct {
> uint32_t l3_offset; /**< offset to L3 hdr, e.g. IPv4, IPv6 */
> uint32_t l4_offset; /**< offset to L4 hdr (TCP, UDP, SCTP, also
> ICMP) */
>
> -   uint32_t l3_len; /**< Layer 3 length */
> -   uint32_t l4_len; /**< Layer 4 length */
> -
> uint32_t frame_len;
> uint32_t headroom;
> uint32_t tailroom;
>
> odp_pktio_t input;
>
> +   /* Members below are not initialized by packet_init() */
> +   uint32_t l3_len; /**< Layer 3 length */
> +   uint32_t l4_len; /**< Layer 4 length */
> +
> uint32_t flow_hash;  /**< Flow hash value */
> odp_time_t timestamp;/**< Timestamp value */
>
> diff --git a/platform/linux-generic/odp_packet.c
> b/platform/linux-generic/odp_packet.c
> index 4f523c9..8dde404 100644
> --- a/platform/linux-generic/odp_packet.c
> +++ b/platform/linux-generic/odp_packet.c
> @@ -49,19 +49,11 @@ void packet_parse_reset(odp_packet_hdr_t *pkt_hdr)
>  static void packet_init(pool_entry_t *pool, odp_packet_hdr_t *pkt_hdr,
> size_t size, int parse)
>  {
> -   /*
> -   * Reset parser metadata.  Note that we clear via memset to make
> -   * this routine indepenent of any additional adds to packet
> metadata.
> -   */
> -   const size_t start_offset = ODP_FIELD_SIZEOF(odp_packet_hdr_t,
> buf_hdr);
> -   uint8_t *start;
> -   size_t len;
> +   pkt_hdr->input_flags.all  = 0;
> +   pkt_hdr->output_flags.all = 0;
> +   pkt_hdr->error_flags.all  = 0;
>
> -   start = (uint8_t *)pkt_hdr + start_offset;
> -   len = sizeof(odp_packet_hdr_t) - start_offset;
> -   memset(start, 0, len);
> -
> -   /* Set metadata items that initialize to non-zero values */
> +   pkt_hdr->l2_offset = 0;
> pkt_hdr->l3_offset = ODP_PACKET_OFFSET_INVALID;
> pkt_hdr->l4_offset = ODP_PACKET_OFFSET_INVALID;
>
> @@ -79,6 +71,8 @@ static void packet_init(pool_entry_t *pool,
> odp_packet_hdr_t *pkt_hdr,
> pkt_hdr->tailroom  =
> (pool->s.seg_size * pkt_hdr->buf_hdr.segcount) -
> (pool->s.headroom + size);
> +
> +   pkt_hdr->input = ODP_PKTIO_INVALID;
>  }
>
>  odp_packet_t packet_alloc(odp_pool_t pool_hdl, uint32_t len, int parse)
> --
> 1.9.1
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>



-- 
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 mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] Test to list of a html message

[Mike Holmes]

The box has been ticked as per the arch call, filtering has been turn on
with defaults.
Mailman should now convert text/html parts to plain text This conversion
happens after MIME attachments have been stripped.



On 16 May 2016 at 10:27, Bill Fischofer  wrote:

> The HTML comes through just fine and I notice it's been auto-converted to
> plain text on the archives.
>
> On Mon, May 16, 2016 at 8:46 AM, Raj Murali  wrote:
>
>> reply to test
>>
>>
>> Raj Murali
>> Director - Linaro Networking Group
>> Skype: rajmurali_s  │ M: +91 98450 70135
>>
>> Linaro.org  *│ *Open source software for ARM SoCs
>>
>>
>> On 16 May 2016 at 19:13, Mike Holmes  wrote:
>>
>> > reply test
>> >
>> > On 16 May 2016 at 09:42, Mike Holmes  wrote:
>> >
>> > >
>> > >
>> > > --
>> > > Mike Holmes
>> > > Technical Manager - Linaro Networking Group
>> > > Linaro.org  *│ *Open source software for ARM
>> > SoCs
>> > > "Work should be fun and collaborative, the rest follows"
>> > >
>> > >
>> > >
>> >
>> >
>> > --
>> > 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 mailing list
>> > lng-odp@lists.linaro.org
>> > https://lists.linaro.org/mailman/listinfo/lng-odp
>> >
>> ___
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>


-- 
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 mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] Test to list of a html message

[Bill Fischofer]

The HTML comes through just fine and I notice it's been auto-converted to
plain text on the archives.

On Mon, May 16, 2016 at 8:46 AM, Raj Murali  wrote:

> reply to test
>
>
> Raj Murali
> Director - Linaro Networking Group
> Skype: rajmurali_s  │ M: +91 98450 70135
>
> Linaro.org  *│ *Open source software for ARM SoCs
>
>
> On 16 May 2016 at 19:13, Mike Holmes  wrote:
>
> > reply test
> >
> > On 16 May 2016 at 09:42, Mike Holmes  wrote:
> >
> > >
> > >
> > > --
> > > Mike Holmes
> > > Technical Manager - Linaro Networking Group
> > > Linaro.org  *│ *Open source software for ARM
> > SoCs
> > > "Work should be fun and collaborative, the rest follows"
> > >
> > >
> > >
> >
> >
> > --
> > 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 mailing list
> > lng-odp@lists.linaro.org
> > https://lists.linaro.org/mailman/listinfo/lng-odp
> >
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] fix: classification: uninitialized pmr param value

[Maxim Uvarov]

On 05/14/16 20:36, Bill Fischofer wrote:
short log should be linux-generic: classification: etc. Can be fixed 
during merge?

yes, fixed and merged,
Maxim.




On Fri, May 13, 2016 at 6:51 AM, Balasubramanian Manoharan 
mailto:bala.manoha...@linaro.org>> wrote:


Fix memory leaked caused by uninitialized pmr param struct

Fixes: https://bugs.linaro.org/show_bug.cgi?id=2227

Signed-off-by: Balasubramanian Manoharan
mailto:bala.manoha...@linaro.org>>


Reviewed-by: Bill Fischofer >


---
 test/validation/classification/odp_classification_test_pmr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git
a/test/validation/classification/odp_classification_test_pmr.c
b/test/validation/classification/odp_classification_test_pmr.c
index 344503a..7c7d07e 100644
--- a/test/validation/classification/odp_classification_test_pmr.c
+++ b/test/validation/classification/odp_classification_test_pmr.c
@@ -250,6 +250,7 @@ void classification_test_pmr_term_tcp_sport(void)
cos = odp_cls_cos_create(cosname, &cls_param);
CU_ASSERT_FATAL(cos != ODP_COS_INVALID);

+   odp_cls_pmr_param_init(&pmr_param);
pmr_param.term = ODP_PMR_TCP_SPORT;
pmr_param.match.value = &val;
pmr_param.match.mask = &mask;
@@ -474,6 +475,7 @@ void classification_test_pmr_term_udp_sport(void)
cos = odp_cls_cos_create(cosname, &cls_param);
CU_ASSERT_FATAL(cos != ODP_COS_INVALID);

+   odp_cls_pmr_param_init(&pmr_param);
pmr_param.term = ODP_PMR_UDP_SPORT;
pmr_param.match.value = &val;
pmr_param.match.mask = &mask;
@@ -690,6 +692,7 @@ void classification_test_pmr_term_dmac(void)
cos = odp_cls_cos_create(cosname, &cls_param);
CU_ASSERT_FATAL(cos != ODP_COS_INVALID);

+   odp_cls_pmr_param_init(&pmr_param);
pmr_param.term = ODP_PMR_DMAC;
pmr_param.match.value = &val;
pmr_param.match.mask = &mask;
--
1.9.1

___
lng-odp mailing list
lng-odp@lists.linaro.org 
https://lists.linaro.org/mailman/listinfo/lng-odp




___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] validation: time: remove print and add verbose faults

[Ivan Khoronzhuk]

Hi Maxim,

I know it's a little late, but printf was here not for testing wait until.
It was here to expose visually time for a second. As ODP doesn't have
other accurate enough time source the printf was added to visually demonstrate
1 second. It allows to see if at least frequency of counter was chosen 
correctly,
as it's most frequent error while time API implementation. It can be easily 
broken
even later while simple setting of some clock divider to wrong value, usually in
two times more or less. That's enough to see with human eye. I think this 
ability
is more needed then saving of some 0.1 second.

On 12.05.16 14:22, Maxim Uvarov wrote:

Having system call inside loop couse unpredictable delay
as the result wrong time diff calculation. Just removing
print make hole test execution shorter on 0.1 seconds
according to cunit stats. Also make verbose errors in
places where we out of limit in cunit. This should be
very helpful to understand DELAY_TOLERANCE value suitable
for all platforms.

Signed-off-by: Maxim Uvarov 
---
  test/validation/time/time.c | 35 +++
  1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/test/validation/time/time.c b/test/validation/time/time.c
index da456ea..f7f3d14 100644
--- a/test/validation/time/time.c
+++ b/test/validation/time/time.c
@@ -331,7 +331,6 @@ static void time_test_wait_until(time_cb time, 
time_from_ns_cb time_from_ns)
for (i = 0; i < WAIT_SECONDS; i++) {
wait = odp_time_sum(wait, second);
odp_time_wait_until(wait);
-   printf("%d..", i + 1);
}
end_time = time();

@@ -341,8 +340,19 @@ static void time_test_wait_until(time_cb time, 
time_from_ns_cb time_from_ns)
upper_limit = time_from_ns(WAIT_SECONDS * ODP_TIME_SEC_IN_NS +
   DELAY_TOLERANCE);

-   CU_ASSERT(odp_time_cmp(wait, lower_limit) >= 0);
-   CU_ASSERT(odp_time_cmp(wait, upper_limit) <= 0);
+   if (odp_time_cmp(wait, lower_limit) < 0) {
+   fprintf(stderr, "Exceed lower limit: "
+   "wait is %" PRIu64 ", lower_limit %" PRIu64 "\n",
+   odp_time_to_ns(wait), odp_time_to_ns(lower_limit));
+   CU_FAIL("Exceed lower limit\n");
+   }
+
+   if (odp_time_cmp(wait, upper_limit) > 0) {
+   fprintf(stderr, "Exceed upper limit: "
+   "wait is %" PRIu64 ", upper_limit %" PRIu64 "\n",
+   odp_time_to_ns(wait), odp_time_to_ns(lower_limit));
+   CU_FAIL("Exceed upper limit\n");
+   }
  }

  void time_test_local_wait_until(void)
@@ -362,10 +372,8 @@ void time_test_wait_ns(void)
odp_time_t start_time, end_time, diff;

start_time = odp_time_local();
-   for (i = 0; i < WAIT_SECONDS; i++) {
+   for (i = 0; i < WAIT_SECONDS; i++)
odp_time_wait_ns(ODP_TIME_SEC_IN_NS);
-   printf("%d..", i + 1);
-   }
end_time = odp_time_local();

diff = odp_time_diff(end_time, start_time);
@@ -375,8 +383,19 @@ void time_test_wait_ns(void)
upper_limit = odp_time_local_from_ns(WAIT_SECONDS * ODP_TIME_SEC_IN_NS +
 DELAY_TOLERANCE);

-   CU_ASSERT(odp_time_cmp(diff, lower_limit) >= 0);
-   CU_ASSERT(odp_time_cmp(diff, upper_limit) <= 0);
+   if (odp_time_cmp(diff, lower_limit) < 0) {
+   fprintf(stderr, "Exceed lower limit: "
+   "diff is %" PRIu64 ", lower_limit %" PRIu64 "\n",
+   odp_time_to_ns(diff), odp_time_to_ns(lower_limit));
+   CU_FAIL("Exceed lower limit\n");
+   }
+
+   if (odp_time_cmp(diff, upper_limit) > 0) {
+   fprintf(stderr, "Exceed upper limit: "
+   "diff is %" PRIu64 ", upper_limit %" PRIu64 "\n",
+   odp_time_to_ns(diff), odp_time_to_ns(lower_limit));
+   CU_FAIL("Exceed upper limit\n");
+   }
  }

  static void time_test_to_u64(time_cb time)



--
Regards,
Ivan Khoronzhuk
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] validation: timer: verify timer successfully allocated

[Maxim Uvarov]

Merged,
Maxim.

On 05/16/16 15:40, Mike Holmes wrote:



On 11 May 2016 at 20:51, Bill Fischofer > wrote:


Resolve bug https://bugs.linaro.org/show_bug.cgi?id=2031 by
adding a check to ensure that at least one timer is allocated before
potentially dividing by zero.

Signed-off-by: Bill Fischofer mailto:bill.fischo...@linaro.org>>


Reviewed-by: Mike Holmes >


---
 test/validation/timer/timer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/validation/timer/timer.c
b/test/validation/timer/timer.c
index aad11aa..9d637d0 100644
--- a/test/validation/timer/timer.c
+++ b/test/validation/timer/timer.c
@@ -304,6 +304,8 @@ static void *worker_entrypoint(void *arg
TEST_UNUSED)
tt[i].tick = TICK_INVALID;
}
allocated = i;
+   if (allocated == 0)
+   CU_FAIL_FATAL("unable to alloc a timer");
odp_atomic_fetch_add_u32(&timers_allocated, allocated);

odp_barrier_wait(&test_barrier);
--
2.7.4

___
lng-odp mailing list
lng-odp@lists.linaro.org 
https://lists.linaro.org/mailman/listinfo/lng-odp




--
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 mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [Bug 2031] CID 157958: Integer handling issues: timer.c

[bugzilla-daemon]

https://bugs.linaro.org/show_bug.cgi?id=2031

Maxim Uvarov  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|IN_PROGRESS |RESOLVED

--- Comment #4 from Maxim Uvarov  ---
commit acca52168e42778498e637b278c7c3538dea7fbc
Author: Bill Fischofer 
Date:   Wed May 11 19:51:18 2016 -0500

validation: timer: verify timer successfully allocated

Resolve bug https://bugs.linaro.org/show_bug.cgi?id=2031 by
adding a check to ensure that at least one timer is allocated before
potentially dividing by zero.

Signed-off-by: Bill Fischofer 
Reviewed-by: Mike Holmes 
Signed-off-by: Maxim Uvarov 

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] Test to list of a html message

[Raj Murali]

reply to test


Raj Murali
Director - Linaro Networking Group
Skype: rajmurali_s  │ M: +91 98450 70135

Linaro.org  *│ *Open source software for ARM SoCs


On 16 May 2016 at 19:13, Mike Holmes  wrote:

> reply test
>
> On 16 May 2016 at 09:42, Mike Holmes  wrote:
>
> >
> >
> > --
> > Mike Holmes
> > Technical Manager - Linaro Networking Group
> > Linaro.org  *│ *Open source software for ARM
> SoCs
> > "Work should be fun and collaborative, the rest follows"
> >
> >
> >
>
>
> --
> 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 mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] Test to list of a html message

[Mike Holmes]

reply test

On 16 May 2016 at 09:42, Mike Holmes  wrote:

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


-- 
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 mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] Test to list of a html message

[Mike Holmes]

-- 
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 mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] Fedora repo changes

[Anders Roxell]

On 11 May 2016 at 21:53, Matt Spencer  wrote:
> Hi Guys
>
> Thought I would use some down time to have a play with ODP.  On a vanilla
> Fedora 23 Server install, there is an issue with the current odp.repo in
> that the default behaviour of ‘def’ is to gpg check the downloads.  So,
> following the instructions on the site fails on ‘dnf update’ as it reads the
> odp repo data.
>
> Can I make two suggestions please?
>
> 1, add the following to odp.repo
> enabled=1
> gpgcheck=0

Thank you, will look into that.

>
> 2, change the instructions from using ‘yum update’ to ‘dnf update’
>
> Also, there is no indication on the site that the RPM’s are arrch64 only.
> Should we be providing x86 binaries too?

This is something that we are working on already.

Cheers,
Anders
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] linux-generic: dpdk: use memset() to initialize dev_info

[Maxim Uvarov]

Merged,
Maxim.

On 05/15/16 00:41, Bill Fischofer wrote:



On Fri, May 13, 2016 at 6:40 AM, Matias Elo > wrote:


Clang build fails due to a missing field in initializer
error. This is a known clang bug
(https://llvm.org/bugs/show_bug.cgi?id=21689). Circumvent
this by using memset() instead.

Signed-off-by: Matias Elo mailto:matias@nokia.com>>


Reviewed-by: Bill Fischofer >


---
 platform/linux-generic/pktio/dpdk.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/platform/linux-generic/pktio/dpdk.c
b/platform/linux-generic/pktio/dpdk.c
index e6af8fb..831dc26 100644
--- a/platform/linux-generic/pktio/dpdk.c
+++ b/platform/linux-generic/pktio/dpdk.c
@@ -177,11 +177,13 @@ static int dpdk_netdev_is_valid(const char *s)

 static uint32_t dpdk_vdev_mtu_get(uint8_t port_id)
 {
-   struct rte_eth_dev_info dev_info = {0};
+   struct rte_eth_dev_info dev_info;
struct ifreq ifr;
int sockfd;
uint32_t mtu;

+   memset(&dev_info, 0, sizeof(struct rte_eth_dev_info));
+
rte_eth_dev_info_get(port_id, &dev_info);
if_indextoname(dev_info.if_index, ifr.ifr_name);

@@ -219,11 +221,13 @@ static uint32_t dpdk_mtu_get(pktio_entry_t
*pktio_entry)

 static int dpdk_vdev_promisc_mode_get(uint8_t port_id)
 {
-   struct rte_eth_dev_info dev_info = {0};
+   struct rte_eth_dev_info dev_info;
struct ifreq ifr;
int sockfd;
int mode;

+   memset(&dev_info, 0, sizeof(struct rte_eth_dev_info));
+
rte_eth_dev_info_get(port_id, &dev_info);
if_indextoname(dev_info.if_index, ifr.ifr_name);

@@ -240,11 +244,13 @@ static int
dpdk_vdev_promisc_mode_get(uint8_t port_id)

 static int dpdk_vdev_promisc_mode_set(uint8_t port_id, int enable)
 {
-   struct rte_eth_dev_info dev_info = {0};
+   struct rte_eth_dev_info dev_info;
struct ifreq ifr;
int sockfd;
int mode;

+   memset(&dev_info, 0, sizeof(struct rte_eth_dev_info));
+
rte_eth_dev_info_get(port_id, &dev_info);
if_indextoname(dev_info.if_index, ifr.ifr_name);

--
1.9.1

___
lng-odp mailing list
lng-odp@lists.linaro.org 
https://lists.linaro.org/mailman/listinfo/lng-odp




___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH v2 0/5] pktio fast path optimization

[Bill Fischofer]

For this series:

Reviewed-and-tested-by: Bill Fischofer 

On Mon, May 16, 2016 at 4:50 AM, Matias Elo  wrote:

> V2:
> + Document odp_packet_hdr_t field initialization (Bill Fischover, Mike
> Holmes)
> + Remove unused payload_offset field from odp_packet_hdr_t
>
> This patch set improves pktio fast path throughput by:
> - Remove unnecessary members from odp_packet_hdr_t and odp_buffer_hdr_t to
>   reduce memory usage (less cache misses).
> - Don't use memset in packet_init() to zero odp_packet_hdr_t contents.
> Instead,
>   initialize required values individually. This is less safe but provides a
>   significant performance gain.
>
> Matias Elo (5):
>   linux-generic: packet: remove l3_protocol and l4_protocol members from
> odp_packet_hdr_t
>   linux-generic: packet: remove vlan_s_tag and vlan_c_tag members from
> odp_packet_hdr_t
>   linux-generic: packet: remove payload_offset member from
> odp_packet_hdr_t
>   linux-generic: buffer: ifdef ipc_addr_offset member from
> odp_buffer_hdr_t
>   linux-generic: packet: initialize only selected odp_packet_hdr_t
> members
>
>  .../linux-generic/include/odp_buffer_internal.h|  8 ++--
>  .../include/odp_classification_inlines.h   |  7 +--
>  .../linux-generic/include/odp_packet_internal.h| 29 ++--
>  platform/linux-generic/odp_packet.c| 51
> --
>  platform/linux-generic/pktio/ipc.c | 13 ++
>  5 files changed, 51 insertions(+), 57 deletions(-)
>
> --
> 1.9.1
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] validation: timer: verify timer successfully allocated

[Mike Holmes]

On 11 May 2016 at 20:51, Bill Fischofer  wrote:

> Resolve bug https://bugs.linaro.org/show_bug.cgi?id=2031 by
> adding a check to ensure that at least one timer is allocated before
> potentially dividing by zero.
>
> Signed-off-by: Bill Fischofer 
>

Reviewed-by: Mike Holmes 


> ---
>  test/validation/timer/timer.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/test/validation/timer/timer.c b/test/validation/timer/timer.c
> index aad11aa..9d637d0 100644
> --- a/test/validation/timer/timer.c
> +++ b/test/validation/timer/timer.c
> @@ -304,6 +304,8 @@ static void *worker_entrypoint(void *arg TEST_UNUSED)
> tt[i].tick = TICK_INVALID;
> }
> allocated = i;
> +   if (allocated == 0)
> +   CU_FAIL_FATAL("unable to alloc a timer");
> odp_atomic_fetch_add_u32(&timers_allocated, allocated);
>
> odp_barrier_wait(&test_barrier);
> --
> 2.7.4
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>



-- 
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 mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] validation: timer: verify timer successfully allocated

[Bill Fischofer]

On Mon, May 16, 2016 at 7:33 AM, Mike Holmes  wrote:

>
> On 11 May 2016 at 20:51, Bill Fischofer  wrote:
>
>> Resolve bug https://bugs.linaro.org/show_bug.cgi?id=2031 by
>> adding a check to ensure that at least one timer is allocated before
>> potentially dividing by zero.
>>
>> Signed-off-by: Bill Fischofer 
>> ---
>>  test/validation/timer/timer.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/test/validation/timer/timer.c b/test/validation/timer/timer.c
>> index aad11aa..9d637d0 100644
>> --- a/test/validation/timer/timer.c
>> +++ b/test/validation/timer/timer.c
>> @@ -304,6 +304,8 @@ static void *worker_entrypoint(void *arg TEST_UNUSED)
>> tt[i].tick = TICK_INVALID;
>> }
>>
>
> Why is it that the allocating loop above here calls break and exits the
> loop forcing us to catch a failure outside, rather than calling
> CU_FAIL_FATAL directly ?
>

Because no individual allocation failure is in itself fatal. The fatal
condition is if all fail, which is why the check is outside the loop.


>
>
>
>> allocated = i;
>> +   if (allocated == 0)
>> +   CU_FAIL_FATAL("unable to alloc a timer");
>> odp_atomic_fetch_add_u32(&timers_allocated, allocated);
>>
>> odp_barrier_wait(&test_barrier);
>> --
>> 2.7.4
>>
>> ___
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
>
>
> --
> 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 mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] validation: timer: verify timer successfully allocated

[Mike Holmes]

On 11 May 2016 at 20:51, Bill Fischofer  wrote:

> Resolve bug https://bugs.linaro.org/show_bug.cgi?id=2031 by
> adding a check to ensure that at least one timer is allocated before
> potentially dividing by zero.
>
> Signed-off-by: Bill Fischofer 
> ---
>  test/validation/timer/timer.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/test/validation/timer/timer.c b/test/validation/timer/timer.c
> index aad11aa..9d637d0 100644
> --- a/test/validation/timer/timer.c
> +++ b/test/validation/timer/timer.c
> @@ -304,6 +304,8 @@ static void *worker_entrypoint(void *arg TEST_UNUSED)
> tt[i].tick = TICK_INVALID;
> }
>

Why is it that the allocating loop above here calls break and exits the
loop forcing us to catch a failure outside, rather than calling
CU_FAIL_FATAL directly ?



> allocated = i;
> +   if (allocated == 0)
> +   CU_FAIL_FATAL("unable to alloc a timer");
> odp_atomic_fetch_add_u32(&timers_allocated, allocated);
>
> odp_barrier_wait(&test_barrier);
> --
> 2.7.4
>
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>



-- 
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 mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] lng-odp mailman settings

[Anders Roxell]

On 2016-05-16 08:27, Savolainen, Petri (Nokia - FI/Espoo) wrote:
> 
> 
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Bill 
> Fischofer
> Sent: Saturday, May 14, 2016 1:55 AM
> To: Brian Brooks 
> Cc: lng-odp 
> Subject: Re: [lng-odp] lng-odp mailman settings
> 
> 
> 
> On Friday, May 13, 2016, Brian Brooks 
> mailto:brian.bro...@linaro.org>> wrote:
> On 05/13 10:07:44, Bill Fischofer wrote:
> > I don't think we need to impose those sort of restrictions. I personally
> > use sylpheed for patches and GMail for everything else and they have no
> > problem sorting things out.
> >
> > The ODP mailing list covers both discussions as well as patches, and HTML
> > is useful for the former. The onus should be on those who have problems
> > with this to upgrade their tools rather than sending everyone else back to
> > the 1980s.
> >
> > On Fri, May 13, 2016 at 9:57 AM, Mike Holmes 
> > > wrote:
> >
> > > All,
> > >
> > > A topic that comes up occasionally that affects some mail tools like
> > > outlook is that html email on this list makes it harder for some folks to
> > > process patches.
> > >
> > > I use mutt specifically to avoid using a fancy email client for all my
> > > patch download/send work, it is powered by coal/steam and would work fine
> > > on a 1970 VAX but there you go.
> > >
> > > Anyway, does anyone object to having mailman reject HTML mail ? One up
> > > side is that I will not waste time blocking offers of cheap watches and 
> > > fax
> > > services from the list :)
> > >
> > > On the other hand I will miss bullet lists and the color red when I am
> > > grumpy.
> > >
> > > Mike
> 
> +1 for plain text
> 
> I think this topic is more about simplicity and best practices rather than
> anything else.
> 
> Here is an example of why HTML doesn't work well with archives:
> https://lists.linaro.org/pipermail/lng-odp/2016-May/023134.html
> That link displays perfectly fine for me.  What problem do you see?
> 
> Did you read it? It’s a mixture of paragraphs from two writers, without any 
> indication who is writing what. The list archive drops out HTML decoration 
> and at least in some cases will not convert (various blue or grey lines, or 
> indentations) into ‘>’.
> 
> As Brian highlights, it’s about simplicity. With one rule (no HTML) we can 
> uniform output of all mail clients and their fonts/indentations/other 
> settings, so that the list is clean and clear to read (both directly and from 
> the archives) and reply (no need to struggle with mismatching styles 
> inherited from the senders HTML settings, etc).
> 
> +1 for plain text

I agree with Brian and Petri.
There shouldn't be any other option than plain text!
That works for discussions as well. "You can mail all those funny mails
to your friends." =)

Just a reflection, since we talking about plain text vs. html... Is
there someone that can follow this thread easily? =)

We have another problem with this thread as well, people top-post,
which is insane! =)

Cheers,
Anders
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCH v2 5/5] linux-generic: packet: initialize only selected odp_packet_hdr_t members

[Matias Elo]

Using memset to initialize struct odp_packet_hdr_t contents
to 0 has a significant overhead. Instead, initialize only
the required members of the struct.

Signed-off-by: Matias Elo 
---
 platform/linux-generic/include/odp_packet_internal.h | 13 +
 platform/linux-generic/odp_packet.c  | 18 ++
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/platform/linux-generic/include/odp_packet_internal.h 
b/platform/linux-generic/include/odp_packet_internal.h
index 1306b05..c87bc9f 100644
--- a/platform/linux-generic/include/odp_packet_internal.h
+++ b/platform/linux-generic/include/odp_packet_internal.h
@@ -4,7 +4,6 @@
  * SPDX-License-Identifier: BSD-3-Clause
  */
 
-
 /**
  * @file
  *
@@ -129,11 +128,16 @@ ODP_STATIC_ASSERT(sizeof(output_flags_t) == 
sizeof(uint32_t),
 
 /**
  * Internal Packet header
+ *
+ * To optimize fast path performance this struct is not initialized to zero in
+ * packet_init(). Because of this any new fields added must be reviewed for
+ * initialization requirements.
  */
 typedef struct {
/* common buffer header */
odp_buffer_hdr_t buf_hdr;
 
+   /* Following members are initialized by packet_init() */
input_flags_t  input_flags;
error_flags_t  error_flags;
output_flags_t output_flags;
@@ -142,15 +146,16 @@ typedef struct {
uint32_t l3_offset; /**< offset to L3 hdr, e.g. IPv4, IPv6 */
uint32_t l4_offset; /**< offset to L4 hdr (TCP, UDP, SCTP, also ICMP) */
 
-   uint32_t l3_len; /**< Layer 3 length */
-   uint32_t l4_len; /**< Layer 4 length */
-
uint32_t frame_len;
uint32_t headroom;
uint32_t tailroom;
 
odp_pktio_t input;
 
+   /* Members below are not initialized by packet_init() */
+   uint32_t l3_len; /**< Layer 3 length */
+   uint32_t l4_len; /**< Layer 4 length */
+
uint32_t flow_hash;  /**< Flow hash value */
odp_time_t timestamp;/**< Timestamp value */
 
diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 4f523c9..8dde404 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -49,19 +49,11 @@ void packet_parse_reset(odp_packet_hdr_t *pkt_hdr)
 static void packet_init(pool_entry_t *pool, odp_packet_hdr_t *pkt_hdr,
size_t size, int parse)
 {
-   /*
-   * Reset parser metadata.  Note that we clear via memset to make
-   * this routine indepenent of any additional adds to packet metadata.
-   */
-   const size_t start_offset = ODP_FIELD_SIZEOF(odp_packet_hdr_t, buf_hdr);
-   uint8_t *start;
-   size_t len;
+   pkt_hdr->input_flags.all  = 0;
+   pkt_hdr->output_flags.all = 0;
+   pkt_hdr->error_flags.all  = 0;
 
-   start = (uint8_t *)pkt_hdr + start_offset;
-   len = sizeof(odp_packet_hdr_t) - start_offset;
-   memset(start, 0, len);
-
-   /* Set metadata items that initialize to non-zero values */
+   pkt_hdr->l2_offset = 0;
pkt_hdr->l3_offset = ODP_PACKET_OFFSET_INVALID;
pkt_hdr->l4_offset = ODP_PACKET_OFFSET_INVALID;
 
@@ -79,6 +71,8 @@ static void packet_init(pool_entry_t *pool, odp_packet_hdr_t 
*pkt_hdr,
pkt_hdr->tailroom  =
(pool->s.seg_size * pkt_hdr->buf_hdr.segcount) -
(pool->s.headroom + size);
+
+   pkt_hdr->input = ODP_PKTIO_INVALID;
 }
 
 odp_packet_t packet_alloc(odp_pool_t pool_hdl, uint32_t len, int parse)
-- 
1.9.1

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCH v2 4/5] linux-generic: buffer: ifdef ipc_addr_offset member from odp_buffer_hdr_t

[Matias Elo]

Define ipc_addr_offset member of struct odp_buffer_hdr_t
only if ipc pktio is enabled to reduce struct size.

Signed-off-by: Matias Elo 
---
 platform/linux-generic/include/odp_buffer_internal.h |  8 +---
 platform/linux-generic/pktio/ipc.c   | 13 +
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/platform/linux-generic/include/odp_buffer_internal.h 
b/platform/linux-generic/include/odp_buffer_internal.h
index 727abb4..47f41c6 100644
--- a/platform/linux-generic/include/odp_buffer_internal.h
+++ b/platform/linux-generic/include/odp_buffer_internal.h
@@ -132,9 +132,6 @@ struct odp_buffer_hdr_t {
uint32_t uarea_size; /* size of user area */
uint32_t segcount;   /* segment count */
uint32_t segsize;/* segment size */
-   /* ipc mapped process can not walk over pointers,
-* offset has to be used */
-   uint64_t ipc_addr_offset[ODP_BUFFER_MAX_SEG];
void*addr[ODP_BUFFER_MAX_SEG]; /* block addrs */
uint64_t order;  /* sequence for ordered queues */
queue_entry_t   *origin_qe;  /* ordered queue origin */
@@ -142,6 +139,11 @@ struct odp_buffer_hdr_t {
queue_entry_t   *target_qe;  /* ordered queue target */
uint64_t sync[ODP_CONFIG_MAX_ORDERED_LOCKS_PER_QUEUE];
};
+#ifdef _ODP_PKTIO_IPC
+   /* ipc mapped process can not walk over pointers,
+* offset has to be used */
+   uint64_t ipc_addr_offset[ODP_BUFFER_MAX_SEG];
+#endif
 };
 
 /** @internal Compile time assert that the
diff --git a/platform/linux-generic/pktio/ipc.c 
b/platform/linux-generic/pktio/ipc.c
index 12cc286..f1cd21c 100644
--- a/platform/linux-generic/pktio/ipc.c
+++ b/platform/linux-generic/pktio/ipc.c
@@ -436,8 +436,15 @@ static inline void *_ipc_buffer_map(odp_buffer_hdr_t *buf,
 {
int seg_index  = offset / buf->segsize;
int seg_offset = offset % buf->segsize;
+#ifdef _ODP_PKTIO_IPC
void *addr = (char *)buf - buf->ipc_addr_offset[seg_index];
+#else
+   /** buf_hdr.ipc_addr_offset defined only when ipc is
+*  enabled. */
+   void *addr = NULL;
 
+   (void)seg_index;
+#endif
if (seglen) {
uint32_t buf_left = limit - offset;
*seglen = seg_offset + buf_left <= buf->segsize ?
@@ -631,8 +638,14 @@ static int ipc_pktio_send(pktio_entry_t *pktio_entry,
 * convert it to offset
 */
for (j = 0; j < ODP_BUFFER_MAX_SEG; j++) {
+#ifdef _ODP_PKTIO_IPC
pkt_hdr->buf_hdr.ipc_addr_offset[j] = (char *)pkt_hdr -
(char *)pkt_hdr->buf_hdr.addr[j];
+#else
+   /** buf_hdr.ipc_addr_offset defined only when ipc is
+*  enabled. */
+   (void)pkt_hdr;
+#endif
}
}
 
-- 
1.9.1

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCH v2 3/5] linux-generic: packet: remove payload_offset member from odp_packet_hdr_t

[Matias Elo]

There is no way to read payload_offset in the ODP API, so
don't save it.

Signed-off-by: Matias Elo 
---
 platform/linux-generic/include/odp_packet_internal.h |  2 --
 platform/linux-generic/odp_packet.c  | 20 ++--
 2 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/platform/linux-generic/include/odp_packet_internal.h 
b/platform/linux-generic/include/odp_packet_internal.h
index 2a12503..1306b05 100644
--- a/platform/linux-generic/include/odp_packet_internal.h
+++ b/platform/linux-generic/include/odp_packet_internal.h
@@ -141,7 +141,6 @@ typedef struct {
uint32_t l2_offset; /**< offset to L2 hdr, e.g. Eth */
uint32_t l3_offset; /**< offset to L3 hdr, e.g. IPv4, IPv6 */
uint32_t l4_offset; /**< offset to L4 hdr (TCP, UDP, SCTP, also ICMP) */
-   uint32_t payload_offset; /**< offset to payload */
 
uint32_t l3_len; /**< Layer 3 length */
uint32_t l4_len; /**< Layer 4 length */
@@ -180,7 +179,6 @@ static inline void 
copy_packet_parser_metadata(odp_packet_hdr_t *src_hdr,
dst_hdr->l2_offset  = src_hdr->l2_offset;
dst_hdr->l3_offset  = src_hdr->l3_offset;
dst_hdr->l4_offset  = src_hdr->l4_offset;
-   dst_hdr->payload_offset = src_hdr->payload_offset;
 
dst_hdr->l3_len = src_hdr->l3_len;
dst_hdr->l4_len = src_hdr->l4_len;
diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 436265e..4f523c9 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -41,7 +41,6 @@ void packet_parse_reset(odp_packet_hdr_t *pkt_hdr)
pkt_hdr->l2_offset= 0;
pkt_hdr->l3_offset= ODP_PACKET_OFFSET_INVALID;
pkt_hdr->l4_offset= ODP_PACKET_OFFSET_INVALID;
-   pkt_hdr->payload_offset   = ODP_PACKET_OFFSET_INVALID;
 }
 
 /**
@@ -65,7 +64,6 @@ static void packet_init(pool_entry_t *pool, odp_packet_hdr_t 
*pkt_hdr,
/* Set metadata items that initialize to non-zero values */
pkt_hdr->l3_offset = ODP_PACKET_OFFSET_INVALID;
pkt_hdr->l4_offset = ODP_PACKET_OFFSET_INVALID;
-   pkt_hdr->payload_offset = ODP_PACKET_OFFSET_INVALID;
 
/* Disable lazy parsing on user allocated packets */
if (!parse)
@@ -1118,7 +1116,8 @@ static inline void parse_tcp(odp_packet_hdr_t *pkt_hdr,
pkt_hdr->l4_len = pkt_hdr->l3_len +
pkt_hdr->l3_offset - pkt_hdr->l4_offset;
 
-   *offset   += (uint32_t)tcp->hl * 4;
+   if (offset)
+   *offset   += (uint32_t)tcp->hl * 4;
*parseptr += (uint32_t)tcp->hl * 4;
 }
 
@@ -1139,7 +1138,8 @@ static inline void parse_udp(odp_packet_hdr_t *pkt_hdr,
 
pkt_hdr->l4_len = udplen;
 
-   *offset   += sizeof(odph_udphdr_t);
+   if (offset)
+   *offset   += sizeof(odph_udphdr_t);
*parseptr += sizeof(odph_udphdr_t);
 }
 
@@ -1272,12 +1272,12 @@ int _odp_parse_common(odp_packet_hdr_t *pkt_hdr, const 
uint8_t *ptr)
 
case ODPH_IPPROTO_TCP:
pkt_hdr->input_flags.tcp = 1;
-   parse_tcp(pkt_hdr, &parseptr, &offset);
+   parse_tcp(pkt_hdr, &parseptr, NULL);
break;
 
case ODPH_IPPROTO_UDP:
pkt_hdr->input_flags.udp = 1;
-   parse_udp(pkt_hdr, &parseptr, &offset);
+   parse_udp(pkt_hdr, &parseptr, NULL);
break;
 
case ODPH_IPPROTO_AH:
@@ -1296,14 +1296,6 @@ int _odp_parse_common(odp_packet_hdr_t *pkt_hdr, const 
uint8_t *ptr)
break;
}
 
-   /*
-   * Anything beyond what we parse here is considered payload.
-   * Note: Payload is really only relevant for TCP and UDP.  For
-   * all other protocols, the payload offset will point to the
-   * final header (ARP, ICMP, AH, ESP, or IP Fragment).
-   */
-   pkt_hdr->payload_offset = offset;
-
 parse_exit:
pkt_hdr->input_flags.parsed_all = 1;
return pkt_hdr->error_flags.all != 0;
-- 
1.9.1

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCH v2 2/5] linux-generic: packet: remove vlan_s_tag and vlan_c_tag members from odp_packet_hdr_t

[Matias Elo]

There is no way to read vlan tags in the ODP API, so don't
save them.

Signed-off-by: Matias Elo 
---
 platform/linux-generic/include/odp_packet_internal.h | 4 
 platform/linux-generic/odp_packet.c  | 4 
 2 files changed, 8 deletions(-)

diff --git a/platform/linux-generic/include/odp_packet_internal.h 
b/platform/linux-generic/include/odp_packet_internal.h
index 508adf8..2a12503 100644
--- a/platform/linux-generic/include/odp_packet_internal.h
+++ b/platform/linux-generic/include/odp_packet_internal.h
@@ -143,8 +143,6 @@ typedef struct {
uint32_t l4_offset; /**< offset to L4 hdr (TCP, UDP, SCTP, also ICMP) */
uint32_t payload_offset; /**< offset to payload */
 
-   uint32_t vlan_s_tag; /**< Parsed 1st VLAN header (S-TAG) */
-   uint32_t vlan_c_tag; /**< Parsed 2nd VLAN header (C-TAG) */
uint32_t l3_len; /**< Layer 3 length */
uint32_t l4_len; /**< Layer 4 length */
 
@@ -184,8 +182,6 @@ static inline void 
copy_packet_parser_metadata(odp_packet_hdr_t *src_hdr,
dst_hdr->l4_offset  = src_hdr->l4_offset;
dst_hdr->payload_offset = src_hdr->payload_offset;
 
-   dst_hdr->vlan_s_tag = src_hdr->vlan_s_tag;
-   dst_hdr->vlan_c_tag = src_hdr->vlan_c_tag;
dst_hdr->l3_len = src_hdr->l3_len;
dst_hdr->l4_len = src_hdr->l4_len;
 }
diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 8681c08..436265e 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -42,8 +42,6 @@ void packet_parse_reset(odp_packet_hdr_t *pkt_hdr)
pkt_hdr->l3_offset= ODP_PACKET_OFFSET_INVALID;
pkt_hdr->l4_offset= ODP_PACKET_OFFSET_INVALID;
pkt_hdr->payload_offset   = ODP_PACKET_OFFSET_INVALID;
-   pkt_hdr->vlan_s_tag   = 0;
-   pkt_hdr->vlan_c_tag   = 0;
 }
 
 /**
@@ -1223,7 +1221,6 @@ int _odp_parse_common(odp_packet_hdr_t *pkt_hdr, const 
uint8_t *ptr)
 
vlan = (const odph_vlanhdr_t *)parseptr;
ethtype = odp_be_to_cpu_16(vlan->type);
-   pkt_hdr->vlan_s_tag = odp_be_to_cpu_16(vlan->tci);
offset += sizeof(odph_vlanhdr_t);
parseptr += sizeof(odph_vlanhdr_t);
}
@@ -1232,7 +1229,6 @@ int _odp_parse_common(odp_packet_hdr_t *pkt_hdr, const 
uint8_t *ptr)
pkt_hdr->input_flags.vlan = 1;
vlan = (const odph_vlanhdr_t *)parseptr;
ethtype = odp_be_to_cpu_16(vlan->type);
-   pkt_hdr->vlan_c_tag = odp_be_to_cpu_16(vlan->tci);
offset += sizeof(odph_vlanhdr_t);
parseptr += sizeof(odph_vlanhdr_t);
}
-- 
1.9.1

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCH v2 1/5] linux-generic: packet: remove l3_protocol and l4_protocol members from odp_packet_hdr_t

[Matias Elo]

Remove unnecessary struct odp_packet_hdr_t members
l3_protocol and l4_protocol to reduce struct size.

l4_protocol was only used by IPsec and is now replaced by
two new input flags ipsec_ah and ipsec_esp.

Signed-off-by: Matias Elo 
---
 platform/linux-generic/include/odp_classification_inlines.h |  7 ++-
 platform/linux-generic/include/odp_packet_internal.h| 10 +-
 platform/linux-generic/odp_packet.c |  9 +
 3 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/platform/linux-generic/include/odp_classification_inlines.h 
b/platform/linux-generic/include/odp_classification_inlines.h
index b8b04ce..08300f5 100644
--- a/platform/linux-generic/include/odp_classification_inlines.h
+++ b/platform/linux-generic/include/odp_classification_inlines.h
@@ -217,16 +217,13 @@ static inline int verify_pmr_ipsec_spi(const uint8_t 
*pkt_addr,
 {
uint32_t spi;
 
-   if (!pkt_hdr->input_flags.ipsec)
-   return 0;
-
pkt_addr += pkt_hdr->l4_offset;
 
-   if (pkt_hdr->l4_protocol == ODPH_IPPROTO_AH) {
+   if (pkt_hdr->input_flags.ipsec_ah) {
const odph_ahhdr_t *ahhdr = (const odph_ahhdr_t *)pkt_addr;
 
spi = odp_be_to_cpu_32(ahhdr->spi);
-   } else if (pkt_hdr->l4_protocol == ODPH_IPPROTO_ESP) {
+   } else if (pkt_hdr->input_flags.ipsec_esp) {
const odph_esphdr_t *esphdr = (const odph_esphdr_t *)pkt_addr;
 
spi = odp_be_to_cpu_32(esphdr->spi);
diff --git a/platform/linux-generic/include/odp_packet_internal.h 
b/platform/linux-generic/include/odp_packet_internal.h
index 93a92a0..508adf8 100644
--- a/platform/linux-generic/include/odp_packet_internal.h
+++ b/platform/linux-generic/include/odp_packet_internal.h
@@ -64,8 +64,12 @@ typedef union {
uint32_t ip_mcast:1;  /**< IP multicast */
uint32_t ipfrag:1;/**< IP fragment */
uint32_t ipopt:1; /**< IP optional headers */
-   uint32_t ipsec:1; /**< IPSec decryption may be needed */
 
+   uint32_t ipsec:1; /**< IPSec packet. Required by the
+  odp_packet_has_ipsec_set() func. */
+   uint32_t ipsec_ah:1;  /**< IPSec authentication header */
+   uint32_t ipsec_esp:1; /**< IPSec encapsulating security
+  payload */
uint32_t udp:1;   /**< UDP */
uint32_t tcp:1;   /**< TCP */
uint32_t tcpopt:1;/**< TCP options present */
@@ -141,9 +145,7 @@ typedef struct {
 
uint32_t vlan_s_tag; /**< Parsed 1st VLAN header (S-TAG) */
uint32_t vlan_c_tag; /**< Parsed 2nd VLAN header (C-TAG) */
-   uint32_t l3_protocol;/**< Parsed L3 protocol */
uint32_t l3_len; /**< Layer 3 length */
-   uint32_t l4_protocol;/**< Parsed L4 protocol */
uint32_t l4_len; /**< Layer 4 length */
 
uint32_t frame_len;
@@ -184,9 +186,7 @@ static inline void 
copy_packet_parser_metadata(odp_packet_hdr_t *src_hdr,
 
dst_hdr->vlan_s_tag = src_hdr->vlan_s_tag;
dst_hdr->vlan_c_tag = src_hdr->vlan_c_tag;
-   dst_hdr->l3_protocol= src_hdr->l3_protocol;
dst_hdr->l3_len = src_hdr->l3_len;
-   dst_hdr->l4_protocol= src_hdr->l4_protocol;
dst_hdr->l4_len = src_hdr->l4_len;
 }
 
diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index b002492..8681c08 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -44,8 +44,6 @@ void packet_parse_reset(odp_packet_hdr_t *pkt_hdr)
pkt_hdr->payload_offset   = ODP_PACKET_OFFSET_INVALID;
pkt_hdr->vlan_s_tag   = 0;
pkt_hdr->vlan_c_tag   = 0;
-   pkt_hdr->l3_protocol  = 0;
-   pkt_hdr->l4_protocol  = 0;
 }
 
 /**
@@ -1242,7 +1240,6 @@ int _odp_parse_common(odp_packet_hdr_t *pkt_hdr, const 
uint8_t *ptr)
/* Set l3_offset+flag only for known ethtypes */
pkt_hdr->input_flags.l3 = 1;
pkt_hdr->l3_offset = offset;
-   pkt_hdr->l3_protocol = ethtype;
 
/* Parse Layer 3 headers */
switch (ethtype) {
@@ -1270,7 +1267,6 @@ int _odp_parse_common(odp_packet_hdr_t *pkt_hdr, const 
uint8_t *ptr)
/* Set l4_offset+flag only for known ip_proto */
pkt_hdr->input_flags.l4 = 1;
pkt_hdr->l4_offset = offset;
-   pkt_hdr->l4_protocol = ip_proto;
 
/* Parse Layer 4 headers */
switch (ip_proto) {
@@ -1289,8 +1285,13 @@ int _odp_parse_common(odp_packet_hdr_t *pkt_hdr, const 
uint8_t *ptr)
break;
 
case ODPH_IPPROTO_AH:
+   pkt_hdr->input_flags.ipsec = 1;
+   pkt_hdr->input_flags.ipsec_ah = 1;
+   break;
+
case ODPH_IPPROTO_ESP:
pkt_hdr->input_flags.ipsec = 1;
+ 

[lng-odp] [PATCH v2 0/5] pktio fast path optimization

[Matias Elo]

V2:
+ Document odp_packet_hdr_t field initialization (Bill Fischover, Mike Holmes)
+ Remove unused payload_offset field from odp_packet_hdr_t

This patch set improves pktio fast path throughput by:
- Remove unnecessary members from odp_packet_hdr_t and odp_buffer_hdr_t to
  reduce memory usage (less cache misses).
- Don't use memset in packet_init() to zero odp_packet_hdr_t contents. Instead,
  initialize required values individually. This is less safe but provides a
  significant performance gain.

Matias Elo (5):
  linux-generic: packet: remove l3_protocol and l4_protocol members from
odp_packet_hdr_t
  linux-generic: packet: remove vlan_s_tag and vlan_c_tag members from
odp_packet_hdr_t
  linux-generic: packet: remove payload_offset member from
odp_packet_hdr_t
  linux-generic: buffer: ifdef ipc_addr_offset member from
odp_buffer_hdr_t
  linux-generic: packet: initialize only selected odp_packet_hdr_t
members

 .../linux-generic/include/odp_buffer_internal.h|  8 ++--
 .../include/odp_classification_inlines.h   |  7 +--
 .../linux-generic/include/odp_packet_internal.h| 29 ++--
 platform/linux-generic/odp_packet.c| 51 --
 platform/linux-generic/pktio/ipc.c | 13 ++
 5 files changed, 51 insertions(+), 57 deletions(-)

-- 
1.9.1

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [Bug 2228] ringtest: Conditional jump or move depends on uninitialised value

[bugzilla-daemon]

https://bugs.linaro.org/show_bug.cgi?id=2228

Fathi Boudra  changed:

   What|Removed |Added

   Assignee|maxim.uva...@linaro.org |yi...@linaro.org
 CC||fathi.bou...@linaro.org

-- 
You are receiving this mail because:
You are on the CC list for the bug.___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] lng-odp mailman settings

[Elo, Matias (Nokia - FI/Espoo)]

+1 for the mandatory plain text as well. At least for us replacing Outlook as 
email client is not really an option and the cursed blue line makes inline 
replies a pain.

-Matias

> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT 
> Zoltan
> Kiss
> Sent: Monday, May 16, 2016 12:32 PM
> To: Mike Holmes ; lng-odp 
> Subject: Re: [lng-odp] lng-odp mailman settings
> 
> +1 for mandatory plain text. HTML mail is for newsletters, but as soon
> as you try to reply to a section of someone else's mail, it breaks down
> to chaos. My observation is that mail clients tend to handle it
> differently, and screwing up each other (and their users) in the process.
> And setting font color for each responses doesn't scale well, it's just
> a band-aid. (and kid themed band-aids could be actually pretty cool, so
> it's worse than band-aid)
> If we could enforce everyone using the same mail client, HTML would
> work, but that's not the case here.
> 
> On 13/05/16 15:57, Mike Holmes wrote:
> > All,
> >
> > A topic that comes up occasionally that affects some mail tools like
> > outlook is that html email on this list makes it harder for some folks
> > to process patches.
> >
> > I use mutt specifically to avoid using a fancy email client for all my
> > patch download/send work, it is powered by coal/steam and would work
> > fine on a 1970 VAX but there you go.
> >
> > Anyway, does anyone object to having mailman reject HTML mail ? One up
> > side is that I will not waste time blocking offers of cheap watches and
> > fax services from the list :)
> >
> > On the other hand I will miss bullet lists and the color red when I am
> > grumpy.
> >
> > Mike
> >
> > --
> > 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 mailing list
> > lng-odp@lists.linaro.org
> > https://lists.linaro.org/mailman/listinfo/lng-odp
> >
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] lng-odp mailman settings

[Zoltan Kiss]

+1 for mandatory plain text. HTML mail is for newsletters, but as soon 
as you try to reply to a section of someone else's mail, it breaks down 
to chaos. My observation is that mail clients tend to handle it 
differently, and screwing up each other (and their users) in the process.
And setting font color for each responses doesn't scale well, it's just 
a band-aid. (and kid themed band-aids could be actually pretty cool, so 
it's worse than band-aid)
If we could enforce everyone using the same mail client, HTML would 
work, but that's not the case here.


On 13/05/16 15:57, Mike Holmes wrote:

All,

A topic that comes up occasionally that affects some mail tools like
outlook is that html email on this list makes it harder for some folks
to process patches.

I use mutt specifically to avoid using a fancy email client for all my
patch download/send work, it is powered by coal/steam and would work
fine on a 1970 VAX but there you go.

Anyway, does anyone object to having mailman reject HTML mail ? One up
side is that I will not waste time blocking offers of cheap watches and
fax services from the list :)

On the other hand I will miss bullet lists and the color red when I am
grumpy.

Mike

--
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 mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] lng-odp mailman settings

[Savolainen, Petri (Nokia - FI/Espoo)]

From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Bill 
Fischofer
Sent: Saturday, May 14, 2016 1:55 AM
To: Brian Brooks 
Cc: lng-odp 
Subject: Re: [lng-odp] lng-odp mailman settings



On Friday, May 13, 2016, Brian Brooks 
mailto:brian.bro...@linaro.org>> wrote:
On 05/13 10:07:44, Bill Fischofer wrote:
> I don't think we need to impose those sort of restrictions. I personally
> use sylpheed for patches and GMail for everything else and they have no
> problem sorting things out.
>
> The ODP mailing list covers both discussions as well as patches, and HTML
> is useful for the former. The onus should be on those who have problems
> with this to upgrade their tools rather than sending everyone else back to
> the 1980s.
>
> On Fri, May 13, 2016 at 9:57 AM, Mike Holmes 
> > wrote:
>
> > All,
> >
> > A topic that comes up occasionally that affects some mail tools like
> > outlook is that html email on this list makes it harder for some folks to
> > process patches.
> >
> > I use mutt specifically to avoid using a fancy email client for all my
> > patch download/send work, it is powered by coal/steam and would work fine
> > on a 1970 VAX but there you go.
> >
> > Anyway, does anyone object to having mailman reject HTML mail ? One up
> > side is that I will not waste time blocking offers of cheap watches and fax
> > services from the list :)
> >
> > On the other hand I will miss bullet lists and the color red when I am
> > grumpy.
> >
> > Mike

+1 for plain text

I think this topic is more about simplicity and best practices rather than
anything else.

Here is an example of why HTML doesn't work well with archives:
https://lists.linaro.org/pipermail/lng-odp/2016-May/023134.html
That link displays perfectly fine for me.  What problem do you see?

Did you read it? It’s a mixture of paragraphs from two writers, without any 
indication who is writing what. The list archive drops out HTML decoration and 
at least in some cases will not convert (various blue or grey lines, or 
indentations) into ‘>’.

As Brian highlights, it’s about simplicity. With one rule (no HTML) we can 
uniform output of all mail clients and their fonts/indentations/other settings, 
so that the list is clean and clear to read (both directly and from the 
archives) and reply (no need to struggle with mismatching styles inherited from 
the senders HTML settings, etc).

+1 for plain text

-Petri

___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH] doc: users-guide: add packet marking documentation

[Bala Manoharan]

Hi,

I have incorporated all the comments in V2 except adding reference for
IEEE802.1Q
I am little skeptical about adding a wikipedia page as reference.

Regards,
Bala

On 15 May 2016 at 03:09, Bill Fischofer  wrote:

>
>
> On Fri, May 13, 2016 at 12:23 AM, Balasubramanian Manoharan <
> bala.manoha...@linaro.org> wrote:
>
>> Updates packet marking api documentation to traffic manager user guide
>>
>> Signed-off-by: Balasubramanian Manoharan 
>> ---
>>  doc/users-guide/users-guide-tm.adoc | 73
>> +
>>  1 file changed, 73 insertions(+)
>>
>> diff --git a/doc/users-guide/users-guide-tm.adoc
>> b/doc/users-guide/users-guide-tm.adoc
>> index 12685b2..06c 100644
>> --- a/doc/users-guide/users-guide-tm.adoc
>> +++ b/doc/users-guide/users-guide-tm.adoc
>> @@ -263,6 +263,79 @@ settings for any TM system, though in most cases a
>> TM system can (and should)
>>  be created/instantiated with smaller values, since lower values will
>> often
>>  result in faster operation and/or less memory used.
>>
>> + Packet Marking
>> +
>> +Packet Marking API is used to mark the packet based upon the final
>> packet color
>>
>
> grammar: The Packet Marking API
>
>
>> +assigned to the packet when it reaches the egress node.
>> +This is an optional feature and if available on the platform is used to
>> reflect
>> +the packet color on IPv4/IPv6 DiffServ filed in accordance with RFC 2474.
>>
>
> Since this results in an HTML doc, RFC's should be annotated as
> hyperlinks, e.g.:
> https://www.ietf.org/rfc/rfc2474.txt[RFC 2474]
>
>
>> +There are three different packet marking fields supported they are,
>> +1). Assured forwarding in accordance with RFC 2597, the DSCP is marked to
>>
>
> https://www.ietf.org/rfc/rfc2597.txt[RFC 2597]
>
>
>> +set the packet Drop precedence in accordance with the color, i.e High
>> Drop
>> +precedence for RED, Medium Drop precedence for YELLOW and leave the DSCP
>> +unchangesd if the color is GREEN.
>>
>
> Typo: unchanged
>
>
>> +2). Explicit Congestion Notification protocol per RFC 3168, where a
>> router
>>
>
> https://www.ietf.org/rfc/rfc3168.txt[RFC 3168]
>
>
>> +encountering congestion can notifiy it by setting the lower 2 bits in
>>
>
> Typo: notify
>
>
>> +DiffServ field to "11" Congestion Encountered code, which will ultimately
>> +reduce the transmission rate of the packet sender.
>> +3). In IEEE 802.1q VLAN tag header contains a DE - Drop Eligibility bit
>> for
>> +marking a packet for Downstream switches, and is valid for Ethernet
>> packet
>> +containing a VLAN tag.
>> +
>> +RFC 3168 is only valid for TCP packets whereas RFC 2597 is valid for
>> IPv4/IPv6
>> +traffic.
>> +
>> +The values are set per color and hence the implementation may support
>> these
>> +parameters only for a specific colors. marking_colors_supported field in
>> +capabilities structure can be used to check which colors are supported
>> for
>> +marking.
>> +
>> +Vlan Marking.
>>
>
> = Vlan Marking
> [These are sub-sections and should be annotated as such]
>
>
>> +
>> +This vlan marking is used to enable the drop eligibility on the packet
>> +based on the packet color. If drop eligibility is enabled then the
>> +implementation will set the one bit VLAN Drop Eligibility Indicator (DEI)
>> +field (but only for pkts that already carry a VLAN tag) of a packet based
>>
>
> I'd spell out packets here. The use of the abbreviation should only be for
> APIs that do so.
>
>
>> +upon the final pkt color assigned to the pkt when it reaches the egress
>>
>
> Again, packet, not pkt here.
>
>
>> +node.  When drop_eligible_enabled is false, then the given color has
>> +no effect on the VLAN fields.  See IEEE 802.1q for more details.
>>
>
> Since IEEE docs are protected, best to just link this one to the Wikipedia
> article on it:
> See https://en.wikipedia.org/wiki/IEEE_802.1Q[IEEE 802.1q] for more
> details.
>
>
>> +vlan_marking_supported boolean in capability structure indicates whether
>> this
>>
>
> `vlan_marking_supported`
>
>
>> +feature is supported by the implementation.
>> +
>> +Explicit Congestion Notification Marking.
>>
>
> = Explicit Congestion Notification Marking
> [No period follows subsection title]
>
>
>> +
>> +The odp_tm_ecn_marking() function allows one to configure the TM
>>
>
> Annotate APIs with backticks `odp_tm_ecn_marking()` to make them monospace
> font for consistency with the rest of the User Guide.
>
>
>> +egress so that the two bit ECN subfield of the eight bit TOS field of an
>> +IPv4 pkt OR the eight bit Traffic Class (TC) field of an IPv6 pkt can be
>>
>
> packet, not pkt
>
>
>> +selectively modified based upon the final color assigned to the pkt when
>> it
>>
>
> packet
>
>
>> +reaches the egress.  Note that the IPv4 header checksum will be updated -
>> +but only if the IPv4 TOS field actually changes as a result of this
>> +setting or the odp_tm_drop_prec_marking setting.  For IPv6, since there
>> is
>>
>
> `odp_tm_drop_prec_marking()`
>
>
>> +no header chec

[lng-odp] [PATCHv2] doc: users-guide: add packet marking documentation

[Balasubramanian Manoharan]

Updates packet marking api documentation to traffic manager user guide

Signed-off-by: Balasubramanian Manoharan 
---
v2: document format update
 doc/users-guide/users-guide-tm.adoc | 73 +
 1 file changed, 73 insertions(+)

diff --git a/doc/users-guide/users-guide-tm.adoc 
b/doc/users-guide/users-guide-tm.adoc
index 12685b2..e02697b 100644
--- a/doc/users-guide/users-guide-tm.adoc
+++ b/doc/users-guide/users-guide-tm.adoc
@@ -263,6 +263,79 @@ settings for any TM system, though in most cases a TM 
system can (and should)
 be created/instantiated with smaller values, since lower values will often
 result in faster operation and/or less memory used.
 
+ Packet Marking
+
+The Packet Marking API is used to mark the packet based upon the final packet
+color assigned to it when it reaches the egress node.
+This is an optional feature and if available on the platform is used to reflect
+the packet color on IPv4/IPv6 DiffServ filed in accordance with 
https://www.ietf.org/rfc/rfc2474.txt[RFC 2474].
+There are three different packet marking fields supported they are,
+1). Assured forwarding in accordance with 
https://www.ietf.org/rfc/rfc2597.txt[RFC 2597], the DSCP is marked to
+set the packet Drop precedence in accordance with the color, i.e High Drop
+precedence for RED, Medium Drop precedence for YELLOW and leave the DSCP
+unchanged if the color is GREEN.
+2). Explicit Congestion Notification protocol per 
https://www.ietf.org/rfc/rfc3168.txt[RFC 3168], where a router
+encountering congestion can notify it by setting the lower 2 bits in
+DiffServ field to "11" Congestion Encountered code, which will ultimately
+reduce the transmission rate of the packet sender.
+3). In IEEE 802.1q VLAN tag header contains a DE - Drop Eligibility bit for
+marking a packet for Downstream switches, and is valid for Ethernet packet
+containing a VLAN tag.
+
+RFC 3168 is only valid for TCP packets whereas RFC 2597 is valid for IPv4/IPv6
+traffic.
+
+The values are set per color and hence the implementation may support these
+parameters only for a specific colors. marking_colors_supported field in
+capabilities structure can be used to check which colors are supported for
+marking.
+
+ Vlan Marking.
+
+This vlan marking is used to enable the drop eligibility on the packet
+based on the packet color. If drop eligibility is enabled then the
+implementation will set the one bit VLAN Drop Eligibility Indicator (DEI)
+field (but only for packets that already carry a VLAN tag) of a packet based
+upon the final packet color assigned to the packet when it reaches the egress
+node.  When drop_eligible_enabled is false, then the given color has
+no effect on the VLAN fields.  See IEEE 802.1q for more details.
+`vlan_marking_supported` boolean in capability structure indicates whether this
+feature is supported by the implementation.
+
+ Explicit Congestion Notification Marking.
+
+The `odp_tm_ecn_marking()` function allows one to configure the TM
+egress so that the two bit ECN subfield of the eight bit TOS field of an
+IPv4 packet OR the eight bit Traffic Class (TC) field of an IPv6 packet can be
+selectively modified based upon the final color assigned to the packet when it
+reaches the egress.  Note that the IPv4 header checksum will be updated -
+but only if the IPv4 TOS field actually changes as a result of this
+setting or the `odp_tm_drop_prec_marking()` setting.  For IPv6, since there is
+no header checksum, nothing needs to be done. If ECN is enabled for a
+particular color then ECN subfield will be set to _ECN_CE_  _i.e.,_ congestion
+experienced.
+`ecn_marking_supported` boolean in capability structure indicates whether this
+feature is supported by the implementation.
+
+ Drop Precedence Marking.
+
+The Drop precedence marking allows one to configure the TM
+egress to support Assured forwarding in accordance with 
https://www.ietf.org/rfc/rfc2597.txt[RFC 2597].
+The Drop Precedence bits are contained within the six bit Differentiated
+Services Code Point subfield of the IPv4 TOS field or the IPv6 Traffic
+Class (TC) field.  Specifically the Drop Precedence sub-subfield can be
+accessed with a DSCP bit mask of 0x06.  When enabled for a given color,
+these two bits will be set to Medium Drop Precedence (value 0x4) if the
+color is ODP_PACKET_YELLOW, set to High Drop Precedence (value 0x6) if
+the color is ODP_PACKET_RED.
+
+Note that the IPv4 header checksum will be updated - but only if the
+IPv4 TOS field actually changes as a result of this setting or the
+`odp_tm_ecn_marking()` setting.  For IPv6, since there is no header checksum,
+nothing else needs to be done.
+`drop_prec_marking_supported` boolean in capability structure indicates whether
+this feature is supported by the implementation.
+
 === Examples
 
 .Create a tm_node chain for two nodes and associate the scheduler
-- 
1.9.1

___
lng-odp mailing list
lng-odp@lists.linaro.org
h

Re: [lng-odp] [PATCH 4/4] linux-generic: packet: initialize only selected odp_packet_hdr_t members

[Elo, Matias (Nokia - FI/Espoo)]

From: EXT Bill Fischofer [mailto:bill.fischo...@linaro.org]
Sent: Saturday, May 14, 2016 8:45 PM
To: Mike Holmes 
Cc: Elo, Matias (Nokia - FI/Espoo) ; LNG ODP Mailman List 

Subject: Re: [lng-odp] [PATCH 4/4] linux-generic: packet: initialize only 
selected odp_packet_hdr_t members

I agree we can't afford to ignore a major performance gain, however this is an 
area that should be documented in the code. Something to the effect that any 
new fields added must be reviewed for initialization requirements.

Sure, I’ll send v2 with improved comments.

-Matias


On Fri, May 13, 2016 at 9:46 AM, Mike Holmes 
mailto:mike.hol...@linaro.org>> wrote:


On 13 May 2016 at 03:29, Elo, Matias (Nokia - FI/Espoo) 
mailto:matias@nokia.com>> wrote:


From: EXT Bill Fischofer 
[mailto:bill.fischo...@linaro.org]
Sent: Thursday, May 12, 2016 10:34 PM
To: Elo, Matias (Nokia - FI/Espoo) 
mailto:matias@nokia.com>>
Cc: LNG ODP Mailman List 
mailto:lng-odp@lists.linaro.org>>
Subject: Re: [lng-odp] [PATCH 4/4] linux-generic: packet: initialize only 
selected odp_packet_hdr_t members



On Thu, May 12, 2016 at 7:36 AM, Matias Elo 
mailto:matias@nokia.com>> wrote:
Using memset to initialize struct odp_packet_hdr_t contents
to 0 has a significant overhead. Instead, initialize only
the required members of the struct.

Signed-off-by: Matias Elo mailto:matias@nokia.com>>
---
 platform/linux-generic/include/odp_packet_internal.h |  8 +---
 platform/linux-generic/odp_packet.c  | 18 ++
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/platform/linux-generic/include/odp_packet_internal.h 
b/platform/linux-generic/include/odp_packet_internal.h
index 2a12503..cdee1e1 100644
--- a/platform/linux-generic/include/odp_packet_internal.h
+++ b/platform/linux-generic/include/odp_packet_internal.h
@@ -143,15 +143,17 @@ typedef struct {
uint32_t l4_offset; /**< offset to L4 hdr (TCP, UDP, SCTP, also ICMP) */
uint32_t payload_offset; /**< offset to payload */

-   uint32_t l3_len; /**< Layer 3 length */
-   uint32_t l4_len; /**< Layer 4 length */
-
uint32_t frame_len;
uint32_t headroom;
uint32_t tailroom;

odp_pktio_t input;

+   /* Values below are not initialized by packet_init() */
+
+   uint32_t l3_len; /**< Layer 3 length */
+   uint32_t l4_len; /**< Layer 4 length */
+
uint32_t flow_hash;  /**< Flow hash value */
odp_time_t timestamp;/**< Timestamp value */

diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 436265e..f5f224d 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -50,23 +50,17 @@ void packet_parse_reset(odp_packet_hdr_t *pkt_hdr)
 static void packet_init(pool_entry_t *pool, odp_packet_hdr_t *pkt_hdr,
size_t size, int parse)
 {
-   /*
-   * Reset parser metadata.  Note that we clear via memset to make
-   * this routine indepenent of any additional adds to packet metadata.
-   */
-   const size_t start_offset = ODP_FIELD_SIZEOF(odp_packet_hdr_t, buf_hdr);
-   uint8_t *start;
-   size_t len;
+   pkt_hdr->input_flags.all  = 0;
+   pkt_hdr->output_flags.all = 0;
+   pkt_hdr->error_flags.all  = 0;

-   start = (uint8_t *)pkt_hdr + start_offset;
-   len = sizeof(odp_packet_hdr_t) - start_offset;
-   memset(start, 0, len);

The reason for this memset is to "future proof" the header by allowing any new 
fields that are added to get initialized to a known value without having to 
update this routine. Is the impact of this single memset() all that large?  I 
can understand wanting to compress the header to possibly avoid another cache 
line reference but this optimization seems inherently error-prone. I'm also 
wondering if this might confuse tools like Coverity which seems to be paranoid 
about uninitialized variables.

Hi Bill,

I can understand the future proofing. However, this single memset() is 
currently the largest cycle hog when forwarding small packets. Below are some 
throughput numbers:

odp_l2fwd -i p1p1 -c 1 -a 0 (netmap pktio):

Niantic (1x10Gbps):7.9 vs 8.6 Mpps (64B) => 8% improvement
Fortville (1x40Gbps):  7.9 vs 8.8 Mpps (64B) => 11% improvement

When the performance gain is this big I would pay the price of being more 
error-prone. Is there a way to run Coverity before merging the patch?

Not easily until we resolve the licence/number of coverty runs per week issue - 
on going :/
Coverty might be ok as long as we dont use the field for anything, and  for 11% 
I am happy to mark it a false positive in coverty if it is flagged
Can we mitigate some of the problems with a doxygen warning on this struct 
about its behaviour ?



-Matias

-
-   /* Set metadata items that initialize to non-zero values */
+   pkt_hdr->l2_offset = 0;