On 15/12/15 08:24, Savolainen, Petri (Nokia - FI/Espoo) wrote:
Actually, this patch set was just merged, but we consider these for the next 
set following shortly (today/tomorrow). See replies below.


-----Original Message-----
From: EXT Zoltan Kiss [mailto:zoltan.k...@linaro.org]
Sent: Monday, December 14, 2015 8:26 PM
To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [API-NEXT PATCH v5 2/7] api: pktio: added multiple
pktio input queues

Hi,

I wanted to raise some questions about this, unfortunately it fell off
the radar for a while.

On 26/11/15 08:35, Petri Savolainen wrote:
Added input queue configuration parameters and functions
to setup multiple input queue and hashing. Added also
functions to query the number of queues and queue handles.
Direct receive does use new odp_pktin_queue_t handle type.

Signed-off-by: Petri Savolainen <petri.savolai...@nokia.com>
---
   include/odp/api/packet_io.h                        | 136
+++++++++++++++++++++
   .../include/odp/plat/packet_io_types.h             |   2 +
   2 files changed, 138 insertions(+)

diff --git a/include/odp/api/packet_io.h b/include/odp/api/packet_io.h
index 264fa75..26c9be5 100644
--- a/include/odp/api/packet_io.h
+++ b/include/odp/api/packet_io.h
@@ -19,6 +19,7 @@ extern "C" {
   #endif

   #include <odp/api/packet_io_stats.h>
+#include <odp/api/queue.h>

   /** @defgroup odp_packet_io ODP PACKET IO
    *  Operations on a packet Input/Output interface.
@@ -42,6 +43,11 @@ extern "C" {
    */

   /**
+ * @typedef odp_pktin_queue_t
+ * Direct packet input queue handle

What's the difference between a ODP_PKTIN_MODE_RECV and a
ODP_PKTIN_MODE_POLL queue? Apart from using different functions for
receive?

_RECV means that application uses only odp_pktio_recv() / _recv_queue() to 
directly fetch packet from the interface. _POLL means that application uses 
only poll type queues (odp_queue_t) to receive packets == odp_queue_deq() / 
_deq_multi(). The third type is for scheduled queues (use only odp_schedule() / 
schedule_multi()).

I don't think that's a real difference between _RECV and _POLL type of input mode. You just call a different function to do exactly the same. E.g. ODP-OVS calls odp_pktio_recv() at the moment to _poll_ on the (only) queue of the NIC, and it opens it as ODP_PKTIN_MODE_RECV. With this patch applied, it can call odp_pktio_recv_queue() to do the same (but with several queues), or open the pktio as ODP_PKTIN_MODE_POLL, and then odp_queue_deq[_multi]() on a ODP_PKTIN_MODE_POLL. I don't see any difference, but it's very confusing. Also, based on the current API definition you should be able to call odp_queue_enq() on a queue associated with an interface created with ODP_PKTIN_MODE_POLL or ODP_PKTIN_MODE_SCHED. odp_pktio_in_queues() will return an array of odp_queue_t type queues with ODP_QUEUE_TYPE_POLL or ODP_QUEUE_TYPE_SCHED. That operation doesn't make any sense.



The plan is to delete ODP_QUEUE_TYPE_PKTIN, after which all odp_queue_t for 
packet input are either _POLL or _SCHED type.

I think the main problem is the disconnection between pktio input/output modes and queue types.
Instead of odp_pktio_[input|output]_mode_t we should just store
- whether input and output are enabled or disabled
- if input enabled, the odp_queue_type_t (poll or sched), so we can avoid duplicating poll and sched in ODP_PKTIN_MODE_*
- if output enabled, whether it's normal or TM queue




+ */
+
+/**
    * @def ODP_PKTIO_INVALID
    * Invalid packet IO handle
    */
@@ -85,6 +91,66 @@ typedef enum odp_pktio_output_mode_t {
   } odp_pktio_output_mode_t;

   /**
+ * Packet input hash protocols
+ *
+ * The list of protocol header field combinations, which are included
into
+ * packet input hash calculation.

What happens if a platform doesn't support a particular field to be
hashed? I guess that means the platform has to emulate that bit from
software, right? Maybe worth to indicate that somehow? Or give a way for
the app to query what fields the platform can include in the hash?


We are going to add this type to odp_pktio_capability_t, so that application 
can query hash capabilities. A platform that does not support hashing can set 
all these flags to zero and max_input_queues to one.

Ok.




+ */
+typedef union odp_pktin_hash_proto_t {
+       /** Protocol header fields for hashing */
+       struct {
+               /** IPv4 addresses and UDP port numbers */
+               uint32_t ipv4_udp : 1;
+               /** IPv4 addresses and TCP port numbers */
+               uint32_t ipv4_tcp : 1;
+               /** IPv4 addresses */
+               uint32_t ipv4     : 1;
+               /** IPv6 addresses and UDP port numbers */
+               uint32_t ipv6_udp : 1;
+               /** IPv6 addresses and TCP port numbers */
+               uint32_t ipv6_tcp : 1;
+               /** IPv6 addresses */
+               uint32_t ipv6     : 1;
+       } proto;
+
+       /** All bits of the bit field structure */
+       uint32_t all_bits;
+} odp_pktin_hash_proto_t;
+
+/**
+ * Packet input queue parameters
+ */
+typedef struct odp_pktio_input_queue_param_t {
+       /** Single thread per queue. Enable performance optimization when
each
+         * queue has only single user.
+         * 0: Queue is multi-thread safe
+         * 1: Queue is used by single thread only */
+       odp_bool_t single_user;
+
+       /** Enable flow hashing
+         * 0: Do not hash flows
+         * 1: Hash flows to input queues */
+       odp_bool_t hash_enable;
+
+       /** Protocol field selection for hashing. Multiple protocols can be
+         * selected. */
+       odp_pktin_hash_proto_t hash_proto;
+
+       /** Number of input queues to be created. More than one input queue
+         * require input hashing or classifier setup. Hash_proto is ignored
+         * when hash_enable is zero or num_queues is one. This value must
be

OVS use the hashes even with one queue, it's useful for fast lookup in
flow tables. Is there any reason to automatically disable it?


This describes input queue hashing (hashing of flows into queues).

Let me reword my concerns:
- What happens if num_queues > 1 and hash_enable = 0? What do we know about the distribution of packets between queues? - If num_queues = 1, hash_proto is ignored (no matter what hashe_enable is), so how do we set up hashing? As I said, we still need it.


It does not control the hash value stored into the packet.
It sounds like it does.

It may very well be present also when using a single input queue.
We would need certainty about that.

In DPDK RSS enablement is not tied to have more than 1 RX queue.





+         * between 1 and interface capability. Queue type is defined by the
+         * input mode. */
+       unsigned num_queues;
+
+       /** Queue parameters for creating input queues in
ODP_PKTIN_MODE_POLL
+         * or ODP_PKTIN_MODE_SCHED modes. Scheduler parameters are
considered
+         * only in ODP_PKTIN_MODE_SCHED mode. */
+       odp_queue_param_t queue_param;
+
+} odp_pktio_input_queue_param_t;
+
+/**
    * Packet IO parameters
    *
    * In minimum, user must select input and output modes. Use 0 for
defaults.
@@ -158,6 +224,67 @@ odp_pktio_t odp_pktio_open(const char *dev,
odp_pool_t pool,
   int odp_pktio_capability(odp_pktio_t pktio, odp_pktio_capability_t
*capa);

   /**
+ * Configure packet input queues
+ *
+ * Setup a number of packet input queues and configure those. The
maximum number
+ * of queues is platform dependent and can be queried with
+ * odp_pktio_capability(). Queue handles for input queues can be
requested with
+ * odp_pktio_in_queues() or odp_pktio_pktin_queues() after this call.
All
+ * requested queues are setup on success, no queues are setup on
failure.

Maybe worth to spell out that you can only call this on a stopped
interface, and if it's not the first time since you opened the pktio,
previous queue handles become invalid.



/**
  * Open a packet IO interface
  *
  * An ODP program can open a single packet IO interface per device, attempts
  * to open an already open device will fail, returning ODP_PKTIO_INVALID with
  * errno set. Use odp_pktio_lookup() to obtain a handle to an already open
  * device. Packet IO parameters provide interface level configuration options.
  *
  * This call does not activate packet receive and transmit on the interface.
  * The interface is activated with a call to odp_pktio_start(). If not
  * specified otherwise, any interface level configuration must not be changed
  * when the interface is active (between start and stop calls).


It's documented the other way around: any pktio interface configuration change 
between start and stop is forbidden, if not explicitly allowed.

I'll add a note about each call invalidating old handles.

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

Reply via email to