Re: [lng-odp] [API-NEXT PATCH v3 1/3] api: packet: allow access to packet RSS hash values

2015-08-20 Thread Jerin Jacob
On Thu, Aug 20, 2015 at 03:20:46PM +0100, Zoltan Kiss wrote:
 Hi,
 
 On 20/08/15 11:53, Jerin Jacob wrote:
 How about following change in OVS plarform specific packet_get_hash code.
 odp_packet_rss_hash_set
 kind of interface wont fit correctly in octeon  platform as hash
 identifier used by hardware in subsequent HW based queue operations
 
 static inline uint32_t
 packet_get_hash(struct dp_packet *p)
 {
 #ifdef DPDK_NETDEV
  return p-mbuf.hash.rss;
 #else
 #ifdef ODP_NETDEV
  unit32_t hash;
  hash = odp_packet_rss_hash(p-odp_pkt);
  if (OVS_UNLIKELY(!hash)
  hash = odp_packet_generate_rss_hash(p-odp_pkt);
  return hash;
 #endif
  return p-rss_hash;
 #endif
 }
 
 I was thinking about this too, but I see two problems:
 
 1. OVS changes the hash if the packet is currently recirculated:
 
 https://github.com/openvswitch/ovs/blob/master/lib/dpif-netdev.c#L3125
 
 And as far as I can see, it doesn't recalculate the hash when the
 recirculation is finished. So the final hash of the packet at the end won't
 be what the platform would generate. OVS doesn't seem to care about it, I
 think the assumption is that the platform won't use this hash anymore, and
 it's role is finished after matching in EMC.
 My first idea to sort this out is to check for recirculation depth in
 netdev_send(), before the send of course. If it's true, then we can
 regenerate the hash using odp_packet_generate_rss_hash()

if its not true then other option could be to hold additional data in
ODP meta data.

 
 2. Minor thing, but if the platform is only able to do this from software
 (like DPDK), it should better be as fast as OVS's hashing at least. OVS uses
 CRC32 on SSE4 x64 CPUs, and Murmurhash on others. I don't know how do they

DPDK do have low level HW accelerated API's for crc 4 and 8 bytes.
Implementing 5 tuple based on CRC on ODP-DPDK port will NOT be be any issue.
And armv8 also has advanced SIMD instruction for CRC 8,4,2 and 1 bytes

 compare e.g. with Toeplitz, but I guess they choose them because they are
 more suitable for the purpose.

I guess RSS hash algorithm used Intel nic is Toeplitz to get good
packet distribution using RSS(not for lookup)




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


Re: [lng-odp] [PATCHv2] linux-generic: pktio: always test loop interface

2015-08-20 Thread Mike Holmes
On 20 August 2015 at 07:41, Stuart Haslam stuart.has...@linaro.org wrote:
 Test using the loop interface even when running as root.

 Fixes bug; https://bugs.linaro.org/show_bug.cgi?id=1735

 Signed-off-by: Stuart Haslam stuart.has...@linaro.org

Reviewed-and-tested-by: Mike Holmes mike.hol...@linaro.org

 ---
 Change in v2 - updated comment about default interfaces

  platform/linux-generic/test/pktio/pktio_run | 25 -
  1 file changed, 16 insertions(+), 9 deletions(-)

 diff --git a/platform/linux-generic/test/pktio/pktio_run 
 b/platform/linux-generic/test/pktio/pktio_run
 index 76a8419..4860455 100755
 --- a/platform/linux-generic/test/pktio/pktio_run
 +++ b/platform/linux-generic/test/pktio/pktio_run
 @@ -46,9 +46,8 @@ run_test()
  {
 local ret=0

 -   # the linux-generic implementation uses environment variables to
 -   # control which socket method is used, so try each combination to
 -   # ensure decent coverage.
 +   # environment variables are used to control which socket method is
 +   # used, so try each combination to ensure decent coverage.
 for distype in MMAP MMSG; do
 unset ODP_PKTIO_DISABLE_SOCKET_${distype}
 done
 @@ -67,26 +66,34 @@ run_test()
 echo !!! FAILED !!!
 fi

 -   exit $ret
 +   return $ret
  }

  run()
  {
 -   #need to be root to set the interface: if not, run with default 
 loopback.
 +   echo pktio: using 'loop' device
 +   pktio_main${EXEEXT}
 +   loop_ret=$?
 +
 +   # need to be root to run tests with real interfaces
 if [ $(id -u) != 0 ]; then
 -   echo pktio: using 'loop' device
 -   pktio_main${EXEEXT}
 -   exit $?
 +   exit $ret
 fi

 if [ $ODP_PKTIO_IF0 =  ]; then
 -   # no interfaces specified on linux-generic, use defaults
 +   # no interfaces specified, use default veth interfaces
 +   # setup by the pktio_env script
 setup_pktio_env clean
 export ODP_PKTIO_IF0=$IF0
 export ODP_PKTIO_IF1=$IF1
 fi

 run_test
 +   ret=$?
 +
 +   [ $ret = 0 ]  ret=$loop_ret
 +
 +   exit $ret
  }

  case $1 in
 --
 2.1.1

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



-- 
Mike Holmes
Technical Manager - Linaro Networking Group
Linaro.org │ Open source software for ARM SoCs
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [API-NEXT PATCH v3 1/3] api: packet: allow access to packet RSS hash values

2015-08-20 Thread Zoltan Kiss



On 20/08/15 17:55, Jerin Jacob wrote:

On Thu, Aug 20, 2015 at 03:20:46PM +0100, Zoltan Kiss wrote:

Hi,

On 20/08/15 11:53, Jerin Jacob wrote:

How about following change in OVS plarform specific packet_get_hash code.
odp_packet_rss_hash_set
kind of interface wont fit correctly in octeon  platform as hash
identifier used by hardware in subsequent HW based queue operations

static inline uint32_t
packet_get_hash(struct dp_packet *p)
{
#ifdef DPDK_NETDEV
 return p-mbuf.hash.rss;
#else
#ifdef ODP_NETDEV
 unit32_t hash;
 hash = odp_packet_rss_hash(p-odp_pkt);
 if (OVS_UNLIKELY(!hash)
hash = odp_packet_generate_rss_hash(p-odp_pkt);
 return hash;
#endif
 return p-rss_hash;
#endif
}


I was thinking about this too, but I see two problems:

1. OVS changes the hash if the packet is currently recirculated:

https://github.com/openvswitch/ovs/blob/master/lib/dpif-netdev.c#L3125

And as far as I can see, it doesn't recalculate the hash when the
recirculation is finished. So the final hash of the packet at the end won't
be what the platform would generate. OVS doesn't seem to care about it, I
think the assumption is that the platform won't use this hash anymore, and
it's role is finished after matching in EMC.
My first idea to sort this out is to check for recirculation depth in
netdev_send(), before the send of course. If it's true, then we can
regenerate the hash using odp_packet_generate_rss_hash()


if its not true then other option could be to hold additional data in
ODP meta data.
I mean we only need to call odp_packet_generate_rss_hash if 
recirculation depth is non-zero. Otherwise OVS won't touch the hash if 
the platform makes sure there is a hash




2. Minor thing, but if the platform is only able to do this from software
(like DPDK), it should better be as fast as OVS's hashing at least. OVS uses
CRC32 on SSE4 x64 CPUs, and Murmurhash on others. I don't know how do they


DPDK do have low level HW accelerated API's for crc 4 and 8 bytes.
Implementing 5 tuple based on CRC on ODP-DPDK port will NOT be be any issue.
And armv8 also has advanced SIMD instruction for CRC 8,4,2 and 1 bytes


Yes, but DPDK would need to generate Toeplitz, not CRC, if the NIC is 
using that. And it might be that it's fast when the NIC does it, but not 
when the CPU.


I'm also considering to let the application know whether the platform 
requires this hash to set before handed back to the platform (e.g. going 
out). Platforms like DPDK doesn't care if the application modifies the 
hash, it won't use it again. So even if it can calculate it fast, it 
would be a waste of cycles. In that case the application wouldn't need 
to worry when changing the hash.





compare e.g. with Toeplitz, but I guess they choose them because they are
more suitable for the purpose.


I guess RSS hash algorithm used Intel nic is Toeplitz to get good
packet distribution using RSS(not for lookup)





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


Re: [lng-odp] [API-NEXT PATCH v3 1/3] api: packet: allow access to packet RSS hash values

2015-08-20 Thread Santosh Shukla
On 20 August 2015 at 14:48, Balasubramanian Manoharan
bala.manoha...@linaro.org wrote:


 On Sunday 16 August 2015 06:14 PM, Santosh Shukla wrote:

 On 15 August 2015 at 00:12, Zoltan Kiss zoltan.k...@linaro.org wrote:

 Applications can read the computed hash (if any) and set it if they
 changed the packet headers or if the platform haven't calculated the
 hash.

 Signed-off-by: Zoltan Kiss zoltan.k...@linaro.org
 ---
 v2:
 - focus on RSS hash only
 - use setter/getter's

 v3:
 - do not mention pointers
 - add a note
 - add new patches for implementation and test

   include/odp/api/packet.h | 30 ++
   1 file changed, 30 insertions(+)

 diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
 index 3a454b5..1ae24bc 100644
 --- a/include/odp/api/packet.h
 +++ b/include/odp/api/packet.h
 @@ -48,6 +48,11 @@ extern C {
* Invalid packet segment
*/

 +/**
 + * @def ODP_PACKET_RSS_INVALID
 + * RSS hash is not set
 + */
 +
   /*
*
* Alloc and free
 @@ -605,6 +610,31 @@ uint32_t odp_packet_l4_offset(odp_packet_t pkt);
   int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset);

   /**
 + * RSS hash value
 + *
 + * Returns the RSS hash stored for the packet.
 + *
 + * @param  pkt  Packet handle
 + *
 + * @return  Hash value
 + * @retval ODP_PACKET_RSS_INVALID if RSS hash is not set.
 + */
 +uint32_t odp_packet_rss_hash(odp_packet_t pkt);
 +
 +/**
 + * Set RSS hash value
 + *
 + * Store the RSS hash for the packet.
 + *
 + * @param  pkt  Packet handle
 + * @param  rss_hash Hash value to set
 + *
 + * @note If the application changes the packet header, it might want to
 + * recalculate this value and set it.
 + */
 +void odp_packet_rss_hash_set(odp_packet_t pkt, uint32_t rss_hash);

 Can this use-case be handled by calling
 odp_packet_generate_rss_hash(odp_packet_t pkt) function where the rss gets
 generated by the implementation rather than being set from application using
 odp_packet_set_rss_hash() function?


Bala, As we discussed and in summary for rest; Considering ovs example
: ovs uses sw based rss_hash only when HW not supporting rss Or HW
generated rss is zero.

hash = packet_get_hash(packet);
if (OVS_UNLIKELY(!hash)) {
hash = miniflow_hash_5tuple(mf, 0);
packet_set_hash(packet, hash);
}
return hash;

and rss_hash generation is inside ovs dp_packet (middle  layer), It is
with in ovs implementation, Not exposed to application layer, so
application won't need to generate rss / update, so no possibility of
rss mis-match /corruption.

 Regards,
 Bala

 +
 +/**
* Tests if packet is segmented
*
* @param pkt  Packet handle
 --

 Reviewed-by : Santosh Shukla santosh.shu...@linaro.org

 1.9.1

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

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


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


Re: [lng-odp] [API-NEXT PATCH v4 1/3] api: pool: add packet user area initializer for pool creation parameters

2015-08-20 Thread Bala Manoharan
Reviewed-by: Balasubramanian Manoharan bala.manoha...@linaro.org

On 15 August 2015 at 00:25, Zoltan Kiss zoltan.k...@linaro.org wrote:

 Applications can preset certain parts of the packet user area, so when that
 memory will be allocated it starts from a known state. If the platform
 allocates the memory during pool creation, it's enough to run the
 constructor after that. If it's allocating memory on demand, it should
 call the constructor each time.
 Porting applications to ODP can benefit from this. If the application can't
 afford to change its whole packet handling to ODP, it's likely it needs to
 maintain its own metadata in the user area. And probably it needs to set
 constant fields in that metadata e.g. to mark that this is an ODP packet,
 and/or store the handle of the packet itself.

 Signed-off-by: Zoltan Kiss zoltan.k...@linaro.org
 ---
 v2:
 - restrict this feature to packet user area
 - expand comments

 v3:
 - include packet.h in pool.h

 v4:
 - fix grammar based on Bill's comments

  include/odp/api/packet.h |  3 +++
  include/odp/api/pool.h   | 26 ++
  2 files changed, 29 insertions(+)

 diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
 index 3a454b5..f5d2142 100644
 --- a/include/odp/api/packet.h
 +++ b/include/odp/api/packet.h
 @@ -73,6 +73,9 @@ extern C {
   * @note The default headroom and tailroom used for packets is specified
 by
   * the ODP_CONFIG_PACKET_HEADROOM and ODP_CONFIG_PACKET_TAILROOM defines
 in
   * odp_config.h.
 + * @note Data changed in user area might be preserved by the platform from
 + * previous usage of the buffer, so values preset in uarea_init() are not
 + * guaranteed.
   */
  odp_packet_t odp_packet_alloc(odp_pool_t pool, uint32_t len);

 diff --git a/include/odp/api/pool.h b/include/odp/api/pool.h
 index 2e79a55..01f770f 100644
 --- a/include/odp/api/pool.h
 +++ b/include/odp/api/pool.h
 @@ -21,6 +21,7 @@ extern C {


  #include odp/std_types.h
 +#include odp/packet.h

  /** @defgroup odp_pool ODP POOL
   *  Operations on a pool.
 @@ -41,6 +42,23 @@ extern C {
  #define ODP_POOL_NAME_LEN  32

  /**
 + * Packet user area initializer callback function for pools.
 + *
 + * @param pkt   Handle of the packet
 + * @param uarea_init_argOpaque pointer defined in odp_pool_param_t
 + *
 + * @note If the application specifies this pointer, it expects that every
 buffer
 + * is initialized exactly once with it when the underlying memory is
 allocated.
 + * It is not called from odp_packet_alloc(), unless the platform chooses
 to
 + * allocate the memory at that point. Applications can only assume that
 this
 + * callback is called once before the packet is first used. Any subsequent
 + * change to the user area might be preserved after odp_packet_free() is
 called,
 + * so applications should take care of (re)initialization if they change
 data
 + * preset by this function.
 + */
 +typedef void (odp_packet_uarea_init_t)(odp_packet_t pkt, void
 *uarea_init_arg);
 +
 +/**
   * Pool parameters
   * Used to communicate pool creation options.
   */
 @@ -82,6 +100,14 @@ typedef struct odp_pool_param_t {
 /** User area size in bytes. Specify as 0 if no
 user
 area is needed. */
 uint32_t uarea_size;
 +
 +   /** Initialize every packet's user area at
 allocation
 +   time. Use NULL if no initialization needed. */
 +   odp_packet_uarea_init_t *uarea_init;
 +
 +   /** Opaque pointer passed to packet user area
 +   constructor. */
 +   void *uarea_init_arg;
 } pkt;
 struct {
 /** Number of timeouts in the pool */
 --
 1.9.1

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

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


[lng-odp] [PATCHv2 08/13] validation: scheduler: using _CU_TEST_INFO()

2015-08-20 Thread Christophe Milard
Signed-off-by: Christophe Milard christophe.mil...@linaro.org
Reviewed-by: Stuart Haslam stuart.has...@linaro.org
---
 test/validation/scheduler/scheduler.c | 60 +--
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/test/validation/scheduler/scheduler.c 
b/test/validation/scheduler/scheduler.c
index 1bcaee6..788f5c7 100644
--- a/test/validation/scheduler/scheduler.c
+++ b/test/validation/scheduler/scheduler.c
@@ -780,36 +780,36 @@ int scheduler_suite_term(void)
 }
 
 CU_TestInfo scheduler_suite[] = {
-   {schedule_wait_time,  scheduler_test_wait_time},
-   {schedule_num_prio,   scheduler_test_num_prio},
-   {schedule_queue_destroy,  scheduler_test_queue_destroy},
-   {schedule_1q_1t_n,scheduler_test_1q_1t_n},
-   {schedule_1q_1t_a,scheduler_test_1q_1t_a},
-   {schedule_1q_1t_o,scheduler_test_1q_1t_o},
-   {schedule_mq_1t_n,scheduler_test_mq_1t_n},
-   {schedule_mq_1t_a,scheduler_test_mq_1t_a},
-   {schedule_mq_1t_o,scheduler_test_mq_1t_o},
-   {schedule_mq_1t_prio_n,   scheduler_test_mq_1t_prio_n},
-   {schedule_mq_1t_prio_a,   scheduler_test_mq_1t_prio_a},
-   {schedule_mq_1t_prio_o,   scheduler_test_mq_1t_prio_o},
-   {schedule_mq_mt_prio_n,   scheduler_test_mq_mt_prio_n},
-   {schedule_mq_mt_prio_a,   scheduler_test_mq_mt_prio_a},
-   {schedule_mq_mt_prio_o,   scheduler_test_mq_mt_prio_o},
-   {schedule_1q_mt_a_excl,   scheduler_test_1q_mt_a_excl},
-   {schedule_multi_1q_1t_n,  scheduler_test_multi_1q_1t_n},
-   {schedule_multi_1q_1t_a,  scheduler_test_multi_1q_1t_a},
-   {schedule_multi_1q_1t_o,  scheduler_test_multi_1q_1t_o},
-   {schedule_multi_mq_1t_n,  scheduler_test_multi_mq_1t_n},
-   {schedule_multi_mq_1t_a,  scheduler_test_multi_mq_1t_a},
-   {schedule_multi_mq_1t_o,  scheduler_test_multi_mq_1t_o},
-   {schedule_multi_mq_1t_prio_n, scheduler_test_multi_mq_1t_prio_n},
-   {schedule_multi_mq_1t_prio_a, scheduler_test_multi_mq_1t_prio_a},
-   {schedule_multi_mq_1t_prio_o, scheduler_test_multi_mq_1t_prio_o},
-   {schedule_multi_mq_mt_prio_n, scheduler_test_multi_mq_mt_prio_n},
-   {schedule_multi_mq_mt_prio_a, scheduler_test_multi_mq_mt_prio_a},
-   {schedule_multi_mq_mt_prio_o, scheduler_test_multi_mq_mt_prio_o},
-   {schedule_multi_1q_mt_a_excl, scheduler_test_multi_1q_mt_a_excl},
-   {schedule_pause_resume,   scheduler_test_pause_resume},
+   _CU_TEST_INFO(scheduler_test_wait_time),
+   _CU_TEST_INFO(scheduler_test_num_prio),
+   _CU_TEST_INFO(scheduler_test_queue_destroy),
+   _CU_TEST_INFO(scheduler_test_1q_1t_n),
+   _CU_TEST_INFO(scheduler_test_1q_1t_a),
+   _CU_TEST_INFO(scheduler_test_1q_1t_o),
+   _CU_TEST_INFO(scheduler_test_mq_1t_n),
+   _CU_TEST_INFO(scheduler_test_mq_1t_a),
+   _CU_TEST_INFO(scheduler_test_mq_1t_o),
+   _CU_TEST_INFO(scheduler_test_mq_1t_prio_n),
+   _CU_TEST_INFO(scheduler_test_mq_1t_prio_a),
+   _CU_TEST_INFO(scheduler_test_mq_1t_prio_o),
+   _CU_TEST_INFO(scheduler_test_mq_mt_prio_n),
+   _CU_TEST_INFO(scheduler_test_mq_mt_prio_a),
+   _CU_TEST_INFO(scheduler_test_mq_mt_prio_o),
+   _CU_TEST_INFO(scheduler_test_1q_mt_a_excl),
+   _CU_TEST_INFO(scheduler_test_multi_1q_1t_n),
+   _CU_TEST_INFO(scheduler_test_multi_1q_1t_a),
+   _CU_TEST_INFO(scheduler_test_multi_1q_1t_o),
+   _CU_TEST_INFO(scheduler_test_multi_mq_1t_n),
+   _CU_TEST_INFO(scheduler_test_multi_mq_1t_a),
+   _CU_TEST_INFO(scheduler_test_multi_mq_1t_o),
+   _CU_TEST_INFO(scheduler_test_multi_mq_1t_prio_n),
+   _CU_TEST_INFO(scheduler_test_multi_mq_1t_prio_a),
+   _CU_TEST_INFO(scheduler_test_multi_mq_1t_prio_o),
+   _CU_TEST_INFO(scheduler_test_multi_mq_mt_prio_n),
+   _CU_TEST_INFO(scheduler_test_multi_mq_mt_prio_a),
+   _CU_TEST_INFO(scheduler_test_multi_mq_mt_prio_o),
+   _CU_TEST_INFO(scheduler_test_multi_1q_mt_a_excl),
+   _CU_TEST_INFO(scheduler_test_pause_resume),
CU_TEST_INFO_NULL,
 };
 
-- 
1.9.1

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


[lng-odp] [PATCHv2 12/13] validation: time: using _CU_TEST_INFO()

2015-08-20 Thread Christophe Milard
Signed-off-by: Christophe Milard christophe.mil...@linaro.org
Reviewed-by: Stuart Haslam stuart.has...@linaro.org
---
 test/validation/time/time.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/validation/time/time.c b/test/validation/time/time.c
index 4fa2eaf..4b81c2c 100644
--- a/test/validation/time/time.c
+++ b/test/validation/time/time.c
@@ -62,9 +62,9 @@ void time_test_odp_time_conversion(void)
 }
 
 CU_TestInfo time_suite_time[] = {
-   {cycles diff, time_test_odp_cycles_diff},
-   {negative diff, time_test_odp_cycles_negative_diff},
-   {conversion, time_test_odp_time_conversion},
+   _CU_TEST_INFO(time_test_odp_cycles_diff),
+   _CU_TEST_INFO(time_test_odp_cycles_negative_diff),
+   _CU_TEST_INFO(time_test_odp_time_conversion),
 CU_TEST_INFO_NULL
 };
 
-- 
1.9.1

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


[lng-odp] [Bug 1615] odp_timer fails in CI with Segmentation fault

2015-08-20 Thread bugzilla-daemon
https://bugs.linaro.org/show_bug.cgi?id=1615

Stuart Haslam stuart.has...@linaro.org changed:

   What|Removed |Added

 CC||stuart.has...@linaro.org

--- Comment #8 from Stuart Haslam stuart.has...@linaro.org ---
I'm seeing a semi-reproducible SEGV running timer_main on x86_64 (with 3 worker
cpus). It's always in the same place;

#0  timer_notify (sigval=sigval@entry=...) at odp_timer.c:643
#1  0x77bd70ff in timer_sigev_thread (arg=0x78c0)
at ../nptl/sysdeps/unix/sysv/linux/timer_routines.c:63
#2  0x7700b182 in start_thread (arg=0x765fd700) at
pthread_create.c:312
#3  0x776f647d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:111

odp_timer.c:643 is;

uint64_t prev_tick = odp_atomic_fetch_inc_u64(tp-cur_tick);

tp has the value that was assigned during odp_timer_pool_create()

HEAD is at a1e62a98b

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


Re: [lng-odp] [API-NEXT PATCH v3 1/3] api: packet: allow access to packet RSS hash values

2015-08-20 Thread Santosh Shukla
On 20 August 2015 at 16:23, Jerin Jacob jerin.ja...@caviumnetworks.com wrote:
 On Thu, Aug 20, 2015 at 03:30:23PM +0530, Santosh Shukla wrote:
 On 20 August 2015 at 14:48, Balasubramanian Manoharan
 bala.manoha...@linaro.org wrote:
 
 
  On Sunday 16 August 2015 06:14 PM, Santosh Shukla wrote:
 
  On 15 August 2015 at 00:12, Zoltan Kiss zoltan.k...@linaro.org wrote:
 
  Applications can read the computed hash (if any) and set it if they
  changed the packet headers or if the platform haven't calculated the
  hash.
 
  Signed-off-by: Zoltan Kiss zoltan.k...@linaro.org
  ---
  v2:
  - focus on RSS hash only
  - use setter/getter's
 
  v3:
  - do not mention pointers
  - add a note
  - add new patches for implementation and test
 
include/odp/api/packet.h | 30 ++
1 file changed, 30 insertions(+)
 
  diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
  index 3a454b5..1ae24bc 100644
  --- a/include/odp/api/packet.h
  +++ b/include/odp/api/packet.h
  @@ -48,6 +48,11 @@ extern C {
 * Invalid packet segment
 */
 
  +/**
  + * @def ODP_PACKET_RSS_INVALID
  + * RSS hash is not set
  + */
  +
/*
 *
 * Alloc and free
  @@ -605,6 +610,31 @@ uint32_t odp_packet_l4_offset(odp_packet_t pkt);
int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset);
 
/**
  + * RSS hash value
  + *
  + * Returns the RSS hash stored for the packet.
  + *
  + * @param  pkt  Packet handle
  + *
  + * @return  Hash value
  + * @retval ODP_PACKET_RSS_INVALID if RSS hash is not set.
  + */
  +uint32_t odp_packet_rss_hash(odp_packet_t pkt);
  +
  +/**
  + * Set RSS hash value
  + *
  + * Store the RSS hash for the packet.
  + *
  + * @param  pkt  Packet handle
  + * @param  rss_hash Hash value to set
  + *
  + * @note If the application changes the packet header, it might want to
  + * recalculate this value and set it.
  + */
  +void odp_packet_rss_hash_set(odp_packet_t pkt, uint32_t rss_hash);
 
  Can this use-case be handled by calling
  odp_packet_generate_rss_hash(odp_packet_t pkt) function where the rss gets
  generated by the implementation rather than being set from application 
  using
  odp_packet_set_rss_hash() function?
 

 Bala, As we discussed and in summary for rest; Considering ovs example
 : ovs uses sw based rss_hash only when HW not supporting rss Or HW
 generated rss is zero.

 hash = packet_get_hash(packet);
 if (OVS_UNLIKELY(!hash)) {
 hash = miniflow_hash_5tuple(mf, 0);
 packet_set_hash(packet, hash);
 }
 return hash;

 and rss_hash generation is inside ovs dp_packet (middle  layer), It is

 How about following change in OVS plarform specific packet_get_hash code.
 odp_packet_rss_hash_set
 kind of interface wont fit correctly in octeon  platform as hash
 identifier used by hardware in subsequent HW based queue operations

 static inline uint32_t
 packet_get_hash(struct dp_packet *p)
 {
 #ifdef DPDK_NETDEV
 return p-mbuf.hash.rss;
 #else
 #ifdef ODP_NETDEV
 unit32_t hash;
 hash = odp_packet_rss_hash(p-odp_pkt);
 if (OVS_UNLIKELY(!hash)
 hash = odp_packet_generate_rss_hash(p-odp_pkt);
 return hash;
 #endif
 return p-rss_hash;
 #endif
 }


Trying to understand your api definition;
odp_packet_rss_hash() - to get rss
and
odp_packet_generate_rss_hash() - generate rss from where :  sw or hw?
In either case, your trying to generate non-zero rss (considering a
valid rss (?) for your platform).

- IIUC _generate_rss_hash() able to take corrective measure so you
won't find need for using rss_hash_set() right?
- In other word, No s/w based rss __setting__ required for octeon.

did I understood all that right?


 with in ovs implementation, Not exposed to application layer, so
 application won't need to generate rss / update, so no possibility of
 rss mis-match /corruption.

  Regards,
  Bala
 
  +
  +/**
 * Tests if packet is segmented
 *
 * @param pkt  Packet handle
  --
 
  Reviewed-by : Santosh Shukla santosh.shu...@linaro.org
 
  1.9.1
 
  ___
  lng-odp mailing list
  lng-odp@lists.linaro.org
  https://lists.linaro.org/mailman/listinfo/lng-odp
 
  ___
  lng-odp mailing list
  lng-odp@lists.linaro.org
  https://lists.linaro.org/mailman/listinfo/lng-odp
 
 
 ___
 lng-odp mailing list
 lng-odp@lists.linaro.org
 https://lists.linaro.org/mailman/listinfo/lng-odp
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [PATCHv4] example:generator:move verbose from worker to control

2015-08-20 Thread Krishna Garapati


  +static void print_global_stats(int num_workers)
  +{
  + uint64_t start, now, diff;
  + uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0;
  + int verbose_interval = 20, worker_count;
  + odp_thrmask_t thrd_mask;
  +
  + start = odp_time_cycles();
  + while (1) {
  + now = odp_time_cycles();
  + diff = odp_time_diff_cycles(start, now);
  + if (odp_time_cycles_to_ns(diff) 
  +verbose_interval * ODP_TIME_SEC) {
  + continue;
  + }

 There's no exit condition check in this loop above, so you'll wait up to
 verbose_interval extra seconds unnecessarily, for example when running
 with -n 100 the workers will finish quickly.

 +
  + start = odp_time_cycles();
  +
  + worker_count = odp_thrmask_worker(thrd_mask);

 There's a potential race here as the worker thread counts get
 incremented after the thread has started (rather than during the
 odph_linux_pthread_create() call), and it's pretty likely this code
 will be reached by the main thread before the workers have started.

 Should wait until worker count has reached num_workers before entering
 the loop.



 verbose_interval * ODP_TIME_SEC internally a sleep time before reaching
this check. Even if we introduce the worker_count == num_workers, we might
have to do a default sleep anyway to make sure we have workers up and
running otherwise we break the loop. do we still need a separate check for
this ?



  + if (worker_count  num_workers)
  + break;

 --
 Stuart.

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


Re: [lng-odp] [API-NEXT PATCHv2 1/3] api: classification: add ODP_PMR_CUSTOM_FRAME

2015-08-20 Thread Benoît Ganne
Hi Bala,

Thanks for your feedback!

 odp_pmr_match_t structure was already defined and this patch just adds 
 offset variable so why don't we just add the additional variable in 
 the existing structure and maybe move the odp_pmr_create() function 
 below this structure if that was the concern.
 It narrows down the changes done by this patch and will be easier to 
 understand the changes required for adding the term PMR_CUSTOM_FRAME.

I disagree, for 2 reasons:
  1) move odp_create would in my opinion complicate code reads  by human:
I usually expect the function to be grouped together, ie finding
odp_create just above odp_destroy
  2) I tried what you suggests and the diffstat is identical (see below)
So it seems to me that it does not reduce the patch size and in the
contrary the header will be harder to read.

Here's the new diff:
 include/odp/api/classification.h | 55 
 1 file changed, 27 insertions(+), 28 deletions(-)

diff --git a/include/odp/api/classification.h b/include/odp/api/classification.h
index f597b26..b63a6c8 100644
--- a/include/odp/api/classification.h
+++ b/include/odp/api/classification.h
@@ -215,31 +215,17 @@ typedef enum odp_pmr_term {
ODP_PMR_IPSEC_SPI,  /** IPsec session identifier(*val=uint32_t)*/
ODP_PMR_LD_VNI, /** NVGRE/VXLAN network identifier
(*val=uint32_t)*/
+   ODP_PMR_CUSTOM_FRAME,   /** Custom match rule, offset from start of
+   frame. The match is defined by the offset, the
+   expected value, and its size. They must be
+   applied before any other PMR.
+   (*val=uint8_t[val_sz])*/
 
/** Inner header may repeat above values with this offset */
ODP_PMR_INNER_HDR_OFF = 32
 } odp_pmr_term_e;
 
 /**
- * Create a packet match rule with mask and value
- *
- * @param[in]  termOne of the enumerated values supported
- * @param[in]  val Value to match against the packet header
- * in native byte order.
- * @param[in]  maskMask to indicate which bits of the header
- * should be matched ('1') and
- * which should be ignored ('0')
- * @param[in]  val_sz  Size of the val and mask arguments,
- * that must match the value size requirement of the
- * specific term.
- *
- * @return Handle of the matching rule
- * @retval ODP_PMR_INVAL on failure
- */
-odp_pmr_t odp_pmr_create(odp_pmr_term_e term, const void *val,
-const void *mask, uint32_t val_sz);
-
-/**
  * Invalidate a packet match rule and vacate its resources
  *
  * @param[in]  pmr_id  Identifier of the PMR to be destroyed
@@ -290,27 +276,40 @@ unsigned long long odp_pmr_terms_cap(void);
 unsigned odp_pmr_terms_avail(void);
 
 /**
- * Following structure is used to define composite packet matching rules
- * in the form of an array of individual match rules.
- * The underlying platform may not support all or any specific combination
- * of value match rules, and the application should take care
- * of inspecting the return value when installing such rules, and perform
- * appropriate fallback action.
+ * Following structure is used to define a packet matching rule
  */
 typedef struct odp_pmr_match_t {
odp_pmr_term_e  term;   /** PMR term value to be matched */
const void  *val;   /** Value to be matched */
const void  *mask;  /** Masked set of bits to be matched */
-   unsigned intval_sz;  /** Size of the term value */
+   uint32_tval_sz;  /** Size of the term value */
+   uint32_toffset;  /** User-defined offset in packet
+Used if term == ODP_PMR_CUSTOM_FRAME only,
+otherwise must be 0 */
 } odp_pmr_match_t;
 
 /**
+ * Create a packet match rule with mask and value
+ *
+ * @param[in]  match   packet matching rule definition
+ *
+ * @return Handle of the matching rule
+ * @retval ODP_PMR_INVAL on failure
+ */
+odp_pmr_t odp_pmr_create(const odp_pmr_match_t *match);
+
+/**
  * @typedef odp_pmr_set_t
  * An opaque handle to a composite packet match rule-set
  */
 
 /**
- * Create a composite packet match rule
+ * Create a composite packet match rule in the form of an array of individual
+ * match rules.
+ * The underlying platform may not support all or any specific combination
+ * of value match rules, and the application should take care
+ * of inspecting the return value when installing such rules, and perform
+ * appropriate fallback action.
  *
  * @param[in]  num_terms   Number of terms in the match rule.
  * @param[in]  terms   Array of num_terms entries, one entry per
@@ -322,7 +321,7 @@ typedef struct odp_pmr_match_t {
  * underlying platform 

Re: [lng-odp] [API-NEXT PATCHv2 1/3] api: classification: add ODP_PMR_CUSTOM_FRAME

2015-08-20 Thread Benoît Ganne

IMO, moving the functions for human readability could be done as a
separate patch.


Fair enough. If this is what is expected, I'll repost a patch serie 
soon. I personally think it is a bit convoluted though :)


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


[lng-odp] [API-NEXT PATCHv3 2/4] api: classification: add ODP_PMR_CUSTOM_FRAME

2015-08-20 Thread Benoît Ganne
This patch only moves definitions. It does
not modify any structures, functions or
comments.

Move odp_pmr_match_t and odp_pmr_create()
definitions before odp_pmr_detroy() for
better readability.

Signed-off-by: Benoît Ganne bga...@kalray.eu
---
 include/odp/api/classification.h | 46 
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/include/odp/api/classification.h b/include/odp/api/classification.h
index b63a6c8..d4e1330 100644
--- a/include/odp/api/classification.h
+++ b/include/odp/api/classification.h
@@ -226,6 +226,29 @@ typedef enum odp_pmr_term {
 } odp_pmr_term_e;
  /**
+ * Following structure is used to define a packet matching rule
+ */
+typedef struct odp_pmr_match_t {
+   odp_pmr_term_e  term;   /** PMR term value to be matched */
+   const void  *val;   /** Value to be matched */
+   const void  *mask;  /** Masked set of bits to be matched */
+   uint32_tval_sz;  /** Size of the term value */
+   uint32_toffset;  /** User-defined offset in packet
+Used if term == ODP_PMR_CUSTOM_FRAME only,
+otherwise must be 0 */
+} odp_pmr_match_t;
+
+/**
+ * Create a packet match rule with mask and value
+ *
+ * @param[in]  match   packet matching rule definition
+ *
+ * @return Handle of the matching rule
+ * @retval ODP_PMR_INVAL on failure
+ */
+odp_pmr_t odp_pmr_create(const odp_pmr_match_t *match);
+
+/**
  * Invalidate a packet match rule and vacate its resources
  *
  * @param[in]  pmr_id  Identifier of the PMR to be destroyed
@@ -276,29 +299,6 @@ unsigned long long odp_pmr_terms_cap(void);
 unsigned odp_pmr_terms_avail(void);
  /**
- * Following structure is used to define a packet matching rule
- */
-typedef struct odp_pmr_match_t {
-   odp_pmr_term_e  term;   /** PMR term value to be matched */
-   const void  *val;   /** Value to be matched */
-   const void  *mask;  /** Masked set of bits to be matched */
-   uint32_tval_sz;  /** Size of the term value */
-   uint32_toffset;  /** User-defined offset in packet
-Used if term == ODP_PMR_CUSTOM_FRAME only,
-otherwise must be 0 */
-} odp_pmr_match_t;
-
-/**
- * Create a packet match rule with mask and value
- *
- * @param[in]  match   packet matching rule definition
- *
- * @return Handle of the matching rule
- * @retval ODP_PMR_INVAL on failure
- */
-odp_pmr_t odp_pmr_create(const odp_pmr_match_t *match);
-
-/**
  * @typedef odp_pmr_set_t
  * An opaque handle to a composite packet match rule-set
  */
-- 
2.5.0

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


[lng-odp] [API-NEXT PATCHv3 3/4] linux-generic: classification: implement ODP_PMR_CUSTOM_FRAME matching

2015-08-20 Thread Benoît Ganne
Signed-off-by: Benoît Ganne bga...@kalray.eu
---
 .../include/odp_classification_datamodel.h |  2 ++
 .../include/odp_classification_inlines.h   | 21 +++
 platform/linux-generic/odp_classification.c| 42 --
 3 files changed, 46 insertions(+), 19 deletions(-)

diff --git a/platform/linux-generic/include/odp_classification_datamodel.h 
b/platform/linux-generic/include/odp_classification_datamodel.h
index 9da54c7..0e216db 100644
--- a/platform/linux-generic/include/odp_classification_datamodel.h
+++ b/platform/linux-generic/include/odp_classification_datamodel.h
@@ -55,6 +55,8 @@ The maximum size of value currently supported in 64 bits
 **/
 typedef struct pmr_term_value {
odp_pmr_term_e  term;   /* PMR Term */
+   uint16_toffset; /** Offset if term == ODP_PMR_CUSTOM_FRAME */
+   uint16_tval_sz; /** Size of the value to be matched */
uint64_tval;/** Value to be matched */
uint64_tmask;   /** Masked set of bits to be matched */
 } pmr_term_value_t;
diff --git a/platform/linux-generic/include/odp_classification_inlines.h 
b/platform/linux-generic/include/odp_classification_inlines.h
index a093211..b9b430f 100644
--- a/platform/linux-generic/include/odp_classification_inlines.h
+++ b/platform/linux-generic/include/odp_classification_inlines.h
@@ -223,6 +223,27 @@ static inline int verify_pmr_ld_vni(uint8_t *pkt_addr 
ODP_UNUSED,
ODP_UNIMPLEMENTED();
return 0;
 }
+
+static inline int verify_pmr_custom_frame(uint8_t *pkt_addr,
+ odp_packet_hdr_t *pkt_hdr,
+ pmr_term_value_t *term_value)
+{
+   uint64_t val = 0;
+   uint16_t offset = term_value-offset;
+   uint16_t val_sz = term_value-val_sz;
+
+   ODP_ASSERT(val_sz = ODP_PMR_TERM_BYTES_MAX);
+
+   if (pkt_hdr-frame_len = offset + val_sz)
+   return 0;
+
+   memcpy(val, pkt_addr + offset, val_sz);
+   if (term_value-val == (val  term_value-mask))
+   return 1;
+
+   return 0;
+}
+
 static inline int verify_pmr_eth_type_0(uint8_t *pkt_addr ODP_UNUSED,
odp_packet_hdr_t *pkt_hdr ODP_UNUSED,
pmr_term_value_t *term_value ODP_UNUSED)
diff --git a/platform/linux-generic/odp_classification.c 
b/platform/linux-generic/odp_classification.c
index fdb544d..6b4868b 100644
--- a/platform/linux-generic/odp_classification.c
+++ b/platform/linux-generic/odp_classification.c
@@ -414,12 +414,24 @@ int odp_cos_with_l3_qos(odp_pktio_t pktio_in,
return 0;
 }
 -odp_pmr_t odp_pmr_create(odp_pmr_term_e term, const void *val,
-const void *mask, uint32_t val_sz)
+static void odp_pmr_create_term(pmr_term_value_t *value,
+   const odp_pmr_match_t *match)
+{
+   value-term = match-term;
+   value-offset = match-offset;
+   value-val_sz = match-val_sz;
+   value-val = 0;
+   value-mask = 0;
+   memcpy(value-val, match-val, match-val_sz);
+   memcpy(value-mask, match-mask, match-val_sz);
+   value-val = value-mask;
+}
+
+odp_pmr_t odp_pmr_create(const odp_pmr_match_t *match)
 {
pmr_t *pmr;
odp_pmr_t id;
-   if (val_sz  ODP_PMR_TERM_BYTES_MAX) {
+   if (match-val_sz  ODP_PMR_TERM_BYTES_MAX) {
ODP_ERR(val_sz greater than max supported limit);
return ODP_PMR_INVAL;
}
@@ -430,12 +442,7 @@ odp_pmr_t odp_pmr_create(odp_pmr_term_e term, const void 
*val,
return ODP_PMR_INVAL;
pmr-s.num_pmr = 1;
-   pmr-s.pmr_term_value[0].term = term;
-   pmr-s.pmr_term_value[0].val =  0;
-   pmr-s.pmr_term_value[0].mask =  0;
-   memcpy(pmr-s.pmr_term_value[0].val, val, val_sz);
-   memcpy(pmr-s.pmr_term_value[0].mask, mask, val_sz);
-   pmr-s.pmr_term_value[0].val = pmr-s.pmr_term_value[0].mask;
+   odp_pmr_create_term(pmr-s.pmr_term_value[0], match);
UNLOCK(pmr-s.lock);
return id;
 }
@@ -536,7 +543,7 @@ unsigned odp_pmr_terms_avail(void)
return count;
 }
 -int odp_pmr_match_set_create(int num_terms, odp_pmr_match_t *terms,
+int odp_pmr_match_set_create(int num_terms, const odp_pmr_match_t *terms,
 odp_pmr_set_t *pmr_set_id)
 {
pmr_t *pmr;
@@ -562,15 +569,7 @@ int odp_pmr_match_set_create(int num_terms, 
odp_pmr_match_t *terms,
val_sz = terms[i].val_sz;
if (val_sz  ODP_PMR_TERM_BYTES_MAX)
continue;
-   pmr-s.pmr_term_value[i].term = terms[i].term;
-   pmr-s.pmr_term_value[i].val = 0;
-   pmr-s.pmr_term_value[i].mask = 0;
-   memcpy(pmr-s.pmr_term_value[i].val,
-  terms[i].val, val_sz);
-   memcpy(pmr-s.pmr_term_value[i].mask,
-  terms[i].mask, val_sz);
-

[lng-odp] [API-NEXT PATCHv3 1/4] api: classification: add ODP_PMR_CUSTOM_FRAME

2015-08-20 Thread Benoît Ganne
The application can now specify a packet offset
in PMR rules. This offset is absolute from the
frame start. It is used to extract the PMR value.

This is useful to support arbitrary backplane
protocols and extensions.

Signed-off-by: Benoît Ganne bga...@kalray.eu
---
 include/odp/api/classification.h | 55 
 1 file changed, 27 insertions(+), 28 deletions(-)

diff --git a/include/odp/api/classification.h b/include/odp/api/classification.h
index f597b26..b63a6c8 100644
--- a/include/odp/api/classification.h
+++ b/include/odp/api/classification.h
@@ -215,31 +215,17 @@ typedef enum odp_pmr_term {
ODP_PMR_IPSEC_SPI,  /** IPsec session identifier(*val=uint32_t)*/
ODP_PMR_LD_VNI, /** NVGRE/VXLAN network identifier
(*val=uint32_t)*/
+   ODP_PMR_CUSTOM_FRAME,   /** Custom match rule, offset from start of
+   frame. The match is defined by the offset, the
+   expected value, and its size. They must be
+   applied before any other PMR.
+   (*val=uint8_t[val_sz])*/
/** Inner header may repeat above values with this offset */
ODP_PMR_INNER_HDR_OFF = 32
 } odp_pmr_term_e;
  /**
- * Create a packet match rule with mask and value
- *
- * @param[in]  termOne of the enumerated values supported
- * @param[in]  val Value to match against the packet header
- * in native byte order.
- * @param[in]  maskMask to indicate which bits of the header
- * should be matched ('1') and
- * which should be ignored ('0')
- * @param[in]  val_sz  Size of the val and mask arguments,
- * that must match the value size requirement of the
- * specific term.
- *
- * @return Handle of the matching rule
- * @retval ODP_PMR_INVAL on failure
- */
-odp_pmr_t odp_pmr_create(odp_pmr_term_e term, const void *val,
-const void *mask, uint32_t val_sz);
-
-/**
  * Invalidate a packet match rule and vacate its resources
  *
  * @param[in]  pmr_id  Identifier of the PMR to be destroyed
@@ -290,27 +276,40 @@ unsigned long long odp_pmr_terms_cap(void);
 unsigned odp_pmr_terms_avail(void);
  /**
- * Following structure is used to define composite packet matching rules
- * in the form of an array of individual match rules.
- * The underlying platform may not support all or any specific combination
- * of value match rules, and the application should take care
- * of inspecting the return value when installing such rules, and perform
- * appropriate fallback action.
+ * Following structure is used to define a packet matching rule
  */
 typedef struct odp_pmr_match_t {
odp_pmr_term_e  term;   /** PMR term value to be matched */
const void  *val;   /** Value to be matched */
const void  *mask;  /** Masked set of bits to be matched */
-   unsigned intval_sz;  /** Size of the term value */
+   uint32_tval_sz;  /** Size of the term value */
+   uint32_toffset;  /** User-defined offset in packet
+Used if term == ODP_PMR_CUSTOM_FRAME only,
+otherwise must be 0 */
 } odp_pmr_match_t;
  /**
+ * Create a packet match rule with mask and value
+ *
+ * @param[in]  match   packet matching rule definition
+ *
+ * @return Handle of the matching rule
+ * @retval ODP_PMR_INVAL on failure
+ */
+odp_pmr_t odp_pmr_create(const odp_pmr_match_t *match);
+
+/**
  * @typedef odp_pmr_set_t
  * An opaque handle to a composite packet match rule-set
  */
  /**
- * Create a composite packet match rule
+ * Create a composite packet match rule in the form of an array of individual
+ * match rules.
+ * The underlying platform may not support all or any specific combination
+ * of value match rules, and the application should take care
+ * of inspecting the return value when installing such rules, and perform
+ * appropriate fallback action.
  *
  * @param[in]  num_terms   Number of terms in the match rule.
  * @param[in]  terms   Array of num_terms entries, one entry per
@@ -322,7 +321,7 @@ typedef struct odp_pmr_match_t {
  * underlying platform classification engine
  * @retval 0 on failure
  */
-int odp_pmr_match_set_create(int num_terms, odp_pmr_match_t *terms,
+int odp_pmr_match_set_create(int num_terms, const odp_pmr_match_t *terms,
 odp_pmr_set_t *pmr_set_id);
  /**
-- 
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 v3 1/3] api: packet: allow access to packet RSS hash values

2015-08-20 Thread Bill Fischofer
The RSS is relevant to packets originating from a NIC and is independent of
the CoS or other flow designators.  It's there mainly because some
applications (e.g., OVS) use it internally, so it's for legacy support.

On Thu, Aug 20, 2015 at 6:43 AM, Jerin Jacob jerin.ja...@caviumnetworks.com
 wrote:

 On Thu, Aug 20, 2015 at 04:52:29PM +0530, Santosh Shukla wrote:
  On 20 August 2015 at 16:23, Jerin Jacob jerin.ja...@caviumnetworks.com
 wrote:
   On Thu, Aug 20, 2015 at 03:30:23PM +0530, Santosh Shukla wrote:
   On 20 August 2015 at 14:48, Balasubramanian Manoharan
   bala.manoha...@linaro.org wrote:
   
   
On Sunday 16 August 2015 06:14 PM, Santosh Shukla wrote:
   
On 15 August 2015 at 00:12, Zoltan Kiss zoltan.k...@linaro.org
 wrote:
   
Applications can read the computed hash (if any) and set it if
 they
changed the packet headers or if the platform haven't calculated
 the
hash.
   
Signed-off-by: Zoltan Kiss zoltan.k...@linaro.org
---
v2:
- focus on RSS hash only
- use setter/getter's
   
v3:
- do not mention pointers
- add a note
- add new patches for implementation and test
   
  include/odp/api/packet.h | 30 ++
  1 file changed, 30 insertions(+)
   
diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
index 3a454b5..1ae24bc 100644
--- a/include/odp/api/packet.h
+++ b/include/odp/api/packet.h
@@ -48,6 +48,11 @@ extern C {
   * Invalid packet segment
   */
   
+/**
+ * @def ODP_PACKET_RSS_INVALID
+ * RSS hash is not set
+ */
+
  /*
   *
   * Alloc and free
@@ -605,6 +610,31 @@ uint32_t odp_packet_l4_offset(odp_packet_t
 pkt);
  int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset);
   
  /**
+ * RSS hash value
+ *
+ * Returns the RSS hash stored for the packet.
+ *
+ * @param  pkt  Packet handle
+ *
+ * @return  Hash value
+ * @retval ODP_PACKET_RSS_INVALID if RSS hash is not set.
+ */
+uint32_t odp_packet_rss_hash(odp_packet_t pkt);
+
+/**
+ * Set RSS hash value
+ *
+ * Store the RSS hash for the packet.
+ *
+ * @param  pkt  Packet handle
+ * @param  rss_hash Hash value to set
+ *
+ * @note If the application changes the packet header, it might
 want to
+ * recalculate this value and set it.
+ */
+void odp_packet_rss_hash_set(odp_packet_t pkt, uint32_t
 rss_hash);
   
Can this use-case be handled by calling
odp_packet_generate_rss_hash(odp_packet_t pkt) function where the
 rss gets
generated by the implementation rather than being set from
 application using
odp_packet_set_rss_hash() function?
   
  
   Bala, As we discussed and in summary for rest; Considering ovs example
   : ovs uses sw based rss_hash only when HW not supporting rss Or HW
   generated rss is zero.
  
   hash = packet_get_hash(packet);
   if (OVS_UNLIKELY(!hash)) {
   hash = miniflow_hash_5tuple(mf, 0);
   packet_set_hash(packet, hash);
   }
   return hash;
  
   and rss_hash generation is inside ovs dp_packet (middle  layer), It is
  
   How about following change in OVS plarform specific packet_get_hash
 code.
   odp_packet_rss_hash_set
   kind of interface wont fit correctly in octeon  platform as hash
   identifier used by hardware in subsequent HW based queue operations
  
   static inline uint32_t
   packet_get_hash(struct dp_packet *p)
   {
   #ifdef DPDK_NETDEV
   return p-mbuf.hash.rss;
   #else
   #ifdef ODP_NETDEV
   unit32_t hash;
   hash = odp_packet_rss_hash(p-odp_pkt);
   if (OVS_UNLIKELY(!hash)
   hash = odp_packet_generate_rss_hash(p-odp_pkt);
   return hash;
   #endif
   return p-rss_hash;
   #endif
   }
  
 
  Trying to understand your api definition;
  odp_packet_rss_hash() - to get rss
  and
  odp_packet_generate_rss_hash() - generate rss from where :  sw or hw?

 NOP if HW is capable, for software generated packet/ non-capable HW it
 will be in SW

  In either case, your trying to generate non-zero rss (considering a
  valid rss (?) for your platform).

 IMO, The term RSS may not be correct in API definition. May be something
 like flow id, make sense across all platform.

 Jerin

 
  - IIUC _generate_rss_hash() able to take corrective measure so you
  won't find need for using rss_hash_set() right?
  - In other word, No s/w based rss __setting__ required for octeon.
 
  did I understood all that right?
 
  
   with in ovs implementation, Not exposed to application layer, so
   application won't need to generate rss / update, so no possibility of
   rss mis-match /corruption.
  
Regards,
Bala
   
+
+/**
   * Tests if packet is segmented
   *
   * @param pkt  Packet handle
--
   
Reviewed-by : Santosh Shukla santosh.shu...@linaro.org
   
1.9.1
   
___
 

Re: [lng-odp] [PATCHv2 00/13] validation: use macro _CU_TEST_INFO() in all tests

2015-08-20 Thread Christophe Milard
since v1, I mean... :-)

On 20 August 2015 at 12:09, Christophe Milard christophe.mil...@linaro.org
wrote:

 Just rebased since v2

 On 20 August 2015 at 12:05, Christophe Milard 
 christophe.mil...@linaro.org wrote:

 This is another step in getting the test environment more homogeneous.
 The usage of this macro guaranties that the test name (string in C_unit)
 will always match the function name.
 This is needed as the C_unit strings will be used towards the C_unit API
 (to skip a test, for instance).

 Christophe Milard (13):
   validation: factorise _CU_TEST_INFO definition
   validation: cpumask: using _CU_TEST_INFO()
   validation: crypto: using _CU_TEST_INFO()
   validation: errno: using _CU_TEST_INFO()
   validation: init: using _CU_TEST_INFO()
   validation: pktio: using _CU_TEST_INFO()
   validation: queue: using _CU_TEST_INFO()
   validation: scheduler: using _CU_TEST_INFO()
   validation: shmem: using _CU_TEST_INFO()
   validation: synchronizers: using _CU_TEST_INFO()
   validation: system: using _CU_TEST_INFO()
   validation: time: using _CU_TEST_INFO()
   validation: timer: using _CU_TEST_INFO()

  test/validation/buffer/buffer.c|  3 --
  .../classification/odp_classification_basic.c  |  1 +
  .../classification/odp_classification_testsuites.h |  3 --
  test/validation/common/odp_cunit_common.h  |  3 ++
  test/validation/cpumask/cpumask.c  | 36 ++---
  test/validation/crypto/odp_crypto_test_inp.c   | 18 +++
  test/validation/errno/errno.c  |  2 +-
  test/validation/init/init.c|  6 +--
  test/validation/packet/packet.c|  3 --
  test/validation/pktio/pktio.c  | 24 -
  test/validation/pool/pool.c|  2 -
  test/validation/queue/queue.c  |  2 +-
  test/validation/random/random.c|  3 --
  test/validation/scheduler/scheduler.c  | 60
 +++---
  test/validation/shmem/shmem.c  |  2 +-
  test/validation/synchronizers/synchronizers.c  | 26 +-
  test/validation/system/system.c| 14 ++---
  test/validation/thread/thread.c|  3 --
  test/validation/time/time.c|  6 +--
  test/validation/timer/timer.c  |  8 +--
  20 files changed, 103 insertions(+), 122 deletions(-)

 --
 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 PATCHv2 1/3] api: classification: add ODP_PMR_CUSTOM_FRAME

2015-08-20 Thread Balasubramanian Manoharan

Hi,

On Tuesday 18 August 2015 10:30 PM, Benoît Ganne wrote:

The application can now specify a packet offset
in PMR rules. This offset is absolute from the
frame start. It is used to extract the PMR value.

This is useful to support arbitrary backplane
protocols and extensions.

Signed-off-by: Benoît Ganne bga...@kalray.eu
---
  include/odp/api/classification.h | 55 
  1 file changed, 27 insertions(+), 28 deletions(-)

diff --git a/include/odp/api/classification.h b/include/odp/api/classification.h
index f597b26..d4e1330 100644
--- a/include/odp/api/classification.h
+++ b/include/odp/api/classification.h
@@ -215,29 +215,38 @@ typedef enum odp_pmr_term {
ODP_PMR_IPSEC_SPI,  /** IPsec session identifier(*val=uint32_t)*/
ODP_PMR_LD_VNI, /** NVGRE/VXLAN network identifier
(*val=uint32_t)*/
+   ODP_PMR_CUSTOM_FRAME,   /** Custom match rule, offset from start of
+   frame. The match is defined by the offset, the
+   expected value, and its size. They must be
+   applied before any other PMR.
+   (*val=uint8_t[val_sz])*/
/** Inner header may repeat above values with this offset */
ODP_PMR_INNER_HDR_OFF = 32
  } odp_pmr_term_e;
   /**
+ * Following structure is used to define a packet matching rule
+ */
+typedef struct odp_pmr_match_t {
+   odp_pmr_term_e  term;   /** PMR term value to be matched */
+   const void  *val;   /** Value to be matched */
+   const void  *mask;  /** Masked set of bits to be matched */
+   uint32_tval_sz;  /** Size of the term value */
+   uint32_toffset;  /** User-defined offset in packet
+Used if term == ODP_PMR_CUSTOM_FRAME only,
+otherwise must be 0 */
+} odp_pmr_match_t;
odp_pmr_match_t structure was already defined and this patch just adds 
offset variable so why don't we just add the additional variable in 
the existing structure and maybe move the odp_pmr_create() function 
below this structure if that was the concern.
It narrows down the changes done by this patch and will be easier to 
understand the changes required for adding the term PMR_CUSTOM_FRAME.


Regards,
Bala

+
+/**
   * Create a packet match rule with mask and value
   *
- * @param[in]  termOne of the enumerated values supported
- * @param[in]  val Value to match against the packet header
- * in native byte order.
- * @param[in]  maskMask to indicate which bits of the header
- * should be matched ('1') and
- * which should be ignored ('0')
- * @param[in]  val_sz  Size of the val and mask arguments,
- * that must match the value size requirement of the
- * specific term.
+ * @param[in]  match   packet matching rule definition
   *
   * @returnHandle of the matching rule
   * @retvalODP_PMR_INVAL on failure
   */
-odp_pmr_t odp_pmr_create(odp_pmr_term_e term, const void *val,
-const void *mask, uint32_t val_sz);
+odp_pmr_t odp_pmr_create(const odp_pmr_match_t *match);
   /**
   * Invalidate a packet match rule and vacate its resources
@@ -290,27 +299,17 @@ unsigned long long odp_pmr_terms_cap(void);
  unsigned odp_pmr_terms_avail(void);
   /**
- * Following structure is used to define composite packet matching rules
- * in the form of an array of individual match rules.
- * The underlying platform may not support all or any specific combination
- * of value match rules, and the application should take care
- * of inspecting the return value when installing such rules, and perform
- * appropriate fallback action.
- */
-typedef struct odp_pmr_match_t {
-   odp_pmr_term_e  term;   /** PMR term value to be matched */
-   const void  *val;   /** Value to be matched */
-   const void  *mask;  /** Masked set of bits to be matched */
-   unsigned intval_sz;  /** Size of the term value */
-} odp_pmr_match_t;
-
-/**
   * @typedef odp_pmr_set_t
   * An opaque handle to a composite packet match rule-set
   */
   /**
- * Create a composite packet match rule
+ * Create a composite packet match rule in the form of an array of individual
+ * match rules.
+ * The underlying platform may not support all or any specific combination
+ * of value match rules, and the application should take care
+ * of inspecting the return value when installing such rules, and perform
+ * appropriate fallback action.
   *
   * @param[in] num_terms   Number of terms in the match rule.
   * @param[in] terms   Array of num_terms entries, one entry per
@@ -322,7 +321,7 @@ typedef struct odp_pmr_match_t {
   *underlying platform classification engine
   * @retval0 on failure
   

[lng-odp] [API-NEXT PATCHv3 4/4] example: classifier: implement ODP_PMR_CUSTOM_FRAME match

2015-08-20 Thread Benoît Ganne
Signed-off-by: Benoît Ganne bga...@kalray.eu
---
 example/classifier/odp_classifier.c | 95 +
 1 file changed, 76 insertions(+), 19 deletions(-)

diff --git a/example/classifier/odp_classifier.c 
b/example/classifier/odp_classifier.c
index 6d6dd59..5f8ec5c 100644
--- a/example/classifier/odp_classifier.c
+++ b/example/classifier/odp_classifier.c
@@ -53,11 +53,12 @@ typedef struct {
odp_pmr_t pmr;  /** Associated pmr handle */
odp_atomic_u64_t packet_count;  /** count of received packets */
char queue_name[ODP_QUEUE_NAME_LEN];/** queue name */
-   int val_sz; /** size of the pmr term */
struct {
odp_pmr_term_e term;/** odp pmr term value */
-   uint32_t val;   /** pmr term value */
-   uint32_t mask;  /** pmr term mask */
+   uint64_t val;   /** pmr term value */
+   uint64_t mask;  /** pmr term mask */
+   uint32_t val_sz;/** size of the pmr term */
+   uint32_t offset;/** pmr term offset */
} rule;
char value[DISPLAY_STRING_LEN]; /** Display string for value */
char mask[DISPLAY_STRING_LEN];  /** Display string for mask */
@@ -86,7 +87,8 @@ static void print_info(char *progname, appl_args_t 
*appl_args);
 static void usage(char *progname);
 static void configure_cos_queue(odp_pktio_t pktio, appl_args_t *args);
 static void configure_default_queue(odp_pktio_t pktio, appl_args_t *args);
-static int convert_str_to_pmr_enum(char *token, odp_pmr_term_e *term);
+static int convert_str_to_pmr_enum(char *token, odp_pmr_term_e *term,
+  uint32_t *offset);
 static int parse_pmr_policy(appl_args_t *appl_args, char *argv[], char 
*optarg);
  static inline
@@ -151,12 +153,13 @@ void print_cls_statistics(appl_args_t *args)
 }
  static inline
-int parse_ipv4_addr(const char *ipaddress, uint32_t *addr)
+int parse_ipv4_addr(const char *ipaddress, uint64_t *addr)
 {
-   int b[4];
+   uint32_t b[4];
int converted;
 -  converted = sscanf(ipaddress, %d.%d.%d.%d,
+   converted = sscanf(ipaddress, % SCNx32 .% SCNx32
+  .% SCNx32 .% SCNx32,
b[3], b[2], b[1], b[0]);
if (4 != converted)
return -1;
@@ -170,16 +173,42 @@ int parse_ipv4_addr(const char *ipaddress, uint32_t *addr)
 }
  static inline
-int parse_ipv4_mask(const char *str, uint32_t *mask)
+int parse_mask(const char *str, uint64_t *mask)
 {
-   uint32_t b;
+   uint64_t b;
int ret;
 -  ret = sscanf(str, % SCNx32, b);
+   ret = sscanf(str, % SCNx64, b);
*mask = b;
return ret != 1;
 }
 +static
+int parse_value(const char *str, uint64_t *val, unsigned int *val_sz)
+{
+   size_t len;
+   size_t i;
+   int converted;
+   union {
+   uint64_t u64;
+   uint8_t u8[8];
+   } buf = {.u64 = 0};
+
+   len = strlen(str);
+   if (len  2 * sizeof(buf))
+   return -1;
+
+   for (i = 0; i  len; i += 2) {
+   converted = sscanf(str[i], %2 SCNx8, buf.u8[i / 2]);
+   if (1 != converted)
+   return -1;
+   }
+
+   *val = buf.u64;
+   *val_sz = len / 2;
+   return 0;
+}
+
 /**
  * Create a pktio handle, optionally associating a default input queue.
  *
@@ -348,10 +377,14 @@ static void configure_cos_queue(odp_pktio_t pktio, 
appl_args_t *args)
sprintf(cos_name, CoS%s, stats-queue_name);
stats-cos = odp_cos_create(cos_name);
 -  stats-pmr = odp_pmr_create(stats-rule.term,
-   stats-rule.val,
-   stats-rule.mask,
-   stats-val_sz);
+   const odp_pmr_match_t match = {
+   .term = stats-rule.term,
+   .val = stats-rule.val,
+   .mask = stats-rule.mask,
+   .val_sz = stats-rule.val_sz,
+   .offset = stats-rule.offset,
+   };
+   stats-pmr = odp_pmr_create(match);
qparam.sched.prio = i % odp_schedule_num_prio();
qparam.sched.sync = ODP_SCHED_SYNC_NONE;
qparam.sched.group = ODP_SCHED_GROUP_ALL;
@@ -556,7 +589,8 @@ static void swap_pkt_addrs(odp_packet_t pkt_tbl[], unsigned 
len)
}
 }
 -static int convert_str_to_pmr_enum(char *token, odp_pmr_term_e *term)
+static int convert_str_to_pmr_enum(char *token, odp_pmr_term_e *term,
+  uint32_t *offset)
 {
if (NULL == token)
return -1;
@@ -564,6 +598,13 @@ static int convert_str_to_pmr_enum(char *token, 
odp_pmr_term_e *term)
if (0 == strcasecmp(token, ODP_PMR_SIP_ADDR)) {
*term = ODP_PMR_SIP_ADDR;
return 0;
+  

[lng-odp] [PATCHv2 01/13] validation: factorise _CU_TEST_INFO definition

2015-08-20 Thread Christophe Milard
The macro _CU_TEST_INFO (used ensure a match between test function names
and test names (strings)) is now defined in .../common/odp_cunit_common.h
as it is common to many API module tests.
Local definitions are also removed.

Signed-off-by: Christophe Milard christophe.mil...@linaro.org
Reviewed-by: Stuart Haslam stuart.has...@linaro.org
---
 test/validation/buffer/buffer.c| 3 ---
 test/validation/classification/odp_classification_basic.c  | 1 +
 test/validation/classification/odp_classification_testsuites.h | 3 ---
 test/validation/common/odp_cunit_common.h  | 3 +++
 test/validation/packet/packet.c| 3 ---
 test/validation/pool/pool.c| 2 --
 test/validation/random/random.c| 3 ---
 test/validation/thread/thread.c| 3 ---
 8 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/test/validation/buffer/buffer.c b/test/validation/buffer/buffer.c
index 52585b9..c62938d 100644
--- a/test/validation/buffer/buffer.c
+++ b/test/validation/buffer/buffer.c
@@ -8,9 +8,6 @@
 #include odp_cunit_common.h
 #include buffer.h
 
-/* Helper macro for CU_TestInfo initialization */
-#define _CU_TEST_INFO(test_func) {#test_func, test_func}
-
 static odp_pool_t raw_pool;
 static odp_buffer_t raw_buffer = ODP_BUFFER_INVALID;
 static const size_t raw_buffer_size = 1500;
diff --git a/test/validation/classification/odp_classification_basic.c 
b/test/validation/classification/odp_classification_basic.c
index 79c775e..3ae0128 100644
--- a/test/validation/classification/odp_classification_basic.c
+++ b/test/validation/classification/odp_classification_basic.c
@@ -4,6 +4,7 @@
  * SPDX-License-Identifier:BSD-3-Clause
  */
 
+#include odp_cunit_common.h
 #include odp_classification_testsuites.h
 #include classification.h
 
diff --git a/test/validation/classification/odp_classification_testsuites.h 
b/test/validation/classification/odp_classification_testsuites.h
index f603f30..37c019d 100644
--- a/test/validation/classification/odp_classification_testsuites.h
+++ b/test/validation/classification/odp_classification_testsuites.h
@@ -11,9 +11,6 @@
 #include CUnit/CUnit.h
 #include CUnit/Basic.h
 
-/* Helper macro for CU_TestInfo initialization */
-#define _CU_TEST_INFO(test_func) {#test_func, test_func}
-
 extern CU_TestInfo classification_suite[];
 extern CU_TestInfo classification_suite_basic[];
 
diff --git a/test/validation/common/odp_cunit_common.h 
b/test/validation/common/odp_cunit_common.h
index 7c2a9dd..6cafaaa 100644
--- a/test/validation/common/odp_cunit_common.h
+++ b/test/validation/common/odp_cunit_common.h
@@ -21,6 +21,9 @@
 /* the function, called by module main(), to run the testsuites: */
 int odp_cunit_run(CU_SuiteInfo testsuites[]);
 
+/* the macro used to have test names (strings) matching function symbols */
+#define _CU_TEST_INFO(test_func) {#test_func, test_func}
+
 typedef struct {
uint32_t foo;
uint32_t bar;
diff --git a/test/validation/packet/packet.c b/test/validation/packet/packet.c
index 99a6745..97ffd14 100644
--- a/test/validation/packet/packet.c
+++ b/test/validation/packet/packet.c
@@ -10,9 +10,6 @@
 #include odp_cunit_common.h
 #include packet.h
 
-/* Helper macro for CU_TestInfo initialization */
-#define _CU_TEST_INFO(test_func) {#test_func, test_func}
-
 #define PACKET_BUF_LEN ODP_CONFIG_PACKET_SEG_LEN_MIN
 /* Reserve some tailroom for tests */
 #define PACKET_TAILROOM_RESERVE  4
diff --git a/test/validation/pool/pool.c b/test/validation/pool/pool.c
index 9247557..44ba155 100644
--- a/test/validation/pool/pool.c
+++ b/test/validation/pool/pool.c
@@ -99,8 +99,6 @@ void pool_test_lookup_info_print(void)
CU_ASSERT(odp_pool_destroy(pool) == 0);
 }
 
-#define _CU_TEST_INFO(test_func) {#test_func, test_func}
-
 CU_TestInfo pool_suite[] = {
_CU_TEST_INFO(pool_test_create_destroy_buffer),
_CU_TEST_INFO(pool_test_create_destroy_packet),
diff --git a/test/validation/random/random.c b/test/validation/random/random.c
index 039c7a3..b6426f4 100644
--- a/test/validation/random/random.c
+++ b/test/validation/random/random.c
@@ -8,9 +8,6 @@
 #include odp_cunit_common.h
 #include random.h
 
-/* Helper macro for CU_TestInfo initialization */
-#define _CU_TEST_INFO(test_func) {#test_func, test_func}
-
 void random_test_get_size(void)
 {
int32_t ret;
diff --git a/test/validation/thread/thread.c b/test/validation/thread/thread.c
index f95172a..d4f3ee0 100644
--- a/test/validation/thread/thread.c
+++ b/test/validation/thread/thread.c
@@ -10,9 +10,6 @@
 #include test_debug.h
 #include thread.h
 
-/* Helper macro for CU_TestInfo initialization */
-#define _CU_TEST_INFO(test_func) {#test_func, test_func}
-
 /* Test thread entry and exit synchronization barriers */
 odp_barrier_t bar_entry;
 odp_barrier_t bar_exit;
-- 
1.9.1

___
lng-odp mailing 

[lng-odp] [PATCHv2 02/13] validation: cpumask: using _CU_TEST_INFO()

2015-08-20 Thread Christophe Milard
Signed-off-by: Christophe Milard christophe.mil...@linaro.org
Reviewed-by: Stuart Haslam stuart.has...@linaro.org
---
 test/validation/cpumask/cpumask.c | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/test/validation/cpumask/cpumask.c 
b/test/validation/cpumask/cpumask.c
index 029baad..6d57028 100644
--- a/test/validation/cpumask/cpumask.c
+++ b/test/validation/cpumask/cpumask.c
@@ -73,24 +73,24 @@ void cpumask_test_odp_cpumask_def(void)
 }
 
 CU_TestInfo cpumask_suite[] = {
-   {odp_cpumask_to/from_str(), cpumask_test_odp_cpumask_to_from_str},
-   {odp_cpumask_equal(),   cpumask_test_odp_cpumask_equal},
-   {odp_cpumask_zero(),cpumask_test_odp_cpumask_zero},
-   {odp_cpumask_set(), cpumask_test_odp_cpumask_set},
-   {odp_cpumask_clr(), cpumask_test_odp_cpumask_clr},
-   {odp_cpumask_isset(),   cpumask_test_odp_cpumask_isset},
-   {odp_cpumask_count(),   cpumask_test_odp_cpumask_count},
-   {odp_cpumask_and(), cpumask_test_odp_cpumask_and},
-   {odp_cpumask_or(),  cpumask_test_odp_cpumask_or},
-   {odp_cpumask_xor(), cpumask_test_odp_cpumask_xor},
-   {odp_cpumask_copy(),cpumask_test_odp_cpumask_copy},
-   {odp_cpumask_first(),   cpumask_test_odp_cpumask_first},
-   {odp_cpumask_last(),cpumask_test_odp_cpumask_last},
-   {odp_cpumask_next(),cpumask_test_odp_cpumask_next},
-   {odp_cpumask_setall(),  cpumask_test_odp_cpumask_setall},
-   {odp_cpumask_def_control(), cpumask_test_odp_cpumask_def_control},
-   {odp_cpumask_def_worker(),  cpumask_test_odp_cpumask_def_worker},
-   {odp_cpumask_def(), cpumask_test_odp_cpumask_def},
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_to_from_str),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_equal),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_zero),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_set),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_clr),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_isset),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_count),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_and),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_or),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_xor),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_copy),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_first),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_last),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_next),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_setall),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_def_control),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_def_worker),
+   _CU_TEST_INFO(cpumask_test_odp_cpumask_def),
CU_TEST_INFO_NULL,
 };
 
