Re: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user context support for interfaces

[Bogdan Pricope]

Hi Bill,

Thank you for trying to push this further.

A problem is that you are pushing details of your internal data organization 
into application. This may get messy to maintain especially if you decide to 
reorganize the table(s) at some point or increase number of pktios.
Btw, why don’t you transform odp_pktio_t into this pktio index?

Else, we can work with such index, but I don’t like the solution (no 
Reviewed-by from me).

The two SW layers is a false problem: once you identified the right interface, 
it is up to application to find the right data structure inside each layer

Bogdan


From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
Sent: Wednesday, April 13, 2016 10:47 PM
To: Ola Liljedahl 
Cc: Savolainen, Petri (Nokia - FI/Espoo) ; Bogdan 
Pricope ; lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user context 
support for interfaces


On Wed, Apr 13, 2016 at 2:40 PM, Ola Liljedahl 
mailto:ola.liljed...@linaro.org>> wrote:


On 13 April 2016 at 21:26, Bill Fischofer 
mailto:bill.fischo...@linaro.org>> wrote:
We already have odp_packet_input() so are you saying asking the application to 
write odp_pktio_id(odp_packet_input(pkt)) is too cumbersome?
I would like to be able to avoid any unnecessary memory accesses. Having to 
first retrieve the packet IO handle and then dereference that data structure to 
retrieve the pktio ID (interface index) seems to require an unnecessary load to 
a data structure not necessarily in the cache (or is it likely the ODP pktio 
data structure will otherwise be referenced for every packet and thus resident 
i the L1 cache?).

I think it is likely the packet metadata will actually contain the pktio 
ID/interface index, not the corresponding pktio handle. So return the index 
directly for use by the application as index into different application 
specific tables.

Actually, the intended performance model for ODP is that packets are classified 
into flows and the flow context associated with the queue delivering the packet 
contains all the info the application needs.  Why try to tie information to 
individual packets other than the (mutable) metadata that the application is 
doing to be working with?



Id can be renamed index if that's the preference.

On Wed, Apr 13, 2016 at 2:22 PM, Ola Liljedahl 
mailto:ola.liljed...@linaro.org>> wrote:


On 13 April 2016 at 19:38, Bill Fischofer 
mailto:bill.fischo...@linaro.org>> wrote:


On Wed, Apr 13, 2016 at 12:36 PM, Bill Fischofer 
mailto:bill.fischo...@linaro.org>> wrote:

On Wed, Apr 13, 2016 at 8:09 AM, Ola Liljedahl 
mailto:ola.liljed...@linaro.org>> wrote:
On 8 April 2016 at 17:25, Savolainen, Petri (Nokia - FI/Espoo) 
mailto:petri.savolai...@nokia.com>> wrote:


From: EXT Bogdan Pricope 
[mailto:bogdan.pric...@enea.com]
Sent: Friday, April 08, 2016 4:50 PM
To: Bill Fischofer 
mailto:bill.fischo...@linaro.org>>; Savolainen, 
Petri (Nokia - FI/Espoo) 
mailto:petri.savolai...@nokia.com>>
Cc: lng-odp@lists.linaro.org
Subject: RE: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user context 
support for interfaces

Hi,

Our path is:
pktio = odp_packet_input(pkt);
A step is invisible here:
queue = odp_pktio_outq_getdef(pktio)
ifnet = ofp_queue_context(queue)

ifnet = ofp_get_ifnet_pktio(pktio);

(Our current workaround is to iterate through the list of interfaces to find 
which interface has this pktio.)

You are saying:
pktio_id = odp_packet_input_id(pkt);
ifnet = ofp_get_ifnet_pktio_id(pktio_id);

Code generated with ID:
pktio_id = pkt_hdr->pktio_id;
ifnet = &ofp_ifnet_table[pktio_id]:
I think a pktio integer identifier (defined as a small integer suitable for 
using as an array index) is the most generic solution. Different SW layers can 
use this pktio ID as an index into *different* tables (different SW layers will 
have different needs). A (single) ODP pktio context pointer only makes one SW 
layer happy (or will promote a monolithic SW design which will be fragile and 
difficult to extend with new functionality).


That sounds reasonable.  So should we consider the following new APIs?
Yes looks good to me except "id" in the name feels a bit short and not specific 
enough. What about "pktioid"? (well that doesn't look great either, I usually 
call this thing "ifindex" for interface index).

But we also want an API to extract the pktio ID from a packet (which identifies 
which pktio the packet was received on).
int odp_packet_pktioid(odp_packet_t pkt);
Will return -1 for a packet that was not received on some interface (i.e. 
returned from odp_packet_alloc()).


uint32_t odp_pktio_to_id(odp_pktio_t pktio);

This returns a small integer index in the range 0..num_pktios-1

Actually that first signature should be int rather than uint32_t since -1 would 
be returned for failure.


odp_pktio_t odp_pktio_from_id(uint32_t id);

For symmetry id should be typed as int rather than uint32_t?


Returns the pktio handle assoc

Re: [lng-odp] [API-NEXT PATCH v2 1/7] api: packet: add extend and trunc

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

Thanks for the review. Although, I’d prefer to get all 6 patches merged in one 
go, since reordering of these will cause conflicts. I’d continue to work 
locally on top of this set. Also the implementation patch set should not 
include these, but go on top. It’s hard to see if something was changed in the 
other set. Bill’s signed-off-by hints that something was changed but I guess 
nothing was changed, except the ordering compared to patches 3 and 5.

Bala, are you OK with patches 3 and 5?

-Petri


From: EXT Bill Fischofer [mailto:bill.fischo...@linaro.org]
Sent: Wednesday, April 13, 2016 5:29 PM
To: Savolainen, Petri (Nokia - FI/Espoo) 
Cc: LNG ODP Mailman List 
Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/7] api: packet: add extend and trunc

For parts 1, 2, 4, and 6

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

I'm OK with merging those parts into api-next to allow parallel 
implementation/test work on them. I believe we  still need to get more input 
and feedback on parts 3 and 5 before these can be merged.

On Wed, Apr 13, 2016 at 9:19 AM, Bill Fischofer 
mailto:bill.fischo...@linaro.org>> wrote:
This latest definition looks good. I'll post a v4 of my implementation patch 
series to reflect it.

On Wed, Apr 13, 2016 at 8:01 AM, Petri Savolainen 
mailto:petri.savolai...@nokia.com>> wrote:
Added functions to extend / truncate packet head / tail more
than current head/tailroom or segment lengths.

Signed-off-by: Petri Savolainen 
mailto:petri.savolai...@nokia.com>>
---
 include/odp/api/spec/packet.h | 144 ++
 1 file changed, 144 insertions(+)

diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h
index 7da353b..6c88458 100644
--- a/include/odp/api/spec/packet.h
+++ b/include/odp/api/spec/packet.h
@@ -406,6 +406,150 @@ void *odp_packet_push_tail(odp_packet_t pkt, uint32_t 
len);
 void *odp_packet_pull_tail(odp_packet_t pkt, uint32_t len);

 /**
+ * Extend packet head
+ *
+ * Increase packet data length at packet head. Functionality is analogous to
+ * odp_packet_push_head() when data length is extended up to headroom size.
+ * When data length is increased more than that, new segments are added into
+ * the packet head and old segment handles become invalid.
+ *
+ * A successful operation overwrites the packet handle with a new handle, which
+ * application must use as the reference to the packet instead of the old
+ * handle. Depending on the implementation, the old and new handles may be
+ * equal.
+ *
+ * The operation return value indicates if any packet data or metadata (e.g.
+ * user_area) were moved in memory during the operation. If some memory areas
+ * were moved, application must use new packet/segment handles to update
+ * data pointers. Otherwise, all old pointers remain valid.
+ *
+ * User is responsible to update packet metadata offsets when needed. Packet
+ * is not modified if operation fails.
+ *
+ * @param[in, out] pkt  Pointer to packet handle. A successful operation 
outputs
+ *  the new packet handle.
+ * @param len   Number of bytes to extend the head
+ * @param[out] data_ptr Pointer to output the new data pointer.
+ *  Ignored when NULL.
+ * @param[out] seg_len  Pointer to output segment length at 'data_ptr' above.
+ *  Ignored when NULL.
+ *
+ * @retval 0   Operation successful, old pointers remain valid
+ * @retval >0  Operation successful, old pointers need to be updated
+ * @retval <0  Operation failed (e.g. due to an allocation failure)
+ */
+int odp_packet_extend_head(odp_packet_t *pkt, uint32_t len, void **data_ptr,
+  uint32_t *seg_len);
+
+/**
+ * Truncate packet head
+ *
+ * Decrease packet data length at packet head. Functionality is analogous to
+ * odp_packet_pull_head() when data length is truncated less than the first
+ * segment data length. When data length is decreased more than that, some head
+ * segments are removed from the packet and old segment handles become invalid.
+ *
+ * A successful operation overwrites the packet handle with a new handle, which
+ * application must use as the reference to the packet instead of the old
+ * handle. Depending on the implementation, the old and new handles may be
+ * equal.
+ *
+ * The operation return value indicates if any packet data or metadata (e.g.
+ * user_area) were moved in memory during the operation. If some memory areas
+ * were moved, application must use new packet/segment handles to update
+ * data pointers. Otherwise, all old pointers remain valid.
+ *
+ * User is responsible to update packet metadata offsets when needed. Packet
+ * is not modified if operation fails.
+ *
+ * @param[in, out] pkt  Pointer to packet handle. A successful operation 
outputs
+ *  the new packet handle.
+ * @param len   Number of bytes to truncate the head (0 ... packet_len)
+ * @param[out] data_ptr Pointer to output the new data 

Re: [lng-odp] [RFC] api: crypto capability support

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

Hi,

See under my proposal on using bit fields and a capability struct similar to 
what we have already for other APIs. Application may easily check if multiple 
ciphers are support and especially if those are implemented with HW offload.

-Petri


odp_crypto_ciphers_t ciphers;
odp_pktio_capability_t capa;

ciphers.all_bits   = 0;
ciphers.3des_cbc   = 1;
ciphers.aes128_cbc = 1;

odp_crypto_capability(&capa);

if (capa.chiphers.all_bits & ciphers.all_bits == 0) {
 // 3des or aes are not supported
 ...
 return -1;
}

if (capa.hw_chiphers.all_bits & ciphers.all_bits == 0) {
 // 3des or aes are not supported in HW
 ...
}



typedef union odp_crypto_ciphers_t {
/** Cipher algorithms */
struct {
/** ODP_CIPHER_ALG_NULL */
uint32_t null   : 1;

/** ODP_CIPHER_ALG_DES */
uint32_t des: 1;

/** ODP_CIPHER_ALG_3DES_CBC */
uint32_t 3des_cbc   : 1;

/** ODP_CIPHER_ALG_AES128_CBC */
uint32_t aes128_cbc : 1;

/** ODP_CIPHER_ALG_AES128_GCM */
uint32_t aes128_gcm : 1;

} bit;

/** All bits of the bit field structure
  *
  * This field can be used to set/clear all flags, or bitwise
  * operations over the entire structure. */
uint32_t all_bits;
} odp_crypto_ciphers_t;


typedef union odp_crypto_auths_t {
/** Cipher algorithms */
struct {
/** ODP_AUTH_ALG_NULL */
uint32_t null   : 1;

/** ODP_AUTH_ALG_MD5_96 */
uint32_t md5_96 : 1;

/** ODP_AUTH_ALG_SHA256_128 */
uint32_t sha256_128 : 1;

/** ODP_AUTH_ALG_AES128_GCM */
uint32_t aes128_gcm : 1;

} bit;

/** All bits of the bit field structure
  *
  * This field can be used to set/clear all flags, or bitwise
  * operations over the entire structure. */
uint32_t all_bits;
} odp_crypto_auths_t;


typedef struct odp_crypto_capability_t {
/** Maximum number of crypto sessions */
uint32_t max_sessions;

/** Supported chipher algorithms */
odp_crypto_ciphers_t chiphers;

/** Chipher algorithms implemented with HW offload */
odp_crypto_ciphers_t hw_chiphers;

/** Supported authentication algorithms */
odp_crypto_auths_t auths;

/** Authentication algorithms implemented with HW offload */
odp_crypto_auths_t hw_auths;

} odp_crypto_capability_t;


/**
 * Query crypto capabilities
 *
 * Outputs crypto capabilities on success.
 *
 * @param[out] capa   Pointer to capability structure for output
 *
 * @retval 0 on success
 * @retval <0 on failure
 */
int odp_crypto_capability(odp_pktio_capability_t *capa);




> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT
> Balakrishna Garapati
> Sent: Wednesday, April 13, 2016 3:33 PM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [RFC] api: crypto capability support
> 
> This RFC provides the support for the applicationis to inquire the given
> cipher, authentication algorithms
> 
> Signed-off-by: Balakrishna Garapati 
> ---
>  include/odp/api/spec/crypto.h | 30 ++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/include/odp/api/spec/crypto.h b/include/odp/api/spec/crypto.h
> index 41beedb..0def211 100644
> --- a/include/odp/api/spec/crypto.h
> +++ b/include/odp/api/spec/crypto.h
> @@ -254,6 +254,18 @@ typedef struct odp_crypto_op_result {
>  } odp_crypto_op_result_t;
> 
>  /**
> + * Crypto API capability result
> + */
> +typedef enum odp_crypto_capability_t {
> + /** crypto algorithm not supported */
> + ODP_CRYPTO_NO_SUPPORT = 0,
> + /** crypto algorithm supported in hardware */
> + ODP_CRYPTO_HW_SUPPORT,
> + /** crypto algortihm supported in software */
> + ODP_CRYPTO_SW_SUPPORT
> +} odp_crypto_capability_t;
> +
> +/**
>   * Crypto session creation (synchronous)
>   *
>   * @param paramsSession parameters
> @@ -368,6 +380,24 @@ uint64_t
> odp_crypto_session_to_u64(odp_crypto_session_t hdl);
>  uint64_t odp_crypto_compl_to_u64(odp_crypto_compl_t hdl);
> 
>  /**
> + * Verify the given crypto cipher algorithm support
> + *
> + * @param alg  odp_cipher_alg_t to be verified
> + * @return odp_crypto_capability_t
> + *
> + */
> +odp_crypto_capability_t odp_crypto_cipher_inquiry(odp_cipher_alg_t alg);
> +
> +/**
> + * Verify the given crypto authentication algorithm support
> + *
> + * @param alg  odp_auth_alg_t to be verified
> + * @return odp_crypto_capability_t
> + *
> + */
> +odp_crypto_capability_t odp_crypto_auth_inquiry(odp_auth_alg_t alg);
> +
> +/**
>   * @}
>   */
> 
> --
> 1.9.1
> 
> ___
> lng-odp mailing list
> lng-odp@lists.linaro.org
> htt

Re: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add pktio id conversion APIs

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

Hi,

There's no need to convert from index to handle. Application should save handle 
into its own data structures. Direct index from packet should be added, since 
it's very likely that packet descriptor stores an index anyway. 
odp_config_pktio_entries() will be renamed to odp_pktio_max_interfaces() or 
similar during config.h cleanup.

So, I think only these two functions under need to be added.

-Petri


/**
 * Get pktio interface index
 *
 * @param pktio   Packet IO handle
 *
 * @returnPacket interface index (0..odp_config_pktio_entries()-1)
 * @retval <0 On failure (e.g., handle not valid)
 */
int odp_pktio_index(odp_pktio_t pktio);


/**
 * Packet input interface index
 *
 * Returns index of the packet IO interface which received the packet or
 * <0 when the packet was allocated/reset by the application.
 *
 * @param pkt   Packet handle
 *
 * @return Packet interface index (0..odp_config_pktio_entries()-1)
 * @retval <0  Packet was not received on any interface
 */
int odp_packet_input_index(odp_packet_t pkt);



> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT
> Bill Fischofer
> Sent: Wednesday, April 13, 2016 10:19 PM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add pktio id
> conversion APIs
> 
> Add the two APIs odp_pktio_to_id() and odp_pktio_from_id() to enable
> applications to convert PktIO handles to and from indices for managing
> user contexts and related associated data.
> 
> Signed-off-by: Bill Fischofer 
> ---
>  include/odp/api/spec/packet_io.h | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/include/odp/api/spec/packet_io.h
> b/include/odp/api/spec/packet_io.h
> index 466cab6..60fb722 100644
> --- a/include/odp/api/spec/packet_io.h
> +++ b/include/odp/api/spec/packet_io.h
> @@ -908,6 +908,26 @@ int odp_pktio_skip_set(odp_pktio_t pktio, uint32_t
> offset);
>  int odp_pktio_headroom_set(odp_pktio_t pktio, uint32_t headroom);
> 
>  /**
> + * Get pktio id from handle
> + *
> + * @param pktio   Pktio handle
> + *
> + * @returnPktio index (0..odp_config_pktio_entries()-1)
> + * @retval <0 Error (e.g., handle not valid)
> + */
> +int odp_pktio_to_id(odp_pktio_t pktio);
> +
> +/**
> + * Get pktio handle from id
> + *
> + * @param id   Pktio index
> + *
> + * @return Pktio handle
> + * @retval ODP_PKTIO_INVALID on error
> + */
> +odp_pktio_t odp_pktio_from_id(int id);
> +
> +/**
>   * Get printable value for an odp_pktio_t
>   *
>   * @param pktio   odp_pktio_t handle to be printed
> --
> 2.5.0
> 
> ___
> 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] [PATCH] validation: time: shorten test duration

[Petri Savolainen]

Time test duration was almost a minute on a 2.6GHz CPU.
Combined local and global time monotonity tests, so that
the there is only single run of the long wait loop.
Shortened wait test time to 3 seconds and long loop into
roughly 14 seconds on a 2.6GHz CPU. There should be enough
head room to keep loop duration over 4 seconds even with
higher CPU frequencies.

Signed-off-by: Petri Savolainen 
---
 test/validation/time/time.c | 63 -
 test/validation/time/time.h |  3 +--
 2 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/test/validation/time/time.c b/test/validation/time/time.c
index cb1ddef..5e55817 100644
--- a/test/validation/time/time.c
+++ b/test/validation/time/time.c
@@ -9,10 +9,11 @@
 #include "time.h"
 
 #define BUSY_LOOP_CNT  3000/* used for t > min resolution */
-#define BUSY_LOOP_CNT_LONG 120 /* used for t > 4 sec */
+#define BUSY_LOOP_CNT_LONG 60  /* used for t > 4 sec */
 #define MIN_TIME_RATE  32000
 #define MAX_TIME_RATE  150
 #define DELAY_TOLERANCE2000/* deviation for 
delay */
+#define WAIT_SECONDS3
 
 static uint64_t local_res;
 static uint64_t global_res;
@@ -98,42 +99,45 @@ void time_test_global_conversion(void)
time_test_conversion(odp_time_global_from_ns, global_res);
 }
 
-static void time_test_monotony(time_cb time)
+void time_test_monotony(void)
 {
volatile uint64_t count = 0;
-   odp_time_t t1, t2, t3;
+   odp_time_t l_t1, l_t2, l_t3;
+   odp_time_t g_t1, g_t2, g_t3;
uint64_t ns1, ns2, ns3;
 
-   t1 = time();
+   l_t1 = odp_time_local();
+   g_t1 = odp_time_global();
 
while (count < BUSY_LOOP_CNT) {
count++;
};
 
-   t2 = time();
+   l_t2 = odp_time_local();
+   g_t2 = odp_time_global();
 
while (count < BUSY_LOOP_CNT_LONG) {
count++;
};
 
-   t3 = time();
+   l_t3 = odp_time_local();
+   g_t3 = odp_time_global();
 
-   ns1 = odp_time_to_ns(t1);
-   ns2 = odp_time_to_ns(t2);
-   ns3 = odp_time_to_ns(t3);
+   ns1 = odp_time_to_ns(l_t1);
+   ns2 = odp_time_to_ns(l_t2);
+   ns3 = odp_time_to_ns(l_t3);
 
+   /* Local time assertions */
CU_ASSERT(ns2 > ns1);
CU_ASSERT(ns3 > ns2);
-}
 
-void time_test_local_monotony(void)
-{
-   time_test_monotony(odp_time_local);
-}
+   ns1 = odp_time_to_ns(g_t1);
+   ns2 = odp_time_to_ns(g_t2);
+   ns3 = odp_time_to_ns(g_t3);
 
-void time_test_global_monotony(void)
-{
-   time_test_monotony(odp_time_global);
+   /* Global time assertions */
+   CU_ASSERT(ns2 > ns1);
+   CU_ASSERT(ns3 > ns2);
 }
 
 static void time_test_cmp(time_cb time, time_from_ns_cb time_from_ns)
@@ -324,16 +328,18 @@ static void time_test_wait_until(time_cb time, 
time_from_ns_cb time_from_ns)
 
start_time = time();
wait = start_time;
-   for (i = 1; i < 6; i++) {
+   for (i = 0; i < WAIT_SECONDS; i++) {
wait = odp_time_sum(wait, second);
odp_time_wait_until(wait);
-   printf("%d..", i);
+   printf("%d..", i + 1);
}
end_time = time();
 
wait = odp_time_diff(end_time, start_time);
-   lower_limit = time_from_ns(5 * ODP_TIME_SEC_IN_NS - DELAY_TOLERANCE);
-   upper_limit = time_from_ns(5 * ODP_TIME_SEC_IN_NS + DELAY_TOLERANCE);
+   lower_limit = time_from_ns(WAIT_SECONDS * ODP_TIME_SEC_IN_NS -
+  DELAY_TOLERANCE);
+   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);
@@ -356,18 +362,18 @@ void time_test_wait_ns(void)
odp_time_t start_time, end_time, diff;
 
start_time = odp_time_local();
-   for (i = 1; i < 6; i++) {
+   for (i = 0; i < WAIT_SECONDS; i++) {
odp_time_wait_ns(ODP_TIME_SEC_IN_NS);
-   printf("%d..", i);
+   printf("%d..", i + 1);
}
end_time = odp_time_local();
 
diff = odp_time_diff(end_time, start_time);
 
-   lower_limit = odp_time_local_from_ns(5 * ODP_TIME_SEC_IN_NS -
-   DELAY_TOLERANCE);
-   upper_limit = odp_time_local_from_ns(5 * ODP_TIME_SEC_IN_NS +
-   DELAY_TOLERANCE);
+   lower_limit = odp_time_local_from_ns(WAIT_SECONDS * ODP_TIME_SEC_IN_NS -
+DELAY_TOLERANCE);
+   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

Re: [lng-odp] [PATCH] validation: pktio: remove octet check from stats test

[Zoltan Kiss]

Ping

On 06/04/16 11:43, Zoltan Kiss wrote:



On 05/04/16 17:16, Zoltan Kiss wrote:

This test sets up two interface and connect them to each other, so in


s/interface/interfaces/

Maxim, could you fix that when commit? (assuming the patch is OK)

And an another note: Maxim told me when I brought this up first to
disable stuff, and I haven't replied then. The problem is you can't
possibly disable all OS services which tries to hook into the interface
up notification, because it is a moving target.


theory these two numbers should be the same. However when you use a pktio
which doesn't have full control of the interface, it could happen that
other players, e.g. various services of the operating system start to
send traffic out on the newly created interfaces. It won't be visible
for ODP when going out, but coming in it will increase the counters.
This breaks the test on ODP-DPDK, unnecessarily. On ODP-Linux it does
not,
because it checks the system level statistics, not the ODP level ones.

Signed-off-by: Zoltan Kiss 
---
diff --git a/test/validation/pktio/pktio.c
b/test/validation/pktio/pktio.c
index cb403a6..73b702c 100644
--- a/test/validation/pktio/pktio.c
+++ b/test/validation/pktio/pktio.c
@@ -1256,7 +1256,6 @@ void pktio_test_statistics_counters(void)
  CU_ASSERT((stats[1].in_ucast_pkts == 0) ||
(stats[1].in_ucast_pkts >= (uint64_t)pkts));
  CU_ASSERT(stats[0].out_ucast_pkts == stats[1].in_ucast_pkts);
-CU_ASSERT(stats[0].out_octets == stats[1].in_octets);
  CU_ASSERT((stats[0].out_octets == 0) ||
(stats[0].out_octets >=
(PKT_LEN_NORMAL * (uint64_t)pkts)));


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

[lng-odp] [Bug 2168] platform/linux-generic/odp_init.c: stage is never used

[bugzilla-daemon]

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

Bill Fischofer  changed:

   What|Removed |Added

   Assignee|lng-odp@lists.linaro.org|bill.fischo...@linaro.org

--- Comment #3 from Bill Fischofer  ---
Taking ownership of this bug.

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

[lng-odp] [Bug 2160] Building natively on AARch64 fails

[bugzilla-daemon]

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

Mike Holmes  changed:

   What|Removed |Added

   Assignee|lng-odp@lists.linaro.org|anders.rox...@linaro.org

-- 
You are receiving this mail because:
You are the assignee for the bug.
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

[lng-odp] [Bug 2160] Building natively on AARch64 fails

[bugzilla-daemon]

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

--- Comment #1 from Anders Roxell  ---
patch posted on the list [1].


[1] https://lists.linaro.org/pipermail/lng-odp/2016-April/021962.html

-- 
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

[lng-odp] [Bug 2139] CID 158529: Dereference after null check: timer.c

[bugzilla-daemon]

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

--- Comment #1 from Mike Holmes  ---
Ivan, any progress on this ?

-- 
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] linux-generic: queue: avoid false positive when compiling with -Og

[Bill Fischofer]

ping.  Maxim, I believe this patch is the simplest solution to the issue.

On Fri, Apr 8, 2016 at 4:06 PM, Bill Fischofer 
wrote:

