| Message ID | A400FC85CF2669428A5A081F01B94F531DA1D8F5@DEMUMBX012.nsn-intra.net |
|---|---|
| State | New |
| Headers | show |
On 31 March 2015 at 16:00, Savolainen, Petri (Nokia - FI/Espoo) < petri.savolainen@nokia.com> wrote: > > > > > *From:* ext Bill Fischofer [mailto:bill.fischofer@linaro.org] > *Sent:* Tuesday, March 31, 2015 3:41 PM > > *To:* Savolainen, Petri (Nokia - FI/Espoo) > *Cc:* LNG ODP Mailman List > *Subject:* Re: [lng-odp] [RFC 6/8] api: packet_io: added > odp_pktio_inq_hash_lookup > > > > > > > > On Tue, Mar 31, 2015 at 2:08 AM, Savolainen, Petri (Nokia - FI/Espoo) < > petri.savolainen@nokia.com> wrote: > > > > > > *From:* ext Bill Fischofer [mailto:bill.fischofer@linaro.org] > *Sent:* Monday, March 30, 2015 11:56 PM > *To:* Savolainen, Petri (Nokia - FI/Espoo) > *Cc:* LNG ODP Mailman List > *Subject:* Re: [lng-odp] [RFC 6/8] api: packet_io: added > odp_pktio_inq_hash_lookup > > > > > > > > On Mon, Mar 30, 2015 at 12:23 PM, Petri Savolainen < > petri.savolainen@nokia.com> wrote: > > Hash lookup is used to find the input queue for a packet. This information > is needed e.g. to setup the queue context. > > Signed-off-by: Petri Savolainen <petri.savolainen@nokia.com> > --- > include/odp/api/packet_io.h | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/include/odp/api/packet_io.h b/include/odp/api/packet_io.h > index 99a5d5d..7e84fe1 100644 > --- a/include/odp/api/packet_io.h > +++ b/include/odp/api/packet_io.h > @@ -20,6 +20,7 @@ extern "C" { > > #include <odp/queue.h> > #include <odp/schedule_types.h> > +#include <odp/packet.h> > > /** @defgroup odp_packet_io ODP PACKET IO > * Operations on a packet. > @@ -149,6 +150,19 @@ odp_pktio_t odp_pktio_open(const char *dev, > odp_pool_t pool, > odp_queue_t odp_pktio_inq_create(odp_pktio_t pktio, const char *name); > > /** > + * Packet input queue hash lookup > + * > + * Find the input queue for a packet. > > > > Shouldn't this be "for a pktio" not "for a packet"? The argument is > odp_pktio_t, not odp_packet_t. > > > > Packet is the second argument. User can test with a packet (header fields > set accordingly) which input queue it (packets of that flow) would land on. > > > > Then I'm not sure I understand the intended use of this API or why it has > an odp_pktio_t as one of its input arguments or is named > odp_pktio_inq_hash_lookup(). From what you described I'd expect it to be: > > > > odp_queue_t odp_packet_inq_hash_lookup(odp_packet_t pkt); > > > > Input queue hashing is configured per interface (parameter to > odp_pktio_open()). > Yes but I would rather have hashing and distribution (to queues) *after* the user-configurable classification stage. Keep the abstraction level high and don't invent API's that just map to some specific HW feature (standard NIC tuple-hashing in this case). Alternatively we can perhaps redefine atomicity and ordering to refer to user-defined flows (not queues) where many such flows are handled by the same queue. Thus we wouldn't need hashing and distribution to a range of queues. > > -Petri > > > > > > -Petri > > > > + * > + * @param pktio Packet IO handle > + * @param packet Packet handle > + * > + * @return Input queue handle > + * @retval ODP_QUEUE_INVALID on failure > + */ > +odp_queue_t odp_pktio_inq_hash_lookup(odp_pktio_t pktio, odp_packet_t > packet); > + > +/** > * Number of packet input queues > * > * @param pktio Packet IO handle > -- > 2.3.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 > >
diff --git a/include/odp/api/packet_io.h b/include/odp/api/packet_io.h index 99a5d5d..7e84fe1 100644 --- a/include/odp/api/packet_io.h +++ b/include/odp/api/packet_io.h @@ -20,6 +20,7 @@ extern "C" { #include <odp/queue.h> #include <odp/schedule_types.h> +#include <odp/packet.h> /** @defgroup odp_packet_io ODP PACKET IO * Operations on a packet. @@ -149,6 +150,19 @@ odp_pktio_t odp_pktio_open(const char *dev, odp_pool_t pool, odp_queue_t odp_pktio_inq_create(odp_pktio_t pktio, const char *name); /** + * Packet input queue hash lookup + * + * Find the input queue for a packet. Shouldn't this be "for a pktio" not "for a packet"? The argument is odp_pktio_t, not odp_packet_t. Packet is the second argument. User can test with a packet (header fields set accordingly) which input queue it (packets of that flow) would land on. Then I'm not sure I understand the intended use of this API or why it has an odp_pktio_t as one of its input arguments or is named odp_pktio_inq_hash_lookup(). From what you described I'd expect it to be: odp_queue_t odp_packet_inq_hash_lookup(odp_packet_t pkt); Input queue hashing is configured per interface (parameter to odp_pktio_open()). -Petri