-- 
1.9.1

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


[lng-odp] [PATCHv2 04/13] validation: errno: using _CU_TEST_INFO()

2015-08-20 Thread Christophe Milard
Signed-off-by: Christophe Milard christophe.mil...@linaro.org
Reviewed-by: Stuart Haslam stuart.has...@linaro.org
---
 test/validation/errno/errno.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/validation/errno/errno.c b/test/validation/errno/errno.c
index 9b6b125..c4f4aab 100644
--- a/test/validation/errno/errno.c
+++ b/test/validation/errno/errno.c
@@ -20,7 +20,7 @@ void errno_test_odp_errno_sunny_day(void)
 }
 
 CU_TestInfo errno_suite[] = {
-   {sunny day, errno_test_odp_errno_sunny_day},
+   _CU_TEST_INFO(errno_test_odp_errno_sunny_day),
CU_TEST_INFO_NULL,
 };
 
-- 
1.9.1

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


[lng-odp] [PATCHv2 00/13] validation: use macro _CU_TEST_INFO() in all tests

2015-08-20 Thread Christophe Milard
This is another step in getting the test environment more homogeneous.
The usage of this macro guaranties that the test name (string in C_unit)
will always match the function name.
This is needed as the C_unit strings will be used towards the C_unit API
(to skip a test, for instance).

