Hi Bill,

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

Reviewed-by: Bogdan Pricope <bogdan.pric...@enea.com>

Thanks,
Bogdan

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

Hi Bogdan,

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

Please review this to see if it meets your needs.

Thanks.

Bill

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

Thank you for trying to push this further.

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

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

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

Bogdan


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

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


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


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

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

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



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

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


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


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

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


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

Hi,

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

ifnet = ofp_get_ifnet_pktio(pktio);

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

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

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


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

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


uint32_t odp_pktio_to_id(odp_pktio_t pktio);

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

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


odp_pktio_t odp_pktio_from_id(uint32_t id);

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


Returns the pktio handle associated with an input ID value (or 
ODP_PKTIO_INVALID if out of range)

-- Ola



vs with context pointer in best case:
pktio_id = pkt_hdr->pktio_id;
ifnet = odp_pktio_table[pktio_id].ctx_ptr;

In worst case there would be additional error checks, pointer reference, etc 
for converting pktio handle into context pointer. Not a big gain or loose, I’m 
thinking if pktio ID would be needed any way.

-Petri

I guess we should use an array stored in a shared memory and use pktio_id as 
index to get interface. I wonder if there are more/less cache misses like this 
than accessing a context pointer from pktio?

I don’t know when/where to connect. Can you send me the details?

Regards,
Bogdan

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

These are good points.  Can we get OFP participation in Monday's ARCH call to 
discuss this?  Alternately I'll put it on the agenda for Tuesday's public call.

On Fri, Apr 8, 2016 at 6:57 AM, Savolainen, Petri (Nokia - FI/Espoo) 
<petri.savolai...@nokia.com<mailto:petri.savolai...@nokia.com>> wrote:
I wonder what would be the HW offload here? Scheduler would prefetch queue 
context for odp_queue_t queues which may be millions, but pktio interfaces (or 
pktin/pktout queues) are few. Application could store/load/prefetch the context 
also itself before or after it polls an interface.

I know the request comes from OFP which used to store interface struct pointer 
into pktio default queue context pointer. This could be solved also in other 
ways, e.g. introduce a pktio ID (0...max num pktios) which would be stored into 
packet (which is many time there anyway) and avoid two lookups inside 
implementation (ID->pktio handle ->context pointer).

-Petri


> -----Original Message-----
> From: lng-odp 
> [mailto:lng-odp-boun...@lists.linaro.org<mailto:lng-odp-boun...@lists.linaro.org>]
>  On Behalf Of EXT
> Bill Fischofer
> Sent: Friday, April 08, 2016 12:22 AM
> To: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org>
> Subject: [lng-odp] [API-NEXT PATCH 1/3] api: pktio: add user context
> support for interfaces
>
> Add the APIs odp_pktio_context() and odp_pktio_context_set() to permit
> applications to associate contexts with PktIO interfaces.
>
> Suggested-by: Bogdan Pricope 
> <bogdan.pric...@enea.com<mailto:bogdan.pric...@enea.com>>
> Signed-off-by: Bill Fischofer 
> <bill.fischo...@linaro.org<mailto:bill.fischo...@linaro.org>>
> ---
>  include/odp/api/spec/packet_io.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/include/odp/api/spec/packet_io.h
> b/include/odp/api/spec/packet_io.h
> index 466cab6..4880432 100644
> --- a/include/odp/api/spec/packet_io.h
> +++ b/include/odp/api/spec/packet_io.h
> @@ -908,6 +908,27 @@ int odp_pktio_skip_set(odp_pktio_t pktio, uint32_t
> offset);
>  int odp_pktio_headroom_set(odp_pktio_t pktio, uint32_t headroom);
>
>  /**
> + * Get pktio user context pointer
> + *
> + * @param pktio   Pktio handle
> + *
> + * @return        Pointer to the pktio user context
> + * @retval NULL   on failure
> + */
> +void *odp_pktio_context(odp_pktio_t pktio);
> +
> +/**
> + * Set pktio user context pointer
> + *
> + * @param pktio   Pktio handle
> + * @param context Address of the pktio context
> + *
> + * @retval 0      on success
> + * @retval <0     on failure
> + */
> +int odp_pktio_context_set(odp_pktio_t pktio, void *context);
> +
> +/**
>   * Get printable value for an odp_pktio_t
>   *
>   * @param pktio   odp_pktio_t handle to be printed
> --
> 2.5.0
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org>
> https://lists.linaro.org/mailman/listinfo/lng-odp


_______________________________________________
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

Reply via email to