> After reviewing the code again, I think the patch stands as the simplest
> solution to this compiler bug. Both type and handle are needed because they
> are cached from the queue_entry while it is locked and then referenced
> after it is unlocked. A direct reference the internal fields is not legal
> unless the queue_entry lock is held.
>
> On Thu, Apr 7, 2016 at 1:38 AM, Maxim Uvarov 
> wrote:
>
>> Mike, it think it's better to remove handle and type completely. And if
>> you already looking to that
>> function it might be reasonable to make it more readable, use
>> odp_config_queues() and use int
>> instead of uint32_t. Something like that:
>>
>>
>> --- a/platform/linux-generic/odp_queue.c
>> +++ b/platform/linux-generic/odp_queue.c
>> @@ -234,10 +234,8 @@ int odp_queue_lock_count(odp_queue_t handle)
>>
>>  odp_queue_t odp_queue_create(const char *name, const odp_queue_param_t
>> *param)
>>  {
>> -   uint32_t i;
>> +   int i;
>> queue_entry_t *queue;
>> -   odp_queue_t handle = ODP_QUEUE_INVALID;
>> -   odp_queue_type_t type;
>> odp_queue_param_t default_param;
>>
>> if (param == NULL) {
>> @@ -245,7 +243,7 @@ odp_queue_t odp_queue_create(const char *name, const
>> odp_queue_param_t *param)
>> param = &default_param;
>> }
>>
>> -   for (i = 0; i < ODP_CONFIG_QUEUES; i++) {
>> +   for (i = 0; i < odp_config_queues(); i++) {
>> queue = &queue_tbl->queue[i];
>>
>> if (queue->s.status != QUEUE_STATUS_FREE)
>> @@ -253,33 +251,29 @@ odp_queue_t odp_queue_create(const char *name,
>> const odp_queue_param_t *param)
>>
>> LOCK(&queue->s.lock);
>> if (queue->s.status == QUEUE_STATUS_FREE) {
>> -   if (queue_init(queue, name, param)) {
>> +   if (queue_init(queue, name, param) ||
>> +   queue->s.handle == ODP_QUEUE_INVALID) {
>> UNLOCK(&queue->s.lock);
>> -   return handle;
>> +   return ODP_QUEUE_INVALID;
>> }
>>
>> -   type = queue->s.type;
>> -
>> -   if (type == ODP_QUEUE_TYPE_SCHED)
>> +   if (queue->s.type == ODP_QUEUE_TYPE_SCHED) {
>> queue->s.status = QUEUE_STATUS_NOTSCHED;
>> -   else
>> +   if (schedule_queue_init(queue)) {
>> +   ODP_ERR("schedule queue init
>> failed\n");
>> + UNLOCK(&queue->s.lock);
>> +   return ODP_QUEUE_INVALID;
>> +   }
>> +   } else {
>> queue->s.status = QUEUE_STATUS_READY;
>> -
>> -   handle = queue->s.handle;
>> +   }
>> UNLOCK(&queue->s.lock);
>> -   break;
>> +   return queue->s.handle;
>> }
>> UNLOCK(&queue->s.lock);
>> }
>>
>> -   if (handle != ODP_QUEUE_INVALID && type == ODP_QUEUE_TYPE_SCHED) {
>> -   if (schedule_queue_init(queue)) {
>> -   ODP_ERR("schedule queue init failed\n");
>> -   return ODP_QUEUE_INVALID;
>> -   }
>> -   }
>> -
>> -   return handle;
>> +   return ODP_QUEUE_INVALID;
>>  }
>>
>> Maxim.
>>
>> On 04/05/16 21:02, Mike Holmes wrote:
>>
>>> Any one object to this?
>>> It impacts the accuracy of the ABI testing tools if -Og cannot be used.
>>>
>>> On 1 April 2016 at 19:03, Bill Fischofer >> > wrote:
>>>
>>> Resolve Bug https://bugs.linaro.org/show_bug.cgi?id=2159 by adding
>>> an
>>> extraneous variable initialization to avoid a false positive error
>>> when
>>> compiling with gcc -Og
>>>
>>> Signed-off-by: Bill Fischofer >> >
>>> ---
>>>  platform/linux-generic/odp_queue.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/platform/linux-generic/odp_queue.c
>>> b/platform/linux-generic/odp_queue.c
>>> index 342ffa2..5963057 100644
>>> --- a/platform/linux-generic/odp_queue.c
>>> +++ b/platform/linux-generic/odp_queue.c
>>> @@ -237,7 +237,7 @@ odp_queue_t odp_queue_create(const char *name,
>>> const odp_queue_param_t *param)
>>> uint32_t i;
>>> queue_entry_t *queue;
>>> odp_queue_t handle = ODP_QUEUE_INVALID;
>>> -   odp_queue_type_t type;
>>> +   odp_queue_type_t type = ODP_QUEUE_TYPE_PLAIN;
>>> odp_queue_param_t default_param;
>>>
>>> if (param == NULL) {
>>> --
>>> 2.5.

Re: [lng-odp] [PATCHv4] linux-generic: pktio: avoid coverity issues by adding explicit rc check

[Bill Fischofer]

ping.  v5 needs review.

On Mon, Apr 4, 2016 at 11:14 AM, Bill Fischofer 
wrote:

> OK, v5 sent with that suggested change.
>
> On Mon, Apr 4, 2016 at 10:26 AM, Maxim Uvarov 
> wrote:
>
>> On 04/04/16 17:42, Bill Fischofer wrote:
>>
>>> Resolve bug https://bugs.linaro.org/show_bug.cgi?id=2138 by adding an
>>> explicit rc check for odp_pktio_capability().
>>>
>>> Signed-off-by: Bill Fischofer 
>>> ---
>>>   platform/linux-generic/odp_packet_io.c | 24 ++--
>>>   1 file changed, 18 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/platform/linux-generic/odp_packet_io.c
>>> b/platform/linux-generic/odp_packet_io.c
>>> index 9192be2..b0f3423 100644
>>> --- a/platform/linux-generic/odp_packet_io.c
>>> +++ b/platform/linux-generic/odp_packet_io.c
>>> @@ -1027,6 +1027,7 @@ int odp_pktin_queue_config(odp_pktio_t pktio,
>>> odp_pktio_capability_t capa;
>>> unsigned num_queues;
>>> unsigned i;
>>> +   int rc;
>>> odp_queue_t queue;
>>> odp_pktin_queue_param_t default_param;
>>>   @@ -1059,10 +1060,15 @@ int odp_pktin_queue_config(odp_pktio_t pktio,
>>> return -1;
>>> }
>>>   - odp_pktio_capability(pktio, &capa);
>>> +   rc = odp_pktio_capability(pktio, &capa);
>>>   - if (num_queues > capa.max_input_queues) {
>>> -   ODP_DBG("pktio %s: too many input queues\n", entry->
>>> s.name);
>>> +   if (rc != 0 || num_queues > capa.max_input_queues) {
>>> +   if (rc != 0)
>>> +   ODP_DBG("pktio %s: unable to read
>>> capabilities\n",
>>> +   entry->s.name);
>>> +   else
>>> +   ODP_DBG("pktio %s: too many input queues\n",
>>> +   entry->s.name);
>>> return -1;
>>>
>>
>> it's that match simple to read?:
>>
>> rc = odp_pktio_capability(pktio, &capa);
>> if (rc) {
>> ODP_DBG("pktio %s: unable to read capabilities\n",
>> entry->s.name);
>> return -1;
>> }
>>
>> if (num_queues > capa.max_input_queues) {
>> ODP_DBG("pktio %s: unable to read capabilities\n", + entry->
>> s.name);
>> return -1;
>> }
>>
>> }
>>>   @@ -1135,6 +1141,7 @@ int odp_pktout_queue_config(odp_pktio_t pktio,
>>> odp_pktio_capability_t capa;
>>> unsigned num_queues;
>>> unsigned i;
>>> +   int rc;
>>> odp_pktout_queue_param_t default_param;
>>> if (param == NULL) {
>>> @@ -1172,10 +1179,15 @@ int odp_pktout_queue_config(odp_pktio_t pktio,
>>> return -1;
>>> }
>>>   - odp_pktio_capability(pktio, &capa);
>>> +   rc = odp_pktio_capability(pktio, &capa);
>>>   - if (num_queues > capa.max_output_queues) {
>>> -   ODP_DBG("pktio %s: too many output queues\n", entry->
>>> s.name);
>>> +   if (rc != 0 || num_queues > capa.max_output_queues) {
>>> +   if (rc != 0)
>>> +   ODP_DBG("pktio %s: unable to read
>>> capabilities\n",
>>> +   entry->s.name);
>>> +   else
>>> +   ODP_DBG("pktio %s: too many output queues\n",
>>> +   entry->s.name);
>>> return -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] [PATCHv2] linux-generic: test: shmem: close fifo

[Christophe Milard]

Ping...

On 1 April 2016 at 11:47, Christophe Milard 
wrote:

> Fixes: https://bugs.linaro.org/show_bug.cgi?id=2147 (CID 159394)
> The fifo is closed at end of test
>
> Signed-off-by: Christophe Milard 
> ---
>  since v1: bug URL added in commit msg (Anders)
>
>  platform/linux-generic/test/shmem/shmem_odp.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/platform/linux-generic/test/shmem/shmem_odp.c
> b/platform/linux-generic/test/shmem/shmem_odp.c
> index df1d3ff..a1f750f 100644
> --- a/platform/linux-generic/test/shmem/shmem_odp.c
> +++ b/platform/linux-generic/test/shmem/shmem_odp.c
> @@ -48,6 +48,7 @@ void shmem_test_odp_shm_proc(void)
> CU_ASSERT_FATAL(fd >= 0);
>
> CU_ASSERT(read(fd, &test_result, sizeof(char)) == 1);
> +   close(fd);
> CU_ASSERT_FATAL(test_result == TEST_SUCCESS);
>
> CU_ASSERT(odp_shm_free(shm) == 0);
> --
> 2.1.4
>
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [Bug 2161] Configure script fails in Cunit check

[bugzilla-daemon]

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

--- Comment #1 from Anders Roxell  ---
patch posted to the list and pushed: "6c83e1e - validation: configure: move
cflags guards to test/m4"

-- 
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] linux-generic: test: shmem: coverity fix

[Christophe Milard]

Ping: time for merge?

On 4 April 2016 at 18:23, Bill Fischofer  wrote:

>
>
> On Mon, Apr 4, 2016 at 4:29 AM, Christophe Milard <
> christophe.mil...@linaro.org> wrote:
>
>> Fixes: https://bugs.linaro.org/show_bug.cgi?id=2148 (CID 159393)
>> The if statement introduced here is redundant with the previous
>> CU_ASSERT_FATAL, but avoids the coverity warning.
>> (coverity probably misses the longjump in CU_ASSERT_FATAL)
>>
>> Signed-off-by: Christophe Milard 
>>
>
> Reviewed-by: Bill Fischofer 
>
>
>> ---
>>
>> NOTE: to be applied on to of: "linux-generic: test: shmem: close fifo"
>>
>>  platform/linux-generic/test/shmem/shmem_odp.c | 8 +---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/platform/linux-generic/test/shmem/shmem_odp.c
>> b/platform/linux-generic/test/shmem/shmem_odp.c
>> index a1f750f..77c5a01 100644
>> --- a/platform/linux-generic/test/shmem/shmem_odp.c
>> +++ b/platform/linux-generic/test/shmem/shmem_odp.c
>> @@ -47,9 +47,11 @@ void shmem_test_odp_shm_proc(void)
>> fd = open(fifo_name, O_RDONLY);
>> CU_ASSERT_FATAL(fd >= 0);
>>
>> -   CU_ASSERT(read(fd, &test_result, sizeof(char)) == 1);
>> -   close(fd);
>> -   CU_ASSERT_FATAL(test_result == TEST_SUCCESS);
>> +   if (fd >= 0) {  /* redundant, but to avoid coverity CID 159393 */
>> +   CU_ASSERT_FATAL(read(fd, &test_result, sizeof(char)) ==
>> 1);
>> +   close(fd);
>> +   CU_ASSERT_FATAL(test_result == TEST_SUCCESS);
>> +   }
>>
>> CU_ASSERT(odp_shm_free(shm) == 0);
>>  }
>> --
>> 2.1.4
>>
>>
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [Bug 2161] Configure script fails in Cunit check

[bugzilla-daemon]

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

Mike Holmes  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|UNCONFIRMED |RESOLVED
 CC||mike.hol...@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] [PATCHv2] linux-generic: pktio: handle transient output queue nonempty conditions

[Bill Fischofer]

ping.  This still needs a merge.

On Mon, Apr 4, 2016 at 3:02 PM, Maxim Uvarov 
wrote:

> In other termination code we have ODP_ERR().  For mostly all things which
> is called from odp_term_global.
> But I like current ASSERT() more but it's more easy to debug that code and
> clean up such things then follow
> bunch of error debug prints. If no objections I'm going to apply this
> simple patch.
>
> Maxim.
>
>
> On 04/02/16 16:52, Bill Fischofer wrote:
>
>> Out queues should be empty. Add an assert to verify this to ensure
>> that memory is not leaked as part of destroy_out_queues(). This addresses
>> bug https://bugs.linaro.org/show_bug.cgi?id=2089
>>
>> Signed-off-by: Bill Fischofer 
>> ---
>> Changes for v2:
>>
>> Switch to ODP_ASSERT() rather than risk looping in the unlikely event that
>> an output queue is non-empty. This prevents the memory leak while catching
>> the bug condition that would have resulted in the leak without this check.
>>
>>   platform/linux-generic/odp_packet_io.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/platform/linux-generic/odp_packet_io.c
>> b/platform/linux-generic/odp_packet_io.c
>> index 9192be2..7b1b137 100644
>> --- a/platform/linux-generic/odp_packet_io.c
>> +++ b/platform/linux-generic/odp_packet_io.c
>> @@ -296,11 +296,12 @@ static void destroy_in_queues(pktio_entry_t *entry,
>> int num)
>> static void destroy_out_queues(pktio_entry_t *entry, int num)
>>   {
>> -   int i;
>> +   int i, rc;
>> for (i = 0; i < num; i++) {
>> if (entry->s.out_queue[i].queue != ODP_QUEUE_INVALID) {
>> -   odp_queue_destroy(entry->s.out_queue[i].queue);
>> +   rc =
>> odp_queue_destroy(entry->s.out_queue[i].queue);
>> +   ODP_ASSERT(rc == 0);
>> entry->s.out_queue[i].queue = ODP_QUEUE_INVALID;
>> }
>> }
>>
>
> ___
> 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] [API-NEXT PATCH v2 1/7] api: packet: add extend and trunc

[Balasubramanian Manoharan]

These changes looks good to me.
Reviewed-by: Balasubramanian Manoharan 

On Thursday 14 April 2016 01:41 PM, Savolainen, Petri (Nokia - FI/Espoo) 
wrote:


Thanks for the review. Although, I’d prefer to get all 6 patches 
merged in one go, since reordering of these will cause conflicts. I’d 
continue to work locally on top of this set. Also the implementation 
patch set should not include these, but go on top. It’s hard to see if 
something was changed in the other set. Bill’s signed-off-by hints 
that something was changed but I guess nothing was changed, except the 
ordering compared to patches 3 and 5.


Bala, are you OK with patches 3 and 5?

-Petri

*From:*EXT Bill Fischofer [mailto:bill.fischo...@linaro.org]
*Sent:* Wednesday, April 13, 2016 5:29 PM
*To:* Savolainen, Petri (Nokia - FI/Espoo) 
*Cc:* LNG ODP Mailman List 
*Subject:* Re: [lng-odp] [API-NEXT PATCH v2 1/7] api: packet: add 
extend and trunc


For parts 1, 2, 4, and 6

Reviewed-by: Bill Fischofer >


I'm OK with merging those parts into api-next to allow parallel 
implementation/test work on them. I believe we  still need to get more 
input and feedback on parts 3 and 5 before these can be merged.


On Wed, Apr 13, 2016 at 9:19 AM, Bill Fischofer 
mailto:bill.fischo...@linaro.org>> wrote:


This latest definition looks good. I'll post a v4 of my
implementation patch series to reflect it.

On Wed, Apr 13, 2016 at 8:01 AM, Petri Savolainen
mailto:petri.savolai...@nokia.com>>
wrote:

Added functions to extend / truncate packet head / tail more
than current head/tailroom or segment lengths.

Signed-off-by: Petri Savolainen mailto:petri.savolai...@nokia.com>>
---
 include/odp/api/spec/packet.h | 144
++
 1 file changed, 144 insertions(+)

diff --git a/include/odp/api/spec/packet.h
b/include/odp/api/spec/packet.h
index 7da353b..6c88458 100644
--- a/include/odp/api/spec/packet.h
+++ b/include/odp/api/spec/packet.h
@@ -406,6 +406,150 @@ void *odp_packet_push_tail(odp_packet_t
pkt, uint32_t len);
 void *odp_packet_pull_tail(odp_packet_t pkt, uint32_t len);

 /**
+ * Extend packet head
+ *
+ * Increase packet data length at packet head. Functionality
is analogous to
+ * odp_packet_push_head() when data length is extended up to
headroom size.
+ * When data length is increased more than that, new segments
are added into
+ * the packet head and old segment handles become invalid.
+ *
+ * A successful operation overwrites the packet handle with a
new handle, which
+ * application must use as the reference to the packet
instead of the old
+ * handle. Depending on the implementation, the old and new
handles may be
+ * equal.
+ *
+ * The operation return value indicates if any packet data or
metadata (e.g.
+ * user_area) were moved in memory during the operation. If
some memory areas
+ * were moved, application must use new packet/segment
handles to update
+ * data pointers. Otherwise, all old pointers remain valid.
+ *
+ * User is responsible to update packet metadata offsets when
needed. Packet
+ * is not modified if operation fails.
+ *
+ * @param[in, out] pkt  Pointer to packet handle. A
successful operation outputs
+ *  the new packet handle.
+ * @param len   Number of bytes to extend the head
+ * @param[out] data_ptr Pointer to output the new data pointer.
+ *  Ignored when NULL.
+ * @param[out] seg_len  Pointer to output segment length at
'data_ptr' above.
+ *  Ignored when NULL.
+ *
+ * @retval 0   Operation successful, old pointers remain valid
+ * @retval >0  Operation successful, old pointers need to be
updated
+ * @retval <0  Operation failed (e.g. due to an allocation
failure)
+ */
+int odp_packet_extend_head(odp_packet_t *pkt, uint32_t len,
void **data_ptr,
+  uint32_t *seg_len);
+
+/**
+ * Truncate packet head
+ *
+ * Decrease packet data length at packet head. Functionality
is analogous to
+ * odp_packet_pull_head() when data length is truncated less
than the first
+ * segment data length. When data length is decreased more
than that, some head
+ * segments are removed from the packet and old segment
handles become invalid.
+ *
+ * A successful operation overwrites the packet handle with a
new handle, which
+ * application

[lng-odp] [Bug 2103] netmap is not tested under CI

[bugzilla-daemon]

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

--- Comment #3 from Mike Holmes  ---
Discussion in ARCH call yesterday resulted in Maxim looking at adding this to
check-odp scripts and reusing the framework there to test the independent
odp-dpdk

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

[lng-odp] [Bug 2090] example/timer/odp_timer_test.c

[bugzilla-daemon]

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

--- Comment #1 from Mike Holmes  ---
Hi Ivan, any progress ?

-- 
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] linux-generic: test: shmem: close fifo

[Mike Holmes]

On 1 April 2016 at 05:47, Christophe Milard 
wrote:

> Fixes: https://bugs.linaro.org/show_bug.cgi?id=2147 (CID 159394)
> The fifo is closed at end of test
>
> Signed-off-by: Christophe Milard 
>

Reviewed-by: Mike Holmes 


> ---
>  since v1: bug URL added in commit msg (Anders)
>
>  platform/linux-generic/test/shmem/shmem_odp.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/platform/linux-generic/test/shmem/shmem_odp.c
> b/platform/linux-generic/test/shmem/shmem_odp.c
> index df1d3ff..a1f750f 100644
> --- a/platform/linux-generic/test/shmem/shmem_odp.c
> +++ b/platform/linux-generic/test/shmem/shmem_odp.c
> @@ -48,6 +48,7 @@ void shmem_test_odp_shm_proc(void)
> CU_ASSERT_FATAL(fd >= 0);
>
> CU_ASSERT(read(fd, &test_result, sizeof(char)) == 1);
> +   close(fd);
> CU_ASSERT_FATAL(test_result == TEST_SUCCESS);
>
> CU_ASSERT(odp_shm_free(shm) == 0);
> --
> 2.1.4
>
>


-- 
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] [API-NEXT PATCH v2] doc/users-quide: correct timer API section

[Ivan Khoronzhuk]

Signed-off-by: Ivan Khoronzhuk 
---
v2..v1:
- just rebased with several not important corrections

 doc/users-guide/users-guide.adoc | 50 
 1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/doc/users-guide/users-guide.adoc b/doc/users-guide/users-guide.adoc
index a2e5058..69b1930 100644
--- a/doc/users-guide/users-guide.adoc
+++ b/doc/users-guide/users-guide.adoc
@@ -335,11 +335,51 @@ The +odp_time_t+ opaque type represents local or global 
timestamps.
 
 === Timer
 Timers are how ODP applications measure and respond to the passage of time.
-Timers are drawn from specialized pools called timer pools that have their
-own abstract type (+odp_timer_pool_t+). Applications may have many timers
-active at the same time and can set them to use either relative or absolute
-time. When timers expire they create events of type +odp_timeout_t+, which
-serve as notifications of timer expiration.
+The timer API is supposed to be used when time synchronization with events is
+needed and has different nature than time API has. Usually, timers require a
+separate h/w module to be used for generating timeouts. Timers are drawn from
+specialized pools called timer pools that have their own abstract type
+(+odp_timer_pool_t+). Applications may have many timers active at the same
+time and can set them to use either relative or absolute time. When timers
+expire they create events of type +odp_timeout_t+, which serve as notifications
+of timer expiration. The event is placed on a queue pointed while timer
+allocation.
+
+Each timer pool can be set with it's own resolution in ns and number of
+supported timers. So, timer pool can be considered as a time source with
+it's own resolution and defined number of timers. All timers in timer pool
+are handled with same time source with same resolution. If user needs two types
+of timers with different requirements for resolution then better to create
+two pools with it's own resolution, it can decrease load on hardware.
+
+An expiration time for the timer is set in it's own ticks, so nanoseconds have
+to be converted first with conversation function +odp_timer_ns_to_tick()+, to
+convert it back to ns use +odp_timer_tick_to_ns()+. Both functions require
+to pass a timer pool used, as it can be sourced with it's own time source that
+can have specific resolution and thus different conversion ratio.
+
+To set a timer to deliver a timeout event the two functions can be used:
++odp_timer_set_abs()+ and +odp_timer_set_rel()+. Both of them require an
+event to be passed and a time interval in ticks of corresponding timer pool.
+The expiration time for a timer can be set based on current tick value for
+a timer pool taken with +odp_timer_current_tick()+, to set a timer
+(absolute time) with a user-provided timeout event, the +odp_timer_set_abs()+
+can be used. An event is the event converted from timeout allocated from 
timeout
+pool with +odp_timeout_alloc()+. The event is returned to timer queue when set
+time interval has expired and can be also converted to timeout with
++odp_timeout_from_event()+, if it's needed. To free timeout the
++odp_timeout_free()+ can be used, if it's presented as event it can be freed
+as event with +odp_event_free()+.
+
+In general, timer pool characterizes time source, and timer is characterized
+with timer pool, user pointer and destination queue for delivering. The timeout
+is characterized with timer, timer pool and time of expiration and is placed to
+queue set while timer allocation. The timeout can be delivered only to the
+destination queue of concrete timer. To get a timer generated a timeout
++odp_timeout_timer()+ can be used.
+
+When timer is freed it's returned to timer pool and is ready to be allocated
+once again. A timer can be canceled with odp_timer_cancel.
 
 === Synchronizer
 Multiple threads operating in parallel typically require various
-- 
1.9.1

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

Re: [lng-odp] [PATCH] validation: scheduler: correct pause/resume sequence

[Ivan Khoronzhuk]

ping

On 18.02.16 17:56, Ivan Khoronzhuk wrote:

ping

On 16.02.16 16:02, Ivan Khoronzhuk wrote:

When test for single thread is finished the following test for
many threads can be started, and for some implementations can
happen that future one event can arrive to main thread, as it was
requested in previous test. As result one event can be lost for rest
threads. So, it's better to pause scheduling for main thread when
it doesn't participate in the multi-threaded test. Also move
pause/resume test closer to beginning, because it's used in tests
before.

Signed-off-by: Ivan Khoronzhuk 
---
  test/validation/scheduler/scheduler.c | 12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/test/validation/scheduler/scheduler.c 
b/test/validation/scheduler/scheduler.c
index dcf01c0..c1b61c5 100644
--- a/test/validation/scheduler/scheduler.c
+++ b/test/validation/scheduler/scheduler.c
@@ -1000,6 +1000,8 @@ static void schedule_common(odp_schedule_sync_t sync, int 
num_queues,
  args.enable_schd_multi = enable_schd_multi;
  args.enable_excl_atomic = 0;/* Not needed with a single CPU */

+/* resume scheduling in case it was paused */
+odp_schedule_resume();
  fill_queues(&args);

  schedule_common_(&args);
@@ -1037,6 +1039,9 @@ static void parallel_execute(odp_schedule_sync_t sync, 
int num_queues,
  args->enable_schd_multi = enable_schd_multi;
  args->enable_excl_atomic = enable_excl_atomic;

+/* disable receive events for main thread */
+exit_schedule_loop();
+
  fill_queues(args);

  /* Create and launch worker threads */
@@ -1249,6 +1254,9 @@ void scheduler_test_pause_resume(void)
  int i;
  int local_bufs = 0;

+/* resume scheduling in case it was paused */
+odp_schedule_resume();
+
  queue = odp_queue_lookup("sched_0_0_n");
  CU_ASSERT(queue != ODP_QUEUE_INVALID);

@@ -1296,6 +1304,8 @@ void scheduler_test_pause_resume(void)
  }

  CU_ASSERT(exit_schedule_loop() == 0);
+
+odp_schedule_resume();
  }

  static int create_queues(void)
@@ -1556,6 +1566,7 @@ odp_testinfo_t scheduler_suite[] = {
  ODP_TEST_INFO(scheduler_test_num_prio),
  ODP_TEST_INFO(scheduler_test_queue_destroy),
  ODP_TEST_INFO(scheduler_test_groups),
+ODP_TEST_INFO(scheduler_test_pause_resume),
  ODP_TEST_INFO(scheduler_test_parallel),
  ODP_TEST_INFO(scheduler_test_atomic),
  ODP_TEST_INFO(scheduler_test_ordered),
@@ -1586,7 +1597,6 @@ odp_testinfo_t scheduler_suite[] = {
  ODP_TEST_INFO(scheduler_test_multi_mq_mt_prio_a),
  ODP_TEST_INFO(scheduler_test_multi_mq_mt_prio_o),
  ODP_TEST_INFO(scheduler_test_multi_1q_mt_a_excl),
-ODP_TEST_INFO(scheduler_test_pause_resume),
  ODP_TEST_INFO_NULL,
  };






--
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: time: shorten test duration

[Ivan Khoronzhuk]

Hi, Petri.

As far I remember you was OK to increase time of testing on even more time.
It was supposed to include future freqs...I know, for now it's not correct..
but anyway. Also I dislike the idea to combine two separate tests in one 
function.
Yes, it makes tests faster, but doesn't reflect unit test nature. Also, Ok,
you've made test shorter supposing that freq can not be fast enough to exit the
4sec range, you've saved ~ minute, but why you decrease 5 sec to 3 sec, it 
helps to
see sense of time..and can be missed easily.

On 14.04.16 14:41, Petri Savolainen wrote:

Time test duration was almost a minute on a 2.6GHz CPU.
Combined local and global time monotonity tests, so that
the there is only single run of the long wait loop.
Shortened wait test time to 3 seconds and long loop into
roughly 14 seconds on a 2.6GHz CPU. There should be enough
head room to keep loop duration over 4 seconds even with
higher CPU frequencies.

Signed-off-by: Petri Savolainen 
---
  test/validation/time/time.c | 63 -
  test/validation/time/time.h |  3 +--
  2 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/test/validation/time/time.c b/test/validation/time/time.c
index cb1ddef..5e55817 100644
--- a/test/validation/time/time.c
+++ b/test/validation/time/time.c
@@ -9,10 +9,11 @@
  #include "time.h"

  #define BUSY_LOOP_CNT 3000/* used for t > min resolution */
-#define BUSY_LOOP_CNT_LONG 120 /* used for t > 4 sec */
+#define BUSY_LOOP_CNT_LONG 60  /* used for t > 4 sec */
  #define MIN_TIME_RATE 32000
  #define MAX_TIME_RATE 150
  #define DELAY_TOLERANCE   2000/* deviation for 
delay */
+#define WAIT_SECONDS3

  static uint64_t local_res;
  static uint64_t global_res;
@@ -98,42 +99,45 @@ void time_test_global_conversion(void)
time_test_conversion(odp_time_global_from_ns, global_res);
  }

-static void time_test_monotony(time_cb time)
+void time_test_monotony(void)
  {
volatile uint64_t count = 0;
-   odp_time_t t1, t2, t3;
+   odp_time_t l_t1, l_t2, l_t3;
+   odp_time_t g_t1, g_t2, g_t3;
uint64_t ns1, ns2, ns3;

-   t1 = time();
+   l_t1 = odp_time_local();
+   g_t1 = odp_time_global();

while (count < BUSY_LOOP_CNT) {
count++;
};

-   t2 = time();
+   l_t2 = odp_time_local();
+   g_t2 = odp_time_global();

while (count < BUSY_LOOP_CNT_LONG) {
count++;
};

-   t3 = time();
+   l_t3 = odp_time_local();
+   g_t3 = odp_time_global();

-   ns1 = odp_time_to_ns(t1);
-   ns2 = odp_time_to_ns(t2);
-   ns3 = odp_time_to_ns(t3);
+   ns1 = odp_time_to_ns(l_t1);
+   ns2 = odp_time_to_ns(l_t2);
+   ns3 = odp_time_to_ns(l_t3);

+   /* Local time assertions */
CU_ASSERT(ns2 > ns1);
CU_ASSERT(ns3 > ns2);
-}

-void time_test_local_monotony(void)
-{
-   time_test_monotony(odp_time_local);
-}
+   ns1 = odp_time_to_ns(g_t1);
+   ns2 = odp_time_to_ns(g_t2);
+   ns3 = odp_time_to_ns(g_t3);

-void time_test_global_monotony(void)
-{
-   time_test_monotony(odp_time_global);
+   /* Global time assertions */
+   CU_ASSERT(ns2 > ns1);
+   CU_ASSERT(ns3 > ns2);
  }

  static void time_test_cmp(time_cb time, time_from_ns_cb time_from_ns)
@@ -324,16 +328,18 @@ static void time_test_wait_until(time_cb time, 
time_from_ns_cb time_from_ns)

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

wait = odp_time_diff(end_time, start_time);
-   lower_limit = time_from_ns(5 * ODP_TIME_SEC_IN_NS - DELAY_TOLERANCE);
-   upper_limit = time_from_ns(5 * ODP_TIME_SEC_IN_NS + DELAY_TOLERANCE);
+   lower_limit = time_from_ns(WAIT_SECONDS * ODP_TIME_SEC_IN_NS -
+  DELAY_TOLERANCE);
+   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);
@@ -356,18 +362,18 @@ void time_test_wait_ns(void)
odp_time_t start_time, end_time, diff;

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

diff = odp_time_diff(end_time, start_time);

-   lower_limit = odp_time_local_from_ns(5 * ODP_TIME_SEC_IN_NS -
-  

Re: [lng-odp] [PATCHv2] linux-generic: test: shmem: close fifo

[Maxim Uvarov]

Merged,
Maxim.

On 04/14/16 16:34, Mike Holmes wrote:



On 1 April 2016 at 05:47, Christophe Milard 
mailto:christophe.mil...@linaro.org>> 
wrote:


Fixes: https://bugs.linaro.org/show_bug.cgi?id=2147 (CID 159394)
The fifo is closed at end of test

Signed-off-by: Christophe Milard mailto:christophe.mil...@linaro.org>>


Reviewed-by: Mike Holmes >


---
 since v1: bug URL added in commit msg (Anders)

 platform/linux-generic/test/shmem/shmem_odp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/platform/linux-generic/test/shmem/shmem_odp.c
b/platform/linux-generic/test/shmem/shmem_odp.c
index df1d3ff..a1f750f 100644
--- a/platform/linux-generic/test/shmem/shmem_odp.c
+++ b/platform/linux-generic/test/shmem/shmem_odp.c
@@ -48,6 +48,7 @@ void shmem_test_odp_shm_proc(void)
CU_ASSERT_FATAL(fd >= 0);

CU_ASSERT(read(fd, &test_result, sizeof(char)) == 1);
+   close(fd);
CU_ASSERT_FATAL(test_result == TEST_SUCCESS);

CU_ASSERT(odp_shm_free(shm) == 0);
--
2.1.4




--
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] [PATCH] pkg: remove packaging in ODP

[Anders Roxell]

Move the packaging scripts to its own git [1] repository.

[1] http://git.linaro.org/lng/odp-packaging.git

Signed-off-by: Anders Roxell 
---
 pkg/debian/README.Debian  |  12 -
 pkg/debian/changelog  | 566 --
 pkg/debian/compat |   1 -
 pkg/debian/control|  70 
 pkg/debian/copyright  |  31 --
 pkg/debian/docs   |   1 -
 pkg/debian/libodp-linux-dev.dirs  |   2 -
 pkg/debian/libodp-linux-dev.install   |   5 -
 pkg/debian/libodp-linux.dirs  |   1 -
 pkg/debian/libodp-linux.install   |   1 -
 pkg/debian/libodphelper-linux-dev.dirs|   2 -
 pkg/debian/libodphelper-linux-dev.install |   4 -
 pkg/debian/libodphelper-linux.dirs|   1 -
 pkg/debian/libodphelper-linux.install |   1 -
 pkg/debian/odp-linux-bin.dirs |   1 -
 pkg/debian/odp-linux-bin.install  |   1 -
 pkg/debian/rules  |  30 --
 pkg/debian/source/format  |   1 -
 pkg/rpm/odp.spec  |  80 -
 scripts/builddeb  |  38 --
 scripts/buildrpm  |  30 --
 scripts/common_pkg_build.sh   |  39 --
 22 files changed, 918 deletions(-)
 delete mode 100644 pkg/debian/README.Debian
 delete mode 100644 pkg/debian/changelog
 delete mode 100644 pkg/debian/compat
 delete mode 100644 pkg/debian/control
 delete mode 100644 pkg/debian/copyright
 delete mode 100644 pkg/debian/docs
 delete mode 100644 pkg/debian/libodp-linux-dev.dirs
 delete mode 100644 pkg/debian/libodp-linux-dev.install
 delete mode 100644 pkg/debian/libodp-linux.dirs
 delete mode 100644 pkg/debian/libodp-linux.install
 delete mode 100644 pkg/debian/libodphelper-linux-dev.dirs
 delete mode 100644 pkg/debian/libodphelper-linux-dev.install
 delete mode 100644 pkg/debian/libodphelper-linux.dirs
 delete mode 100644 pkg/debian/libodphelper-linux.install
 delete mode 100644 pkg/debian/odp-linux-bin.dirs
 delete mode 100644 pkg/debian/odp-linux-bin.install
 delete mode 100755 pkg/debian/rules
 delete mode 100644 pkg/debian/source/format
 delete mode 100644 pkg/rpm/odp.spec
 delete mode 100755 scripts/builddeb
 delete mode 100755 scripts/buildrpm
 delete mode 100644 scripts/common_pkg_build.sh

diff --git a/pkg/debian/README.Debian b/pkg/debian/README.Debian
deleted file mode 100644
index b8e47e4..000
--- a/pkg/debian/README.Debian
+++ /dev/null
@@ -1,12 +0,0 @@
-opendataplane for Debian
-
-
-For up to date information please visit http://www.opendataplane.org
-The OpenDataPlane (ODP) project has been established to produce an open-source,
-cross-platform set of application programming interfaces (APIs) for the
-networking data plane.
-
-It provides a library such as openvswitch with a portable API that facilitates
-platform independence and access to hardware acceleration.
-
- -- Anders Roxell   Mon, 22 Dec 2014 19:07:06 +0100
diff --git a/pkg/debian/changelog b/pkg/debian/changelog
deleted file mode 100644
index 4d8d9a8..000
--- a/pkg/debian/changelog
+++ /dev/null
@@ -1,566 +0,0 @@
-opendataplane (1.8.0.0-1) unstable; urgency=low
-   * ODP release v1.8
-
- -- Maxim Uvarov   Fri, 04 Mar 2016 13:32:05 +0300
-
-opendataplane (1.7.0.0-1) unstable; urgency=low
-   * ODP release v1.7
-
- -- Maxim Uvarov   Fri, 05 Feb 2016 13:05:00 +0300
-
-opendataplane (1.6.0.0-1) unstable; urgency=low
-   * ODP release v1.6
-
- -- Maxim Uvarov   Mon, 28 Dec 2015 17:01:58 +0300
-
-opendataplane (1.5.0.0-1) unstable; urgency=low
-   * ODP release v1.5
-
- -- Maxim Uvarov   Mon, 30 Nov 2015 13:08:43 +0300
-
-opendataplane (1.4.1.0-1) unstable; urgency=low
-   * Validation
-   - pktio: test transmit error recovery
-   - schedule: add chaos test
-   - check return code from odp_queue_lock_count()
-   - scheduler: test ordered queue reorder processing
-   - pktio: initialize queue parameters correctly
-   - pktio: test for transmit error handling
-   - pktio: add support for direct receive
-   - pktio: pass interface index rather than name
-   - pktio: fix start_stop test
-   - test: l2fwd: separate rx and tx drop counters
-   - test: l2fwd: increase burst size
-   - test: l2fwd: optimize statistics usage
-   - test: l2fwd: optimize queue mode
-   - test: l2fwd: start pktios after worker thread create
-   - test: l2fwd: added option to disable error check
-   - example/ipsec: Increase ip_data_len for Tunnel mode
-   - example: ipsec: check push_tail return code
-   * General:
-   - linux-generic: pktio: handle transmit errors correctly
-   - pktio socket_mmap: recover from transmit errors but 1890
-   - pktio: increase MTU of loop interface
-   - ordered queues: fix race condition during order release
- and out of order.
-   - configure: move HAVE_PCAP AM_CONDITIONAL to 

Re: [lng-odp] [PATCH] validation: time: shorten test duration

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

Hi,

I said back then that we need one test case that runs longer than 4 sec, other 
test cases can be shorter. This patch  does that - combines long lasting tests 
into single >4sec run.

The rationale for shorter duration is that many of us need to run 'make check' 
multiple times a day and the benefit from every second we can save is 
multiplied by the number users and test runs per user. This patch saves roughly 
40 seconds per user per test run.

Optimally, time validation test should be able to verify the API in a bit more 
than 4 seconds. Also a single, very long test case makes people wonder if the 
system hanged...

You could add a configure option for a longer test run, but by default the time 
test (or any other test) should not take a minute.

-Petri


> -Original Message-
> From: EXT Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
> Sent: Thursday, April 14, 2016 5:00 PM
> To: Savolainen, Petri (Nokia - FI/Espoo) ;
> lng-odp@lists.linaro.org
> Subject: Re: [lng-odp] [PATCH] validation: time: shorten test duration
> 
> Hi, Petri.
> 
> As far I remember you was OK to increase time of testing on even more
> time.
> It was supposed to include future freqs...I know, for now it's not
> correct..
> but anyway. Also I dislike the idea to combine two separate tests in one
> function.
> Yes, it makes tests faster, but doesn't reflect unit test nature. Also,
> Ok,
> you've made test shorter supposing that freq can not be fast enough to
> exit the
> 4sec range, you've saved ~ minute, but why you decrease 5 sec to 3 sec, it
> helps to
> see sense of time..and can be missed easily.
> 
> On 14.04.16 14:41, Petri Savolainen wrote:
> > Time test duration was almost a minute on a 2.6GHz CPU.
> > Combined local and global time monotonity tests, so that
> > the there is only single run of the long wait loop.
> > Shortened wait test time to 3 seconds and long loop into
> > roughly 14 seconds on a 2.6GHz CPU. There should be enough
> > head room to keep loop duration over 4 seconds even with
> > higher CPU frequencies.
> >
> > Signed-off-by: Petri Savolainen 
> > ---
> >   test/validation/time/time.c | 63 -
> 
> >   test/validation/time/time.h |  3 +--
> >   2 files changed, 35 insertions(+), 31 deletions(-)
> >
> > diff --git a/test/validation/time/time.c b/test/validation/time/time.c
> > index cb1ddef..5e55817 100644
> > --- a/test/validation/time/time.c
> > +++ b/test/validation/time/time.c
> > @@ -9,10 +9,11 @@
> >   #include "time.h"
> >
> >   #define BUSY_LOOP_CNT 3000/* used for t > min
> resolution */
> > -#define BUSY_LOOP_CNT_LONG 120 /* used for t > 4 sec */
> > +#define BUSY_LOOP_CNT_LONG 60  /* used for t > 4 sec */
> >   #define MIN_TIME_RATE 32000
> >   #define MAX_TIME_RATE 150
> >   #define DELAY_TOLERANCE   2000/* deviation for 
> > delay
> */
> > +#define WAIT_SECONDS3
> >
> >   static uint64_t local_res;
> >   static uint64_t global_res;
> > @@ -98,42 +99,45 @@ void time_test_global_conversion(void)
> > time_test_conversion(odp_time_global_from_ns, global_res);
> >   }
> >
> > -static void time_test_monotony(time_cb time)
> > +void time_test_monotony(void)
> >   {
> > volatile uint64_t count = 0;
> > -   odp_time_t t1, t2, t3;
> > +   odp_time_t l_t1, l_t2, l_t3;
> > +   odp_time_t g_t1, g_t2, g_t3;
> > uint64_t ns1, ns2, ns3;
> >
> > -   t1 = time();
> > +   l_t1 = odp_time_local();
> > +   g_t1 = odp_time_global();
> >
> > while (count < BUSY_LOOP_CNT) {
> > count++;
> > };
> >
> > -   t2 = time();
> > +   l_t2 = odp_time_local();
> > +   g_t2 = odp_time_global();
> >
> > while (count < BUSY_LOOP_CNT_LONG) {
> > count++;
> > };
> >
> > -   t3 = time();
> > +   l_t3 = odp_time_local();
> > +   g_t3 = odp_time_global();
> >
> > -   ns1 = odp_time_to_ns(t1);
> > -   ns2 = odp_time_to_ns(t2);
> > -   ns3 = odp_time_to_ns(t3);
> > +   ns1 = odp_time_to_ns(l_t1);
> > +   ns2 = odp_time_to_ns(l_t2);
> > +   ns3 = odp_time_to_ns(l_t3);
> >
> > +   /* Local time assertions */
> > CU_ASSERT(ns2 > ns1);
> > CU_ASSERT(ns3 > ns2);
> > -}
> >
> > -void time_test_local_monotony(void)
> > -{
> > -   time_test_monotony(odp_time_local);
> > -}
> > +   ns1 = odp_time_to_ns(g_t1);
> > +   ns2 = odp_time_to_ns(g_t2);
> > +   ns3 = odp_time_to_ns(g_t3);
> >
> > -void time_test_global_monotony(void)
> > -{
> > -   time_test_monotony(odp_time_global);
> > +   /* Global time assertions */
> > +   CU_ASSERT(ns2 > ns1);
> > +   CU_ASSERT(ns3 > ns2);
> >   }
> >
> >   static void time_test_cmp(time_cb time, time_from_ns_cb time_from_ns)
> > @@ -324,16 +328,18 @@ static void time_test_wait_until(time_cb time,
> time_from_ns_cb time_from_ns)
> >
> > start_time = time();
> > wait = start_time;
> > -   for (i = 1; i < 6; i++) {
> > +   for (i = 0; i < WAIT_SECONDS; i++) {
> > wait = od

Re: [lng-odp] [PATCH] pkg: remove packaging in ODP

[Ricardo Salveti]

On Thu, Apr 14, 2016 at 11:00 AM, Anders Roxell
 wrote:
> Move the packaging scripts to its own git [1] repository.
>
> [1] http://git.linaro.org/lng/odp-packaging.git
>
> Signed-off-by: Anders Roxell 
> ---
>  pkg/debian/README.Debian  |  12 -
>  pkg/debian/changelog  | 566 
> --
>  pkg/debian/compat |   1 -
>  pkg/debian/control|  70 
>  pkg/debian/copyright  |  31 --
>  pkg/debian/docs   |   1 -
>  pkg/debian/libodp-linux-dev.dirs  |   2 -
>  pkg/debian/libodp-linux-dev.install   |   5 -
>  pkg/debian/libodp-linux.dirs  |   1 -
>  pkg/debian/libodp-linux.install   |   1 -
>  pkg/debian/libodphelper-linux-dev.dirs|   2 -
>  pkg/debian/libodphelper-linux-dev.install |   4 -
>  pkg/debian/libodphelper-linux.dirs|   1 -
>  pkg/debian/libodphelper-linux.install |   1 -
>  pkg/debian/odp-linux-bin.dirs |   1 -
>  pkg/debian/odp-linux-bin.install  |   1 -
>  pkg/debian/rules  |  30 --
>  pkg/debian/source/format  |   1 -
>  pkg/rpm/odp.spec  |  80 -
>  scripts/builddeb  |  38 --
>  scripts/buildrpm  |  30 --
>  scripts/common_pkg_build.sh   |  39 --
>  22 files changed, 918 deletions(-)
>  delete mode 100644 pkg/debian/README.Debian
>  delete mode 100644 pkg/debian/changelog
>  delete mode 100644 pkg/debian/compat
>  delete mode 100644 pkg/debian/control
>  delete mode 100644 pkg/debian/copyright
>  delete mode 100644 pkg/debian/docs
>  delete mode 100644 pkg/debian/libodp-linux-dev.dirs
>  delete mode 100644 pkg/debian/libodp-linux-dev.install
>  delete mode 100644 pkg/debian/libodp-linux.dirs
>  delete mode 100644 pkg/debian/libodp-linux.install
>  delete mode 100644 pkg/debian/libodphelper-linux-dev.dirs
>  delete mode 100644 pkg/debian/libodphelper-linux-dev.install
>  delete mode 100644 pkg/debian/libodphelper-linux.dirs
>  delete mode 100644 pkg/debian/libodphelper-linux.install
>  delete mode 100644 pkg/debian/odp-linux-bin.dirs
>  delete mode 100644 pkg/debian/odp-linux-bin.install
>  delete mode 100755 pkg/debian/rules
>  delete mode 100644 pkg/debian/source/format
>  delete mode 100644 pkg/rpm/odp.spec
>  delete mode 100755 scripts/builddeb
>  delete mode 100755 scripts/buildrpm
>  delete mode 100644 scripts/common_pkg_build.sh
>
> diff --git a/pkg/debian/README.Debian b/pkg/debian/README.Debian
> deleted file mode 100644
> index b8e47e4..000
> --- a/pkg/debian/README.Debian
> +++ /dev/null
> @@ -1,12 +0,0 @@
> -opendataplane for Debian
> -
> -
> -For up to date information please visit http://www.opendataplane.org
> -The OpenDataPlane (ODP) project has been established to produce an 
> open-source,
> -cross-platform set of application programming interfaces (APIs) for the
> -networking data plane.
> -
> -It provides a library such as openvswitch with a portable API that 
> facilitates
> -platform independence and access to hardware acceleration.
> -
> - -- Anders Roxell   Mon, 22 Dec 2014 19:07:06 +0100
> diff --git a/pkg/debian/changelog b/pkg/debian/changelog
> deleted file mode 100644
> index 4d8d9a8..000
> --- a/pkg/debian/changelog
> +++ /dev/null
> @@ -1,566 +0,0 @@
> -opendataplane (1.8.0.0-1) unstable; urgency=low
> -   * ODP release v1.8
> -
> - -- Maxim Uvarov   Fri, 04 Mar 2016 13:32:05 +0300
> -
> -opendataplane (1.7.0.0-1) unstable; urgency=low
> -   * ODP release v1.7
> -
> - -- Maxim Uvarov   Fri, 05 Feb 2016 13:05:00 +0300
> -
> -opendataplane (1.6.0.0-1) unstable; urgency=low
> -   * ODP release v1.6
> -
> - -- Maxim Uvarov   Mon, 28 Dec 2015 17:01:58 +0300
> -
> -opendataplane (1.5.0.0-1) unstable; urgency=low
> -   * ODP release v1.5
> -
> - -- Maxim Uvarov   Mon, 30 Nov 2015 13:08:43 +0300
> -
> -opendataplane (1.4.1.0-1) unstable; urgency=low
> -   * Validation
> -   - pktio: test transmit error recovery
> -   - schedule: add chaos test
> -   - check return code from odp_queue_lock_count()
> -   - scheduler: test ordered queue reorder processing
> -   - pktio: initialize queue parameters correctly
> -   - pktio: test for transmit error handling
> -   - pktio: add support for direct receive
> -   - pktio: pass interface index rather than name
> -   - pktio: fix start_stop test
> -   - test: l2fwd: separate rx and tx drop counters
> -   - test: l2fwd: increase burst size
> -   - test: l2fwd: optimize statistics usage
> -   - test: l2fwd: optimize queue mode
> -   - test: l2fwd: start pktios after worker thread create
> -   - test: l2fwd: added option to disable error check
> -   - example/ipsec: Increase ip_data_len for Tunnel mode
> -   - example: ipsec: check push_tail return code
> -   * General:
> -   - linux-generic: pktio: 

[lng-odp] [API-NEXT PATCH] api: queue: add queue context length

[Petri Savolainen]

Added queue context length parameter, which is a hint for
the implementation for how much to prefetch. Added the same in
to the set function. It's not needed for get function since
application can store the same into context data and
implementation may avoid to save the length if it does not
need it (e.g. always prefetches a fixed number of bytes).

Signed-off-by: Petri Savolainen 
---
 include/odp/api/spec/queue.h  |  9 -
 platform/linux-generic/odp_queue.c|  3 ++-
 test/validation/queue/queue.c | 10 ++
 test/validation/scheduler/scheduler.c |  6 +++---
 4 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h
index 51d94a2..ecb1a6d 100644
--- a/include/odp/api/spec/queue.h
+++ b/include/odp/api/spec/queue.h
@@ -139,6 +139,12 @@ typedef struct odp_queue_param_t {
  * pointer for prefetching the context data. Default value of the
  * pointer is NULL. */
void *context;
+
+   /** Queue context data length in bytes
+ *
+ * The implementation may use this value as a hint for the number of
+ * context data bytes to prefetch. Default value is zero (no hint). */
+   uint32_t context_len;
 } odp_queue_param_t;
 
 /**
@@ -191,11 +197,12 @@ odp_queue_t odp_queue_lookup(const char *name);
  *
  * @param queueQueue handle
  * @param context  Address to the queue context
+ * @param len  Queue context data length in bytes
  *
  * @retval 0 on success
  * @retval <0 on failure
  */
-int odp_queue_context_set(odp_queue_t queue, void *context);
+int odp_queue_context_set(odp_queue_t queue, void *context, uint32_t len);
 
 /**
  * Get queue context
diff --git a/platform/linux-generic/odp_queue.c 
b/platform/linux-generic/odp_queue.c
index 342ffa2..9a9462d 100644
--- a/platform/linux-generic/odp_queue.c
+++ b/platform/linux-generic/odp_queue.c
@@ -344,7 +344,8 @@ int odp_queue_destroy(odp_queue_t handle)
return 0;
 }
 
-int odp_queue_context_set(odp_queue_t handle, void *context)
+int odp_queue_context_set(odp_queue_t handle, void *context,
+ uint32_t len ODP_UNUSED)
 {
queue_entry_t *queue;
queue = queue_to_qentry(handle);
diff --git a/test/validation/queue/queue.c b/test/validation/queue/queue.c
index 50c6857..96d21fa 100644
--- a/test/validation/queue/queue.c
+++ b/test/validation/queue/queue.c
@@ -12,7 +12,7 @@
 #define MSG_POOL_SIZE   (4 * 1024 * 1024)
 #define CONFIG_MAX_ITERATION(100)
 
-static int queue_contest = 0xff;
+static int queue_context = 0xff;
 static odp_pool_t pool;
 
 int queue_suite_init(void)
@@ -76,10 +76,11 @@ void queue_test_sunnydays(void)
CU_ASSERT_EQUAL(ODP_SCHED_SYNC_PARALLEL,
odp_queue_sched_type(queue_id));
 
-   CU_ASSERT(0 == odp_queue_context_set(queue_id, &queue_contest));
+   CU_ASSERT(0 == odp_queue_context_set(queue_id, &queue_context,
+sizeof(queue_context)));
 
prtn = odp_queue_context(queue_id);
-   CU_ASSERT(&queue_contest == (int *)prtn);
+   CU_ASSERT(&queue_context == (int *)prtn);
 
msg_pool = odp_pool_lookup("msg_pool");
buf = odp_buffer_alloc(msg_pool);
@@ -144,7 +145,8 @@ void queue_test_info(void)
/* Create a plain queue and set context */
q_plain = odp_queue_create(nq_plain, NULL);
CU_ASSERT(ODP_QUEUE_INVALID != q_plain);
-   CU_ASSERT(odp_queue_context_set(q_plain, q_plain_ctx) == 0);
+   CU_ASSERT(odp_queue_context_set(q_plain, q_plain_ctx,
+   sizeof(q_plain_ctx)) == 0);
 
/* Create a scheduled ordered queue with explicitly set params */
odp_queue_param_init(¶m);
diff --git a/test/validation/scheduler/scheduler.c 
b/test/validation/scheduler/scheduler.c
index ae98401..dae859e 100644
--- a/test/validation/scheduler/scheduler.c
+++ b/test/validation/scheduler/scheduler.c
@@ -592,7 +592,7 @@ static void chaos_run(unsigned int qtype)
CU_ASSERT_FATAL(globals->chaos_q[i].handle !=
ODP_QUEUE_INVALID);
rc = odp_queue_context_set(globals->chaos_q[i].handle,
-  CHAOS_NDX_TO_PTR(i));
+  CHAOS_NDX_TO_PTR(i), 0);
CU_ASSERT_FATAL(rc == 0);
}
 
@@ -1374,7 +1374,7 @@ static int create_queues(void)
pqctx->ctx_handle = queue_ctx_buf;
pqctx->sequence = 0;
 
-   rc = odp_queue_context_set(pq, pqctx);
+   rc = odp_queue_context_set(pq, pqctx, 0);
 
if (rc != 0) {
printf("Cannot set plain queue context\n");
@@ -1419,7 +1419,7 @@ static int create_queues(void)
qctx->lock_sequence[ndx] = 0;
}
 
-

Re: [lng-odp] [PATCH] validation: time: shorten test duration

[Ivan Khoronzhuk]

On 14.04.16 17:15, Savolainen, Petri (Nokia - FI/Espoo) wrote:

Hi,

I said back then that we need one test case that runs longer than 4 sec, other 
test cases can be shorter. This patch  does that - combines long lasting tests 
into single >4sec run.

The rationale for shorter duration is that many of us need to run 'make check' 
multiple times a day and the benefit from every second we can save is 
multiplied by the number users and test runs per user. This patch saves roughly 
40 seconds per user per test run.

Optimally, time validation test should be able to verify the API in a bit more 
than 4 seconds. Also a single, very long test case makes people wonder if the 
system hanged...

You could add a configure option for a longer test run, but by default the time 
test (or any other test) should not take a minute.

-Petri

That's because there is no general way to get correct CPU freq,
in another way it's possible to figure out needed number of loops in runtime...





-Original Message-
From: EXT Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
Sent: Thursday, April 14, 2016 5:00 PM
To: Savolainen, Petri (Nokia - FI/Espoo) ;
lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCH] validation: time: shorten test duration

Hi, Petri.

As far I remember you was OK to increase time of testing on even more
time.
It was supposed to include future freqs...I know, for now it's not
correct..
but anyway. Also I dislike the idea to combine two separate tests in one
function.
Yes, it makes tests faster, but doesn't reflect unit test nature. Also,
Ok,
you've made test shorter supposing that freq can not be fast enough to
exit the
4sec range, you've saved ~ minute, but why you decrease 5 sec to 3 sec, it
helps to
see sense of time..and can be missed easily.

On 14.04.16 14:41, Petri Savolainen wrote:

Time test duration was almost a minute on a 2.6GHz CPU.
Combined local and global time monotonity tests, so that
the there is only single run of the long wait loop.
Shortened wait test time to 3 seconds and long loop into
roughly 14 seconds on a 2.6GHz CPU. There should be enough
head room to keep loop duration over 4 seconds even with
higher CPU frequencies.

Signed-off-by: Petri Savolainen 
---
   test/validation/time/time.c | 63 -



   test/validation/time/time.h |  3 +--
   2 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/test/validation/time/time.c b/test/validation/time/time.c
index cb1ddef..5e55817 100644
--- a/test/validation/time/time.c
+++ b/test/validation/time/time.c
@@ -9,10 +9,11 @@
   #include "time.h"

   #define BUSY_LOOP_CNT3000/* used for t > min

resolution */

-#define BUSY_LOOP_CNT_LONG 120 /* used for t > 4 sec */
+#define BUSY_LOOP_CNT_LONG 60  /* used for t > 4 sec */
   #define MIN_TIME_RATE32000
   #define MAX_TIME_RATE150
   #define DELAY_TOLERANCE  2000/* deviation for 
delay

*/

+#define WAIT_SECONDS3

   static uint64_t local_res;
   static uint64_t global_res;
@@ -98,42 +99,45 @@ void time_test_global_conversion(void)
time_test_conversion(odp_time_global_from_ns, global_res);
   }

-static void time_test_monotony(time_cb time)
+void time_test_monotony(void)
   {
volatile uint64_t count = 0;
-   odp_time_t t1, t2, t3;
+   odp_time_t l_t1, l_t2, l_t3;
+   odp_time_t g_t1, g_t2, g_t3;
uint64_t ns1, ns2, ns3;

-   t1 = time();
+   l_t1 = odp_time_local();
+   g_t1 = odp_time_global();

while (count < BUSY_LOOP_CNT) {
count++;
};

-   t2 = time();
+   l_t2 = odp_time_local();
+   g_t2 = odp_time_global();

while (count < BUSY_LOOP_CNT_LONG) {
count++;
};

-   t3 = time();
+   l_t3 = odp_time_local();
+   g_t3 = odp_time_global();

-   ns1 = odp_time_to_ns(t1);
-   ns2 = odp_time_to_ns(t2);
-   ns3 = odp_time_to_ns(t3);
+   ns1 = odp_time_to_ns(l_t1);
+   ns2 = odp_time_to_ns(l_t2);
+   ns3 = odp_time_to_ns(l_t3);

+   /* Local time assertions */
CU_ASSERT(ns2 > ns1);
CU_ASSERT(ns3 > ns2);
-}

-void time_test_local_monotony(void)
-{
-   time_test_monotony(odp_time_local);
-}
+   ns1 = odp_time_to_ns(g_t1);
+   ns2 = odp_time_to_ns(g_t2);
+   ns3 = odp_time_to_ns(g_t3);

-void time_test_global_monotony(void)
-{
-   time_test_monotony(odp_time_global);
+   /* Global time assertions */
+   CU_ASSERT(ns2 > ns1);
+   CU_ASSERT(ns3 > ns2);
   }

   static void time_test_cmp(time_cb time, time_from_ns_cb time_from_ns)
@@ -324,16 +328,18 @@ static void time_test_wait_until(time_cb time,

time_from_ns_cb time_from_ns)


start_time = time();
wait = start_time;
-   for (i = 1; i < 6; i++) {
+   for (i = 0; i < WAIT_SECONDS; i++) {
wait = odp_time_sum(wai

Re: [lng-odp] [PATCH] validation: time: shorten test duration

[Ivan Khoronzhuk]

What about to do smth like below
(I didn't normalize it to ns...but that are tehnical details)

uint64_t c1, c2;
uint64_t loop_time;

c1 = odp_cpu_cycles();

while (loop_time <= 4) {
c2 = odp_cpu_cycles();
cycle_diff = odp_cpu_cycles_diff(c2, c1);
loop_time += cycle_diff / odp_cpu_hz();
c1 = c2;
};


On 14.04.16 17:24, Ivan Khoronzhuk wrote:



On 14.04.16 17:15, Savolainen, Petri (Nokia - FI/Espoo) wrote:

Hi,

I said back then that we need one test case that runs longer than 4 sec, other 
test cases can be shorter. This patch  does that - combines long lasting tests 
into single >4sec run.

The rationale for shorter duration is that many of us need to run 'make check' 
multiple times a day and the benefit from every second we can save is 
multiplied by the number users and test runs per user. This patch saves roughly 
40 seconds per user per test run.

Optimally, time validation test should be able to verify the API in a bit more 
than 4 seconds. Also a single, very long test case makes people wonder if the 
system hanged...

You could add a configure option for a longer test run, but by default the time 
test (or any other test) should not take a minute.

-Petri

That's because there is no general way to get correct CPU freq,
in another way it's possible to figure out needed number of loops in runtime...





-Original Message-
From: EXT Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
Sent: Thursday, April 14, 2016 5:00 PM
To: Savolainen, Petri (Nokia - FI/Espoo) ;
lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCH] validation: time: shorten test duration

Hi, Petri.

As far I remember you was OK to increase time of testing on even more
time.
It was supposed to include future freqs...I know, for now it's not
correct..
but anyway. Also I dislike the idea to combine two separate tests in one
function.
Yes, it makes tests faster, but doesn't reflect unit test nature. Also,
Ok,
you've made test shorter supposing that freq can not be fast enough to
exit the
4sec range, you've saved ~ minute, but why you decrease 5 sec to 3 sec, it
helps to
see sense of time..and can be missed easily.

On 14.04.16 14:41, Petri Savolainen wrote:

Time test duration was almost a minute on a 2.6GHz CPU.
Combined local and global time monotonity tests, so that
the there is only single run of the long wait loop.
Shortened wait test time to 3 seconds and long loop into
roughly 14 seconds on a 2.6GHz CPU. There should be enough
head room to keep loop duration over 4 seconds even with
higher CPU frequencies.

Signed-off-by: Petri Savolainen 
---
   test/validation/time/time.c | 63 -



   test/validation/time/time.h |  3 +--
   2 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/test/validation/time/time.c b/test/validation/time/time.c
index cb1ddef..5e55817 100644
--- a/test/validation/time/time.c
+++ b/test/validation/time/time.c
@@ -9,10 +9,11 @@
   #include "time.h"

   #define BUSY_LOOP_CNT3000/* used for t > min

resolution */

-#define BUSY_LOOP_CNT_LONG120 /* used for t > 4 sec */
+#define BUSY_LOOP_CNT_LONG60  /* used for t > 4 sec */
   #define MIN_TIME_RATE32000
   #define MAX_TIME_RATE150
   #define DELAY_TOLERANCE2000/* deviation for delay

*/

+#define WAIT_SECONDS3

   static uint64_t local_res;
   static uint64_t global_res;
@@ -98,42 +99,45 @@ void time_test_global_conversion(void)
   time_test_conversion(odp_time_global_from_ns, global_res);
   }

-static void time_test_monotony(time_cb time)
+void time_test_monotony(void)
   {
   volatile uint64_t count = 0;
-odp_time_t t1, t2, t3;
+odp_time_t l_t1, l_t2, l_t3;
+odp_time_t g_t1, g_t2, g_t3;
   uint64_t ns1, ns2, ns3;

-t1 = time();
+l_t1 = odp_time_local();
+g_t1 = odp_time_global();

   while (count < BUSY_LOOP_CNT) {
   count++;
   };

-t2 = time();
+l_t2 = odp_time_local();
+g_t2 = odp_time_global();

   while (count < BUSY_LOOP_CNT_LONG) {
   count++;
   };

-t3 = time();
+l_t3 = odp_time_local();
+g_t3 = odp_time_global();

-ns1 = odp_time_to_ns(t1);
-ns2 = odp_time_to_ns(t2);
-ns3 = odp_time_to_ns(t3);
+ns1 = odp_time_to_ns(l_t1);
+ns2 = odp_time_to_ns(l_t2);
+ns3 = odp_time_to_ns(l_t3);

+/* Local time assertions */
   CU_ASSERT(ns2 > ns1);
   CU_ASSERT(ns3 > ns2);
-}

-void time_test_local_monotony(void)
-{
-time_test_monotony(odp_time_local);
-}
+ns1 = odp_time_to_ns(g_t1);
+ns2 = odp_time_to_ns(g_t2);
+ns3 = odp_time_to_ns(g_t3);

-void time_test_global_monotony(void)
-{
-time_test_monotony(odp_time_global);
+/* Global time assertions */
+CU_ASSERT(ns2 > ns1);
+CU_ASSERT(ns3 > ns2);
   }

   static void time_test_cmp(time_cb 

Re: [lng-odp] [PATCH] linux-generic: test: shmem: coverity fix

[Maxim Uvarov]

On 04/04/16 12:29, Christophe Milard wrote:

Fixes: https://bugs.linaro.org/show_bug.cgi?id=2148 (CID 159393)
The if statement introduced here is redundant with the previous
CU_ASSERT_FATAL, but avoids the coverity warning.
(coverity probably misses the longjump in CU_ASSERT_FATAL)

Signed-off-by: Christophe Milard 
---

NOTE: to be applied on to of: "linux-generic: test: shmem: close fifo"

  platform/linux-generic/test/shmem/shmem_odp.c | 8 +---
  1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/platform/linux-generic/test/shmem/shmem_odp.c 
b/platform/linux-generic/test/shmem/shmem_odp.c
index a1f750f..77c5a01 100644
--- a/platform/linux-generic/test/shmem/shmem_odp.c
+++ b/platform/linux-generic/test/shmem/shmem_odp.c
@@ -47,9 +47,11 @@ void shmem_test_odp_shm_proc(void)
fd = open(fifo_name, O_RDONLY);
CU_ASSERT_FATAL(fd >= 0);
  
-	CU_ASSERT(read(fd, &test_result, sizeof(char)) == 1);

-   close(fd);
-   CU_ASSERT_FATAL(test_result == TEST_SUCCESS);
+   if (fd >= 0) {  /* redundant, but to avoid coverity CID 159393 */

CID number should be removed from test code as well as all that comment.

Also you are breaking test logic. I.e. if (fd == 0) then you don't do 
read() and test test_result.  And in that

case test will be successful.

So I think only think you need is to add ASSERTS for fd and close(fd).

Regards,
Maxim.


+   CU_ASSERT_FATAL(read(fd, &test_result, sizeof(char)) == 1);
+   close(fd);
+   CU_ASSERT_FATAL(test_result == TEST_SUCCESS);
+   }
  
  	CU_ASSERT(odp_shm_free(shm) == 0);

  }


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

Re: [lng-odp] [PATCH] pkg: remove packaging in ODP

[Maxim Uvarov]

Merged,

yes, it's more easy to maintain it somewhere outside.

Maxim.

On 04/14/16 17:18, Ricardo Salveti wrote:

On Thu, Apr 14, 2016 at 11:00 AM, Anders Roxell
 wrote:

Move the packaging scripts to its own git [1] repository.

[1] http://git.linaro.org/lng/odp-packaging.git

Signed-off-by: Anders Roxell 
---
  pkg/debian/README.Debian  |  12 -
  pkg/debian/changelog  | 566 --
  pkg/debian/compat |   1 -
  pkg/debian/control|  70 
  pkg/debian/copyright  |  31 --
  pkg/debian/docs   |   1 -
  pkg/debian/libodp-linux-dev.dirs  |   2 -
  pkg/debian/libodp-linux-dev.install   |   5 -
  pkg/debian/libodp-linux.dirs  |   1 -
  pkg/debian/libodp-linux.install   |   1 -
  pkg/debian/libodphelper-linux-dev.dirs|   2 -
  pkg/debian/libodphelper-linux-dev.install |   4 -
  pkg/debian/libodphelper-linux.dirs|   1 -
  pkg/debian/libodphelper-linux.install |   1 -
  pkg/debian/odp-linux-bin.dirs |   1 -
  pkg/debian/odp-linux-bin.install  |   1 -
  pkg/debian/rules  |  30 --
  pkg/debian/source/format  |   1 -
  pkg/rpm/odp.spec  |  80 -
  scripts/builddeb  |  38 --
  scripts/buildrpm  |  30 --
  scripts/common_pkg_build.sh   |  39 --
  22 files changed, 918 deletions(-)
  delete mode 100644 pkg/debian/README.Debian
  delete mode 100644 pkg/debian/changelog
  delete mode 100644 pkg/debian/compat
  delete mode 100644 pkg/debian/control
  delete mode 100644 pkg/debian/copyright
  delete mode 100644 pkg/debian/docs
  delete mode 100644 pkg/debian/libodp-linux-dev.dirs
  delete mode 100644 pkg/debian/libodp-linux-dev.install
  delete mode 100644 pkg/debian/libodp-linux.dirs
  delete mode 100644 pkg/debian/libodp-linux.install
  delete mode 100644 pkg/debian/libodphelper-linux-dev.dirs
  delete mode 100644 pkg/debian/libodphelper-linux-dev.install
  delete mode 100644 pkg/debian/libodphelper-linux.dirs
  delete mode 100644 pkg/debian/libodphelper-linux.install
  delete mode 100644 pkg/debian/odp-linux-bin.dirs
  delete mode 100644 pkg/debian/odp-linux-bin.install
  delete mode 100755 pkg/debian/rules
  delete mode 100644 pkg/debian/source/format
  delete mode 100644 pkg/rpm/odp.spec
  delete mode 100755 scripts/builddeb
  delete mode 100755 scripts/buildrpm
  delete mode 100644 scripts/common_pkg_build.sh

diff --git a/pkg/debian/README.Debian b/pkg/debian/README.Debian
deleted file mode 100644
index b8e47e4..000
--- a/pkg/debian/README.Debian
+++ /dev/null
@@ -1,12 +0,0 @@
-opendataplane for Debian
-
-
-For up to date information please visit http://www.opendataplane.org
-The OpenDataPlane (ODP) project has been established to produce an open-source,
-cross-platform set of application programming interfaces (APIs) for the
-networking data plane.
-
-It provides a library such as openvswitch with a portable API that facilitates
-platform independence and access to hardware acceleration.
-
- -- Anders Roxell   Mon, 22 Dec 2014 19:07:06 +0100
diff --git a/pkg/debian/changelog b/pkg/debian/changelog
deleted file mode 100644
index 4d8d9a8..000
--- a/pkg/debian/changelog
+++ /dev/null
@@ -1,566 +0,0 @@
-opendataplane (1.8.0.0-1) unstable; urgency=low
-   * ODP release v1.8
-
- -- Maxim Uvarov   Fri, 04 Mar 2016 13:32:05 +0300
-
-opendataplane (1.7.0.0-1) unstable; urgency=low
-   * ODP release v1.7
-
- -- Maxim Uvarov   Fri, 05 Feb 2016 13:05:00 +0300
-
-opendataplane (1.6.0.0-1) unstable; urgency=low
-   * ODP release v1.6
-
- -- Maxim Uvarov   Mon, 28 Dec 2015 17:01:58 +0300
-
-opendataplane (1.5.0.0-1) unstable; urgency=low
-   * ODP release v1.5
-
- -- Maxim Uvarov   Mon, 30 Nov 2015 13:08:43 +0300
-
-opendataplane (1.4.1.0-1) unstable; urgency=low
-   * Validation
-   - pktio: test transmit error recovery
-   - schedule: add chaos test
-   - check return code from odp_queue_lock_count()
-   - scheduler: test ordered queue reorder processing
-   - pktio: initialize queue parameters correctly
-   - pktio: test for transmit error handling
-   - pktio: add support for direct receive
-   - pktio: pass interface index rather than name
-   - pktio: fix start_stop test
-   - test: l2fwd: separate rx and tx drop counters
-   - test: l2fwd: increase burst size
-   - test: l2fwd: optimize statistics usage
-   - test: l2fwd: optimize queue mode
-   - test: l2fwd: start pktios after worker thread create
-   - test: l2fwd: added option to disable error check
-   - example/ipsec: Increase ip_data_len for Tunnel mode
-   - example: ipsec: check push_tail return code
-   * General:
-   - linux-generic: pktio: handle transmit errors correctly
-   - pktio socket_mmap: recover from tra

Re: [lng-odp] [PATCHv5] linux-generic: pktio: avoid coverity issues by adding explicit rc check

[Maxim Uvarov]

Merged with short description change to:
linux-generic: pktio: add explicit rc check for odp_pktio_capability

no need to write tool which you used to find a bug. Only what exactly
was done and what is the problem.

Thank you,
Maxim.

On 04/04/16 19:13, Bill Fischofer wrote:

Resolve bug https://bugs.linaro.org/show_bug.cgi?id=2138 by adding an
explicit rc check for odp_pktio_capability().

Signed-off-by: Bill Fischofer 
---
  platform/linux-generic/odp_packet_io.c | 16 ++--
  1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/platform/linux-generic/odp_packet_io.c 
b/platform/linux-generic/odp_packet_io.c
index 9192be2..afe5c34 100644
--- a/platform/linux-generic/odp_packet_io.c
+++ b/platform/linux-generic/odp_packet_io.c
@@ -1027,6 +1027,7 @@ int odp_pktin_queue_config(odp_pktio_t pktio,
odp_pktio_capability_t capa;
unsigned num_queues;
unsigned i;
+   int rc;
odp_queue_t queue;
odp_pktin_queue_param_t default_param;
  
@@ -1059,7 +1060,12 @@ int odp_pktin_queue_config(odp_pktio_t pktio,

return -1;
}
  
-	odp_pktio_capability(pktio, &capa);

+   rc = odp_pktio_capability(pktio, &capa);
+   if (rc) {
+   ODP_DBG("pktio %s: unable to read capabilities\n",
+   entry->s.name);
+   return -1;
+   }
  
  	if (num_queues > capa.max_input_queues) {

ODP_DBG("pktio %s: too many input queues\n", entry->s.name);
@@ -1135,6 +1141,7 @@ int odp_pktout_queue_config(odp_pktio_t pktio,
odp_pktio_capability_t capa;
unsigned num_queues;
unsigned i;
+   int rc;
odp_pktout_queue_param_t default_param;
  
  	if (param == NULL) {

@@ -1172,7 +1179,12 @@ int odp_pktout_queue_config(odp_pktio_t pktio,
return -1;
}
  
-	odp_pktio_capability(pktio, &capa);

+   rc = odp_pktio_capability(pktio, &capa);
+   if (rc) {
+   ODP_DBG("pktio %s: unable to read capabilities\n",
+   entry->s.name);
+   return -1;
+   }
  
  	if (num_queues > capa.max_output_queues) {

ODP_DBG("pktio %s: too many output queues\n", entry->s.name);


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

Re: [lng-odp] [RFC] api: crypto capability support

[Krishna Garapati]

Thanks Petri, now it is clear what you have in mind. BTW, it looks like a
patch that can be proposed. So, please send it as patch version for review

/Krishna

On 14 April 2016 at 10:50, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolai...@nokia.com> wrote:

> Hi,
>
> See under my proposal on using bit fields and a capability struct similar
> to what we have already for other APIs. Application may easily check if
> multiple ciphers are support and especially if those are implemented with
> HW offload.
>
> -Petri
>
>
> odp_crypto_ciphers_t ciphers;
> odp_pktio_capability_t capa;
>
> ciphers.all_bits   = 0;
> ciphers.3des_cbc   = 1;
> ciphers.aes128_cbc = 1;
>
> odp_crypto_capability(&capa);
>
> if (capa.chiphers.all_bits & ciphers.all_bits == 0) {
>  // 3des or aes are not supported
>  ...
>  return -1;
> }
>
> if (capa.hw_chiphers.all_bits & ciphers.all_bits == 0) {
>  // 3des or aes are not supported in HW
>  ...
> }
>
>
>
> typedef union odp_crypto_ciphers_t {
> /** Cipher algorithms */
> struct {
> /** ODP_CIPHER_ALG_NULL */
> uint32_t null   : 1;
>
> /** ODP_CIPHER_ALG_DES */
> uint32_t des: 1;
>
> /** ODP_CIPHER_ALG_3DES_CBC */
> uint32_t 3des_cbc   : 1;
>
> /** ODP_CIPHER_ALG_AES128_CBC */
> uint32_t aes128_cbc : 1;
>
> /** ODP_CIPHER_ALG_AES128_GCM */
> uint32_t aes128_gcm : 1;
>
> } bit;
>
> /** All bits of the bit field structure
>   *
>   * This field can be used to set/clear all flags, or bitwise
>   * operations over the entire structure. */
> uint32_t all_bits;
> } odp_crypto_ciphers_t;
>
>
> typedef union odp_crypto_auths_t {
> /** Cipher algorithms */
> struct {
> /** ODP_AUTH_ALG_NULL */
> uint32_t null   : 1;
>
> /** ODP_AUTH_ALG_MD5_96 */
> uint32_t md5_96 : 1;
>
> /** ODP_AUTH_ALG_SHA256_128 */
> uint32_t sha256_128 : 1;
>
> /** ODP_AUTH_ALG_AES128_GCM */
> uint32_t aes128_gcm : 1;
>
> } bit;
>
> /** All bits of the bit field structure
>   *
>   * This field can be used to set/clear all flags, or bitwise
>   * operations over the entire structure. */
> uint32_t all_bits;
> } odp_crypto_auths_t;
>
>
> typedef struct odp_crypto_capability_t {
> /** Maximum number of crypto sessions */
> uint32_t max_sessions;
>
> /** Supported chipher algorithms */
> odp_crypto_ciphers_t chiphers;
>
> /** Chipher algorithms implemented with HW offload */
> odp_crypto_ciphers_t hw_chiphers;
>
> /** Supported authentication algorithms */
> odp_crypto_auths_t auths;
>
> /** Authentication algorithms implemented with HW offload */
> odp_crypto_auths_t hw_auths;
>
> } odp_crypto_capability_t;
>
>
> /**
>  * Query crypto capabilities
>  *
>  * Outputs crypto capabilities on success.
>  *
>  * @param[out] capa   Pointer to capability structure for output
>  *
>  * @retval 0 on success
>  * @retval <0 on failure
>  */
> int odp_crypto_capability(odp_pktio_capability_t *capa);
>
>
>
>
> > -Original Message-
> > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT
> > Balakrishna Garapati
> > Sent: Wednesday, April 13, 2016 3:33 PM
> > To: lng-odp@lists.linaro.org
> > Subject: [lng-odp] [RFC] api: crypto capability support
> >
> > This RFC provides the support for the applicationis to inquire the given
> > cipher, authentication algorithms
> >
> > Signed-off-by: Balakrishna Garapati 
> > ---
> >  include/odp/api/spec/crypto.h | 30 ++
> >  1 file changed, 30 insertions(+)
> >
> > diff --git a/include/odp/api/spec/crypto.h
> b/include/odp/api/spec/crypto.h
> > index 41beedb..0def211 100644
> > --- a/include/odp/api/spec/crypto.h
> > +++ b/include/odp/api/spec/crypto.h
> > @@ -254,6 +254,18 @@ typedef struct odp_crypto_op_result {
> >  } odp_crypto_op_result_t;
> >
> >  /**
> > + * Crypto API capability result
> > + */
> > +typedef enum odp_crypto_capability_t {
> > + /** crypto algorithm not supported */
> > + ODP_CRYPTO_NO_SUPPORT = 0,
> > + /** crypto algorithm supported in hardware */
> > + ODP_CRYPTO_HW_SUPPORT,
> > + /** crypto algortihm supported in software */
> > + ODP_CRYPTO_SW_SUPPORT
> > +} odp_crypto_capability_t;
> > +
> > +/**
> >   * Crypto session creation (synchronous)
> >   *
> >   * @param paramsSession parameters
> > @@ -368,6 +380,24 @@ uint64_t
> > odp_crypto_session_to_u64(odp_crypto_session_t hdl);
> >  uint64_t odp_crypto_compl_to_u64(odp_crypto_compl_t hdl);
> >
> >  /**
> > + * Verify the given crypto cipher algorithm support
> > + *
> > + * @param alg  odp_cipher_alg_t to

Re: [lng-odp] [RFC] api: crypto capability support

[Ivan Khoronzhuk]

Frequently used name: caps

odp_crypto_capablity_t -> odp_crypto_caps_t

On 14.04.16 11:50, Savolainen, Petri (Nokia - FI/Espoo) wrote:

Hi,

See under my proposal on using bit fields and a capability struct similar to 
what we have already for other APIs. Application may easily check if multiple 
ciphers are support and especially if those are implemented with HW offload.

-Petri


odp_crypto_ciphers_t ciphers;
odp_pktio_capability_t capa;

ciphers.all_bits   = 0;
ciphers.3des_cbc   = 1;
ciphers.aes128_cbc = 1;

odp_crypto_capability(&capa);

if (capa.chiphers.all_bits & ciphers.all_bits == 0) {
  // 3des or aes are not supported
  ...
  return -1;
}

if (capa.hw_chiphers.all_bits & ciphers.all_bits == 0) {
  // 3des or aes are not supported in HW
  ...
}



typedef union odp_crypto_ciphers_t {
/** Cipher algorithms */
struct {
/** ODP_CIPHER_ALG_NULL */
uint32_t null   : 1;

/** ODP_CIPHER_ALG_DES */
uint32_t des: 1;

/** ODP_CIPHER_ALG_3DES_CBC */
uint32_t 3des_cbc   : 1;

/** ODP_CIPHER_ALG_AES128_CBC */
uint32_t aes128_cbc : 1;

/** ODP_CIPHER_ALG_AES128_GCM */
uint32_t aes128_gcm : 1;

} bit;

/** All bits of the bit field structure
  *
  * This field can be used to set/clear all flags, or bitwise
  * operations over the entire structure. */
uint32_t all_bits;
} odp_crypto_ciphers_t;


typedef union odp_crypto_auths_t {
/** Cipher algorithms */
struct {
/** ODP_AUTH_ALG_NULL */
uint32_t null   : 1;

/** ODP_AUTH_ALG_MD5_96 */
uint32_t md5_96 : 1;

/** ODP_AUTH_ALG_SHA256_128 */
uint32_t sha256_128 : 1;

/** ODP_AUTH_ALG_AES128_GCM */
uint32_t aes128_gcm : 1;

} bit;

/** All bits of the bit field structure
  *
  * This field can be used to set/clear all flags, or bitwise
  * operations over the entire structure. */
uint32_t all_bits;
} odp_crypto_auths_t;


typedef struct odp_crypto_capability_t {
/** Maximum number of crypto sessions */
uint32_t max_sessions;

/** Supported chipher algorithms */
odp_crypto_ciphers_t chiphers;

/** Chipher algorithms implemented with HW offload */
odp_crypto_ciphers_t hw_chiphers;

/** Supported authentication algorithms */
odp_crypto_auths_t auths;

/** Authentication algorithms implemented with HW offload */
odp_crypto_auths_t hw_auths;

} odp_crypto_capability_t;


/**
  * Query crypto capabilities
  *
  * Outputs crypto capabilities on success.
  *
  * @param[out] capa   Pointer to capability structure for output
  *
  * @retval 0 on success
  * @retval <0 on failure
  */
int odp_crypto_capability(odp_pktio_capability_t *capa);





-Original Message-
From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT
Balakrishna Garapati
Sent: Wednesday, April 13, 2016 3:33 PM
To: lng-odp@lists.linaro.org
Subject: [lng-odp] [RFC] api: crypto capability support

This RFC provides the support for the applicationis to inquire the given
cipher, authentication algorithms

Signed-off-by: Balakrishna Garapati 
---
  include/odp/api/spec/crypto.h | 30 ++
  1 file changed, 30 insertions(+)

diff --git a/include/odp/api/spec/crypto.h b/include/odp/api/spec/crypto.h
index 41beedb..0def211 100644
--- a/include/odp/api/spec/crypto.h
+++ b/include/odp/api/spec/crypto.h
@@ -254,6 +254,18 @@ typedef struct odp_crypto_op_result {
  } odp_crypto_op_result_t;

  /**
+ * Crypto API capability result
+ */
+typedef enum odp_crypto_capability_t {
+   /** crypto algorithm not supported */
+   ODP_CRYPTO_NO_SUPPORT = 0,
+   /** crypto algorithm supported in hardware */
+   ODP_CRYPTO_HW_SUPPORT,
+   /** crypto algortihm supported in software */
+   ODP_CRYPTO_SW_SUPPORT
+} odp_crypto_capability_t;
+
+/**
   * Crypto session creation (synchronous)
   *
   * @param paramsSession parameters
@@ -368,6 +380,24 @@ uint64_t
odp_crypto_session_to_u64(odp_crypto_session_t hdl);
  uint64_t odp_crypto_compl_to_u64(odp_crypto_compl_t hdl);

  /**
+ * Verify the given crypto cipher algorithm support
+ *
+ * @param alg  odp_cipher_alg_t to be verified
+ * @return odp_crypto_capability_t
+ *
+ */
+odp_crypto_capability_t odp_crypto_cipher_inquiry(odp_cipher_alg_t alg);
+
+/**
+ * Verify the given crypto authentication algorithm support
+ *
+ * @param alg  odp_auth_alg_t to be verified
+ * @return odp_crypto_capability_t
+ *
+ */
+odp_crypto_capability_t odp_crypto_auth_inquiry(odp_auth_alg_t alg);
+
+/**
   * @}
   */

--
1.9.1

___
lng-odp mailing

Re: [lng-odp] [PATCH] linux-generic: add arch specific *.h files to EXTRA_DIST

[Maxim Uvarov]

Merged,
Maxim.

On 04/13/16 23:21, Ricardo Salveti wrote:

On Wed, Apr 13, 2016 at 3:13 PM, Anders Roxell  wrote:

To make the build from a tarball work again from an architecture that
you didn't create the tarball from.

Signed-off-by: Anders Roxell 
---
  platform/linux-generic/Makefile.am | 4 
  1 file changed, 4 insertions(+)

diff --git a/platform/linux-generic/Makefile.am 
b/platform/linux-generic/Makefile.am
index 54f35d6..7b93f19 100644
--- a/platform/linux-generic/Makefile.am
+++ b/platform/linux-generic/Makefile.am
@@ -170,12 +170,16 @@ __LIB__libodp_linux_la_SOURCES = \
arch/@ARCH@/odp_sysinfo_parse.c

  EXTRA_DIST = \
+arch/linux/odp/api/cpu_arch.h \
  arch/linux/odp_cpu_arch.c \
  arch/linux/odp_sysinfo_parse.c \
+arch/mips64/odp/api/cpu_arch.h \
  arch/mips64/odp_cpu_arch.c \
  arch/mips64/odp_sysinfo_parse.c \
+arch/powerpc/odp/api/cpu_arch.h \
  arch/powerpc/odp_cpu_arch.c \
  arch/powerpc/odp_sysinfo_parse.c \
+arch/x86/odp/api/cpu_arch.h \
  arch/x86/odp_cpu_arch.c \
  arch/x86/odp_sysinfo_parse.c

--
2.1.4

Thanks, this allows me to create the tarball on x86 and build the
package on arm64 :-)

Reviewed-and-tested-by: Ricardo Salveti 

Cheers,


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

Re: [lng-odp] [PATCH] linux-generic: queue: avoid false positive when compiling with -Og

[Maxim Uvarov]

On 04/14/16 15:45, Bill Fischofer wrote:

ping.  Maxim, I believe this patch is the simplest solution to the issue.



Bill,  this patch fixes existence problem with initializing variable. 
But it looks a little bit like a hack, where
logically one function slitted on 2 pieces without any reason. (Or I do 
not see this reason.)
Since nobody gave review yet, I will send my patch to list also. And we 
can merge mine or your.


Maxim.


On Fri, Apr 8, 2016 at 4:06 PM, Bill Fischofer 
mailto:bill.fischo...@linaro.org>> wrote:


After reviewing the code again, I think the patch stands as the
simplest solution to this compiler bug. Both type and handle are
needed because they are cached from the queue_entry while it is
locked and then referenced after it is unlocked. A direct
reference the internal fields is not legal unless the queue_entry
lock is held.

On Thu, Apr 7, 2016 at 1:38 AM, Maxim Uvarov
mailto:maxim.uva...@linaro.org>> wrote:

Mike, it think it's better to remove handle and type
completely. And if you already looking to that
function it might be reasonable to make it more readable, use
odp_config_queues() and use int
instead of uint32_t. Something like that:


--- a/platform/linux-generic/odp_queue.c
+++ b/platform/linux-generic/odp_queue.c
@@ -234,10 +234,8 @@ int odp_queue_lock_count(odp_queue_t handle)

 odp_queue_t odp_queue_create(const char *name, const
odp_queue_param_t *param)
 {
-   uint32_t i;
+   int i;
queue_entry_t *queue;
-   odp_queue_t handle = ODP_QUEUE_INVALID;
-   odp_queue_type_t type;
odp_queue_param_t default_param;

if (param == NULL) {
@@ -245,7 +243,7 @@ odp_queue_t odp_queue_create(const char
*name, const odp_queue_param_t *param)
param = &default_param;
}

-   for (i = 0; i < ODP_CONFIG_QUEUES; i++) {
+   for (i = 0; i < odp_config_queues(); i++) {
queue = &queue_tbl->queue[i];

if (queue->s.status != QUEUE_STATUS_FREE)
@@ -253,33 +251,29 @@ odp_queue_t odp_queue_create(const char
*name, const odp_queue_param_t *param)

LOCK(&queue->s.lock);
if (queue->s.status == QUEUE_STATUS_FREE) {
-   if (queue_init(queue, name, param)) {
+   if (queue_init(queue, name, param) ||
+   queue->s.handle ==
ODP_QUEUE_INVALID) {
UNLOCK(&queue->s.lock);
-   return handle;
+   return ODP_QUEUE_INVALID;
}

-   type = queue->s.type;
-
-   if (type == ODP_QUEUE_TYPE_SCHED)
+   if (queue->s.type ==
ODP_QUEUE_TYPE_SCHED) {
queue->s.status =
QUEUE_STATUS_NOTSCHED;
-   else
+   if (schedule_queue_init(queue)) {
+  ODP_ERR("schedule queue init failed\n");
+ UNLOCK(&queue->s.lock);
+   return ODP_QUEUE_INVALID;
+   }
+   } else {
queue->s.status =
QUEUE_STATUS_READY;
-
-   handle = queue->s.handle;
+   }
UNLOCK(&queue->s.lock);
-   break;
+   return queue->s.handle;
}
UNLOCK(&queue->s.lock);
}

-   if (handle != ODP_QUEUE_INVALID && type ==
ODP_QUEUE_TYPE_SCHED) {
-   if (schedule_queue_init(queue)) {
-   ODP_ERR("schedule queue init failed\n");
-   return ODP_QUEUE_INVALID;
-   }
-   }
-
-   return handle;
+   return ODP_QUEUE_INVALID;
 }

Maxim.

On 04/05/16 21:02, Mike Holmes wrote:

Any one object to this?
It impacts the accuracy of the ABI testing tools if -Og
cannot be used.

On 1 April 2016 at 19:03, Bill Fischofer
mailto:bill.fischo...@linaro.org>
>> wrote:

Resolve Bug
https://bugs.linaro.org/show_bug.cgi?id=2159 by adding an
extraneous variable initialization to avoid a false
positive error
when
compiling w

[lng-odp] [PATCH] linux-generic: rework odp_queue_create

[Maxim Uvarov]

- use odp_config_queues() to get number of queues and int cycle
  iterrator. That makes it more easy to reuse that function for
  other platfroms.;
- remove declaring variables to invalid at the bottom and check for
  them down the code;
- make code more easy to read;

Signed-off-by: Maxim Uvarov 
---
 platform/linux-generic/odp_queue.c | 36 +++-
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/platform/linux-generic/odp_queue.c 
b/platform/linux-generic/odp_queue.c
index 342ffa2..0f7fd15 100644
--- a/platform/linux-generic/odp_queue.c
+++ b/platform/linux-generic/odp_queue.c
@@ -234,10 +234,8 @@ int odp_queue_lock_count(odp_queue_t handle)
 
 odp_queue_t odp_queue_create(const char *name, const odp_queue_param_t *param)
 {
-   uint32_t i;
+   int i;
queue_entry_t *queue;
-   odp_queue_t handle = ODP_QUEUE_INVALID;
-   odp_queue_type_t type;
odp_queue_param_t default_param;
 
if (param == NULL) {
@@ -245,7 +243,7 @@ odp_queue_t odp_queue_create(const char *name, const 
odp_queue_param_t *param)
param = &default_param;
}
 
-   for (i = 0; i < ODP_CONFIG_QUEUES; i++) {
+   for (i = 0; i < odp_config_queues(); i++) {
queue = &queue_tbl->queue[i];
 
if (queue->s.status != QUEUE_STATUS_FREE)
@@ -253,33 +251,29 @@ odp_queue_t odp_queue_create(const char *name, const 
odp_queue_param_t *param)
 
LOCK(&queue->s.lock);
if (queue->s.status == QUEUE_STATUS_FREE) {
-   if (queue_init(queue, name, param)) {
+   if (queue_init(queue, name, param) ||
+   queue->s.handle == ODP_QUEUE_INVALID) {
UNLOCK(&queue->s.lock);
-   return handle;
+   return ODP_QUEUE_INVALID;
}
 
-   type = queue->s.type;
-
-   if (type == ODP_QUEUE_TYPE_SCHED)
+   if (queue->s.type == ODP_QUEUE_TYPE_SCHED) {
queue->s.status = QUEUE_STATUS_NOTSCHED;
-   else
+   if (schedule_queue_init(queue)) {
+   ODP_ERR("schedule queue init failed\n");
+   UNLOCK(&queue->s.lock);
+   return ODP_QUEUE_INVALID;
+   }
+   } else {
queue->s.status = QUEUE_STATUS_READY;
-
-   handle = queue->s.handle;
+   }
UNLOCK(&queue->s.lock);
-   break;
+   return queue->s.handle;
}
UNLOCK(&queue->s.lock);
}
 
-   if (handle != ODP_QUEUE_INVALID && type == ODP_QUEUE_TYPE_SCHED) {
-   if (schedule_queue_init(queue)) {
-   ODP_ERR("schedule queue init failed\n");
-   return ODP_QUEUE_INVALID;
-   }
-   }
-
-   return handle;
+   return ODP_QUEUE_INVALID;
 }
 
 void queue_destroy_finalize(queue_entry_t *queue)
-- 
2.7.1.250.gff4ea60

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

Re: [lng-odp] [PATCH] linux-generic: rework odp_queue_create

[Bill Fischofer]

On Thu, Apr 14, 2016 at 11:35 AM, Maxim Uvarov 
wrote:

> - use odp_config_queues() to get number of queues and int cycle
>   iterrator. That makes it more easy to reuse that function for
>   other platfroms.;
> - remove declaring variables to invalid at the bottom and check for
>   them down the code;
> - make code more easy to read;
>
> Signed-off-by: Maxim Uvarov 
> ---
>  platform/linux-generic/odp_queue.c | 36
> +++-
>  1 file changed, 15 insertions(+), 21 deletions(-)
>
> diff --git a/platform/linux-generic/odp_queue.c
> b/platform/linux-generic/odp_queue.c
> index 342ffa2..0f7fd15 100644
> --- a/platform/linux-generic/odp_queue.c
> +++ b/platform/linux-generic/odp_queue.c
> @@ -234,10 +234,8 @@ int odp_queue_lock_count(odp_queue_t handle)
>
>  odp_queue_t odp_queue_create(const char *name, const odp_queue_param_t
> *param)
>  {
> -   uint32_t i;
> +   int i;
> queue_entry_t *queue;
> -   odp_queue_t handle = ODP_QUEUE_INVALID;
> -   odp_queue_type_t type;
> odp_queue_param_t default_param;
>
> if (param == NULL) {
> @@ -245,7 +243,7 @@ odp_queue_t odp_queue_create(const char *name, const
> odp_queue_param_t *param)
> param = &default_param;
> }
>
> -   for (i = 0; i < ODP_CONFIG_QUEUES; i++) {
> +   for (i = 0; i < odp_config_queues(); i++) {
>

This is needlessly inefficient.  The #defines are perfectly OK for
implementation internal use.  This makes an extra function call for each
iteration.


> queue = &queue_tbl->queue[i];
>
> if (queue->s.status != QUEUE_STATUS_FREE)
> @@ -253,33 +251,29 @@ odp_queue_t odp_queue_create(const char *name, const
> odp_queue_param_t *param)
>
> LOCK(&queue->s.lock);
> if (queue->s.status == QUEUE_STATUS_FREE) {
> -   if (queue_init(queue, name, param)) {
> +   if (queue_init(queue, name, param) ||
> +   queue->s.handle == ODP_QUEUE_INVALID) {
>

This is a redundant check as this field is initialized part part of
queue_init_global() and does not change


> UNLOCK(&queue->s.lock);
> -   return handle;
> +   return ODP_QUEUE_INVALID;
> }
>
> -   type = queue->s.type;
> -
> -   if (type == ODP_QUEUE_TYPE_SCHED)
> +   if (queue->s.type == ODP_QUEUE_TYPE_SCHED) {
> queue->s.status = QUEUE_STATUS_NOTSCHED;
> -   else
> +   if (schedule_queue_init(queue)) {
>

In the original code this routine is called with the queue unlocked. While
holding the lock across this call should still work, in general we want to
release locks as soon as possible so I don't see this as an improvement.


> +   ODP_ERR("schedule queue init
> failed\n");
> +   UNLOCK(&queue->s.lock);
> +   return ODP_QUEUE_INVALID;
> +   }
> +   } else {
> queue->s.status = QUEUE_STATUS_READY;
> -
> -   handle = queue->s.handle;
> +   }
> UNLOCK(&queue->s.lock);
> -   break;
> +   return queue->s.handle;
> }
> UNLOCK(&queue->s.lock);
> }
>
> -   if (handle != ODP_QUEUE_INVALID && type == ODP_QUEUE_TYPE_SCHED) {
> -   if (schedule_queue_init(queue)) {
> -   ODP_ERR("schedule queue init failed\n");
> -   return ODP_QUEUE_INVALID;
> -   }
> -   }
> -
> -   return handle;
> +   return ODP_QUEUE_INVALID;
>  }
>
>  void queue_destroy_finalize(queue_entry_t *queue)
> --
> 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] [PATCH] linux-generic: queue: avoid false positive when compiling with -Og

[Bill Fischofer]

On Thu, Apr 14, 2016 at 11:34 AM, Maxim Uvarov 
wrote:

> On 04/14/16 15:45, Bill Fischofer wrote:
>
>> ping.  Maxim, I believe this patch is the simplest solution to the issue.
>>
>>
> Bill,  this patch fixes existence problem with initializing variable. But
> it looks a little bit like a hack, where
> logically one function slitted on 2 pieces without any reason. (Or I do
> not see this reason.)
> Since nobody gave review yet, I will send my patch to list also. And we
> can merge mine or your.
>

It is a hack to work around a GCC compiler bug. The variable it's
complaining about can never be uninitialized and other optimization levels
clearly see that but -Og does not. But it's a very efficient hack. :)


>
> Maxim.
>
>
> On Fri, Apr 8, 2016 at 4:06 PM, Bill Fischofer > > wrote:
>>
>> After reviewing the code again, I think the patch stands as the
>> simplest solution to this compiler bug. Both type and handle are
>> needed because they are cached from the queue_entry while it is
>> locked and then referenced after it is unlocked. A direct
>> reference the internal fields is not legal unless the queue_entry
>> lock is held.
>>
>> On Thu, Apr 7, 2016 at 1:38 AM, Maxim Uvarov
>> mailto:maxim.uva...@linaro.org>> wrote:
>>
>> Mike, it think it's better to remove handle and type
>> completely. And if you already looking to that
>> function it might be reasonable to make it more readable, use
>> odp_config_queues() and use int
>> instead of uint32_t. Something like that:
>>
>>
>> --- a/platform/linux-generic/odp_queue.c
>> +++ b/platform/linux-generic/odp_queue.c
>> @@ -234,10 +234,8 @@ int odp_queue_lock_count(odp_queue_t handle)
>>
>>  odp_queue_t odp_queue_create(const char *name, const
>> odp_queue_param_t *param)
>>  {
>> -   uint32_t i;
>> +   int i;
>> queue_entry_t *queue;
>> -   odp_queue_t handle = ODP_QUEUE_INVALID;
>> -   odp_queue_type_t type;
>> odp_queue_param_t default_param;
>>
>> if (param == NULL) {
>> @@ -245,7 +243,7 @@ odp_queue_t odp_queue_create(const char
>> *name, const odp_queue_param_t *param)
>> param = &default_param;
>> }
>>
>> -   for (i = 0; i < ODP_CONFIG_QUEUES; i++) {
>> +   for (i = 0; i < odp_config_queues(); i++) {
>> queue = &queue_tbl->queue[i];
>>
>> if (queue->s.status != QUEUE_STATUS_FREE)
>> @@ -253,33 +251,29 @@ odp_queue_t odp_queue_create(const char
>> *name, const odp_queue_param_t *param)
>>
>> LOCK(&queue->s.lock);
>> if (queue->s.status == QUEUE_STATUS_FREE) {
>> -   if (queue_init(queue, name, param)) {
>> +   if (queue_init(queue, name, param) ||
>> +   queue->s.handle ==
>> ODP_QUEUE_INVALID) {
>> UNLOCK(&queue->s.lock);
>> -   return handle;
>> +   return ODP_QUEUE_INVALID;
>> }
>>
>> -   type = queue->s.type;
>> -
>> -   if (type == ODP_QUEUE_TYPE_SCHED)
>> +   if (queue->s.type ==
>> ODP_QUEUE_TYPE_SCHED) {
>> queue->s.status =
>> QUEUE_STATUS_NOTSCHED;
>> -   else
>> +   if (schedule_queue_init(queue)) {
>> +  ODP_ERR("schedule queue init failed\n");
>> + UNLOCK(&queue->s.lock);
>> +   return ODP_QUEUE_INVALID;
>> +   }
>> +   } else {
>> queue->s.status =
>> QUEUE_STATUS_READY;
>> -
>> -   handle = queue->s.handle;
>> +   }
>> UNLOCK(&queue->s.lock);
>> -   break;
>> +   return queue->s.handle;
>> }
>> UNLOCK(&queue->s.lock);
>> }
>>
>> -   if (handle != ODP_QUEUE_INVALID && type ==
>> ODP_QUEUE_TYPE_SCHED) {
>> -   if (schedule_queue_init(queue)) {
>> -   ODP_ERR("schedule queue init failed\n");
>> -   return ODP_QUEUE_INVALID;
>> -   }
>> -   }
>> -
>> -   return handle;
>> +   return ODP_QUEUE_INVALID;
>>  }
>>
>> Maxim.
>>
>> On 04/05/16 21:02, Mike Holmes wrote:
>

Re: [lng-odp] [RFC] api: crypto capability support

[Bill Fischofer]

On Thu, Apr 14, 2016 at 10:17 AM, Ivan Khoronzhuk <
ivan.khoronz...@linaro.org> wrote:

> Frequently used name: caps
>
> odp_crypto_capablity_t -> odp_crypto_caps_t


We're using capability consistently in other areas (classification, etc.)
so crypto should follow the same naming convention.  I prefer capability
since caps is a nonstandard abbreviation that may cause confusion.


>
>
> On 14.04.16 11:50, Savolainen, Petri (Nokia - FI/Espoo) wrote:
>
>> Hi,
>>
>> See under my proposal on using bit fields and a capability struct similar
>> to what we have already for other APIs. Application may easily check if
>> multiple ciphers are support and especially if those are implemented with
>> HW offload.
>>
>> -Petri
>>
>>
>> odp_crypto_ciphers_t ciphers;
>> odp_pktio_capability_t capa;
>>
>> ciphers.all_bits   = 0;
>> ciphers.3des_cbc   = 1;
>> ciphers.aes128_cbc = 1;
>>
>> odp_crypto_capability(&capa);
>>
>> if (capa.chiphers.all_bits & ciphers.all_bits == 0) {
>>   // 3des or aes are not supported
>>   ...
>>   return -1;
>> }
>>
>> if (capa.hw_chiphers.all_bits & ciphers.all_bits == 0) {
>>   // 3des or aes are not supported in HW
>>   ...
>> }
>>
>>
>>
>> typedef union odp_crypto_ciphers_t {
>> /** Cipher algorithms */
>> struct {
>> /** ODP_CIPHER_ALG_NULL */
>> uint32_t null   : 1;
>>
>> /** ODP_CIPHER_ALG_DES */
>> uint32_t des: 1;
>>
>> /** ODP_CIPHER_ALG_3DES_CBC */
>> uint32_t 3des_cbc   : 1;
>>
>> /** ODP_CIPHER_ALG_AES128_CBC */
>> uint32_t aes128_cbc : 1;
>>
>> /** ODP_CIPHER_ALG_AES128_GCM */
>> uint32_t aes128_gcm : 1;
>>
>> } bit;
>>
>> /** All bits of the bit field structure
>>   *
>>   * This field can be used to set/clear all flags, or bitwise
>>   * operations over the entire structure. */
>> uint32_t all_bits;
>> } odp_crypto_ciphers_t;
>>
>>
>> typedef union odp_crypto_auths_t {
>> /** Cipher algorithms */
>> struct {
>> /** ODP_AUTH_ALG_NULL */
>> uint32_t null   : 1;
>>
>> /** ODP_AUTH_ALG_MD5_96 */
>> uint32_t md5_96 : 1;
>>
>> /** ODP_AUTH_ALG_SHA256_128 */
>> uint32_t sha256_128 : 1;
>>
>> /** ODP_AUTH_ALG_AES128_GCM */
>> uint32_t aes128_gcm : 1;
>>
>> } bit;
>>
>> /** All bits of the bit field structure
>>   *
>>   * This field can be used to set/clear all flags, or bitwise
>>   * operations over the entire structure. */
>> uint32_t all_bits;
>> } odp_crypto_auths_t;
>>
>>
>> typedef struct odp_crypto_capability_t {
>> /** Maximum number of crypto sessions */
>> uint32_t max_sessions;
>>
>> /** Supported chipher algorithms */
>> odp_crypto_ciphers_t chiphers;
>>
>> /** Chipher algorithms implemented with HW offload */
>> odp_crypto_ciphers_t hw_chiphers;
>>
>> /** Supported authentication algorithms */
>> odp_crypto_auths_t auths;
>>
>> /** Authentication algorithms implemented with HW offload */
>> odp_crypto_auths_t hw_auths;
>>
>> } odp_crypto_capability_t;
>>
>>
>> /**
>>   * Query crypto capabilities
>>   *
>>   * Outputs crypto capabilities on success.
>>   *
>>   * @param[out] capa   Pointer to capability structure for output
>>   *
>>   * @retval 0 on success
>>   * @retval <0 on failure
>>   */
>> int odp_crypto_capability(odp_pktio_capability_t *capa);
>>
>>
>>
>>
>> -Original Message-
>>> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT
>>> Balakrishna Garapati
>>> Sent: Wednesday, April 13, 2016 3:33 PM
>>> To: lng-odp@lists.linaro.org
>>> Subject: [lng-odp] [RFC] api: crypto capability support
>>>
>>> This RFC provides the support for the applicationis to inquire the given
>>> cipher, authentication algorithms
>>>
>>> Signed-off-by: Balakrishna Garapati 
>>> ---
>>>   include/odp/api/spec/crypto.h | 30 ++
>>>   1 file changed, 30 insertions(+)
>>>
>>> diff --git a/include/odp/api/spec/crypto.h
>>> b/include/odp/api/spec/crypto.h
>>> index 41beedb..0def211 100644
>>> --- a/include/odp/api/spec/crypto.h
>>> +++ b/include/odp/api/spec/crypto.h
>>> @@ -254,6 +254,18 @@ typedef struct odp_crypto_op_result {
>>>   } odp_crypto_op_result_t;
>>>
>>>   /**
>>> + * Crypto API capability result
>>> + */
>>> +typedef enum odp_crypto_capability_t {
>>> +   /** crypto algorithm not supported */
>>> +   ODP_CRYPTO_NO_SUPPORT = 0,
>>> +   /** crypto algorithm supported in hardware */
>>> +   ODP_CRYPTO_HW_SUPPORT,
>>> +   /** crypto algortihm supported in software */
>>> +   ODP_CRYPTO_SW_SUPPORT
>>> +} odp_crypto_capability_t;
>>> +
>>> +/**
>>>* Crypto session creation (s

Re: [lng-odp] [API-NEXT PATCH] api: queue: add queue context length

[Bill Fischofer]

On Thu, Apr 14, 2016 at 9:21 AM, Petri Savolainen <
petri.savolai...@nokia.com> wrote:

> Added queue context length parameter, which is a hint for
> the implementation for how much to prefetch. Added the same in
> to the set function. It's not needed for get function since
> application can store the same into context data and
> implementation may avoid to save the length if it does not
> need it (e.g. always prefetches a fixed number of bytes).
>

The code looks fine, but I'm a bit confused about this comment.  Is the
length the actual byte length of the context or is it a prefetch length
request?  If the latter (i.e., the actual struct pointed to by the user
context is may be longer than this field) then perhaps prefetch_len would
be a better name to make it clear what its intended purpose is.


>
> Signed-off-by: Petri Savolainen 
> ---
>  include/odp/api/spec/queue.h  |  9 -
>  platform/linux-generic/odp_queue.c|  3 ++-
>  test/validation/queue/queue.c | 10 ++
>  test/validation/scheduler/scheduler.c |  6 +++---
>  4 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/include/odp/api/spec/queue.h b/include/odp/api/spec/queue.h
> index 51d94a2..ecb1a6d 100644
> --- a/include/odp/api/spec/queue.h
> +++ b/include/odp/api/spec/queue.h
> @@ -139,6 +139,12 @@ typedef struct odp_queue_param_t {
>   * pointer for prefetching the context data. Default value of the
>   * pointer is NULL. */
> void *context;
> +
> +   /** Queue context data length in bytes
> + *
> + * The implementation may use this value as a hint for the number
> of
> + * context data bytes to prefetch. Default value is zero (no
> hint). */
> +   uint32_t context_len;
>  } odp_queue_param_t;
>
>  /**
> @@ -191,11 +197,12 @@ odp_queue_t odp_queue_lookup(const char *name);
>   *
>   * @param queueQueue handle
>   * @param context  Address to the queue context
> + * @param len  Queue context data length in bytes
>   *
>   * @retval 0 on success
>   * @retval <0 on failure
>   */
> -int odp_queue_context_set(odp_queue_t queue, void *context);
> +int odp_queue_context_set(odp_queue_t queue, void *context, uint32_t len);
>
>  /**
>   * Get queue context
> diff --git a/platform/linux-generic/odp_queue.c
> b/platform/linux-generic/odp_queue.c
> index 342ffa2..9a9462d 100644
> --- a/platform/linux-generic/odp_queue.c
> +++ b/platform/linux-generic/odp_queue.c
> @@ -344,7 +344,8 @@ int odp_queue_destroy(odp_queue_t handle)
> return 0;
>  }
>
> -int odp_queue_context_set(odp_queue_t handle, void *context)
> +int odp_queue_context_set(odp_queue_t handle, void *context,
> + uint32_t len ODP_UNUSED)
>  {
> queue_entry_t *queue;
> queue = queue_to_qentry(handle);
> diff --git a/test/validation/queue/queue.c b/test/validation/queue/queue.c
> index 50c6857..96d21fa 100644
> --- a/test/validation/queue/queue.c
> +++ b/test/validation/queue/queue.c
> @@ -12,7 +12,7 @@
>  #define MSG_POOL_SIZE   (4 * 1024 * 1024)
>  #define CONFIG_MAX_ITERATION(100)
>
> -static int queue_contest = 0xff;
> +static int queue_context = 0xff;
>  static odp_pool_t pool;
>
>  int queue_suite_init(void)
> @@ -76,10 +76,11 @@ void queue_test_sunnydays(void)
> CU_ASSERT_EQUAL(ODP_SCHED_SYNC_PARALLEL,
> odp_queue_sched_type(queue_id));
>
> -   CU_ASSERT(0 == odp_queue_context_set(queue_id, &queue_contest));
> +   CU_ASSERT(0 == odp_queue_context_set(queue_id, &queue_context,
> +sizeof(queue_context)));
>
> prtn = odp_queue_context(queue_id);
> -   CU_ASSERT(&queue_contest == (int *)prtn);
> +   CU_ASSERT(&queue_context == (int *)prtn);
>
> msg_pool = odp_pool_lookup("msg_pool");
> buf = odp_buffer_alloc(msg_pool);
> @@ -144,7 +145,8 @@ void queue_test_info(void)
> /* Create a plain queue and set context */
> q_plain = odp_queue_create(nq_plain, NULL);
> CU_ASSERT(ODP_QUEUE_INVALID != q_plain);
> -   CU_ASSERT(odp_queue_context_set(q_plain, q_plain_ctx) == 0);
> +   CU_ASSERT(odp_queue_context_set(q_plain, q_plain_ctx,
> +   sizeof(q_plain_ctx)) == 0);
>
> /* Create a scheduled ordered queue with explicitly set params */
> odp_queue_param_init(¶m);
> diff --git a/test/validation/scheduler/scheduler.c
> b/test/validation/scheduler/scheduler.c
> index ae98401..dae859e 100644
> --- a/test/validation/scheduler/scheduler.c
> +++ b/test/validation/scheduler/scheduler.c
> @@ -592,7 +592,7 @@ static void chaos_run(unsigned int qtype)
> CU_ASSERT_FATAL(globals->chaos_q[i].handle !=
> ODP_QUEUE_INVALID);
> rc = odp_queue_context_set(globals->chaos_q[i].handle,
> -  CHAOS_NDX_TO_PTR(i));
> +  C

Re: [lng-odp] New traffic_mngr API

[Bill Fischofer]

This is very good discussion, but +cc the ODP mailing list as this sort of
detail belongs on the list so that others can see and contribute to it.

Bill

On Thu, Apr 14, 2016 at 12:20 PM, Barry Spinney 
wrote:

>
>
> So my latest comments are in *green* below.  We are starting to run out
>
> of bold primary colors, so we had better settle this soon.
>
>
>
> OK, in summary I will remove priority setting, require TCP as a condition
>
> for setting ECN and research the ability/need to change a subset of
>
> of the dscp field (perhaps a dscp mask parameter).
>
>
>
> Thanx Barry.
>
>
>
>
>
> *From:* Bala Manoharan [mailto:bala.manoha...@linaro.org]
> *Sent:* Thursday, April 14, 2016 12:13 PM
>
> *To:* Barry Spinney
> *Cc:* Bill Fischofer; Savolainen, Petri (Nokia - FI/Espoo)
> *Subject:* Re: New traffic_mngr API
>
>
>
> Hi Barry,
>
>
>
> My comments are in blue below.
>
>
> Regards,
> Bala
>
>
>
> On 14 April 2016 at 20:45, Barry Spinney  wrote:
>
>
>
> My comments are in red below.
>
> In summary, as indicated by my comments below, the ONLY TM marking
>
> change I am planning on doing NOW is the merge the IPv4 and IPv6 function
>
> calls and capability fields.  Based upon additional feedback from you
> (see Questions
>
> included in the red comments) and others like Petri and Bill, there could
> still be
>
> more changes.
>
>
>
> Thanx Barry
>
>
>
> *From:* Bala Manoharan [mailto:bala.manoha...@linaro.org]
> *Sent:* Thursday, April 14, 2016 8:01 AM
> *To:* Barry Spinney
> *Cc:* Bill Fischofer; Savolainen, Petri (Nokia - FI/Espoo)
> *Subject:* Re: New traffic_mngr API
>
> Hi Barry,
>
> IMO, we can drop the idea of having priority value (both for vlan and
> dscp) set as part of marking in this first version (Monarch version) so
> that the API gets simplified.
>
> So, my question is WHY did Cavium consider having the ability to set the
> priority based on egress color valuable in the first place?
>
> And then why drop this feature?
>
> So my thinking is that if Cavium had legitimate uses for this capability
> it seems reasonable that some other SOC will eventually want it.
>
> That leads me to want to keep it.
>
> HOWEVER IF Cavium eliminated this capability because further customer use
> case research showed no one wanted this feature,
>
> then that would lead me to eliminate it.
>
> So I propose keeping it UNTIL I hear from you that Cavium discovered that
> NO one wanted or could benefit from this feature.
>
>
>
> [Bala] I can say from Cavium that this feature of changing priority will
> NOT be implemented since there is no use-case and IMO we can remove this
> ability. We can add this feature in the future if some other platforms
> support the same.
>
>
>
> *[Barry]  Ok OK, I give up.  I will remove priority changing.*
>
>
>
> int odp_tm_vlan_marking(odp_tm_t   odp_tm,
>
> odp_packet_color_t color,
>
> odp_bool_t priority_enabled,
>
> uint8_tnew_priority,
>
> odp_bool_t
> drop_eligible_enabled);
>
> In your proposed vlan marking API defined above it expects the user to
> provide both drop eligibility and new_priority value, hence if a platform
> supports only drop eligibility then the function cannot succeed since it
> currently expects the implementation to support both.
>
> No. The new_priority value is ignored unless priority_enabled is TRUE.  If
> the platform only supports the setting of drop_eligible_enabled, then
>
> an ODP App can detect this be calling this function twice (per color?) –
> once with one enabled TRUE and the other FALSE and then vice versa.
>
> If the call fails then the App knows that that capability does not exist
> on this platform (at least for that color).  If BOTH calls fail for SOME
> color
>
> (say green) but if at least one succeeded for a different color, then the
> App knows that that color is not supported for Marking.
>
>
>
> [Bala] Yes. My concern was to avoid calling of this function twice by the
> application instead we can split this into two functions one setting
> drop_eligibility and other setting priority so that for platforms which do
> not set priority value there is no need to configure this boolean.
>
>
>
> Also you have defined two different APIs for TOS marking each one for ipv4
> and ipv6, I am not sure if such a differentiation is needed, we can combine
> them into a single API.
>
> I also don’t know if such a differentiation is needed.  I have no problem
> combining them into a single *_IP_TOS_* function call .
>
> [Bala] Okay.
>
>
>
> *[Barry] I have completed the changes to traffic_mngr.h  and
> platform/linux-generic/include/odp_traffic_mngr_internal.h *
>
> *to** merge these into ONE, and am almost finished with the
> platform/linux-generic/odp_traffic_mngr.c  changes.*
>
>
>
> We can have the following assumptions while defining the Marking API for
> simplicity
>
> * The marking APIs

Re: [lng-odp] [PATCH] linux-generic: rework odp_queue_create

[Maxim Uvarov]

On 14 April 2016 at 22:16, Bill Fischofer  wrote:

>
>
> On Thu, Apr 14, 2016 at 11:35 AM, Maxim Uvarov 
> wrote:
>
>> - use odp_config_queues() to get number of queues and int cycle
>>   iterrator. That makes it more easy to reuse that function for
>>   other platfroms.;
>> - remove declaring variables to invalid at the bottom and check for
>>   them down the code;
>> - make code more easy to read;
>>
>> Signed-off-by: Maxim Uvarov 
>> ---
>>  platform/linux-generic/odp_queue.c | 36
>> +++-
>>  1 file changed, 15 insertions(+), 21 deletions(-)
>>
>> diff --git a/platform/linux-generic/odp_queue.c
>> b/platform/linux-generic/odp_queue.c
>> index 342ffa2..0f7fd15 100644
>> --- a/platform/linux-generic/odp_queue.c
>> +++ b/platform/linux-generic/odp_queue.c
>> @@ -234,10 +234,8 @@ int odp_queue_lock_count(odp_queue_t handle)
>>
>>  odp_queue_t odp_queue_create(const char *name, const odp_queue_param_t
>> *param)
>>  {
>> -   uint32_t i;
>> +   int i;
>> queue_entry_t *queue;
>> -   odp_queue_t handle = ODP_QUEUE_INVALID;
>> -   odp_queue_type_t type;
>> odp_queue_param_t default_param;
>>
>> if (param == NULL) {
>> @@ -245,7 +243,7 @@ odp_queue_t odp_queue_create(const char *name, const
>> odp_queue_param_t *param)
>> param = &default_param;
>> }
>>
>> -   for (i = 0; i < ODP_CONFIG_QUEUES; i++) {
>> +   for (i = 0; i < odp_config_queues(); i++) {
>>
>
> This is needlessly inefficient.  The #defines are perfectly OK for
> implementation internal use.  This makes an extra function call for each
> iteration.
>
>
>> queue = &queue_tbl->queue[i];
>>
>> if (queue->s.status != QUEUE_STATUS_FREE)
>> @@ -253,33 +251,29 @@ odp_queue_t odp_queue_create(const char *name,
>> const odp_queue_param_t *param)
>>
>> LOCK(&queue->s.lock);
>> if (queue->s.status == QUEUE_STATUS_FREE) {
>> -   if (queue_init(queue, name, param)) {
>> +   if (queue_init(queue, name, param) ||
>> +   queue->s.handle == ODP_QUEUE_INVALID) {
>>
>
> This is a redundant check as this field is initialized part part of
> queue_init_global() and does not change
>
>
>> UNLOCK(&queue->s.lock);
>> -   return handle;
>> +   return ODP_QUEUE_INVALID;
>> }
>>
>> -   type = queue->s.type;
>> -
>> -   if (type == ODP_QUEUE_TYPE_SCHED)
>> +   if (queue->s.type == ODP_QUEUE_TYPE_SCHED) {
>> queue->s.status = QUEUE_STATUS_NOTSCHED;
>> -   else
>> +   if (schedule_queue_init(queue)) {
>>
>
> In the original code this routine is called with the queue unlocked. While
> holding the lock across this call should still work, in general we want to
> release locks as soon as possible so I don't see this as an improvement.
>
>

ah, that is main comment.  If we want schedule_queue_init() run without
lock than it's better to use current code, I think, so I will apply your
patch.


One small note about current code which I just realized (might be it's too
late for today).

schedule_queue_init() may fail. Should we in that case set queue->s.status
back to QUEUE_STATUS_FREE  before return ODP_QUEUE_INVALID ?

Maxim.




> +   ODP_ERR("schedule queue init
>> failed\n");
>> +   UNLOCK(&queue->s.lock);
>> +   return ODP_QUEUE_INVALID;
>> +   }
>> +   } else {
>> queue->s.status = QUEUE_STATUS_READY;
>> -
>> -   handle = queue->s.handle;
>> +   }
>> UNLOCK(&queue->s.lock);
>> -   break;
>> +   return queue->s.handle;
>> }
>> UNLOCK(&queue->s.lock);
>> }
>>
>> -   if (handle != ODP_QUEUE_INVALID && type == ODP_QUEUE_TYPE_SCHED) {
>> -   if (schedule_queue_init(queue)) {
>> -   ODP_ERR("schedule queue init failed\n");
>> -   return ODP_QUEUE_INVALID;
>> -   }
>> -   }
>> -
>> -   return handle;
>> +   return ODP_QUEUE_INVALID;
>>  }
>>
>>  void queue_destroy_finalize(queue_entry_t *queue)
>> --
>> 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] [PATCH] linux-generic: queue: avoid false positive when compiling with -Og

[Mike Holmes]

Is this not a reference to cases where we never pass through the following
 code ?

if (queue->s.status == QUEUE_STATUS_FREE)  is never true then type is never
set.

Can a bug elsewhere create such a case that leads to a cascading error that
appears far from where it originated?




On 14 April 2016 at 15:20, Bill Fischofer  wrote:

>
>
> On Thu, Apr 14, 2016 at 11:34 AM, Maxim Uvarov 
> wrote:
>
>> On 04/14/16 15:45, Bill Fischofer wrote:
>>
>>> ping.  Maxim, I believe this patch is the simplest solution to the issue.
>>>
>>>
>> Bill,  this patch fixes existence problem with initializing variable. But
>> it looks a little bit like a hack, where
>> logically one function slitted on 2 pieces without any reason. (Or I do
>> not see this reason.)
>> Since nobody gave review yet, I will send my patch to list also. And we
>> can merge mine or your.
>>
>
> It is a hack to work around a GCC compiler bug. The variable it's
> complaining about can never be uninitialized and other optimization levels
> clearly see that but -Og does not. But it's a very efficient hack. :)
>
>
>>
>> Maxim.
>>
>>
>> On Fri, Apr 8, 2016 at 4:06 PM, Bill Fischofer >> > wrote:
>>>
>>> After reviewing the code again, I think the patch stands as the
>>> simplest solution to this compiler bug. Both type and handle are
>>> needed because they are cached from the queue_entry while it is
>>> locked and then referenced after it is unlocked. A direct
>>> reference the internal fields is not legal unless the queue_entry
>>> lock is held.
>>>
>>> On Thu, Apr 7, 2016 at 1:38 AM, Maxim Uvarov
>>> mailto:maxim.uva...@linaro.org>> wrote:
>>>
>>> Mike, it think it's better to remove handle and type
>>> completely. And if you already looking to that
>>> function it might be reasonable to make it more readable, use
>>> odp_config_queues() and use int
>>> instead of uint32_t. Something like that:
>>>
>>>
>>> --- a/platform/linux-generic/odp_queue.c
>>> +++ b/platform/linux-generic/odp_queue.c
>>> @@ -234,10 +234,8 @@ int odp_queue_lock_count(odp_queue_t handle)
>>>
>>>  odp_queue_t odp_queue_create(const char *name, const
>>> odp_queue_param_t *param)
>>>  {
>>> -   uint32_t i;
>>> +   int i;
>>> queue_entry_t *queue;
>>> -   odp_queue_t handle = ODP_QUEUE_INVALID;
>>> -   odp_queue_type_t type;
>>> odp_queue_param_t default_param;
>>>
>>> if (param == NULL) {
>>> @@ -245,7 +243,7 @@ odp_queue_t odp_queue_create(const char
>>> *name, const odp_queue_param_t *param)
>>> param = &default_param;
>>> }
>>>
>>> -   for (i = 0; i < ODP_CONFIG_QUEUES; i++) {
>>> +   for (i = 0; i < odp_config_queues(); i++) {
>>> queue = &queue_tbl->queue[i];
>>>
>>> if (queue->s.status != QUEUE_STATUS_FREE)
>>> @@ -253,33 +251,29 @@ odp_queue_t odp_queue_create(const char
>>> *name, const odp_queue_param_t *param)
>>>
>>> LOCK(&queue->s.lock);
>>> if (queue->s.status == QUEUE_STATUS_FREE) {
>>> -   if (queue_init(queue, name, param)) {
>>> +   if (queue_init(queue, name, param) ||
>>> +   queue->s.handle ==
>>> ODP_QUEUE_INVALID) {
>>> UNLOCK(&queue->s.lock);
>>> -   return handle;
>>> +   return ODP_QUEUE_INVALID;
>>> }
>>>
>>> -   type = queue->s.type;
>>> -
>>> -   if (type == ODP_QUEUE_TYPE_SCHED)
>>> +   if (queue->s.type ==
>>> ODP_QUEUE_TYPE_SCHED) {
>>> queue->s.status =
>>> QUEUE_STATUS_NOTSCHED;
>>> -   else
>>> +   if (schedule_queue_init(queue)) {
>>> +  ODP_ERR("schedule queue init failed\n");
>>> + UNLOCK(&queue->s.lock);
>>> +   return ODP_QUEUE_INVALID;
>>> +   }
>>> +   } else {
>>> queue->s.status =
>>> QUEUE_STATUS_READY;
>>> -
>>> -   handle = queue->s.handle;
>>> +   }
>>> UNLOCK(&queue->s.lock);
>>> -   break;
>>> +   return queue->s.handle;
>>> }
>>> UNLOCK(&queue->s.lock);
>>> }
>>>
>>> -   if (handle != ODP_QUEUE_INVALID && type ==
>>> 

Re: [lng-odp] [PATCH] linux-generic: queue: avoid false positive when compiling with -Og

[Bill Fischofer]

On Thu, Apr 14, 2016 at 3:14 PM, Mike Holmes  wrote:

> Is this not a reference to cases where we never pass through the following
>  code ?
>
> if (queue->s.status == QUEUE_STATUS_FREE)  is never true then type is
> never set.
>
> Can a bug elsewhere create such a case that leads to a cascading error
> that appears far from where it originated?
>
>
>
No.  The compiler is stating (falsely) that type may be uninitialized.  But
type is only ever referenced if handle != ODP_QUEUE_INVALID and since
handle is initialized to ODP_QUEUE_INVALID the only way it gets set to a
non-invalid value is if a queue entry is being processed in which case type
is set to queue->s.type.  GCC clearly sees this at other optimization
levels.  The bug is with -Og.


>
>
> On 14 April 2016 at 15:20, Bill Fischofer 
> wrote:
>
>>
>>
>> On Thu, Apr 14, 2016 at 11:34 AM, Maxim Uvarov 
>> wrote:
>>
>>> On 04/14/16 15:45, Bill Fischofer wrote:
>>>
 ping.  Maxim, I believe this patch is the simplest solution to the
 issue.


>>> Bill,  this patch fixes existence problem with initializing variable.
>>> But it looks a little bit like a hack, where
>>> logically one function slitted on 2 pieces without any reason. (Or I do
>>> not see this reason.)
>>> Since nobody gave review yet, I will send my patch to list also. And we
>>> can merge mine or your.
>>>
>>
>> It is a hack to work around a GCC compiler bug. The variable it's
>> complaining about can never be uninitialized and other optimization levels
>> clearly see that but -Og does not. But it's a very efficient hack. :)
>>
>>
>>>
>>> Maxim.
>>>
>>>
>>> On Fri, Apr 8, 2016 at 4:06 PM, Bill Fischofer <
 bill.fischo...@linaro.org > wrote:

 After reviewing the code again, I think the patch stands as the
 simplest solution to this compiler bug. Both type and handle are
 needed because they are cached from the queue_entry while it is
 locked and then referenced after it is unlocked. A direct
 reference the internal fields is not legal unless the queue_entry
 lock is held.

 On Thu, Apr 7, 2016 at 1:38 AM, Maxim Uvarov
 mailto:maxim.uva...@linaro.org>> wrote:

 Mike, it think it's better to remove handle and type
 completely. And if you already looking to that
 function it might be reasonable to make it more readable, use
 odp_config_queues() and use int
 instead of uint32_t. Something like that:


 --- a/platform/linux-generic/odp_queue.c
 +++ b/platform/linux-generic/odp_queue.c
 @@ -234,10 +234,8 @@ int odp_queue_lock_count(odp_queue_t
 handle)

  odp_queue_t odp_queue_create(const char *name, const
 odp_queue_param_t *param)
  {
 -   uint32_t i;
 +   int i;
 queue_entry_t *queue;
 -   odp_queue_t handle = ODP_QUEUE_INVALID;
 -   odp_queue_type_t type;
 odp_queue_param_t default_param;

 if (param == NULL) {
 @@ -245,7 +243,7 @@ odp_queue_t odp_queue_create(const char
 *name, const odp_queue_param_t *param)
 param = &default_param;
 }

 -   for (i = 0; i < ODP_CONFIG_QUEUES; i++) {
 +   for (i = 0; i < odp_config_queues(); i++) {
 queue = &queue_tbl->queue[i];

 if (queue->s.status != QUEUE_STATUS_FREE)
 @@ -253,33 +251,29 @@ odp_queue_t odp_queue_create(const char
 *name, const odp_queue_param_t *param)

 LOCK(&queue->s.lock);
 if (queue->s.status == QUEUE_STATUS_FREE) {
 -   if (queue_init(queue, name, param)) {
 +   if (queue_init(queue, name, param) ||
 +   queue->s.handle ==
 ODP_QUEUE_INVALID) {
 UNLOCK(&queue->s.lock);
 -   return handle;
 +   return ODP_QUEUE_INVALID;
 }

 -   type = queue->s.type;
 -
 -   if (type == ODP_QUEUE_TYPE_SCHED)
 +   if (queue->s.type ==
 ODP_QUEUE_TYPE_SCHED) {
 queue->s.status =
 QUEUE_STATUS_NOTSCHED;
 -   else
 +   if (schedule_queue_init(queue))
 {
 +  ODP_ERR("schedule queue init failed\n");
 + UNLOCK(&queue->s.lock);
 +   return
 ODP_QUEUE_INV

Re: [lng-odp] [PATCH] linux-generic: rework odp_queue_create

[Bill Fischofer]

On Thu, Apr 14, 2016 at 2:50 PM, Maxim Uvarov 
wrote:

>
>
> On 14 April 2016 at 22:16, Bill Fischofer 
> wrote:
>
>>
>>
>> On Thu, Apr 14, 2016 at 11:35 AM, Maxim Uvarov 
>> wrote:
>>
>>> - use odp_config_queues() to get number of queues and int cycle
>>>   iterrator. That makes it more easy to reuse that function for
>>>   other platfroms.;
>>> - remove declaring variables to invalid at the bottom and check for
>>>   them down the code;
>>> - make code more easy to read;
>>>
>>> Signed-off-by: Maxim Uvarov 
>>> ---
>>>  platform/linux-generic/odp_queue.c | 36
>>> +++-
>>>  1 file changed, 15 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/platform/linux-generic/odp_queue.c
>>> b/platform/linux-generic/odp_queue.c
>>> index 342ffa2..0f7fd15 100644
>>> --- a/platform/linux-generic/odp_queue.c
>>> +++ b/platform/linux-generic/odp_queue.c
>>> @@ -234,10 +234,8 @@ int odp_queue_lock_count(odp_queue_t handle)
>>>
>>>  odp_queue_t odp_queue_create(const char *name, const odp_queue_param_t
>>> *param)
>>>  {
>>> -   uint32_t i;
>>> +   int i;
>>> queue_entry_t *queue;
>>> -   odp_queue_t handle = ODP_QUEUE_INVALID;
>>> -   odp_queue_type_t type;
>>> odp_queue_param_t default_param;
>>>
>>> if (param == NULL) {
>>> @@ -245,7 +243,7 @@ odp_queue_t odp_queue_create(const char *name, const
>>> odp_queue_param_t *param)
>>> param = &default_param;
>>> }
>>>
>>> -   for (i = 0; i < ODP_CONFIG_QUEUES; i++) {
>>> +   for (i = 0; i < odp_config_queues(); i++) {
>>>
>>
>> This is needlessly inefficient.  The #defines are perfectly OK for
>> implementation internal use.  This makes an extra function call for each
>> iteration.
>>
>>
>>> queue = &queue_tbl->queue[i];
>>>
>>> if (queue->s.status != QUEUE_STATUS_FREE)
>>> @@ -253,33 +251,29 @@ odp_queue_t odp_queue_create(const char *name,
>>> const odp_queue_param_t *param)
>>>
>>> LOCK(&queue->s.lock);
>>> if (queue->s.status == QUEUE_STATUS_FREE) {
>>> -   if (queue_init(queue, name, param)) {
>>> +   if (queue_init(queue, name, param) ||
>>> +   queue->s.handle == ODP_QUEUE_INVALID) {
>>>
>>
>> This is a redundant check as this field is initialized part part of
>> queue_init_global() and does not change
>>
>>
>>> UNLOCK(&queue->s.lock);
>>> -   return handle;
>>> +   return ODP_QUEUE_INVALID;
>>> }
>>>
>>> -   type = queue->s.type;
>>> -
>>> -   if (type == ODP_QUEUE_TYPE_SCHED)
>>> +   if (queue->s.type == ODP_QUEUE_TYPE_SCHED) {
>>> queue->s.status = QUEUE_STATUS_NOTSCHED;
>>> -   else
>>> +   if (schedule_queue_init(queue)) {
>>>
>>
>> In the original code this routine is called with the queue unlocked.
>> While holding the lock across this call should still work, in general we
>> want to release locks as soon as possible so I don't see this as an
>> improvement.
>>
>>
>
> ah, that is main comment.  If we want schedule_queue_init() run without
> lock than it's better to use current code, I think, so I will apply your
> patch.
>
>
> One small note about current code which I just realized (might be it's too
> late for today).
>
> schedule_queue_init() may fail. Should we in that case set queue->s.status
> back to QUEUE_STATUS_FREE  before
>

That's a good catch.  However that should be done by odp_queue_destroy()
since at the point of the failure the queue lock is not held.  It's a one
line fix.  If you want to add it to this patch I'm fine with that or I can
submit a follow-up bug and patch.


> return ODP_QUEUE_INVALID ?
>
> Maxim.
>
>
>
>
>> +   ODP_ERR("schedule queue init
>>> failed\n");
>>> +   UNLOCK(&queue->s.lock);
>>> +   return ODP_QUEUE_INVALID;
>>> +   }
>>> +   } else {
>>> queue->s.status = QUEUE_STATUS_READY;
>>> -
>>> -   handle = queue->s.handle;
>>> +   }
>>> UNLOCK(&queue->s.lock);
>>> -   break;
>>> +   return queue->s.handle;
>>> }
>>> UNLOCK(&queue->s.lock);
>>> }
>>>
>>> -   if (handle != ODP_QUEUE_INVALID && type == ODP_QUEUE_TYPE_SCHED)
>>> {
>>> -   if (schedule_queue_init(queue)) {
>>> -   ODP_ERR("schedule queue init failed\n");
>>> -   return ODP_QUEUE_INVALID;
>>> -   }
>>> -   }
>>> -
>>> -   return handle;
>>> +   return ODP_QUEUE_INVALID;
>>>  }
>>>
>>>  

Re: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add pktio id conversion APIs

[Bill Fischofer]

On Thu, Apr 14, 2016 at 5:21 AM, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolai...@nokia.com> wrote:

> Hi,
>
> There's no need to convert from index to handle. Application should save
> handle into its own data structures. Direct index from packet should be
> added, since it's very likely that packet descriptor stores an index
> anyway. odp_config_pktio_entries() will be renamed to
> odp_pktio_max_interfaces() or similar during config.h cleanup.
>
> So, I think only these two functions under need to be added.
>
>
I'll send a v2 with the suggested changes, however I see no harm in
providing the index->handle converter for symmetry.


> -Petri
>
>
> /**
>  * Get pktio interface index
>  *
>  * @param pktio   Packet IO handle
>  *
>  * @returnPacket interface index (0..odp_config_pktio_entries()-1)
>  * @retval <0 On failure (e.g., handle not valid)
>  */
> int odp_pktio_index(odp_pktio_t pktio);
>
>
> /**
>  * Packet input interface index
>  *
>  * Returns index of the packet IO interface which received the packet or
>  * <0 when the packet was allocated/reset by the application.
>  *
>  * @param pkt   Packet handle
>  *
>  * @return Packet interface index (0..odp_config_pktio_entries()-1)
>  * @retval <0  Packet was not received on any interface
>  */
> int odp_packet_input_index(odp_packet_t pkt);
>
>
>
> > -Original Message-
> > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of EXT
> > Bill Fischofer
> > Sent: Wednesday, April 13, 2016 10:19 PM
> > To: lng-odp@lists.linaro.org
> > Subject: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add pktio id
> > conversion APIs
> >
> > Add the two APIs odp_pktio_to_id() and odp_pktio_from_id() to enable
> > applications to convert PktIO handles to and from indices for managing
> > user contexts and related associated data.
> >
> > Signed-off-by: Bill Fischofer 
> > ---
> >  include/odp/api/spec/packet_io.h | 20 
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/include/odp/api/spec/packet_io.h
> > b/include/odp/api/spec/packet_io.h
> > index 466cab6..60fb722 100644
> > --- a/include/odp/api/spec/packet_io.h
> > +++ b/include/odp/api/spec/packet_io.h
> > @@ -908,6 +908,26 @@ int odp_pktio_skip_set(odp_pktio_t pktio, uint32_t
> > offset);
> >  int odp_pktio_headroom_set(odp_pktio_t pktio, uint32_t headroom);
> >
> >  /**
> > + * Get pktio id from handle
> > + *
> > + * @param pktio   Pktio handle
> > + *
> > + * @returnPktio index (0..odp_config_pktio_entries()-1)
> > + * @retval <0 Error (e.g., handle not valid)
> > + */
> > +int odp_pktio_to_id(odp_pktio_t pktio);
> > +
> > +/**
> > + * Get pktio handle from id
> > + *
> > + * @param id   Pktio index
> > + *
> > + * @return Pktio handle
> > + * @retval ODP_PKTIO_INVALID on error
> > + */
> > +odp_pktio_t odp_pktio_from_id(int id);
> > +
> > +/**
> >   * Get printable value for an odp_pktio_t
> >   *
> >   * @param pktio   odp_pktio_t handle to be printed
> > --
> > 2.5.0
> >
> > ___
> > 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] [API-NEXT PATCHv2 1/6] api: pktio: add pktio index conversion APIs

[Bill Fischofer]

Add the two APIs odp_pktio_to_index() and odp_pktio_from_index() to enable
applications to convert PktIO handles to and from indices for managing
user contexts and related associated data.

Signed-off-by: Bill Fischofer 
---
 include/odp/api/spec/packet_io.h | 20 
 1 file changed, 20 insertions(+)

diff --git a/include/odp/api/spec/packet_io.h b/include/odp/api/spec/packet_io.h
index 466cab6..bb14c43 100644
--- a/include/odp/api/spec/packet_io.h
+++ b/include/odp/api/spec/packet_io.h
@@ -908,6 +908,26 @@ int odp_pktio_skip_set(odp_pktio_t pktio, uint32_t offset);
 int odp_pktio_headroom_set(odp_pktio_t pktio, uint32_t headroom);
 
 /**
+ * Get pktio index from handle
+ *
+ * @param pktio   Pktio handle
+ *
+ * @returnPktio index (0..odp_config_pktio_entries()-1)
+ * @retval <0 Error (e.g., handle not valid)
+ */
+int odp_pktio_to_index(odp_pktio_t pktio);
+
+/**
+ * Get pktio handle from index
+ *
+ * @param ndx  Pktio index
+ *
+ * @return Pktio handle
+ * @retval ODP_PKTIO_INVALID on error
+ */
+odp_pktio_t odp_pktio_from_index(int ndx);
+
+/**
  * Get printable value for an odp_pktio_t
  *
  * @param pktio   odp_pktio_t handle to be printed
-- 
2.5.0

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

[lng-odp] [API-NEXT PATCHv2 3/6] validation: pktio: add validation tests for pktio index functions

[Bill Fischofer]

Signed-off-by: Bill Fischofer 
---
 test/validation/pktio/pktio.c | 22 ++
 test/validation/pktio/pktio.h |  1 +
 2 files changed, 23 insertions(+)

diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
index d52a520..10eba6c 100644
--- a/test/validation/pktio/pktio.c
+++ b/test/validation/pktio/pktio.c
@@ -943,6 +943,27 @@ void pktio_test_lookup(void)
CU_ASSERT(odp_pktio_lookup(iface_name[0]) == ODP_PKTIO_INVALID);
 }
 
+void pktio_test_index(void)
+{
+   odp_pktio_t pktio, pktio_inval = ODP_PKTIO_INVALID;
+   odp_pktio_param_t pktio_param;
+   int ndx;
+
+   odp_pktio_param_init(&pktio_param);
+   pktio_param.in_mode = ODP_PKTIN_MODE_SCHED;
+
+   pktio = odp_pktio_open(iface_name[0], default_pkt_pool, &pktio_param);
+   CU_ASSERT(pktio != ODP_PKTIO_INVALID);
+
+   ndx = odp_pktio_to_index(pktio);
+   CU_ASSERT(ndx >= 0);
+   CU_ASSERT(odp_pktio_to_index(pktio_inval) < 0);
+   CU_ASSERT(odp_pktio_from_index(ndx) == pktio);
+
+   CU_ASSERT(odp_pktio_close(pktio) == 0);
+   CU_ASSERT(odp_pktio_to_index(pktio) < 0);
+}
+
 static void pktio_test_print(void)
 {
odp_pktio_t pktio;
@@ -1770,6 +1791,7 @@ int pktio_suite_term(void)
 odp_testinfo_t pktio_suite_unsegmented[] = {
ODP_TEST_INFO(pktio_test_open),
ODP_TEST_INFO(pktio_test_lookup),
+   ODP_TEST_INFO(pktio_test_index),
ODP_TEST_INFO(pktio_test_print),
ODP_TEST_INFO(pktio_test_pktin_queue_config_direct),
ODP_TEST_INFO(pktio_test_pktin_queue_config_sched),
diff --git a/test/validation/pktio/pktio.h b/test/validation/pktio/pktio.h
index 14db6e1..495031b 100644
--- a/test/validation/pktio/pktio.h
+++ b/test/validation/pktio/pktio.h
@@ -24,6 +24,7 @@ void pktio_test_mac(void);
 void pktio_test_inq_remdef(void);
 void pktio_test_open(void);
 void pktio_test_lookup(void);
+void pktio_test_index(void);
 void pktio_test_inq(void);
 void pktio_test_pktin_queue_config_direct(void);
 void pktio_test_pktin_queue_config_sched(void);
-- 
2.5.0

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

[lng-odp] [API-NEXT PATCHv2 2/6] linux-generic: pktio: implement pktio index conversion functions

[Bill Fischofer]

Signed-off-by: Bill Fischofer 
---
 platform/linux-generic/odp_packet_io.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/platform/linux-generic/odp_packet_io.c 
b/platform/linux-generic/odp_packet_io.c
index 574f987..41d93e1 100644
--- a/platform/linux-generic/odp_packet_io.c
+++ b/platform/linux-generic/odp_packet_io.c
@@ -921,6 +921,24 @@ int odp_pktio_info(odp_pktio_t id, odp_pktio_info_t *info)
return 0;
 }
 
+int odp_pktio_to_index(odp_pktio_t pktio)
+{
+   pktio_entry_t *entry = get_pktio_entry(pktio);
+
+   if (!entry || is_free(entry))
+   return -1;
+
+   return pktio_to_id(pktio);
+}
+
+odp_pktio_t odp_pktio_from_index(int ndx)
+{
+   if (ndx < 0 || ndx > ODP_CONFIG_PKTIO_ENTRIES)
+   return ODP_PKTIO_INVALID;
+
+   return _odp_cast_scalar(odp_pktio_t, ndx + 1);
+}
+
 void odp_pktio_print(odp_pktio_t id)
 {
pktio_entry_t *entry;
-- 
2.5.0

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

[lng-odp] [API-NEXT PATCHv2 4/6] api: packet: add odp_packet_input_index() api

[Bill Fischofer]

Signed-off-by: Bill Fischofer 
---
 include/odp/api/spec/packet.h | 13 +
 1 file changed, 13 insertions(+)

diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h
index 7da353b..7606597 100644
--- a/include/odp/api/spec/packet.h
+++ b/include/odp/api/spec/packet.h
@@ -458,6 +458,19 @@ odp_pool_t odp_packet_pool(odp_packet_t pkt);
 odp_pktio_t odp_packet_input(odp_packet_t pkt);
 
 /**
+ * Packet input interface index
+ *
+ * Returns the index of the PktIO interface that received the packet, or
+ * <0 when the packet was allocated/reset by the application.
+ *
+ * @param pkt   Packet handle
+ *
+ * @return Packet interface index (0..odp_config_pktio_entries()-1)
+ * @retval <0  Packet was not received on any interface
+ */
+int odp_packet_input_index(odp_packet_t pkt);
+
+/**
  * User context pointer
  *
  * Return previously stored user context pointer.
-- 
2.5.0

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

[lng-odp] [API-NEXT PATCHv2 5/6] linux-generic: packet: implement odp_packet_input_index()

[Bill Fischofer]

Signed-off-by: Bill Fischofer 
---
 platform/linux-generic/odp_packet.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 2c44316..49e52dd 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -330,6 +330,11 @@ odp_pktio_t odp_packet_input(odp_packet_t pkt)
return odp_packet_hdr(pkt)->input;
 }
 
+int odp_packet_input_index(odp_packet_t pkt)
+{
+   return odp_pktio_to_index(odp_packet_hdr(pkt)->input);
+}
+
 void *odp_packet_user_ptr(odp_packet_t pkt)
 {
return odp_packet_hdr(pkt)->buf_hdr.buf_ctx;
-- 
2.5.0

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

[lng-odp] [API-NEXT PATCHv2 6/6] validation: test: add test for odp_packet_input_index()

[Bill Fischofer]

Signed-off-by: Bill Fischofer 
---
 test/validation/packet/packet.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/validation/packet/packet.c b/test/validation/packet/packet.c
index a764ed9..e28b9a6 100644
--- a/test/validation/packet/packet.c
+++ b/test/validation/packet/packet.c
@@ -252,6 +252,7 @@ void packet_test_basic_metadata(void)
CU_ASSERT(odp_packet_pool(pkt) != ODP_POOL_INVALID);
/* Packet was allocated by application so shouldn't have valid pktio. */
CU_ASSERT(odp_packet_input(pkt) == ODP_PKTIO_INVALID);
+   CU_ASSERT(odp_packet_input_index(pkt) < 0);
 
odp_packet_flow_hash_set(pkt, UINT32_MAX);
CU_ASSERT(odp_packet_has_flow_hash(pkt));
-- 
2.5.0

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

Re: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user context support for interfaces

[Bill Fischofer]

Hi Bogdan,

Patches to add the handle->index conversion functions have been posted.
The latest series begins at http://patches.opendataplane.org/patch/5575/

Please review this to see if it meets your needs.

Thanks.

Bill

On Thu, Apr 14, 2016 at 2:10 AM, Bogdan Pricope 
wrote:

> Hi Bill,
>
>
>
> Thank you for trying to push this further.
>
>
>
> A problem is that you are pushing details of your internal data
> organization into application. This may get messy to maintain especially if
> you decide to reorganize the table(s) at some point or increase number of
> pktios.
>
> Btw, why don’t you transform odp_pktio_t into this pktio index?
>
>
>
> Else, we can work with such index, but I don’t like the solution (no
> Reviewed-by from me).
>
>
>
> The two SW layers is a false problem: once you identified the right
> interface, it is up to application to find the right data structure inside
> each layer
>
>
>
> Bogdan
>
>
>
>
>
> *From:* Bill Fischofer [mailto:bill.fischo...@linaro.org]
> *Sent:* Wednesday, April 13, 2016 10:47 PM
> *To:* Ola Liljedahl 
> *Cc:* Savolainen, Petri (Nokia - FI/Espoo) ;
> Bogdan Pricope ; lng-odp@lists.linaro.org
>
> *Subject:* Re: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user
> context support for interfaces
>
>
>
>
>
> On Wed, Apr 13, 2016 at 2:40 PM, Ola Liljedahl 
> wrote:
>
>
>
>
>
> On 13 April 2016 at 21:26, Bill Fischofer 
> wrote:
>
> We already have odp_packet_input() so are you saying asking the
> application to write odp_pktio_id(odp_packet_input(pkt)) is too cumbersome?
>
> I would like to be able to avoid any unnecessary memory accesses. Having
> to first retrieve the packet IO handle and then dereference that data
> structure to retrieve the pktio ID (interface index) seems to require an
> unnecessary load to a data structure not necessarily in the cache (or is it
> likely the ODP pktio data structure will otherwise be referenced for every
> packet and thus resident i the L1 cache?).
>
>
>
> I think it is likely the packet metadata will actually contain the pktio
> ID/interface index, not the corresponding pktio handle. So return the index
> directly for use by the application as index into different application
> specific tables.
>
>
>
> Actually, the intended performance model for ODP is that packets are
> classified into flows and the flow context associated with the queue
> delivering the packet contains all the info the application needs.  Why try
> to tie information to individual packets other than the (mutable) metadata
> that the application is doing to be working with?
>
>
>
>
>
>
>
> Id can be renamed index if that's the preference.
>
>
>
> On Wed, Apr 13, 2016 at 2:22 PM, Ola Liljedahl 
> wrote:
>
>
>
>
>
> On 13 April 2016 at 19:38, Bill Fischofer 
> wrote:
>
>
>
>
>
> On Wed, Apr 13, 2016 at 12:36 PM, Bill Fischofer <
> bill.fischo...@linaro.org> wrote:
>
>
>
> On Wed, Apr 13, 2016 at 8:09 AM, Ola Liljedahl 
> wrote:
>
> On 8 April 2016 at 17:25, Savolainen, Petri (Nokia - FI/Espoo) <
> petri.savolai...@nokia.com> wrote:
>
>
>
>
>
> *From:* EXT Bogdan Pricope [mailto:bogdan.pric...@enea.com]
> *Sent:* Friday, April 08, 2016 4:50 PM
> *To:* Bill Fischofer ; Savolainen, Petri
> (Nokia - FI/Espoo) 
> *Cc:* lng-odp@lists.linaro.org
> *Subject:* RE: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user
> context support for interfaces
>
>
>
> Hi,
>
>
>
> Our path is:
>
> pktio = odp_packet_input(pkt);
>
> A step is invisible here:
>
> queue = odp_pktio_outq_getdef(pktio)
>
> ifnet = ofp_queue_context(queue)
>
>
>
> ifnet = ofp_get_ifnet_pktio(pktio);
>
>
>
> (Our current workaround is to iterate through the list of interfaces to
> find which interface has this pktio.)
>
>
>
> You are saying:
>
> pktio_id = odp_packet_input_id(pkt);
>
> ifnet = ofp_get_ifnet_pktio_id(pktio_id);
>
>
>
> Code generated with ID:
>
> pktio_id = pkt_hdr->pktio_id;
>
> ifnet = &ofp_ifnet_table[pktio_id]:
>
> I think a pktio integer identifier (defined as a small integer suitable
> for using as an array index) is the most generic solution. Different SW
> layers can use this pktio ID as an index into *different* tables (different
> SW layers will have different needs). A (single) ODP pktio context pointer
> only makes one SW layer happy (or will promote a monolithic SW design which
> will be fragile and difficult to extend with new functionality).
>
>
>
>
>
> That sounds reasonable.  So should we consider the following new APIs?
>
> Yes looks good to me except "id" in the name feels a bit short and not
> specific enough. What about "pktioid"? (well that doesn't look great
> either, I usually call this thing "ifindex" for interface index).
>
>
>
> But we also want an API to extract the pktio ID from a packet (which
> identifies which pktio the packet was received on).
>
> int odp_packet_pktioid(odp_packet_t pkt);
>
> Will return -1 for a packet that was not received on some interface (i.e.
> returned from odp_packet_alloc()).
>
>
>
>
>
> uint32_t odp_pktio_to_id(odp_pktio_t pkti

Re: [lng-odp] [API-NEXT PATCH v2 1/7] api: packet: add extend and trunc

[Bill Fischofer]

Thanks Bala.  There are a couple of glitches in the tests that I'm working
through, so a v3 will be posted later today to address those.  I'll include
your review in those.

There are no changes to these APIs as Petri's v2 patch defines them.  Just
grouping them here for easier integration along with their associated
implementation and CUnit tests.

On Thu, Apr 14, 2016 at 7:49 AM, Balasubramanian Manoharan <
bala.manoha...@linaro.org> wrote:

> These changes looks good to me.
> Reviewed-by: Balasubramanian Manoharan 
> 
>
>
> On Thursday 14 April 2016 01:41 PM, Savolainen, Petri (Nokia - FI/Espoo)
> wrote:
>
> Thanks for the review. Although, I’d prefer to get all 6 patches merged in
> one go, since reordering of these will cause conflicts. I’d continue to
> work locally on top of this set. Also the implementation patch set should
> not include these, but go on top. It’s hard to see if something was changed
> in the other set. Bill’s signed-off-by hints that something was changed but
> I guess nothing was changed, except the ordering compared to patches 3 and
> 5.
>
>
>
> Bala, are you OK with patches 3 and 5?
>
>
>
> -Petri
>
>
>
>
>
> *From:* EXT Bill Fischofer [mailto:bill.fischo...@linaro.org
> ]
> *Sent:* Wednesday, April 13, 2016 5:29 PM
> *To:* Savolainen, Petri (Nokia - FI/Espoo) 
> 
> *Cc:* LNG ODP Mailman List 
> 
> *Subject:* Re: [lng-odp] [API-NEXT PATCH v2 1/7] api: packet: add extend
> and trunc
>
>
>
> For parts 1, 2, 4, and 6
>
>
>
> Reviewed-by: Bill Fischofer < 
> bill.fischo...@linaro.org>
>
>
>
> I'm OK with merging those parts into api-next to allow parallel
> implementation/test work on them. I believe we  still need to get more
> input and feedback on parts 3 and 5 before these can be merged.
>
>
>
> On Wed, Apr 13, 2016 at 9:19 AM, Bill Fischofer 
> wrote:
>
> This latest definition looks good. I'll post a v4 of my implementation
> patch series to reflect it.
>
>
>
> On Wed, Apr 13, 2016 at 8:01 AM, Petri Savolainen <
> petri.savolai...@nokia.com> wrote:
>
> Added functions to extend / truncate packet head / tail more
> than current head/tailroom or segment lengths.
>
> Signed-off-by: Petri Savolainen < 
> petri.savolai...@nokia.com>
> ---
>  include/odp/api/spec/packet.h | 144
> ++
>  1 file changed, 144 insertions(+)
>
> diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h
> index 7da353b..6c88458 100644
> --- a/include/odp/api/spec/packet.h
> +++ b/include/odp/api/spec/packet.h
> @@ -406,6 +406,150 @@ void *odp_packet_push_tail(odp_packet_t pkt,
> uint32_t len);
>  void *odp_packet_pull_tail(odp_packet_t pkt, uint32_t len);
>
>  /**
> + * Extend packet head
> + *
> + * Increase packet data length at packet head. Functionality is analogous
> to
> + * odp_packet_push_head() when data length is extended up to headroom
> size.
> + * When data length is increased more than that, new segments are added
> into
> + * the packet head and old segment handles become invalid.
> + *
> + * A successful operation overwrites the packet handle with a new handle,
> which
> + * application must use as the reference to the packet instead of the old
> + * handle. Depending on the implementation, the old and new handles may be
> + * equal.
> + *
> + * The operation return value indicates if any packet data or metadata
> (e.g.
> + * user_area) were moved in memory during the operation. If some memory
> areas
> + * were moved, application must use new packet/segment handles to update
> + * data pointers. Otherwise, all old pointers remain valid.
> + *
> + * User is responsible to update packet metadata offsets when needed.
> Packet
> + * is not modified if operation fails.
> + *
> + * @param[in, out] pkt  Pointer to packet handle. A successful operation
> outputs
> + *  the new packet handle.
> + * @param len   Number of bytes to extend the head
> + * @param[out] data_ptr Pointer to output the new data pointer.
> + *  Ignored when NULL.
> + * @param[out] seg_len  Pointer to output segment length at 'data_ptr'
> above.
> + *  Ignored when NULL.
> + *
> + * @retval 0   Operation successful, old pointers remain valid
> + * @retval >0  Operation successful, old pointers need to be updated
> + * @retval <0  Operation failed (e.g. due to an allocation failure)
> + */
> +int odp_packet_extend_head(odp_packet_t *pkt, uint32_t len, void
> **data_ptr,
> +  uint32_t *seg_len);
> +
> +/**
> + * Truncate packet head
> + *
> + * Decrease packet data length at packet head. Functionality is analogous
> to
> + * odp_packet_pull_head() when data length is truncated less than the
> first
> + * segment data length. When data length is decreased more than that,
> some head
> + * segments are removed from the packet and old segment handles become
> invalid.
> + *
> + * A successful operation overwrites the packet handle with a new handle,
> which
> + * application must use

Re: [lng-odp] [PATCH] validation: pktio: remove octet check from stats test

[Bill Fischofer]

On Tue, Apr 5, 2016 at 11:16 AM, Zoltan Kiss  wrote:

> This test sets up two interface and connect them to each other, so in
> theory these two numbers should be the same. However when you use a pktio
> which doesn't have full control of the interface, it could happen that
> other players, e.g. various services of the operating system start to
> send traffic out on the newly created interfaces. It won't be visible
> for ODP when going out, but coming in it will increase the counters.
> This breaks the test on ODP-DPDK, unnecessarily. On ODP-Linux it does not,
> because it checks the system level statistics, not the ODP level ones.
>
> Signed-off-by: Zoltan Kiss 
>

Reviewed-by: Bill Fischofer 


> ---
> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
> index cb403a6..73b702c 100644
> --- a/test/validation/pktio/pktio.c
> +++ b/test/validation/pktio/pktio.c
> @@ -1256,7 +1256,6 @@ void pktio_test_statistics_counters(void)
> CU_ASSERT((stats[1].in_ucast_pkts == 0) ||
>   (stats[1].in_ucast_pkts >= (uint64_t)pkts));
> CU_ASSERT(stats[0].out_ucast_pkts ==
> stats[1].in_ucast_pkts);
> -   CU_ASSERT(stats[0].out_octets == stats[1].in_octets);
> CU_ASSERT((stats[0].out_octets == 0) ||
>   (stats[0].out_octets >=
>   (PKT_LEN_NORMAL * (uint64_t)pkts)));
> ___
> 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] [API-NEXT PATCHv5 00/10] add extend, trunc, and prefetch support

[Bill Fischofer]

This patch series implements the packet extend, trunc, and prefetch APIs.

This patch includes the validation tests but updated user guide documentation
will be a separate series as the new diagrams would be too distracting for this
series.

Note that part 8 has a couple of false positive checkpatch warnings. CU_ASSERT
is a macro that must be enclosed in braces in if/else clauses but checkpatch
doesn't understand this. Compilation will fail if they are removed.

Changes in v4:
- Use latest (Petri v2) API definitions for extend and truncate
- Also incorporate implemnetation and test for odp_packet_prefetch()

Changes in v5:
- Miscellaneous bug fixes in segment manipulation and head/tail calculation
- Use packet references in push/pull validation tests to match api needs

Bill Fischofer (6):
  linux-generic: packet: correct odp_packet_buf_len
  validation: packet: add test for correct odp_packet_buffer_len()
  linux-generic: packet: implement packet extend/trunc apis
  validation: packet: add tests for extend/trunc packet head/tail
  linux-generic: packet: implement odp_packet_prefetch()
  validation: packet: add validation test for odp_packet_prefetch()

Petri Savolainen (4):
  api: packet: add extend and trunc
  api: packet: re-organize file content
  api: packet: remove seg_buf_addr and seg_buf_len
  api: packet: add packet data prefetch

 include/odp/api/spec/packet.h  | 601 -
 .../linux-generic/include/odp_buffer_internal.h|   4 +
 .../linux-generic/include/odp_packet_internal.h|  43 ++
 platform/linux-generic/odp_packet.c|  96 +++-
 platform/linux-generic/odp_pool.c  |  75 +++
 test/validation/packet/packet.c| 239 +---
 test/validation/packet/packet.h|   1 +
 7 files changed, 727 insertions(+), 332 deletions(-)

-- 
2.5.0

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

[lng-odp] [API-NEXT PATCHv5 01/10] api: packet: add extend and trunc

[Bill Fischofer]

From: Petri Savolainen 

Added functions to extend / truncate packet head / tail more
than current head/tailroom or segment lengths.

Signed-off-by: Petri Savolainen 
Reviewed-by: Bill Fischofer 
Reviewed-by: Bala Manoharan 
Signed-off-by: Bill Fischofer 
---
 include/odp/api/spec/packet.h | 144 ++
 1 file changed, 144 insertions(+)

diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h
index 7da353b..6c88458 100644
--- a/include/odp/api/spec/packet.h
+++ b/include/odp/api/spec/packet.h
@@ -406,6 +406,150 @@ void *odp_packet_push_tail(odp_packet_t pkt, uint32_t 
len);
 void *odp_packet_pull_tail(odp_packet_t pkt, uint32_t len);
 
 /**
+ * Extend packet head
+ *
+ * Increase packet data length at packet head. Functionality is analogous to
+ * odp_packet_push_head() when data length is extended up to headroom size.
+ * When data length is increased more than that, new segments are added into
+ * the packet head and old segment handles become invalid.
+ *
+ * A successful operation overwrites the packet handle with a new handle, which
+ * application must use as the reference to the packet instead of the old
+ * handle. Depending on the implementation, the old and new handles may be
+ * equal.
+ *
+ * The operation return value indicates if any packet data or metadata (e.g.
+ * user_area) were moved in memory during the operation. If some memory areas
+ * were moved, application must use new packet/segment handles to update
+ * data pointers. Otherwise, all old pointers remain valid.
+ *
+ * User is responsible to update packet metadata offsets when needed. Packet
+ * is not modified if operation fails.
+ *
+ * @param[in, out] pkt  Pointer to packet handle. A successful operation 
outputs
+ *  the new packet handle.
+ * @param len   Number of bytes to extend the head
+ * @param[out] data_ptr Pointer to output the new data pointer.
+ *  Ignored when NULL.
+ * @param[out] seg_len  Pointer to output segment length at 'data_ptr' above.
+ *  Ignored when NULL.
+ *
+ * @retval 0   Operation successful, old pointers remain valid
+ * @retval >0  Operation successful, old pointers need to be updated
+ * @retval <0  Operation failed (e.g. due to an allocation failure)
+ */
+int odp_packet_extend_head(odp_packet_t *pkt, uint32_t len, void **data_ptr,
+  uint32_t *seg_len);
+
+/**
+ * Truncate packet head
+ *
+ * Decrease packet data length at packet head. Functionality is analogous to
+ * odp_packet_pull_head() when data length is truncated less than the first
+ * segment data length. When data length is decreased more than that, some head
+ * segments are removed from the packet and old segment handles become invalid.
+ *
+ * A successful operation overwrites the packet handle with a new handle, which
+ * application must use as the reference to the packet instead of the old
+ * handle. Depending on the implementation, the old and new handles may be
+ * equal.
+ *
+ * The operation return value indicates if any packet data or metadata (e.g.
+ * user_area) were moved in memory during the operation. If some memory areas
+ * were moved, application must use new packet/segment handles to update
+ * data pointers. Otherwise, all old pointers remain valid.
+ *
+ * User is responsible to update packet metadata offsets when needed. Packet
+ * is not modified if operation fails.
+ *
+ * @param[in, out] pkt  Pointer to packet handle. A successful operation 
outputs
+ *  the new packet handle.
+ * @param len   Number of bytes to truncate the head (0 ... packet_len)
+ * @param[out] data_ptr Pointer to output the new data pointer.
+ *  Ignored when NULL.
+ * @param[out] seg_len  Pointer to output segment length at 'data_ptr' above.
+ *  Ignored when NULL.
+ *
+ * @retval 0   Operation successful, old pointers remain valid
+ * @retval >0  Operation successful, old pointers need to be updated
+ * @retval <0  Operation failed
+ */
+int odp_packet_trunc_head(odp_packet_t *pkt, uint32_t len, void **data_ptr,
+ uint32_t *seg_len);
+
+/**
+ * Extend packet tail
+ *
+ * Increase packet data length at packet tail. Functionality is analogous to
+ * odp_packet_push_tail() when data length is extended up to tailroom size.
+ * When data length is increased more than that, new segments are added into
+ * the packet tail and old segment handles become invalid.
+ *
+ * A successful operation overwrites the packet handle with a new handle, which
+ * application must use as the reference to the packet instead of the old
+ * handle. Depending on the implementation, the old and new handles may be
+ * equal.
+ *
+ * The operation return value indicates if any packet data or metadata (e.g.
+ * user_area) were moved in memory during the operation. If some memory areas
+ * were moved, application must use new packe

[lng-odp] [API-NEXT PATCHv5 02/10] api: packet: re-organize file content

[Bill Fischofer]

From: Petri Savolainen 

Grouped packet allocation, segmentation and packet head/tail
manipulation functions together. Rest of the file are metadata
read/write functions which do not modify packet data storage.

Signed-off-by: Petri Savolainen 
Reviewed-by: Bill Fischofer 
Reviewed-by: Bala Manoharan 
Signed-off-by: Bill Fischofer 
---
 include/odp/api/spec/packet.h | 478 +-
 1 file changed, 234 insertions(+), 244 deletions(-)

diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h
index 6c88458..36eb89a 100644
--- a/include/odp/api/spec/packet.h
+++ b/include/odp/api/spec/packet.h
@@ -293,6 +293,27 @@ uint32_t odp_packet_tailroom(odp_packet_t pkt);
 void *odp_packet_tail(odp_packet_t pkt);
 
 /**
+ * Packet offset pointer
+ *
+ * Returns pointer to data in the packet offset. The packet level byte offset 
is
+ * calculated from the current odp_packet_data() position. Optionally outputs
+ * handle to the segment and number of data bytes in the segment following the
+ * pointer.
+ *
+ * @param  pkt  Packet handle
+ * @param  offset   Byte offset into the packet
+ * @param[out] len  Number of data bytes remaining in the segment (output).
+ *  Ignored when NULL.
+ * @param[out] seg  Handle to the segment containing the address (output).
+ *  Ignored when NULL.
+ *
+ * @return Pointer to the offset
+ * @retval NULL  Requested offset exceeds packet length
+ */
+void *odp_packet_offset(odp_packet_t pkt, uint32_t offset, uint32_t *len,
+   odp_packet_seg_t *seg);
+
+/**
  * Push out packet head
  *
  * Increase packet data length by moving packet head into packet headroom.
@@ -550,25 +571,223 @@ int odp_packet_trunc_tail(odp_packet_t *pkt, uint32_t 
len, void **tail_ptr,
  uint32_t *tailroom);
 
 /**
- * Packet offset pointer
+ * Add data into an offset
  *
- * Returns pointer to data in the packet offset. The packet level byte offset 
is
- * calculated from the current odp_packet_data() position. Optionally outputs
- * handle to the segment and number of data bytes in the segment following the
- * pointer.
+ * Increases packet data length by adding new data area into the specified
+ * offset. The operation returns a new packet handle on success. It may modify
+ * packet segmentation and move data. Handles and pointers must be updated
+ * after the operation. User is responsible to update packet metadata offsets
+ * when needed. The packet is not modified on an error.
  *
- * @param  pkt  Packet handle
- * @param  offset   Byte offset into the packet
- * @param[out] len  Number of data bytes remaining in the segment (output).
- *  Ignored when NULL.
- * @param[out] seg  Handle to the segment containing the address (output).
- *  Ignored when NULL.
+ * @param pkt Packet handle
+ * @param offset  Byte offset into the packet
+ * @param len Number of bytes to add into the offset
  *
- * @return Pointer to the offset
- * @retval NULL  Requested offset exceeds packet length
+ * @return New packet handle
+ * @retval ODP_PACKET_INVALID on failure
  */
-void *odp_packet_offset(odp_packet_t pkt, uint32_t offset, uint32_t *len,
-   odp_packet_seg_t *seg);
+odp_packet_t odp_packet_add_data(odp_packet_t pkt, uint32_t offset,
+uint32_t len);
+
+/**
+ * Remove data from an offset
+ *
+ * Decreases packet data length by removing data from the specified offset.
+ * The operation returns a new packet handle on success, and may modify
+ * packet segmentation and move data. Handles and pointers must be updated
+ * after the operation. User is responsible to update packet metadata offsets
+ * when needed. The packet is not modified on an error.
+ *
+ * @param pkt Packet handle
+ * @param offset  Byte offset into the packet
+ * @param len Number of bytes to remove from the offset
+ *
+ * @return New packet handle
+ * @retval ODP_PACKET_INVALID on failure
+ */
+odp_packet_t odp_packet_rem_data(odp_packet_t pkt, uint32_t offset,
+uint32_t len);
+
+/*
+ *
+ * Segmentation
+ * 
+ *
+ */
+
+/**
+ * Tests if packet is segmented
+ *
+ * @param pkt  Packet handle
+ *
+ * @retval 0 Packet is not segmented
+ * @retval 1 Packet is segmented
+ */
+int odp_packet_is_segmented(odp_packet_t pkt);
+
+/**
+ * Number of segments
+ *
+ * Returns number of segments in the packet. A packet has always at least one
+ * segment.
+ *
+ * @param pkt  Packet handle
+ *
+ * @return Number of segments (>0)
+ */
+int odp_packet_num_segs(odp_packet_t pkt);
+
+/**
+ * First segment in packet
+ *
+ * A packet has always the first segment (has at least one segment).
+ *
+ * @param pkt  Packet handle
+ *
+ * @return Handle to the first segment
+ */
+odp_packet_seg_t odp_packet_first_seg(odp_packet

[lng-odp] [API-NEXT PATCHv5 03/10] api: packet: remove seg_buf_addr and seg_buf_len

[Bill Fischofer]

From: Petri Savolainen 

Removed definitions to access segment memory outside of segment
data. A tighter segment specification makes segment manipulation
definitions cleaner. Only packet level head-/tailrooms and segment
data are moved with the packet, segment level head-/tailrooms are not.

Signed-off-by: Petri Savolainen 
Reviewed-by: Bill Fischofer 
Reviewed-by: Bala Manoharan 
Signed-off-by: Bill Fischofer 
---
 include/odp/api/spec/packet.h   | 29 -
 platform/linux-generic/odp_packet.c | 14 --
 test/validation/packet/packet.c | 30 +++---
 3 files changed, 7 insertions(+), 66 deletions(-)

diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h
index 36eb89a..6e2302c 100644
--- a/include/odp/api/spec/packet.h
+++ b/include/odp/api/spec/packet.h
@@ -675,35 +675,6 @@ odp_packet_seg_t odp_packet_last_seg(odp_packet_t pkt);
 odp_packet_seg_t odp_packet_next_seg(odp_packet_t pkt, odp_packet_seg_t seg);
 
 /**
- * Segment buffer address
- *
- * Returns start address of the segment.
- *
- * @param pkt  Packet handle
- * @param seg  Segment handle
- *
- * @return  Start address of the segment
- * @retval NULL on failure
- *
- * @see odp_packet_seg_buf_len()
- */
-void *odp_packet_seg_buf_addr(odp_packet_t pkt, odp_packet_seg_t seg);
-
-/**
- * Segment buffer length
- *
- * Returns segment buffer length in bytes.
- *
- * @param pkt  Packet handle
- * @param seg  Segment handle
- *
- * @return  Segment buffer length in bytes
- *
- * @see odp_packet_seg_buf_addr()
- */
-uint32_t odp_packet_seg_buf_len(odp_packet_t pkt, odp_packet_seg_t seg);
-
-/**
  * Segment data pointer
  *
  * Returns pointer to the first byte of data in the segment.
diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 2c44316..4202a90 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -491,20 +491,6 @@ odp_packet_seg_t odp_packet_next_seg(odp_packet_t pkt, 
odp_packet_seg_t seg)
  *
  */
 
-void *odp_packet_seg_buf_addr(odp_packet_t pkt, odp_packet_seg_t seg)
-{
-   odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
-
-   return segment_map(&pkt_hdr->buf_hdr, (odp_buffer_seg_t)seg, NULL,
-  pkt_hdr->headroom + pkt_hdr->frame_len, 0);
-}
-
-uint32_t odp_packet_seg_buf_len(odp_packet_t pkt,
-   odp_packet_seg_t seg ODP_UNUSED)
-{
-   return odp_packet_hdr(pkt)->buf_hdr.segsize;
-}
-
 void *odp_packet_seg_data(odp_packet_t pkt, odp_packet_seg_t seg)
 {
odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
diff --git a/test/validation/packet/packet.c b/test/validation/packet/packet.c
index a764ed9..d59e96c 100644
--- a/test/validation/packet/packet.c
+++ b/test/validation/packet/packet.c
@@ -483,7 +483,7 @@ void packet_test_tailroom(void)
 void packet_test_segments(void)
 {
int num_segs, seg_index;
-   uint32_t data_len, buf_len;
+   uint32_t data_len;
odp_packet_seg_t seg;
odp_packet_t pkt = test_packet;
odp_packet_t seg_pkt = segmented_test_packet;
@@ -504,28 +504,20 @@ void packet_test_segments(void)
CU_ASSERT(odp_packet_is_segmented(seg_pkt) == 1);
 
seg = odp_packet_first_seg(pkt);
-   buf_len = 0;
data_len = 0;
seg_index = 0;
while (seg_index < num_segs && seg != ODP_PACKET_SEG_INVALID) {
-   uint32_t seg_data_len, seg_buf_len;
-   void *seg_buf_addr, *seg_data;
+   uint32_t seg_data_len;
+   void *seg_data;
 
-   seg_buf_addr = odp_packet_seg_buf_addr(pkt, seg);
-   seg_buf_len  = odp_packet_seg_buf_len(pkt, seg);
seg_data_len = odp_packet_seg_data_len(pkt, seg);
seg_data = odp_packet_seg_data(pkt, seg);
 
-   CU_ASSERT(seg_buf_len > 0);
CU_ASSERT(seg_data_len > 0);
-   CU_ASSERT(seg_buf_len >= seg_data_len);
CU_ASSERT_PTR_NOT_NULL(seg_data);
-   CU_ASSERT_PTR_NOT_NULL(seg_buf_addr);
-   CU_ASSERT(seg_data >= seg_buf_addr);
CU_ASSERT(odp_packet_seg_to_u64(seg) !=
  odp_packet_seg_to_u64(ODP_PACKET_SEG_INVALID));
 
-   buf_len += seg_buf_len;
data_len += seg_data_len;
 
/** @todo: touch memory in a segment */
@@ -534,7 +526,7 @@ void packet_test_segments(void)
}
 
CU_ASSERT(seg_index == num_segs);
-   CU_ASSERT(buf_len == odp_packet_buf_len(pkt));
+   CU_ASSERT(data_len <= odp_packet_buf_len(pkt));
CU_ASSERT(data_len == odp_packet_len(pkt));
 
if (seg_index == num_segs)
@@ -543,29 +535,21 @@ void packet_test_segments(void)
seg = odp_packet_first_seg(seg_pkt);
num_segs = odp_packet_num_segs(seg_pkt);
 
-   buf_len = 0;
data_len = 0;
seg_index = 0;
 
while (seg_index

[lng-odp] [API-NEXT PATCHv5 05/10] linux-generic: packet: correct odp_packet_buf_len

[Bill Fischofer]

odp_packet_buf_len() must return buffer length over all segments

Signed-off-by: Bill Fischofer 
Reviewed-by: Bala Manoharan 
---
 platform/linux-generic/odp_packet.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 4202a90..2abb227 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -211,7 +211,9 @@ void *odp_packet_head(odp_packet_t pkt)
 
 uint32_t odp_packet_buf_len(odp_packet_t pkt)
 {
-   return odp_packet_hdr(pkt)->buf_hdr.size;
+   odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
+
+   return pkt_hdr->buf_hdr.size * pkt_hdr->buf_hdr.segcount;
 }
 
 void *odp_packet_data(odp_packet_t pkt)
-- 
2.5.0

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

[lng-odp] [API-NEXT PATCHv5 04/10] api: packet: add packet data prefetch

[Bill Fischofer]

From: Petri Savolainen 

For improved cache hit rate, prefetch may be used to prefetch packet
data before accessing it the first time. Largest benefit may be expected
for the data that is deep in the packet. Typically, packet head and
metadata are automatically prefetched anyway.

Signed-off-by: Petri Savolainen 
Reviewed-by: Bill Fischofer 
Reviewed-by: Bala Manoharan 
Signed-off-by: Bill Fischofer 
---
 include/odp/api/spec/packet.h | 12 
 1 file changed, 12 insertions(+)

diff --git a/include/odp/api/spec/packet.h b/include/odp/api/spec/packet.h
index 6e2302c..0dddf17 100644
--- a/include/odp/api/spec/packet.h
+++ b/include/odp/api/spec/packet.h
@@ -314,6 +314,18 @@ void *odp_packet_offset(odp_packet_t pkt, uint32_t offset, 
uint32_t *len,
odp_packet_seg_t *seg);
 
 /**
+ * Packet data prefetch
+ *
+ * Prefetch 'len' bytes of packet data starting from 'offset' into various
+ * caches close to the calling thread.
+ *
+ * @param  pkt  Packet handle
+ * @param  offset   Byte offset into packet data
+ * @param  len  Number of bytes to prefetch starting from 'offset'
+ */
+void odp_packet_prefetch(odp_packet_t pkt, uint32_t offset, uint32_t len);
+
+/**
  * Push out packet head
  *
  * Increase packet data length by moving packet head into packet headroom.
-- 
2.5.0

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

[lng-odp] [API-NEXT PATCHv5 08/10] validation: packet: add tests for extend/trunc packet head/tail

[Bill Fischofer]

Signed-off-by: Bill Fischofer 
Reviewed-by: Bala Manoharan 
---
 test/validation/packet/packet.c | 188 +---
 1 file changed, 137 insertions(+), 51 deletions(-)

diff --git a/test/validation/packet/packet.c b/test/validation/packet/packet.c
index dc40cfb..da7ed5c 100644
--- a/test/validation/packet/packet.c
+++ b/test/validation/packet/packet.c
@@ -371,37 +371,68 @@ void packet_test_layer_offsets(void)
CU_ASSERT(l3_addr != l4_addr);
 }
 
-static void _verify_headroom_shift(odp_packet_t packet,
+static void _verify_headroom_shift(odp_packet_t *pkt,
   int shift)
 {
-   uint32_t room = odp_packet_headroom(packet);
-   uint32_t seg_data_len = odp_packet_seg_len(packet);
-   uint32_t pkt_data_len = odp_packet_len(packet);
+   uint32_t room = odp_packet_headroom(*pkt);
+   uint32_t seg_data_len = odp_packet_seg_len(*pkt);
+   uint32_t pkt_data_len = odp_packet_len(*pkt);
void *data;
-   char *data_orig = odp_packet_data(packet);
-   char *head_orig = odp_packet_head(packet);
+   char *data_orig = odp_packet_data(*pkt);
+   char *head_orig = odp_packet_head(*pkt);
+   uint32_t seg_len;
+   int extended, rc;;
 
-   if (shift >= 0)
-   data = odp_packet_push_head(packet, shift);
-   else
-   data = odp_packet_pull_head(packet, -shift);
+   if (shift >= 0) {
+   if (abs(shift) <= room) {
+   data = odp_packet_push_head(*pkt, shift);
+   extended = 0;
+   } else {
+   rc = odp_packet_extend_head(pkt, shift,
+   &data, &seg_len);
+   extended = 1;
+   }
+   } else {
+   if (abs(shift) <= seg_data_len) {
+   data = odp_packet_pull_head(*pkt, -shift);
+   extended = 0;
+   } else {
+   rc = odp_packet_trunc_head(pkt, -shift,
+  &data, &seg_len);
+   extended = 1;
+   }
+   }
 
CU_ASSERT_PTR_NOT_NULL(data);
-   CU_ASSERT(odp_packet_headroom(packet) == room - shift);
-   CU_ASSERT(odp_packet_seg_len(packet) == seg_data_len + shift);
-   CU_ASSERT(odp_packet_len(packet) == pkt_data_len + shift);
-   CU_ASSERT(odp_packet_data(packet) == data);
-   CU_ASSERT(odp_packet_head(packet) == head_orig);
-   CU_ASSERT(data == data_orig - shift);
+   if (extended) {
+   CU_ASSERT(rc >= 0);
+   if (shift >= 0) {
+   CU_ASSERT(odp_packet_seg_len(*pkt) == shift - room);
+   } else {
+   CU_ASSERT(odp_packet_headroom(*pkt) >=
+ abs(shift) - seg_data_len);
+   }
+   CU_ASSERT(odp_packet_head(*pkt) != head_orig);
+   } else {
+   CU_ASSERT(odp_packet_headroom(*pkt) == room - shift);
+   CU_ASSERT(odp_packet_seg_len(*pkt) == seg_data_len + shift);
+   CU_ASSERT(data == data_orig - shift);
+   CU_ASSERT(odp_packet_head(*pkt) == head_orig);
+   }
+
+   CU_ASSERT(odp_packet_len(*pkt) == pkt_data_len + shift);
+   CU_ASSERT(odp_packet_data(*pkt) == data);
 }
 
 void packet_test_headroom(void)
 {
-   odp_packet_t pkt = test_packet;
+   odp_packet_t pkt = odp_packet_copy(test_packet,
+  odp_packet_pool(test_packet));
uint32_t room;
uint32_t seg_data_len;
uint32_t push_val, pull_val;
 
+   CU_ASSERT_FATAL(pkt != ODP_PACKET_INVALID);
room = odp_packet_headroom(pkt);
 
 #if ODP_CONFIG_PACKET_HEADROOM != 0 /* Avoid 'always true' warning */
@@ -413,68 +444,115 @@ void packet_test_headroom(void)
pull_val = seg_data_len / 2;
push_val = room;
 
-   _verify_headroom_shift(pkt, -pull_val);
-   _verify_headroom_shift(pkt, push_val + pull_val);
-   _verify_headroom_shift(pkt, -push_val);
-   _verify_headroom_shift(pkt, 0);
+   _verify_headroom_shift(&pkt, -pull_val);
+   _verify_headroom_shift(&pkt, push_val + pull_val);
+   _verify_headroom_shift(&pkt, -push_val);
+   _verify_headroom_shift(&pkt, 0);
+
+   if (segmentation_supported) {
+   push_val = room * 2;
+   _verify_headroom_shift(&pkt, push_val);
+   _verify_headroom_shift(&pkt, 0);
+   _verify_headroom_shift(&pkt, -push_val);
+   }
+
+   odp_packet_free(pkt);
 }
 
-static void _verify_tailroom_shift(odp_packet_t pkt,
+static void _verify_tailroom_shift(odp_packet_t *pkt,
   int shift)
 {
odp_packet_seg_t seg;
uint32_t room;
-   uint32_t seg_data_len, pkt_data_len;
+   uint32_t seg_data_len, pkt_data_len, seg_len;
void *tail;
 

[lng-odp] [API-NEXT PATCHv5 06/10] validation: packet: add test for correct odp_packet_buffer_len()

[Bill Fischofer]

Extend the packet_test_length() test to properly test odp_packet_buf_len()

Signed-off-by: Bill Fischofer 
Reviewed-by: Bala Manoharan 
---
 test/validation/packet/packet.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/test/validation/packet/packet.c b/test/validation/packet/packet.c
index d59e96c..dc40cfb 100644
--- a/test/validation/packet/packet.c
+++ b/test/validation/packet/packet.c
@@ -277,6 +277,20 @@ void packet_test_length(void)
CU_ASSERT(tailroom >= ODP_CONFIG_PACKET_TAILROOM);
 #endif
CU_ASSERT(buf_len >= packet_len + headroom + tailroom);
+
+   pkt = segmented_test_packet;
+   buf_len = odp_packet_buf_len(pkt);
+   headroom = odp_packet_headroom(pkt);
+   tailroom = odp_packet_tailroom(pkt);
+
+   CU_ASSERT(odp_packet_len(pkt) == segmented_packet_len);
+#if ODP_CONFIG_PACKET_HEADROOM != 0 /* Avoid 'always true' warning */
+   CU_ASSERT(headroom >= ODP_CONFIG_PACKET_HEADROOM);
+#endif
+#if ODP_CONFIG_PACKET_TAILROOM != 0 /* Avoid 'always true' warning */
+   CU_ASSERT(tailroom >= ODP_CONFIG_PACKET_TAILROOM);
+#endif
+   CU_ASSERT(buf_len >= packet_len + headroom + tailroom);
 }
 
 void packet_test_debug(void)
-- 
2.5.0

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

[lng-odp] [API-NEXT PATCHv5 07/10] linux-generic: packet: implement packet extend/trunc apis

[Bill Fischofer]

Implement the following APIs:
odp_packet_extend_head()
odp_packet_trunc_head()
odp_packet_extend_tail()
odp_packet_trunc_tail()

Signed-off-by: Bill Fischofer 
Reviewed-by: Bala Manoharan 
---
 .../linux-generic/include/odp_buffer_internal.h|  4 ++
 .../linux-generic/include/odp_packet_internal.h| 43 +
 platform/linux-generic/odp_packet.c| 71 +++-
 platform/linux-generic/odp_pool.c  | 75 ++
 4 files changed, 192 insertions(+), 1 deletion(-)

diff --git a/platform/linux-generic/include/odp_buffer_internal.h 
b/platform/linux-generic/include/odp_buffer_internal.h
index ea092ca..0a4c290 100644
--- a/platform/linux-generic/include/odp_buffer_internal.h
+++ b/platform/linux-generic/include/odp_buffer_internal.h
@@ -170,6 +170,10 @@ typedef struct {
 odp_buffer_t buffer_alloc(odp_pool_t pool, size_t size);
 int buffer_alloc_multi(odp_pool_t pool_hdl, size_t size,
   odp_buffer_t buf[], int num);
+int seg_alloc_head(odp_buffer_hdr_t *buf_hdr, int segcount);
+void seg_free_head(odp_buffer_hdr_t *buf_hdr, int segcount);
+int seg_alloc_tail(odp_buffer_hdr_t *buf_hdr, int segcount);
+void seg_free_tail(odp_buffer_hdr_t *buf_hdr, int segcount);
 
 #ifdef __cplusplus
 }
diff --git a/platform/linux-generic/include/odp_packet_internal.h 
b/platform/linux-generic/include/odp_packet_internal.h
index 92b770f..20cab8a 100644
--- a/platform/linux-generic/include/odp_packet_internal.h
+++ b/platform/linux-generic/include/odp_packet_internal.h
@@ -210,12 +210,55 @@ static inline void pull_head(odp_packet_hdr_t *pkt_hdr, 
size_t len)
pkt_hdr->frame_len -= len;
 }
 
+static inline int push_head_seg(odp_packet_hdr_t *pkt_hdr, size_t len)
+{
+   uint32_t extrasegs =
+   (len - pkt_hdr->headroom + pkt_hdr->buf_hdr.segsize - 1) /
+   pkt_hdr->buf_hdr.segsize;
+
+   if (pkt_hdr->buf_hdr.segcount + extrasegs > ODP_BUFFER_MAX_SEG ||
+   seg_alloc_head(&pkt_hdr->buf_hdr, extrasegs))
+   return -1;
+
+   pkt_hdr->headroom += extrasegs * pkt_hdr->buf_hdr.segsize;
+   return 0;
+}
+
+static inline void pull_head_seg(odp_packet_hdr_t *pkt_hdr)
+{
+   uint32_t extrasegs = (pkt_hdr->headroom - 1) / pkt_hdr->buf_hdr.segsize;
+
+   seg_free_head(&pkt_hdr->buf_hdr, extrasegs);
+   pkt_hdr->headroom -= extrasegs * pkt_hdr->buf_hdr.segsize;
+}
+
 static inline void push_tail(odp_packet_hdr_t *pkt_hdr, size_t len)
 {
pkt_hdr->tailroom  -= len;
pkt_hdr->frame_len += len;
 }
 
+static inline int push_tail_seg(odp_packet_hdr_t *pkt_hdr, size_t len)
+{
+   uint32_t extrasegs =
+   (len - pkt_hdr->tailroom + pkt_hdr->buf_hdr.segsize - 1) /
+   pkt_hdr->buf_hdr.segsize;
+
+   if (pkt_hdr->buf_hdr.segcount + extrasegs > ODP_BUFFER_MAX_SEG ||
+   seg_alloc_tail(&pkt_hdr->buf_hdr, extrasegs))
+   return -1;
+
+   pkt_hdr->tailroom += extrasegs * pkt_hdr->buf_hdr.segsize;
+   return 0;
+}
+
+static inline void pull_tail_seg(odp_packet_hdr_t *pkt_hdr)
+{
+   uint32_t extrasegs = (pkt_hdr->tailroom - 1) / pkt_hdr->buf_hdr.segsize;
+
+   seg_free_tail(&pkt_hdr->buf_hdr, extrasegs);
+   pkt_hdr->tailroom -= extrasegs * pkt_hdr->buf_hdr.segsize;
+}
 
 static inline void pull_tail(odp_packet_hdr_t *pkt_hdr, size_t len)
 {
diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 2abb227..77f995e 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -115,9 +115,11 @@ odp_packet_t packet_alloc(odp_pool_t pool_hdl, uint32_t 
len, int parse)
}
 
pkt_hdr = odp_packet_hdr(pkt);
-
packet_init(pool, pkt_hdr, len, parse);
 
+   if (pkt_hdr->tailroom >= pkt_hdr->buf_hdr.segsize)
+   pull_tail_seg(pkt_hdr);
+
return pkt;
 }
 
@@ -264,6 +266,21 @@ void *odp_packet_push_head(odp_packet_t pkt, uint32_t len)
return packet_map(pkt_hdr, 0, NULL);
 }
 
+int odp_packet_extend_head(odp_packet_t *pkt, uint32_t len,
+  void **data_ptr, uint32_t *seg_len)
+{
+   odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(*pkt);
+
+   if (len > pkt_hdr->headroom && push_head_seg(pkt_hdr, len))
+   return -1;
+
+   push_head(pkt_hdr, len);
+
+   if (data_ptr)
+   *data_ptr = packet_map(pkt_hdr, 0, seg_len);
+   return 0;
+}
+
 void *odp_packet_pull_head(odp_packet_t pkt, uint32_t len)
 {
odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
@@ -275,6 +292,23 @@ void *odp_packet_pull_head(odp_packet_t pkt, uint32_t len)
return packet_map(pkt_hdr, 0, NULL);
 }
 
+int odp_packet_trunc_head(odp_packet_t *pkt, uint32_t len,
+ void **data_ptr, uint32_t *seg_len)
+{
+   odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(*pkt);
+
+   if (len > pkt_hdr->frame_len)
+   return -1;
+
+   pull_head(pkt

[lng-odp] [API-NEXT PATCHv5 10/10] validation: packet: add validation test for odp_packet_prefetch()

[Bill Fischofer]

Signed-off-by: Bill Fischofer 
Reviewed-by: Bala Manoharan 
---
 test/validation/packet/packet.c | 7 +++
 test/validation/packet/packet.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/test/validation/packet/packet.c b/test/validation/packet/packet.c
index da7ed5c..82e34a6 100644
--- a/test/validation/packet/packet.c
+++ b/test/validation/packet/packet.c
@@ -293,6 +293,12 @@ void packet_test_length(void)
CU_ASSERT(buf_len >= packet_len + headroom + tailroom);
 }
 
+void packet_test_prefetch(void)
+{
+   odp_packet_prefetch(test_packet, 0, odp_packet_len(test_packet));
+   CU_PASS();
+}
+
 void packet_test_debug(void)
 {
CU_ASSERT(odp_packet_is_valid(test_packet) == 1);
@@ -957,6 +963,7 @@ odp_testinfo_t packet_suite[] = {
ODP_TEST_INFO(packet_test_debug),
ODP_TEST_INFO(packet_test_segments),
ODP_TEST_INFO(packet_test_length),
+   ODP_TEST_INFO(packet_test_prefetch),
ODP_TEST_INFO(packet_test_headroom),
ODP_TEST_INFO(packet_test_tailroom),
ODP_TEST_INFO(packet_test_context),
diff --git a/test/validation/packet/packet.h b/test/validation/packet/packet.h
index 0d78feb..23ce424 100644
--- a/test/validation/packet/packet.h
+++ b/test/validation/packet/packet.h
@@ -16,6 +16,7 @@ void packet_test_alloc_segmented(void);
 void packet_test_event_conversion(void);
 void packet_test_basic_metadata(void);
 void packet_test_length(void);
+void packet_test_prefetch(void);
 void packet_test_debug(void);
 void packet_test_context(void);
 void packet_test_layer_offsets(void);
-- 
2.5.0

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

[lng-odp] [API-NEXT PATCHv5 09/10] linux-generic: packet: implement odp_packet_prefetch()

[Bill Fischofer]

Note that this function is a no-op on linux-generic

Signed-off-by: Bill Fischofer 
Reviewed-by: Bala Manoharan 
---
 platform/linux-generic/odp_packet.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/platform/linux-generic/odp_packet.c 
b/platform/linux-generic/odp_packet.c
index 77f995e..2df6ed4 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -384,6 +384,13 @@ void *odp_packet_offset(odp_packet_t pkt, uint32_t offset, 
uint32_t *len,
return addr;
 }
 
+/* This function is a no-op in linux-generic */
+void odp_packet_prefetch(odp_packet_t pkt ODP_UNUSED,
+uint32_t offset ODP_UNUSED,
+uint32_t len ODP_UNUSED)
+{
+}
+
 /*
  *
  * Meta-data
-- 
2.5.0

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

Re: [lng-odp] [PATCH] linux-generic: rework odp_queue_create

[Maxim Uvarov]

On 04/14/16 23:29, Bill Fischofer wrote:



On Thu, Apr 14, 2016 at 2:50 PM, Maxim Uvarov > wrote:




On 14 April 2016 at 22:16, Bill Fischofer
mailto:bill.fischo...@linaro.org>> wrote:



On Thu, Apr 14, 2016 at 11:35 AM, Maxim Uvarov
mailto:maxim.uva...@linaro.org>> wrote:

- use odp_config_queues() to get number of queues and int
cycle
  iterrator. That makes it more easy to reuse that
function for
  other platfroms.;
- remove declaring variables to invalid at the bottom and
check for
  them down the code;
- make code more easy to read;

Signed-off-by: Maxim Uvarov mailto:maxim.uva...@linaro.org>>
---
 platform/linux-generic/odp_queue.c | 36
+++-
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/platform/linux-generic/odp_queue.c
b/platform/linux-generic/odp_queue.c
index 342ffa2..0f7fd15 100644
--- a/platform/linux-generic/odp_queue.c
+++ b/platform/linux-generic/odp_queue.c
@@ -234,10 +234,8 @@ int odp_queue_lock_count(odp_queue_t
handle)

 odp_queue_t odp_queue_create(const char *name, const
odp_queue_param_t *param)
 {
-   uint32_t i;
+   int i;
queue_entry_t *queue;
-   odp_queue_t handle = ODP_QUEUE_INVALID;
-   odp_queue_type_t type;
odp_queue_param_t default_param;

if (param == NULL) {
@@ -245,7 +243,7 @@ odp_queue_t odp_queue_create(const
char *name, const odp_queue_param_t *param)
param = &default_param;
}

-   for (i = 0; i < ODP_CONFIG_QUEUES; i++) {
+   for (i = 0; i < odp_config_queues(); i++) {


This is needlessly inefficient. The #defines are perfectly OK
for implementation internal use.  This makes an extra function
call for each iteration.

queue = &queue_tbl->queue[i];

if (queue->s.status != QUEUE_STATUS_FREE)
@@ -253,33 +251,29 @@ odp_queue_t odp_queue_create(const
char *name, const odp_queue_param_t *param)

LOCK(&queue->s.lock);
if (queue->s.status == QUEUE_STATUS_FREE) {
-   if (queue_init(queue, name, param)) {
+   if (queue_init(queue, name, param) ||
+  queue->s.handle == ODP_QUEUE_INVALID) {


This is a redundant check as this field is initialized part
part of queue_init_global() and does not change

UNLOCK(&queue->s.lock);
-  return handle;
+  return ODP_QUEUE_INVALID;
}

-   type = queue->s.type;
-
-   if (type == ODP_QUEUE_TYPE_SCHED)
+   if (queue->s.type ==
ODP_QUEUE_TYPE_SCHED) {
queue->s.status = QUEUE_STATUS_NOTSCHED;
-   else
+   if
(schedule_queue_init(queue)) {


In the original code this routine is called with the queue
unlocked. While holding the lock across this call should still
work, in general we want to release locks as soon as possible
so I don't see this as an improvement.


ah, that is main comment.  If we want schedule_queue_init() run
without lock than it's better to use current code, I think, so I
will apply your patch.


One small note about current code which I just realized (might be
it's too late for today).

schedule_queue_init() may fail. Should we in that case set
queue->s.status back to QUEUE_STATUS_FREE  before


That's a good catch.  However that should be done by 
odp_queue_destroy() since at the point of the failure the queue lock 
is not held.  It's a one line fix.  If you want to add it to this 
patch I'm fine with that or I can submit a follow-up bug and patch.


I think separate patch for that to not depend on original patch.

Maxim.


return ODP_QUEUE_INVALID ?

Maxim.


+  ODP_ERR("schedule queue init failed\n");
+  UNLOCK(&queue->s.lock);
+  return ODP_QUEUE_INVALID;
+   }
+   } else {
queue->s.status = QUEUE_STATUS_READY;
-
-   handle = queue->s.handle;
+   }
UNLOCK(&queue->s.lock);
-   break;

Re: [lng-odp] [PATCH 1/2] linux-generic: classification: fix error checking _odp_packet_classifier()

[Bala Manoharan]

On 13 April 2016 at 19:51, Zoltan Kiss  wrote:

> Hi,
>
> I've sent a new patch, I just wanted to mention that I couldn't find this
> behavior described in user-guide-cls.adoc
>

I will add these information to user-guide document.

Regards,
Bala

>
> Zoli
>
> On 13/04/16 13:40, Bala Manoharan wrote:
>
>>
>>
>> The classification module drops a
>> packet only on specific cases either on error CoS or when the
>> packet
>> pool is full.
>>
>>
>> Could you point me to the documentation where this behavior is
>> described?
>>
>>
>> The scenario is described in classification users-guide documentation.
>> Pls let me know if we need to add additional information in the document.
>>
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [PATCH v2] linux-generic: classification: fix error checking _odp_packet_classifier()

[Bala Manoharan]

Reviewed-by: Balasubramanian Manoharan 

On 13 April 2016 at 19:50, Zoltan Kiss  wrote:

> In case of error the 'pkt' should be released by the calling function.
> Currently loopback gives it back to the receiver and report it as success
> in the stats.
>
> Signed-off-by: Zoltan Kiss 
> ---
>
> v2: handle release in caller instead, and adjust stats.
>
>  platform/linux-generic/odp_classification.c | 10 +++---
>  platform/linux-generic/pktio/loop.c |  9 ++---
>  2 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/platform/linux-generic/odp_classification.c
> b/platform/linux-generic/odp_classification.c
> index 8522023..3a18a78 100644
> --- a/platform/linux-generic/odp_classification.c
> +++ b/platform/linux-generic/odp_classification.c
> @@ -745,21 +745,17 @@ int _odp_packet_classifier(pktio_entry_t *entry,
> odp_packet_t pkt)
> if (cos == NULL)
> return -1;
>
> -   if (cos->s.pool == NULL) {
> -   odp_packet_free(pkt);
> +   if (cos->s.pool == NULL)
> return -1;
> -   }
>
> -   if (cos->s.queue == NULL) {
> -   odp_packet_free(pkt);
> +   if (cos->s.queue == NULL)
> return -1;
> -   }
>
> if (odp_packet_pool(pkt) != cos->s.pool->s.pool_hdl) {
> new_pkt = odp_packet_copy(pkt, cos->s.pool->s.pool_hdl);
> -   odp_packet_free(pkt);
> if (new_pkt == ODP_PACKET_INVALID)
> return -1;
> +   odp_packet_free(pkt);
> } else {
> new_pkt = pkt;
> }
> diff --git a/platform/linux-generic/pktio/loop.c
> b/platform/linux-generic/pktio/loop.c
> index 0ea6d0e..f6a8c1d 100644
> --- a/platform/linux-generic/pktio/loop.c
> +++ b/platform/linux-generic/pktio/loop.c
> @@ -70,10 +70,13 @@ static int loopback_recv(pktio_entry_t *pktio_entry,
> odp_packet_t pkts[],
> pkt_hdr = odp_packet_hdr(pkt);
> packet_parse_reset(pkt_hdr);
> packet_parse_l2(pkt_hdr);
> -   if (0 > _odp_packet_classifier(pktio_entry, pkt)) {
> -   pkts[j++] = pkt;
> +   if (!_odp_packet_classifier(pktio_entry, pkt)) {
> pktio_entry->s.stats.in_octets +=
> -   odp_packet_len(pkts[i]);
> +   odp_packet_len(pkt);
> +   } else {
> +   pktio_entry->s.stats.in_errors +=
> +   odp_packet_len(pkt);
> +   odp_packet_free(pkt);
> }
> }
> nbr = j;
> --
> 1.9.1
>
>
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Re: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user context support for interfaces

[Bogdan Pricope]

Hi Bill,

Yes, it should work (with some effort) fine with OFP.

Reviewed-by: Bogdan Pricope 

Thanks,
Bogdan

From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
Sent: Friday, April 15, 2016 12:16 AM
To: Bogdan Pricope 
Cc: Ola Liljedahl ; Savolainen, Petri (Nokia - 
FI/Espoo) ; lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user context 
support for interfaces

Hi Bogdan,

Patches to add the handle->index conversion functions have been posted.  The 
latest series begins at http://patches.opendataplane.org/patch/5575/

Please review this to see if it meets your needs.

Thanks.

Bill

On Thu, Apr 14, 2016 at 2:10 AM, Bogdan Pricope 
mailto:bogdan.pric...@enea.com>> wrote:
Hi Bill,

Thank you for trying to push this further.

A problem is that you are pushing details of your internal data organization 
into application. This may get messy to maintain especially if you decide to 
reorganize the table(s) at some point or increase number of pktios.
Btw, why don’t you transform odp_pktio_t into this pktio index?

Else, we can work with such index, but I don’t like the solution (no 
Reviewed-by from me).

The two SW layers is a false problem: once you identified the right interface, 
it is up to application to find the right data structure inside each layer

Bogdan


From: Bill Fischofer 
[mailto:bill.fischo...@linaro.org]
Sent: Wednesday, April 13, 2016 10:47 PM
To: Ola Liljedahl mailto:ola.liljed...@linaro.org>>
Cc: Savolainen, Petri (Nokia - FI/Espoo) 
mailto:petri.savolai...@nokia.com>>; Bogdan Pricope 
mailto:bogdan.pric...@enea.com>>; 
lng-odp@lists.linaro.org

Subject: Re: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user context 
support for interfaces


On Wed, Apr 13, 2016 at 2:40 PM, Ola Liljedahl 
mailto:ola.liljed...@linaro.org>> wrote:


On 13 April 2016 at 21:26, Bill Fischofer 
mailto:bill.fischo...@linaro.org>> wrote:
We already have odp_packet_input() so are you saying asking the application to 
write odp_pktio_id(odp_packet_input(pkt)) is too cumbersome?
I would like to be able to avoid any unnecessary memory accesses. Having to 
first retrieve the packet IO handle and then dereference that data structure to 
retrieve the pktio ID (interface index) seems to require an unnecessary load to 
a data structure not necessarily in the cache (or is it likely the ODP pktio 
data structure will otherwise be referenced for every packet and thus resident 
i the L1 cache?).

I think it is likely the packet metadata will actually contain the pktio 
ID/interface index, not the corresponding pktio handle. So return the index 
directly for use by the application as index into different application 
specific tables.

Actually, the intended performance model for ODP is that packets are classified 
into flows and the flow context associated with the queue delivering the packet 
contains all the info the application needs.  Why try to tie information to 
individual packets other than the (mutable) metadata that the application is 
doing to be working with?



Id can be renamed index if that's the preference.

On Wed, Apr 13, 2016 at 2:22 PM, Ola Liljedahl 
mailto:ola.liljed...@linaro.org>> wrote:


On 13 April 2016 at 19:38, Bill Fischofer 
mailto:bill.fischo...@linaro.org>> wrote:


On Wed, Apr 13, 2016 at 12:36 PM, Bill Fischofer 
mailto:bill.fischo...@linaro.org>> wrote:

On Wed, Apr 13, 2016 at 8:09 AM, Ola Liljedahl 
mailto:ola.liljed...@linaro.org>> wrote:
On 8 April 2016 at 17:25, Savolainen, Petri (Nokia - FI/Espoo) 
mailto:petri.savolai...@nokia.com>> wrote:


From: EXT Bogdan Pricope 
[mailto:bogdan.pric...@enea.com]
Sent: Friday, April 08, 2016 4:50 PM
To: Bill Fischofer 
mailto:bill.fischo...@linaro.org>>; Savolainen, 
Petri (Nokia - FI/Espoo) 
mailto:petri.savolai...@nokia.com>>
Cc: lng-odp@lists.linaro.org
Subject: RE: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user context 
support for interfaces

Hi,

Our path is:
pktio = odp_packet_input(pkt);
A step is invisible here:
queue = odp_pktio_outq_getdef(pktio)
ifnet = ofp_queue_context(queue)

ifnet = ofp_get_ifnet_pktio(pktio);

(Our current workaround is to iterate through the list of interfaces to find 
which interface has this pktio.)

You are saying:
pktio_id = odp_packet_input_id(pkt);
ifnet = ofp_get_ifnet_pktio_id(pktio_id);

Code generated with ID:
pktio_id = pkt_hdr->pktio_id;
ifnet = &ofp_ifnet_table[pktio_id]:
I think a pktio integer identifier (defined as a small integer suitable for 
using as an array index) is the most generic solution. Different SW layers can 
use this pktio ID as an index into *different* tables (different SW layers will 
have different needs). A (single) ODP pktio context pointer only makes one SW 
layer happy (or will promote a monolithic SW design which will be fragile and 
difficult to extend with new functionality).


That sounds reasona

Re: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user context support for interfaces

[Maxim Uvarov]

On 04/15/16 08:56, Bogdan Pricope wrote:


Hi Bill,

Yes, it should work (with some effort) fine with OFP.

Reviewed-by: Bogdan Pricope 



just to clarify, is that review for current patchset or for [lng-odp] 
[API-NEXT PATCHv2 1/6] api: pktio: add pktio index conversion APIs ?


Maxim.


Thanks,

Bogdan

*From:*Bill Fischofer [mailto:bill.fischo...@linaro.org]
*Sent:* Friday, April 15, 2016 12:16 AM
*To:* Bogdan Pricope 
*Cc:* Ola Liljedahl ; Savolainen, Petri 
(Nokia - FI/Espoo) ; lng-odp@lists.linaro.org
*Subject:* Re: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user 
context support for interfaces


Hi Bogdan,

Patches to add the handle->index conversion functions have been 
posted.  The latest series begins at 
http://patches.opendataplane.org/patch/5575/


Please review this to see if it meets your needs.

Thanks.

Bill

On Thu, Apr 14, 2016 at 2:10 AM, Bogdan Pricope 
mailto:bogdan.pric...@enea.com>> wrote:


Hi Bill,

Thank you for trying to push this further.

A problem is that you are pushing details of your internal data
organization into application. This may get messy to maintain
especially if you decide to reorganize the table(s) at some point
or increase number of pktios.

Btw, why don’t you transform odp_pktio_t into this pktio index?

Else, we can work with such index, but I don’t like the solution
(no Reviewed-by from me).

The two SW layers is a false problem: once you identified the
right interface, it is up to application to find the right data
structure inside each layer

Bogdan

*From:*Bill Fischofer [mailto:bill.fischo...@linaro.org
]
*Sent:* Wednesday, April 13, 2016 10:47 PM
*To:* Ola Liljedahl mailto:ola.liljed...@linaro.org>>
*Cc:* Savolainen, Petri (Nokia - FI/Espoo)
mailto:petri.savolai...@nokia.com>>;
Bogdan Pricope mailto:bogdan.pric...@enea.com>>; lng-odp@lists.linaro.org



*Subject:* Re: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user
context support for interfaces

On Wed, Apr 13, 2016 at 2:40 PM, Ola Liljedahl
mailto:ola.liljed...@linaro.org>> wrote:

On 13 April 2016 at 21:26, Bill Fischofer
mailto:bill.fischo...@linaro.org>>
wrote:

We already have odp_packet_input() so are you saying
asking the application to write
odp_pktio_id(odp_packet_input(pkt)) is too cumbersome?

I would like to be able to avoid any unnecessary memory
accesses. Having to first retrieve the packet IO handle and
then dereference that data structure to retrieve the pktio ID
(interface index) seems to require an unnecessary load to a
data structure not necessarily in the cache (or is it likely
the ODP pktio data structure will otherwise be referenced for
every packet and thus resident i the L1 cache?).

I think it is likely the packet metadata will actually contain
the pktio ID/interface index, not the corresponding pktio
handle. So return the index directly for use by the
application as index into different application specific tables.

Actually, the intended performance model for ODP is that packets
are classified into flows and the flow context associated with the
queue delivering the packet contains all the info the application
needs.  Why try to tie information to individual packets other
than the (mutable) metadata that the application is doing to be
working with?

Id can be renamed index if that's the preference.

On Wed, Apr 13, 2016 at 2:22 PM, Ola Liljedahl
mailto:ola.liljed...@linaro.org>> wrote:

On 13 April 2016 at 19:38, Bill Fischofer
mailto:bill.fischo...@linaro.org>> wrote:

On Wed, Apr 13, 2016 at 12:36 PM, Bill Fischofer
mailto:bill.fischo...@linaro.org>> wrote:

On Wed, Apr 13, 2016 at 8:09 AM, Ola Liljedahl
mailto:ola.liljed...@linaro.org>> wrote:

On 8 April 2016 at 17:25, Savolainen,
Petri (Nokia - FI/Espoo)
mailto:petri.savolai...@nokia.com>> wrote:

*From:*EXT Bogdan Pricope
[mailto:bogdan.pric...@enea.com
]
*Sent:* Friday, April 08, 2016 4:50 PM
*To:* Bill Fischofer
mailto:bill.fischo...@linaro.org>>;
Savolainen, Petri (Nokia - FI/Espoo)
mailto:petri.savolai...@nokia.com>>
*Cc:* lng-odp@lists.linaro.org