Christophe Milard (13):
  validation: factorise _CU_TEST_INFO definition
  validation: cpumask: using _CU_TEST_INFO()
  validation: crypto: using _CU_TEST_INFO()
  validation: errno: using _CU_TEST_INFO()
  validation: init: using _CU_TEST_INFO()
  validation: pktio: using _CU_TEST_INFO()
  validation: queue: using _CU_TEST_INFO()
  validation: scheduler: using _CU_TEST_INFO()
  validation: shmem: using _CU_TEST_INFO()
  validation: synchronizers: using _CU_TEST_INFO()
  validation: system: using _CU_TEST_INFO()
  validation: time: using _CU_TEST_INFO()
  validation: timer: using _CU_TEST_INFO()

 test/validation/buffer/buffer.c|  3 --
 .../classification/odp_classification_basic.c  |  1 +
 .../classification/odp_classification_testsuites.h |  3 --
 test/validation/common/odp_cunit_common.h  |  3 ++
 test/validation/cpumask/cpumask.c  | 36 ++---
 test/validation/crypto/odp_crypto_test_inp.c   | 18 +++
 test/validation/errno/errno.c  |  2 +-
 test/validation/init/init.c|  6 +--
 test/validation/packet/packet.c|  3 --
 test/validation/pktio/pktio.c  | 24 -
 test/validation/pool/pool.c|  2 -
 test/validation/queue/queue.c  |  2 +-
 test/validation/random/random.c|  3 --
 test/validation/scheduler/scheduler.c  | 60 +++---
 test/validation/shmem/shmem.c  |  2 +-
 test/validation/synchronizers/synchronizers.c  | 26 +-
 test/validation/system/system.c| 14 ++---
 test/validation/thread/thread.c|  3 --
 test/validation/time/time.c|  6 +--
 test/validation/timer/timer.c  |  8 +--
 20 files changed, 103 insertions(+), 122 deletions(-)

-- 
1.9.1

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


[lng-odp] [PATCHv2 05/13] validation: init: using _CU_TEST_INFO()

2015-08-20 Thread Christophe Milard
Signed-off-by: Christophe Milard christophe.mil...@linaro.org
Reviewed-by: Stuart Haslam stuart.has...@linaro.org
---
 test/validation/init/init.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/validation/init/init.c b/test/validation/init/init.c
index 12aa554..ee4483c 100644
--- a/test/validation/init/init.c
+++ b/test/validation/init/init.c
@@ -38,7 +38,7 @@ void init_test_odp_init_global_replace_abort(void)
 }
 
 CU_TestInfo init_suite_abort[] = {
-   {replace abort,  init_test_odp_init_global_replace_abort},
+   _CU_TEST_INFO(init_test_odp_init_global_replace_abort),
CU_TEST_INFO_NULL,
 };
 
@@ -83,7 +83,7 @@ void init_test_odp_init_global_replace_log(void)
 }
 
 CU_TestInfo init_suite_log[] = {
-   {replace log,  init_test_odp_init_global_replace_log},
+   _CU_TEST_INFO(init_test_odp_init_global_replace_log),
CU_TEST_INFO_NULL,
 };
 
@@ -131,7 +131,7 @@ void init_test_odp_init_global(void)
 }
 
 CU_TestInfo init_suite_ok[] = {
-   {test_odp_init_global,  init_test_odp_init_global},
+   _CU_TEST_INFO(init_test_odp_init_global),
CU_TEST_INFO_NULL,
 };
 
-- 
1.9.1

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


[lng-odp] [PATCHv2 10/13] validation: synchronizers: using _CU_TEST_INFO()

2015-08-20 Thread Christophe Milard
Signed-off-by: Christophe Milard christophe.mil...@linaro.org
Reviewed-by: Stuart Haslam stuart.has...@linaro.org
---
 test/validation/synchronizers/synchronizers.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/test/validation/synchronizers/synchronizers.c 
b/test/validation/synchronizers/synchronizers.c
index 7c521ca..0a31a40 100644
--- a/test/validation/synchronizers/synchronizers.c
+++ b/test/validation/synchronizers/synchronizers.c
@@ -941,8 +941,8 @@ void synchronizers_test_barrier_functional(void)
 }
 
 CU_TestInfo synchronizers_suite_barrier[] = {
-   {no_barrier_functional, synchronizers_test_no_barrier_functional},
-   {barrier_functional, synchronizers_test_barrier_functional},
+   _CU_TEST_INFO(synchronizers_test_no_barrier_functional),
+   _CU_TEST_INFO(synchronizers_test_barrier_functional),
CU_TEST_INFO_NULL
 };
 
@@ -957,7 +957,7 @@ void synchronizers_test_no_lock_functional(void)
 }
 
 CU_TestInfo synchronizers_suite_no_locking[] = {
-   {no_lock_functional, synchronizers_test_no_lock_functional},
+   _CU_TEST_INFO(synchronizers_test_no_lock_functional),
CU_TEST_INFO_NULL
 };
 
@@ -982,8 +982,8 @@ void synchronizers_test_spinlock_functional(void)
 }
 
 CU_TestInfo synchronizers_suite_spinlock[] = {
-   {spinlock_api, synchronizers_test_spinlock_api},
-   {spinlock_functional, synchronizers_test_spinlock_functional},
+   _CU_TEST_INFO(synchronizers_test_spinlock_api),
+   _CU_TEST_INFO(synchronizers_test_spinlock_functional),
CU_TEST_INFO_NULL
 };
 
@@ -1009,8 +1009,8 @@ void synchronizers_test_ticketlock_functional(void)
 }
 
 CU_TestInfo synchronizers_suite_ticketlock[] = {
-   {ticketlock_api, synchronizers_test_ticketlock_api},
-   {ticketlock_functional, synchronizers_test_ticketlock_functional},
+   _CU_TEST_INFO(synchronizers_test_ticketlock_api),
+   _CU_TEST_INFO(synchronizers_test_ticketlock_functional),
CU_TEST_INFO_NULL
 };
 
@@ -1035,8 +1035,8 @@ void synchronizers_test_rwlock_functional(void)
 }
 
 CU_TestInfo synchronizers_suite_rwlock[] = {
-   {rwlock_api, synchronizers_test_rwlock_api},
-   {rwlock_functional, synchronizers_test_rwlock_functional},
+   _CU_TEST_INFO(synchronizers_test_rwlock_api),
+   _CU_TEST_INFO(synchronizers_test_rwlock_functional),
CU_TEST_INFO_NULL
 };
 
@@ -1187,10 +1187,10 @@ void synchronizers_test_atomic_fetch_add_sub(void)
 }
 
 CU_TestInfo synchronizers_suite_atomic[] = {
-   {atomic_inc_dec, synchronizers_test_atomic_inc_dec},
-   {atomic_add_sub, synchronizers_test_atomic_add_sub},
-   {atomic_fetch_inc_dec, synchronizers_test_atomic_fetch_inc_dec},
-   {atomic_fetch_add_sub, synchronizers_test_atomic_fetch_add_sub},
+   _CU_TEST_INFO(synchronizers_test_atomic_inc_dec),
+   _CU_TEST_INFO(synchronizers_test_atomic_add_sub),
+   _CU_TEST_INFO(synchronizers_test_atomic_fetch_inc_dec),
+   _CU_TEST_INFO(synchronizers_test_atomic_fetch_add_sub),
CU_TEST_INFO_NULL,
 };
 
-- 
1.9.1

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


[lng-odp] [PATCHv2 09/13] validation: shmem: using _CU_TEST_INFO()

2015-08-20 Thread Christophe Milard
Signed-off-by: Christophe Milard christophe.mil...@linaro.org
Reviewed-by: Stuart Haslam stuart.has...@linaro.org
---
 test/validation/shmem/shmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/validation/shmem/shmem.c b/test/validation/shmem/shmem.c
index 3abba0c..6dc579a 100644
--- a/test/validation/shmem/shmem.c
+++ b/test/validation/shmem/shmem.c
@@ -77,7 +77,7 @@ void shmem_test_odp_shm_sunnyday(void)
 }
 
 CU_TestInfo shmem_suite[] = {
-   {test_odp_shm_creat,  shmem_test_odp_shm_sunnyday},
+   _CU_TEST_INFO(shmem_test_odp_shm_sunnyday),
CU_TEST_INFO_NULL,
 };
 
-- 
1.9.1

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


[lng-odp] [PATCHv2 03/13] validation: crypto: using _CU_TEST_INFO()

2015-08-20 Thread Christophe Milard
Signed-off-by: Christophe Milard christophe.mil...@linaro.org
Reviewed-by: Stuart Haslam stuart.has...@linaro.org
---
 test/validation/crypto/odp_crypto_test_inp.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/test/validation/crypto/odp_crypto_test_inp.c 
b/test/validation/crypto/odp_crypto_test_inp.c
index 90fe8d0..187a04c 100644
--- a/test/validation/crypto/odp_crypto_test_inp.c
+++ b/test/validation/crypto/odp_crypto_test_inp.c
@@ -6,6 +6,7 @@
 
 #include odp.h
 #include CUnit/Basic.h
+#include odp_cunit_common.h
 #include test_vectors.h
 #include odp_crypto_test_inp.h
 #include crypto.h
@@ -132,7 +133,6 @@ cleanup:
  * operation for 3DES_CBC algorithm. IV for the operation is the session IV.
  * In addition the test verifies if the implementation can use the
  * packet buffer as completion event buffer.*/
-#define ASYNC_INP_ENC_ALG_3DES_CBC ENC_ALG_3DES_CBC
 void crypto_test_enc_alg_3des_cbc(void)
 {
odp_crypto_key_t cipher_key = { .data = NULL, .length = 0 },
@@ -165,7 +165,6 @@ void crypto_test_enc_alg_3des_cbc(void)
 /* This test verifies the correctness of encode (plaintext - ciphertext)
  * operation for 3DES_CBC algorithm. IV for the operation is the operation IV.
  * */
-#define ASYNC_INP_ENC_ALG_3DES_CBC_OVR_IV  ENC_ALG_3DES_CBC_OVR_IV
 void crypto_test_enc_alg_3des_cbc_ovr_iv(void)
 {
odp_crypto_key_t cipher_key = { .data = NULL, .length = 0 },
@@ -199,7 +198,6 @@ void crypto_test_enc_alg_3des_cbc_ovr_iv(void)
  * In addition the test verifies if the implementation can use the
  * packet buffer as completion event buffer.
  * */
-#define ASYNC_INP_DEC_ALG_3DES_CBC DEC_ALG_3DES_CBC
 void crypto_test_dec_alg_3des_cbc(void)
 {
odp_crypto_key_t cipher_key = { .data = NULL, .length = 0 },
@@ -234,7 +232,6 @@ void crypto_test_dec_alg_3des_cbc(void)
  * In addition the test verifies if the implementation can use the
  * packet buffer as completion event buffer.
  * */
-#define ASYNC_INP_DEC_ALG_3DES_CBC_OVR_IV  DEC_ALG_3DES_CBC_OVR_IV
 void crypto_test_dec_alg_3des_cbc_ovr_iv(void)
 {
odp_crypto_key_t cipher_key = { .data = NULL, .length = 0 },
@@ -270,7 +267,6 @@ void crypto_test_dec_alg_3des_cbc_ovr_iv(void)
  * In addition the test verifies if the implementation can use the
  * packet buffer as completion event buffer.
  * */
-#define ASYNC_INP_ALG_HMAC_MD5 ALG_HMAC_MD5
 void crypto_test_alg_hmac_md5(void)
 {
odp_crypto_key_t cipher_key = { .data = NULL, .length = 0 },
@@ -324,12 +320,10 @@ int crypto_suite_async_init(void)
 }
 
 CU_TestInfo crypto_suite[] = {
-   {ASYNC_INP_ENC_ALG_3DES_CBC, crypto_test_enc_alg_3des_cbc },
-   {ASYNC_INP_DEC_ALG_3DES_CBC, crypto_test_dec_alg_3des_cbc },
-   {ASYNC_INP_ENC_ALG_3DES_CBC_OVR_IV,
-   crypto_test_enc_alg_3des_cbc_ovr_iv },
-   {ASYNC_INP_DEC_ALG_3DES_CBC_OVR_IV,
-   crypto_test_dec_alg_3des_cbc_ovr_iv },
-   {ASYNC_INP_ALG_HMAC_MD5, crypto_test_alg_hmac_md5 },
+   _CU_TEST_INFO(crypto_test_enc_alg_3des_cbc),
+   _CU_TEST_INFO(crypto_test_dec_alg_3des_cbc),
+   _CU_TEST_INFO(crypto_test_enc_alg_3des_cbc_ovr_iv),
+   _CU_TEST_INFO(crypto_test_dec_alg_3des_cbc_ovr_iv),
+   _CU_TEST_INFO(crypto_test_alg_hmac_md5),
CU_TEST_INFO_NULL,
 };
-- 
1.9.1

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


[lng-odp] [PATCHv2 13/13] validation: timer: using _CU_TEST_INFO()

2015-08-20 Thread Christophe Milard
Signed-off-by: Christophe Milard christophe.mil...@linaro.org
Reviewed-by: Stuart Haslam stuart.has...@linaro.org
---
 test/validation/timer/timer.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/validation/timer/timer.c b/test/validation/timer/timer.c
index 30b2b43..2d04090 100644
--- a/test/validation/timer/timer.c
+++ b/test/validation/timer/timer.c
@@ -530,10 +530,10 @@ void timer_test_odp_timer_all(void)
 }
 
 CU_TestInfo timer_suite[] = {
-   {test_timeout_pool_alloc,  timer_test_timeout_pool_alloc},
-   {test_timeout_pool_free,  timer_test_timeout_pool_free},
-   {test_odp_timer_cancel,  timer_test_odp_timer_cancel},
-   {test_odp_timer_all,  timer_test_odp_timer_all},
+   _CU_TEST_INFO(timer_test_timeout_pool_alloc),
+   _CU_TEST_INFO(timer_test_timeout_pool_free),
+   _CU_TEST_INFO(timer_test_odp_timer_cancel),
+   _CU_TEST_INFO(timer_test_odp_timer_all),
CU_TEST_INFO_NULL,
 };
 
-- 
1.9.1

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


[lng-odp] [PATCHv2 11/13] validation: system: using _CU_TEST_INFO()

2015-08-20 Thread Christophe Milard
Signed-off-by: Christophe Milard christophe.mil...@linaro.org
Reviewed-by: Stuart Haslam stuart.has...@linaro.org
---
 test/validation/system/system.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/test/validation/system/system.c b/test/validation/system/system.c
index 7687f20..5f03aa1 100644
--- a/test/validation/system/system.c
+++ b/test/validation/system/system.c
@@ -84,13 +84,13 @@ void system_test_odp_sys_cpu_hz(void)
 }
 
 CU_TestInfo system_suite[] = {
-   {odp version,  system_test_odp_version_numbers},
-   {odp_cpu_count,  system_test_odp_cpu_count},
-   {odp_sys_cache_line_size,  system_test_odp_sys_cache_line_size},
-   {odp_sys_cpu_model_str,  system_test_odp_sys_cpu_model_str},
-   {odp_sys_page_size,  system_test_odp_sys_page_size},
-   {odp_sys_huge_page_size,  system_test_odp_sys_huge_page_size},
-   {odp_sys_cpu_hz,  system_test_odp_sys_cpu_hz},
+   _CU_TEST_INFO(system_test_odp_version_numbers),
+   _CU_TEST_INFO(system_test_odp_cpu_count),
+   _CU_TEST_INFO(system_test_odp_sys_cache_line_size),
+   _CU_TEST_INFO(system_test_odp_sys_cpu_model_str),
+   _CU_TEST_INFO(system_test_odp_sys_page_size),
+   _CU_TEST_INFO(system_test_odp_sys_huge_page_size),
+   _CU_TEST_INFO(system_test_odp_sys_cpu_hz),
CU_TEST_INFO_NULL,
 };
 
-- 
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 v3 1/3] api: packet: allow access to packet RSS hash values

2015-08-20 Thread Jerin Jacob
On Thu, Aug 20, 2015 at 04:52:29PM +0530, Santosh Shukla wrote:
 On 20 August 2015 at 16:23, Jerin Jacob jerin.ja...@caviumnetworks.com 
 wrote:
  On Thu, Aug 20, 2015 at 03:30:23PM +0530, Santosh Shukla wrote:
  On 20 August 2015 at 14:48, Balasubramanian Manoharan
  bala.manoha...@linaro.org wrote:
  
  
   On Sunday 16 August 2015 06:14 PM, Santosh Shukla wrote:
  
   On 15 August 2015 at 00:12, Zoltan Kiss zoltan.k...@linaro.org wrote:
  
   Applications can read the computed hash (if any) and set it if they
   changed the packet headers or if the platform haven't calculated the
   hash.
  
   Signed-off-by: Zoltan Kiss zoltan.k...@linaro.org
   ---
   v2:
   - focus on RSS hash only
   - use setter/getter's
  
   v3:
   - do not mention pointers
   - add a note
   - add new patches for implementation and test
  
 include/odp/api/packet.h | 30 ++
 1 file changed, 30 insertions(+)
  
   diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
   index 3a454b5..1ae24bc 100644
   --- a/include/odp/api/packet.h
   +++ b/include/odp/api/packet.h
   @@ -48,6 +48,11 @@ extern C {
  * Invalid packet segment
  */
  
   +/**
   + * @def ODP_PACKET_RSS_INVALID
   + * RSS hash is not set
   + */
   +
 /*
  *
  * Alloc and free
   @@ -605,6 +610,31 @@ uint32_t odp_packet_l4_offset(odp_packet_t pkt);
 int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset);
  
 /**
   + * RSS hash value
   + *
   + * Returns the RSS hash stored for the packet.
   + *
   + * @param  pkt  Packet handle
   + *
   + * @return  Hash value
   + * @retval ODP_PACKET_RSS_INVALID if RSS hash is not set.
   + */
   +uint32_t odp_packet_rss_hash(odp_packet_t pkt);
   +
   +/**
   + * Set RSS hash value
   + *
   + * Store the RSS hash for the packet.
   + *
   + * @param  pkt  Packet handle
   + * @param  rss_hash Hash value to set
   + *
   + * @note If the application changes the packet header, it might want 
   to
   + * recalculate this value and set it.
   + */
   +void odp_packet_rss_hash_set(odp_packet_t pkt, uint32_t rss_hash);
  
   Can this use-case be handled by calling
   odp_packet_generate_rss_hash(odp_packet_t pkt) function where the rss 
   gets
   generated by the implementation rather than being set from application 
   using
   odp_packet_set_rss_hash() function?
  
 
  Bala, As we discussed and in summary for rest; Considering ovs example
  : ovs uses sw based rss_hash only when HW not supporting rss Or HW
  generated rss is zero.
 
  hash = packet_get_hash(packet);
  if (OVS_UNLIKELY(!hash)) {
  hash = miniflow_hash_5tuple(mf, 0);
  packet_set_hash(packet, hash);
  }
  return hash;
 
  and rss_hash generation is inside ovs dp_packet (middle  layer), It is
 
  How about following change in OVS plarform specific packet_get_hash code.
  odp_packet_rss_hash_set
  kind of interface wont fit correctly in octeon  platform as hash
  identifier used by hardware in subsequent HW based queue operations
 
  static inline uint32_t
  packet_get_hash(struct dp_packet *p)
  {
  #ifdef DPDK_NETDEV
  return p-mbuf.hash.rss;
  #else
  #ifdef ODP_NETDEV
  unit32_t hash;
  hash = odp_packet_rss_hash(p-odp_pkt);
  if (OVS_UNLIKELY(!hash)
  hash = odp_packet_generate_rss_hash(p-odp_pkt);
  return hash;
  #endif
  return p-rss_hash;
  #endif
  }
 
 
 Trying to understand your api definition;
 odp_packet_rss_hash() - to get rss
 and
 odp_packet_generate_rss_hash() - generate rss from where :  sw or hw?

NOP if HW is capable, for software generated packet/ non-capable HW it
will be in SW

 In either case, your trying to generate non-zero rss (considering a
 valid rss (?) for your platform).

IMO, The term RSS may not be correct in API definition. May be something
like flow id, make sense across all platform.

Jerin

 
 - IIUC _generate_rss_hash() able to take corrective measure so you
 won't find need for using rss_hash_set() right?
 - In other word, No s/w based rss __setting__ required for octeon.
 
 did I understood all that right?
 
 
  with in ovs implementation, Not exposed to application layer, so
  application won't need to generate rss / update, so no possibility of
  rss mis-match /corruption.
 
   Regards,
   Bala
  
   +
   +/**
  * Tests if packet is segmented
  *
  * @param pkt  Packet handle
   --
  
   Reviewed-by : Santosh Shukla santosh.shu...@linaro.org
  
   1.9.1
  
   ___
   lng-odp mailing list
   lng-odp@lists.linaro.org
   https://lists.linaro.org/mailman/listinfo/lng-odp
  
   ___
   lng-odp mailing list
   lng-odp@lists.linaro.org
   https://lists.linaro.org/mailman/listinfo/lng-odp
  
  
  ___
  lng-odp mailing list
  lng-odp@lists.linaro.org
  https://lists.linaro.org/mailman/listinfo/lng-odp

[lng-odp] [PATCHv2 07/13] validation: queue: using _CU_TEST_INFO()

2015-08-20 Thread Christophe Milard
Signed-off-by: Christophe Milard christophe.mil...@linaro.org
Reviewed-by: Stuart Haslam stuart.has...@linaro.org
---
 test/validation/queue/queue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/validation/queue/queue.c b/test/validation/queue/queue.c
index 5b2a13a..74a730d 100644
--- a/test/validation/queue/queue.c
+++ b/test/validation/queue/queue.c
@@ -115,7 +115,7 @@ void queue_test_sunnydays(void)
 }
 
 CU_TestInfo queue_suite[] = {
-   {queue sunnyday,  queue_test_sunnydays},
+   _CU_TEST_INFO(queue_test_sunnydays),
CU_TEST_INFO_NULL,
 };
 
-- 
1.9.1

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


[lng-odp] [PATCHv2 06/13] validation: pktio: using _CU_TEST_INFO()

2015-08-20 Thread Christophe Milard
Signed-off-by: Christophe Milard christophe.mil...@linaro.org
Reviewed-by: Stuart Haslam stuart.has...@linaro.org
---
 test/validation/pktio/pktio.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
index be4f940..cb2a9ed 100644
--- a/test/validation/pktio/pktio.c
+++ b/test/validation/pktio/pktio.c
@@ -729,18 +729,18 @@ int pktio_suite_term(void)
 }
 
 CU_TestInfo pktio_suite_unsegmented[] = {
-   {pktio open,  pktio_test_open},
-   {pktio lookup,pktio_test_lookup},
-   {pktio inq,   pktio_test_inq},
-   {pktio poll queues,   pktio_test_poll_queue},
-   {pktio poll multi,pktio_test_poll_multi},
-   {pktio sched queues,  pktio_test_sched_queue},
-   {pktio sched multi,   pktio_test_sched_multi},
-   {pktio jumbo frames,  pktio_test_jumbo},
-   {pktio mtu,   pktio_test_mtu},
-   {pktio promisc mode,  pktio_test_promisc},
-   {pktio mac,   pktio_test_mac},
-   {pktio inq_remdef,pktio_test_inq_remdef},
+   _CU_TEST_INFO(pktio_test_open),
+   _CU_TEST_INFO(pktio_test_lookup),
+   _CU_TEST_INFO(pktio_test_inq),
+   _CU_TEST_INFO(pktio_test_poll_queue),
+   _CU_TEST_INFO(pktio_test_poll_multi),
+   _CU_TEST_INFO(pktio_test_sched_queue),
+   _CU_TEST_INFO(pktio_test_sched_multi),
+   _CU_TEST_INFO(pktio_test_jumbo),
+   _CU_TEST_INFO(pktio_test_mtu),
+   _CU_TEST_INFO(pktio_test_promisc),
+   _CU_TEST_INFO(pktio_test_mac),
+   _CU_TEST_INFO(pktio_test_inq_remdef),
CU_TEST_INFO_NULL
 };
 
-- 
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 PATCHv2 1/3] api: classification: add ODP_PMR_CUSTOM_FRAME

2015-08-20 Thread Balasubramanian Manoharan

Hi,

On Thursday 20 August 2015 04:07 PM, Benoît Ganne wrote:

Hi Bala,

Thanks for your feedback!


odp_pmr_match_t structure was already defined and this patch just adds
offset variable so why don't we just add the additional variable in
the existing structure and maybe move the odp_pmr_create() function
below this structure if that was the concern.
It narrows down the changes done by this patch and will be easier to
understand the changes required for adding the term PMR_CUSTOM_FRAME.

I disagree, for 2 reasons:
   1) move odp_create would in my opinion complicate code reads  by human:
I usually expect the function to be grouped together, ie finding
odp_create just above odp_destroy
IMO, moving the functions for human readability could be done as a 
separate patch.

   2) I tried what you suggests and the diffstat is identical (see below)
So it seems to me that it does not reduce the patch size and in the
contrary the header will be harder to read.
In the new diffstat it is clear that you have added an additional 
variable to the existing odp_pmr_match_t structure which I think is better
as it clearly states that your requirement of supporting 
PMR_CUSTOM_FRAME needs an additional variable in the existing structure 
and that it did

not create a new structure altogether.

Regards,
Bala


Here's the new diff:
  include/odp/api/classification.h | 55 
  1 file changed, 27 insertions(+), 28 deletions(-)

diff --git a/include/odp/api/classification.h b/include/odp/api/classification.h
index f597b26..b63a6c8 100644
--- a/include/odp/api/classification.h
+++ b/include/odp/api/classification.h
@@ -215,31 +215,17 @@ typedef enum odp_pmr_term {
ODP_PMR_IPSEC_SPI,  /** IPsec session identifier(*val=uint32_t)*/
ODP_PMR_LD_VNI, /** NVGRE/VXLAN network identifier
(*val=uint32_t)*/
+   ODP_PMR_CUSTOM_FRAME,   /** Custom match rule, offset from start of
+   frame. The match is defined by the offset, the
+   expected value, and its size. They must be
+   applied before any other PMR.
+   (*val=uint8_t[val_sz])*/
  
  	/** Inner header may repeat above values with this offset */

ODP_PMR_INNER_HDR_OFF = 32
  } odp_pmr_term_e;
  
  /**

- * Create a packet match rule with mask and value
- *
- * @param[in]  termOne of the enumerated values supported
- * @param[in]  val Value to match against the packet header
- * in native byte order.
- * @param[in]  maskMask to indicate which bits of the header
- * should be matched ('1') and
- * which should be ignored ('0')
- * @param[in]  val_sz  Size of the val and mask arguments,
- * that must match the value size requirement of the
- * specific term.
- *
- * @return Handle of the matching rule
- * @retval ODP_PMR_INVAL on failure
- */
-odp_pmr_t odp_pmr_create(odp_pmr_term_e term, const void *val,
-const void *mask, uint32_t val_sz);
-
-/**
   * Invalidate a packet match rule and vacate its resources
   *
   * @param[in] pmr_id  Identifier of the PMR to be destroyed
@@ -290,27 +276,40 @@ unsigned long long odp_pmr_terms_cap(void);
  unsigned odp_pmr_terms_avail(void);
  
  /**

- * Following structure is used to define composite packet matching rules
- * in the form of an array of individual match rules.
- * The underlying platform may not support all or any specific combination
- * of value match rules, and the application should take care
- * of inspecting the return value when installing such rules, and perform
- * appropriate fallback action.
+ * Following structure is used to define a packet matching rule
   */
  typedef struct odp_pmr_match_t {
odp_pmr_term_e  term;   /** PMR term value to be matched */
const void  *val;   /** Value to be matched */
const void  *mask;  /** Masked set of bits to be matched */
-   unsigned intval_sz;  /** Size of the term value */
+   uint32_tval_sz;  /** Size of the term value */
+   uint32_toffset;  /** User-defined offset in packet
+Used if term == ODP_PMR_CUSTOM_FRAME only,
+otherwise must be 0 */
  } odp_pmr_match_t;
  
  /**

+ * Create a packet match rule with mask and value
+ *
+ * @param[in]  match   packet matching rule definition
+ *
+ * @return Handle of the matching rule
+ * @retval ODP_PMR_INVAL on failure
+ */
+odp_pmr_t odp_pmr_create(const odp_pmr_match_t *match);
+
+/**
   * @typedef odp_pmr_set_t
   * An opaque handle to a composite packet match rule-set
   */
  
  /**

- * Create a composite packet match rule
+ * Create a composite packet match rule in the form of an array of individual
+ * match 

Re: [lng-odp] [API-NEXT PATCH v3 1/3] api: packet: allow access to packet RSS hash values

2015-08-20 Thread Jerin Jacob
On Thu, Aug 20, 2015 at 03:30:23PM +0530, Santosh Shukla wrote:
 On 20 August 2015 at 14:48, Balasubramanian Manoharan
 bala.manoha...@linaro.org wrote:
 
 
  On Sunday 16 August 2015 06:14 PM, Santosh Shukla wrote:
 
  On 15 August 2015 at 00:12, Zoltan Kiss zoltan.k...@linaro.org wrote:
 
  Applications can read the computed hash (if any) and set it if they
  changed the packet headers or if the platform haven't calculated the
  hash.
 
  Signed-off-by: Zoltan Kiss zoltan.k...@linaro.org
  ---
  v2:
  - focus on RSS hash only
  - use setter/getter's
 
  v3:
  - do not mention pointers
  - add a note
  - add new patches for implementation and test
 
include/odp/api/packet.h | 30 ++
1 file changed, 30 insertions(+)
 
  diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
  index 3a454b5..1ae24bc 100644
  --- a/include/odp/api/packet.h
  +++ b/include/odp/api/packet.h
  @@ -48,6 +48,11 @@ extern C {
 * Invalid packet segment
 */
 
  +/**
  + * @def ODP_PACKET_RSS_INVALID
  + * RSS hash is not set
  + */
  +
/*
 *
 * Alloc and free
  @@ -605,6 +610,31 @@ uint32_t odp_packet_l4_offset(odp_packet_t pkt);
int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset);
 
/**
  + * RSS hash value
  + *
  + * Returns the RSS hash stored for the packet.
  + *
  + * @param  pkt  Packet handle
  + *
  + * @return  Hash value
  + * @retval ODP_PACKET_RSS_INVALID if RSS hash is not set.
  + */
  +uint32_t odp_packet_rss_hash(odp_packet_t pkt);
  +
  +/**
  + * Set RSS hash value
  + *
  + * Store the RSS hash for the packet.
  + *
  + * @param  pkt  Packet handle
  + * @param  rss_hash Hash value to set
  + *
  + * @note If the application changes the packet header, it might want to
  + * recalculate this value and set it.
  + */
  +void odp_packet_rss_hash_set(odp_packet_t pkt, uint32_t rss_hash);
 
  Can this use-case be handled by calling
  odp_packet_generate_rss_hash(odp_packet_t pkt) function where the rss gets
  generated by the implementation rather than being set from application using
  odp_packet_set_rss_hash() function?
 
 
 Bala, As we discussed and in summary for rest; Considering ovs example
 : ovs uses sw based rss_hash only when HW not supporting rss Or HW
 generated rss is zero.
 
 hash = packet_get_hash(packet);
 if (OVS_UNLIKELY(!hash)) {
 hash = miniflow_hash_5tuple(mf, 0);
 packet_set_hash(packet, hash);
 }
 return hash;
 
 and rss_hash generation is inside ovs dp_packet (middle  layer), It is

How about following change in OVS plarform specific packet_get_hash code.
odp_packet_rss_hash_set
kind of interface wont fit correctly in octeon  platform as hash
identifier used by hardware in subsequent HW based queue operations

static inline uint32_t
packet_get_hash(struct dp_packet *p)
{
#ifdef DPDK_NETDEV
return p-mbuf.hash.rss;
#else
#ifdef ODP_NETDEV
unit32_t hash;
hash = odp_packet_rss_hash(p-odp_pkt);
if (OVS_UNLIKELY(!hash)
hash = odp_packet_generate_rss_hash(p-odp_pkt);
return hash;
#endif
return p-rss_hash;
#endif
}


 with in ovs implementation, Not exposed to application layer, so
 application won't need to generate rss / update, so no possibility of
 rss mis-match /corruption.
 
  Regards,
  Bala
 
  +
  +/**
 * Tests if packet is segmented
 *
 * @param pkt  Packet handle
  --
 
  Reviewed-by : Santosh Shukla santosh.shu...@linaro.org
 
  1.9.1
 
  ___
  lng-odp mailing list
  lng-odp@lists.linaro.org
  https://lists.linaro.org/mailman/listinfo/lng-odp
 
  ___
  lng-odp mailing list
  lng-odp@lists.linaro.org
  https://lists.linaro.org/mailman/listinfo/lng-odp
 
 
 ___
 lng-odp mailing list
 lng-odp@lists.linaro.org
 https://lists.linaro.org/mailman/listinfo/lng-odp
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


[lng-odp] [API-NEXT PATCHv3 0/4] ODP_PMR_CUSTOM_FRAME support

2015-08-20 Thread Benoît Ganne

v3:  - split the api patch in two: 1st one to modify api, 2nd one
   to move declarations for readability

v2:  - rename ODP_PMR_OFFSET_ABS to ODP_PMR_CUSTOM_FRAME
 - better doc (including the fact that ODP_PMR_CUSTOM_FRAME
   should be applied before other PMR)
 - reuse odp_pmr_match_t for both single PMR and PMR sets
   creation

Benoît Ganne (4):
  api: classification: add ODP_PMR_CUSTOM_FRAME
  api: classification: add ODP_PMR_CUSTOM_FRAME
  linux-generic: classification: implement ODP_PMR_CUSTOM_FRAME matching
  example: classifier: implement ODP_PMR_CUSTOM_FRAME match

 example/classifier/odp_classifier.c| 95 
+-

 include/odp/api/classification.h   | 55 ++---
 .../include/odp_classification_datamodel.h |  2 +
 .../include/odp_classification_inlines.h   | 21 +
 platform/linux-generic/odp_classification.c| 42 +-
 5 files changed, 149 insertions(+), 66 deletions(-)

--
2.5.0

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


[lng-odp] [PATCHv2] linux-generic: pktio: always test loop interface

2015-08-20 Thread Stuart Haslam
Test using the loop interface even when running as root.

Fixes bug; https://bugs.linaro.org/show_bug.cgi?id=1735

Signed-off-by: Stuart Haslam stuart.has...@linaro.org
---
Change in v2 - updated comment about default interfaces

 platform/linux-generic/test/pktio/pktio_run | 25 -
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/platform/linux-generic/test/pktio/pktio_run 
b/platform/linux-generic/test/pktio/pktio_run
index 76a8419..4860455 100755
--- a/platform/linux-generic/test/pktio/pktio_run
+++ b/platform/linux-generic/test/pktio/pktio_run
@@ -46,9 +46,8 @@ run_test()
 {
local ret=0
 
-   # the linux-generic implementation uses environment variables to
-   # control which socket method is used, so try each combination to
-   # ensure decent coverage.
+   # environment variables are used to control which socket method is
+   # used, so try each combination to ensure decent coverage.
for distype in MMAP MMSG; do
unset ODP_PKTIO_DISABLE_SOCKET_${distype}
done
@@ -67,26 +66,34 @@ run_test()
echo !!! FAILED !!!
fi
 
-   exit $ret
+   return $ret
 }
 
 run()
 {
-   #need to be root to set the interface: if not, run with default 
loopback.
+   echo pktio: using 'loop' device
+   pktio_main${EXEEXT}
+   loop_ret=$?
+
+   # need to be root to run tests with real interfaces
if [ $(id -u) != 0 ]; then
-   echo pktio: using 'loop' device
-   pktio_main${EXEEXT}
-   exit $?
+   exit $ret
fi
 
if [ $ODP_PKTIO_IF0 =  ]; then
-   # no interfaces specified on linux-generic, use defaults
+   # no interfaces specified, use default veth interfaces
+   # setup by the pktio_env script
setup_pktio_env clean
export ODP_PKTIO_IF0=$IF0
export ODP_PKTIO_IF1=$IF1
fi
 
run_test
+   ret=$?
+
+   [ $ret = 0 ]  ret=$loop_ret
+
+   exit $ret
 }
 
 case $1 in
-- 
2.1.1

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


Re: [lng-odp] [PATCHv4] example:generator:move verbose from worker to control

2015-08-20 Thread Stuart Haslam
On Thu, Aug 20, 2015 at 01:39:29PM +0200, Krishna Garapati wrote:
 
 
   +static void print_global_stats(int num_workers)
   +{
   + uint64_t start, now, diff;
   + uint64_t pkts, pkts_prev = 0, pps, maximum_pps = 0;
   + int verbose_interval = 20, worker_count;
   + odp_thrmask_t thrd_mask;
   +
   + start = odp_time_cycles();
   + while (1) {
   + now = odp_time_cycles();
   + diff = odp_time_diff_cycles(start, now);
   + if (odp_time_cycles_to_ns(diff) 
   +verbose_interval * ODP_TIME_SEC) {
   + continue;
   + }
 
  There's no exit condition check in this loop above, so you'll wait up to
  verbose_interval extra seconds unnecessarily, for example when running
  with -n 100 the workers will finish quickly.
 
  +
   + start = odp_time_cycles();
   +
   + worker_count = odp_thrmask_worker(thrd_mask);
 
  There's a potential race here as the worker thread counts get
  incremented after the thread has started (rather than during the
  odph_linux_pthread_create() call), and it's pretty likely this code
  will be reached by the main thread before the workers have started.
 
  Should wait until worker count has reached num_workers before entering
  the loop.
 
 
 
  verbose_interval * ODP_TIME_SEC internally a sleep time before reaching
 this check. Even if we introduce the worker_count == num_workers, we might
 have to do a default sleep anyway to make sure we have workers up and
 running otherwise we break the loop. do we still need a separate check for
 this ?
 

Sleeps/delays are generally a bad way to resolve race conditions, since
the race is still there but you're just avoiding it. If at some point in
the future we were to make verbose_interval configurable then it may be
exposed again. It's better to have an explicit check that the precondition
(workers started) has been met. Also as I said in the previous comment
the existing 20 second delay before entering the rest of the loop causes
other problems.

I was thinking of something like;

while (odp_thrmask_workers(thr_mask)  num_workers) { }

while (odp_thrmask_workers(thr_mask) == num_workers)
{
..
}

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


Re: [lng-odp] [API-NEXT PATCH v3 1/3] api: packet: allow access to packet RSS hash values

2015-08-20 Thread Zoltan Kiss



On 20/08/15 12:43, Jerin Jacob wrote:

IMO, The term RSS may not be correct in API definition. May be something
like flow id, make sense across all platform.


Good idea. I'll replace rss with flow instead.
___
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp


Re: [lng-odp] [API-NEXT PATCH v2] api: packet: allow access to packet RSS hash values

2015-08-20 Thread Bill Fischofer
An RSS hash is not a flow identifier.  NICs produce RSS hashes so it makes
sense to be honest about that.

On Thursday, August 20, 2015, Zoltan Kiss zoltan.k...@linaro.org wrote:



 On 20/08/15 12:43, Jerin Jacob wrote:

 IMO, The term RSS may not be correct in API definition. May be something
 like flow id, make sense across all platform.


 Good idea. I'll replace rss with flow instead.
 ___
 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 v3 1/3] api: packet: allow access to packet RSS hash values

2015-08-20 Thread Zoltan Kiss



On 20/08/15 17:43, Bill Fischofer wrote:

Remember that the purpose of RSS is simply to help spread arriving
packets to different receiving CPUs.  Also, the RSS has itself is not
static.  It takes as input the RSS key, which defines the packet fields
over which the hash is calculated, and the low order bits of it are used
as an index into the RSS indirection table.  Both the keys and the
indirection table are dynamically updated by SW, so the interpretation
of a given RSS hash value will vary.  So I can't look at a packet and
tell you the RSS hash without knowing what key to use, and the
interpretation of the hash is dependent on the indirection table (also
not part of the hash).  Once the packet has been delivered in accordance
with the information in the indirection table the meaning of the RSS
hash is no longer current.  At that point it's just a pseudo-random set
of bits that is indirectly associated with what were the packet fields
of interest at the time the packet was originally received.


Currently the purpose is just to have a hash of certain headers, which 
defines a flow. Later on we can add APIs about how to define that set of 
headers, right now these bits are the important bits for OVS.

Also, the key an the indirection table is not relevant for OVS's use case.



On Thu, Aug 20, 2015 at 9:20 AM, Zoltan Kiss zoltan.k...@linaro.org
mailto:zoltan.k...@linaro.org wrote:

Hi,

On 20/08/15 11:53, Jerin Jacob wrote:

How about following change in OVS plarform specific
packet_get_hash code.
odp_packet_rss_hash_set
kind of interface wont fit correctly in octeon  platform as hash
identifier used by hardware in subsequent HW based queue operations

static inline uint32_t
packet_get_hash(struct dp_packet *p)
{
#ifdef DPDK_NETDEV
  return p-mbuf.hash.rss;
#else
#ifdef ODP_NETDEV
  unit32_t hash;
  hash = odp_packet_rss_hash(p-odp_pkt);
  if (OVS_UNLIKELY(!hash)
 hash = odp_packet_generate_rss_hash(p-odp_pkt);
  return hash;
#endif
  return p-rss_hash;
#endif
}


I was thinking about this too, but I see two problems:

1. OVS changes the hash if the packet is currently recirculated:

https://github.com/openvswitch/ovs/blob/master/lib/dpif-netdev.c#L3125

And as far as I can see, it doesn't recalculate the hash when the
recirculation is finished. So the final hash of the packet at the
end won't be what the platform would generate. OVS doesn't seem to
care about it, I think the assumption is that the platform won't use
this hash anymore, and it's role is finished after matching in EMC.
My first idea to sort this out is to check for recirculation depth
in netdev_send(), before the send of course. If it's true, then we
can regenerate the hash using odp_packet_generate_rss_hash()

2. Minor thing, but if the platform is only able to do this from
software (like DPDK), it should better be as fast as OVS's hashing
at least. OVS uses CRC32 on SSE4 x64 CPUs, and Murmurhash on others.
I don't know how do they compare e.g. with Toeplitz, but I guess
they choose them because they are more suitable for the purpose.

___
lng-odp mailing list
lng-odp@lists.linaro.org mailto: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/4] api: time: unbind CPU cycles from time API

2015-08-20 Thread Savolainen, Petri (Nokia - FI/Espoo)


 -Original Message-
 From: ext Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
 Sent: Thursday, August 20, 2015 4:49 PM
 To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org;
 Stuart Haslam; Maxim Uvarov
 Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind CPU
 cycles from time API
 
 
 On 20.08.15 11:22, Savolainen, Petri (Nokia - FI/Espoo) wrote:
  Since it's time API, the prefix should be odp_time_
 
  Maybe we need to use an abstract time type (odp_time_t) to make it
 clear that application should not  directly calculate time (or ticks)
 but use APIs. We could add other (to_sec, to_ms, ...) conversion calls
 later.
 
  odp_time_t odp_time(void);
  odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2);
  uint64_t odp_time_to_ns(odp_time_t time);
  odp_time_t odp_time_from_ns(uint64_t ns);
 
  Additionally, resolution maybe interesting (since it may vary a lot:
 from 1ns to tens of ms)
 
  // Time resolution in nanoseconds
  uint64_t odp_time_resolution_ns(void);
 
 
  -Petri
 
 
 
 I'm Ok to change it ot odp_time.
 Stuart, Maxim what do you say?
 
 According resolution, I had sent patch based on this series
 validation: time: use timer resolution instead of TOLERANCE
 https://lists.linaro.org/pipermail/lng-odp/2015-August/014435.html
 As an example it uses odp_time_tick_to_ns(1) to get resolution.
 I think no need in this API.

With opaque types (odp_time_t),

odp_time_to_ns(1);


would be an illegal call. Time value would be packed into odp_time_t in an 
implementation specific way.
Application would calculate in nanosec (uint64_t), and would use the API to 
convert between nsec and odp_time_t.

-Petri




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


[lng-odp] [PATCHv6] example:generator:move verbose from worker to control

2015-08-20 Thread Balakrishna.Garapati
Signed-off-by: Balakrishna.Garapati balakrishna.garap...@linaro.org
---
  v5:Fixed race condition with workers
 removed print infor for UDP mode
 removed send, receive print info from send thread
 removed hard coded num_workers = 1 for ping mode
 introduced condtion check for ping mode for receive thread to exit

  v6: moved prev_pkt = pkts to if condition.
 
 example/generator/odp_generator.c | 103 ++
 1 file changed, 70 insertions(+), 33 deletions(-)

diff --git a/example/generator/odp_generator.c 
b/example/generator/odp_generator.c
index f9f219d..2837586 100644
--- a/example/generator/odp_generator.c
+++ b/example/generator/odp_generator.c
@@ -102,6 +102,7 @@ static void usage(char *progname);
 static int scan_ip(char *buf, unsigned int *paddr);
 static int scan_mac(char *in, odph_ethaddr_t *des);
 static void tv_sub(struct timeval *recvtime, struct timeval *sendtime);
+static void print_global_stats(int num_workers);
 
 /**
  * Sleep for the specified amount of milliseconds
@@ -372,7 +373,6 @@ static odp_pktio_t create_pktio(const char *dev, odp_pool_t 
pool)
 static void *gen_send_thread(void *arg)
 {
int thr;
-   uint64_t start, now, diff;
odp_pktio_t pktio;
thread_args_t *thr_args;
odp_queue_t outq_def;
@@ -394,7 +394,6 @@ static void *gen_send_thread(void *arg)
return NULL;
}
 
-   start = odp_time_cycles();
printf(  [%02i] created mode: SEND\n, thr);
for (;;) {
int err;
@@ -435,15 +434,6 @@ static void *gen_send_thread(void *arg)
= (unsigned int)args-appl.number) {
break;
}
-
-   now = odp_time_cycles();
-   diff = odp_time_diff_cycles(start, now);
-   if (odp_time_cycles_to_ns(diff)  20 * ODP_TIME_SEC) {
-   start = odp_time_cycles();
-   printf(  [%02i] total send: %ju\n,
-  thr, odp_atomic_load_u64(counters.seq));
-   fflush(stdout);
-   }
}
 
/* receive number of reply pks until timeout */
@@ -461,15 +451,6 @@ static void *gen_send_thread(void *arg)
}
}
 
-   /* print info */
-   if (args-appl.mode == APPL_MODE_UDP) {
-   printf(  [%02i] total send: %ju\n,
-  thr, odp_atomic_load_u64(counters.seq));
-   } else if (args-appl.mode == APPL_MODE_PING) {
-   printf(  [%02i] total send: %ju total receive: %ju\n,
-  thr, odp_atomic_load_u64(counters.seq),
-  odp_atomic_load_u64(counters.icmp));
-   }
return arg;
 }
 
@@ -485,7 +466,6 @@ static void print_pkts(int thr, odp_packet_t pkt_tbl[], 
unsigned len)
odp_packet_t pkt;
char *buf;
odph_ipv4hdr_t *ip;
-   odph_udphdr_t *udp;
odph_icmphdr_t *icmp;
struct timeval tvrecv;
struct timeval tvsend;
@@ -503,20 +483,13 @@ static void print_pkts(int thr, odp_packet_t pkt_tbl[], 
unsigned len)
continue;
 
odp_atomic_inc_u64(counters.ip);
-   rlen += sprintf(msg, receive Packet proto:IP );
buf = odp_packet_data(pkt);
ip = (odph_ipv4hdr_t *)(buf + odp_packet_l3_offset(pkt));
-   rlen += sprintf(msg + rlen, id %d ,
-   odp_be_to_cpu_16(ip-id));
offset = odp_packet_l4_offset(pkt);
 
/* udp */
if (ip-proto == ODPH_IPPROTO_UDP) {
odp_atomic_inc_u64(counters.udp);
-   udp = (odph_udphdr_t *)(buf + offset);
-   rlen += sprintf(msg + rlen, UDP payload %d ,
-   odp_be_to_cpu_16(udp-length) -
-   ODPH_UDPHDR_LEN);
}
 
/* icmp */
@@ -540,10 +513,10 @@ static void print_pkts(int thr, odp_packet_t pkt_tbl[], 
unsigned len)
rlen += sprintf(msg + rlen,
Icmp Echo Request);
}
-   }
 
-   msg[rlen] = '\0';
-   printf(  [%02i] %s\n, thr, msg);
+   msg[rlen] = '\0';
+   printf(  [%02i] %s\n, thr, msg);
+   }
}
 }
 
@@ -573,6 +546,12 @@ static void *gen_recv_thread(void *arg)
printf(  [%02i] created mode: RECEIVE\n, thr);
 
for (;;) {
+   if (args-appl.number != -1 
+   odp_atomic_load_u64(counters.icmp) =
+   (unsigned int)args-appl.number) {
+   break;
+   }
+
/* Use schedule to get buf from any input queue */
ev = odp_schedule(NULL, ODP_SCHED_WAIT);
 
@@ -590,6 +569,64 @@ static void 

[lng-odp] [PATCHv5] example:generator:move verbose from worker to control

2015-08-20 Thread Balakrishna.Garapati
Signed-off-by: Balakrishna.Garapati balakrishna.garap...@linaro.org
---
 v5: Fixed race condition with workers
 removed print infor for UDP mode
 removed send, receive print info from send thread
 removed hard coded num_workers = 1 for ping mode
 introduced condtion check for ping mode for receive thread to exit

 example/generator/odp_generator.c | 104 ++
 1 file changed, 71 insertions(+), 33 deletions(-)

diff --git a/example/generator/odp_generator.c 
b/example/generator/odp_generator.c
index f9f219d..ed6991f 100644
--- a/example/generator/odp_generator.c
+++ b/example/generator/odp_generator.c
@@ -102,6 +102,7 @@ static void usage(char *progname);
 static int scan_ip(char *buf, unsigned int *paddr);
 static int scan_mac(char *in, odph_ethaddr_t *des);
 static void tv_sub(struct timeval *recvtime, struct timeval *sendtime);
+static void print_global_stats(int num_workers);
 
 /**
  * Sleep for the specified amount of milliseconds
@@ -372,7 +373,6 @@ static odp_pktio_t create_pktio(const char *dev, odp_pool_t 
pool)
 static void *gen_send_thread(void *arg)
 {
int thr;
-   uint64_t start, now, diff;
odp_pktio_t pktio;
thread_args_t *thr_args;
odp_queue_t outq_def;
@@ -394,7 +394,6 @@ static void *gen_send_thread(void *arg)
return NULL;
}
 
-   start = odp_time_cycles();
printf(  [%02i] created mode: SEND\n, thr);
for (;;) {
int err;
@@ -435,15 +434,6 @@ static void *gen_send_thread(void *arg)
= (unsigned int)args-appl.number) {
break;
}
-
-   now = odp_time_cycles();
-   diff = odp_time_diff_cycles(start, now);
-   if (odp_time_cycles_to_ns(diff)  20 * ODP_TIME_SEC) {
-   start = odp_time_cycles();
-   printf(  [%02i] total send: %ju\n,
-  thr, odp_atomic_load_u64(counters.seq));
-   fflush(stdout);
-   }
}
 
/* receive number of reply pks until timeout */
@@ -461,15 +451,6 @@ static void *gen_send_thread(void *arg)
}
}
 
-   /* print info */
-   if (args-appl.mode == APPL_MODE_UDP) {
-   printf(  [%02i] total send: %ju\n,
-  thr, odp_atomic_load_u64(counters.seq));
-   } else if (args-appl.mode == APPL_MODE_PING) {
-   printf(  [%02i] total send: %ju total receive: %ju\n,
-  thr, odp_atomic_load_u64(counters.seq),
-  odp_atomic_load_u64(counters.icmp));
-   }
return arg;
 }
 
@@ -485,7 +466,6 @@ static void print_pkts(int thr, odp_packet_t pkt_tbl[], 
unsigned len)
odp_packet_t pkt;
char *buf;
odph_ipv4hdr_t *ip;
-   odph_udphdr_t *udp;
odph_icmphdr_t *icmp;
struct timeval tvrecv;
struct timeval tvsend;
@@ -503,20 +483,13 @@ static void print_pkts(int thr, odp_packet_t pkt_tbl[], 
unsigned len)
continue;
 
odp_atomic_inc_u64(counters.ip);
-   rlen += sprintf(msg, receive Packet proto:IP );
buf = odp_packet_data(pkt);
ip = (odph_ipv4hdr_t *)(buf + odp_packet_l3_offset(pkt));
-   rlen += sprintf(msg + rlen, id %d ,
-   odp_be_to_cpu_16(ip-id));
offset = odp_packet_l4_offset(pkt);
 
/* udp */
if (ip-proto == ODPH_IPPROTO_UDP) {
odp_atomic_inc_u64(counters.udp);
-   udp = (odph_udphdr_t *)(buf + offset);
-   rlen += sprintf(msg + rlen, UDP payload %d ,
-   odp_be_to_cpu_16(udp-length) -
-   ODPH_UDPHDR_LEN);
}
 
/* icmp */
@@ -540,10 +513,10 @@ static void print_pkts(int thr, odp_packet_t pkt_tbl[], 
unsigned len)
rlen += sprintf(msg + rlen,
Icmp Echo Request);
}
-   }
 
-   msg[rlen] = '\0';
-   printf(  [%02i] %s\n, thr, msg);
+   msg[rlen] = '\0';
+   printf(  [%02i] %s\n, thr, msg);
+   }
}
 }
 
@@ -573,6 +546,12 @@ static void *gen_recv_thread(void *arg)
printf(  [%02i] created mode: RECEIVE\n, thr);
 
for (;;) {
+   if (args-appl.number != -1 
+   odp_atomic_load_u64(counters.icmp) =
+   (unsigned int)args-appl.number) {
+   break;
+   }
+
/* Use schedule to get buf from any input queue */
ev = odp_schedule(NULL, ODP_SCHED_WAIT);
 
@@ -590,6 +569,65 @@ static void *gen_recv_thread(void *arg)
 
return arg;
 }

Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind CPU cycles from time API

2015-08-20 Thread Ivan Khoronzhuk


On 20.08.15 11:22, Savolainen, Petri (Nokia - FI/Espoo) wrote:

Since it's time API, the prefix should be odp_time_

Maybe we need to use an abstract time type (odp_time_t) to make it clear that 
application should not  directly calculate time (or ticks) but use APIs. We 
could add other (to_sec, to_ms, ...) conversion calls later.

odp_time_t odp_time(void);
odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2);
uint64_t odp_time_to_ns(odp_time_t time);
odp_time_t odp_time_from_ns(uint64_t ns);

Additionally, resolution maybe interesting (since it may vary a lot: from 1ns 
to tens of ms)

// Time resolution in nanoseconds
uint64_t odp_time_resolution_ns(void);


-Petri




I'm Ok to change it ot odp_time.
Stuart, Maxim what do you say?

According resolution, I had sent patch based on this series
validation: time: use timer resolution instead of TOLERANCE
https://lists.linaro.org/pipermail/lng-odp/2015-August/014435.html
As an example it uses odp_time_tick_to_ns(1) to get resolution.
I think no need in this API.




-Original Message-
From: ext Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
Sent: Wednesday, August 19, 2015 5:53 PM
To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org;
Stuart Haslam; Maxim Uvarov
Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind CPU
cycles from time API

+

On 19.08.15 17:49, Ivan Khoronzhuk wrote:



On 19.08.15 17:12, Savolainen, Petri (Nokia - FI/Espoo) wrote:

Hi,

As I mentioned in the call, I'd like to avoid confusion between time

and timer (timer tick). In minimum, the API documentation should not
refer to timer. Also could term 'tick' be removed altogether and just
use 'time'?


Something like this:

uint64_t odp_time(void);
uint64_t odp_time_diff(uint64_t t1, uint64_t t2);
uint64_t odp_time_to_ns(uint64_t time);
uint64_t odp_time_from_ns(uint64_t ns);

-Petri


Sorry I've unintentionally missed this call.

It's confusing only at first glance. I'm used to this already.
Even don't know, it seems to be reasonable, but any way we
have similar function odp_time_to_ns. And avoiding word tick
makes the functions a little strange, like you take time, by default
it is in ticks, but time is so general word... name odp_time_to_ns
sounds like you convert time to ns, how it can be?

But if it can make life easier I have no objection, I don't know

conclusion

about this on the call but if Maxim, Stuart and others are OK with it

I

can change it.



Maybe
odp_tick()
odp_tick_diff()
odp_tick_to_ns()
odp_tick_from_ns()







diff --git a/include/odp/api/time.h b/include/odp/api/time.h
index b0072fc..b48dcae 100644
--- a/include/odp/api/time.h
+++ b/include/odp/api/time.h
@@ -30,11 +30,11 @@ extern C {


   /**
- * Current time in CPU cycles
+ * Current time in ticks of best hi-resolution timer available
*
- * @return Current time in CPU cycles
+ * @return Current time in timer ticks
*/
-uint64_t odp_time_cycles(void);
+uint64_t odp_time_tick(void);


   /**
@@ -43,29 +43,29 @@ uint64_t odp_time_cycles(void);
* @param t1First time stamp
* @param t2Second time stamp
*
- * @return Difference of time stamps in CPU cycles
+ * @return Difference of time stamps in timer ticks
*/
-uint64_t odp_time_diff_cycles(uint64_t t1, uint64_t t2);
+uint64_t odp_time_ticks_diff(uint64_t t1, uint64_t t2);


   /**
- * Convert CPU cycles to nanoseconds
+ * Convert timer ticks to nanoseconds
*
- * @param cycles  Time in CPU cycles
+ * @param ticks  Time in timer ticks
*
* @return Time in nanoseconds
*/
-uint64_t odp_time_cycles_to_ns(uint64_t cycles);
+uint64_t odp_time_tick_to_ns(uint64_t ticks);


   /**
- * Convert nanoseconds to CPU cycles
+ * Convert nanoseconds to ticks of best hi-resolution timer

available

*
* @param ns  Time in nanoseconds
*
- * @return Time in CPU cycles
+ * @return Time in timer ticks
*/
-uint64_t odp_time_ns_to_cycles(uint64_t ns);
+uint64_t odp_time_ns_to_tick(uint64_t ns);




--
Regards,
Ivan Khoronzhuk


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


Re: [lng-odp] [API-NEXT PATCH] api: define pktio statistics api

2015-08-20 Thread Stuart Haslam
On Wed, Aug 19, 2015 at 03:22:59PM +0300, Maxim Uvarov wrote:
 Signed-off-by: Maxim Uvarov maxim.uva...@linaro.org
 ---
  include/odp/api/packet_io_stats.h  | 77 
 ++
  platform/linux-generic/include/odp/packet_io.h |  1 +
  2 files changed, 78 insertions(+)
  create mode 100644 include/odp/api/packet_io_stats.h
 
 diff --git a/include/odp/api/packet_io_stats.h 
 b/include/odp/api/packet_io_stats.h
 new file mode 100644
 index 000..731a96f
 --- /dev/null
 +++ b/include/odp/api/packet_io_stats.h
 @@ -0,0 +1,77 @@
 +/* Copyright (c) 2015, Linaro Limited
 + * All rights reserved.
 + *
 + * SPDX-License-Identifier: BSD-3-Clause
 + */
 +
 +/**
 + * @file
 + *
 + * ODP Packet IO
 + */
 +
 +#ifndef ODP_API_PACKET_IO_STATS_H_
 +#define ODP_API_PACKET_IO_STATS_H_
 +
 +#ifdef __cplusplus
 +extern C {
 +#endif
 +
 +/** @defgroup odp_packet_io ODP PACKET IO
 + *  @{
 + */
 +
 +/**
 + * Packet IO statistics
 + *
 + */
 +typedef struct odp_pktio_stats_t {
 + uint64_t collisions;  /** number of collisions */
 + uint64_t multicast;   /** multicast packets received */
 +
 + uint64_t rx_bytes;/** total bytes received */
 + uint64_t rx_crc_errors;   /** received packets with crc error */
 + uint64_t rx_dropped;  /** no space in buffers */
 + uint64_t rx_errors;   /** bad packets received */
 + uint64_t rx_fifo_errors;  /** recv'r fifo overrun */
 + uint64_t rx_frame_errors; /** recv'd frame alignment error */
 + uint64_t rx_length_errors;/** bad packets length */
 + uint64_t rx_missed_errors;/** receiver missed packet */
 + uint64_t rx_over_errors;  /** receiver ring buff overflow */
 + uint64_t rx_packets;  /** total packets received   */
 +
 + uint64_t tx_aborted_errors;   /** packets aborted during
 +   transmission by a network device (e.g:
 +   because of a medium collision) */
 + uint64_t tx_bytes;/** total bytes transmitted */
 + uint64_t tx_carrier_errors;   /** not transmitted packets because of
 + carrier errors (e.g: physical link down) */
 + uint64_t tx_dropped;  /** no resources to transmit packet*/
 + uint64_t tx_errors;   /** packets transmit problems */
 + uint64_t tx_fifo_errors;  /** packets transmit FIFO errors */
 + uint64_t tx_heartbeat_errors; /** packets transmitted that have been
 +   reported as heartbeat errors */
 + uint64_t tx_packets;  /** total packets transmitted */
 + uint64_t tx_window_errors;/** number of packets not successfully
 +   transmitted due to a window collision.*/
 +} odp_pktio_stats_t;

Are all pktios required to support all of these stats? (some of them
seem to be Ethernet specific) If not, how do I tell which ones are
supported?

Are they always enabled or can they be enabled/disabled at build or run
time?

 +
 +/**
 + * Get statistics for pktio handle
 + *
 + * @parampktioPacket IO handle
 + * @param[out]   *stats   Output buffer
 + * @retval  0 on success
 + * @retval 0 on failure
 + */
 +int odp_pktio_stats(odp_pktio_t pktio, odp_pktio_stats_t *stats);

What about resetting the stats?

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


Re: [lng-odp] [API-NEXT PATCH v3 1/3] api: packet: allow access to packet RSS hash values

2015-08-20 Thread Jacob, Jerin

​OVS just need flow signature/hash for miniflow looup.IMO, no need to specify 
RSS in normative ODP API spec it very specific to NIC.


From: Bill Fischofer bill.fischo...@linaro.org
Sent: Thursday, August 20, 2015 5:32 PM
To: Jacob, Jerin
Cc: Santosh Shukla; LNG ODP Mailman List
Subject: Re: [lng-odp] [API-NEXT PATCH v3 1/3] api: packet: allow access to 
packet RSS hash values

The RSS is relevant to packets originating from a NIC and is independent of the 
CoS or other flow designators.  It's there mainly because some applications 
(e.g., OVS) use it internally, so it's for legacy support.



On Thu, Aug 20, 2015 at 6:43 AM, Jerin Jacob 
jerin.ja...@caviumnetworks.commailto:jerin.ja...@caviumnetworks.com wrote:
On Thu, Aug 20, 2015 at 04:52:29PM +0530, Santosh Shukla wrote:
 On 20 August 2015 at 16:23, Jerin Jacob 
 jerin.ja...@caviumnetworks.commailto:jerin.ja...@caviumnetworks.com wrote:
  On Thu, Aug 20, 2015 at 03:30:23PM +0530, Santosh Shukla wrote:
  On 20 August 2015 at 14:48, Balasubramanian Manoharan
  bala.manoha...@linaro.orgmailto:bala.manoha...@linaro.org wrote:
  
  
   On Sunday 16 August 2015 06:14 PM, Santosh Shukla wrote:
  
   On 15 August 2015 at 00:12, Zoltan Kiss 
   zoltan.k...@linaro.orgmailto:zoltan.k...@linaro.org wrote:
  
   Applications can read the computed hash (if any) and set it if they
   changed the packet headers or if the platform haven't calculated the
   hash.
  
   Signed-off-by: Zoltan Kiss 
   zoltan.k...@linaro.orgmailto:zoltan.k...@linaro.org
   ---
   v2:
   - focus on RSS hash only
   - use setter/getter's
  
   v3:
   - do not mention pointers
   - add a note
   - add new patches for implementation and test
  
 include/odp/api/packet.h | 30 ++
 1 file changed, 30 insertions(+)
  
   diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
   index 3a454b5..1ae24bc 100644
   --- a/include/odp/api/packet.h
   +++ b/include/odp/api/packet.h
   @@ -48,6 +48,11 @@ extern C {
  * Invalid packet segment
  */
  
   +/**
   + * @def ODP_PACKET_RSS_INVALID
   + * RSS hash is not set
   + */
   +
 /*
  *
  * Alloc and free
   @@ -605,6 +610,31 @@ uint32_t odp_packet_l4_offset(odp_packet_t pkt);
 int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t offset);
  
 /**
   + * RSS hash value
   + *
   + * Returns the RSS hash stored for the packet.
   + *
   + * @param  pkt  Packet handle
   + *
   + * @return  Hash value
   + * @retval ODP_PACKET_RSS_INVALID if RSS hash is not set.
   + */
   +uint32_t odp_packet_rss_hash(odp_packet_t pkt);
   +
   +/**
   + * Set RSS hash value
   + *
   + * Store the RSS hash for the packet.
   + *
   + * @param  pkt  Packet handle
   + * @param  rss_hash Hash value to set
   + *
   + * @note If the application changes the packet header, it might want 
   to
   + * recalculate this value and set it.
   + */
   +void odp_packet_rss_hash_set(odp_packet_t pkt, uint32_t rss_hash);
  
   Can this use-case be handled by calling
   odp_packet_generate_rss_hash(odp_packet_t pkt) function where the rss 
   gets
   generated by the implementation rather than being set from application 
   using
   odp_packet_set_rss_hash() function?
  
 
  Bala, As we discussed and in summary for rest; Considering ovs example
  : ovs uses sw based rss_hash only when HW not supporting rss Or HW
  generated rss is zero.
 
  hash = packet_get_hash(packet);
  if (OVS_UNLIKELY(!hash)) {
  hash = miniflow_hash_5tuple(mf, 0);
  packet_set_hash(packet, hash);
  }
  return hash;
 
  and rss_hash generation is inside ovs dp_packet (middle  layer), It is
 
  How about following change in OVS plarform specific packet_get_hash code.
  odp_packet_rss_hash_set
  kind of interface wont fit correctly in octeon  platform as hash
  identifier used by hardware in subsequent HW based queue operations
 
  static inline uint32_t
  packet_get_hash(struct dp_packet *p)
  {
  #ifdef DPDK_NETDEV
  return p-mbuf.hash.rss;
  #else
  #ifdef ODP_NETDEV
  unit32_t hash;
  hash = odp_packet_rss_hash(p-odp_pkt);
  if (OVS_UNLIKELY(!hash)
  hash = odp_packet_generate_rss_hash(p-odp_pkt);
  return hash;
  #endif
  return p-rss_hash;
  #endif
  }
 

 Trying to understand your api definition;
 odp_packet_rss_hash() - to get rss
 and
 odp_packet_generate_rss_hash() - generate rss from where :  sw or hw?

NOP if HW is capable, for software generated packet/ non-capable HW it
will be in SW

 In either case, your trying to generate non-zero rss (considering a
 valid rss (?) for your platform).

IMO, The term RSS may not be correct in API definition. May be something
like flow id, make sense across all platform.

Jerin


 - IIUC _generate_rss_hash() able to take corrective measure so you
 won't find need for using rss_hash_set() right?
 - In other word, No s/w based rss __setting__ required for octeon.

 did I 

Re: [lng-odp] [API-NEXT PATCH v3 1/3] api: packet: allow access to packet RSS hash values

2015-08-20 Thread Bala Manoharan
Hi,

On 20 August 2015 at 17:32, Bill Fischofer bill.fischo...@linaro.org
wrote:

 The RSS is relevant to packets originating from a NIC and is independent
 of the CoS or other flow designators.  It's there mainly because some
 applications (e.g., OVS) use it internally, so it's for legacy support.


This API might be used for legacy applications but if the HW generates RSS
by default then that value can be used by the application, somehow this API
is seen as a requirement only for OVS but getting the hash value of the
packet will be used by other applications also. So we should solve the
generic use-case rather than focussing for OVS alone.

The issue here is to avoid the application to configure the hash value
since it is possible that the application configures a hash value different
from what the implementation would have generated for that particular
packet flow and this would cause an issue in some platforms. This was the
basis on which the removal of set_rss_hash() function was suggested.

Regards,
Bala


 On Thu, Aug 20, 2015 at 6:43 AM, Jerin Jacob 
 jerin.ja...@caviumnetworks.com wrote:

 On Thu, Aug 20, 2015 at 04:52:29PM +0530, Santosh Shukla wrote:
  On 20 August 2015 at 16:23, Jerin Jacob jerin.ja...@caviumnetworks.com
 wrote:
   On Thu, Aug 20, 2015 at 03:30:23PM +0530, Santosh Shukla wrote:
   On 20 August 2015 at 14:48, Balasubramanian Manoharan
   bala.manoha...@linaro.org wrote:
   
   
On Sunday 16 August 2015 06:14 PM, Santosh Shukla wrote:
   
On 15 August 2015 at 00:12, Zoltan Kiss zoltan.k...@linaro.org
 wrote:
   
Applications can read the computed hash (if any) and set it if
 they
changed the packet headers or if the platform haven't calculated
 the
hash.
   
Signed-off-by: Zoltan Kiss zoltan.k...@linaro.org
---
v2:
- focus on RSS hash only
- use setter/getter's
   
v3:
- do not mention pointers
- add a note
- add new patches for implementation and test
   
  include/odp/api/packet.h | 30 ++
  1 file changed, 30 insertions(+)
   
diff --git a/include/odp/api/packet.h b/include/odp/api/packet.h
index 3a454b5..1ae24bc 100644
--- a/include/odp/api/packet.h
+++ b/include/odp/api/packet.h
@@ -48,6 +48,11 @@ extern C {
   * Invalid packet segment
   */
   
+/**
+ * @def ODP_PACKET_RSS_INVALID
+ * RSS hash is not set
+ */
+
  /*
   *
   * Alloc and free
@@ -605,6 +610,31 @@ uint32_t odp_packet_l4_offset(odp_packet_t
 pkt);
  int odp_packet_l4_offset_set(odp_packet_t pkt, uint32_t
 offset);
   
  /**
+ * RSS hash value
+ *
+ * Returns the RSS hash stored for the packet.
+ *
+ * @param  pkt  Packet handle
+ *
+ * @return  Hash value
+ * @retval ODP_PACKET_RSS_INVALID if RSS hash is not set.
+ */
+uint32_t odp_packet_rss_hash(odp_packet_t pkt);
+
+/**
+ * Set RSS hash value
+ *
+ * Store the RSS hash for the packet.
+ *
+ * @param  pkt  Packet handle
+ * @param  rss_hash Hash value to set
+ *
+ * @note If the application changes the packet header, it might
 want to
+ * recalculate this value and set it.
+ */
+void odp_packet_rss_hash_set(odp_packet_t pkt, uint32_t
 rss_hash);
   
Can this use-case be handled by calling
odp_packet_generate_rss_hash(odp_packet_t pkt) function where the
 rss gets
generated by the implementation rather than being set from
 application using
odp_packet_set_rss_hash() function?
   
  
   Bala, As we discussed and in summary for rest; Considering ovs
 example
   : ovs uses sw based rss_hash only when HW not supporting rss Or HW
   generated rss is zero.
  
   hash = packet_get_hash(packet);
   if (OVS_UNLIKELY(!hash)) {
   hash = miniflow_hash_5tuple(mf, 0);
   packet_set_hash(packet, hash);
   }
   return hash;
  
   and rss_hash generation is inside ovs dp_packet (middle  layer), It
 is
  
   How about following change in OVS plarform specific packet_get_hash
 code.
   odp_packet_rss_hash_set
   kind of interface wont fit correctly in octeon  platform as hash
   identifier used by hardware in subsequent HW based queue operations
  
   static inline uint32_t
   packet_get_hash(struct dp_packet *p)
   {
   #ifdef DPDK_NETDEV
   return p-mbuf.hash.rss;
   #else
   #ifdef ODP_NETDEV
   unit32_t hash;
   hash = odp_packet_rss_hash(p-odp_pkt);
   if (OVS_UNLIKELY(!hash)
   hash = odp_packet_generate_rss_hash(p-odp_pkt);
   return hash;
   #endif
   return p-rss_hash;
   #endif
   }
  
 
  Trying to understand your api definition;
  odp_packet_rss_hash() - to get rss
  and
  odp_packet_generate_rss_hash() - generate rss from where :  sw or hw?

 NOP if HW is capable, for software generated packet/ non-capable HW it
 will be in SW

  In either case, your trying to generate non-zero rss (considering a
  valid rss (?) for 

[lng-odp] [Bug 1701] validation/packet assumes the number of available segments

2015-08-20 Thread bugzilla-daemon
https://bugs.linaro.org/show_bug.cgi?id=1701

--- Comment #1 from Bill Fischofer bill.fischo...@linaro.org ---
Currently being worked on by Nicolas.  Patch
http://patches.opendataplane.org/patch/2776/ posted.

-- 
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] [API-NEXT PATCH v2 1/4] api: time: unbind CPU cycles from time API

2015-08-20 Thread Ivan Khoronzhuk



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




-Original Message-
From: ext Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
Sent: Thursday, August 20, 2015 4:49 PM
To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org;
Stuart Haslam; Maxim Uvarov
Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind CPU
cycles from time API


On 20.08.15 11:22, Savolainen, Petri (Nokia - FI/Espoo) wrote:

Since it's time API, the prefix should be odp_time_

Maybe we need to use an abstract time type (odp_time_t) to make it

clear that application should not  directly calculate time (or ticks)
but use APIs. We could add other (to_sec, to_ms, ...) conversion calls
later.


odp_time_t odp_time(void);
odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2);
uint64_t odp_time_to_ns(odp_time_t time);
odp_time_t odp_time_from_ns(uint64_t ns);

Additionally, resolution maybe interesting (since it may vary a lot:

from 1ns to tens of ms)


// Time resolution in nanoseconds
uint64_t odp_time_resolution_ns(void);


-Petri




I'm Ok to change it ot odp_time.
Stuart, Maxim what do you say?

According resolution, I had sent patch based on this series
validation: time: use timer resolution instead of TOLERANCE
https://lists.linaro.org/pipermail/lng-odp/2015-August/014435.html
As an example it uses odp_time_tick_to_ns(1) to get resolution.
I think no need in this API.


With opaque types (odp_time_t),

odp_time_to_ns(1);


would be an illegal call. Time value would be packed into odp_time_t in an 
implementation specific way.
Application would calculate in nanosec (uint64_t), and would use the API to 
convert between nsec and odp_time_t.


Can be..not sure about odp_time_t, seems it's logical, it asks us to use API 
and abstracts time from units.
But it requires a little more changes...and it be good to know if others are OK 
before changing it.



-Petri






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


[lng-odp] [PATCH 1/2] Validation: system: fix uninitialised variable

2015-08-20 Thread Mike Holmes
It is possible that a code change leaves s=NULL; default char_ok to
failure

Signed-off-by: Mike Holmes mike.hol...@linaro.org
---
 test/validation/system/system.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/validation/system/system.c b/test/validation/system/system.c
index 7687f20..2b788d2 100644
--- a/test/validation/system/system.c
+++ b/test/validation/system/system.c
@@ -12,7 +12,7 @@
 
 void system_test_odp_version_numbers(void)
 {
-   int char_ok;
+   int char_ok = 0;
char version_string[128];
char *s = version_string;
 
-- 
2.1.4

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


[lng-odp] [PATCH 2/2] validation: add test on unused retvals

2015-08-20 Thread Mike Holmes
Fix unused return codes by testing for validity

Signed-off-by: Mike Holmes mike.hol...@linaro.org
---
 test/validation/classification/odp_classification_tests.c | 1 +
 test/validation/common/mask_common.c  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/test/validation/classification/odp_classification_tests.c 
b/test/validation/classification/odp_classification_tests.c
index 0e0c4eb..a312dc9 100644
--- a/test/validation/classification/odp_classification_tests.c
+++ b/test/validation/classification/odp_classification_tests.c
@@ -407,6 +407,7 @@ void configure_cls_pmr_chain(void)
 
retval = odp_cos_set_queue(cos_list[CLS_PMR_CHAIN_DST],
   queue_list[CLS_PMR_CHAIN_DST]);
+   CU_ASSERT(retval == 0);
 
parse_ipv4_string(CLS_PMR_CHAIN_SADDR, addr, mask);
pmr_list[CLS_PMR_CHAIN_SRC] = odp_pmr_create(ODP_PMR_SIP_ADDR, addr,
diff --git a/test/validation/common/mask_common.c 
b/test/validation/common/mask_common.c
index fce7725..9b1a23f 100644
--- a/test/validation/common/mask_common.c
+++ b/test/validation/common/mask_common.c
@@ -145,6 +145,7 @@ MASK_TESTFUNC(to_from_str)
 
str_sz = _odp_mask_to_str(mask, buf_out,
  stringlen(TEST_MASK_0) + 1);
+   CU_ASSERT(str_sz  == (int32_t)stringlen(TEST_MASK_0) + 1);
 
CU_ASSERT_NSTRING_EQUAL(buf_out, TEST_MASK_0,
stringlen(TEST_MASK_0) + 1);
-- 
2.1.4

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


Re: [lng-odp] [PATCH 2/2] validation: add test on unused retvals

2015-08-20 Thread Bill Fischofer
On Thu, Aug 20, 2015 at 2:18 PM, Mike Holmes mike.hol...@linaro.org wrote:

 Fix unused return codes by testing for validity

 Signed-off-by: Mike Holmes mike.hol...@linaro.org


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


 ---
  test/validation/classification/odp_classification_tests.c | 1 +
  test/validation/common/mask_common.c  | 1 +
  2 files changed, 2 insertions(+)

 diff --git a/test/validation/classification/odp_classification_tests.c
 b/test/validation/classification/odp_classification_tests.c
 index 0e0c4eb..a312dc9 100644
 --- a/test/validation/classification/odp_classification_tests.c
 +++ b/test/validation/classification/odp_classification_tests.c
 @@ -407,6 +407,7 @@ void configure_cls_pmr_chain(void)

 retval = odp_cos_set_queue(cos_list[CLS_PMR_CHAIN_DST],
queue_list[CLS_PMR_CHAIN_DST]);
 +   CU_ASSERT(retval == 0);

 parse_ipv4_string(CLS_PMR_CHAIN_SADDR, addr, mask);
 pmr_list[CLS_PMR_CHAIN_SRC] = odp_pmr_create(ODP_PMR_SIP_ADDR,
 addr,
 diff --git a/test/validation/common/mask_common.c
 b/test/validation/common/mask_common.c
 index fce7725..9b1a23f 100644
 --- a/test/validation/common/mask_common.c
 +++ b/test/validation/common/mask_common.c
 @@ -145,6 +145,7 @@ MASK_TESTFUNC(to_from_str)

 str_sz = _odp_mask_to_str(mask, buf_out,
   stringlen(TEST_MASK_0) + 1);
 +   CU_ASSERT(str_sz  == (int32_t)stringlen(TEST_MASK_0) + 1);

 CU_ASSERT_NSTRING_EQUAL(buf_out, TEST_MASK_0,
 stringlen(TEST_MASK_0) + 1);
 --
 2.1.4

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

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


Re: [lng-odp] [PATCH 1/2] Validation: system: fix uninitialised variable

2015-08-20 Thread Bill Fischofer
On Thu, Aug 20, 2015 at 2:18 PM, Mike Holmes mike.hol...@linaro.org wrote:

 It is possible that a code change leaves s=NULL; default char_ok to
 failure

 Signed-off-by: Mike Holmes mike.hol...@linaro.org


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


 ---
  test/validation/system/system.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/test/validation/system/system.c
 b/test/validation/system/system.c
 index 7687f20..2b788d2 100644
 --- a/test/validation/system/system.c
 +++ b/test/validation/system/system.c
 @@ -12,7 +12,7 @@

  void system_test_odp_version_numbers(void)
  {
 -   int char_ok;
 +   int char_ok = 0;
 char version_string[128];
 char *s = version_string;

 --
 2.1.4

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

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


Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind CPU cycles from time API

2015-08-20 Thread Stuart Haslam
On Thu, Aug 20, 2015 at 05:58:09PM +0300, Ivan Khoronzhuk wrote:
 
 
 On 20.08.15 17:17, Savolainen, Petri (Nokia - FI/Espoo) wrote:
 
 
 -Original Message-
 From: ext Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
 Sent: Thursday, August 20, 2015 4:49 PM
 To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org;
 Stuart Haslam; Maxim Uvarov
 Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/4] api: time: unbind CPU
 cycles from time API
 
 
 On 20.08.15 11:22, Savolainen, Petri (Nokia - FI/Espoo) wrote:
 Since it's time API, the prefix should be odp_time_
 
 Maybe we need to use an abstract time type (odp_time_t) to make it
 clear that application should not  directly calculate time (or ticks)
 but use APIs. We could add other (to_sec, to_ms, ...) conversion calls
 later.
 
 odp_time_t odp_time(void);
 odp_time_t odp_time_diff(odp_time_t t1, odp_time_t t2);
 uint64_t odp_time_to_ns(odp_time_t time);
 odp_time_t odp_time_from_ns(uint64_t ns);
 
 Additionally, resolution maybe interesting (since it may vary a lot:
 from 1ns to tens of ms)

Really? I had been thinking of a resolution somewhere between roughly  1ns
(multiple GHz as in a CPU cycle counter) to around 100ns (10MHz).

Having a resolution of tens of ms would be limiting for applications
that use this for interval timing, e.g. odp_pktio_perf uses it for
inter-burst gap timing.

 
 // Time resolution in nanoseconds
 uint64_t odp_time_resolution_ns(void);
 
 
 -Petri
 
 
 
 I'm Ok to change it ot odp_time.
 Stuart, Maxim what do you say?
 
 According resolution, I had sent patch based on this series
 validation: time: use timer resolution instead of TOLERANCE
 https://lists.linaro.org/pipermail/lng-odp/2015-August/014435.html
 As an example it uses odp_time_tick_to_ns(1) to get resolution.
 I think no need in this API.
 
 With opaque types (odp_time_t),
 
 odp_time_to_ns(1);
 
 
 would be an illegal call. Time value would be packed into odp_time_t in an 
 implementation specific way.
 Application would calculate in nanosec (uint64_t), and would use the API to 
 convert between nsec and odp_time_t.
 
 Can be..not sure about odp_time_t, seems it's logical, it asks us to use API 
 and abstracts time from units.
 But it requires a little more changes...and it be good to know if others are 
 OK before changing it.
 

The suggested changes seem sensible to me, the application should be
working with wall-clock units so removing the base units makes that a
bit clearer.

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


Re: [lng-odp] [API-NEXT PATCH v3 1/3] api: packet: allow access to packet RSS hash values

2015-08-20 Thread Bill Fischofer
Remember that the purpose of RSS is simply to help spread arriving packets
to different receiving CPUs.  Also, the RSS has itself is not static.  It
takes as input the RSS key, which defines the packet fields over which the
hash is calculated, and the low order bits of it are used as an index into
the RSS indirection table.  Both the keys and the indirection table are
dynamically updated by SW, so the interpretation of a given RSS hash value
will vary.  So I can't look at a packet and tell you the RSS hash without
knowing what key to use, and the interpretation of the hash is dependent on
the indirection table (also not part of the hash).  Once the packet has
been delivered in accordance with the information in the indirection table
the meaning of the RSS hash is no longer current.  At that point it's
just a pseudo-random set of bits that is indirectly associated with what
were the packet fields of interest at the time the packet was originally
received.

On Thu, Aug 20, 2015 at 9:20 AM, Zoltan Kiss zoltan.k...@linaro.org wrote:

 Hi,

 On 20/08/15 11:53, Jerin Jacob wrote:

 How about following change in OVS plarform specific packet_get_hash code.
 odp_packet_rss_hash_set
 kind of interface wont fit correctly in octeon  platform as hash
 identifier used by hardware in subsequent HW based queue operations

 static inline uint32_t
 packet_get_hash(struct dp_packet *p)
 {
 #ifdef DPDK_NETDEV
  return p-mbuf.hash.rss;
 #else
 #ifdef ODP_NETDEV
  unit32_t hash;
  hash = odp_packet_rss_hash(p-odp_pkt);
  if (OVS_UNLIKELY(!hash)
 hash = odp_packet_generate_rss_hash(p-odp_pkt);
  return hash;
 #endif
  return p-rss_hash;
 #endif
 }


 I was thinking about this too, but I see two problems:

 1. OVS changes the hash if the packet is currently recirculated:

 https://github.com/openvswitch/ovs/blob/master/lib/dpif-netdev.c#L3125

 And as far as I can see, it doesn't recalculate the hash when the
 recirculation is finished. So the final hash of the packet at the end won't
 be what the platform would generate. OVS doesn't seem to care about it, I
 think the assumption is that the platform won't use this hash anymore, and
 it's role is finished after matching in EMC.
 My first idea to sort this out is to check for recirculation depth in
 netdev_send(), before the send of course. If it's true, then we can
 regenerate the hash using odp_packet_generate_rss_hash()

 2. Minor thing, but if the platform is only able to do this from software
 (like DPDK), it should better be as fast as OVS's hashing at least. OVS
 uses CRC32 on SSE4 x64 CPUs, and Murmurhash on others. I don't know how do
 they compare e.g. with Toeplitz, but I guess they choose them because they
 are more suitable for the purpose.

 ___
 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 1/3] linux-generic: pktio: increase MTU of loop interface

2015-08-20 Thread Stuart Haslam
There's no need to enforce an artificial MTU for the loop interface.
Previously this value was reported but not enforced.

Signed-off-by: Stuart Haslam stuart.has...@linaro.org
---
 platform/linux-generic/pktio/loop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/platform/linux-generic/pktio/loop.c 
b/platform/linux-generic/pktio/loop.c
index 188a9ee..55371f2 100644
--- a/platform/linux-generic/pktio/loop.c
+++ b/platform/linux-generic/pktio/loop.c
@@ -19,7 +19,7 @@
 #include odp/helper/ip.h
 
 /* MTU to be reported for the loop interface */
-#define PKTIO_LOOP_MTU 1500
+#define PKTIO_LOOP_MTU INT_MAX
 /* MAC address for the loop interface */
 static const char pktio_loop_mac[] = {0x02, 0xe9, 0x34, 0x80, 0x73, 0x01};
 
-- 
2.1.1

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


[lng-odp] [PATCH 2/3] linux-generic: pktio: handle transmit errors correctly

2015-08-20 Thread Stuart Haslam
Errors which occur while sending packets via odp_pktio_send() aren't
handled correctly or even consistently across the two socket based
implementations.

The problems being addressed are;

 - calls may block indefinitely in certain error conditions (mmsg)
 - packets may be freed and reported as being sent correctly even though
   they weren't really sent (mmap)
 - return value doesn't always accurately reflect number of packets sent
 - inconsistent use of __odp_errno

Signed-off-by: Stuart Haslam stuart.has...@linaro.org
---
 .../linux-generic/include/odp_packet_io_internal.h |  6 ++
 platform/linux-generic/pktio/socket.c  | 27 +++---
 platform/linux-generic/pktio/socket_mmap.c | 96 +++---
 3 files changed, 70 insertions(+), 59 deletions(-)

diff --git a/platform/linux-generic/include/odp_packet_io_internal.h 
b/platform/linux-generic/include/odp_packet_io_internal.h
index a5d324e..09d67f5 100644
--- a/platform/linux-generic/include/odp_packet_io_internal.h
+++ b/platform/linux-generic/include/odp_packet_io_internal.h
@@ -28,6 +28,12 @@ extern C {
 #include odp/hints.h
 #include net/if.h
 
+/** Determine if a socket read/write error should be reported. Transient errors
+ *  that simply require the caller to retry are ignored, the _send/_recv APIs
+ *  are non-blocking and it is the caller's responsibility to retry if the
+ *  requested number of packets were not handled. */
+#define SOCK_ERR_REPORT(e) (e != EAGAIN  e != EWOULDBLOCK  e != EINTR)
+
 /* Forward declaration */
 struct pktio_if_ops;
 
diff --git a/platform/linux-generic/pktio/socket.c 
b/platform/linux-generic/pktio/socket.c
index 45040fd..a96ec90 100644
--- a/platform/linux-generic/pktio/socket.c
+++ b/platform/linux-generic/pktio/socket.c
@@ -373,9 +373,7 @@ static int sock_mmsg_send(pktio_entry_t *pktio_entry,
struct iovec iovecs[ODP_PACKET_SOCKET_MAX_BURST_TX][ODP_BUFFER_MAX_SEG];
int ret;
int sockfd;
-   unsigned i;
-   unsigned sent_msgs = 0;
-   unsigned flags;
+   unsigned n, i;
 
if (odp_unlikely(len  ODP_PACKET_SOCKET_MAX_BURST_TX))
return -1;
@@ -389,17 +387,24 @@ static int sock_mmsg_send(pktio_entry_t *pktio_entry,
iovecs[i]);
}
 
-   flags = MSG_DONTWAIT;
-   for (i = 0; i  len; i += sent_msgs) {
-   ret = sendmmsg(sockfd, msgvec[i], len - i, flags);
-   sent_msgs = ret  0 ? (unsigned)ret : 0;
-   flags = 0;  /* blocking for next rounds */
+   for (i = 0; i  len; ) {
+   ret = sendmmsg(sockfd, msgvec[i], len - i, MSG_DONTWAIT);
+   if (odp_unlikely(ret = -1)) {
+   if (i == 0  SOCK_ERR_REPORT(errno)) {
+   __odp_errno = errno;
+   ODP_ERR(sendmmsg(): %s\n, strerror(errno));
+   return -1;
+   }
+   break;
+   }
+
+   i += ret;
}
 
-   for (i = 0; i  len; i++)
-   odp_packet_free(pkt_table[i]);
+   for (n = 0; n  i; ++n)
+   odp_packet_free(pkt_table[n]);
 
-   return len;
+   return i;
 }
 
 /*
diff --git a/platform/linux-generic/pktio/socket_mmap.c 
b/platform/linux-generic/pktio/socket_mmap.c
index 7d42678..b394c2c 100644
--- a/platform/linux-generic/pktio/socket_mmap.c
+++ b/platform/linux-generic/pktio/socket_mmap.c
@@ -174,49 +174,70 @@ static inline unsigned pkt_mmap_v2_tx(int sock, struct 
ring *ring,
 {
union frame_map ppd;
uint32_t pkt_len;
-   unsigned frame_num, next_frame_num;
+   unsigned first_frame_num, frame_num, frame_count;
int ret;
-   unsigned i = 0;
+   uint8_t *buf;
+   unsigned n, i = 0;
+   unsigned nb_tx = 0;
+   int send_errno;
 
-   frame_num = ring-frame_num;
+   first_frame_num = ring-frame_num;
+   frame_num = first_frame_num;
+   frame_count = ring-rd_num;
 
while (i  len) {
-   if (mmap_tx_kernel_ready(ring-rd[frame_num].iov_base)) {
-   ppd.raw = ring-rd[frame_num].iov_base;
+   ppd.raw = ring-rd[frame_num].iov_base;
+   if (!odp_unlikely(mmap_tx_kernel_ready(ppd.raw)))
+   break;
 
-   next_frame_num = (frame_num + 1) % ring-rd_num;
+   pkt_len = odp_packet_len(pkt_table[i]);
+   ppd.v2-tp_h.tp_snaplen = pkt_len;
+   ppd.v2-tp_h.tp_len = pkt_len;
 
-   pkt_len = odp_packet_len(pkt_table[i]);
-   ppd.v2-tp_h.tp_snaplen = pkt_len;
-   ppd.v2-tp_h.tp_len = pkt_len;
+   buf = (uint8_t *)ppd.raw + TPACKET2_HDRLEN -
+  sizeof(struct sockaddr_ll);
+   odp_packet_copydata_out(pkt_table[i], 0, pkt_len, buf);
 
-   

[lng-odp] [PATCH 3/3] validation: pktio: test for transmit error handling

2015-08-20 Thread Stuart Haslam
Test that transmit errors are handled correctly by attempting to send a
packet larger than the MTU of the interface.

Signed-off-by: Stuart Haslam stuart.has...@linaro.org
---
 test/validation/pktio/pktio.c | 168 +-
 1 file changed, 148 insertions(+), 20 deletions(-)

diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
index be4f940..340d691 100644
--- a/test/validation/pktio/pktio.c
+++ b/test/validation/pktio/pktio.c
@@ -21,6 +21,7 @@
 #define MAX_NUM_IFACES 2
 #define TEST_SEQ_INVALID   ((uint32_t)~0)
 #define TEST_SEQ_MAGIC 0x92749451
+#define TX_BATCH_LEN   4
 
 /** interface names used for testing */
 static const char *iface_name[MAX_NUM_IFACES];
@@ -34,6 +35,7 @@ typedef struct {
odp_pktio_t id;
odp_queue_t outq;
odp_queue_t inq;
+   enum { PKTIN_MODE_RECV, PKTIN_MODE_POLL, PKTIN_MODE_SCHED } mode;
 } pktio_info_t;
 
 /** magic number and sequence at start of UDP payload */
@@ -336,31 +338,40 @@ static odp_event_t queue_deq_wait_time(odp_queue_t queue, 
uint64_t ns)
return ODP_EVENT_INVALID;
 }
 
-static odp_packet_t wait_for_packet(odp_queue_t queue,
+static odp_packet_t wait_for_packet(pktio_info_t *pktio_rx,
uint32_t seq, uint64_t ns)
 {
uint64_t start, now, diff;
odp_event_t ev;
-   odp_packet_t pkt = ODP_PACKET_INVALID;
+   odp_packet_t pkt;
 
start = odp_time_cycles();
 
do {
-   if (queue != ODP_QUEUE_INVALID 
-   odp_queue_type(queue) == ODP_QUEUE_TYPE_POLL)
-   ev = queue_deq_wait_time(queue, ns);
-   else
-   ev  = odp_schedule(NULL, ns);
-
-   if (ev != ODP_EVENT_INVALID) {
-   if (odp_event_type(ev) == ODP_EVENT_PACKET) {
-   pkt = odp_packet_from_event(ev);
-   if (pktio_pkt_seq(pkt) == seq)
-   return pkt;
+   pkt = ODP_PACKET_INVALID;
+
+   if (pktio_rx-mode == PKTIN_MODE_RECV) {
+   odp_pktio_recv(pktio_rx-id, pkt, 1);
+   } else {
+   if (pktio_rx-mode == PKTIN_MODE_POLL)
+   ev = queue_deq_wait_time(pktio_rx-inq, ns);
+   else
+   ev = odp_schedule(NULL,
+ odp_schedule_wait_time(ns));
+
+   if (ev != ODP_EVENT_INVALID) {
+   if (odp_event_type(ev) == ODP_EVENT_PACKET)
+   pkt = odp_packet_from_event(ev);
+   else
+   odp_event_free(ev);
}
+   }
 
-   /* not interested in this event */
-   odp_buffer_free(odp_buffer_from_event(ev));
+   if (pkt != ODP_PACKET_INVALID) {
+   if (pktio_pkt_seq(pkt) == seq)
+   return pkt;
+
+   odp_packet_free(pkt);
}
 
now = odp_time_cycles();
@@ -424,7 +435,7 @@ static void pktio_txrx_multi(pktio_info_t *pktio_a, 
pktio_info_t *pktio_b,
 
/* and wait for them to arrive back */
for (i = 0; i  num_pkts; ++i) {
-   rx_pkt = wait_for_packet(pktio_b-inq, tx_seq[i], ODP_TIME_SEC);
+   rx_pkt = wait_for_packet(pktio_b, tx_seq[i], ODP_TIME_SEC);
 
if (rx_pkt == ODP_PACKET_INVALID)
break;
@@ -454,10 +465,13 @@ static void test_txrx(odp_queue_type_t q_type, int 
num_pkts)
}
create_inq(io-id, q_type);
io-outq = odp_pktio_outq_getdef(io-id);
-   if (q_type == ODP_QUEUE_TYPE_POLL)
+   if (q_type == ODP_QUEUE_TYPE_POLL) {
+   io-mode = PKTIN_MODE_POLL;
io-inq = odp_pktio_inq_getdef(io-id);
-   else
+   } else {
+   io-mode = PKTIN_MODE_SCHED;
io-inq = ODP_QUEUE_INVALID;
+   }
}
 
/* if we have two interfaces then send through one and receive on
@@ -479,7 +493,7 @@ void pktio_test_poll_queue(void)
 
 void pktio_test_poll_multi(void)
 {
-   test_txrx(ODP_QUEUE_TYPE_POLL, 4);
+   test_txrx(ODP_QUEUE_TYPE_POLL, TX_BATCH_LEN);
 }
 
 void pktio_test_sched_queue(void)
@@ -489,7 +503,7 @@ void pktio_test_sched_queue(void)
 
 void pktio_test_sched_multi(void)
 {
-   test_txrx(ODP_QUEUE_TYPE_SCHED, 4);
+   test_txrx(ODP_QUEUE_TYPE_SCHED, TX_BATCH_LEN);
 }
 
 void pktio_test_jumbo(void)
@@ -632,6 +646,118 @@ void pktio_test_inq(void)
CU_ASSERT(odp_pktio_close(pktio) == 0);
 }
 
+static void pktio_test_send_failure(void)
+{
+